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 11:36:51 UTC
[syncope] branch master 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 master
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/master by this push:
new 6583c74 [SYNCOPE-1417] Raise exception when more than one plain attribute is requested for ordering
6583c74 is described below
commit 6583c746d7418b992c0615d57f1228d37fece5ce
Author: Francesco Chicchiriccò <il...@apache.org>
AuthorDate: Fri Dec 14 12:36:08 2018 +0100
[SYNCOPE-1417] Raise exception when more than one plain attribute is requested for ordering
---
.../core/persistence/jpa/dao/JPAAnySearchDAO.java | 17 +++++
.../core/persistence/jpa/outer/AnySearchTest.java | 78 ++++++++++++++++------
.../org/apache/syncope/fit/core/SearchITCase.java | 21 +++++-
3 files changed, 92 insertions(+), 24 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 0688100..ee20e8d 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
@@ -194,6 +194,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);
}
@@ -324,12 +326,27 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
OrderBySupport obs = new OrderBySupport();
+ Set<String> orderByUniquePlainSchemas = new HashSet<>();
+ Set<String> orderByNonUniquePlainSchemas = new HashSet<>();
orderBy.forEach(clause -> {
OrderBySupport.Item item = new OrderBySupport.Item();
if (anyUtils.getField(clause.getField()) == null) {
PlainSchema schema = schemaDAO.find(clause.getField());
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;
+ }
parseOrderByForPlainSchema(svs, obs, item, clause, schema, clause.getField());
}
} else {
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 3c0d484..6d91b57 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,20 +21,25 @@ package org.apache.syncope.core.persistence.jpa.outer;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.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;
import org.apache.syncope.core.persistence.api.entity.Role;
-import org.apache.syncope.core.persistence.api.entity.group.Group;
import org.apache.syncope.core.persistence.api.entity.user.User;
import org.apache.syncope.core.persistence.jpa.AbstractTest;
import org.junit.jupiter.api.Test;
@@ -57,27 +62,6 @@ public class AnySearchTest extends AbstractTest {
private RoleDAO roleDAO;
@Test
- public void issueSYNCOPE95() {
- for (Group group : groupDAO.findAll(1, 100)) {
- groupDAO.delete(group.getKey());
- }
- entityManager().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 +91,54 @@ public class AnySearchTest extends AbstractTest {
assertEquals(1, users.size());
assertEquals("c9b2dec2-00a7-4855-97c0-d854842b4b24", users.get(0).getKey());
}
+
+ @Test
+ public void issueSYNCOPE95() {
+ groupDAO.findAll(1, 100).forEach(group -> {
+ groupDAO.delete(group.getKey());
+ });
+ entityManager().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 253898f..f11de53 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,10 +23,12 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import javax.ws.rs.core.Response;
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.request.AnyObjectCR;
import org.apache.syncope.common.lib.request.AnyObjectUR;
@@ -43,6 +45,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;
@@ -432,9 +435,9 @@ public class SearchITCase extends AbstractITCase {
assertNotNull(matchingUsers);
assertFalse(matchingUsers.getResult().isEmpty());
- for (UserTO user : matchingUsers.getResult()) {
+ matchingUsers.getResult().forEach(user -> {
assertNotNull(user);
- }
+ });
}
@Test
@@ -541,4 +544,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());
+ }
+ }
}