SUP-14472: Two nodes with same path/segment field if binary is filled (…

…#1470) * SUP-14472: Two nodes with same path/segment field if binary is filled * LTS changelog * Unit tests * Warning Co-authored-by: Serhii Plyhun <[email protected]>
gentics · Nov 30, 2022 · 6d4594e · 6d4594e
1 parent a753457
commit 6d4594e
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 3 deletions.
diff --git a/LTS-CHANGELOG.adoc b/LTS-CHANGELOG.adoc
@@ -17,6 +17,11 @@ include::content/docs/variables.adoc-include[]
 The LTS changelog lists releases which are only accessible via a commercial subscription.
 All fixes and changes in LTS releases will be released the next minor release. Changes from LTS 1.4.x will be included in release 1.5.0.
 
+[[v1.8.15]]
+== 1.8.15 (TBD)
+
+icon:check[] Core: Having a binary non-segment field update might break the uniqueness of the segment field value, allowing creation of multiple nodes with the same segment/webroot value. This has now been fixed.
+
 [[v1.8.14]]
 == 1.8.14 (22.11.2022)
 

diff --git a/core/src/main/java/com/gentics/mesh/core/endpoint/node/BinaryUploadHandlerImpl.java b/core/src/main/java/com/gentics/mesh/core/endpoint/node/BinaryUploadHandlerImpl.java
@@ -372,6 +372,8 @@ private Single<NodeResponse> storeUploadInGraph(InternalActionContext ac, List<C
 			newDraftVersion.removeField(oldField);
 
 			// If the binary field is the segment field, we need to update the webroot info in the node
+			// TODO FIXME This is already called in `PersistingContentDao.connectFieldContainer()`. Normally one should not update a container without reconnecting versions,
+			// but currently MeshLocalClient does this, which may be illegal. The check and call below should be removed, once MeshLocalClientImpl is improved.
 			if (field.getFieldKey().equals(contentDao.getSchemaContainerVersion(newDraftVersion).getSchema().getSegmentField())) {
 				contentDao.updateWebrootPathInfo(newDraftVersion, branch.getUuid(), "node_conflicting_segmentfield_upload");
 			}

diff --git a/mdm/common/src/main/java/com/gentics/mesh/core/data/dao/PersistingContentDao.java b/mdm/common/src/main/java/com/gentics/mesh/core/data/dao/PersistingContentDao.java
@@ -235,10 +235,11 @@ default void connectFieldContainer(HibNode node, HibNodeFieldContainer newContai
 			// remove existing draft edge
 			if (draftEdge != null) {
 				removeEdge(draftEdge);
-				updateWebrootPathInfo(newContainer, branchUuid, "node_conflicting_segmentfield_update");
 			}
 			// create a new draft edge
 			createContainerEdge(node, newContainer, branchUuid, languageTag, DRAFT);
+
+			updateWebrootPathInfo(newContainer, branchUuid, "node_conflicting_segmentfield_update");
 		}
 
 		// if there is no initial edge, create one

diff --git a/.../tests-core/src/main/java/com/gentics/mesh/core/node/NodeWebRootConflictEndpointTest.java b/.../tests-core/src/main/java/com/gentics/mesh/core/node/NodeWebRootConflictEndpointTest.java
@@ -6,8 +6,8 @@
 import static com.gentics.mesh.test.TestSize.FULL;
 import static io.netty.handler.codec.http.HttpResponseStatus.CONFLICT;
 
-import com.gentics.mesh.parameter.impl.NodeParametersImpl;
-import com.gentics.mesh.rest.client.MeshWebrootResponse;
+import java.io.ByteArrayInputStream;
+
 import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
@@ -20,11 +20,20 @@
 import com.gentics.mesh.core.rest.node.NodeCreateRequest;
 import com.gentics.mesh.core.rest.node.NodeResponse;
 import com.gentics.mesh.core.rest.node.NodeUpdateRequest;
+import com.gentics.mesh.core.rest.schema.BinaryFieldSchema;
+import com.gentics.mesh.core.rest.schema.StringFieldSchema;
+import com.gentics.mesh.core.rest.schema.impl.BinaryFieldSchemaImpl;
+import com.gentics.mesh.core.rest.schema.impl.SchemaCreateRequest;
 import com.gentics.mesh.core.rest.schema.impl.SchemaReferenceImpl;
+import com.gentics.mesh.core.rest.schema.impl.SchemaResponse;
+import com.gentics.mesh.core.rest.schema.impl.StringFieldSchemaImpl;
 import com.gentics.mesh.parameter.impl.VersioningParametersImpl;
+import com.gentics.mesh.rest.client.MeshWebrootResponse;
 import com.gentics.mesh.test.MeshTestSetting;
 import com.gentics.mesh.test.context.AbstractMeshTest;
 
+import io.vertx.core.buffer.Buffer;
+
 @MeshTestSetting(testSize = FULL, startServer = true)
 public class NodeWebRootConflictEndpointTest extends AbstractMeshTest {
 
@@ -491,4 +500,117 @@ public void testDuplicateWithPublished() {
 		call(() -> client().publishNode(PROJECT_NAME, otherNodeUuid), CONFLICT, "node_conflicting_segmentfield_publish", "slug",
 			conflictingName);
 	}
+
+	@Test
+	public void testBinaryNonSegmentFieldBreaksUniquenessNodeApi() {
+		SchemaResponse binContentSchema = tx(tx -> {
+			SchemaCreateRequest request = new SchemaCreateRequest();
+			request.setName("schema_" + System.currentTimeMillis());
+			request.setContainer(false);
+			request.setSegmentField("name");
+
+			StringFieldSchema nameFieldSchema = new StringFieldSchemaImpl();
+			nameFieldSchema.setName("name");
+			nameFieldSchema.setLabel("Name");
+			request.addField(nameFieldSchema);
+
+			BinaryFieldSchema binaryFieldSchema = new BinaryFieldSchemaImpl();
+			binaryFieldSchema.setName("binary");
+			binaryFieldSchema.setLabel("Binary Data");
+			request.addField(binaryFieldSchema);
+
+			SchemaResponse schema = call(() -> client().createSchema(request));
+
+			call(() -> client().assignSchemaToProject(PROJECT_NAME, schema.getUuid()));
+
+			return schema;
+		});		
+
+		tx(() -> {
+			// create the initial content
+			NodeCreateRequest create = new NodeCreateRequest();
+			create.setParentNodeUuid(folderUuid());
+			create.setLanguage("en");
+			create.setSchema(new SchemaReferenceImpl().setName(binContentSchema.getName()).setUuid(binContentSchema.getUuid()));
+			create.getFields().put("name", FieldUtil.createStringField("title_12345"));
+			NodeResponse node1 = client().createNode(project().getName(), create).toSingle().blockingGet();
+
+			// create the illegal content - node cannot appear with already used segment value
+			NodeCreateRequest illegalCreate = new NodeCreateRequest();
+			illegalCreate.setParentNodeUuid(folderUuid());
+			illegalCreate.setLanguage("en");
+			illegalCreate.setSchema(new SchemaReferenceImpl().setName(binContentSchema.getName()).setUuid(binContentSchema.getUuid()));
+			illegalCreate.getFields().put("name", FieldUtil.createStringField("title_12345"));
+			call(() -> client().createNode(PROJECT_NAME, illegalCreate), CONFLICT, "node_conflicting_segmentfield_update", "name", "title_12345");
+
+			// update binary field
+			Buffer buffer = getBuffer("/pictures/android-gps.jpg");
+			call(() -> client().updateNodeBinaryField(PROJECT_NAME, node1.getUuid(), "en", "0.1", "binary", new ByteArrayInputStream(buffer.getBytes()), buffer.length(), "test.jpg", "image/jpeg"));
+
+			// check the node still cannot appear with the same segment value
+			call(() -> client().createNode(PROJECT_NAME, illegalCreate), CONFLICT, "node_conflicting_segmentfield_update", "name", "title_12345");
+
+			// cleanup
+			call(() -> client().deleteNode(PROJECT_NAME, node1.getUuid()));
+			call(() -> client().unassignSchemaFromProject(PROJECT_NAME, binContentSchema.getUuid()));
+			call(() -> client().deleteSchema(binContentSchema.getUuid()));
+		});
+	}
+
+
+	@Test
+	public void testBinaryNonSegmentFieldBreaksUniquenessWebrootApi() {
+		SchemaResponse binContentSchema = tx(tx -> {
+			SchemaCreateRequest request = new SchemaCreateRequest();
+			request.setName("schema_" + System.currentTimeMillis());
+			request.setContainer(false);
+			request.setSegmentField("name");
+
+			StringFieldSchema nameFieldSchema = new StringFieldSchemaImpl();
+			nameFieldSchema.setName("name");
+			nameFieldSchema.setLabel("Name");
+			request.addField(nameFieldSchema);
+
+			BinaryFieldSchema binaryFieldSchema = new BinaryFieldSchemaImpl();
+			binaryFieldSchema.setName("binary");
+			binaryFieldSchema.setLabel("Binary Data");
+			request.addField(binaryFieldSchema);
+
+			SchemaResponse schema = call(() -> client().createSchema(request));
+
+			call(() -> client().assignSchemaToProject(PROJECT_NAME, schema.getUuid()));
+
+			return schema;
+		});		
+
+		tx(() -> {
+			// create the initial content
+			NodeCreateRequest create = new NodeCreateRequest();
+			create.setParentNodeUuid(folderUuid());
+			create.setLanguage("en");
+			create.setSchema(new SchemaReferenceImpl().setName(binContentSchema.getName()).setUuid(binContentSchema.getUuid()));
+			create.getFields().put("name", FieldUtil.createStringField("title_12345"));
+			NodeResponse node1 = client().webrootCreate(PROJECT_NAME, "/title_12345", create).toSingle().blockingGet();
+
+			// create the illegal content - node cannot appear with already used segment value
+			NodeCreateRequest illegalCreate = new NodeCreateRequest();
+			illegalCreate.setParentNodeUuid(folderUuid());
+			illegalCreate.setLanguage("en");
+			illegalCreate.setSchema(new SchemaReferenceImpl().setName(binContentSchema.getName()).setUuid(binContentSchema.getUuid()));
+			illegalCreate.getFields().put("name", FieldUtil.createStringField("title_12345"));
+			call(() -> client().webrootCreate(PROJECT_NAME, "/title_12345", illegalCreate), CONFLICT, "node_conflicting_segmentfield_update", "name", "title_12345");
+
+			// update binary field
+			Buffer buffer = getBuffer("/pictures/android-gps.jpg");
+			call(() -> client().updateNodeBinaryField(PROJECT_NAME, node1.getUuid(), "en", "0.1", "binary", new ByteArrayInputStream(buffer.getBytes()), buffer.length(), "test.jpg", "image/jpeg"));
+
+			// check the node still cannot appear with the same segment value
+			call(() -> client().webrootCreate(PROJECT_NAME, "/title_12345", illegalCreate), CONFLICT, "node_conflicting_segmentfield_update", "name", "title_12345");
+
+			// cleanup
+			call(() -> client().deleteNode(PROJECT_NAME, node1.getUuid()));
+			call(() -> client().unassignSchemaFromProject(PROJECT_NAME, binContentSchema.getUuid()));
+			call(() -> client().deleteSchema(binContentSchema.getUuid()));
+		});
+	}
 }