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 ju...@apache.org on 2013/08/28 03:33:22 UTC
svn commit: r1518048 - in
/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr:
NodeImpl.java delegate/NodeDelegate.java lock/LockImpl.java
lock/LockManagerImpl.java lock/LockOperation.java
Author: jukka
Date: Wed Aug 28 01:33:21 2013
New Revision: 1518048
URL: http://svn.apache.org/r1518048
Log:
OAK-150: Basic JCR LockManager support
Various minor locking fixes & improvements
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockManagerImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockOperation.java
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1518048&r1=1518047&r2=1518048&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Wed Aug 28 01:33:21 2013
@@ -84,7 +84,6 @@ import org.slf4j.LoggerFactory;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.Arrays.asList;
import static java.util.Collections.singleton;
-import static org.apache.jackrabbit.JcrConstants.JCR_LOCKISDEEP;
import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
import static org.apache.jackrabbit.oak.api.Type.NAME;
@@ -1118,36 +1117,33 @@ public class NodeImpl<T extends NodeDele
@Override
public boolean isLocked() throws RepositoryException {
- return perform(new NodeOperation<Boolean>(dlg) {
+ return perform(new LockOperation<Boolean>(sessionDelegate, dlg) {
@Override
- public Boolean perform() throws RepositoryException {
+ public Boolean perform(NodeDelegate node) {
return node.isLocked();
}
});
}
- /**
- * Checks whether this node holds a lock by looking for the
- * {@code jcr:lockIsDeep} property.
- */
@Override
public boolean holdsLock() throws RepositoryException {
- return perform(new NodeOperation<Boolean>(dlg) {
+ return perform(new LockOperation<Boolean>(sessionDelegate, dlg) {
@Override
- public Boolean perform() throws RepositoryException {
- return node.getTree().hasProperty(JCR_LOCKISDEEP);
+ public Boolean perform(NodeDelegate node) {
+ return node.holdsLock(false);
}
});
}
@Override @Nonnull
public Lock getLock() throws RepositoryException {
- NodeDelegate lock = perform(new NodeOperation<NodeDelegate>(dlg) {
- @Override
- public NodeDelegate perform() {
- return node.getLock();
- }
- });
+ NodeDelegate lock = perform(
+ new LockOperation<NodeDelegate>(sessionDelegate, dlg) {
+ @Override
+ public NodeDelegate perform(NodeDelegate node) {
+ return node.getLock();
+ }
+ });
if (lock != null) {
return new LockImpl(sessionContext, lock);
} else {
@@ -1164,7 +1160,12 @@ public class NodeImpl<T extends NodeDele
perform(new LockOperation<Void>(sessionDelegate, dlg) {
@Override
public Void perform(NodeDelegate node) throws RepositoryException {
+ if (node.getStatus() != Status.EXISTING) {
+ throw new LockException(
+ "Unable to lock a node with pending changes");
+ }
node.lock(isDeep);
+ session.refresh(true);
return null;
}
});
@@ -1181,6 +1182,7 @@ public class NodeImpl<T extends NodeDele
@Override
public Void perform(NodeDelegate node) throws RepositoryException {
node.unlock();
+ session.refresh(true);
return null;
}
});
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java?rev=1518048&r1=1518047&r2=1518048&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java Wed Aug 28 01:33:21 2013
@@ -392,6 +392,8 @@ public class NodeDelegate extends ItemDe
}
public void removeMixin(String typeName) throws RepositoryException {
+ boolean wasLockable = isNodeType(MIX_LOCKABLE);
+
Tree tree = getTree();
Set<String> mixins = newLinkedHashSet(getNames(tree, JCR_MIXINTYPES));
if (!mixins.remove(typeName)) {
@@ -400,6 +402,13 @@ public class NodeDelegate extends ItemDe
}
tree.setProperty(JCR_MIXINTYPES, mixins, NAMES);
+ boolean isLockable = isNodeType(MIX_LOCKABLE);
+ if (wasLockable && !isLockable && holdsLock(false)) {
+ // TODO: This should probably be done in a commit hook
+ unlock();
+ sessionDelegate.refresh(true);
+ }
+
// We need to remove all protected properties and child nodes
// associated with the removed mixin type, as there's no way for
// the client to do that. Other items defined in this mixin type
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockImpl.java?rev=1518048&r1=1518047&r2=1518048&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockImpl.java Wed Aug 28 01:33:21 2013
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.lock.Lock;
+import javax.jcr.lock.LockException;
import org.apache.jackrabbit.oak.jcr.NodeImpl;
import org.apache.jackrabbit.oak.jcr.SessionContext;
@@ -73,23 +74,33 @@ public final class LockImpl implements L
@Override
public boolean isLive() {
- return safePerform(new NodeOperation<Boolean>(delegate) {
- @Override
- public Boolean perform() throws RepositoryException {
- return node.holdsLock(false);
- }
- });
+ return context.getSession().isLive() && safePerform(
+ new NodeOperation<Boolean>(delegate) {
+ @Override
+ public Boolean perform() throws RepositoryException {
+ return node.holdsLock(false);
+ }
+ });
}
@Override
public String getLockToken() {
- return null;
+ return safePerform(new NodeOperation<String>(delegate) {
+ @Override
+ public String perform() throws RepositoryException {
+ return node.getPath();
+ }
+ });
}
@Override
public long getSecondsRemaining() {
- return Long.MAX_VALUE;
+ if (isLive()) {
+ return Long.MAX_VALUE;
+ } else {
+ return -1;
+ }
}
@Override
@@ -103,7 +114,10 @@ public final class LockImpl implements L
}
@Override
- public void refresh() {
+ public void refresh() throws LockException {
+ if (!isLive()) {
+ throw new LockException("This lock is not alive");
+ }
}
//-----------------------------------------------------------< private >--
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockManagerImpl.java?rev=1518048&r1=1518047&r2=1518048&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockManagerImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockManagerImpl.java Wed Aug 28 01:33:21 2013
@@ -28,6 +28,7 @@ import javax.jcr.lock.Lock;
import javax.jcr.lock.LockException;
import javax.jcr.lock.LockManager;
+import org.apache.jackrabbit.oak.api.Tree.Status;
import org.apache.jackrabbit.oak.jcr.SessionContext;
import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
@@ -115,11 +116,18 @@ public class LockManagerImpl implements
@Override
protected NodeDelegate perform(NodeDelegate node)
throws RepositoryException {
+ if (node.getStatus() != Status.EXISTING) {
+ throw new LockException(
+ "Unable to lock a node with pending changes");
+ }
node.lock(isDeep);
+ session.refresh(true);
return node;
}
});
- sessionContext.getSession().refresh(true); // TODO: better refresh
+ if (!isSessionScoped) {
+ addLockToken(absPath);
+ }
return new LockImpl(sessionContext, lock);
}
@@ -130,10 +138,10 @@ public class LockManagerImpl implements
protected Void perform(NodeDelegate node)
throws RepositoryException {
node.unlock();
+ session.refresh(true);
return null;
}
});
- sessionContext.getSession().refresh(true); // TODO: better refresh
}
private <T> T perform(SessionOperation<T> operation)
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockOperation.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockOperation.java?rev=1518048&r1=1518047&r2=1518048&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockOperation.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/lock/LockOperation.java Wed Aug 28 01:33:21 2013
@@ -31,7 +31,7 @@ import org.apache.jackrabbit.oak.jcr.ope
*/
public abstract class LockOperation<T> extends SessionOperation<T> {
- private final SessionDelegate session;
+ protected final SessionDelegate session;
private final NodeDelegate node;
@@ -52,7 +52,13 @@ public abstract class LockOperation<T> e
}
@Override
+ public boolean isRefresh() {
+ return true;
+ }
+
+ @Override
public T perform() throws RepositoryException {
+ session.refresh(true);
if (node != null) {
return perform(node);
} else {
Re: svn commit: r1518048 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr:
NodeImpl.java delegate/NodeDelegate.java lock/LockImpl.java lock/LockManagerImpl.java
lock/LockOperation.java
Posted by Michael Dürig <md...@apache.org>.
On 11.9.13 3:38 , Jukka Zitting wrote:
> Hi,
>
> On Wed, Sep 11, 2013 at 5:19 AM, Michael Dürig <md...@apache.org> wrote:
>> What is the reason for returning true here? Only operations that refresh the
>> session should return true here. If returning true is necessary here, you
>> might be exploiting a side effect and we should try to come up with a more
>> explicit solution.
>
> The rationale behind this is that the lock operations (since they are
> inherently cross-session ones) always refresh the session to the
> latest state (see LockOperation.perform). It's not fully correct from
> a strict locking semantics point of view, but achieves the kind of
> "fuzzy locking" functionality I was aiming at.
Makes sense, thanks for clarifying. I initially didn't spot the
overloaded perform(NodeDelegate), which the initial perform delegates to.
Michael
>
> BR,
>
> Jukka
>
Re: svn commit: r1518048 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr:
NodeImpl.java delegate/NodeDelegate.java lock/LockImpl.java
lock/LockManagerImpl.java lock/LockOperation.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Sep 11, 2013 at 5:19 AM, Michael Dürig <md...@apache.org> wrote:
> What is the reason for returning true here? Only operations that refresh the
> session should return true here. If returning true is necessary here, you
> might be exploiting a side effect and we should try to come up with a more
> explicit solution.
The rationale behind this is that the lock operations (since they are
inherently cross-session ones) always refresh the session to the
latest state (see LockOperation.perform). It's not fully correct from
a strict locking semantics point of view, but achieves the kind of
"fuzzy locking" functionality I was aiming at.
BR,
Jukka
Re: svn commit: r1518048 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr:
NodeImpl.java delegate/NodeDelegate.java lock/LockImpl.java lock/LockManagerImpl.java
lock/LockOperation.java
Posted by Michael Dürig <md...@apache.org>.
Jukka,
On 28.8.13 3:33 , jukka@apache.org wrote:
> Author: jukka
> Date: Wed Aug 28 01:33:21 2013
> New Revision: 1518048
>
> URL: http://svn.apache.org/r1518048
> Log:
> OAK-150: Basic JCR LockManager support
...
>
> @Override
> + public boolean isRefresh() {
> + return true;
> + }
> +
What is the reason for returning true here? Only operations that refresh
the session should return true here. If returning true is necessary
here, you might be exploiting a side effect and we should try to come up
with a more explicit solution.
Michael