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:52 UTC

[syncope] branch 2_1_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_1_X
in repository https://gitbox.apache.org/repos/asf/syncope.git


The following commit(s) were added to refs/heads/2_1_X by this push:
     new 3e6f71c  [SYNCOPE-1417] Raise exception when more than one plain attribute is requested for ordering
3e6f71c is described below

commit 3e6f71c1e74ebf0fcfa02913b6268fb0539d686c
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 076fb34..847f2aa 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.patch.AnyObjectPatch;
 import org.apache.syncope.common.lib.patch.AttrPatch;
@@ -40,6 +42,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;
@@ -429,9 +432,9 @@ public class SearchITCase extends AbstractITCase {
         assertNotNull(matchingUsers);
 
         assertFalse(matchingUsers.getResult().isEmpty());
-        for (UserTO user : matchingUsers.getResult()) {
+        matchingUsers.getResult().forEach(user -> {
             assertNotNull(user);
-        }
+        });
     }
 
     @Test
@@ -542,4 +545,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());
+        }
+    }
 }