This is an automated email from the ASF dual-hosted git repository.
nfsantos pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new fc066c3d68 OAK-11658 - Nodes of type nt:resource in new index
definitions must contain a jcr:uuid and it must be unique (#2231)
fc066c3d68 is described below
commit fc066c3d6815c373bcc348d89dcc7465a6276ac6
Author: Nuno Santos <[email protected]>
AuthorDate: Fri Apr 18 09:39:44 2025 +0200
OAK-11658 - Nodes of type nt:resource in new index definitions must contain
a jcr:uuid and it must be unique (#2231)
---
.../index/importer/IndexDefinitionUpdater.java | 87 +++++++++++++++-----
.../index/importer/IndexDefinitionUpdaterTest.java | 96 ++++++++++++++++------
.../index/importer/index-def-jcruuid-fix.json | 24 ++++++
3 files changed, 165 insertions(+), 42 deletions(-)
diff --git
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdater.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdater.java
index 08571d448b..18944e0ac9 100644
---
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdater.java
+++
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdater.java
@@ -18,16 +18,12 @@
*/
package org.apache.jackrabbit.oak.plugins.index.importer;
-import java.io.File;
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-
import org.apache.commons.io.FileUtils;
+import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.commons.conditions.Validate;
import org.apache.jackrabbit.oak.commons.json.JsopReader;
@@ -43,6 +39,15 @@ import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
import static
org.apache.jackrabbit.oak.commons.conditions.Validate.checkArgument;
import static
org.apache.jackrabbit.oak.plugins.index.importer.NodeStoreUtils.childBuilder;
import static
org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
@@ -52,7 +57,8 @@ public class IndexDefinitionUpdater {
* Name of file which would be check for presence of index-definitions
*/
public static final String INDEX_DEFINITIONS_JSON =
"index-definitions.json";
- private final Logger log = LoggerFactory.getLogger(getClass());
+ private static final Logger LOG =
LoggerFactory.getLogger(IndexDefinitionUpdater.class);
+
private final Map<String, NodeState> indexNodeStates;
public IndexDefinitionUpdater(File file) throws IOException {
@@ -75,25 +81,24 @@ public class IndexDefinitionUpdater {
String indexNodeName = PathUtils.getName(indexPath);
NodeState newDefinition = indexNodeStates.get(indexPath);
+ newDefinition = addOrModifyJcrUUID(newDefinition);
String parentPath = PathUtils.getParentPath(indexPath);
NodeState parent = NodeStateUtils.getNode(rootBuilder.getBaseState(),
parentPath);
Validate.checkState(parent.exists(), "Parent node at path [%s] not
found while " +
"adding new index definition for [%s]. Intermediate paths node
must exist for new index " +
- "nodes to be created", parentPath,indexPath);
+ "nodes to be created", parentPath, indexPath);
NodeState indexDefinitionNode = parent.getChildNode(indexNodeName);
if (indexDefinitionNode.exists()) {
- log.info("Updating index definition at path [{}]. Changes are ",
indexPath);
+ LOG.info("Updating index definition at path [{}]. Changes are ",
indexPath);
String diff =
JsopDiff.diffToJsop(cloneVisibleState(indexDefinitionNode),
cloneVisibleState(newDefinition));
- log.info(diff);
+ LOG.info(diff);
} else {
- log.info("Adding new index definition at path [{}]", indexPath);
+ LOG.info("Adding new index definition at path [{}]", indexPath);
}
-
-
NodeBuilder indexBuilderParent = childBuilder(rootBuilder, parentPath);
//Use Tree api to ensure that :childOrder property is properly updated
@@ -104,15 +109,15 @@ public class IndexDefinitionUpdater {
return indexBuilderParent.getChildNode(indexNodeName);
}
- public Set<String> getIndexPaths(){
+ public Set<String> getIndexPaths() {
return indexNodeStates.keySet();
}
- public NodeState getIndexState(String indexPath){
+ public NodeState getIndexState(String indexPath) {
return indexNodeStates.getOrDefault(indexPath, EMPTY_NODE);
}
- private static Map<String, NodeState> getIndexDefnStates(String json)
throws IOException {
+ private static Map<String, NodeState> getIndexDefnStates(String json) {
Base64BlobSerializer blobHandler = new Base64BlobSerializer();
Map<String, NodeState> indexDefns = new HashMap<>();
JsopReader reader = new JsopTokenizer(json);
@@ -139,12 +144,18 @@ public class IndexDefinitionUpdater {
return indexDefns;
}
- private static NodeState cloneVisibleState(NodeState state){
+ private static NodeState cloneVisibleState(NodeState state) {
NodeBuilder builder = EMPTY_NODE.builder();
new ApplyVisibleDiff(builder).apply(state);
return builder.getNodeState();
}
+ private static NodeState addOrModifyJcrUUID(NodeState state) {
+ NodeBuilder builder = EMPTY_NODE.builder();
+ new AddOrUpdateJCRUuid(builder).apply(state);
+ return builder.getNodeState();
+ }
+
private static class ApplyVisibleDiff extends ApplyDiff {
public ApplyVisibleDiff(NodeBuilder builder) {
super(builder);
@@ -152,11 +163,49 @@ public class IndexDefinitionUpdater {
@Override
public boolean childNodeAdded(String name, NodeState after) {
- if (NodeStateUtils.isHidden(name)){
+ if (NodeStateUtils.isHidden(name)) {
return true;
}
return after.compareAgainstBaseState(
EMPTY_NODE, new ApplyVisibleDiff(builder.child(name)));
}
}
+
+ /**
+ * Change the new index definitions to comply with Oak requirements that
all nt:resource nodes
+ * must have a jcr:uuid property. Additionally, if the definition contains
nt:resource nodes
+ * with a jcr:uuid, replace them by new UUIDs to avoid any possible
conflicts with the existing
+ * definitions. This is for cases where the new definition is copied from
an existing one and
+ * the UUIDs are not changed.
+ */
+ private static class AddOrUpdateJCRUuid extends ApplyDiff {
+ public AddOrUpdateJCRUuid(NodeBuilder builder) {
+ super(builder);
+ }
+
+ @Override
+ public boolean propertyAdded(PropertyState after) {
+ if (after.getName().equals(JcrConstants.JCR_UUID)) {
+ addNewJcrUUIDProperty(builder);
+ } else {
+ builder.setProperty(after);
+ }
+ return true;
+ }
+
+ @Override
+ public boolean childNodeAdded(String name, NodeState after) {
+ NodeBuilder newChild = builder.setChildNode(name, after);
+ String primaryType = after.getName(JcrConstants.JCR_PRIMARYTYPE);
+ if (primaryType != null &&
primaryType.equals(JcrConstants.NT_RESOURCE) &&
!newChild.hasProperty(JcrConstants.JCR_UUID)) {
+ addNewJcrUUIDProperty(newChild);
+ }
+ return after.compareAgainstBaseState(EMPTY_NODE, new
AddOrUpdateJCRUuid(builder.child(name)));
+ }
+
+ private static void addNewJcrUUIDProperty(NodeBuilder builder) {
+ String uuid =
UUID.randomUUID().toString().toLowerCase(Locale.ROOT);
+ builder.setProperty(JcrConstants.JCR_UUID, uuid, Type.STRING);
+ }
+ }
}
diff --git
a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdaterTest.java
b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdaterTest.java
index 78d00fa96e..09be2490de 100644
---
a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdaterTest.java
+++
b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexDefinitionUpdaterTest.java
@@ -18,15 +18,9 @@
*/
package org.apache.jackrabbit.oak.plugins.index.importer;
-import java.io.File;
-import java.io.IOException;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
+import org.apache.commons.io.IOUtils;
import org.apache.felix.inventory.Format;
+import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Tree;
@@ -42,24 +36,39 @@ import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.jetbrains.annotations.Nullable;
import org.json.simple.JSONObject;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
import static java.util.Arrays.asList;
import static
org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
public class IndexDefinitionUpdaterTest {
@Rule
public TemporaryFolder folder = new TemporaryFolder(new File("target"));
- private NodeStore store = new MemoryNodeStore();
+ private final NodeStore store = new MemoryNodeStore();
@Test
- public void update() throws Exception{
+ public void update() throws Exception {
NodeBuilder builder = store.getRoot().builder();
builder.child("oak:index").child("fooIndex").setProperty("foo", "bar");
builder.child("oak:index").child("fooIndex").child("a").setProperty("foo2", 2);
@@ -71,15 +80,15 @@ public class IndexDefinitionUpdaterTest {
NodeState root = store.getRoot();
-
assertTrue(NodeStateUtils.getNode(root,"/oak:index/fooIndex").exists());
-
assertTrue(NodeStateUtils.getNode(root,"/oak:index/fooIndex/b").exists());
-
assertFalse(NodeStateUtils.getNode(root,"/oak:index/fooIndex/a").exists());
+ assertTrue(NodeStateUtils.getNode(root,
"/oak:index/fooIndex").exists());
+ assertTrue(NodeStateUtils.getNode(root,
"/oak:index/fooIndex/b").exists());
+ assertFalse(NodeStateUtils.getNode(root,
"/oak:index/fooIndex/a").exists());
-
assertTrue(NodeStateUtils.getNode(root,"/oak:index/barIndex").exists());
+ assertTrue(NodeStateUtils.getNode(root,
"/oak:index/barIndex").exists());
}
- @Test (expected = IllegalStateException.class)
- public void updateNonExistingParent() throws Exception{
+ @Test(expected = IllegalStateException.class)
+ public void updateNonExistingParent() throws Exception {
NodeBuilder builder = store.getRoot().builder();
builder.child("oak:index").child("fooIndex").setProperty("foo", "bar");
@@ -91,8 +100,8 @@ public class IndexDefinitionUpdaterTest {
applyJson(json);
}
- @Test (expected = IllegalArgumentException.class)
- public void invalidJson() throws Exception{
+ @Test(expected = IllegalArgumentException.class)
+ public void invalidJson() throws Exception {
Map<String, Object> map = new HashMap<>();
map.put("a", Map.of("a2", "b2"));
String json = JSONObject.toJSONString(map);
@@ -100,7 +109,7 @@ public class IndexDefinitionUpdaterTest {
}
@Test
- public void applyToIndexPath() throws Exception{
+ public void applyToIndexPath() throws Exception {
String json = "{\"/oak:index/barIndex\": {\n" +
" \"compatVersion\": 2,\n" +
" \"type\": \"lucene\",\n" +
@@ -122,7 +131,7 @@ public class IndexDefinitionUpdaterTest {
}
@Test
- public void newIndexAndOrderableChildren() throws Exception{
+ public void newIndexAndOrderableChildren() throws Exception {
String json = "{\"/oak:index/barIndex\": {\n" +
" \"compatVersion\": 2,\n" +
" \"type\": \"lucene\",\n" +
@@ -149,6 +158,48 @@ public class IndexDefinitionUpdaterTest {
}
+
+ @Test
+ public void fixJCRUUID() throws Exception {
+ String json;
+ try (InputStream is =
getClass().getResourceAsStream("/org/apache/jackrabbit/oak/plugins/index/importer/index-def-jcruuid-fix.json"))
{
+ json = IOUtils.toString(is, StandardCharsets.UTF_8);
+ }
+
+ NodeBuilder builder = store.getRoot().builder();
+ Tree root = TreeFactory.createTree(builder);
+ Tree oakIndex = root.addChild("oak:index");
+ store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+ IndexDefinitionUpdater updater = new IndexDefinitionUpdater(json);
+ builder = store.getRoot().builder();
+
+ NodeBuilder idxBuilder = updater.apply(builder, "/oak:index/barIndex");
+
+ NodeBuilder barIndexBuilder = builder
+ .getChildNode("oak:index")
+ .getChildNode("barIndex");
+ // The node config_without_uuid.xml does not contain any jcr:uuid, so
we only check that now there is one
+ checkContainsJcrUUID(barIndexBuilder
+ .getChildNode("config_without_uuid.xml")
+ .getChildNode("jcr:content"));
+
+ // This node config_with_uuid.xml contains a jcr:uuid, so check that
the new jcr:uuid is different
+ UUID newUuid = checkContainsJcrUUID(barIndexBuilder
+ .getChildNode("config_with_uuid.xml")
+ .getChildNode("jcr:content"));
+
assertFalse(newUuid.toString().equalsIgnoreCase("6db23ff7-986d-4c3d-bf3d-ea9eaf8b5f65"));
+
+ }
+
+ private UUID checkContainsJcrUUID(NodeBuilder builder) {
+ @Nullable PropertyState jcrUuid =
builder.getProperty(JcrConstants.JCR_UUID);
+ assertNotNull(jcrUuid);
+ String newUuid = jcrUuid.getValue(Type.STRING);
+ assertNotNull(newUuid);
+ return UUID.fromString(newUuid);
+ }
+
private void applyJson(String json) throws IOException,
CommitFailedException {
IndexDefinitionUpdater update = new IndexDefinitionUpdater(json);
@@ -177,8 +228,7 @@ public class IndexDefinitionUpdaterTest {
store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
- IndexDefinitionPrinter printer = new IndexDefinitionPrinter(store,
- () -> asList(indexPaths));
+ IndexDefinitionPrinter printer = new IndexDefinitionPrinter(store, ()
-> asList(indexPaths));
StringWriter sw = new StringWriter();
PrintWriter pw = new PrintWriter(sw);
diff --git
a/oak-core/src/test/resources/org/apache/jackrabbit/oak/plugins/index/importer/index-def-jcruuid-fix.json
b/oak-core/src/test/resources/org/apache/jackrabbit/oak/plugins/index/importer/index-def-jcruuid-fix.json
new file mode 100644
index 0000000000..a403147507
--- /dev/null
+++
b/oak-core/src/test/resources/org/apache/jackrabbit/oak/plugins/index/importer/index-def-jcruuid-fix.json
@@ -0,0 +1,24 @@
+{
+ "/oak:index/barIndex": {
+ "jcr:primaryType": "nam:oak:QueryIndexDefinition",
+ "config_without_uuid.xml": {
+ "jcr:created": "dat:2024-07-11T11:37:51.727Z",
+ "jcr:primaryType": "nam:nt:file",
+ "jcr:content": {
+ "jcr:data": ":blobId:XYZ",
+ "jcr:mimeType": "text/xml",
+ "jcr:primaryType": "nam:nt:resource"
+ }
+ },
+ "config_with_uuid.xml": {
+ "jcr:created": "dat:2024-07-11T11:37:51.727Z",
+ "jcr:primaryType": "nam:nt:file",
+ "jcr:content": {
+ "jcr:data": ":blobId:XYZ",
+ "jcr:mimeType": "text/xml",
+ "jcr:primaryType": "nam:nt:resource",
+ "jcr:uuid": "6db23ff7-986d-4c3d-bf3d-ea9eaf8b5f65"
+ }
+ }
+ }
+}
