Author: angela
Date: Wed May 31 10:14:40 2017
New Revision: 1797008
URL: http://svn.apache.org/viewvc?rev=1797008&view=rev
Log:
OAK-5882 : Improve coverage for oak.security code in oak-core
OAK-6290 : UserQueryManager.findAuthorizables fails with
IllegalArgumentException when there are multiple selectors (test illustrating
the issue)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java?rev=1797008&r1=1797007&r2=1797008&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
Wed May 31 10:14:40 2017
@@ -332,9 +332,8 @@ public class UserQueryManager {
@Override
public boolean apply(@Nullable Authorizable input) {
try {
- if (input != null && !authorizableIds.contains(input.getID()))
{
- authorizableIds.add(input.getID());
- return true;
+ if (input != null) {
+ return authorizableIds.add(input.getID());
}
} catch (RepositoryException e) {
log.debug("Failed to retrieve authorizable ID " +
e.getMessage());
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java?rev=1797008&r1=1797007&r2=1797008&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java
Wed May 31 10:14:40 2017
@@ -35,11 +35,11 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.user.QueryBuilder;
import org.apache.jackrabbit.api.security.user.User;
import org.apache.jackrabbit.oak.AbstractSecurityTest;
-import org.apache.jackrabbit.oak.plugins.value.jcr.ValueFactoryImpl;
import org.apache.jackrabbit.oak.security.user.UserManagerImpl;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -58,9 +58,10 @@ public class UserQueryManagerTest extend
private ValueFactory valueFactory;
private UserQueryManager queryMgr;
private User user;
- private String userId;
private String propertyName;
+ private Value v;
+
private List<Group> groups = new ArrayList();
@Before
@@ -69,11 +70,11 @@ public class UserQueryManagerTest extend
UserManagerImpl userMgr = (UserManagerImpl) getUserManager(root);
user = getTestUser();
- userId = user.getID();
queryMgr = new UserQueryManager(userMgr, namePathMapper,
getUserConfiguration().getParameters(), root);
- valueFactory = new ValueFactoryImpl(root, namePathMapper);
+ valueFactory = getValueFactory(root);
propertyName = "testProperty";
+ v = valueFactory.createValue("value");
getQueryEngineSettings().setFailTraversal(false);
}
@@ -82,6 +83,7 @@ public class UserQueryManagerTest extend
public void after() throws Exception {
try {
getQueryEngineSettings().setFailTraversal(true);
+ user.removeProperty(propertyName);
for (Group g : groups) {
g.remove();
}
@@ -133,15 +135,8 @@ public class UserQueryManagerTest extend
user.setProperty(propertyName, vs);
root.commit();
- try {
- Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value \\, containing backslash",
AuthorizableType.USER, true);
- assertTrue("expected result", result.hasNext());
- assertEquals(user.getID(), result.next().getID());
- assertFalse("expected no more results", result.hasNext());
- } finally {
- user.removeProperty(propertyName);
- root.commit();
- }
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value \\, containing backslash",
AuthorizableType.USER, true);
+ assertResultContainsAuthorizables(result, user);
}
@Test
@@ -150,50 +145,89 @@ public class UserQueryManagerTest extend
user.setProperty(propertyName, vs);
root.commit();
- try {
- Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value \\, containing backslash",
AuthorizableType.USER, false);
- assertTrue("expected result", result.hasNext());
- assertEquals(user.getID(), result.next().getID());
- assertFalse("expected no more results", result.hasNext());
- } finally {
- user.removeProperty(propertyName);
- root.commit();
- }
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value \\, containing backslash",
AuthorizableType.USER, false);
+ assertResultContainsAuthorizables(result, user);
}
@Test
public void testFindNodesNonExactWithApostrophe() throws Exception {
Value vs = valueFactory.createValue("value ' with apostrophe");
- try {
- user.setProperty(propertyName, vs);
- root.commit();
-
- Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value ' with apostrophe",
AuthorizableType.USER, false);
+ user.setProperty(propertyName, vs);
+ root.commit();
- assertTrue("expected result", result.hasNext());
- assertEquals(user.getID(), result.next().getID());
- assertFalse("expected no more results", result.hasNext());
- } finally {
- user.removeProperty(propertyName);
- root.commit();
- }
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value ' with apostrophe",
AuthorizableType.USER, false);
+ assertResultContainsAuthorizables(result, user);
}
@Test
public void testFindNodesExactWithApostrophe() throws Exception {
Value vs = valueFactory.createValue("value ' with apostrophe");
- try {
- user.setProperty(propertyName, vs);
- root.commit();
+ user.setProperty(propertyName, vs);
+ root.commit();
- Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value ' with apostrophe",
AuthorizableType.USER, true);
- assertTrue("expected result", result.hasNext());
- assertEquals(user.getID(), result.next().getID());
- assertFalse("expected no more results", result.hasNext());
- } finally {
- user.removeProperty(propertyName);
- root.commit();
- }
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, "value ' with apostrophe",
AuthorizableType.USER, true);
+ assertResultContainsAuthorizables(result, user);
+ }
+
+ @Test
+ public void testFindWithCurrentRelPathTypeMismatch() throws Exception {
+ user.setProperty(propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result = queryMgr.findAuthorizables("./" +
propertyName, v.getString(), AuthorizableType.GROUP, false);
+ assertResultContainsAuthorizables(result);
+ }
+
+
+ @Test
+ public void testFindWithCurrentRelPath() throws Exception {
+ user.setProperty(propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result = queryMgr.findAuthorizables("./" +
propertyName, v.getString(), AuthorizableType.USER, false);
+ assertResultContainsAuthorizables(result, user);
+ }
+
+ @Test
+ public void testFindWithRelPath() throws Exception {
+ user.setProperty(propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables("rel/path/to/" + propertyName, v.getString(),
AuthorizableType.USER, false);
+ assertResultContainsAuthorizables(result);
+ }
+
+ @Ignore("OAK-6290")
+ @Test
+ public void testFindWithRelPathMultipleSelectorNames() throws Exception {
+ user.setProperty(propertyName, v);
+ Group g = createGroup("g", null);
+ g.setProperty("rel/path/to/" + propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables("rel/path/to/" + propertyName, v.getString(),
AuthorizableType.AUTHORIZABLE, false);
+ assertResultContainsAuthorizables(result, g);
+ }
+
+ @Test
+ public void testFindWithRelPathTypeMismatch() throws Exception {
+ user.setProperty(propertyName, v);
+ Group g = createGroup("g", null);
+ g.setProperty("rel/path/to/" + propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables("rel/path/to/" + propertyName, v.getString(),
AuthorizableType.USER, false);
+ assertResultContainsAuthorizables(result);
+ }
+
+ @Test
+ public void testFilterDuplicateResults() throws Exception {
+ user.setProperty(propertyName, v);
+ user.setProperty("rel/path/to/" + propertyName, v);
+ root.commit();
+
+ Iterator<Authorizable> result =
queryMgr.findAuthorizables(propertyName, v.getString(),
AuthorizableType.AUTHORIZABLE, false);
+ assertResultContainsAuthorizables(result, user);
}
@Test
@@ -210,6 +244,7 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeEveryoneNonExisting() throws Exception {
+ String userId = user.getID();
Query q = new Query() {
@Override
public <T> void build(QueryBuilder<T> builder) {
@@ -224,8 +259,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeEveryoneFiltersEveryone() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup(null, EveryonePrincipal.getInstance());
g.setProperty(propertyName, v);
user.setProperty(propertyName, v);
@@ -245,8 +278,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeEveryoneWithIdDiffersPrincipalName() throws
Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup("eGroup", EveryonePrincipal.getInstance());
g.setProperty(propertyName, v);
user.setProperty(propertyName, v);
@@ -266,8 +297,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryNoScope() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup(null, EveryonePrincipal.getInstance());
g.setProperty(propertyName, v);
user.setProperty(propertyName, v);
@@ -286,8 +315,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeNotMember() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup("g1", null);
user.setProperty(propertyName, v);
root.commit();
@@ -306,8 +333,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeDeclaredMember() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup("g1", null);
g.addMember(user);
user.setProperty(propertyName, v);
@@ -327,8 +352,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeDeclaredMembership() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup("g1", null);
Group g2 = createGroup("g2", null);
g.addMember(g2);
@@ -350,8 +373,6 @@ public class UserQueryManagerTest extend
@Test
public void testQueryScopeInheritedMembership() throws Exception {
- Value v = getValueFactory(root).createValue("value");
-
Group g = createGroup("g1", null);
Group g2 = createGroup("g2", null);
g.addMember(g2);
@@ -372,19 +393,18 @@ public class UserQueryManagerTest extend
@Test
public void testQueryBoundWithoutSortOrder() throws Exception {
- ValueFactory vf = getValueFactory(root);
Group g = createGroup("g1", null);
- g.setProperty(propertyName, vf.createValue(50));
+ g.setProperty(propertyName, valueFactory.createValue(50));
Group g2 = createGroup("g2", null);
- g2.setProperty(propertyName, vf.createValue(60));
- user.setProperty(propertyName, vf.createValue(101));
+ g2.setProperty(propertyName, valueFactory.createValue(60));
+ user.setProperty(propertyName, valueFactory.createValue(101));
root.commit();
Query q = new Query() {
@Override
public <T> void build(QueryBuilder<T> builder) {
- builder.setLimit(vf.createValue(100), Long.MAX_VALUE);
- builder.setCondition(builder.gt(propertyName,
vf.createValue(20)));
+ builder.setLimit(valueFactory.createValue(100),
Long.MAX_VALUE);
+ builder.setCondition(builder.gt(propertyName,
valueFactory.createValue(20)));
}
};
@@ -394,20 +414,19 @@ public class UserQueryManagerTest extend
@Test
public void testQueryBoundWithSortOrder() throws Exception {
- ValueFactory vf = getValueFactory(root);
Group g = createGroup("g1", null);
- g.setProperty(propertyName, vf.createValue(50));
+ g.setProperty(propertyName, valueFactory.createValue(50));
Group g2 = createGroup("g2", null);
- g2.setProperty(propertyName, vf.createValue(60));
- user.setProperty(propertyName, vf.createValue(101));
+ g2.setProperty(propertyName, valueFactory.createValue(60));
+ user.setProperty(propertyName, valueFactory.createValue(101));
root.commit();
Query q = new Query() {
@Override
public <T> void build(QueryBuilder<T> builder) {
- builder.setLimit(vf.createValue(100), Long.MAX_VALUE);
+ builder.setLimit(valueFactory.createValue(100),
Long.MAX_VALUE);
builder.setSortOrder(propertyName,
QueryBuilder.Direction.ASCENDING);
- builder.setCondition(builder.gt(propertyName,
vf.createValue(20)));
+ builder.setCondition(builder.gt(propertyName,
valueFactory.createValue(20)));
}
};
@@ -417,18 +436,17 @@ public class UserQueryManagerTest extend
@Test
public void testQueryBoundWithSortOrderMissingCondition() throws Exception
{
- ValueFactory vf = getValueFactory(root);
Group g = createGroup("g1", null);
- g.setProperty(propertyName, vf.createValue(50));
+ g.setProperty(propertyName, valueFactory.createValue(50));
Group g2 = createGroup("g2", null);
- g2.setProperty(propertyName, vf.createValue(60));
- user.setProperty(propertyName, vf.createValue(101));
+ g2.setProperty(propertyName, valueFactory.createValue(60));
+ user.setProperty(propertyName, valueFactory.createValue(101));
root.commit();
Query q = new Query() {
@Override
public <T> void build(QueryBuilder<T> builder) {
- builder.setLimit(vf.createValue(100), Long.MAX_VALUE);
+ builder.setLimit(valueFactory.createValue(100),
Long.MAX_VALUE);
builder.setSortOrder(propertyName,
QueryBuilder.Direction.ASCENDING);
}
};
@@ -439,12 +457,11 @@ public class UserQueryManagerTest extend
@Test
public void testQuerySortIgnoreCase() throws Exception {
- ValueFactory vf = getValueFactory(root);
Group g = createGroup("g1", null);
- g.setProperty(propertyName, vf.createValue("aaa"));
+ g.setProperty(propertyName, valueFactory.createValue("aaa"));
Group g2 = createGroup("g2", null);
- g2.setProperty(propertyName, vf.createValue("BBB"));
- user.setProperty(propertyName, vf.createValue("c"));
+ g2.setProperty(propertyName, valueFactory.createValue("BBB"));
+ user.setProperty(propertyName, valueFactory.createValue("c"));
root.commit();
Query q = new Query() {
@@ -461,12 +478,11 @@ public class UserQueryManagerTest extend
@Test
public void testQuerySortRespectCase() throws Exception {
- ValueFactory vf = getValueFactory(root);
Group g = createGroup("g1", null);
- g.setProperty(propertyName, vf.createValue("aaa"));
+ g.setProperty(propertyName, valueFactory.createValue("aaa"));
Group g2 = createGroup("g2", null);
- g2.setProperty(propertyName, vf.createValue("BBB"));
- user.setProperty(propertyName, vf.createValue("c"));
+ g2.setProperty(propertyName, valueFactory.createValue("BBB"));
+ user.setProperty(propertyName, valueFactory.createValue("c"));
root.commit();
Query q = new Query() {