diff --git a/src/main/java/io/vertx/ext/mongo/impl/MongoClientImpl.java b/src/main/java/io/vertx/ext/mongo/impl/MongoClientImpl.java index 4e224efa..30e800b6 100644 --- a/src/main/java/io/vertx/ext/mongo/impl/MongoClientImpl.java +++ b/src/main/java/io/vertx/ext/mongo/impl/MongoClientImpl.java @@ -55,7 +55,9 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -207,7 +209,7 @@ public Future close() { requireNonNull(options, OPTIONS_CANNOT_BE_NULL); MongoCollection coll = getCollection(collection, options.getWriteOption()); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); Bson bupdate = wrap(encodeKeyWhenUseObjectId(generateIdIfNeeded(query, update, options))); com.mongodb.client.model.UpdateOptions updateOptions = new com.mongodb.client.model.UpdateOptions().upsert(options.isUpsert()); @@ -246,7 +248,7 @@ public Future close() { requireNonNull(options, OPTIONS_CANNOT_BE_NULL); MongoCollection coll = getCollection(collection, options.getWriteOption()); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); List bpipeline = new ArrayList<>(pipeline.size()); for (int i=0 ; i coll = getCollection(collection, options.getWriteOption()); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); com.mongodb.client.model.ReplaceOptions replaceOptions = new com.mongodb.client.model.ReplaceOptions().upsert(options.isUpsert()); if (options.getHint() != null) { replaceOptions.hint(wrap(options.getHint())); @@ -332,7 +334,7 @@ public Future> findWithOptions(String collection, JsonObject qu requireNonNull(query, QUERY_CANNOT_BE_NULL); Promise> promise = vertx.promise(); - doFind(collection, encodeKeyWhenUseObjectId(query), options) + doFind(collection, deepEncodeKeyWhenUseObjectId(query), options) .subscribe(new MappingAndBufferingSubscriber<>(this::decodeKeyWhenUseObjectId, promise)); return promise.future(); } @@ -355,7 +357,7 @@ public ReadStream findBatchWithOptions(String collection, JsonObject requireNonNull(collection, COLLECTION_CANNOT_BE_NULL); requireNonNull(query, QUERY_CANNOT_BE_NULL); - JsonObject encodedQuery = encodeKeyWhenUseObjectId(query); + JsonObject encodedQuery = deepEncodeKeyWhenUseObjectId(query); Bson bquery = wrap(encodedQuery); Bson bfields = wrap(fields); @@ -377,7 +379,7 @@ public ReadStream findBatchWithOptions(String collection, JsonObject requireNonNull(findOptions, FIND_OPTIONS_CANNOT_BE_NULL); requireNonNull(updateOptions, "update options cannot be null"); - JsonObject encodedQuery = encodeKeyWhenUseObjectId(query); + JsonObject encodedQuery = deepEncodeKeyWhenUseObjectId(query); Bson bquery = wrap(encodedQuery); Bson bupdate = wrap(update); @@ -428,7 +430,7 @@ public ReadStream findBatchWithOptions(String collection, JsonObject requireNonNull(findOptions, FIND_OPTIONS_CANNOT_BE_NULL); requireNonNull(updateOptions, "update options cannot be null"); - JsonObject encodedQuery = encodeKeyWhenUseObjectId(query); + JsonObject encodedQuery = deepEncodeKeyWhenUseObjectId(query); Bson bquery = wrap(encodedQuery); FindOneAndReplaceOptions foarOptions = new FindOneAndReplaceOptions(); @@ -470,7 +472,7 @@ public ReadStream findBatchWithOptions(String collection, JsonObject requireNonNull(query, QUERY_CANNOT_BE_NULL); requireNonNull(findOptions, FIND_OPTIONS_CANNOT_BE_NULL); - JsonObject encodedQuery = encodeKeyWhenUseObjectId(query); + JsonObject encodedQuery = deepEncodeKeyWhenUseObjectId(query); Bson bquery = wrap(encodedQuery); FindOneAndDeleteOptions foadOptions = new FindOneAndDeleteOptions(); @@ -504,7 +506,7 @@ public Future countWithOptions(String collection, JsonObject query, CountO requireNonNull(collection, COLLECTION_CANNOT_BE_NULL); requireNonNull(query, QUERY_CANNOT_BE_NULL); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); MongoCollection coll = getCollection(collection); Promise promise = vertx.promise(); Publisher countPublisher = countOptions != null @@ -525,7 +527,7 @@ public Future countWithOptions(String collection, JsonObject query, CountO requireNonNull(query, QUERY_CANNOT_BE_NULL); MongoCollection coll = getCollection(collection, writeOption); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); Promise promise = vertx.promise(); coll.deleteMany(bquery).subscribe(new SingleResultSubscriber<>(promise)); return promise.future().map(Utils::toMongoClientDeleteResult); @@ -542,7 +544,7 @@ public Future countWithOptions(String collection, JsonObject query, CountO requireNonNull(query, QUERY_CANNOT_BE_NULL); MongoCollection coll = getCollection(collection, writeOption); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); Promise promise = vertx.promise(); coll.deleteOne(bquery).subscribe(new SingleResultSubscriber<>(promise)); return promise.future().map(Utils::toMongoClientDeleteResult); @@ -571,7 +573,7 @@ private List> convertBulkOperations(List o for (BulkOperation bulkOperation : operations) { switch (bulkOperation.getType()) { case DELETE: - Bson bsonFilter = toBson(encodeKeyWhenUseObjectId(bulkOperation.getFilter())); + Bson bsonFilter = toBson(deepEncodeKeyWhenUseObjectId(bulkOperation.getFilter())); DeleteOptions deleteOptions = new DeleteOptions(); if (bulkOperation.getHint() != null) { deleteOptions.hint(toBson(bulkOperation.getHint())); @@ -602,11 +604,11 @@ private List> convertBulkOperations(List o if (bulkOperation.getHintString() != null && !bulkOperation.getHintString().isEmpty()) { replaceOptions.hintString(bulkOperation.getHintString()); } - result.add(new ReplaceOneModel<>(toBson(encodeKeyWhenUseObjectId(bulkOperation.getFilter())), bulkOperation.getDocument(), + result.add(new ReplaceOneModel<>(toBson(deepEncodeKeyWhenUseObjectId(bulkOperation.getFilter())), bulkOperation.getDocument(), replaceOptions.upsert(bulkOperation.isUpsert()))); break; case UPDATE: - Bson filter = toBson(encodeKeyWhenUseObjectId(bulkOperation.getFilter())); + Bson filter = toBson(deepEncodeKeyWhenUseObjectId(bulkOperation.getFilter())); Bson document = toBson(encodeKeyWhenUseObjectId(bulkOperation.getDocument())); com.mongodb.client.model.UpdateOptions updateOptions = new com.mongodb.client.model.UpdateOptions() .upsert(bulkOperation.isUpsert()); @@ -872,7 +874,7 @@ private DistinctPublisher findDistinctValuesWithQuery(String collection, Stri requireNonNull(fieldName, FIELD_NAME_CANNOT_BE_NULL); requireNonNull(query, QUERY_CANNOT_BE_NULL); - JsonObject encodedQuery = encodeKeyWhenUseObjectId(query); + JsonObject encodedQuery = deepEncodeKeyWhenUseObjectId(query); Bson bquery = wrap(encodedQuery); @@ -905,8 +907,60 @@ private AggregatePublisher doAggregate(final String collection, fina return aggregate; } + + JsonArray deepEncodeKeyWhenUseObjectId(JsonArray arr) { + if(!useObjectId) return arr; + + JsonArray newArr = new JsonArray(new ArrayList<>(arr.size())); + + for (Object item : arr) { + if (item instanceof JsonArray) { + newArr.add(deepEncodeKeyWhenUseObjectId((JsonArray) item)); + } else if(item instanceof List) { + newArr.add(deepEncodeKeyWhenUseObjectId(new JsonArray((List) item))); + } else if (item instanceof JsonObject) { + newArr.add(deepEncodeKeyWhenUseObjectId((JsonObject) item)); + } else if (item instanceof Map) { + newArr.add(deepEncodeKeyWhenUseObjectId(new JsonObject((Map) item))); + } else { + newArr.add(item); + } + } + + return newArr; + } + + JsonObject deepEncodeKeyWhenUseObjectId(JsonObject json) { + if(!useObjectId) return json; + + JsonObject newJson = new JsonObject(new LinkedHashMap<>(json.size())); + + for (Map.Entry entry : json) { + String key = entry.getKey(); + Object value = entry.getValue(); + if (key.equals(ID_FIELD) + && value instanceof String + && ObjectId.isValid((String) value)) { + newJson.put(key, new JsonObject().put(JsonObjectCodec.OID_FIELD, value)); + } else if (value instanceof JsonObject) { + newJson.put(key, deepEncodeKeyWhenUseObjectId((JsonObject) value)); + } else if (value instanceof Map) { + newJson.put(key, deepEncodeKeyWhenUseObjectId(new JsonObject((Map) value))); + } else if (value instanceof JsonArray) { + newJson.put(key, deepEncodeKeyWhenUseObjectId((JsonArray) value)); + } else if (value instanceof List) { + newJson.put(key, deepEncodeKeyWhenUseObjectId(new JsonArray((List) value))); + } else { + newJson.put(key, value); + } + } + + return newJson; + } + JsonObject encodeKeyWhenUseObjectId(JsonObject json) { - if (!useObjectId) return json; + if (!useObjectId) + return json; Object idString = json.getValue(ID_FIELD, null); if (idString instanceof String && ObjectId.isValid((String) idString)) { @@ -932,7 +986,7 @@ private JsonObject decodeKeyWhenUseObjectId(JsonObject json) { private FindPublisher doFind(String collection, JsonObject query, FindOptions options) { MongoCollection coll = getCollection(collection); - Bson bquery = wrap(encodeKeyWhenUseObjectId(query)); + Bson bquery = wrap(deepEncodeKeyWhenUseObjectId(query)); FindPublisher find = coll.find(bquery, JsonObject.class); if (options.getLimit() != -1) { find.limit(options.getLimit()); diff --git a/src/test/java/io/vertx/ext/mongo/MongoClientWithObjectIdTest.java b/src/test/java/io/vertx/ext/mongo/MongoClientWithObjectIdTest.java index b8f031fa..f322a499 100644 --- a/src/test/java/io/vertx/ext/mongo/MongoClientWithObjectIdTest.java +++ b/src/test/java/io/vertx/ext/mongo/MongoClientWithObjectIdTest.java @@ -1,9 +1,13 @@ package io.vertx.ext.mongo; +import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import org.bson.types.ObjectId; import org.junit.Test; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.CountDownLatch; import static io.vertx.ext.mongo.WriteOption.ACKNOWLEDGED; @@ -77,6 +81,32 @@ public void testFindOneReturnsStringId() throws Exception { await(); } + @Test + public void testFindOneWithNestedQueryReturnsStringId() throws Exception { + String collection = randomCollection(); + mongoClient.createCollection(collection).onComplete(onSuccess(res -> { + JsonObject orig = createDoc(); + JsonObject doc = orig.copy(); + String objectId = getObjectId(doc); + JsonObject query = JsonObject.of("$and", JsonArray.of( + JsonObject.of("foo", "bar"), + JsonObject.of("_id", objectId))); + mongoClient.insert(collection, doc).onComplete(onSuccess(id -> { + // no auto-generated objectId from mongo + assertNull(id); + mongoClient.findOne(collection, query, null).onComplete(onSuccess(obj -> { + assertTrue(obj.containsKey("_id")); + assertTrue(obj.getValue("_id") instanceof String); + obj.remove("_id"); + // nested "_id" will not be modified when insert + assertEquals(orig, obj); + testComplete(); + })); + })); + })); + await(); + } + @Test public void testFindOneReturnsNothing() throws Exception { String collection = randomCollection(); @@ -116,6 +146,65 @@ public void testFindReturnsStringId() throws Exception { await(); } + + @Test + public void testFindWithNestedQueryReturnsStringId() throws Exception { + String collection = randomCollection(); + mongoClient.createCollection(collection).onComplete(onSuccess(res -> { + JsonObject orig = createDoc(); + JsonObject doc = orig.copy(); + String objectId = getObjectId(doc); + JsonObject query = JsonObject.of("$and", JsonArray.of( + JsonObject.of("foo", "bar"), + JsonObject.of("_id", objectId))); + mongoClient.insert(collection, doc).onComplete(onSuccess(id -> { + // no auto-generated objectId from mongo + assertNull(id); + mongoClient.find(collection, query).onComplete(onSuccess(list -> { + assertTrue(list.size() == 1); + JsonObject obj = list.get(0); + assertTrue(obj.containsKey("_id")); + assertTrue(obj.getValue("_id") instanceof String); + obj.remove("_id"); + // nested "_id" will not be modified when insert + assertEquals(orig, obj); + testComplete(); + })); + })); + })); + await(); + } + + @Test + public void testFindWithNestedQueryWithListMapReturnsStringId() throws Exception { + String collection = randomCollection(); + mongoClient.createCollection(collection).onComplete(onSuccess(res -> { + JsonObject orig = createDoc(); + JsonObject doc = orig.copy(); + String objectId = getObjectId(doc); + Map m1 = new HashMap<>(); + m1.put("foo", "bar"); + Map m2 = new HashMap<>(); + m2.put("_id", objectId); + JsonObject query = JsonObject.of("$and", Arrays.asList(m1, m2)); + mongoClient.insert(collection, doc).onComplete(onSuccess(id -> { + // no auto-generated objectId from mongo + assertNull(id); + mongoClient.find(collection, query).onComplete(onSuccess(list -> { + assertTrue(list.size() == 1); + JsonObject obj = list.get(0); + assertTrue(obj.containsKey("_id")); + assertTrue(obj.getValue("_id") instanceof String); + obj.remove("_id"); + // nested "_id" will not be modified when insert + assertEquals(orig, obj); + testComplete(); + })); + })); + })); + await(); + } + @Test @Override public void testInsertPreexistingObjectID() throws Exception { diff --git a/src/test/java/io/vertx/ext/mongo/MongoTestBase.java b/src/test/java/io/vertx/ext/mongo/MongoTestBase.java index 486733ed..b325267d 100644 --- a/src/test/java/io/vertx/ext/mongo/MongoTestBase.java +++ b/src/test/java/io/vertx/ext/mongo/MongoTestBase.java @@ -182,7 +182,14 @@ protected JsonObject createDoc() { .put("myarr", new JsonArray() .add("blah") .add(true) - .add(312))); + .add(312))) + .put("nested_id1", new JsonObject() + .put("_id", new ObjectId().toHexString())) + .put("nested_id2", new JsonArray() + .add(new JsonObject() + .put("_id", new ObjectId().toHexString())) + .add(new JsonObject() + .put("_id", new ObjectId().toHexString()))); } protected JsonObject createDoc(int num) { @@ -210,4 +217,28 @@ protected JsonObject createDocWithAmbiguitiesDependingOnLocale(int num) { .put("longval", 123456789L).put("dblval", 1.23); } + // WARN: try to getObjectId from doc will generate new objectId on doc if not exists + protected String getObjectId(JsonObject doc) { + Object idVal = doc.getValue("_id"); + + // auto generate when not exists + if(idVal == null) { + String _id = new ObjectId().toHexString(); + doc.put("_id", JsonObject.of("$oid", _id)); + return _id; + } + + // return string + if(idVal instanceof String) { + return (String) idVal; + } + + // return $oid from ObjectId object + if(idVal instanceof JsonObject) { + return ((JsonObject) idVal).getString("$oid"); + } + + return null; + } + }