Index: ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/RepMembersConflictHandler.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/RepMembersConflictHandler.java (revision ) +++ ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/RepMembersConflictHandler.java (revision ) @@ -0,0 +1,158 @@ +/* + * 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.security.principal; + +import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.plugins.memory.PropertyBuilder; +import org.apache.jackrabbit.oak.spi.commit.PartialConflictHandler; +import org.apache.jackrabbit.oak.spi.security.user.UserConstants; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.jackrabbit.oak.spi.state.NodeState; + +import java.util.HashSet; +import java.util.Set; + +/** + * The RepMembersConflictHandler takes care of merging the rep:members property + * during parallel updates. Change merges are done pessimistically, in that only members of both change sets + * are included in the result. + * + * The conflict handler deals with the following conflicts: + * + */ +public class RepMembersConflictHandler implements PartialConflictHandler { + + @Override + public Resolution addExistingProperty(NodeBuilder parent, PropertyState ours, PropertyState theirs) { + if (isRepMembersProperty(theirs)) { + mergeAdd(parent, ours, theirs); + return Resolution.MERGED; + } else { + return null; + } + } + + @Override + public Resolution changeChangedProperty(NodeBuilder parent, PropertyState ours, PropertyState theirs) { + if (isRepMembersProperty(theirs)) { + mergeChange(parent, ours, theirs); + return Resolution.MERGED; + } else { + return null; + } + } + + @Override + public Resolution changeDeletedProperty(NodeBuilder parent, PropertyState ours) { + if (isRepMembersProperty(ours)) { + // removing the members property takes precedence + return Resolution.THEIRS; + } else { + return null; + } + } + + @Override + public Resolution deleteChangedProperty(NodeBuilder parent, PropertyState theirs) { + if (isRepMembersProperty(theirs)) { + // removing the members property takes precedence + return Resolution.OURS; + } else { + return null; + } + } + + @Override + public Resolution deleteDeletedProperty(NodeBuilder parent, PropertyState ours) { + if (isRepMembersProperty(ours)) { + // both are removing the members property, resolve + return Resolution.THEIRS; + } else { + return null; + } + } + + @Override + public Resolution addExistingNode(NodeBuilder parent, String name, NodeState ours, NodeState theirs) { + return null; + } + + @Override + public Resolution changeDeletedNode(NodeBuilder parent, String name, NodeState ours) { + return null; + } + + @Override + public Resolution deleteChangedNode(NodeBuilder parent, String name, NodeState theirs) { + return null; + } + + @Override + public Resolution deleteDeletedNode(NodeBuilder parent, String name) { + return null; + } + + //----------------------------< internal >---------------------------------- + + private static void mergeAdd(NodeBuilder parent, PropertyState ours, PropertyState theirs) { + PropertyBuilder merged = PropertyBuilder.array(Type.STRING).assignFrom(theirs); + Set theirMembers = Sets.newHashSet(theirs.getValue(Type.STRINGS)); + + // merge the two member-lists in a non-duplicative way + for (String member : ours.getValue(Type.STRINGS)) { + if (!theirMembers.contains(member)) { + merged.addValue(member); + } + } + + parent.setProperty(merged.getPropertyState()); + } + + private static void mergeChange(NodeBuilder parent, PropertyState ours, PropertyState theirs) { + PropertyBuilder merged = PropertyBuilder.array(Type.STRING); + merged.setName(UserConstants.REP_MEMBERS); + merged.setArray(); + + Set theirMembers = Sets.newHashSet(theirs.getValue(Type.STRINGS)); + Set ourMembers = Sets.newHashSet(ours.getValue(Type.STRINGS)); + + // merge ours and theirs to a de-duplicated set + HashSet combined = Sets.newHashSet(Iterables.concat(theirMembers, ourMembers)); + + // merge only those members that are present in ours as well as theirs + for (String member : combined) { + if (theirMembers.contains(member) && ourMembers.contains(member)) { + merged.addValue(member); + } + } + + parent.setProperty(merged.getPropertyState()); + } + + private static boolean isRepMembersProperty(PropertyState p) { + return UserConstants.REP_MEMBERS.equals(p.getName()); + } +} Index: ../../oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/ConcurrentGroupUpdateTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../../oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/ConcurrentGroupUpdateTest.java (revision ) +++ ../../oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/ConcurrentGroupUpdateTest.java (revision ) @@ -0,0 +1,218 @@ +/* + * 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.security; + +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.api.security.user.User; +import org.apache.jackrabbit.api.security.user.UserManager; +import org.apache.jackrabbit.oak.AbstractSecurityTest; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.Root; +import org.junit.Test; + +import javax.jcr.RepositoryException; +import javax.security.auth.login.LoginException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class ConcurrentGroupUpdateTest extends AbstractSecurityTest { + + /** + * The number of threads. + */ + private static final int NUM_WORKERS = 5; + + /** + * The id of the test group + */ + private static final String GROUP_ID = "test-groupId"; + + private Group group; + private User[] users; + + @Override + public void before() throws Exception { + super.before(); + UserManager um = getUserManager(root); + // create a group to receive users + group = um.createGroup(GROUP_ID); + + // create future members of the above group + User u1 = um.createUser("u1", "pass"); + User u2 = um.createUser("u2", "pass"); + User u3 = um.createUser("u3", "pass"); + User u4 = um.createUser("u4", "pass"); + User u5 = um.createUser("u5", "pass"); + root.commit(); + + users = new User[]{u1, u2, u3, u4, u5}; + } + + @Test + public void testConcurrentGroupMemberAdd() throws Exception { + + // parallel addition of users to group + startWorkers(Worker.MODE.ADD); + + // refresh to get the latest changes + root.refresh(); + + // verify users are now members (merged result) + assertTrue(group.isMember(users[0])); + assertTrue(group.isMember(users[1])); + assertTrue(group.isMember(users[2])); + assertTrue(group.isMember(users[3])); + assertTrue(group.isMember(users[4])); + } + + @Test + public void testConcurrentGroupMemberRemove() throws Exception { + + // first, add all users to the group + addUsersToGroup(); + + // parallel removal of users from group + startWorkers(Worker.MODE.REMOVE); + + // refresh to get the latest changes + root.refresh(); + + // verify users are no longer members (merged result) + assertFalse(group.isMember(users[0])); + assertFalse(group.isMember(users[1])); + assertFalse(group.isMember(users[2])); + assertFalse(group.isMember(users[3])); + assertFalse(group.isMember(users[4])); + } + + @Test + public void testConcurrentGroupMemberMixAddRemove() throws Exception { + + group.addMember(users[1]); + group.addMember(users[3]); + root.commit(); + + // parallel removal of users from group + startWorkers(Worker.MODE.MIX); + + // refresh to get the latest changes + root.refresh(); + } + + private void startWorkers(Worker.MODE mode) throws RepositoryException, LoginException, InterruptedException { + + List exceptions = Collections.synchronizedList(new ArrayList()); + List worker = new ArrayList(); + + // effect changes on the group in parallel with the given mode and users + for (int i = 0; i < NUM_WORKERS; i++) { + // each worker shall process using its own "session" + Root r = login(getAdminCredentials()).getLatestRoot(); + worker.add( + new Thread( + new Worker( + mode, + getUserManager(r), + r, + users[i].getID(), + exceptions + ) + ) + ); + } + + for (Thread t : worker) { + t.start(); + } + for (Thread t : worker) { + t.join(); + } + + for (Exception e : exceptions) { + e.printStackTrace(System.err); + fail(e.getMessage()); + } + } + + private void addUsersToGroup() throws RepositoryException, CommitFailedException { + for (User u : users) { + group.addMember(u); + } + root.commit(); + } + + private static final class Worker implements Runnable { + + public enum MODE { + ADD, + MIX, + REMOVE + } + + private final MODE mode; + private final UserManager um; + private final Root root; + private final String userId; + private final List exceptions; + + public Worker(MODE mode, + UserManager um, + Root root, + String userId, + List exceptions + ) { + this.mode = mode; + this.um = um; + this.root = root; + this.userId = userId; + this.exceptions = exceptions; + } + + @Override + public void run() { + try { + Group g = (Group) um.getAuthorizable(GROUP_ID); + User u = (User) um.getAuthorizable(userId); + switch (mode) { + case ADD: + g.addMember(u); + break; + case REMOVE: + g.removeMember(u); + break; + case MIX: + if (g.isMember(u)) { + g.removeMember(u); + } else { + g.addMember(u); + } + break; + default: + // do nothing + } + root.commit(); + } catch (Exception e) { + exceptions.add(e); + } + } + } +} Index: ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/JcrConflictHandler.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/JcrConflictHandler.java (revision 1701814) +++ ../../oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/commit/JcrConflictHandler.java (revision ) @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugins.commit; import com.google.common.collect.ImmutableList; +import org.apache.jackrabbit.oak.security.principal.RepMembersConflictHandler; import org.apache.jackrabbit.oak.spi.commit.CompositeConflictHandler; /** @@ -32,6 +33,7 @@ */ public static CompositeConflictHandler createJcrConflictHandler() { return new CompositeConflictHandler(ImmutableList.of( + new RepMembersConflictHandler(), new JcrLastModifiedConflictHandler(), new ChildOrderConflictHandler(), new AnnotatingConflictHandler()