reschke commented on code in PR #1474:
URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1629067216
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java:
##########
@@ -145,27 +146,73 @@ public void propertyChanged(PropertyState before,
PropertyState after)
return;
}
String propName = after.getName();
+
+ // Updates the checked-out / checked-in state of the currently
processed node when the JCR_ISCHECKEDOUT property
+ // change is processed.
if (propName.equals(JCR_ISCHECKEDOUT)) {
if (wasCheckedIn()) {
vMgr.checkout(node);
} else {
vMgr.checkin(node);
}
+
+ /*Completes the restore of a version from version history.
Review Comment:
```suggestion
/* Completes the restore of a version from version history.
```
##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveVersionableNodeWithNodeRepositoryTest.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr;
+
+import org.apache.jackrabbit.oak.NodeStoreFixtures;
+import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.test.NotExecutableException;
+import org.junit.Test;
+import org.junit.runners.Parameterized;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+import javax.jcr.version.Version;
+import javax.jcr.version.VersionHistory;
+import javax.jcr.version.VersionIterator;
+import javax.jcr.version.VersionManager;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import static java.util.Collections.singleton;
+import static
org.apache.jackrabbit.oak.commons.FixturesHelper.Fixture.DOCUMENT_NS;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for moving versionable nodes over deleted versionable nodes using the
DOCUMENT_NS fixture.
+ */
+public class MoveVersionableNodeWithNodeRepositoryTest extends
AbstractRepositoryTest {
+
+ private static final String mixVersionable = "mix:versionable";
+
+ public MoveVersionableNodeWithNodeRepositoryTest(NodeStoreFixture fixture)
{
+ super(fixture);
+ }
+
+ @Parameterized.Parameters(name="{0}")
+ public static Collection<Object[]> memoryFixture() {
+ return NodeStoreFixtures.asJunitParameters(singleton(DOCUMENT_NS));
+ }
+
+ /**
+ * Creates a versionable node with the specified name under the given
parent node, then
+ * saves the session.
+ * @param parent
+ * @param nodeName
+ * @return
+ * @throws Exception
+ */
+ private Node createVersionableNode(Node parent, String nodeName) throws
Exception {
+ Node newNode = (parent.hasNode(nodeName)) ? parent.getNode(nodeName) :
parent.addNode(nodeName);
+ if (newNode.canAddMixin(mixVersionable)) {
+ newNode.addMixin(mixVersionable);
+ } else {
+ throw new NotExecutableException();
+ }
+ newNode.getSession().save();
+ return newNode;
+ }
+
+ /**
+ * Checks out the node, sets the property then saves the session and
checks the node back in.
+ * To be used in tests where version history needs to be populated.
+ */
+ private void setNodePropertyAndCheckIn(Node node, String propertyName,
String propertyValue) throws Exception {
+ node.checkout();
+ node.setProperty(propertyName, propertyValue);
+ node.getSession().save();
+ node.checkin();
+ }
+
+ /*
+ * 1. Create a versionable unstructured node at
nodeName1/nodeName2/sourceNode
+ * 2. Create a versionable unstructured node at
nodeName1/nodeName3/sourceNode
+ * 3. create version histories for both nodes
+ * 4. remove nodeName1/nodeName3/nodeName1 (that's because move(src,dest)
throws an exception if dest already exists)
+ * 5. move nodeName1/nodeName2/sourceNode to
nodeName1/nodeName3/sourceNode and call session.save()
+ * 6. should work according to JCR specification - reproduces bug for
OAK-8848: will throw ConstraintViolationException
+ * - "Property is protected: jcr:versionHistory
+ */
+ @Test
+ public void
testMoveNodeWithVersionHistoryOverDeletedNodeWithVersionHistory() throws
Exception {
+
+ Session session = getAdminSession();
+ Node testRootNode = session.getRootNode().addNode("node1");
+
+ String newNodeName = "sourceNode";
+ Node sourceParent = testRootNode.addNode("node2"); //
nodeName1/nodeName2
+ Node sourceNode = createVersionableNode(sourceParent, newNodeName); //
nodeName1/nodeName2/sourceNode
+ Node destParent = testRootNode.addNode("node3"); // nodeName1/nodeName3
+ Node destNode = createVersionableNode(destParent, "destNode"); //
nodeName1/nodeName3/sourceNode
+
+ String destPath = destNode.getPath();
+
+ // add version histories for sourceNode and destNode
+ setNodePropertyAndCheckIn(sourceNode, "sourceNode_testProp",
"sourceNode_testValue_1");
+
+ session.save();
+
+ setNodePropertyAndCheckIn(sourceNode, "sourceNode_testProp",
"sourceNode_testValue_2");
+ setNodePropertyAndCheckIn(destNode, "destNode_testProp",
"destNode_testValue_2");
+
+ session.save();
+
+ VersionHistory delNodeVersionHistory = destNode.getVersionHistory();
+ String delNodeVHPath = delNodeVersionHistory.getPath();
+
+ session.removeItem(destNode.getPath());
+
+ session.move(sourceNode.getPath(), destPath);
+ session.save();
+
+ // check setting property - via NodeImpl - on moved node
+ setNodePropertyAndCheckIn(sourceNode, "testProp", "testValue");
+
+ // check version history - TODO HORIA: should be in separate method -
EXTRACT
+ VersionManager versionManager =
session.getWorkspace().getVersionManager();
+ VersionIterator versionIterator;
+
+ //TODO HORIA: create submethods that get the version histories for the
source and dest node and do assertions on them in ALL TESTS
+ List<Version> sourceNodeVersions = new ArrayList<>();
+
+ //get the version histories for the removed node and the moved node
+ try {
+ VersionHistory sourceNodeHistory = (VersionHistory)
session.getNode(delNodeVHPath);
+
+ versionIterator = sourceNodeHistory.getAllVersions();
+ while (versionIterator.hasNext()) {
+ sourceNodeVersions.add(versionIterator.nextVersion());
+ }
+ }catch (Exception ex) {
Review Comment:
```suggestion
} catch (Exception ex) {
```
##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java:
##########
@@ -45,7 +45,7 @@ public class VersionManagementTest extends
AbstractEvaluationTest {
@Override
@Before
- protected void setUp() throws Exception {
+ public void setUp() throws Exception {
Review Comment:
why are changes needed in this class?
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java:
##########
@@ -145,27 +146,73 @@ public void propertyChanged(PropertyState before,
PropertyState after)
return;
}
String propName = after.getName();
+
+ // Updates the checked-out / checked-in state of the currently
processed node when the JCR_ISCHECKEDOUT property
+ // change is processed.
if (propName.equals(JCR_ISCHECKEDOUT)) {
if (wasCheckedIn()) {
vMgr.checkout(node);
} else {
vMgr.checkin(node);
}
+
+ /*Completes the restore of a version from version history.
+
+ When the JCR_BASEVERSION property is processed, a check is made for the
current base version property.
+ If a restore is currently in progress for the current base version (the
check for this is that the
+ current base version name has the format "restore-[UUID of the version
to restore to]"), then the restore
+ is completed for the current node to the version specified by the UUID.
+
+ If a node that was moved or copied to the location of a deleted node is
currently being processed
+ (see OAK-8848 for context), the restore operation must NOT be performed
when the JCR_BASEVERSION property change
+ is processed for the node.
+
+ TODO: check if this code ever runs when
after.getValue(Type.REFERENCE).startsWith(RESTORE_PREFIX) is FALSE,
Review Comment:
Is this still a TODO?
##########
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveVersionableNodeWithNodeRepositoryTest.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr;
+
+import org.apache.jackrabbit.oak.NodeStoreFixtures;
+import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.test.NotExecutableException;
+import org.junit.Test;
+import org.junit.runners.Parameterized;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+import javax.jcr.version.Version;
+import javax.jcr.version.VersionHistory;
+import javax.jcr.version.VersionIterator;
+import javax.jcr.version.VersionManager;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import static java.util.Collections.singleton;
+import static
org.apache.jackrabbit.oak.commons.FixturesHelper.Fixture.DOCUMENT_NS;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for moving versionable nodes over deleted versionable nodes using the
DOCUMENT_NS fixture.
+ */
+public class MoveVersionableNodeWithNodeRepositoryTest extends
AbstractRepositoryTest {
+
+ private static final String mixVersionable = "mix:versionable";
Review Comment:
we should have a constant for this elsewhere...
org.apache.jackrabbit.JcrConstants.MIX_VERSIONABLE;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
