Skip to content

Commit 05778ef

Browse files
committed
Fix for Bug#103324 (32770013), X DevAPI Collection.replaceOne() missing matching _id check.
1 parent 48219f2 commit 05778ef

File tree

5 files changed

+33
-39
lines changed

5 files changed

+33
-39
lines changed

CHANGES

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
Version 8.0.28
55

6+
- Fix for Bug#103324 (32770013), X DevAPI Collection.replaceOne() missing matching _id check.
7+
68
- Fix for Bug#105197 (33461744), Statement.executeQuery() may return non-navigable ResultSet.
79

810
- Fix for Bug#105323 (33507321), README.md contains broken links.

src/main/resources/com/mysql/cj/LocalizedErrorMessages.properties

+2
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ Clob.11=Cannot truncate CLOB of length
103103
Clob.12=\ to length of
104104
Clob.13=.
105105

106+
Collection.DocIdMismatch=Replacement document has an _id that is different than the matched document.
107+
106108
ColumnDefinition.0={0} is not applicable to the {1} type of column ''{2}''.
107109
ColumnDefinition.1=Length must be specified before decimals for column ''{0}''.
108110

src/main/user-impl/java/com/mysql/cj/xdevapi/CollectionImpl.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2020, Oracle and/or its affiliates.
2+
* Copyright (c) 2015, 2021, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify it under
55
* the terms of the GNU General Public License, version 2.0, as published by the
@@ -188,6 +188,10 @@ public Result replaceOne(String id, DbDoc doc) {
188188
if (doc == null) {
189189
throw new XDevAPIError(Messages.getString("CreateTableStatement.0", new String[] { "doc" }));
190190
}
191+
JsonValue docId = doc.get("_id");
192+
if (docId != null && (!JsonString.class.isInstance(docId) || !id.equals(((JsonString) docId).getString()))) {
193+
throw new XDevAPIError(Messages.getString("Collection.DocIdMismatch"));
194+
}
191195
return modify("_id = :id").set("$", doc).bind("id", id).execute();
192196
}
193197

@@ -214,10 +218,11 @@ public Result addOrReplaceOne(String id, DbDoc doc) {
214218
if (doc == null) {
215219
throw new XDevAPIError(Messages.getString("CreateTableStatement.0", new String[] { "doc" }));
216220
}
217-
if (doc.get("_id") == null) {
221+
JsonValue docId = doc.get("_id");
222+
if (docId == null) {
218223
doc.add("_id", new JsonString().setValue(id));
219-
} else if (!id.equals(((JsonString) doc.get("_id")).getString())) {
220-
throw new XDevAPIError("Document already has an _id that doesn't match to id parameter");
224+
} else if (!JsonString.class.isInstance(docId) || !id.equals(((JsonString) docId).getString())) {
225+
throw new XDevAPIError(Messages.getString("Collection.DocIdMismatch"));
221226
}
222227
return add(doc).setUpsert(true).execute();
223228
}

src/test/java/testsuite/x/devapi/CollectionAddTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ public void testAddOrReplaceOne() {
260260
assertTrue(this.collection.find("a = 4").execute().hasNext());
261261

262262
// a new document with _id field that doesn't match id parameter
263-
assertThrows(XDevAPIError.class, "Document already has an _id that doesn't match to id parameter", new Callable<Void>() {
263+
assertThrows(XDevAPIError.class, "Replacement document has an _id that is different than the matched document\\.", new Callable<Void>() {
264264
public Void call() throws Exception {
265265
CollectionAddTest.this.collection.addOrReplaceOne("id2",
266266
CollectionAddTest.this.collection.newDoc().add("_id", new JsonString().setValue("id111")));

src/test/java/testsuite/x/devapi/CollectionModifyTest.java

+19-34
Original file line numberDiff line numberDiff line change
@@ -549,42 +549,23 @@ public void testReplaceOne() {
549549

550550
DbDoc doc = this.collection.getOne("existingId");
551551
assertNotNull(doc);
552-
assertEquals(new Integer(2), ((JsonNumber) doc.get("a")).getInteger());
552+
assertEquals(2, ((JsonNumber) doc.get("a")).getInteger());
553553

554-
res = this.collection.replaceOne("notExistingId", "{\"_id\":\"existingId\",\"a\":3}");
555-
assertEquals(0, res.getAffectedItemsCount());
556-
557-
res = this.collection.replaceOne("", "{\"_id\":\"existingId\",\"a\":3}");
558-
assertEquals(0, res.getAffectedItemsCount());
559-
560-
/*
561-
* FR5.1 The id of the document must remain immutable:
562-
*
563-
* Use a collection with some documents
564-
* Fetch a document
565-
* Modify _id: _new_id_ and modify any other field of the document
566-
* Call replaceOne() giving original ID and modified document: expect affected = 1
567-
* Fetch the document again, ensure other document modifications took place
568-
* Ensure no document with _new_id_ was added to the collection
569-
*/
570-
this.collection.remove("1=1").execute();
571-
assertEquals(0, this.collection.count());
572-
this.collection.add("{\"_id\":\"id1\",\"a\":1}").execute();
573-
574-
doc = this.collection.getOne("id1");
575-
assertNotNull(doc);
576-
((JsonString) doc.get("_id")).setValue("id2");
577-
((JsonNumber) doc.get("a")).setValue("2");
578-
res = this.collection.replaceOne("id1", doc);
579-
assertEquals(1, res.getAffectedItemsCount());
580-
581-
doc = this.collection.getOne("id1");
582-
assertNotNull(doc);
583-
assertEquals("id1", ((JsonString) doc.get("_id")).getString());
584-
assertEquals(new Integer(2), ((JsonNumber) doc.get("a")).getInteger());
554+
// Original behavior changed by Bug#32770013.
555+
assertThrows(XDevAPIError.class, "Replacement document has an _id that is different than the matched document\\.", new Callable<Void>() {
556+
public Void call() throws Exception {
557+
CollectionModifyTest.this.collection.replaceOne("nonExistingId", "{\"_id\":\"existingId\",\"a\":3}");
558+
return null;
559+
}
560+
});
585561

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

589570
/*
590571
* FR5.2 The id of the document must remain immutable:
@@ -596,6 +577,10 @@ public void testReplaceOne() {
596577
* Fetch the document again, ensure other document modifications took place
597578
* Ensure the number of documents in the collection is unaltered
598579
*/
580+
this.collection.remove("true").execute();
581+
assertEquals(0, this.collection.count());
582+
this.collection.add("{\"_id\":\"id1\",\"a\":1}").execute();
583+
599584
doc = this.collection.getOne("id1");
600585
assertNotNull(doc);
601586
doc.remove("_id");

0 commit comments

Comments
 (0)