You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2010/08/12 17:31:35 UTC
svn commit: r984830 - in
/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user:
AuthorizableImpl.java MembershipCache.java
Author: angela
Date: Thu Aug 12 15:31:34 2010
New Revision: 984830
URL: http://svn.apache.org/viewvc?rev=984830&view=rev
Log:
JCR-2713 - UserManagement: Don't read cached memberships if session has pending (group) changes
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java?rev=984830&r1=984829&r2=984830&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/AuthorizableImpl.java Thu Aug 12 15:31:34 2010
@@ -125,7 +125,7 @@ abstract class AuthorizableImpl implemen
if (prop.isMultiple()) {
return prop.getValues();
} else {
- return new Value[] {prop.getValue()};
+ return new Value[]{prop.getValue()};
}
}
}
@@ -199,6 +199,7 @@ abstract class AuthorizableImpl implemen
throw e;
}
}
+
/**
* @see Authorizable#removeProperty(String)
*/
@@ -308,10 +309,21 @@ abstract class AuthorizableImpl implemen
private Iterator<Group> collectMembership(boolean includeIndirect) throws RepositoryException {
Collection<String> groupNodeIds;
- if (includeIndirect) {
- groupNodeIds = userManager.getMembershipCache().getMemberOf(node.getIdentifier());
+ MembershipCache cache = userManager.getMembershipCache();
+ String nid = node.getIdentifier();
+
+ if (node.getSession().hasPendingChanges()) {
+ // avoid retrieving outdated cache entries or filling the cache with
+ // invalid data due to group-membership changes pending on the
+ // current session.
+ // this is mainly for backwards compatibility reasons (no cache present)
+ // where transient changes (in non-autosave mode) were reflected to the
+ // editing session (see JCR-2713)
+ Session session = node.getSession();
+ groupNodeIds = (includeIndirect) ? cache.collectMembership(nid, session) : cache.collectDeclaredMembership(nid, session);
} else {
- groupNodeIds = userManager.getMembershipCache().getDeclaredMemberOf(node.getIdentifier());
+ // retrieve cached membership. there are no pending changes.
+ groupNodeIds = (includeIndirect) ? cache.getMemberOf(nid) : cache.getDeclaredMemberOf(nid);
}
Set<Group> groups = new HashSet<Group>(groupNodeIds.size());
@@ -341,7 +353,7 @@ abstract class AuthorizableImpl implemen
PropertyDefinition def = prop.getDefinition();
if (def.isProtected()) {
return false;
- } else {
+ } else {
NodeTypeImpl declaringNt = (NodeTypeImpl) prop.getDefinition().getDeclaringNodeType();
return declaringNt.isNodeType(UserConstants.NT_REP_AUTHORIZABLE);
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java?rev=984830&r1=984829&r2=984830&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/MembershipCache.java Thu Aug 12 15:31:34 2010
@@ -78,9 +78,40 @@ public class MembershipCache implements
return Collections.unmodifiableCollection(groupNodeIds);
}
- //------------------------------------------------------------< private >---
+ /**
+ * Collects the declared memberships for the specified identifier of an
+ * authorizable using the specified session.
+ *
+ * @param authorizableNodeIdentifier
+ * @param session
+ * @return
+ * @throws RepositoryException
+ */
+ Collection<String> collectDeclaredMembership(String authorizableNodeIdentifier, Session session) throws RepositoryException {
+ Collection<String> groupNodeIds = collectDeclaredMembershipFromReferences(authorizableNodeIdentifier, session);
+ if (groupNodeIds == null) {
+ groupNodeIds = collectDeclaredMembershipFromTraversal(authorizableNodeIdentifier, session);
+ }
+ return groupNodeIds;
+ }
/**
+ * Collects the complete memberships for the specified identifier of an
+ * authorizable using the specified session.
+ *
+ * @param authorizableNodeIdentifier
+ * @param session
+ * @return
+ * @throws RepositoryException
+ */
+ Collection<String> collectMembership(String authorizableNodeIdentifier, Session session) throws RepositoryException {
+ Set<String> groupNodeIds = new HashSet<String>();
+ memberOf(authorizableNodeIdentifier, groupNodeIds, session);
+ return groupNodeIds;
+ }
+
+ //------------------------------------------------------------< private >---
+ /**
* @param authorizableNodeIdentifier
* @return
* @throws RepositoryException
@@ -92,10 +123,7 @@ public class MembershipCache implements
// concurrent read operations using the system session of this workspace.
Session session = getSession();
try {
- groupNodeIds = collectDeclaredMembershipFromReferences(authorizableNodeIdentifier, session);
- if (groupNodeIds == null) {
- groupNodeIds = collectDeclaredMembershipFromTraversal(authorizableNodeIdentifier, session);
- }
+ groupNodeIds = collectDeclaredMembership(authorizableNodeIdentifier, session);
cache.put(authorizableNodeIdentifier, Collections.unmodifiableCollection(groupNodeIds));
}
finally {
@@ -108,6 +136,12 @@ public class MembershipCache implements
return groupNodeIds;
}
+ /**
+ *
+ * @param authorizableNodeIdentifier
+ * @param groupNodeIds
+ * @throws RepositoryException
+ */
private void memberOf(String authorizableNodeIdentifier, Collection<String> groupNodeIds) throws RepositoryException {
Collection<String> declared = declaredMemberOf(authorizableNodeIdentifier);
for (String identifier : declared) {
@@ -117,9 +151,31 @@ public class MembershipCache implements
}
}
+ /**
+ *
+ * @param authorizableNodeIdentifier
+ * @param groupNodeIds
+ * @param session
+ * @throws RepositoryException
+ */
+ private void memberOf(String authorizableNodeIdentifier, Collection<String> groupNodeIds, Session session) throws RepositoryException {
+ Collection<String> declared = collectDeclaredMembership(authorizableNodeIdentifier, session);
+ for (String identifier : declared) {
+ if (groupNodeIds.add(identifier)) {
+ memberOf(identifier, groupNodeIds, session);
+ }
+ }
+ }
+
+ /**
+ *
+ * @param authorizableNodeIdentifier
+ * @param session
+ * @return
+ * @throws RepositoryException
+ */
private Collection<String> collectDeclaredMembershipFromReferences(String authorizableNodeIdentifier,
Session session) throws RepositoryException {
-
Set<String> pIds = new HashSet<String>();
Set<String> nIds = new HashSet<String>();