From 3b8d137a113376d7dac9010b9207d435df2622f7 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 17 Nov 2022 16:55:46 -0500 Subject: [PATCH] docs: annotate all Option factory methods with their Nullability bounds (#1775) Update all option factory methods to explicitly state their Nullability bounds. All methods which take non-primitive parameters are @NonNull, and will be published in the javadocs. Add new tests to explicitly verify null argument validation. All factory methods now check their args for nullness, instead of depending on implicit checking. --- .../java/com/google/cloud/storage/Blob.java | 7 +- .../java/com/google/cloud/storage/Bucket.java | 25 +-- .../com/google/cloud/storage/BucketInfo.java | 4 +- .../com/google/cloud/storage/Storage.java | 88 +++++----- .../com/google/cloud/storage/UnifiedOpts.java | 114 ++++++++----- .../google/cloud/storage/StorageFixture.java | 2 +- .../google/cloud/storage/UnifiedOptsTest.java | 160 ++++++++++++++++++ 7 files changed, 300 insertions(+), 100 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java index 55c036962..c12962d3f 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Blob.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; +import org.checkerframework.checker.nullness.qual.NonNull; /** * An object in Google Cloud Storage. A {@code Blob} object includes the {@code BlobId} instance, @@ -128,7 +129,7 @@ public static BlobSourceOption metagenerationNotMatch() { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption decryptionKey(Key key) { + public static BlobSourceOption decryptionKey(@NonNull Key key) { return new BlobSourceOption(UnifiedOpts.decryptionKey(key)); } @@ -139,7 +140,7 @@ public static BlobSourceOption decryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption decryptionKey(String key) { + public static BlobSourceOption decryptionKey(@NonNull String key) { return new BlobSourceOption(UnifiedOpts.decryptionKey(key)); } @@ -148,7 +149,7 @@ public static BlobSourceOption decryptionKey(String key) { * bucket has requester_pays flag enabled. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption userProject(String userProject) { + public static BlobSourceOption userProject(@NonNull String userProject) { return new BlobSourceOption(UnifiedOpts.userProject(userProject)); } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java index cbfbf5251..86aaeed6b 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java @@ -44,6 +44,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import org.checkerframework.checker.nullness.qual.NonNull; /** * A Google cloud storage bucket. @@ -93,7 +94,7 @@ public static BucketSourceOption metagenerationNotMatch() { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketSourceOption userProject(String userProject) { + public static BucketSourceOption userProject(@NonNull String userProject) { return new BucketSourceOption(UnifiedOpts.userProject(userProject)); } @@ -142,7 +143,7 @@ private BlobTargetOption(ObjectTargetOpt opt) { /** Returns an option for specifying blob's predefined ACL configuration. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption predefinedAcl(Storage.PredefinedAcl acl) { + public static BlobTargetOption predefinedAcl(Storage.@NonNull PredefinedAcl acl) { return new BlobTargetOption(UnifiedOpts.predefinedAcl(acl)); } @@ -201,7 +202,7 @@ public static BlobTargetOption metagenerationNotMatch(long metageneration) { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption encryptionKey(Key key) { + public static BlobTargetOption encryptionKey(@NonNull Key key) { return new BlobTargetOption(UnifiedOpts.encryptionKey(key)); } @@ -212,7 +213,7 @@ public static BlobTargetOption encryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption encryptionKey(String key) { + public static BlobTargetOption encryptionKey(@NonNull String key) { return new BlobTargetOption(UnifiedOpts.encryptionKey(key)); } @@ -222,7 +223,7 @@ public static BlobTargetOption encryptionKey(String key) { * @param kmsKeyName the KMS key resource id */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption kmsKeyName(String kmsKeyName) { + public static BlobTargetOption kmsKeyName(@NonNull String kmsKeyName) { return new BlobTargetOption(UnifiedOpts.kmsKeyName(kmsKeyName)); } @@ -231,7 +232,7 @@ public static BlobTargetOption kmsKeyName(String kmsKeyName) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption userProject(String userProject) { + public static BlobTargetOption userProject(@NonNull String userProject) { return new BlobTargetOption(UnifiedOpts.userProject(userProject)); } @@ -263,7 +264,7 @@ private BlobWriteOption(ObjectTargetOpt opt) { /** Returns an option for specifying blob's predefined ACL configuration. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption predefinedAcl(Storage.PredefinedAcl acl) { + public static BlobWriteOption predefinedAcl(Storage.@NonNull PredefinedAcl acl) { return new BlobWriteOption(UnifiedOpts.predefinedAcl(acl)); } @@ -322,7 +323,7 @@ public static BlobWriteOption metagenerationNotMatch(long metageneration) { * fail if blobs' data MD5 hash does not match the provided value. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption md5Match(String md5) { + public static BlobWriteOption md5Match(@NonNull String md5) { return new BlobWriteOption(UnifiedOpts.md5Match(md5)); } @@ -331,7 +332,7 @@ public static BlobWriteOption md5Match(String md5) { * will fail if blobs' data CRC32C checksum does not match the provided value. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption crc32cMatch(String crc32c) { + public static BlobWriteOption crc32cMatch(@NonNull String crc32c) { return new BlobWriteOption(UnifiedOpts.crc32cMatch(crc32c)); } @@ -340,7 +341,7 @@ public static BlobWriteOption crc32cMatch(String crc32c) { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption encryptionKey(Key key) { + public static BlobWriteOption encryptionKey(@NonNull Key key) { return new BlobWriteOption(UnifiedOpts.encryptionKey(key)); } @@ -351,7 +352,7 @@ public static BlobWriteOption encryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption encryptionKey(String key) { + public static BlobWriteOption encryptionKey(@NonNull String key) { return new BlobWriteOption(UnifiedOpts.encryptionKey(key)); } @@ -360,7 +361,7 @@ public static BlobWriteOption encryptionKey(String key) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption userProject(String userProject) { + public static BlobWriteOption userProject(@NonNull String userProject) { return new BlobWriteOption(UnifiedOpts.userProject(userProject)); } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java index 8857f55d3..2a2c7256a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java @@ -1061,7 +1061,7 @@ public static DeleteLifecycleAction newDeleteAction() { * @param storageClass The new storage class to use when conditions are met for this action. */ public static SetStorageClassLifecycleAction newSetStorageClassAction( - StorageClass storageClass) { + @NonNull StorageClass storageClass) { return new SetStorageClassLifecycleAction(storageClass); } @@ -1080,7 +1080,7 @@ public static LifecycleAction newAbortIncompleteMPUploadAction() { * generally not be used, instead use the supported actions, and upgrade the library if necessary * to get new supported actions. */ - public static LifecycleAction newLifecycleAction(String actionType) { + public static LifecycleAction newLifecycleAction(@NonNull String actionType) { return new LifecycleAction(actionType); } } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java index 1d416cea4..180c44d35 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.Objects.requireNonNull; import com.google.api.core.InternalExtensionOnly; import com.google.api.gax.paging.Page; @@ -65,6 +66,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; +import org.checkerframework.checker.nullness.qual.NonNull; /** * An interface for Google Cloud Storage. @@ -306,13 +308,13 @@ private BucketTargetOption(BucketTargetOpt opt) { /** Returns an option for specifying bucket's predefined ACL configuration. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketTargetOption predefinedAcl(PredefinedAcl acl) { + public static BucketTargetOption predefinedAcl(@NonNull PredefinedAcl acl) { return new BucketTargetOption(UnifiedOpts.predefinedAcl(acl)); } /** Returns an option for specifying bucket's default ACL configuration for blobs. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketTargetOption predefinedDefaultObjectAcl(PredefinedAcl acl) { + public static BucketTargetOption predefinedDefaultObjectAcl(@NonNull PredefinedAcl acl) { return new BucketTargetOption(UnifiedOpts.predefinedDefaultObjectAcl(acl)); } @@ -339,7 +341,7 @@ public static BucketTargetOption metagenerationNotMatch() { * `requester_pays` flag enabled to assign operation costs. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketTargetOption userProject(String userProject) { + public static BucketTargetOption userProject(@NonNull String userProject) { return new BucketTargetOption(UnifiedOpts.userProject(userProject)); } @@ -352,7 +354,7 @@ public static BucketTargetOption userProject(String userProject) { * patch */ @TransportCompatibility({Transport.HTTP}) - public static BucketTargetOption projection(String projection) { + public static BucketTargetOption projection(@NonNull String projection) { return new BucketTargetOption(UnifiedOpts.projection(projection)); } } @@ -389,7 +391,7 @@ public static BucketSourceOption metagenerationNotMatch(long metageneration) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketSourceOption userProject(String userProject) { + public static BucketSourceOption userProject(@NonNull String userProject) { return new BucketSourceOption(UnifiedOpts.userProject(userProject)); } @@ -411,7 +413,7 @@ private ListHmacKeysOption(HmacKeyListOpt opt) { * keys for all accounts will be listed. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static ListHmacKeysOption serviceAccount(ServiceAccount serviceAccount) { + public static ListHmacKeysOption serviceAccount(@NonNull ServiceAccount serviceAccount) { return new ListHmacKeysOption(UnifiedOpts.serviceAccount(serviceAccount)); } @@ -423,7 +425,7 @@ public static ListHmacKeysOption maxResults(long pageSize) { /** Returns an option to specify the page token from which to start listing HMAC keys. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static ListHmacKeysOption pageToken(String pageToken) { + public static ListHmacKeysOption pageToken(@NonNull String pageToken) { return new ListHmacKeysOption(UnifiedOpts.pageToken(pageToken)); } @@ -441,7 +443,7 @@ public static ListHmacKeysOption showDeletedKeys(boolean showDeletedKeys) { * Requester Pays buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static ListHmacKeysOption userProject(String userProject) { + public static ListHmacKeysOption userProject(@NonNull String userProject) { return new ListHmacKeysOption(UnifiedOpts.userProject(userProject)); } @@ -450,7 +452,7 @@ public static ListHmacKeysOption userProject(String userProject) { * Application Default Credentials. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static ListHmacKeysOption projectId(String projectId) { + public static ListHmacKeysOption projectId(@NonNull String projectId) { return new ListHmacKeysOption(UnifiedOpts.projectId(projectId)); } } @@ -467,7 +469,7 @@ private CreateHmacKeyOption(HmacKeyTargetOpt opt) { * Requester Pays buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static CreateHmacKeyOption userProject(String userProject) { + public static CreateHmacKeyOption userProject(@NonNull String userProject) { return new CreateHmacKeyOption(UnifiedOpts.userProject(userProject)); } @@ -476,7 +478,7 @@ public static CreateHmacKeyOption userProject(String userProject) { * Application Default Credentials. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static CreateHmacKeyOption projectId(String projectId) { + public static CreateHmacKeyOption projectId(@NonNull String projectId) { return new CreateHmacKeyOption(UnifiedOpts.projectId(projectId)); } } @@ -492,7 +494,7 @@ private GetHmacKeyOption(HmacKeySourceOpt opt) { * Requester Pays buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static GetHmacKeyOption userProject(String userProject) { + public static GetHmacKeyOption userProject(@NonNull String userProject) { return new GetHmacKeyOption(UnifiedOpts.userProject(userProject)); } @@ -501,7 +503,7 @@ public static GetHmacKeyOption userProject(String userProject) { * Application Default Credentials. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static GetHmacKeyOption projectId(String projectId) { + public static GetHmacKeyOption projectId(@NonNull String projectId) { return new GetHmacKeyOption(UnifiedOpts.projectId(projectId)); } } @@ -517,7 +519,7 @@ private DeleteHmacKeyOption(HmacKeyTargetOpt opt) { * Requester Pays buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static DeleteHmacKeyOption userProject(String userProject) { + public static DeleteHmacKeyOption userProject(@NonNull String userProject) { return new DeleteHmacKeyOption(UnifiedOpts.userProject(userProject)); } } @@ -533,7 +535,7 @@ private UpdateHmacKeyOption(HmacKeyTargetOpt opt) { * Requester Pays buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static UpdateHmacKeyOption userProject(String userProject) { + public static UpdateHmacKeyOption userProject(@NonNull String userProject) { return new UpdateHmacKeyOption(UnifiedOpts.userProject(userProject)); } } @@ -570,7 +572,7 @@ public static BucketGetOption metagenerationNotMatch(long metageneration) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketGetOption userProject(String userProject) { + public static BucketGetOption userProject(@NonNull String userProject) { return new BucketGetOption(UnifiedOpts.userProject(userProject)); } @@ -582,6 +584,7 @@ public static BucketGetOption userProject(String userProject) { */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) public static BucketGetOption fields(BucketField... fields) { + requireNonNull(fields, "fields must be non null"); ImmutableSet set = ImmutableSet.builder() .addAll(BucketField.REQUIRED_FIELDS) @@ -602,7 +605,7 @@ class BlobTargetOption extends Option { /** Returns an option for specifying blob's predefined ACL configuration. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption predefinedAcl(PredefinedAcl acl) { + public static BlobTargetOption predefinedAcl(@NonNull PredefinedAcl acl) { return new BlobTargetOption(UnifiedOpts.predefinedAcl(acl)); } @@ -674,7 +677,7 @@ public static BlobTargetOption detectContentType() { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption encryptionKey(Key key) { + public static BlobTargetOption encryptionKey(@NonNull Key key) { return new BlobTargetOption(UnifiedOpts.encryptionKey(key)); } @@ -683,7 +686,7 @@ public static BlobTargetOption encryptionKey(Key key) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption userProject(String userProject) { + public static BlobTargetOption userProject(@NonNull String userProject) { return new BlobTargetOption(UnifiedOpts.userProject(userProject)); } @@ -694,13 +697,13 @@ public static BlobTargetOption userProject(String userProject) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption encryptionKey(String key) { + public static BlobTargetOption encryptionKey(@NonNull String key) { return new BlobTargetOption(UnifiedOpts.encryptionKey(key)); } /** Returns an option to set a customer-managed key for server-side encryption of the blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobTargetOption kmsKeyName(String kmsKeyName) { + public static BlobTargetOption kmsKeyName(@NonNull String kmsKeyName) { return new BlobTargetOption(UnifiedOpts.kmsKeyName(kmsKeyName)); } } @@ -716,7 +719,7 @@ class BlobWriteOption extends OptionShim implements Serializabl /** Returns an option for specifying blob's predefined ACL configuration. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption predefinedAcl(PredefinedAcl acl) { + public static BlobWriteOption predefinedAcl(@NonNull PredefinedAcl acl) { return new BlobWriteOption(UnifiedOpts.predefinedAcl(acl)); } @@ -790,7 +793,7 @@ public static BlobWriteOption crc32cMatch() { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption encryptionKey(Key key) { + public static BlobWriteOption encryptionKey(@NonNull Key key) { return new BlobWriteOption(UnifiedOpts.encryptionKey(key)); } @@ -801,7 +804,7 @@ public static BlobWriteOption encryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption encryptionKey(String key) { + public static BlobWriteOption encryptionKey(@NonNull String key) { return new BlobWriteOption(UnifiedOpts.encryptionKey(key)); } @@ -811,7 +814,7 @@ public static BlobWriteOption encryptionKey(String key) { * @param kmsKeyName the KMS key resource id */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption kmsKeyName(String kmsKeyName) { + public static BlobWriteOption kmsKeyName(@NonNull String kmsKeyName) { return new BlobWriteOption(UnifiedOpts.kmsKeyName(kmsKeyName)); } @@ -820,7 +823,7 @@ public static BlobWriteOption kmsKeyName(String kmsKeyName) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobWriteOption userProject(String userProject) { + public static BlobWriteOption userProject(@NonNull String userProject) { return new BlobWriteOption(UnifiedOpts.userProject(userProject)); } @@ -925,7 +928,7 @@ public static BlobSourceOption metagenerationNotMatch(long metageneration) { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption decryptionKey(Key key) { + public static BlobSourceOption decryptionKey(@NonNull Key key) { return new BlobSourceOption(UnifiedOpts.decryptionKey(key)); } @@ -936,7 +939,7 @@ public static BlobSourceOption decryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption decryptionKey(String key) { + public static BlobSourceOption decryptionKey(@NonNull String key) { return new BlobSourceOption(UnifiedOpts.decryptionKey(key)); } @@ -945,7 +948,7 @@ public static BlobSourceOption decryptionKey(String key) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobSourceOption userProject(String userProject) { + public static BlobSourceOption userProject(@NonNull String userProject) { return new BlobSourceOption(UnifiedOpts.userProject(userProject)); } @@ -1041,6 +1044,7 @@ public static BlobGetOption metagenerationNotMatch(long metageneration) { */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) public static BlobGetOption fields(BlobField... fields) { + requireNonNull(fields, "fields must be non null"); ImmutableSet set = ImmutableSet.builder().addAll(BlobField.REQUIRED_FIELDS).add(fields).build(); return new BlobGetOption(UnifiedOpts.fields(set)); @@ -1051,7 +1055,7 @@ public static BlobGetOption fields(BlobField... fields) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobGetOption userProject(String userProject) { + public static BlobGetOption userProject(@NonNull String userProject) { return new BlobGetOption(UnifiedOpts.userProject(userProject)); } @@ -1060,7 +1064,7 @@ public static BlobGetOption userProject(String userProject) { * blob. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobGetOption decryptionKey(Key key) { + public static BlobGetOption decryptionKey(@NonNull Key key) { return new BlobGetOption(UnifiedOpts.decryptionKey(key)); } @@ -1071,7 +1075,7 @@ public static BlobGetOption decryptionKey(Key key) { * @param key the AES256 encoded in base64 */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobGetOption decryptionKey(String key) { + public static BlobGetOption decryptionKey(@NonNull String key) { return new BlobGetOption(UnifiedOpts.decryptionKey(key)); } @@ -1103,7 +1107,7 @@ public static BucketListOption pageSize(long pageSize) { /** Returns an option to specify the page token from which to start listing buckets. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketListOption pageToken(String pageToken) { + public static BucketListOption pageToken(@NonNull String pageToken) { return new BucketListOption(UnifiedOpts.pageToken(pageToken)); } @@ -1112,7 +1116,7 @@ public static BucketListOption pageToken(String pageToken) { * prefix. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketListOption prefix(String prefix) { + public static BucketListOption prefix(@NonNull String prefix) { return new BucketListOption(UnifiedOpts.prefix(prefix)); } @@ -1121,7 +1125,7 @@ public static BucketListOption prefix(String prefix) { * with 'requester_pays' flag. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BucketListOption userProject(String userProject) { + public static BucketListOption userProject(@NonNull String userProject) { return new BucketListOption(UnifiedOpts.userProject(userProject)); } @@ -1133,6 +1137,7 @@ public static BucketListOption userProject(String userProject) { */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) public static BucketListOption fields(BucketField... fields) { + requireNonNull(fields, "fields must be non null"); ImmutableSet set = Streams.concat( Stream.of(NamedField.literal("nextPageToken")), @@ -1160,7 +1165,7 @@ public static BlobListOption pageSize(long pageSize) { /** Returns an option to specify the page token from which to start listing blobs. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption pageToken(String pageToken) { + public static BlobListOption pageToken(@NonNull String pageToken) { return new BlobListOption(UnifiedOpts.pageToken(pageToken)); } @@ -1169,7 +1174,7 @@ public static BlobListOption pageToken(String pageToken) { * prefix. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption prefix(String prefix) { + public static BlobListOption prefix(@NonNull String prefix) { return new BlobListOption(UnifiedOpts.prefix(prefix)); } @@ -1195,7 +1200,7 @@ public static BlobListOption currentDirectory() { * as well. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption delimiter(String delimiter) { + public static BlobListOption delimiter(@NonNull String delimiter) { return new BlobListOption(UnifiedOpts.delimiter(delimiter)); } @@ -1207,7 +1212,7 @@ public static BlobListOption delimiter(String delimiter) { * @param startOffset startOffset to filter the results */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption startOffset(String startOffset) { + public static BlobListOption startOffset(@NonNull String startOffset) { return new BlobListOption(UnifiedOpts.startOffset(startOffset)); } @@ -1219,7 +1224,7 @@ public static BlobListOption startOffset(String startOffset) { * @param endOffset endOffset to filter the results */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption endOffset(String endOffset) { + public static BlobListOption endOffset(@NonNull String endOffset) { return new BlobListOption(UnifiedOpts.endOffset(endOffset)); } @@ -1230,7 +1235,7 @@ public static BlobListOption endOffset(String endOffset) { * @param userProject projectId of the billing user project. */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) - public static BlobListOption userProject(String userProject) { + public static BlobListOption userProject(@NonNull String userProject) { return new BlobListOption(UnifiedOpts.userProject(userProject)); } @@ -1252,6 +1257,7 @@ public static BlobListOption versions(boolean versions) { */ @TransportCompatibility({Transport.HTTP, Transport.GRPC}) public static BlobListOption fields(BlobField... fields) { + requireNonNull(fields, "fields must be non null"); ImmutableSet set = Streams.concat( Stream.of(NamedField.literal("nextPageToken"), NamedField.literal("prefixes")), diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java index 3a237ec0c..934c2c298 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/UnifiedOpts.java @@ -19,6 +19,7 @@ import static com.google.cloud.storage.Utils.projectNameCodec; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Predicates.not; +import static java.util.Objects.requireNonNull; import com.google.api.gax.grpc.GrpcCallContext; import com.google.cloud.storage.Conversions.Decoder; @@ -72,6 +73,7 @@ import java.util.stream.Stream; import javax.crypto.spec.SecretKeySpec; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.RequiresNonNull; /** * The set of all "Options" we currently support for per-call parameters. @@ -82,6 +84,7 @@ * string parameters for JSON. In the case of gRPC, sometimes the parameters are in the specific * request message or in grpc metadata. */ +@SuppressWarnings({"deprecation", "DeprecatedIsStillUsed"}) final class UnifiedOpts { /** Base interface type for each of the new options we're supporting. */ @@ -335,24 +338,32 @@ static Crc32cMatch crc32cMatch(int i) { return new Crc32cMatch(i); } - static Crc32cMatch crc32cMatch(String i) { - return new Crc32cMatch(Utils.crc32cCodec.decode(i)); + static Crc32cMatch crc32cMatch(@NonNull String crc32c) { + requireNonNull(crc32c, "crc32c must be non null"); + return new Crc32cMatch(Utils.crc32cCodec.decode(crc32c)); } static Delimiter currentDirectory() { return new Delimiter("/"); } - static DecryptionKey decryptionKey(String s) { - return new DecryptionKey(new SecretKeySpec(BaseEncoding.base64().decode(s), "AES256")); + static DecryptionKey decryptionKey(@NonNull String decryptionKey) { + requireNonNull(decryptionKey, "decryptionKey must be non null"); + return new DecryptionKey( + new SecretKeySpec(BaseEncoding.base64().decode(decryptionKey), "AES256")); } - static DecryptionKey decryptionKey(Key k) { - return new DecryptionKey(k); + @RequiresNonNull({"decryptionKey", "#1.getEncoded()", "#1.getAlgorithm()"}) + static DecryptionKey decryptionKey(@NonNull Key decryptionKey) { + requireNonNull(decryptionKey, "decryptionKey must be non null"); + requireNonNull(decryptionKey.getEncoded(), "decryptionKey.getEncoded() must be non null"); + requireNonNull(decryptionKey.getAlgorithm(), "decryptionKey.getAlgorithm() must be non null"); + return new DecryptionKey(decryptionKey); } - static Delimiter delimiter(String s) { - return new Delimiter(s); + static Delimiter delimiter(@NonNull String delimiter) { + requireNonNull(delimiter, "delimiter must be non null"); + return new Delimiter(delimiter); } @Deprecated @@ -368,20 +379,25 @@ static GenerationMatch doesNotExist() { return new GenerationMatch(0); } - static EncryptionKey encryptionKey(String s) { - return new EncryptionKey(new SecretKeySpec(BaseEncoding.base64().decode(s), "AES256")); + static EncryptionKey encryptionKey(@NonNull String encryptionKey) { + requireNonNull(encryptionKey, "encryptionKey must be non null"); + return new EncryptionKey( + new SecretKeySpec(BaseEncoding.base64().decode(encryptionKey), "AES256")); } - static EncryptionKey encryptionKey(Key k) { - return new EncryptionKey(k); + static EncryptionKey encryptionKey(@NonNull Key encryptionKey) { + requireNonNull(encryptionKey, "encryptionKey must be non null"); + return new EncryptionKey(encryptionKey); } - static EndOffset endOffset(String s) { - return new EndOffset(s); + static EndOffset endOffset(@NonNull String endOffset) { + requireNonNull(endOffset, "endOffset must be non null"); + return new EndOffset(endOffset); } - static Fields fields(ImmutableSet s) { - return new Fields(s); + static Fields fields(@NonNull ImmutableSet fields) { + requireNonNull(fields, "fields must be non null"); + return new Fields(fields); } static GenerationMatch generationMatch(long l) { @@ -392,12 +408,14 @@ static GenerationNotMatch generationNotMatch(long l) { return new GenerationNotMatch(l); } - static KmsKeyName kmsKeyName(String s) { - return new KmsKeyName(s); + static KmsKeyName kmsKeyName(@NonNull String kmsKeyName) { + requireNonNull(kmsKeyName, "kmsKeyName must be non null"); + return new KmsKeyName(kmsKeyName); } - static Md5Match md5Match(String s) { - return new Md5Match(s); + static Md5Match md5Match(@NonNull String md5) { + requireNonNull(md5, "md5 must be non null"); + return new Md5Match(md5); } static MetagenerationMatch metagenerationMatch(long l) { @@ -412,28 +430,35 @@ static PageSize pageSize(long l) { return new PageSize(l); } - static PageToken pageToken(String s) { - return new PageToken(s); + static PageToken pageToken(@NonNull String pageToken) { + requireNonNull(pageToken, "pageToken must be non null"); + return new PageToken(pageToken); } - static PredefinedAcl predefinedAcl(Storage.PredefinedAcl p) { - return new PredefinedAcl(p.getEntry()); + static PredefinedAcl predefinedAcl(Storage.@NonNull PredefinedAcl predefinedAcl) { + requireNonNull(predefinedAcl, "predefinedAcl must be non null"); + return new PredefinedAcl(predefinedAcl.getEntry()); } - static PredefinedDefaultObjectAcl predefinedDefaultObjectAcl(Storage.PredefinedAcl p) { - return new PredefinedDefaultObjectAcl(p.getEntry()); + static PredefinedDefaultObjectAcl predefinedDefaultObjectAcl( + Storage.@NonNull PredefinedAcl predefinedAcl) { + requireNonNull(predefinedAcl, "predefinedAcl must be non null"); + return new PredefinedDefaultObjectAcl(predefinedAcl.getEntry()); } - static Prefix prefix(String s) { - return new Prefix(s); + static Prefix prefix(@NonNull String prefix) { + requireNonNull(prefix, "prefix must be non null"); + return new Prefix(prefix); } - static ProjectId projectId(String s) { - return new ProjectId(s); + static ProjectId projectId(@NonNull String projectId) { + requireNonNull(projectId, "projectId must be non null"); + return new ProjectId(projectId); } - static Projection projection(String s) { - return new Projection(s); + static Projection projection(@NonNull String projection) { + requireNonNull(projection, "projection must be non null"); + return new Projection(projection); } static RequestedPolicyVersion requestedPolicyVersion(long l) { @@ -444,12 +469,17 @@ static ReturnRawInputStream returnRawInputStream(boolean b) { return new ReturnRawInputStream(b); } - static ServiceAccount serviceAccount(com.google.cloud.storage.ServiceAccount s) { - return new ServiceAccount(s.getEmail()); + @RequiresNonNull({"serviceAccount", "#1.getEmail()"}) + static ServiceAccount serviceAccount( + com.google.cloud.storage.@NonNull ServiceAccount serviceAccount) { + requireNonNull(serviceAccount, "serviceAccount must be non null"); + requireNonNull(serviceAccount.getEmail(), "serviceAccount.getEmail() must be non null"); + return new ServiceAccount(serviceAccount.getEmail()); } @VisibleForTesting - static SetContentType setContentType(String s) { + static SetContentType setContentType(@NonNull String s) { + requireNonNull(s, "s must be non null"); return new SetContentType(s); } @@ -457,12 +487,14 @@ static ShowDeletedKeys showDeletedKeys(boolean b) { return new ShowDeletedKeys(b); } - static StartOffset startOffset(String s) { - return new StartOffset(s); + static StartOffset startOffset(@NonNull String startOffset) { + requireNonNull(startOffset, "startOffset must be non null"); + return new StartOffset(startOffset); } - static UserProject userProject(String s) { - return new UserProject(s); + static UserProject userProject(@NonNull String userProject) { + requireNonNull(userProject, "userProject must be non null"); + return new UserProject(userProject); } static VersionsFilter versionsFilter(boolean b) { @@ -1969,8 +2001,8 @@ private abstract static class RpcOptVal implements Opt { protected final T val; private RpcOptVal(StorageRpc.Option key, T val) { - this.key = key; - this.val = val; + this.key = requireNonNull(key, "key must be non null"); + this.val = requireNonNull(val, "val must be non null"); } public Mapper> mapper() { diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java index 26991b1bd..f0d33c0a5 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageFixture.java @@ -68,7 +68,7 @@ public static StorageFixture from(Supplier opts) { return of(() -> opts.get().getService()); } - public static StorageFixture of(Supplier<@NonNull Storage> s) { + public static StorageFixture of(@NonNull Supplier<@NonNull Storage> s) { return new StorageFixture(s); } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/UnifiedOptsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/UnifiedOptsTest.java index 82a2d9b05..400d7e198 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/UnifiedOptsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/UnifiedOptsTest.java @@ -17,10 +17,20 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assert.assertThrows; +import com.google.cloud.storage.UnifiedOpts.Opt; import com.google.cloud.storage.UnifiedOpts.Opts; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; import org.junit.Test; +import org.junit.runners.model.MultipleFailureException; public final class UnifiedOptsTest { @@ -35,4 +45,154 @@ public void opts_validation_uniqueKeys() { assertThat(iae).hasMessageThat().contains("GENERATION_MATCH"); } + + @Test + public void validateFactoryMethodEnforceNonNull_unifiedOpts() throws Exception { + validateFactoryMethodEnforceNonNull(UnifiedOpts.class, Opt.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_blobGetOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BlobGetOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_blobListOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BlobListOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_blobSourceOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BlobSourceOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_blobTargetOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BlobTargetOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_blobWriteOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BlobWriteOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_bucket_blobTargetOption() throws Exception { + validateFactoryMethodEnforceNonNull(Bucket.BlobTargetOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_bucket_blobWriteOption() throws Exception { + validateFactoryMethodEnforceNonNull(Bucket.BlobWriteOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_blob_blobSourceOption() throws Exception { + validateFactoryMethodEnforceNonNull(Blob.BlobSourceOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_bucketGetOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BucketGetOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_bucketListOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BucketListOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_bucketSourceOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BucketSourceOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_bucket_bucketSourceOption() throws Exception { + validateFactoryMethodEnforceNonNull(Bucket.BucketSourceOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_bucketTargetOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.BucketTargetOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_createHmacKeyOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.CreateHmacKeyOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_deleteHmacKeyOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.DeleteHmacKeyOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_getHmacKeyOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.GetHmacKeyOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_listHmacKeyOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.ListHmacKeysOption.class); + } + + @Test + public void validateFactoryMethodEnforceNonNull_storage_updateHmacKeyOption() throws Exception { + validateFactoryMethodEnforceNonNull(Storage.UpdateHmacKeyOption.class); + } + + private static void validateFactoryMethodEnforceNonNull(Class classToSearch) throws Exception { + validateFactoryMethodEnforceNonNull(classToSearch, classToSearch); + } + + private static void validateFactoryMethodEnforceNonNull( + Class classToSearch, Class returnSuperType) throws Exception { + List methods = findFactoryMethodsWithNullableParam(classToSearch, returnSuperType); + assertThat(methods).isNotEmpty(); + List errors = + methods.stream() + .map( + m -> { + try { + String msg = + String.format("Method %s did not throw expected NullPointerException", m); + try { + m.invoke(null, new Object[] {null}); + return new AssertionError(msg); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof NullPointerException) { + NullPointerException cause = (NullPointerException) e.getCause(); + assertWithMessage(msg) + .that(cause) + .hasMessageThat() + .contains("must be non null"); + return null; + } + return new AssertionError(msg, e); + } catch (Throwable t) { + return new AssertionError(msg, t); + } + } catch (Throwable e) { + return e; + } + }) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + MultipleFailureException.assertEmpty(errors); + } + + private static List findFactoryMethodsWithNullableParam( + Class classToSearch, Class returnSuperType) { + Method[] methods = classToSearch.getDeclaredMethods(); + return Arrays.stream(methods) + .filter( + m -> { + boolean isStatic = Modifier.isStatic(m.getModifiers()); + boolean isOpt = returnSuperType.isAssignableFrom(m.getReturnType()); + boolean hasParam = m.getParameterCount() == 1; + boolean isParamNonPrimitive = hasParam && !m.getParameterTypes()[0].isPrimitive(); + return isStatic && isOpt && hasParam && isParamNonPrimitive; + }) + .collect(Collectors.toList()); + } }