JAVA-5767 Support $lookup in CSFLE and QE #1638

katcharov · 2025-02-24T21:59:17Z

JAVA-5767

Draft due to failing tests (4 and 6).

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java

...r-sync/src/test/functional/com/mongodb/internal/connection/OidcAuthenticationProseTests.java

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java

…ncryption25Lookup.java Co-authored-by: Viacheslav Babanin <[email protected]>

katcharov · 2025-02-25T14:55:29Z

mongodb-crypt/build.gradle.kts

@@ -60,7 +60,7 @@ val jnaLibsPath: String = System.getProperty("jnaLibsPath", "${jnaResourcesDir}$
 val jnaResources: String = System.getProperty("jna.library.path", jnaLibsPath)

 // Download jnaLibs that match the git tag or revision to jnaResourcesBuildDir
-val downloadRevision = "9a88ac5698e8e3ffcd6580b98c247f0126f26c40" // r1.11.0
+val downloadRevision = "67f10bfb8de69549987cc62a1d4548d7b511a7ef" // TODO


TODO, confirm the hash we want to use

Once libmongocrypt is released, we can replace this with the tag, e.g. 1.13.0, as that download url is also available

Yes. Tagged commits are uploaded to a path with the tag. For reference, on 1.12.0 the Files tab shows two uploaded URLs:

https://mciuploads.s3.amazonaws.com/libmongocrypt/java/085a0ce6538a28179da6bfd2927aea106924443a/libmongocrypt-java.tar.gz https://mciuploads.s3.amazonaws.com/libmongocrypt/java/1.12.0/libmongocrypt-java.tar.gz

Should we create a ticket for this //TODO until libmongocrypt is released, so we don't forget to update this hash to a tag before the driver release?

Release is imminent, so let's just wait a bit to merge.

1.13.0 is released. Upload is available at:

https://mciuploads.s3.amazonaws.com/libmongocrypt/java/1.13.0/libmongocrypt-java.tar.gz

driver-sync/src/main/com/mongodb/client/internal/CollectionInfoRetriever.java

jyemin

I don't plan to review the prose tests in detail, but LGTM on the rest.

kevinAlbs · 2025-02-26T19:00:55Z

mongodb-crypt/build.gradle.kts

@@ -60,7 +60,7 @@ val jnaLibsPath: String = System.getProperty("jnaLibsPath", "${jnaResourcesDir}$
 val jnaResources: String = System.getProperty("jna.library.path", jnaLibsPath)

 // Download jnaLibs that match the git tag or revision to jnaResourcesBuildDir
-val downloadRevision = "9a88ac5698e8e3ffcd6580b98c247f0126f26c40" // r1.11.0
+val downloadRevision = "67f10bfb8de69549987cc62a1d4548d7b511a7ef" // TODO


Yes. Tagged commits are uploaded to a path with the tag. For reference, on 1.12.0 the Files tab shows two uploaded URLs:

https://mciuploads.s3.amazonaws.com/libmongocrypt/java/085a0ce6538a28179da6bfd2927aea106924443a/libmongocrypt-java.tar.gz https://mciuploads.s3.amazonaws.com/libmongocrypt/java/1.12.0/libmongocrypt-java.tar.gz

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java

vbabanin · 2025-02-26T20:30:34Z

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java

+            "csfle, csfle2",
+            "qe, qe2",
+            "no_schema, no_schema2"})
+    void cases1Through7(final String from, final String to) {


[nit] Just a bit more context that the test is about using multiple auto-encryption schemas. It might save time understanding its purpose in the future. Alternatively, "test" prefix or comment above a test could help.

Suggested change

void cases1Through7(final String from, final String to) {

void shouldUseMultipleAutoEncryptionSchemasCases1Through7(final String from, final String to) {

I used the "test" prefix.

For prose tests, I think the priority is being able to easily reference the prose test spec file. Tests should be numbered, with the number near the start of the test or file to ensure alphabetical sorting. If extra wording will help clarify, tests should match spec wording of the individual test (so tests should end up with mostly different names), and if the spec's test case naming is unclear, an edit should be made to the spec.

vbabanin · 2025-02-26T20:30:57Z

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java

+    }
+
+    @Test
+    void case8() {


Suggested change

void case8() {

void shouldUseMultipleAutoEncryptionSchemasCase8() {

vbabanin · 2025-02-26T20:31:09Z

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java

+    }
+
+    @Test
+    void case9() {


Suggested change

void case9() {

void shouldUseMultipleAutoEncryptionSchemasCase9() {

vbabanin

LGTM! Just couple of nits above.

…ncryption25LookupProseTests.java Co-authored-by: Viacheslav Babanin <[email protected]>

mongodb-crypt/build.gradle.kts

Co-authored-by: Jeff Yemin <[email protected]>

Support $lookup in CSFLE and QE

6906e31

katcharov requested a review from vbabanin February 24, 2025 21:59

katcharov commented Feb 24, 2025

View reviewed changes

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved

katcharov commented Feb 24, 2025

View reviewed changes

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved

vbabanin reviewed Feb 25, 2025

View reviewed changes

katcharov and others added 2 commits February 25, 2025 07:25

Update driver-sync/src/test/functional/com/mongodb/client/ClientSideE…

1a4d7bd

…ncryption25Lookup.java Co-authored-by: Viacheslav Babanin <[email protected]>

PR fixes

533bcdf

katcharov commented Feb 25, 2025

View reviewed changes

jyemin requested changes Feb 25, 2025

View reviewed changes

driver-sync/src/main/com/mongodb/client/internal/CollectionInfoRetriever.java Outdated Show resolved Hide resolved

katcharov added 3 commits February 25, 2025 09:40

PR fixes

8e3be8a

Spec updates

c59330c

PR fixes

ebf4d32

katcharov marked this pull request as ready for review February 25, 2025 23:48

katcharov mentioned this pull request Feb 26, 2025

DRIVERS-3082 add prose tests for $lookup mongodb/specifications#1757

Merged

2 tasks

katcharov requested review from vbabanin and jyemin February 26, 2025 14:37

jyemin approved these changes Feb 26, 2025

View reviewed changes

kevinAlbs approved these changes Feb 26, 2025

View reviewed changes

vbabanin reviewed Feb 26, 2025

View reviewed changes

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java Show resolved Hide resolved

vbabanin reviewed Feb 26, 2025

View reviewed changes

vbabanin approved these changes Feb 26, 2025

View reviewed changes

katcharov and others added 2 commits February 27, 2025 07:06

Update driver-sync/src/test/functional/com/mongodb/client/ClientSideE…

b06e199

…ncryption25LookupProseTests.java Co-authored-by: Viacheslav Babanin <[email protected]>

pr fixes

6789b7c

jyemin requested changes Feb 27, 2025

View reviewed changes

mongodb-crypt/build.gradle.kts Outdated Show resolved Hide resolved

Update mongodb-crypt/build.gradle.kts

acfbba9

Co-authored-by: Jeff Yemin <[email protected]>

katcharov requested a review from jyemin February 27, 2025 18:51

jyemin approved these changes Feb 27, 2025

View reviewed changes

katcharov merged commit eeceb63 into mongodb:main Feb 27, 2025
60 checks passed

katcharov deleted the JAVA-5767 branch February 27, 2025 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JAVA-5767 Support $lookup in CSFLE and QE #1638

JAVA-5767 Support $lookup in CSFLE and QE #1638

katcharov commented Feb 24, 2025 •

edited

Loading

katcharov Feb 25, 2025

jyemin Feb 25, 2025

kevinAlbs Feb 26, 2025

vbabanin Feb 26, 2025

jyemin Feb 26, 2025

kevinAlbs Feb 27, 2025

jyemin left a comment

kevinAlbs Feb 26, 2025

vbabanin Feb 26, 2025 •

edited

Loading

katcharov Feb 27, 2025

vbabanin Feb 26, 2025

vbabanin Feb 26, 2025

vbabanin left a comment •

edited

Loading

	void cases1Through7(final String from, final String to) {
	void shouldUseMultipleAutoEncryptionSchemasCases1Through7(final String from, final String to) {

	void case8() {
	void shouldUseMultipleAutoEncryptionSchemasCase8() {

	void case9() {
	void shouldUseMultipleAutoEncryptionSchemasCase9() {

JAVA-5767 Support $lookup in CSFLE and QE #1638

JAVA-5767 Support $lookup in CSFLE and QE #1638

Conversation

katcharov commented Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyemin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbabanin Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbabanin left a comment • edited Loading

Choose a reason for hiding this comment

katcharov commented Feb 24, 2025 •

edited

Loading

vbabanin Feb 26, 2025 •

edited

Loading

vbabanin left a comment •

edited

Loading