You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2022/10/14 07:31:31 UTC
[jackrabbit-oak] 01/01: OAK-9966 : Internal code calls Node.isCheckedOut and VersionManager.isCheckedOut
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch OAK_9966
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit c46892b8f7eb1cb01f6f2deae126be93150b9d23
Author: angela <an...@adobe.com>
AuthorDate: Fri Oct 14 09:31:15 2022 +0200
OAK-9966 : Internal code calls Node.isCheckedOut and VersionManager.isCheckedOut
---
.../plugins/version/ReadOnlyVersionManager.java | 22 +++++------
.../apache/jackrabbit/oak/jcr/query/QueryImpl.java | 4 +-
.../jackrabbit/oak/jcr/session/ItemImpl.java | 6 +--
.../jackrabbit/oak/jcr/session/NodeImpl.java | 39 ++++++++++--------
.../jackrabbit/oak/jcr/session/PropertyImpl.java | 27 ++++++++-----
.../jackrabbit/oak/jcr/session/SessionImpl.java | 4 +-
.../jackrabbit/oak/jcr/session/WorkspaceImpl.java | 6 ++-
.../oak/jcr/version/VersionManagerImpl.java | 34 +++++++++-------
.../jackrabbit/oak/jcr/xml/ImporterImpl.java | 7 ++--
.../oak/jcr/version/VersionableTest.java | 46 ++++++++++++++++++++++
10 files changed, 132 insertions(+), 63 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 7281cc5f29..c4e1216fdf 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
@@ -112,18 +112,16 @@ public abstract class ReadOnlyVersionManager {
* @return whether the tree is checked out or not.
*/
public boolean isCheckedOut(@NotNull Tree tree) {
- if (checkNotNull(tree).exists()) {
- PropertyState p = tree.getProperty(VersionConstants.JCR_ISCHECKEDOUT);
- if (p != null) {
- return p.getValue(Type.BOOLEAN);
- }
- } else {
- // FIXME: this actually means access to the tree is restricted
- // and may result in wrong isCheckedOut value. This should never
- // be the case in a commit hook because it operates on non-access-
- // controlled NodeStates. This means consistency is not at risk
- // but it may mean oak-jcr sees a node as checked out even though
- // it is in fact read-only because of a checked-in ancestor.
+ // NOTE: if the given tree does not exist it mean that it is a non-accessible ancestor
+ // in this case it may result in wrong isCheckedOut value. This should never
+ // be the case in a commit hook because it operates on non-access-
+ // controlled NodeStates. This means consistency is not at risk
+ // but it may mean oak-jcr sees a node as checked out even though
+ // 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) {
+ return p.getValue(Type.BOOLEAN);
}
if (tree.isRoot()) {
return true;
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryImpl.java
index b4dc9fae07..8a6e26a565 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/query/QueryImpl.java
@@ -159,8 +159,8 @@ public class QueryImpl implements Query {
if (parentDelegate == null) {
throw new PathNotFoundException("The specified path does not exist: " + parent);
}
- Node parentNode = NodeImpl.createNode(parentDelegate, sessionContext);
- if (!parentNode.isCheckedOut()) {
+ NodeImpl parentNode = NodeImpl.createNode(parentDelegate, sessionContext);
+ if (!parentNode.internalIsCheckedOut()) {
throw new VersionException("Cannot store query. Node at " +
absPath + " is checked in.");
}
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/ItemImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/ItemImpl.java
index 9a0f861d48..5340383580 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/ItemImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/ItemImpl.java
@@ -40,7 +40,6 @@ import javax.jcr.UnsupportedRepositoryOperationException;
import javax.jcr.Value;
import javax.jcr.ValueFactory;
import javax.jcr.nodetype.ConstraintViolationException;
-import javax.jcr.version.VersionManager;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
@@ -50,6 +49,7 @@ import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
import org.apache.jackrabbit.oak.jcr.session.operation.ItemOperation;
import org.apache.jackrabbit.oak.jcr.session.operation.SessionOperation;
+import org.apache.jackrabbit.oak.jcr.version.VersionManagerImpl;
import org.apache.jackrabbit.oak.plugins.memory.PropertyBuilder;
import org.apache.jackrabbit.oak.plugins.nodetype.write.ReadWriteNodeTypeManager;
import org.jetbrains.annotations.NotNull;
@@ -349,8 +349,8 @@ abstract class ItemImpl<T extends ItemDelegate> implements Item {
}
@NotNull
- VersionManager getVersionManager() throws RepositoryException {
- return sessionContext.getWorkspace().getVersionManager();
+ VersionManagerImpl getVersionManager() {
+ return sessionContext.getWorkspace().internalGetVersionManager();
}
@NotNull
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
index 816f91d0b6..ab7cf7f677 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
@@ -83,10 +83,12 @@ import org.apache.jackrabbit.oak.commons.LazyValue;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
import org.apache.jackrabbit.oak.jcr.delegate.PropertyDelegate;
+import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
import org.apache.jackrabbit.oak.jcr.delegate.VersionManagerDelegate;
import org.apache.jackrabbit.oak.jcr.lock.LockDeprecation;
import org.apache.jackrabbit.oak.jcr.session.operation.ItemOperation;
import org.apache.jackrabbit.oak.jcr.session.operation.NodeOperation;
+import org.apache.jackrabbit.oak.jcr.session.operation.SessionOperation;
import org.apache.jackrabbit.oak.jcr.version.VersionHistoryImpl;
import org.apache.jackrabbit.oak.jcr.version.VersionImpl;
import org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager;
@@ -956,7 +958,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut()) {
+ if (!internalIsCheckedOut()) {
throw new VersionException(format("Cannot set primary type. Node [%s] is checked in.", getNodePath()));
}
}
@@ -978,7 +980,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut()) {
+ if (!internalIsCheckedOut()) {
throw new VersionException(format(
"Cannot add mixin type. Node [%s] is checked in.", getNodePath()));
}
@@ -997,7 +999,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut()) {
+ if (!internalIsCheckedOut()) {
throw new VersionException(format(
"Cannot remove mixin type. Node [%s] is checked in.", getNodePath()));
}
@@ -1028,9 +1030,9 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
public Boolean perform() throws RepositoryException {
PropertyState prop = PropertyStates.createProperty(JCR_MIXINTYPES, singleton(oakTypeName), NAMES);
return sessionContext.getAccessManager().hasPermissions(
- node.getTree(), prop, Permissions.NODE_TYPE_MANAGEMENT)
+ node.getTree(), prop, Permissions.NODE_TYPE_MANAGEMENT)
&& !node.isProtected()
- && getVersionManager().isCheckedOut(toJcrPath(dlg.getPath())) // TODO: avoid nested calls
+ && getVersionManager().isCheckedOut(node)
&& node.canAddMixin(oakTypeName);
}
});
@@ -1140,13 +1142,14 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
*/
@Override
public boolean isCheckedOut() throws RepositoryException {
- try {
- return getVersionManager().isCheckedOut(getPath());
- } catch (UnsupportedRepositoryOperationException ex) {
- // when versioning is not supported all nodes are considered to be
- // checked out
- return true;
- }
+ final SessionDelegate sessionDelegate = sessionContext.getSessionDelegate();
+ return sessionDelegate.perform(new SessionOperation<Boolean>("isCheckedOut") {
+ @NotNull
+ @Override
+ public Boolean perform() throws RepositoryException {
+ return internalIsCheckedOut();
+ }
+ });
}
/**
@@ -1295,6 +1298,10 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
}
//------------------------------------------------------------< internal >---
+ public boolean internalIsCheckedOut() throws RepositoryException {
+ return getVersionManager().isCheckedOut(dlg);
+ }
+
@Nullable
private String getPrimaryTypeName(@NotNull Tree tree) {
return TreeUtil.getPrimaryTypeName(tree, getReadOnlyTree(tree));
@@ -1392,7 +1399,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut() && getOPV(dlg.getTree(), state) != OnParentVersionAction.IGNORE) {
+ if (!internalIsCheckedOut() && getOPV(dlg.getTree(), state) != OnParentVersionAction.IGNORE) {
throw new VersionException(format(
"Cannot set property. Node [%s] is checked in.", getNodePath()));
}
@@ -1438,7 +1445,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut() && getOPV(dlg.getTree(), state) != OnParentVersionAction.IGNORE) {
+ if (!internalIsCheckedOut() && getOPV(dlg.getTree(), state) != OnParentVersionAction.IGNORE) {
throw new VersionException(format(
"Cannot set property. Node [%s] is checked in.", getNodePath()));
}
@@ -1484,7 +1491,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
super.checkPreconditions();
PropertyDelegate property = dlg.getPropertyOrNull(oakName);
if (property != null &&
- !isCheckedOut() &&
+ !internalIsCheckedOut() &&
getOPV(dlg.getTree(), property.getPropertyState()) != OnParentVersionAction.IGNORE) {
throw new VersionException(format(
"Cannot remove property. Node [%s] is checked in.", getNodePath()));
@@ -1611,7 +1618,7 @@ public class NodeImpl<T extends NodeDelegate> extends ItemImpl<T> implements Jac
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!isCheckedOut()) {
+ if (!internalIsCheckedOut()) {
throw new VersionException(format("Cannot set mixin types. Node [%s] is checked in.", getNodePath()));
}
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/PropertyImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/PropertyImpl.java
index ded63e6e2f..83d4a22ed2 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/PropertyImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/PropertyImpl.java
@@ -76,12 +76,7 @@ public class PropertyImpl extends ItemImpl<PropertyDelegate> implements Property
@NotNull
@Override
public Node perform() throws RepositoryException {
- NodeDelegate parent = property.getParent();
- if (parent == null) {
- throw new AccessDeniedException();
- } else {
- return NodeImpl.createNode(parent, sessionContext);
- }
+ return internalGetParent(property, sessionContext);
}
});
}
@@ -114,7 +109,7 @@ public class PropertyImpl extends ItemImpl<PropertyDelegate> implements Property
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!getParent().isCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
+ if (!parentIsCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
throw new VersionException(
"Cannot set property. Node is checked in.");
}
@@ -464,7 +459,7 @@ public class PropertyImpl extends ItemImpl<PropertyDelegate> implements Property
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!getParent().isCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
+ if (!parentIsCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
throw new VersionException(
"Cannot set property. Node is checked in.");
}
@@ -500,7 +495,7 @@ public class PropertyImpl extends ItemImpl<PropertyDelegate> implements Property
@Override
public void checkPreconditions() throws RepositoryException {
super.checkPreconditions();
- if (!getParent().isCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
+ if (!parentIsCheckedOut() && getDefinition().getOnParentVersion() != OnParentVersionAction.IGNORE) {
throw new VersionException(
"Cannot set property. Node is checked in.");
}
@@ -532,4 +527,18 @@ public class PropertyImpl extends ItemImpl<PropertyDelegate> implements Property
});
}
+ private boolean parentIsCheckedOut() throws RepositoryException {
+ return internalGetParent(dlg, sessionContext).internalIsCheckedOut();
+ }
+
+ @NotNull
+ private static NodeImpl<? extends NodeDelegate> internalGetParent(@NotNull PropertyDelegate propertyDlg, @NotNull SessionContext sessionContext) throws RepositoryException {
+ NodeDelegate parent = propertyDlg.getParent();
+ if (parent == null) {
+ throw new AccessDeniedException();
+ } else {
+ return NodeImpl.createNode(parent, sessionContext);
+ }
+ }
+
}
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionImpl.java
index 72b783cfbf..41e579766b 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionImpl.java
@@ -686,8 +686,8 @@ public class SessionImpl implements JackrabbitSession {
}
boolean isNode = ((ItemImpl<?>) target).isNode();
- Node parent = (isNode) ? (Node) target : ((ItemImpl<?>) target).getParent();
- if (!parent.isCheckedOut()) {
+ NodeImpl parent = (NodeImpl) ((isNode) ? target : ((ItemImpl<?>) target).getParent());
+ if (!parent.internalIsCheckedOut()) {
return false;
}
boolean hasLocking = sessionContext.getRepository().getDescriptorValue(Repository.OPTION_LOCKING_SUPPORTED)
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/WorkspaceImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/WorkspaceImpl.java
index 58135d5ed9..2975b1eb9a 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/WorkspaceImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/WorkspaceImpl.java
@@ -326,7 +326,11 @@ public class WorkspaceImpl implements JackrabbitWorkspace {
public PrivilegeManager getPrivilegeManager() throws RepositoryException {
return sessionContext.getPrivilegeManager();
}
-
+
+ //------------------------------------------------------------< internal >---
+ public @NotNull VersionManagerImpl internalGetVersionManager() {
+ return versionManager;
+ }
//------------------------------------------------------------< private >---
private void ensureIsAlive() throws RepositoryException {
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionManagerImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionManagerImpl.java
index 3a2f74589c..3106806c77 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionManagerImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/version/VersionManagerImpl.java
@@ -252,7 +252,7 @@ public class VersionManagerImpl implements VersionManager {
@Override
public boolean isCheckedOut(final String absPath) throws RepositoryException {
final SessionDelegate sessionDelegate = sessionContext.getSessionDelegate();
- return sessionDelegate.perform(new SessionOperation<Boolean>("isCheckoutOut") {
+ return sessionDelegate.perform(new SessionOperation<Boolean>("isCheckedOut") {
@NotNull
@Override
public Boolean perform() throws RepositoryException {
@@ -261,20 +261,7 @@ public class VersionManagerImpl implements VersionManager {
if (nodeDelegate == null) {
throw new PathNotFoundException(absPath);
}
- boolean isCheckedOut = versionManagerDelegate.isCheckedOut(nodeDelegate);
- if (!isCheckedOut) {
- // check OPV
- ReadWriteNodeTypeManager ntMgr = sessionContext.getWorkspace().getNodeTypeManager();
- NodeDelegate parent = nodeDelegate.getParent();
- NodeDefinition definition;
- if (parent == null) {
- definition = ntMgr.getRootDefinition();
- } else {
- definition = ntMgr.getDefinition(parent.getTree(), nodeDelegate.getTree());
- }
- isCheckedOut = definition.getOnParentVersion() == OnParentVersionAction.IGNORE;
- }
- return isCheckedOut;
+ return isCheckedOut(nodeDelegate);
}
});
}
@@ -381,6 +368,23 @@ public class VersionManagerImpl implements VersionManager {
//----------------------------< internal >----------------------------------
+ public boolean isCheckedOut(final @NotNull NodeDelegate nodeDelegate) throws RepositoryException {
+ boolean isCheckedOut = versionManagerDelegate.isCheckedOut(nodeDelegate);
+ if (!isCheckedOut) {
+ // check OPV
+ ReadWriteNodeTypeManager ntMgr = sessionContext.getWorkspace().getNodeTypeManager();
+ NodeDelegate parent = nodeDelegate.getParent();
+ NodeDefinition definition;
+ if (parent == null) {
+ definition = ntMgr.getRootDefinition();
+ } else {
+ definition = ntMgr.getDefinition(parent.getTree(), nodeDelegate.getTree());
+ }
+ isCheckedOut = definition.getOnParentVersion() == OnParentVersionAction.IGNORE;
+ }
+ return isCheckedOut;
+ }
+
private void checkPendingChangesForRestore(SessionDelegate sessionDelegate)
throws InvalidItemStateException {
if (sessionDelegate.hasPendingChanges()) {
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
index 5049fbb363..a8f6f9934e 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
@@ -35,7 +35,6 @@ import javax.jcr.nodetype.ConstraintViolationException;
import javax.jcr.nodetype.NodeDefinition;
import javax.jcr.nodetype.PropertyDefinition;
import javax.jcr.version.VersionException;
-import javax.jcr.version.VersionManager;
import com.google.common.base.Function;
import com.google.common.base.Predicates;
@@ -48,10 +47,12 @@ import org.apache.jackrabbit.oak.api.Root;
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.jcr.delegate.NodeDelegate;
import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
import org.apache.jackrabbit.oak.jcr.security.AccessManager;
import org.apache.jackrabbit.oak.jcr.session.SessionContext;
import org.apache.jackrabbit.oak.jcr.session.WorkspaceImpl;
+import org.apache.jackrabbit.oak.jcr.version.VersionManagerImpl;
import org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
@@ -143,8 +144,8 @@ public class ImporterImpl implements Importer {
}
WorkspaceImpl wsp = sessionContext.getWorkspace();
- VersionManager vMgr = wsp.getVersionManager();
- if (!vMgr.isCheckedOut(absPath)) {
+ VersionManagerImpl vMgr = wsp.internalGetVersionManager();
+ if (!vMgr.isCheckedOut(new NodeDelegate(sd, importTargetTree))) {
throw new VersionException("Target node is checked in.");
}
boolean hasLocking = sessionContext.getRepository().getDescriptorValue(Repository.OPTION_LOCKING_SUPPORTED).getBoolean();
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 f7da2d27c1..4bab4b6e72 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
@@ -16,11 +16,13 @@
*/
package org.apache.jackrabbit.oak.jcr.version;
+import javax.jcr.GuestCredentials;
import javax.jcr.Node;
import javax.jcr.NodeIterator;
import javax.jcr.Property;
import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
+import javax.jcr.Session;
import javax.jcr.version.Version;
import javax.jcr.version.VersionException;
import javax.jcr.version.VersionHistory;
@@ -28,6 +30,10 @@ import javax.jcr.version.VersionManager;
import com.google.common.base.Function;
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
+import org.apache.jackrabbit.oak.api.Tree;
+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;
@@ -316,6 +322,46 @@ public class VersionableTest extends AbstractJCRTest {
assertSuccessors(history, of("1.1.0", "1.3"), "1.1");
}
+ /**
+ * See {@link org.apache.jackrabbit.oak.plugins.version.ReadOnlyVersionManager#isCheckedOut(Tree)} for details.
+ */
+ public void testIsCheckedOutOnNonAccessibleParent() throws Exception {
+ Node node = testRootNode.addNode(nodeName1, testNodeType);
+ node.addMixin(mixVersionable);
+ Node child = node.addNode(nodeName2, testNodeType);
+
+ String nodePath = node.getPath();
+ String childPath = child.getPath();
+
+ // grant the guest-session read-access to child but not the parent.
+ AccessControlUtils.deny(node, EveryonePrincipal.NAME, PrivilegeConstants.JCR_READ);
+ AccessControlUtils.allow(child, EveryonePrincipal.NAME, PrivilegeConstants.JCR_READ);
+ superuser.save();
+
+ node.checkin();
+ assertFalse(node.isCheckedOut());
+ assertFalse(child.isCheckedOut());
+
+ Session guest = getHelper().getRepository().login(new GuestCredentials());
+ assertFalse(guest.nodeExists(nodePath));
+ assertTrue(guest.nodeExists((childPath)));
+
+ assertTrue(guest.getWorkspace().getVersionManager().isCheckedOut(childPath));
+ assertTrue(guest.getNode(childPath).isCheckedOut());
+
+ // grant the guest-session read-access on the parent as well -> checkin-status is visible
+ AccessControlUtils.allow(node, EveryonePrincipal.NAME, PrivilegeConstants.JCR_READ);
+ superuser.save();
+
+ guest.refresh(false);
+ assertTrue(guest.nodeExists(nodePath));
+ assertFalse(guest.getWorkspace().getVersionManager().isCheckedOut(childPath));
+ assertFalse(guest.getWorkspace().getVersionManager().isCheckedOut(nodePath));
+
+ assertFalse(guest.getNode(childPath).isCheckedOut());
+ assertFalse(guest.getNode(nodePath).isCheckedOut());
+ }
+
private static void assertSuccessors(VersionHistory history, Set<String> expectedSuccessors, String versionName) throws RepositoryException {
assertEquals(expectedSuccessors, getNames(history.getVersion(versionName).getSuccessors()));
}