This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 62a3f776cb OAK-10471: Implement ConflictHandler for
UserPrincipalProvider Cache
62a3f776cb is described below
commit 62a3f776cb4febe17003f8d550c749846d561324
Author: Nicola Scendoni <[email protected]>
AuthorDate: Thu Oct 12 14:07:18 2023 +0200
OAK-10471: Implement ConflictHandler for UserPrincipalProvider Cache
---
.../oak/security/user/CacheConflictHandler.java | 95 +++++++++++
.../oak/security/user/UserConfigurationImpl.java | 2 +-
.../security/user/CacheConflictHandlerTest.java | 181 +++++++++++++++++++++
.../security/user/UserConfigurationImplTest.java | 2 +-
4 files changed, 278 insertions(+), 2 deletions(-)
diff --git
a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandler.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandler.java
new file mode 100644
index 0000000000..4122c1b070
--- /dev/null
+++
b/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);
+ }
+
+
+}
diff --git
a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
index 06696dc4a6..84e3826ff2 100644
---
a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
+++
b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImpl.java
@@ -263,7 +263,7 @@ public class UserConfigurationImpl extends
ConfigurationBase implements UserConf
@NotNull
@Override
public List<ThreeWayConflictHandler> getConflictHandlers() {
- return ImmutableList.of(new RepMembersConflictHandler());
+ return ImmutableList.of(new RepMembersConflictHandler(), new
CacheConflictHandler());
}
@NotNull
diff --git
a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandlerTest.java
b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/CacheConflictHandlerTest.java
new file mode 100644
index 0000000000..d32bf4813d
--- /dev/null
+++
b/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));
+
+ }
+}
\ No newline at end of file
diff --git
a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java
b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java
index d026cf5440..f08789c99d 100644
---
a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java
+++
b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java
@@ -83,7 +83,7 @@ public class UserConfigurationImplTest extends
AbstractSecurityTest {
UserConfigurationImpl configuration = new UserConfigurationImpl();
List<ThreeWayConflictHandler> conflictHandlers =
configuration.getConflictHandlers();
- assertEquals(1, conflictHandlers.size());
+ assertEquals(2, conflictHandlers.size());
assertTrue(conflictHandlers.get(0) instanceof
RepMembersConflictHandler);
}
