JAVA-5767 Support $lookup in CSFLE and QE #1638
Conversation
driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved
driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved
...r-sync/src/test/functional/com/mongodb/internal/connection/OidcAuthenticationProseTests.java Outdated Show resolved Hide resolved
driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved
driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25Lookup.java Outdated Show resolved Hide resolved
…ncryption25Lookup.java Co-authored-by: Viacheslav Babanin <frest0512@gmail.com>
mongodb-crypt/build.gradle.kts Outdated
| | ||
| // Download jnaLibs that match the git tag or revision to jnaResourcesBuildDir | ||
| val downloadRevision = "9a88ac5698e8e3ffcd6580b98c247f0126f26c40" // r1.11.0 | ||
| val downloadRevision = "67f10bfb8de69549987cc62a1d4548d7b511a7ef" // TODO |
There was a problem hiding this comment.
TODO, confirm the hash we want to use
There was a problem hiding this comment.
Once libmongocrypt is released, we can replace this with the tag, e.g. 1.13.0, as that download url is also available
There was a problem hiding this comment.
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 
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Release is imminent, so let's just wait a bit to merge.
There was a problem hiding this comment.
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 Outdated Show resolved Hide resolved
jyemin left a comment
jyemin left a comment There was a problem hiding this comment.
I don't plan to review the prose tests in detail, but LGTM on the rest.
mongodb-crypt/build.gradle.kts Outdated
| | ||
| // Download jnaLibs that match the git tag or revision to jnaResourcesBuildDir | ||
| val downloadRevision = "9a88ac5698e8e3ffcd6580b98c247f0126f26c40" // r1.11.0 | ||
| val downloadRevision = "67f10bfb8de69549987cc62a1d4548d7b511a7ef" // TODO |
There was a problem hiding this comment.
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 Show resolved Hide resolved
| "csfle, csfle2", | ||
| "qe, qe2", | ||
| "no_schema, no_schema2"}) | ||
| void cases1Through7(final String from, final String to) { |
There was a problem hiding this comment.
[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.
| void cases1Through7(final String from, final String to) { | |
| void shouldUseMultipleAutoEncryptionSchemasCases1Through7(final String from, final String to) { |
There was a problem hiding this comment.
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.
| } | ||
| | ||
| @Test | ||
| void case8() { |
There was a problem hiding this comment.
| void case8() { | |
| void shouldUseMultipleAutoEncryptionSchemasCase8() { |
| } | ||
| | ||
| @Test | ||
| void case9() { |
There was a problem hiding this comment.
| void case9() { | |
| void shouldUseMultipleAutoEncryptionSchemasCase9() { |
…ncryption25LookupProseTests.java Co-authored-by: Viacheslav Babanin <frest0512@gmail.com>
Co-authored-by: Jeff Yemin <jeff.yemin@mongodb.com>
4 participants
JAVA-5767
Draft due to failing tests (4 and 6).