OAK-10471: Implement ConflictHandler for UserPrincipalProvider Cache

oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandler.java

@@ -0,0 +1,95 @@
+/*
+ * 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.user;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.commit.DefaultThreeWayConflictHandler;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The {@code CacheConflictHandler} takes care of merging the {@code rep:expiration} property
+ * during parallel updates.
+ *<p>
+ * The conflict handler deals with the following conflicts:
+ * <ul>
+ *     <li>{@code addExistingProperty}  : {@code Resolution.IGNORED}, We should not have add conflints, since the {@code rep:{@code rep:expiration}} node is created with the user</li>
+ *     <li>{@code changeDeletedProperty}: {@code Resolution.IGNORED},</li>
+ *     <li>{@code changeChangedProperty}: {@code Resolution.MERGED}, the properties with higher {@code rep:expiration} get merged</li>
+ *     <li>{@code deleteChangedProperty}: {@code Resolution.IGNORED} .</li>
+ *     <li>{@code deleteDeletedProperty}: {@code Resolution.IGNORED}.</li>
+ *     <li>{@code changeDeletedNode}    : {@code Resolution.IGNORED}, .</li>
+ *     <li>{@code deleteChangedNode}    : {@code Resolution.IGNORED}, </li>
+ *     <li>{@code deleteDeletedNode}    : {@code Resolution.IGNORED}.</li>
+ * </ul>
+ */
+
+class CacheConflictHandler extends DefaultThreeWayConflictHandler {
+
+    private static final Logger LOG = LoggerFactory.getLogger(CacheConflictHandler.class);
+
+    protected CacheConflictHandler() {
+        super(Resolution.IGNORED);
+
+    }
+
+    private Resolution resolveRepExpirationConflict(@NotNull NodeBuilder parent, @NotNull PropertyState ours, @NotNull PropertyState theirs,
+                                         PropertyState base) {
+        if (CacheConstants.REP_EXPIRATION.equals(ours.getName()) && CacheConstants.REP_EXPIRATION.equals(theirs.getName())){
+
+            PropertyBuilder<Long> merged = PropertyBuilder.scalar(Type.LONG);
+            merged.setName(CacheConstants.REP_EXPIRATION);
+
+            //if base is bigger than ours and theirs, then use base. This should never happens
+            if (base != null &&
+                    base.getValue(Type.LONG) > ours.getValue(Type.LONG)  &&
+                    base.getValue(Type.LONG) > theirs.getValue(Type.LONG)){
+                merged.setValue(base.getValue(Type.LONG));
+                LOG.warn("base is bigger than ours and theirs. This was supposed to never happens");
+                return Resolution.MERGED;
+            }
+
+            //if ours is bigger than theirs, then use ours
+            //otherwise use theirs
+            if (ours.getValue(Type.LONG) > theirs.getValue(Type.LONG)){
+                merged.setValue(ours.getValue(Type.LONG));
+            } else {
+                merged.setValue(theirs.getValue(Type.LONG));
+            }
+            parent.setProperty(merged.getPropertyState());
+            LOG.debug("Resolved conflict for property {} our value: {}, their value {}, merged value: {}", CacheConstants.REP_EXPIRATION, ours.getValue(Type.LONG), theirs.getValue(Type.LONG), merged.getValue(0));
+            return Resolution.MERGED;
+        }
+        return Resolution.IGNORED;
+
+    }
+
+    @Override
+    public Resolution changeChangedProperty(@NotNull NodeBuilder parent, @NotNull PropertyState ours, @NotNull PropertyState theirs,
+                                            @NotNull PropertyState base) {
+
+        return resolveRepExpirationConflict(parent, ours, theirs, base);
+    }
+
+
+}

oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java

@@ -263,7 +263,7 @@ public List<? extends ValidatorProvider> getValidators(@NotNull String workspace
     @NotNull
     @Override
     public List<ThreeWayConflictHandler> getConflictHandlers() {
-        return ImmutableList.of(new RepMembersConflictHandler());
+        return ImmutableList.of(new RepMembersConflictHandler(), new CacheConflictHandler());
     }
 
     @NotNull

oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandlerTest.java

@@ -0,0 +1,181 @@
+/*
+ * 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.user;
+
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.api.ContentSession;
+import org.apache.jackrabbit.oak.api.PropertyState;
+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.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.authentication.SystemSubject;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalConfiguration;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider;
+import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Test;
+
+import javax.security.auth.Subject;
+import java.security.Principal;
+import java.security.PrivilegedExceptionAction;
+import java.util.Set;
+import java.util.UUID;
+
+import static org.apache.jackrabbit.oak.security.user.CacheConstants.REP_EXPIRATION;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class CacheConflictHandlerTest extends AbstractSecurityTest {
+
+    static final String PARAM_CACHE_EXPIRATION = "cacheExpiration";
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        String groupId = "testGroup" + UUID.randomUUID();
+        @NotNull Group testGroup = getUserManager(root).createGroup(groupId);
+        testGroup.addMember(getTestUser());
+
+        String groupId2 = "testGroup" + UUID.randomUUID() + "2";
+        @NotNull Group testGroup2 = getUserManager(root).createGroup(groupId2);
+        testGroup.addMember(testGroup2);
+
+        String groupId3 = "testGroup" + UUID.randomUUID() + "3";
+        @NotNull Group testGroup3 = getUserManager(root).createGroup(groupId3);
+
+        root.commit();
+    }
+
+    private Tree getCacheTree(Root root) throws Exception {
+        return getCacheTree(root, getTestUser().getPath());
+    }
+
+    private Tree getCacheTree(Root root, String authorizablePath) {
+        return root.getTree(authorizablePath + '/' + CacheConstants.REP_CACHE);
+    }
+
+    @Override
+    protected ConfigurationParameters getSecurityConfigParameters() {
+        return ConfigurationParameters.of(
+                UserConfiguration.NAME,
+                ConfigurationParameters.of(PARAM_CACHE_EXPIRATION, 3600 * 1000)
+        );
+    }
+
+    @Test
+    public void testChangeChangedPropertyLower() throws Exception {
+
+        PrincipalConfiguration pc = getConfig(PrincipalConfiguration.class);
+
+        Root oursRoot = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> login(null)).getLatestRoot();
+        Root theirsRoot = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> login(null)).getLatestRoot();
+
+        PrincipalProvider oursPP = pc.getPrincipalProvider(oursRoot, namePathMapper);
+        PrincipalProvider theirsPP = pc.getPrincipalProvider(theirsRoot, namePathMapper);
+
+        // set of principals that read from user + membership-provider -> cache being filled
+        oursPP.getPrincipals(getTestUser().getID());
+        assertTrue(getCacheTree(oursRoot).exists());
+        getCacheTree(oursRoot).getProperty("rep:expiration").getValue(Type.LONG).longValue();
+
+        theirsPP.getPrincipals(getTestUser().getID());
+        assertTrue(getCacheTree(theirsRoot).exists());
+        long theirExpiration = getCacheTree(theirsRoot).getProperty("rep:expiration").getValue(Type.LONG).longValue();
+
+
+        Tree ourCache = getCacheTree(oursRoot);
+        ourCache.setProperty(REP_EXPIRATION, 2);
+        oursRoot.commit(CacheValidatorProvider.asCommitAttributes());
+
+        root.commit();
+        assertEquals(getCacheTree(root).getProperty(REP_EXPIRATION).getValue(Type.LONG).longValue(), theirExpiration);
+
+    }
+
+    @Test
+    public void testChangeChangedPropertyHigher() throws Exception {
+
+        PrincipalConfiguration pc = getConfig(PrincipalConfiguration.class);
+
+        Root oursRoot = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> login(null)).getLatestRoot();
+        Root theirsRoot = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> login(null)).getLatestRoot();
+
+        PrincipalProvider oursPP = pc.getPrincipalProvider(oursRoot, namePathMapper);
+        PrincipalProvider theirsPP = pc.getPrincipalProvider(theirsRoot, namePathMapper);
+
+        // set of principals that read from user + membership-provider -> cache being filled
+        Set<? extends Principal> ourPrincipals = oursPP.getPrincipals(getTestUser().getID());
+        assertTrue(getCacheTree(oursRoot).exists());
+        getCacheTree(oursRoot).getProperty("rep:expiration").getValue(Type.LONG).longValue();
+
+        Set<? extends Principal> theirPrincipals = theirsPP.getPrincipals(getTestUser().getID());
+        assertTrue(getCacheTree(theirsRoot).exists());
+        long theirExpiration = getCacheTree(theirsRoot).getProperty("rep:expiration").getValue(Type.LONG).longValue();
+
+
+        Tree ourCache = getCacheTree(oursRoot);
+        ourCache.setProperty(REP_EXPIRATION, theirExpiration + 1000);
+        oursRoot.commit(CacheValidatorProvider.asCommitAttributes());
+
+        root.commit();
+        assertEquals(getCacheTree(root).getProperty(REP_EXPIRATION).getValue(Type.LONG).longValue(), theirExpiration + 1000);
+
+    }
+
+    @Test
+    public void testChangeChangedPropertyBaseHigher() {
+        NodeBuilder parent = mock(NodeBuilder.class);
+
+        PropertyState ours = mock(PropertyState.class);
+        PropertyState base = mock(PropertyState.class);
+        PropertyState theirs = mock(PropertyState.class);
+
+        when(ours.getName()).thenReturn(REP_EXPIRATION);
+        when(base.getName()).thenReturn(REP_EXPIRATION);
+        when(theirs.getName()).thenReturn(REP_EXPIRATION);
+
+        when(ours.getValue(Type.LONG)).thenReturn(1000L);
+        when(base.getValue(Type.LONG)).thenReturn(2000L);
+        when(theirs.getValue(Type.LONG)).thenReturn(900L);
+
+        CacheConflictHandler handler = new CacheConflictHandler();
+        assertEquals(CacheConflictHandler.Resolution.MERGED, handler.changeChangedProperty(parent, ours, theirs, base));
+
+    }
+
+    @Test
+    public void testChangeChangedPropertyIgnore() {
+        NodeBuilder parent = mock(NodeBuilder.class);
+
+        PropertyState ours = mock(PropertyState.class);
+        PropertyState base = mock(PropertyState.class);
+        PropertyState theirs = mock(PropertyState.class);
+
+        CacheConflictHandler handler = new CacheConflictHandler();
+        assertEquals(CacheConflictHandler.Resolution.IGNORED, handler.changeChangedProperty(parent, ours, theirs, base));
+
+    }
+}

oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java

@@ -83,7 +83,7 @@ public void testGetConflictHandlers() {
         UserConfigurationImpl configuration = new UserConfigurationImpl();
 
         List<ThreeWayConflictHandler> conflictHandlers = configuration.getConflictHandlers();
-        assertEquals(1, conflictHandlers.size());
+        assertEquals(2, conflictHandlers.size());
         assertTrue(conflictHandlers.get(0) instanceof RepMembersConflictHandler);
     }