Skip to content

Commit

Permalink
Fix for Bug#107510 (Bug#34259416), Empty string given to set() from C…
Browse files Browse the repository at this point in the history
…ollection.modify() replaces full document.

Change-Id: Iebd779f2467cd9c34e4a8931def97bd93cd6cf9b
  • Loading branch information
fjssilva committed Jun 8, 2022
1 parent 0604bf4 commit 32892f1
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 18 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.30

- Fix for Bug#107510 (Bug#34259416), Empty string given to set() from Collection.modify() replaces full document.

- Fix for Bug#106758 (33973048), DatabaseMetaData.getTypeInfo returns AUTO_INCREMENT = false for all datatypes.

- Fix for Bug#34090350, Update mappings for utf8mb3 and utf8mb4 collations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ Blob.invalidStreamPos=Position ''pos'' can not be < 1 or > blob length.
Blob.8=Emulated BLOB locators must come from a ResultSet with only one table selected, and all primary keys selected
Blob.9=BLOB data not found! Did primary keys change?


Buffer.0=Payload length can not be larger than buffer size.
Buffer.1=Buffer length is less than expected payload length.

Expand Down Expand Up @@ -190,7 +189,6 @@ ConnectionString.24=Using named pipes with DNS SRV lookup is not allowed.
ConnectionString.25=The option ''{0}'' cannot be set. Live management of connections is not supported with DNS SRV lookup.
ConnectionString.26=Unable to locate any hosts for {0}.


ConnectionWrapper.0=Can''t set autocommit to ''true'' on an XAConnection
ConnectionWrapper.1=Can''t call commit() on an XAConnection associated with a global transaction
ConnectionWrapper.2=Can''t call rollback() on an XAConnection associated with a global transaction
Expand Down Expand Up @@ -676,6 +674,7 @@ Util.5=Error reading from InputStream
#
# Exceptions
#

AssertionFailedException.0=ASSERTION FAILED: Exception
AssertionFailedException.1=\ that should not be thrown, was thrown
AssertionFailedException.2=ASSERTION FAILED: {0}
Expand Down
14 changes: 7 additions & 7 deletions src/main/user-api/java/com/mysql/cj/xdevapi/ModifyStatement.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, 2022, 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 @@ -76,11 +76,11 @@ public interface ModifyStatement extends Statement<ModifyStatement, Result> {
/**
* Nullify the given fields.
*
* @param fields
* @param docPath
* one or more field names
* @return {@link ModifyStatement}
*/
ModifyStatement unset(String... fields);
ModifyStatement unset(String... docPath);

/**
* Takes in a patch object and applies it on all documents matching the modify() filter, using the JSON_MERGE_PATCH() function.
Expand Down Expand Up @@ -111,22 +111,22 @@ public interface ModifyStatement extends Statement<ModifyStatement, Result> {
/**
* Insert a value into the specified array.
*
* @param field
* @param docPath
* document path to the array field
* @param value
* value to insert
* @return {@link ModifyStatement}
*/
ModifyStatement arrayInsert(String field, Object value);
ModifyStatement arrayInsert(String docPath, Object value);

/**
* Append a value to the specified array.
*
* @param field
* @param docPath
* document path to the array field
* @param value
* value to append
* @return {@link ModifyStatement}
*/
ModifyStatement arrayAppend(String field, Object value);
ModifyStatement arrayAppend(String docPath, Object value);
}
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, 2022, 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 @@ -49,7 +49,7 @@ public class ModifyStatementImpl extends FilterableStatement<ModifyStatement, Re
/* package private */ ModifyStatementImpl(MysqlxSession mysqlxSession, String schema, String collection, String criteria) {
super(new DocFilterParams(schema, collection, false));
this.mysqlxSession = mysqlxSession;
if (criteria == null || criteria.trim().length() == 0) {
if (criteria == null || criteria.trim().isEmpty()) {
throw new XDevAPIError(Messages.getString("ModifyStatement.0", new String[] { "criteria" }));
}
this.filterParams.setCriteria(criteria);
Expand Down Expand Up @@ -95,9 +95,12 @@ public ModifyStatement change(String docPath, Object value) {
}

@Override
public ModifyStatement unset(String... fields) {
public ModifyStatement unset(String... docPath) {
resetPrepareState();
this.updates.addAll(Arrays.stream(fields).map(docPath -> new UpdateSpec(UpdateType.ITEM_REMOVE, docPath)).collect(Collectors.toList()));
if (docPath == null) {
throw new XDevAPIError(Messages.getString("ModifyStatement.0", new String[] { "docPath" }));
}
this.updates.addAll(Arrays.stream(docPath).map(dp -> new UpdateSpec(UpdateType.ITEM_REMOVE, dp)).collect(Collectors.toList()));
return this;
}

Expand All @@ -110,14 +113,14 @@ public ModifyStatement patch(DbDoc document) {
@Override
public ModifyStatement patch(String document) {
resetPrepareState();
this.updates.add(new UpdateSpec(UpdateType.MERGE_PATCH, "").setValue(Expression.expr(document)));
this.updates.add(new UpdateSpec(UpdateType.MERGE_PATCH).setValue(Expression.expr(document)));
return this;
}

@Override
public ModifyStatement arrayInsert(String field, Object value) {
public ModifyStatement arrayInsert(String docPath, Object value) {
resetPrepareState();
this.updates.add(new UpdateSpec(UpdateType.ARRAY_INSERT, field).setValue(value));
this.updates.add(new UpdateSpec(UpdateType.ARRAY_INSERT, docPath).setValue(value));
return this;
}

Expand Down
17 changes: 16 additions & 1 deletion src/main/user-impl/java/com/mysql/cj/xdevapi/UpdateSpec.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, 2022, 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 @@ -29,6 +29,7 @@

package com.mysql.cj.xdevapi;

import com.mysql.cj.Messages;
import com.mysql.cj.x.protobuf.MysqlxCrud.UpdateOperation;
import com.mysql.cj.x.protobuf.MysqlxExpr.ColumnIdentifier;
import com.mysql.cj.x.protobuf.MysqlxExpr.Expr;
Expand All @@ -43,6 +44,17 @@ public class UpdateSpec {
private ColumnIdentifier source;
private Expr value;

/**
* Constructor.
*
* @param updateType
* update operation type
*/
public UpdateSpec(UpdateType updateType) {
this.updateType = UpdateOperation.UpdateType.valueOf(updateType.name());
this.source = ColumnIdentifier.getDefaultInstance();
}

/**
* Constructor.
*
Expand All @@ -53,6 +65,9 @@ public class UpdateSpec {
*/
public UpdateSpec(UpdateType updateType, String source) {
this.updateType = UpdateOperation.UpdateType.valueOf(updateType.name());
if (source == null || source.trim().isEmpty()) {
throw new XDevAPIError(Messages.getString("ModifyStatement.0", new String[] { "docPath" }));
}
// accommodate parser's documentField() handling by removing "$"
if (source.length() > 0 && source.charAt(0) == '$') {
source = source.substring(1);
Expand Down
90 changes: 89 additions & 1 deletion src/test/java/testsuite/x/devapi/CollectionModifyTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2021, Oracle and/or its affiliates.
* Copyright (c) 2015, 2022, 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 @@ -49,6 +49,7 @@
import org.junit.jupiter.api.Test;

import com.mysql.cj.ServerVersion;
import com.mysql.cj.exceptions.CJException;
import com.mysql.cj.xdevapi.AddResult;
import com.mysql.cj.xdevapi.Collection;
import com.mysql.cj.xdevapi.DbDoc;
Expand Down Expand Up @@ -1890,4 +1891,91 @@ public void testCollectionModifyAsyncMany() throws Exception {
doc = docs.next();
assertEquals((long) (maxrec) / 2, (long) (((JsonNumber) doc.get("cnt")).getInteger()));
}

/**
* Tests fix for Bug#107510 (Bug#34259416), Empty string given to set() from Collection.modify() replaces full document.
*
* @throws Exception
*/
@Test
public void testBug107510() throws Exception {
this.collection.add("{\"bug\": \"testBug107510\"}").execute();
DbDoc doc = this.collection.find().execute().fetchOne();
assertEquals("testBug107510", ((JsonString) doc.get("bug")).getString());

// .set()
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").set("", JsonParser.parseDoc("{\"bug\": \"testBug34259416\"}")).execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").set(null, JsonParser.parseDoc("{\"bug\": \"testBug34259416\"}")).execute();
return null;
});
doc = this.collection.find().execute().fetchOne();
assertEquals("testBug107510", ((JsonString) doc.get("bug")).getString());

this.collection.modify("true").set("$", JsonParser.parseDoc("{\"bug\": \"testBug34259416\"}")).execute();
doc = this.collection.find().execute().fetchOne();
assertEquals("testBug34259416", ((JsonString) doc.get("bug")).getString());

this.collection.modify("true").set("$.type", "BUG1").execute();
doc = this.collection.find().execute().fetchOne();
assertEquals("testBug34259416", ((JsonString) doc.get("bug")).getString());
assertEquals("BUG1", ((JsonString) doc.get("type")).getString());

this.collection.modify("true").set(".type", "BUG2").execute();
doc = this.collection.find().execute().fetchOne();
assertEquals("testBug34259416", ((JsonString) doc.get("bug")).getString());
assertEquals("BUG2", ((JsonString) doc.get("type")).getString());

this.collection.modify("true").set("type", "BUG3").execute();
doc = this.collection.find().execute().fetchOne();
assertEquals("testBug34259416", ((JsonString) doc.get("bug")).getString());
assertEquals("BUG3", ((JsonString) doc.get("type")).getString());

// .unset()
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").unset("").execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").unset((String) null).execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").unset((String[]) null).execute();
return null;
});

// .change()
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").change("", "").execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").change(null, "").execute();
return null;
});

// .arrayAppend()
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").arrayAppend("", "").execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").arrayAppend(null, "").execute();
return null;
});

// .arrayInsert()
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").arrayInsert("", "").execute();
return null;
});
assertThrows(CJException.class, "Parameter 'docPath' must not be null or empty\\.", () -> {
this.collection.modify("true").arrayInsert(null, "").execute();
return null;
});
}
}

0 comments on commit 32892f1

Please sign in to comment.