You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by il...@apache.org on 2018/12/14 09:48:32 UTC
[syncope] branch 2_0_X updated: [SYNCOPE-1417] Raise exception when
more than one plain attribute is requested for ordering
This is an automated email from the ASF dual-hosted git repository.
ilgrosso pushed a commit to branch 2_0_X
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/2_0_X by this push:
new a5f2bcd [SYNCOPE-1417] Raise exception when more than one plain attribute is requested for ordering
a5f2bcd is described below
commit a5f2bcd8509d6afba0a189a29ce8d71e35af123d
Author: Francesco Chicchiriccò <il...@apache.org>
AuthorDate: Fri Dec 14 10:48:19 2018 +0100
[SYNCOPE-1417] Raise exception when more than one plain attribute is requested for ordering
---
.../core/persistence/jpa/dao/JPAAnySearchDAO.java | 49 ++++++++++----
.../core/persistence/jpa/outer/AnySearchTest.java | 77 ++++++++++++++++------
.../org/apache/syncope/fit/core/SearchITCase.java | 20 +++++-
3 files changed, 110 insertions(+), 36 deletions(-)
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
index 1fcd907..718b0bb 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAAnySearchDAO.java
@@ -127,7 +127,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
Pair<String, Set<String>> filter = getAdminRealmsFilter(adminRealms, svs, parameters);
// 1. get the query string from the search condition
- Pair<StringBuilder, Set<String>> queryInfo =
+ Pair<StringBuilder, Set<String>> queryInfo =
getQuery(buildEffectiveCond(cond, filter.getRight()), parameters, svs);
StringBuilder queryString = queryInfo.getLeft();
@@ -164,8 +164,8 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
Pair<String, Set<String>> filter = getAdminRealmsFilter(adminRealms, svs, parameters);
// 1. get the query string from the search condition
- Pair<StringBuilder, Set<String>> queryInfo = getQuery(buildEffectiveCond(cond, filter.getRight()),
- parameters, svs);
+ Pair<StringBuilder, Set<String>> queryInfo =
+ getQuery(buildEffectiveCond(cond, filter.getRight()), parameters, svs);
StringBuilder queryString = queryInfo.getLeft();
@@ -197,6 +197,8 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
// 6. Prepare the result (avoiding duplicates)
return buildResult(query.getResultList(), kind);
+ } catch (SyncopeClientException e) {
+ throw e;
} catch (Exception e) {
LOG.error("While searching for {}", kind, e);
}
@@ -240,7 +242,9 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
}
private StringBuilder buildWhere(
- final SearchSupport svs, final Set<String> involvedPlainAttrs, final OrderBySupport obs) {
+ final SearchSupport svs,
+ final Set<String> involvedPlainAttrs,
+ final OrderBySupport obs) {
Set<String> attrs = new HashSet<>(involvedPlainAttrs);
for (OrderBySupport.Item item : obs.items) {
@@ -324,6 +328,9 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
OrderBySupport obs = new OrderBySupport();
+ Set<String> orderByUniquePlainSchemas = new HashSet<>();
+ Set<String> orderByNonUniquePlainSchemas = new HashSet<>();
+
for (OrderByClause clause : filterOrderBy(orderBy)) {
OrderBySupport.Item item = new OrderBySupport.Item();
@@ -333,6 +340,20 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
if (ReflectionUtils.findField(attrUtils.anyClass(), fieldName) == null) {
PlainSchema schema = schemaDAO.find(fieldName);
if (schema != null) {
+ if (schema.isUniqueConstraint()) {
+ orderByUniquePlainSchemas.add(schema.getKey());
+ } else {
+ orderByNonUniquePlainSchemas.add(schema.getKey());
+ }
+ if (orderByUniquePlainSchemas.size() > 1 || orderByNonUniquePlainSchemas.size() > 1) {
+ SyncopeClientException invalidSearch =
+ SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ invalidSearch.getElements().add("Order by more than one attribute is not allowed; "
+ + "remove one from " + (orderByUniquePlainSchemas.size() > 1
+ ? orderByUniquePlainSchemas : orderByNonUniquePlainSchemas));
+ throw invalidSearch;
+ }
+
// keep track of involvement of non-mandatory schemas in the order by clauses
obs.nonMandatorySchemas = !"true".equals(schema.getMandatoryCondition());
@@ -382,10 +403,12 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
}
private Pair<StringBuilder, Set<String>> getQuery(
- final SearchCond cond, final List<Object> parameters, final SearchSupport svs) {
- StringBuilder query = new StringBuilder();
- Set<String> involvedAttributes = new HashSet<>();
+ final SearchCond cond,
+ final List<Object> parameters,
+ final SearchSupport svs) {
+ StringBuilder query = new StringBuilder();
+ Set<String> involvedPlainAttrs = new HashSet<>();
switch (cond.getType()) {
case LEAF:
case NOT_LEAF:
@@ -425,7 +448,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
query.append(getQuery(cond.getAttributeCond(),
cond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
try {
- involvedAttributes.add(check(cond.getAttributeCond(), svs.anyTypeKind).getLeft().getKey());
+ involvedPlainAttrs.add(check(cond.getAttributeCond(), svs.anyTypeKind).getLeft().getKey());
} catch (IllegalArgumentException e) {
// ignore
}
@@ -437,10 +460,10 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
case AND:
Pair<StringBuilder, Set<String>> leftAndInfo = getQuery(cond.getLeftSearchCond(), parameters, svs);
- involvedAttributes.addAll(leftAndInfo.getRight());
+ involvedPlainAttrs.addAll(leftAndInfo.getRight());
Pair<StringBuilder, Set<String>> rigthAndInfo = getQuery(cond.getRightSearchCond(), parameters, svs);
- involvedAttributes.addAll(rigthAndInfo.getRight());
+ involvedPlainAttrs.addAll(rigthAndInfo.getRight());
String andSubQuery = leftAndInfo.getKey().toString();
// Add extra parentheses
@@ -453,10 +476,10 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
case OR:
Pair<StringBuilder, Set<String>> leftOrInfo = getQuery(cond.getLeftSearchCond(), parameters, svs);
- involvedAttributes.addAll(leftOrInfo.getRight());
+ involvedPlainAttrs.addAll(leftOrInfo.getRight());
Pair<StringBuilder, Set<String>> rigthOrInfo = getQuery(cond.getRightSearchCond(), parameters, svs);
- involvedAttributes.addAll(rigthOrInfo.getRight());
+ involvedPlainAttrs.addAll(rigthOrInfo.getRight());
String orSubQuery = leftOrInfo.getKey().toString();
// Add extra parentheses
@@ -470,7 +493,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
default:
}
- return Pair.of(query, involvedAttributes);
+ return Pair.of(query, involvedPlainAttrs);
}
private String getQuery(
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
index 9d9d287..31c9e17 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/AnySearchTest.java
@@ -21,15 +21,21 @@ package org.apache.syncope.core.persistence.jpa.outer;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import java.util.ArrayList;
import java.util.List;
+import org.apache.syncope.common.lib.SyncopeClientException;
import org.apache.syncope.common.lib.types.AnyTypeKind;
+import org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.lib.types.StandardEntitlement;
import org.apache.syncope.core.persistence.api.dao.GroupDAO;
import org.apache.syncope.core.persistence.api.dao.RealmDAO;
import org.apache.syncope.core.persistence.api.dao.RoleDAO;
import org.apache.syncope.core.persistence.api.dao.AnySearchDAO;
+import org.apache.syncope.core.persistence.api.dao.search.AnyCond;
import org.apache.syncope.core.persistence.api.dao.search.AttributeCond;
+import org.apache.syncope.core.persistence.api.dao.search.OrderByClause;
import org.apache.syncope.core.persistence.api.dao.search.RoleCond;
import org.apache.syncope.core.persistence.api.dao.search.SearchCond;
import org.apache.syncope.core.persistence.api.entity.user.DynRoleMembership;
@@ -57,27 +63,6 @@ public class AnySearchTest extends AbstractTest {
private RoleDAO roleDAO;
@Test
- public void issueSYNCOPE95() {
- for (Group group : groupDAO.findAll(1, 100)) {
- groupDAO.delete(group.getKey());
- }
- groupDAO.flush();
-
- AttributeCond coolLeafCond = new AttributeCond(AttributeCond.Type.EQ);
- coolLeafCond.setSchema("cool");
- coolLeafCond.setExpression("true");
-
- SearchCond cond = SearchCond.getLeafCond(coolLeafCond);
- assertTrue(cond.isValid());
-
- List<User> users = searchDAO.search(cond, AnyTypeKind.USER);
- assertNotNull(users);
- assertEquals(1, users.size());
-
- assertEquals("c9b2dec2-00a7-4855-97c0-d854842b4b24", users.get(0).getKey());
- }
-
- @Test
public void searchByDynMembership() {
// 1. create role with dynamic membership
Role role = entityFactory.newEntity(Role.class);
@@ -107,4 +92,54 @@ public class AnySearchTest extends AbstractTest {
assertEquals(1, users.size());
assertEquals("c9b2dec2-00a7-4855-97c0-d854842b4b24", users.get(0).getKey());
}
+
+ @Test
+ public void issueSYNCOPE95() {
+ for (Group group : groupDAO.findAll(1, 100)) {
+ groupDAO.delete(group.getKey());
+ }
+ groupDAO.flush();
+
+ AttributeCond coolLeafCond = new AttributeCond(AttributeCond.Type.EQ);
+ coolLeafCond.setSchema("cool");
+ coolLeafCond.setExpression("true");
+
+ SearchCond cond = SearchCond.getLeafCond(coolLeafCond);
+ assertTrue(cond.isValid());
+
+ List<User> users = searchDAO.search(cond, AnyTypeKind.USER);
+ assertNotNull(users);
+ assertEquals(1, users.size());
+
+ assertEquals("c9b2dec2-00a7-4855-97c0-d854842b4b24", users.get(0).getKey());
+ }
+
+ @Test
+ public void issueSYNCOPE1417() {
+ AnyCond usernameLeafCond = new AnyCond(AnyCond.Type.EQ);
+ usernameLeafCond.setSchema("username");
+ usernameLeafCond.setExpression("rossini");
+ AttributeCond idRightCond = new AttributeCond(AttributeCond.Type.LIKE);
+ idRightCond.setSchema("fullname");
+ idRightCond.setExpression("Giuseppe V%");
+ SearchCond searchCondition = SearchCond.getOrCond(
+ SearchCond.getLeafCond(usernameLeafCond), SearchCond.getLeafCond(idRightCond));
+
+ List<OrderByClause> orderByClauses = new ArrayList<>();
+ OrderByClause orderByClause = new OrderByClause();
+ orderByClause.setField("surname");
+ orderByClause.setDirection(OrderByClause.Direction.DESC);
+ orderByClauses.add(orderByClause);
+ orderByClause = new OrderByClause();
+ orderByClause.setField("firstname");
+ orderByClause.setDirection(OrderByClause.Direction.ASC);
+ orderByClauses.add(orderByClause);
+
+ try {
+ searchDAO.search(searchCondition, orderByClauses, AnyTypeKind.USER);
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ }
+ }
}
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
index 54b81f4..65003df 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/SearchITCase.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.util.Collection;
import javax.ws.rs.core.Response;
@@ -31,12 +32,12 @@ import org.apache.commons.collections4.IterableUtils;
import org.apache.commons.collections4.Predicate;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.syncope.client.lib.SyncopeClient;
+import org.apache.syncope.common.lib.SyncopeClientException;
import org.apache.syncope.common.lib.SyncopeConstants;
import org.apache.syncope.common.lib.patch.AnyObjectPatch;
import org.apache.syncope.common.lib.patch.AttrPatch;
import org.apache.syncope.common.lib.patch.MembershipPatch;
import org.apache.syncope.common.lib.patch.UserPatch;
-import org.apache.syncope.common.lib.search.SpecialAttr;
import org.apache.syncope.common.lib.to.AnyObjectTO;
import org.apache.syncope.common.lib.to.AnyTypeTO;
import org.apache.syncope.common.lib.to.PagedResult;
@@ -45,6 +46,7 @@ import org.apache.syncope.common.lib.to.MembershipTO;
import org.apache.syncope.common.lib.to.RoleTO;
import org.apache.syncope.common.lib.to.UserTO;
import org.apache.syncope.common.lib.types.AnyTypeKind;
+import org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.rest.api.beans.AnyQuery;
import org.apache.syncope.common.rest.api.service.RoleService;
import org.apache.syncope.fit.AbstractITCase;
@@ -306,7 +308,7 @@ public class SearchITCase extends AbstractITCase {
and("username").equalTo("bellini").query()).
build());
assertEquals(users, issueSYNCOPE1321);
-
+
// SYNCOPE-1416 (check the search for attributes of type different from stringvalue)
PagedResult<UserTO> issueSYNCOPE1416 = userService.search(new AnyQuery.Builder().
realm(SyncopeConstants.ROOT_REALM).
@@ -627,4 +629,18 @@ public class SearchITCase extends AbstractITCase {
assertNotNull(groups);
assertFalse(groups.getResult().isEmpty());
}
+
+ @Test
+ public void issueSYNCOPE1417() {
+ try {
+ userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql(SyncopeClient.getUserSearchConditionBuilder().is("userId").equalTo("*@apache.org").query()).
+ orderBy(SyncopeClient.getOrderByClauseBuilder().asc("surname").desc("firstname").build()).build());
+ if (!ElasticsearchDetector.isElasticSearchEnabled(syncopeService)) {
+ fail();
+ }
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ }
+ }
}