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 an...@apache.org on 2023/03/28 18:13:47 UTC
[jackrabbit-oak] 01/01: OAK-10151 : oak-auth-external tests fail with Guava 20
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch OAK-10151
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 30783fdbebc9fd424210fa619f52c340fc19dd35
Author: angela <an...@adobe.com>
AuthorDate: Tue Mar 28 20:13:29 2023 +0200
OAK-10151 : oak-auth-external tests fail with Guava 20
---
.../external/impl/DynamicGroupsTest.java | 8 +-
.../external/impl/DynamicSyncContextTest.java | 4 +-
.../external/impl/DynamicSyncTest.java | 4 +-
.../jackrabbit/oak/commons/collect/Iterators.java | 83 +++++++++++++++++++++
.../oak/commons/collect/IteratorsTest.java | 85 ++++++++++++++++++++++
.../oak/security/user/AuthorizableImpl.java | 12 ++-
.../oak/security/user/AuthorizableIterator.java | 24 +++---
.../jackrabbit/oak/security/user/GroupImpl.java | 4 +-
8 files changed, 199 insertions(+), 25 deletions(-)
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
index 28975e3254..445c99b3f7 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
@@ -302,11 +302,11 @@ public class DynamicGroupsTest extends DynamicSyncContextTest {
String groupId2 = declaredGroupRefs.get(1).getId();
Group local = um.createGroup("localGroup");
local.addMembers(groupId, groupId2);
- userManager.createGroup(EveryonePrincipal.getInstance());
+ um.createGroup(EveryonePrincipal.getInstance());
r.commit();
- Authorizable a = getUserManager(r).getAuthorizable(PREVIOUS_SYNCED_ID);
- assertFalse(Iterators.contains(a.memberOf(), local));
+ Authorizable a = um.getAuthorizable(PREVIOUS_SYNCED_ID);
+ assertTrue(getIds(a.memberOf()).contains(local.getID()));
// sync again to establish dynamic membership
syncContext.setForceUserSync(true);
@@ -322,7 +322,7 @@ public class DynamicGroupsTest extends DynamicSyncContextTest {
assertFalse(groupIds.contains("localGroup"));
assertFalse(local.isMember(a));
} else {
- assertEquals((membershipNestingDepth > 1) ? 5 : 4, groupIds.size());
+ assertEquals("Found "+groupIds, (membershipNestingDepth > 1) ? 5 : 4, groupIds.size());
assertTrue(groupIds.contains("localGroup"));
assertTrue(local.isMember(a));
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
index dcd69d6ee9..712fc8eaca 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
@@ -49,6 +49,7 @@ import org.junit.Test;
import javax.jcr.RepositoryException;
import javax.jcr.Value;
import java.util.HashSet;
+import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
@@ -159,7 +160,8 @@ public class DynamicSyncContextTest extends AbstractDynamicTest {
Group gr = userManager.getAuthorizable(ref.getId(), Group.class);
assertNotNull(gr);
assertTrue(gr.isMember(a));
- assertTrue(Iterators.contains(a.memberOf(), gr));
+ List<String> ids = getIds(a.memberOf());
+ assertTrue("Expected "+ids+ " to contain "+gr.getID(), ids.contains(gr.getID()));
if (Iterables.contains(declaredGroupRefs, ref)) {
assertTrue(gr.isDeclaredMember(a));
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncTest.java
index 1f83bfad5d..61a67c8ebc 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncTest.java
@@ -230,8 +230,8 @@ public class DynamicSyncTest extends AbstractDynamicTest {
private static void assertExpectedIds(@NotNull Set<String> expectedIds, @NotNull Iterator<? extends Authorizable>... iterators) {
for (Iterator<? extends Authorizable> it : iterators) {
List<String> ids = getIds(it);
- assertEquals(expectedIds.size(), ids.size());
- assertTrue(ids.containsAll(expectedIds));
+ assertEquals("Expected "+expectedIds+" found "+ids, expectedIds.size(), ids.size());
+ assertTrue("Expected "+expectedIds+" found "+ids, ids.containsAll(expectedIds));
}
}
}
\ No newline at end of file
diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collect/Iterators.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collect/Iterators.java
new file mode 100644
index 0000000000..d2ee9754d8
--- /dev/null
+++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collect/Iterators.java
@@ -0,0 +1,83 @@
+/*
+ * 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.commons.collect;
+
+import com.google.common.collect.Lists;
+import org.apache.jackrabbit.commons.iterator.AbstractLazyIterator;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+public class Iterators {
+
+ private Iterators() {}
+
+ /**
+ * Combines two iterators into a single iterator. The returned iterator iterates across the elements of each
+ * iterator. The returned iterator does not support remove.
+ *
+ * @param input1 The first iterator to concatenate
+ * @param input2 The second iterator to concatenate
+ * @return An iterator wrapping the passed iterators.
+ */
+ @NotNull
+ public static <T> Iterator<T> concat(@NotNull Iterator<? extends T> input1, @NotNull Iterator<? extends T> input2) {
+ return concat(Lists.newArrayList(input1, input2));
+
+ }
+
+ /**
+ * Combines multiple iterators into a single iterator. The returned iterator iterates across the elements of each
+ * iterator in inputs. The returned iterator does not support remove.
+ *
+ * @param inputs The iterators to concatenate
+ * @return An iterator wrapping the passed iterators.
+ */
+ @NotNull
+ public static <T> Iterator<T> concat(@NotNull Iterator<? extends T>... inputs) {
+ switch (inputs.length) {
+ case 0: return Collections.emptyIterator();
+ case 1: return (Iterator<T>) inputs[0];
+ default: return concat(Lists.newArrayList(inputs));
+ }
+ }
+
+ @NotNull
+ private static <T> Iterator<T> concat(@NotNull List<Iterator<? extends T>> inputs) {
+ return new AbstractLazyIterator<T>() {
+ final List<Iterator<? extends T>> l = inputs;
+ Iterator<? extends T> it = l.remove(0);
+
+ @Override
+ protected T getNext() {
+ if (!it.hasNext()) {
+ if (l.isEmpty()) {
+ it = Collections.emptyIterator();
+ } else {
+ it = l.remove(0);
+ }
+ }
+ if (it.hasNext()) {
+ return it.next();
+ }
+ return null;
+ }
+ };
+ }
+}
\ No newline at end of file
diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collect/IteratorsTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collect/IteratorsTest.java
new file mode 100644
index 0000000000..2df83ee750
--- /dev/null
+++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collect/IteratorsTest.java
@@ -0,0 +1,85 @@
+/*
+ * 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.commons.collect;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class IteratorsTest {
+
+ @Test
+ public void testRemove() {
+ Iterator<String> it = Iterators.concat(Collections.singletonList("a").iterator(), Collections.singletonList("b").iterator());
+ while (it.hasNext()) {
+ it.next();
+ try {
+ it.remove();
+ fail("Iterator.remove is not supported");
+ } catch (UnsupportedOperationException e) {
+ // success
+ }
+ }
+ }
+
+ @Test
+ public void testZeroIterator() {
+ assertSame(Collections.emptyIterator(), Iterators.concat());
+ }
+
+ @Test
+ public void testSingleIterator() {
+ Iterator<String> it = Collections.singletonList("a").iterator();
+ assertSame(it, Iterators.concat(it));
+ }
+
+ @Test
+ public void testTwoIterators() {
+ Iterator<String> it1 = Collections.singletonList("a").iterator();
+ Iterator<String> it2 = Collections.singletonList("b").iterator();
+
+ assertEquals(ImmutableList.of("a", "b"), ImmutableList.copyOf(Iterators.concat(it1, it2)));
+ }
+
+ @Test
+ public void testRepeatedHasNext() {
+ Iterator<String> it1 = Collections.singletonList("a").iterator();
+ Iterator<String> it2 = Collections.singletonList("b").iterator();
+ Iterator<String> it3 = Collections.singletonList("c").iterator();
+
+ Iterator<String> it = Iterators.concat(it1, it2, it3);
+ List<String> result = new ArrayList<>();
+ while (it.hasNext()) {
+ assertTrue(it.hasNext());
+ String s = it.next();
+ assertNotNull(s);
+ result.add(s);
+ }
+ assertEquals(Lists.newArrayList("a", "b", "c"), result);
+ }
+}
\ No newline at end of file
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
index f9647c8e4e..40d1c734ef 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
@@ -288,9 +288,13 @@ abstract class AuthorizableImpl implements Authorizable, UserConstants, TreeAwar
MembershipProvider mMgr = getMembershipProvider();
Iterator<Tree> trees = mMgr.getMembership(getTree(), includeInherited);
-
- AuthorizableIterator groups = (!trees.hasNext()) ? AuthorizableIterator.empty() : AuthorizableIterator.create(trees, userManager, AuthorizableType.GROUP);
- AuthorizableIterator allGroups = AuthorizableIterator.create(true, dynamicGroups, groups);
- return new RangeIteratorAdapter(allGroups);
+
+ if (!trees.hasNext()) {
+ return new RangeIteratorAdapter(AuthorizableIterator.create(true, dynamicGroups));
+ } else {
+ AuthorizableIterator groups = AuthorizableIterator.create(trees, userManager, AuthorizableType.GROUP);
+ AuthorizableIterator allGroups = AuthorizableIterator.create(true, dynamicGroups, groups);
+ return new RangeIteratorAdapter(allGroups);
+ }
}
}
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableIterator.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableIterator.java
index 03be2bf3c6..6648c8ac68 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableIterator.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableIterator.java
@@ -29,13 +29,14 @@ import org.slf4j.LoggerFactory;
import javax.jcr.RangeIterator;
import javax.jcr.RepositoryException;
-import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
+import static org.apache.jackrabbit.oak.commons.collect.Iterators.concat;
+
/**
* AuthorizableIterator...
*/
@@ -43,12 +44,10 @@ final class AuthorizableIterator implements Iterator<Authorizable> {
private static final Logger log = LoggerFactory.getLogger(AuthorizableIterator.class);
- private final Iterator<Authorizable> authorizables;
+ private final Iterator<? extends Authorizable> authorizables;
private final long size;
private final Set<String> servedIds;
-
- private static AuthorizableIterator EMPTY = new AuthorizableIterator(Collections.emptyIterator(), 0, false);
-
+
@NotNull
static AuthorizableIterator create(@NotNull Iterator<Tree> authorizableTrees,
@NotNull UserManagerImpl userManager,
@@ -57,6 +56,12 @@ final class AuthorizableIterator implements Iterator<Authorizable> {
long size = getSize(authorizableTrees);
return new AuthorizableIterator(it, size, false);
}
+
+ @NotNull
+ static AuthorizableIterator create(boolean filterDuplicates, @NotNull Iterator<? extends Authorizable> it1) {
+ long size = getSize(it1);
+ return new AuthorizableIterator(it1, size, filterDuplicates);
+ }
@NotNull
static AuthorizableIterator create(boolean filterDuplicates, @NotNull Iterator<? extends Authorizable> it1, @NotNull Iterator<? extends Authorizable> it2) {
@@ -70,15 +75,10 @@ final class AuthorizableIterator implements Iterator<Authorizable> {
size = LongUtils.safeAdd(size, l);
}
}
- return new AuthorizableIterator(Iterators.concat(it1, it2), size, filterDuplicates);
- }
-
- @NotNull
- static AuthorizableIterator empty() {
- return EMPTY;
+ return new AuthorizableIterator(concat(it1, it2), size, filterDuplicates);
}
- private AuthorizableIterator(Iterator<Authorizable> authorizables, long size, boolean filterDuplicates) {
+ private AuthorizableIterator(Iterator<? extends Authorizable> authorizables, long size, boolean filterDuplicates) {
if (filterDuplicates) {
this.servedIds = new HashSet<>();
this.authorizables = Iterators.filter(authorizables, authorizable -> {
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java
index 48076dcdb8..f41484ee0e 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/GroupImpl.java
@@ -213,13 +213,13 @@ class GroupImpl extends AuthorizableImpl implements Group {
DynamicMembershipProvider dmp = getUserManager().getDynamicMembershipProvider();
Iterator<Authorizable> dynamicMembers = dmp.getMembers(this, includeInherited);
if (dmp.coversAllMembers(this)) {
- return AuthorizableIterator.create(true, dynamicMembers, AuthorizableIterator.empty());
+ return AuthorizableIterator.create(true, dynamicMembers);
}
// dynamic membership didn't cover all members -> extract from group-tree
Iterator<Tree> trees = getMembershipProvider().getMembers(getTree(), includeInherited);
if (!trees.hasNext()) {
- return AuthorizableIterator.create(true, dynamicMembers, AuthorizableIterator.empty());
+ return AuthorizableIterator.create(true, dynamicMembers);
}
Iterator<Authorizable> members = AuthorizableIterator.create(trees, userMgr, AuthorizableType.AUTHORIZABLE);