Skip to content

Commit

Permalink
Fix for Bug#103324 (32770013), X DevAPI Collection.replaceOne() missi…
Browse files Browse the repository at this point in the history
…ng matching _id check.
  • Loading branch information
fjssilva committed Nov 25, 2021
1 parent 48219f2 commit 05778ef
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 39 deletions.
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

Version 8.0.28

- Fix for Bug#103324 (32770013), X DevAPI Collection.replaceOne() missing matching _id check.

- Fix for Bug#105197 (33461744), Statement.executeQuery() may return non-navigable ResultSet.

- Fix for Bug#105323 (33507321), README.md contains broken links.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ Clob.11=Cannot truncate CLOB of length
Clob.12=\ to length of
Clob.13=.

Collection.DocIdMismatch=Replacement document has an _id that is different than the matched document.

ColumnDefinition.0={0} is not applicable to the {1} type of column ''{2}''.
ColumnDefinition.1=Length must be specified before decimals for column ''{0}''.

Expand Down
13 changes: 9 additions & 4 deletions src/main/user-impl/java/com/mysql/cj/xdevapi/CollectionImpl.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates.
* Copyright (c) 2015, 2021, Oracle and/or its affiliates.
*
* This program is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, version 2.0, as published by the
Expand Down Expand Up @@ -188,6 +188,10 @@ public Result replaceOne(String id, DbDoc doc) {
if (doc == null) {
throw new XDevAPIError(Messages.getString("CreateTableStatement.0", new String[] { "doc" }));
}
JsonValue docId = doc.get("_id");
if (docId != null && (!JsonString.class.isInstance(docId) || !id.equals(((JsonString) docId).getString()))) {
throw new XDevAPIError(Messages.getString("Collection.DocIdMismatch"));
}
return modify("_id = :id").set("$", doc).bind("id", id).execute();
}

Expand All @@ -214,10 +218,11 @@ public Result addOrReplaceOne(String id, DbDoc doc) {
if (doc == null) {
throw new XDevAPIError(Messages.getString("CreateTableStatement.0", new String[] { "doc" }));
}
if (doc.get("_id") == null) {
JsonValue docId = doc.get("_id");
if (docId == null) {
doc.add("_id", new JsonString().setValue(id));
} else if (!id.equals(((JsonString) doc.get("_id")).getString())) {
throw new XDevAPIError("Document already has an _id that doesn't match to id parameter");
} else if (!JsonString.class.isInstance(docId) || !id.equals(((JsonString) docId).getString())) {
throw new XDevAPIError(Messages.getString("Collection.DocIdMismatch"));
}
return add(doc).setUpsert(true).execute();
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/testsuite/x/devapi/CollectionAddTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void testAddOrReplaceOne() {
assertTrue(this.collection.find("a = 4").execute().hasNext());

// a new document with _id field that doesn't match id parameter
assertThrows(XDevAPIError.class, "Document already has an _id that doesn't match to id parameter", new Callable<Void>() {
assertThrows(XDevAPIError.class, "Replacement document has an _id that is different than the matched document\\.", new Callable<Void>() {
public Void call() throws Exception {
CollectionAddTest.this.collection.addOrReplaceOne("id2",
CollectionAddTest.this.collection.newDoc().add("_id", new JsonString().setValue("id111")));
Expand Down
53 changes: 19 additions & 34 deletions src/test/java/testsuite/x/devapi/CollectionModifyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -549,42 +549,23 @@ public void testReplaceOne() {

DbDoc doc = this.collection.getOne("existingId");
assertNotNull(doc);
assertEquals(new Integer(2), ((JsonNumber) doc.get("a")).getInteger());
assertEquals(2, ((JsonNumber) doc.get("a")).getInteger());

res = this.collection.replaceOne("notExistingId", "{\"_id\":\"existingId\",\"a\":3}");
assertEquals(0, res.getAffectedItemsCount());

res = this.collection.replaceOne("", "{\"_id\":\"existingId\",\"a\":3}");
assertEquals(0, res.getAffectedItemsCount());

/*
* FR5.1 The id of the document must remain immutable:
*
* Use a collection with some documents
* Fetch a document
* Modify _id: _new_id_ and modify any other field of the document
* Call replaceOne() giving original ID and modified document: expect affected = 1
* Fetch the document again, ensure other document modifications took place
* Ensure no document with _new_id_ was added to the collection
*/
this.collection.remove("1=1").execute();
assertEquals(0, this.collection.count());
this.collection.add("{\"_id\":\"id1\",\"a\":1}").execute();

doc = this.collection.getOne("id1");
assertNotNull(doc);
((JsonString) doc.get("_id")).setValue("id2");
((JsonNumber) doc.get("a")).setValue("2");
res = this.collection.replaceOne("id1", doc);
assertEquals(1, res.getAffectedItemsCount());

doc = this.collection.getOne("id1");
assertNotNull(doc);
assertEquals("id1", ((JsonString) doc.get("_id")).getString());
assertEquals(new Integer(2), ((JsonNumber) doc.get("a")).getInteger());
// Original behavior changed by Bug#32770013.
assertThrows(XDevAPIError.class, "Replacement document has an _id that is different than the matched document\\.", new Callable<Void>() {
public Void call() throws Exception {
CollectionModifyTest.this.collection.replaceOne("nonExistingId", "{\"_id\":\"existingId\",\"a\":3}");
return null;
}
});

doc = this.collection.getOne("id2");
assertNull(doc);
// Original behavior changed by Bug#32770013.
assertThrows(XDevAPIError.class, "Replacement document has an _id that is different than the matched document\\.", new Callable<Void>() {
public Void call() throws Exception {
CollectionModifyTest.this.collection.replaceOne("", "{\"_id\":\"existingId\",\"a\":3}");
return null;
}
});

/*
* FR5.2 The id of the document must remain immutable:
Expand All @@ -596,6 +577,10 @@ public void testReplaceOne() {
* Fetch the document again, ensure other document modifications took place
* Ensure the number of documents in the collection is unaltered
*/
this.collection.remove("true").execute();
assertEquals(0, this.collection.count());
this.collection.add("{\"_id\":\"id1\",\"a\":1}").execute();

doc = this.collection.getOne("id1");
assertNotNull(doc);
doc.remove("_id");
Expand Down

0 comments on commit 05778ef

Please sign in to comment.