Make ClientEncryptionSettings#keyVaultMongoClientSettings non-nullable (

#1087) * Update Javadoc * Add notNull assertion in ClientEncryptionSettings constructor * Remove unnecessary null checks and corresponding SpotBugs exclusions for them JAVA-4861
mongodb · Feb 22, 2023 · a19fad2 · a19fad2
1 parent 08dc4be
commit a19fad2
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 34 deletions.
diff --git a/config/spotbugs/exclude.xml b/config/spotbugs/exclude.xml
@@ -240,18 +240,4 @@
         <Method name="acquirePermitOrGetAvailableOpenedConnection"/>
         <Bug pattern="NS_NON_SHORT_CIRCUIT"/>
     </Match>
-
-    <!-- Can actually be null, but is not annotated as `@Nullable`. Annotating it as such causes warnings
-         in other places where `null` is not handled, see https://jira.mongodb.org/browse/JAVA-4861.
-         When the aforementioned ticket is done, it will be clear what to do with the warnings suppressed here. -->
-    <Match>
-        <Class name="com.mongodb.client.internal.ClientEncryptionImpl"/>
-        <Method name="createEncryptedCollection"/>
-        <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
-    </Match>
-    <Match>
-        <Class name="com.mongodb.reactivestreams.client.internal.vault.ClientEncryptionImpl"/>
-        <Method name="~.*createEncryptedCollection.*"/>
-        <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
-    </Match>
 </FindBugsFilter>
diff --git a/driver-core/src/main/com/mongodb/ClientEncryptionSettings.java b/driver-core/src/main/com/mongodb/ClientEncryptionSettings.java
@@ -55,14 +55,14 @@ public static final class Builder {
         private Map<String, SSLContext> kmsProviderSslContextMap = new HashMap<>();
 
         /**
-         * Sets the key vault settings.
+         * Sets the {@link MongoClientSettings} that will be used to access the key vault.
          *
-         * @param keyVaultMongoClientSettings the key vault mongo client settings, which may be null.
+         * @param keyVaultMongoClientSettings the key vault mongo client settings, which may not be null.
          * @return this
          * @see #getKeyVaultMongoClientSettings()
          */
         public Builder keyVaultMongoClientSettings(final MongoClientSettings keyVaultMongoClientSettings) {
-            this.keyVaultMongoClientSettings = keyVaultMongoClientSettings;
+            this.keyVaultMongoClientSettings = notNull("keyVaultMongoClientSettings", keyVaultMongoClientSettings);
             return this;
         }
 
@@ -143,15 +143,9 @@ public static Builder builder() {
     }
 
     /**
-     * Gets the key vault settings.
+     * Gets the {@link MongoClientSettings} that will be used to access the key vault.
      *
-     * <p>
-     * The key vault collection is assumed to reside on the same MongoDB cluster as indicated by the connecting URI. But the optional
-     * keyVaultMongoClientSettings can be used to route data key queries to a separate MongoDB cluster, or the same cluster but with a
-     * different credential.
-     * </p>
-     * @return the key vault settings, which may be null to indicate that the same {@code MongoClient} should be used to access the key
-     * vault collection as is used for the rest of the application.
+     * @return the key vault settings, which may be not be null
      */
     public MongoClientSettings getKeyVaultMongoClientSettings() {
         return keyVaultMongoClientSettings;
@@ -260,7 +254,7 @@ public Map<String, SSLContext> getKmsProviderSslContextMap() {
     }
 
     private ClientEncryptionSettings(final Builder builder) {
-        this.keyVaultMongoClientSettings = builder.keyVaultMongoClientSettings;
+        this.keyVaultMongoClientSettings = notNull("keyVaultMongoClientSettings", builder.keyVaultMongoClientSettings);
         this.keyVaultNamespace = notNull("keyVaultNamespace", builder.keyVaultNamespace);
         this.kmsProviders = notNull("kmsProviders", builder.kmsProviders);
         this.kmsProviderPropertySuppliers = notNull("kmsProviderPropertySuppliers", builder.kmsProviderPropertySuppliers);

diff --git a/driver-core/src/test/functional/com/mongodb/internal/capi/MongoCryptHelperTest.java b/driver-core/src/test/functional/com/mongodb/internal/capi/MongoCryptHelperTest.java
@@ -19,6 +19,7 @@
 import com.mongodb.AutoEncryptionSettings;
 import com.mongodb.ClientEncryptionSettings;
 import com.mongodb.MongoClientException;
+import com.mongodb.MongoClientSettings;
 import com.mongodb.client.model.vault.RewrapManyDataKeyOptions;
 import com.mongodb.crypt.capi.MongoCryptOptions;
 import org.bson.BsonDocument;
@@ -52,6 +53,7 @@ public void createsExpectedMongoCryptOptionsUsingClientEncryptionSettings() {
         ClientEncryptionSettings settings = ClientEncryptionSettings
                 .builder()
                 .kmsProviders(kmsProvidersRaw)
+                .keyVaultMongoClientSettings(MongoClientSettings.builder().build())
                 .keyVaultNamespace("a.b")
                 .build();
         MongoCryptOptions mongoCryptOptions = MongoCryptHelper.createMongoCryptOptions(settings);

diff --git a/...eams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java b/...eams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java
@@ -17,7 +17,6 @@
 package com.mongodb.reactivestreams.client.internal.vault;
 
 import com.mongodb.ClientEncryptionSettings;
-import com.mongodb.MongoClientSettings;
 import com.mongodb.MongoConfigurationException;
 import com.mongodb.MongoNamespace;
 import com.mongodb.MongoUpdatedEncryptedFieldsException;
@@ -33,6 +32,7 @@
 import com.mongodb.client.model.vault.RewrapManyDataKeyOptions;
 import com.mongodb.client.model.vault.RewrapManyDataKeyResult;
 import com.mongodb.client.result.DeleteResult;
+import com.mongodb.internal.VisibleForTesting;
 import com.mongodb.reactivestreams.client.FindPublisher;
 import com.mongodb.reactivestreams.client.MongoClient;
 import com.mongodb.reactivestreams.client.MongoClients;
@@ -59,6 +59,7 @@
 import java.util.stream.Collectors;
 
 import static com.mongodb.assertions.Assertions.notNull;
+import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;
 import static com.mongodb.internal.capi.MongoCryptHelper.validateRewrapManyDataKeyOptions;
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
@@ -78,6 +79,7 @@ public ClientEncryptionImpl(final ClientEncryptionSettings options) {
         this(MongoClients.create(options.getKeyVaultMongoClientSettings()), options);
     }
 
+    @VisibleForTesting(otherwise = PRIVATE)
     public ClientEncryptionImpl(final MongoClient keyVaultClient, final ClientEncryptionSettings options) {
         this.keyVaultClient = keyVaultClient;
         this.crypt = Crypts.create(keyVaultClient, options);
@@ -208,9 +210,7 @@ public Publisher<BsonDocument> createEncryptedCollection(final MongoDatabase dat
         if (rawEncryptedFields == null) {
             throw new MongoConfigurationException(format("`encryptedFields` is not configured for the collection %s.", namespace));
         }
-        CodecRegistry codecRegistry = options.getKeyVaultMongoClientSettings() == null
-                ? MongoClientSettings.getDefaultCodecRegistry()
-                : options.getKeyVaultMongoClientSettings().getCodecRegistry();
+        CodecRegistry codecRegistry = options.getKeyVaultMongoClientSettings().getCodecRegistry();
         BsonDocument encryptedFields = rawEncryptedFields.toBsonDocument(BsonDocument.class, codecRegistry);
         BsonValue fields = encryptedFields.get("fields");
         if (fields != null && fields.isArray()) {

diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java
@@ -17,7 +17,6 @@
 package com.mongodb.client.internal;
 
 import com.mongodb.ClientEncryptionSettings;
-import com.mongodb.MongoClientSettings;
 import com.mongodb.MongoConfigurationException;
 import com.mongodb.MongoNamespace;
 import com.mongodb.MongoUpdatedEncryptedFieldsException;
@@ -39,6 +38,7 @@
 import com.mongodb.client.model.vault.RewrapManyDataKeyResult;
 import com.mongodb.client.result.DeleteResult;
 import com.mongodb.client.vault.ClientEncryption;
+import com.mongodb.internal.VisibleForTesting;
 import org.bson.BsonArray;
 import org.bson.BsonBinary;
 import org.bson.BsonDocument;
@@ -54,6 +54,7 @@
 import java.util.stream.Collectors;
 
 import static com.mongodb.assertions.Assertions.notNull;
+import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;
 import static com.mongodb.internal.capi.MongoCryptHelper.validateRewrapManyDataKeyOptions;
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
@@ -73,6 +74,7 @@ public ClientEncryptionImpl(final ClientEncryptionSettings options) {
         this(MongoClients.create(options.getKeyVaultMongoClientSettings()), options);
     }
 
+    @VisibleForTesting(otherwise = PRIVATE)
     public ClientEncryptionImpl(final MongoClient keyVaultClient, final ClientEncryptionSettings options) {
         this.keyVaultClient = keyVaultClient;
         this.crypt = Crypts.create(keyVaultClient, options);
@@ -195,9 +197,7 @@ public BsonDocument createEncryptedCollection(final MongoDatabase database, fina
         if (rawEncryptedFields == null) {
             throw new MongoConfigurationException(format("`encryptedFields` is not configured for the collection %s.", namespace));
         }
-        CodecRegistry codecRegistry = options.getKeyVaultMongoClientSettings() == null
-                ? MongoClientSettings.getDefaultCodecRegistry()
-                : options.getKeyVaultMongoClientSettings().getCodecRegistry();
+        CodecRegistry codecRegistry = options.getKeyVaultMongoClientSettings().getCodecRegistry();
         BsonDocument encryptedFields = rawEncryptedFields.toBsonDocument(BsonDocument.class, codecRegistry);
         BsonValue fields = encryptedFields.get("fields");
         if (fields != null && fields.isArray()) {

diff --git a/driver-sync/src/test/functional/com/mongodb/client/unified/Entities.java b/driver-sync/src/test/functional/com/mongodb/client/unified/Entities.java
@@ -596,6 +596,8 @@ private void initClientEncryption(final BsonDocument entity, final String id,
 
         MongoClient mongoClient = null;
         ClientEncryptionSettings.Builder builder = ClientEncryptionSettings.builder();
+        // this is ignored in preference to the keyVaultClient, but required to be non-null in the ClientEncryptionSettings constructor
+        builder.keyVaultMongoClientSettings(MongoClientSettings.builder().build());
         for (Map.Entry<String, BsonValue> entry : clientEncryptionOpts.entrySet()) {
             switch (entry.getKey()) {
                 case "keyVaultClient":