This is an automated email from the ASF dual-hosted git repository.

baedke pushed a commit to branch issue/oak-10377
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git


The following commit(s) were added to refs/heads/issue/oak-10377 by this push:
     new 3a31a31008 OAK-10377: Checked-out state of non-versionable nodes is 
not handled correctly
3a31a31008 is described below

commit 3a31a3100890da186c29b2f5a19d3a6e0b46ed60
Author: Manfred Baedke <[email protected]>
AuthorDate: Tue Sep 19 13:28:58 2023 +0200

    OAK-10377: Checked-out state of non-versionable nodes is not handled 
correctly
    
    Added versionability check, test cases.
---
 .../plugins/version/ReadOnlyVersionManager.java    |  2 +-
 .../oak/plugins/version/VersionEditor.java         |  2 +-
 .../version/ReadOnlyVersionManagerTest.java        | 14 +++-
 .../oak/jcr/version/VersionableTest.java           | 75 ++++++++++++++++++++++
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git 
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManager.java
 
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManager.java
index 18abe645fc..6b45537d44 100644
--- 
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManager.java
+++ 
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManager.java
@@ -120,7 +120,7 @@ public abstract class ReadOnlyVersionManager {
         // it is in fact read-only because of a checked-in (but 
non-accessible) ancestor.
         // if this turns out to be an issue see NodeImpl#getReadOnlyTree for 
an potential fix
         PropertyState p = tree.getProperty(VersionConstants.JCR_ISCHECKEDOUT);
-        if (p != null) {
+        if (p != null && isVersionable(tree)) {
             return p.getValue(Type.BOOLEAN);
         }
         if (tree.isRoot()) {
diff --git 
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java
 
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java
index 41c98bbb72..e29842575c 100644
--- 
a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java
+++ 
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java
@@ -212,7 +212,7 @@ class VersionEditor implements Editor {
     private boolean wasCheckedIn() {
         PropertyState prop = before.getProperty(JCR_ISCHECKEDOUT);
         if (prop != null) {
-            return !prop.getValue(Type.BOOLEAN);
+            return !prop.getValue(Type.BOOLEAN) && isVersionable();
         }
         // new node or not versionable, check parent
         return parent != null && parent.wasCheckedIn();
diff --git 
a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManagerTest.java
 
b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManagerTest.java
index 50c3e97cf4..685eb93d45 100644
--- 
a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManagerTest.java
+++ 
b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/version/ReadOnlyVersionManagerTest.java
@@ -216,4 +216,16 @@ public class ReadOnlyVersionManagerTest extends 
AbstractSecurityTest {
         assertTrue(history.exists());
         assertEquals(historyUuid, 
history.getProperty(JCR_UUID).getValue(Type.STRING));
     }
-}
+
+    @Test
+    public void testCheckedOutNonVersionable() throws RepositoryException, 
CommitFailedException {
+        NodeUtil node = new NodeUtil(root.getTree("/"));
+        node.addChild("nonVersionable", NodeTypeConstants.NT_OAK_UNSTRUCTURED);
+        root.commit();
+
+        Tree nonVersionable = root.getTree("/nonVersionable");
+        nonVersionable.setProperty(JCR_ISCHECKEDOUT, Boolean.FALSE, 
Type.BOOLEAN);
+        root.commit();
+
+        assertTrue(versionManager.isCheckedOut(nonVersionable));
+    }}
diff --git 
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionableTest.java
 
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionableTest.java
index 42177444a6..2ffc932815 100644
--- 
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionableTest.java
+++ 
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionableTest.java
@@ -36,6 +36,7 @@ import 
org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 import org.apache.jackrabbit.test.AbstractJCRTest;
 import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
 
 import java.util.Set;
 
@@ -362,6 +363,80 @@ public class VersionableTest extends AbstractJCRTest {
         assertFalse(guest.getNode(nodePath).isCheckedOut());
     }
 
+    public void testNonVersionableCheckedOut() throws Exception {
+        Node node = testRootNode.addNode(nodeName1, "nt:unstructured");
+        superuser.save();
+
+        assertTrue(node.isCheckedOut());
+
+        node.setProperty("jcr:isCheckedOut", false);
+        superuser.save();
+
+        assertTrue(node.isCheckedOut());
+    }
+
+    public void testModifyNonVersionableNodeWithCheckedOutProperty() throws 
Exception {
+        Node node = testRootNode.addNode(nodeName1, "nt:unstructured");
+        superuser.save();
+
+        assertTrue(node.isCheckedOut());
+
+        node.setProperty("jcr:isCheckedOut", false);
+        superuser.save();
+
+        node.setProperty("test", true);
+        superuser.save();
+
+        assertTrue(node.getProperty("test").getBoolean());
+
+        node.setProperty("test", false);
+        superuser.save();
+
+        assertFalse(node.getProperty("test").getBoolean());
+
+        node.getProperty("test").remove();
+        superuser.save();
+        assertFalse(node.hasProperty("test"));
+
+        node.addNode(nodeName2, "nt:unstructured");
+        superuser.save();
+
+        assertTrue(node.hasNode(nodeName2));
+
+        node.getNode(nodeName2).remove();
+        superuser.save();
+
+        assertFalse(node.hasNode(nodeName2));
+    }
+
+   public void testAddRemoveMixinVersionable() throws Exception {
+        Node node = testRootNode.addNode(nodeName1, "nt:unstructured");
+        node.addMixin(mixVersionable);
+        superuser.save();
+        assertTrue(node.hasProperty(jcrIsCheckedOut));
+        assertTrue(node.isCheckedOut());
+        node.checkin();
+        superuser.save();
+        assertFalse(node.isCheckedOut());
+        node.checkout();
+        superuser.save();
+        assertTrue(node.isCheckedOut());
+        node.removeMixin(mixVersionable);
+        superuser.save();
+        assertTrue(node.isCheckedOut());
+        assertFalse(node.hasProperty(jcrIsCheckedOut));
+        node.addMixin(mixVersionable);
+        superuser.save();
+        assertTrue(node.hasProperty(jcrIsCheckedOut));
+        assertTrue(node.isCheckedOut());
+        node.checkin();
+        superuser.save();
+        assertFalse(node.isCheckedOut());
+        node.checkout();
+        superuser.save();
+        assertTrue(node.isCheckedOut());
+    }
+
     private static void assertSuccessors(VersionHistory history, Set<String> 
expectedSuccessors, String versionName) throws RepositoryException {
         assertEquals(expectedSuccessors, 
getNames(history.getVersion(versionName).getSuccessors()));
     }

Reply via email to