From 36f0d68e9a91e86b8a255f32450e728fabc1efe0 Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Wed, 15 Feb 2023 17:05:53 -0500 Subject: [PATCH 1/3] Fix documentation of ClientEncryptionSettings#keyVaultMongoClientSettings property to indicate it can't be null * Update Javadoc * Add notNull assertion in ClientEncryptionSettings constructor * Remove unnecessary null checks and corresponding SpotBugs exclusions for them JAVA-4861 --- config/spotbugs/exclude.xml | 14 -------------- .../com/mongodb/ClientEncryptionSettings.java | 18 ++++++------------ .../internal/vault/ClientEncryptionImpl.java | 5 +---- .../client/internal/ClientEncryptionImpl.java | 5 +---- 4 files changed, 8 insertions(+), 34 deletions(-) diff --git a/config/spotbugs/exclude.xml b/config/spotbugs/exclude.xml index 8cfa95fb4ec..2a8b69a2e7b 100644 --- a/config/spotbugs/exclude.xml +++ b/config/spotbugs/exclude.xml @@ -240,18 +240,4 @@ - - - - - - - - - - - - diff --git a/driver-core/src/main/com/mongodb/ClientEncryptionSettings.java b/driver-core/src/main/com/mongodb/ClientEncryptionSettings.java index 1367547b6c3..2df4b3363d4 100644 --- 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 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. * - *

- * 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. - *

- * @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 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-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java index 5b4331fa982..65cb8eeb19a 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java +++ b/driver-reactive-streams/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; @@ -208,9 +207,7 @@ public Publisher 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 index b8462f058c2..f166c1bba6f 100644 --- 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; @@ -195,9 +194,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()) { From c61fd605be8b530a5f9cf220eef02780b417d704 Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Wed, 15 Feb 2023 17:55:45 -0500 Subject: [PATCH 2/3] Fix test --- .../com/mongodb/internal/capi/MongoCryptHelperTest.java | 2 ++ 1 file changed, 2 insertions(+) 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 index 9feac3e57a8..c7d82748efb 100644 --- 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); From dab8070b852b3fc9cbcc253292ad789c044c9dec Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Wed, 15 Feb 2023 19:11:38 -0500 Subject: [PATCH 3/3] Fix unified tests --- .../client/internal/vault/ClientEncryptionImpl.java | 3 +++ .../main/com/mongodb/client/internal/ClientEncryptionImpl.java | 3 +++ .../test/functional/com/mongodb/client/unified/Entities.java | 2 ++ 3 files changed, 8 insertions(+) diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java index 65cb8eeb19a..b6c3cb73c61 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/vault/ClientEncryptionImpl.java @@ -32,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; @@ -58,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; @@ -77,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); diff --git a/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java b/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java index f166c1bba6f..fad1c711d64 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/ClientEncryptionImpl.java @@ -38,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; @@ -53,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; @@ -72,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); 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 index 0dd1041117b..b3838000006 100644 --- 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 entry : clientEncryptionOpts.entrySet()) { switch (entry.getKey()) { case "keyVaultClient":