Deprecate mutable API of PrimitiveSet.
Some package-private methods that are not used anymore in the package can be removed completely:
- newPrimitiveSetWithAnnotations: Now annotations can only be set with the builder API.
- makeImmutable: Immutable objects can now only be created with the builder API. And we can make the isMutable variable final.
Also, annotate getPrimary with @Nullable, and add a test that shows that indeed null is returned when no primary is set.
PiperOrigin-RevId: 455380275
Change-Id: If208f869b0418e14c653001f5f0263c0b136db6d
diff --git a/src/main/java/com/google/crypto/tink/BUILD.bazel b/src/main/java/com/google/crypto/tink/BUILD.bazel
index 254e4c5..30d7107 100644
--- a/src/main/java/com/google/crypto/tink/BUILD.bazel
+++ b/src/main/java/com/google/crypto/tink/BUILD.bazel
@@ -464,6 +464,7 @@
"//src/main/java/com/google/crypto/tink/annotations:alpha",
"//src/main/java/com/google/crypto/tink/monitoring:monitoring_annotations",
"//src/main/java/com/google/crypto/tink/subtle:hex",
+ "@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_errorprone_error_prone_annotations",
],
)
@@ -478,6 +479,7 @@
"//src/main/java/com/google/crypto/tink/annotations:alpha-android",
"//src/main/java/com/google/crypto/tink/monitoring:monitoring_annotations-android",
"//src/main/java/com/google/crypto/tink/subtle:hex-android",
+ "@maven//:com_google_code_findbugs_jsr305",
"@maven//:com_google_errorprone_error_prone_annotations",
],
)
diff --git a/src/main/java/com/google/crypto/tink/PrimitiveSet.java b/src/main/java/com/google/crypto/tink/PrimitiveSet.java
index 1ab1bdd..9a8595b 100644
--- a/src/main/java/com/google/crypto/tink/PrimitiveSet.java
+++ b/src/main/java/com/google/crypto/tink/PrimitiveSet.java
@@ -31,6 +31,7 @@
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import javax.annotation.Nullable;
/**
* A container class for a set of primitives -- implementations of cryptographic primitives offered
@@ -168,6 +169,7 @@
}
/** @return the entry with the primary primitive. */
+ @Nullable
public Entry<P> getPrimary() {
return primary;
}
@@ -207,8 +209,9 @@
private Entry<P> primary;
private final Class<P> primitiveClass;
private final MonitoringAnnotations annotations;
- private boolean isMutable;
+ private final boolean isMutable;
+ @Deprecated
private PrimitiveSet(Class<P> primitiveClass) {
this.primitives = new ConcurrentHashMap<>();
this.primitiveClass = primitiveClass;
@@ -216,13 +219,6 @@
this.isMutable = true;
}
- private PrimitiveSet(MonitoringAnnotations annotations, Class<P> primitiveClass) {
- this.primitives = new ConcurrentHashMap<>();
- this.primitiveClass = primitiveClass;
- this.annotations = annotations;
- this.isMutable = true;
- }
-
/** Creates an immutable PrimitiveSet. It is used by the Builder.*/
private PrimitiveSet(ConcurrentMap<Prefix, List<Entry<P>>> primitives,
Entry<P> primary, MonitoringAnnotations annotations, Class<P> primitiveClass) {
@@ -233,30 +229,22 @@
this.isMutable = false;
}
+ /**
+ * Creates a new mutable PrimitiveSet.
+ *
+ * @deprecated use {@link Builder} instead.
+ */
+ @Deprecated
public static <P> PrimitiveSet<P> newPrimitiveSet(Class<P> primitiveClass) {
return new PrimitiveSet<P>(primitiveClass);
}
- // Only used by KeysetHandle.
- static <P> PrimitiveSet<P> newPrimitiveSetWithAnnotations(
- MonitoringAnnotations annotations, Class<P> primitiveClass) {
- return new PrimitiveSet<P>(annotations, primitiveClass);
- }
-
- /**
- * Makes the object immutable.
- *
- * <p>Calls to the methods {@link setPrimary} or {@link addPrimitive} after calling this method
- * will throw an {@code IllegalStateException}.
- */
- void makeImmutable() {
- this.isMutable = false;
- }
-
/** Sets given Entry {@code primary} as the primary one.
*
- * @throws IllegalStateException if {@link makeImmutable} has been called before.
+ * @throws IllegalStateException if object has been created by the {@link Builder}.
+ * @deprecated use {@link Builder.addPrimaryPrimitive} instead.
*/
+ @Deprecated
public void setPrimary(final Entry<P> primary) {
if (!isMutable) {
throw new IllegalStateException("setPrimary cannot be called on an immutable primitive set");
@@ -280,8 +268,10 @@
*
* @return the added {@link Entry}
*
- * @throws IllegalStateException if {@link makeImmutable} has been called before.
+ * @throws IllegalStateException if object has been created by the {@link Builder}.
+ * @deprecated use {@link Builder.addPrimitive} or {@link Builder.addPrimaryPrimitive} instead.
*/
+ @Deprecated
public Entry<P> addPrimitive(final P primitive, Keyset.Key key)
throws GeneralSecurityException {
if (!isMutable) {
diff --git a/src/test/java/com/google/crypto/tink/PrimitiveSetTest.java b/src/test/java/com/google/crypto/tink/PrimitiveSetTest.java
index 047181d..ac05dc3 100644
--- a/src/test/java/com/google/crypto/tink/PrimitiveSetTest.java
+++ b/src/test/java/com/google/crypto/tink/PrimitiveSetTest.java
@@ -70,7 +70,7 @@
}
@Test
- public void testBasicFunctionality() throws Exception {
+ public void testBasicFunctionalityWithDeprecatedMutableInterface() throws Exception {
PrimitiveSet<Mac> pset = PrimitiveSet.newPrimitiveSet(Mac.class);
Key key1 =
Key.newBuilder()
@@ -92,24 +92,7 @@
.setStatus(KeyStatusType.ENABLED)
.setOutputPrefixType(OutputPrefixType.LEGACY)
.build();
- PrimitiveSet.Entry<Mac> lastEntry = pset.addPrimitive(new DummyMac1(), key3);
-
- // After the primitive set has been built, it is preferable to call makeImmutable() to make it
- // immutable.
- pset.makeImmutable();
-
- // Check that setPrimary and addPrimitive now throw an Exception.
- assertThrows(IllegalStateException.class, () -> pset.setPrimary(lastEntry));
- assertThrows(
- IllegalStateException.class,
- () ->
- pset.addPrimitive(
- new DummyMac1(),
- Key.newBuilder()
- .setKeyId(4)
- .setStatus(KeyStatusType.ENABLED)
- .setOutputPrefixType(OutputPrefixType.TINK)
- .build()));
+ pset.addPrimitive(new DummyMac1(), key3);
assertThat(pset.getAll()).hasSize(3);
@@ -261,6 +244,21 @@
}
@Test
+ public void testNoPrimary_getPrimaryReturnsNull() throws Exception {
+ Key key =
+ Key.newBuilder()
+ .setKeyId(1)
+ .setStatus(KeyStatusType.ENABLED)
+ .setOutputPrefixType(OutputPrefixType.TINK)
+ .build();
+ PrimitiveSet<Mac> pset =
+ PrimitiveSet.newBuilder(Mac.class)
+ .addPrimitive(new DummyMac1(), key)
+ .build();
+ assertThat(pset.getPrimary()).isNull();
+ }
+
+ @Test
public void testEntryGetKeyFormatToString() throws Exception {
PrimitiveSet<Mac> pset = PrimitiveSet.newPrimitiveSet(Mac.class);
Key key1 =
@@ -286,15 +284,11 @@
public void testWithAnnotations() throws Exception {
MonitoringAnnotations annotations =
MonitoringAnnotations.newBuilder().add("name", "value").build();
- PrimitiveSet<Mac> pset = PrimitiveSet.newPrimitiveSetWithAnnotations(annotations, Mac.class);
+ PrimitiveSet<Mac> pset = PrimitiveSet.newBuilder(Mac.class).setAnnotations(annotations).build();
HashMap<String, String> expected = new HashMap<>();
expected.put("name", "value");
assertThat(pset.getAnnotations().toMap()).containsExactlyEntriesIn(expected);
-
- PrimitiveSet<Mac> pset2 =
- PrimitiveSet.newBuilder(Mac.class).setAnnotations(annotations).build();
- assertThat(pset2.getAnnotations().toMap()).containsExactlyEntriesIn(expected);
}
@Test
