Skip to content

Ensure findOne does not set batchSize=1 #1659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 1, 2025
158 changes: 158 additions & 0 deletions driver-core/src/test/resources/unified-test-format/crud/findOne.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
{
"description": "findOne",
"schemaVersion": "1.0",
"createEntities": [
{
"client": {
"id": "client0",
"observeEvents": [
"commandStartedEvent"
]
}
},
{
"database": {
"id": "database0",
"client": "client0",
"databaseName": "find-tests"
}
},
{
"collection": {
"id": "collection0",
"database": "database0",
"collectionName": "coll0"
}
}
],
"initialData": [
{
"collectionName": "coll0",
"databaseName": "find-tests",
"documents": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
},
{
"_id": 4,
"x": 44
},
{
"_id": 5,
"x": 55
},
{
"_id": 6,
"x": 66
}
]
}
],
"tests": [
{
"description": "FindOne with filter",
"operations": [
{
"object": "collection0",
"name": "findOne",
"arguments": {
"filter": {
"_id": 1
}
},
"expectResult": {
"_id": 1,
"x": 11
}
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"find": "coll0",
"filter": {
"_id": 1
},
"batchSize": {
"$$exists": false
},
"limit": 1,
"singleBatch": true
},
"commandName": "find",
"databaseName": "find-tests"
}
}
]
}
]
},
{
"description": "FindOne with filter, sort, and skip",
"operations": [
{
"object": "collection0",
"name": "findOne",
"arguments": {
"filter": {
"_id": {
"$gt": 2
}
},
"sort": {
"_id": 1
},
"skip": 2
},
"expectResult": {
"_id": 5,
"x": 55
}
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"find": "coll0",
"filter": {
"_id": {
"$gt": 2
}
},
"sort": {
"_id": 1
},
"skip": 2,
"batchSize": {
"$$exists": false
},
"limit": 1,
"singleBatch": true
},
"commandName": "find",
"databaseName": "find-tests"
}
}
]
}
]
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class UnifiedCrudTest() : UnifiedTest() {
@JvmStatic
@Throws(URISyntaxException::class, IOException::class)
fun data(): Collection<Arguments>? {
return getTestData("unified-test-format/crud", true)
return getTestData("unified-test-format/crud", true, Language.KOTLIN)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,8 @@ internal abstract class UnifiedTest() : JUnifiedTest() {
): ClientEncryption {
TODO("Not yet implemented - JAVA-4896")
}

override fun isReactive(): Boolean = true

override fun getLanguage(): Language = Language.KOTLIN
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class UnifiedCrudTest() : UnifiedTest() {
@JvmStatic
@Throws(URISyntaxException::class, IOException::class)
fun data(): Collection<Arguments>? {
return getTestData("unified-test-format/crud", false)
return getTestData("unified-test-format/crud", false, Language.KOTLIN)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,6 @@ internal abstract class UnifiedTest() : JUnifiedTest() {
): ClientEncryption {
TODO("Not yet implemented - JAVA-4896")
}

override fun getLanguage(): Language = Language.KOTLIN
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,6 @@ protected void postCleanUp(final TestDef testDef) {

@NonNull
protected static Collection<Arguments> getTestData(final String directory) {
return getTestData(directory, true);
return getTestData(directory, true, Language.JAVA);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,11 @@ OperationResult executeFind(final BsonDocument operation) {
});
}

/**
* There is no explicit {@code findOne()} method in {@link MongoCollection} class.
* Its feature was emulated by {@link FindIterable#first()}, which would close cursor on server
* by setting {@code batchSize} and {@code limit} appropriately.
*/
OperationResult executeFindOne(final BsonDocument operation) {
return resultOf(() -> createFindIterable(operation).first());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ protected ClientEncryption createClientEncryption(final MongoClient keyVaultClie

@NonNull
protected static Collection<Arguments> getTestData(final String directory) {
return getTestData(directory, false);
return getTestData(directory, false, Language.JAVA);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ public Entities getEntities() {
}

@NonNull
protected static Collection<Arguments> getTestData(final String directory, final boolean isReactive) {
protected static Collection<Arguments> getTestData(final String directory, final boolean isReactive, final Language language) {
List<Arguments> data = new ArrayList<>();
for (BsonDocument fileDocument : getTestDocuments("/" + directory + "/")) {
for (BsonValue cur : fileDocument.getArray("tests")) {

final BsonDocument testDocument = cur.asDocument();
String testDescription = testDocument.getString("description").getValue();
String fileDescription = fileDocument.getString("description").getValue();
TestDef testDef = testDef(directory, fileDescription, testDescription, isReactive);
TestDef testDef = testDef(directory, fileDescription, testDescription, isReactive, language);
applyCustomizations(testDef);

boolean forceFlaky = testDef.wasAssignedModifier(Modifier.FORCE_FLAKY);
Expand Down Expand Up @@ -240,7 +240,7 @@ public void setUp(
rootContext.getAssertionContext().push(ContextElement.ofTest(definition));
ignoreExtraEvents = false;
if (directoryName != null && fileDescription != null && testDescription != null) {
testDef = testDef(directoryName, fileDescription, testDescription, isReactive());
testDef = testDef(directoryName, fileDescription, testDescription, isReactive(), getLanguage());
applyCustomizations(testDef);

boolean skip = testDef.wasAssignedModifier(Modifier.SKIP);
Expand Down Expand Up @@ -329,6 +329,10 @@ protected boolean isReactive() {
return false;
}

protected Language getLanguage() {
return Language.JAVA;
}

@ParameterizedTest(name = "{0}")
@MethodSource("data")
public void shouldPassAllOutcomes(
Expand Down Expand Up @@ -1112,4 +1116,8 @@ protected void ignoreExtraCommandEvents(final boolean ignoreExtraEvents) {
protected void ignoreExtraEvents() {
this.ignoreExtraEvents = true;
}

public enum Language {
JAVA, KOTLIN
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ public static void applyCustomizations(final TestDef def) {
.test("crud", "findOneAndDelete-hint-unacknowledged", "Unacknowledged findOneAndDelete with hint string on 4.4+ server")
.test("crud", "findOneAndDelete-hint-unacknowledged", "Unacknowledged findOneAndDelete with hint document on 4.4+ server");

def.skipNoncompliant("https://jira.mongodb.org/browse/JAVA-5838")
.when(() -> def.isReactive() && UnifiedTest.Language.KOTLIN.equals(def.getLanguage()))
.file("crud", "findOne");

// gridfs

def.skipDeprecated("contentType is deprecated in GridFS spec, and 4.x Java driver no longer supports it")
Expand Down Expand Up @@ -256,24 +260,28 @@ public static void applyCustomizations(final TestDef def) {

private UnifiedTestModifications() {}

public static TestDef testDef(final String dir, final String file, final String test, final boolean reactive) {
return new TestDef(dir, file, test, reactive);
public static TestDef testDef(final String dir, final String file, final String test, final boolean reactive,
final UnifiedTest.Language language) {
return new TestDef(dir, file, test, reactive, language);
}

public static final class TestDef {

private final String dir;
private final String file;
private final String test;
private final boolean reactive;
private final UnifiedTest.Language language;

private final List<Modifier> modifiers = new ArrayList<>();
private Function<Throwable, Boolean> matchesThrowable;

private TestDef(final String dir, final String file, final String test, final boolean reactive) {
private TestDef(final String dir, final String file, final String test, final boolean reactive, final UnifiedTest.Language language) {
this.dir = assertNotNull(dir);
this.file = assertNotNull(file);
this.test = assertNotNull(test);
this.reactive = reactive;
this.language = assertNotNull(language);
}

/**
Expand Down Expand Up @@ -354,6 +362,10 @@ public boolean isReactive() {
return reactive;
}

public UnifiedTest.Language getLanguage() {
return language;
}

public boolean wasAssignedModifier(final Modifier modifier) {
return this.modifiers.contains(modifier);
}
Expand Down