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 2016/12/21 11:52:11 UTC

syncope git commit: [SYNCOPE-983] Improving search performance by skipping unnecessary checks for mandatory schemas and when the query is performed by admin

Repository: syncope
Updated Branches:
  refs/heads/1_2_X e8f149b32 -> 9bc75f24f


[SYNCOPE-983] Improving search performance by skipping unnecessary checks for mandatory schemas and when the query is performed by admin


Project: http://git-wip-us.apache.org/repos/asf/syncope/repo
Commit: http://git-wip-us.apache.org/repos/asf/syncope/commit/9bc75f24
Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/9bc75f24
Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/9bc75f24

Branch: refs/heads/1_2_X
Commit: 9bc75f24fd80f3c7cab62a86c4a79a27e20460b8
Parents: e8f149b
Author: Francesco Chicchiricc� <il...@apache.org>
Authored: Wed Dec 21 12:52:03 2016 +0100
Committer: Francesco Chicchiricc� <il...@apache.org>
Committed: Wed Dec 21 12:52:03 2016 +0100

----------------------------------------------------------------------
 .../persistence/dao/impl/OrderBySupport.java    |   2 +
 .../persistence/dao/impl/SearchSupport.java     |   4 +-
 .../dao/impl/SubjectSearchDAOImpl.java          | 202 +++++++++++--------
 .../persistence/dao/AttributableSearchTest.java |  33 ++-
 .../syncope/core/rest/RoleTestITCase.java       |   2 -
 .../syncope/core/rest/SearchTestITCase.java     |  88 ++++----
 6 files changed, 194 insertions(+), 137 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/OrderBySupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/OrderBySupport.java b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/OrderBySupport.java
index d7d8734..8690947 100644
--- a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/OrderBySupport.java
+++ b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/OrderBySupport.java
@@ -44,4 +44,6 @@ class OrderBySupport {
 
     protected List<Item> items = new ArrayList<Item>();
 
+    protected boolean nonMandatorySchemas = false;
+
 }

http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SearchSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SearchSupport.java b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SearchSupport.java
index f3ce185..a69d487 100644
--- a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SearchSupport.java
+++ b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SearchSupport.java
@@ -47,7 +47,9 @@ class SearchSupport {
         }
     }
 
-    private final SubjectType type;
+    protected final SubjectType type;
+
+    protected boolean nonMandatorySchemas = false;
 
     public SearchSupport(final SubjectType type) {
         this.type = type;

http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
index 895a6d9..d901bdf 100644
--- a/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
+++ b/core/src/main/java/org/apache/syncope/core/persistence/dao/impl/SubjectSearchDAOImpl.java
@@ -50,6 +50,7 @@ import org.apache.syncope.core.persistence.dao.search.OrderByClause;
 import org.apache.syncope.core.persistence.dao.search.ResourceCond;
 import org.apache.syncope.core.persistence.dao.search.SearchCond;
 import org.apache.syncope.core.util.AttributableUtil;
+import org.apache.syncope.core.util.EntitlementUtil;
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
@@ -71,8 +72,11 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
     @Autowired
     private SchemaDAO schemaDAO;
 
+    @Autowired(required = false)
+    private String adminUser;
+
     private String getAdminRolesFilter(final Set<Long> adminRoles, final SubjectType type) {
-        final StringBuilder adminRolesFilter = new StringBuilder();
+        StringBuilder adminRolesFilter = new StringBuilder("u.subject_id NOT IN (");
 
         if (type == SubjectType.USER) {
             adminRolesFilter.append("SELECT syncopeUser_id AS subject_id FROM Membership WHERE syncopeRole_id NOT IN (");
@@ -98,7 +102,7 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
             adminRolesFilter.append(")");
         }
 
-        return adminRolesFilter.toString();
+        return adminRolesFilter.append(')').toString();
     }
 
     @Override
@@ -107,12 +111,14 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
 
         // 1. get the query string from the search condition
         SearchSupport svs = new SearchSupport(type);
-        StringBuilder queryString = getQuery(searchCondition, parameters, type, svs);
+        StringBuilder queryString = getQuery(searchCondition, parameters, svs);
 
-        // 2. take into account administrative roles
+        // 2. take into account administrative roles (skips checks for admin)
         queryString.insert(0, "SELECT u.subject_id FROM (");
-        queryString.append(") u WHERE subject_id NOT IN (");
-        queryString.append(getAdminRolesFilter(adminRoles, type)).append(')');
+        queryString.append(") u ");
+        if (!adminUser.equals(EntitlementUtil.getAuthenticatedUsername())) {
+            queryString.append("WHERE ").append(getAdminRolesFilter(adminRoles, type));
+        }
 
         // 3. prepare the COUNT query
         queryString.insert(0, "SELECT COUNT(subject_id) FROM (");
@@ -168,14 +174,14 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
     }
 
     @Override
-    public <T extends AbstractSubject> boolean matches(final T subject, final SearchCond searchCondition,
-            final SubjectType type) {
+    public <T extends AbstractSubject> boolean matches(
+            final T subject, final SearchCond searchCondition, final SubjectType type) {
 
         List<Object> parameters = Collections.synchronizedList(new ArrayList<Object>());
 
         // 1. get the query string from the search condition
         SearchSupport svs = new SearchSupport(type);
-        StringBuilder queryString = getQuery(searchCondition, parameters, type, svs);
+        StringBuilder queryString = getQuery(searchCondition, parameters, svs);
 
         boolean matches;
         if (queryString.length() == 0) {
@@ -223,52 +229,55 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
         }
     }
 
-    private StringBuilder buildSelect(final OrderBySupport orderBySupport) {
+    private StringBuilder buildSelect(final OrderBySupport obs) {
         final StringBuilder select = new StringBuilder("SELECT u.subject_id");
 
-        for (OrderBySupport.Item obs : orderBySupport.items) {
-            select.append(',').append(obs.select);
+        for (OrderBySupport.Item item : obs.items) {
+            select.append(',').append(item.select);
         }
         select.append(" FROM ");
 
         return select;
     }
 
-    private StringBuilder buildWhere(final OrderBySupport orderBySupport, final SubjectType type) {
-        SearchSupport svs = new SearchSupport(type);
+    private StringBuilder buildWhere(final SearchSupport svs, final OrderBySupport obs) {
         final StringBuilder where = new StringBuilder(" u");
-        for (SearchSupport.SearchView searchView : orderBySupport.views) {
+        for (SearchSupport.SearchView searchView : obs.views) {
             where.append(',');
             if (searchView.name.equals(svs.attr().name)) {
-                where.append(" (SELECT * FROM ").append(searchView.name).append(" UNION ").
-                        append("SELECT * FROM ").append(svs.nullAttr().name).append(')');
+                where.append(" (SELECT * FROM ").append(searchView.name);
+
+                if (svs.nonMandatorySchemas || obs.nonMandatorySchemas) {
+                    where.append(" UNION SELECT * FROM ").append(svs.nullAttr().name);
+                }
+
+                where.append(')');
             } else {
                 where.append(searchView.name);
             }
             where.append(' ').append(searchView.alias);
         }
         where.append(" WHERE ");
-        for (SearchSupport.SearchView searchView : orderBySupport.views) {
+        for (SearchSupport.SearchView searchView : obs.views) {
             where.append("u.subject_id=").append(searchView.alias).append(".subject_id AND ");
         }
 
-        for (OrderBySupport.Item obs : orderBySupport.items) {
-            if (StringUtils.isNotBlank(obs.where)) {
-                where.append(obs.where).append(" AND ");
+        for (OrderBySupport.Item item : obs.items) {
+            if (StringUtils.isNotBlank(item.where)) {
+                where.append(item.where).append(" AND ");
             }
         }
-        where.append("u.subject_id NOT IN (");
 
         return where;
     }
 
-    private StringBuilder buildOrderBy(final OrderBySupport orderBySupport) {
+    private StringBuilder buildOrderBy(final OrderBySupport obs) {
         final StringBuilder orderBy = new StringBuilder();
 
-        for (OrderBySupport.Item obs : orderBySupport.items) {
-            orderBy.append(obs.orderBy).append(',');
+        for (OrderBySupport.Item item : obs.items) {
+            orderBy.append(item.orderBy).append(',');
         }
-        if (!orderBySupport.items.isEmpty()) {
+        if (!obs.items.isEmpty()) {
             orderBy.insert(0, " ORDER BY ");
             orderBy.deleteCharAt(orderBy.length() - 1);
         }
@@ -276,83 +285,97 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
         return orderBy;
     }
 
-    private OrderBySupport parseOrderBy(final SubjectType type, final SearchSupport svs,
-            final List<OrderByClause> orderByClauses) {
-
-        final AttributableUtil attrUtil = AttributableUtil.getInstance(type.asAttributableType());
+    private OrderBySupport parseOrderBy(final SearchSupport svs, final List<OrderByClause> orderByClauses) {
+        final AttributableUtil attrUtil = AttributableUtil.getInstance(svs.type.asAttributableType());
 
-        OrderBySupport orderBySupport = new OrderBySupport();
+        OrderBySupport obs = new OrderBySupport();
 
         for (OrderByClause clause : orderByClauses) {
-            OrderBySupport.Item obs = new OrderBySupport.Item();
+            OrderBySupport.Item item = new OrderBySupport.Item();
 
             Field subjectField = ReflectionUtils.findField(attrUtil.attributableClass(), clause.getField());
             if (subjectField == null) {
                 AbstractNormalSchema schema = schemaDAO.find(clause.getField(), attrUtil.schemaClass());
                 if (schema != null) {
+                    // keep track of involvement of non-mandatory schemas in the order by clauses
+                    obs.nonMandatorySchemas = !"true".equals(schema.getMandatoryCondition());
+
                     if (schema.isUniqueConstraint()) {
-                        orderBySupport.views.add(svs.uniqueAttr());
+                        obs.views.add(svs.uniqueAttr());
 
-                        obs.select = new StringBuilder().
+                        item.select = new StringBuilder().
                                 append(svs.uniqueAttr().alias).append('.').append(svs.fieldName(schema.getType())).
                                 append(" AS ").append(clause.getField()).toString();
-                        obs.where = new StringBuilder().
+                        item.where = new StringBuilder().
                                 append(svs.uniqueAttr().alias).
                                 append(".schema_name='").append(clause.getField()).append("'").toString();
-                        obs.orderBy = clause.getField() + " " + clause.getDirection().name();
+                        item.orderBy = clause.getField() + " " + clause.getDirection().name();
                     } else {
-                        orderBySupport.views.add(svs.attr());
+                        obs.views.add(svs.attr());
 
-                        obs.select = new StringBuilder().
+                        item.select = new StringBuilder().
                                 append(svs.attr().alias).append('.').append(svs.fieldName(schema.getType())).
                                 append(" AS ").append(clause.getField()).toString();
-                        obs.where = new StringBuilder().
+                        item.where = new StringBuilder().
                                 append(svs.attr().alias).
                                 append(".schema_name='").append(clause.getField()).append("'").toString();
-                        obs.orderBy = clause.getField() + " " + clause.getDirection().name();
+                        item.orderBy = clause.getField() + " " + clause.getDirection().name();
                     }
                 }
             } else {
-                orderBySupport.views.add(svs.field());
+                obs.views.add(svs.field());
 
-                obs.select = svs.field().alias + "." + clause.getField();
-                obs.where = StringUtils.EMPTY;
-                obs.orderBy = svs.field().alias + "." + clause.getField() + " " + clause.getDirection().name();
+                item.select = svs.field().alias + "." + clause.getField();
+                item.where = StringUtils.EMPTY;
+                item.orderBy = svs.field().alias + "." + clause.getField() + " " + clause.getDirection().name();
             }
 
-            if (obs.isEmpty()) {
+            if (item.isEmpty()) {
                 LOG.warn("Cannot build any valid clause from {}", clause);
             } else {
-                orderBySupport.items.add(obs);
+                obs.items.add(item);
             }
         }
 
-        return orderBySupport;
+        return obs;
     }
 
     @SuppressWarnings("unchecked")
-    private <T extends AbstractSubject> List<T> doSearch(final Set<Long> adminRoles,
-            final SearchCond nodeCond, final int page, final int itemsPerPage, final List<OrderByClause> orderBy,
+    private <T extends AbstractSubject> List<T> doSearch(
+            final Set<Long> adminRoles,
+            final SearchCond cond,
+            final int page,
+            final int itemsPerPage,
+            final List<OrderByClause> orderBy,
             final SubjectType type) {
 
         List<Object> parameters = Collections.synchronizedList(new ArrayList<Object>());
 
         // 1. get the query string from the search condition
         SearchSupport svs = new SearchSupport(type);
-        StringBuilder queryString = getQuery(nodeCond, parameters, type, svs);
+        StringBuilder queryString = getQuery(cond, parameters, svs);
 
         // 2. take into account administrative roles and ordering
-        OrderBySupport orderBySupport = parseOrderBy(type, svs, orderBy);
+        OrderBySupport obs = parseOrderBy(svs, orderBy);
         if (queryString.charAt(0) == '(') {
-            queryString.insert(0, buildSelect(orderBySupport));
-            queryString.append(buildWhere(orderBySupport, type));
+            queryString.insert(0, buildSelect(obs));
+            queryString.append(buildWhere(svs, obs));
+        } else {
+            queryString.insert(0, buildSelect(obs).append('('));
+            queryString.append(')').append(buildWhere(svs, obs));
+        }
+        // skips checks for admin
+        if (adminUser.equals(EntitlementUtil.getAuthenticatedUsername())) {
+            String qss = queryString.toString();
+            if (qss.endsWith(" WHERE ")) {
+                queryString.setLength(queryString.length() - 6);
+            } else if (qss.endsWith(" AND ")) {
+                queryString.setLength(queryString.length() - 4);
+            }
         } else {
-            queryString.insert(0, buildSelect(orderBySupport).append('('));
-            queryString.append(')').append(buildWhere(orderBySupport, type));
+            queryString.append(getAdminRolesFilter(adminRoles, type));
         }
-        queryString.
-                append(getAdminRolesFilter(adminRoles, type)).append(')').
-                append(buildOrderBy(orderBySupport));
+        queryString.append(buildOrderBy(obs));
 
         // 3. prepare the search query
         Query query = entityManager.createNativeQuery(queryString.toString());
@@ -394,54 +417,52 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
         return result;
     }
 
-    private StringBuilder getQuery(final SearchCond nodeCond, final List<Object> parameters,
-            final SubjectType type, final SearchSupport svs) {
-
+    private StringBuilder getQuery(final SearchCond cond, final List<Object> parameters, final SearchSupport svs) {
         StringBuilder query = new StringBuilder();
 
-        switch (nodeCond.getType()) {
+        switch (cond.getType()) {
 
             case LEAF:
             case NOT_LEAF:
-                if (nodeCond.getMembershipCond() != null && SubjectType.USER == type) {
-                    query.append(getQuery(nodeCond.getMembershipCond(), nodeCond.getType() == SearchCond.Type.NOT_LEAF,
+                if (cond.getMembershipCond() != null && SubjectType.USER == svs.type) {
+                    query.append(getQuery(cond.getMembershipCond(), cond.getType() == SearchCond.Type.NOT_LEAF,
                             parameters, svs));
                 }
-                if (nodeCond.getResourceCond() != null) {
-                    query.append(getQuery(nodeCond.getResourceCond(),
-                            nodeCond.getType() == SearchCond.Type.NOT_LEAF, parameters, type, svs));
+                if (cond.getResourceCond() != null) {
+                    query.append(getQuery(cond.getResourceCond(),
+                            cond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
                 }
-                if (nodeCond.getEntitlementCond() != null) {
-                    query.append(getQuery(nodeCond.getEntitlementCond(),
-                            nodeCond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
+                if (cond.getEntitlementCond() != null) {
+                    query.append(getQuery(cond.getEntitlementCond(),
+                            cond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
                 }
-                if (nodeCond.getAttributeCond() != null) {
-                    query.append(getQuery(nodeCond.getAttributeCond(),
-                            nodeCond.getType() == SearchCond.Type.NOT_LEAF, parameters, type, svs));
+                if (cond.getAttributeCond() != null) {
+                    query.append(getQuery(cond.getAttributeCond(),
+                            cond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
                 }
-                if (nodeCond.getSubjectCond() != null) {
-                    query.append(getQuery(nodeCond.getSubjectCond(),
-                            nodeCond.getType() == SearchCond.Type.NOT_LEAF, parameters, type, svs));
+                if (cond.getSubjectCond() != null) {
+                    query.append(getQuery(cond.getSubjectCond(),
+                            cond.getType() == SearchCond.Type.NOT_LEAF, parameters, svs));
                 }
                 break;
 
             case AND:
-                String andSubQuery = getQuery(nodeCond.getLeftNodeCond(), parameters, type, svs).toString();
+                String andSubQuery = getQuery(cond.getLeftNodeCond(), parameters, svs).toString();
                 // Add extra parentheses
                 andSubQuery = andSubQuery.replaceFirst("WHERE ", "WHERE (");
                 query.append(andSubQuery).
                         append(" AND subject_id IN ( ").
-                        append(getQuery(nodeCond.getRightNodeCond(), parameters, type, svs)).
+                        append(getQuery(cond.getRightNodeCond(), parameters, svs)).
                         append("))");
                 break;
 
             case OR:
-                String orSubQuery = getQuery(nodeCond.getLeftNodeCond(), parameters, type, svs).toString();
+                String orSubQuery = getQuery(cond.getLeftNodeCond(), parameters, svs).toString();
                 // Add extra parentheses
                 orSubQuery = orSubQuery.replaceFirst("WHERE ", "WHERE (");
                 query.append(orSubQuery).
                         append(" OR subject_id IN ( ").
-                        append(getQuery(nodeCond.getRightNodeCond(), parameters, type, svs)).
+                        append(getQuery(cond.getRightNodeCond(), parameters, svs)).
                         append("))");
                 break;
 
@@ -471,8 +492,8 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
         return query.toString();
     }
 
-    private String getQuery(final ResourceCond cond, final boolean not, final List<Object> parameters,
-            final SubjectType type, final SearchSupport svs) {
+    private String getQuery(
+            final ResourceCond cond, final boolean not, final List<Object> parameters, final SearchSupport svs) {
 
         final StringBuilder query = new StringBuilder("SELECT DISTINCT subject_id FROM ").
                 append(svs.field().name).append(" WHERE ");
@@ -488,7 +509,7 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
                 append(" WHERE resource_name=?").
                 append(setParameter(parameters, cond.getResourceName()));
 
-        if (type == SubjectType.USER) {
+        if (svs.type == SubjectType.USER) {
             query.append(" UNION SELECT DISTINCT subject_id FROM ").
                     append(svs.roleResource().name).
                     append(" WHERE resource_name=?").
@@ -624,10 +645,10 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
         }
     }
 
-    private String getQuery(final AttributeCond cond, final boolean not, final List<Object> parameters,
-            final SubjectType type, final SearchSupport svs) {
+    private String getQuery(
+            final AttributeCond cond, final boolean not, final List<Object> parameters, final SearchSupport svs) {
 
-        final AttributableUtil attrUtil = AttributableUtil.getInstance(type.asAttributableType());
+        final AttributableUtil attrUtil = AttributableUtil.getInstance(svs.type.asAttributableType());
 
         AbstractNormalSchema schema = schemaDAO.find(cond.getSchema(), attrUtil.schemaClass());
         if (schema == null) {
@@ -635,6 +656,9 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
             return EMPTY_ATTR_QUERY;
         }
 
+        // keep track of involvement of non-mandatory schemas in the search condition
+        svs.nonMandatorySchemas = !"true".equals(schema.getMandatoryCondition());
+
         AbstractAttrValue attrValue = attrUtil.newAttrValue();
         try {
             if (cond.getType() != AttributeCond.Type.LIKE
@@ -673,10 +697,10 @@ public class SubjectSearchDAOImpl extends AbstractDAOImpl implements SubjectSear
     }
 
     @SuppressWarnings("rawtypes")
-    private String getQuery(final SubjectCond cond, final boolean not, final List<Object> parameters,
-            final SubjectType type, final SearchSupport svs) {
+    private String getQuery(
+            final SubjectCond cond, final boolean not, final List<Object> parameters, final SearchSupport svs) {
 
-        final AttributableUtil attrUtil = AttributableUtil.getInstance(type.asAttributableType());
+        final AttributableUtil attrUtil = AttributableUtil.getInstance(svs.type.asAttributableType());
 
         int subjFieldIdx = ArrayUtils.indexOf(SUBJECT_FIELDS, StringUtils.substringBeforeLast(cond.getSchema(), "_"));
         Field subjectField = ReflectionUtils.findField(

http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/test/java/org/apache/syncope/core/persistence/dao/AttributableSearchTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/syncope/core/persistence/dao/AttributableSearchTest.java b/core/src/test/java/org/apache/syncope/core/persistence/dao/AttributableSearchTest.java
index 97f37fa..9364733 100644
--- a/core/src/test/java/org/apache/syncope/core/persistence/dao/AttributableSearchTest.java
+++ b/core/src/test/java/org/apache/syncope/core/persistence/dao/AttributableSearchTest.java
@@ -102,8 +102,8 @@ public class AttributableSearchTest {
         loginDateCond.setSchema("loginDate");
         loginDateCond.setExpression("2009-05-26");
 
-        SearchCond subCond = SearchCond.getAndCond(SearchCond.getLeafCond(fullnameLeafCond), SearchCond.getLeafCond(
-                membershipCond));
+        SearchCond subCond = SearchCond.getAndCond(
+                SearchCond.getLeafCond(fullnameLeafCond), SearchCond.getLeafCond(membershipCond));
 
         assertTrue(subCond.isValid());
 
@@ -561,9 +561,8 @@ public class AttributableSearchTest {
         genderCond.setSchema("gender");
         genderCond.setExpression("M");
 
-        SearchCond orCond =
-                SearchCond.getOrCond(SearchCond.getLeafCond(rossiniCond),
-                        SearchCond.getLeafCond(genderCond));
+        SearchCond orCond = SearchCond.getOrCond(
+                SearchCond.getLeafCond(rossiniCond), SearchCond.getLeafCond(genderCond));
 
         AttributeCond belliniCond = new AttributeCond(AttributeCond.Type.EQ);
         belliniCond.setSchema("surname");
@@ -577,4 +576,28 @@ public class AttributableSearchTest {
         assertNotNull(users);
         assertEquals(1, users.size());
     }
+
+    @Test
+    public void issueSYNCOPE983() {
+        AttributeCond fullnameLeafCond = new AttributeCond(AttributeCond.Type.LIKE);
+        fullnameLeafCond.setSchema("surname");
+        fullnameLeafCond.setExpression("%o%");
+
+        List<OrderByClause> orderByClauses = new ArrayList<OrderByClause>();
+        OrderByClause orderByClause = new OrderByClause();
+        orderByClause.setField("surname");
+        orderByClause.setDirection(OrderByClause.Direction.ASC);
+        orderByClauses.add(orderByClause);
+        orderByClause = new OrderByClause();
+        orderByClause.setField("username");
+        orderByClause.setDirection(OrderByClause.Direction.DESC);
+        orderByClauses.add(orderByClause);
+
+        List<SyncopeUser> users = searchDAO.search(
+                EntitlementUtil.getRoleIds(entitlementDAO.findAll()),
+                SearchCond.getLeafCond(fullnameLeafCond),
+                orderByClauses,
+                SubjectType.USER);
+        assertFalse(users.isEmpty());
+    }
 }

http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/test/java/org/apache/syncope/core/rest/RoleTestITCase.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/syncope/core/rest/RoleTestITCase.java b/core/src/test/java/org/apache/syncope/core/rest/RoleTestITCase.java
index 51d4330..58f6102 100644
--- a/core/src/test/java/org/apache/syncope/core/rest/RoleTestITCase.java
+++ b/core/src/test/java/org/apache/syncope/core/rest/RoleTestITCase.java
@@ -70,7 +70,6 @@ import org.apache.syncope.common.wrap.ResourceName;
 import org.apache.syncope.core.quartz.AbstractTaskJob;
 import org.identityconnectors.framework.common.objects.Name;
 import org.junit.FixMethodOrder;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runners.MethodSorters;
 
@@ -117,7 +116,6 @@ public class RoleTestITCase extends AbstractTest {
     }
 
     @Test
-    @Ignore
     public void create() {
         RoleTO roleTO = buildRoleTO("lastRole");
         roleTO.getRVirAttrTemplates().add("rvirtualdata");

http://git-wip-us.apache.org/repos/asf/syncope/blob/9bc75f24/core/src/test/java/org/apache/syncope/core/rest/SearchTestITCase.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/syncope/core/rest/SearchTestITCase.java b/core/src/test/java/org/apache/syncope/core/rest/SearchTestITCase.java
index 8561ea0..e1626b4 100644
--- a/core/src/test/java/org/apache/syncope/core/rest/SearchTestITCase.java
+++ b/core/src/test/java/org/apache/syncope/core/rest/SearchTestITCase.java
@@ -20,6 +20,7 @@ package org.apache.syncope.core.rest;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -41,24 +42,24 @@ public class SearchTestITCase extends AbstractTest {
     @Test
     public void searchUser() {
         // LIKE
-        PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().
                         is("fullname").equalTo("*o*").and("fullname").equalTo("*i*").query());
-        assertNotNull(matchedUsers);
-        assertFalse(matchedUsers.getResult().isEmpty());
+        assertNotNull(users);
+        assertFalse(users.getResult().isEmpty());
 
-        for (UserTO user : matchedUsers.getResult()) {
+        for (UserTO user : users.getResult()) {
             assertNotNull(user);
         }
 
         // ISNULL
-        matchedUsers = userService.search(
+        users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().isNull("loginDate").query());
-        assertNotNull(matchedUsers);
-        assertFalse(matchedUsers.getResult().isEmpty());
+        assertNotNull(users);
+        assertFalse(users.getResult().isEmpty());
 
-        Set<Long> userIds = new HashSet<Long>(matchedUsers.getResult().size());
-        for (UserTO user : matchedUsers.getResult()) {
+        Set<Long> userIds = new HashSet<Long>(users.getResult().size());
+        for (UserTO user : users.getResult()) {
             userIds.add(user.getId());
         }
         assertTrue(userIds.contains(2L));
@@ -83,7 +84,7 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchByUsernameAndId() {
-        final PagedResult<UserTO> matchingUsers = userService.search(
+        PagedResult<UserTO> matchingUsers = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().
                         is("username").equalTo("rossini").and("id").lessThan(2).query());
 
@@ -95,7 +96,7 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchByRolenameAndId() {
-        final PagedResult<RoleTO> matchingRoles = roleService.search(
+        PagedResult<RoleTO> matchingRoles = roleService.search(
                 SyncopeClient.getRoleSearchConditionBuilder().is("name").equalTo("root").and("id").lessThan(2).query());
 
         assertNotNull(matchingRoles);
@@ -106,13 +107,13 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchUserByResourceName() {
-        PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().hasResources(RESOURCE_NAME_MAPPINGS2).query());
-        assertNotNull(matchedUsers);
-        assertFalse(matchedUsers.getResult().isEmpty());
+        assertNotNull(users);
+        assertFalse(users.getResult().isEmpty());
 
-        Set<Long> userIds = new HashSet<Long>(matchedUsers.getResult().size());
-        for (UserTO user : matchedUsers.getResult()) {
+        Set<Long> userIds = new HashSet<Long>(users.getResult().size());
+        for (UserTO user : users.getResult()) {
             userIds.add(user.getId());
         }
 
@@ -123,24 +124,24 @@ public class SearchTestITCase extends AbstractTest {
     @Test
     public void paginatedSearch() {
         // LIKE
-        PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().
                         is("fullname").equalTo("*o*").and("fullname").equalTo("*i*").query(), 1, 2);
-        assertNotNull(matchedUsers);
+        assertNotNull(users);
 
-        assertFalse(matchedUsers.getResult().isEmpty());
-        for (UserTO user : matchedUsers.getResult()) {
+        assertFalse(users.getResult().isEmpty());
+        for (UserTO user : users.getResult()) {
             assertNotNull(user);
         }
 
         // ISNULL
-        matchedUsers = userService.search(
+        users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().isNull("loginDate").query(), 1, 2);
 
-        assertNotNull(matchedUsers);
-        assertFalse(matchedUsers.getResult().isEmpty());
-        Set<Long> userIds = new HashSet<Long>(matchedUsers.getResult().size());
-        for (UserTO user : matchedUsers.getResult()) {
+        assertNotNull(users);
+        assertFalse(users.getResult().isEmpty());
+        Set<Long> userIds = new HashSet<Long>(users.getResult().size());
+        for (UserTO user : users.getResult()) {
             userIds.add(user.getId());
         }
         assertEquals(2, userIds.size());
@@ -148,7 +149,7 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchByBooleanSubjectCond() {
-        final PagedResult<RoleTO> matchingRoles = roleService.search(
+        PagedResult<RoleTO> matchingRoles = roleService.search(
                 SyncopeClient.getRoleSearchConditionBuilder().is("inheritAttrs").equalTo("true").query());
         assertNotNull(matchingRoles);
         assertFalse(matchingRoles.getResult().isEmpty());
@@ -169,7 +170,7 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchByEntitlement() {
-        final PagedResult<RoleTO> matchingRoles = roleService.search(
+        PagedResult<RoleTO> matchingRoles = roleService.search(
                 SyncopeClient.getRoleSearchConditionBuilder().hasEntitlements("USER_LIST", "USER_READ").query());
         assertNotNull(matchingRoles);
         assertFalse(matchingRoles.getResult().isEmpty());
@@ -177,7 +178,7 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void searchByRelationshipSubjectCond() {
-        final PagedResult<RoleTO> matchingRoles = roleService.search(SyncopeClient.getRoleSearchConditionBuilder().
+        PagedResult<RoleTO> matchingRoles = roleService.search(SyncopeClient.getRoleSearchConditionBuilder().
                 isNotNull("passwordPolicy").and("userOwner").equalTo(5).query());
 
         assertNotNull(matchingRoles);
@@ -188,31 +189,31 @@ public class SearchTestITCase extends AbstractTest {
 
     @Test
     public void nested() {
-        PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 "((fullname==*o*,fullname==*i*);$resources!=ws-target-resource-1)", 1, 2);
-        assertNotNull(matchedUsers);
+        assertNotNull(users);
 
-        assertFalse(matchedUsers.getResult().isEmpty());
-        for (UserTO user : matchedUsers.getResult()) {
+        assertFalse(users.getResult().isEmpty());
+        for (UserTO user : users.getResult()) {
             assertNotNull(user);
         }
     }
 
     @Test
     public void orderBy() {
-        final PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().is("userId").equalTo("*@apache.org").query(),
                 SyncopeClient.getOrderByClauseBuilder().asc("status").desc("firstname").build());
 
-        assertFalse(matchedUsers.getResult().isEmpty());
-        for (UserTO user : matchedUsers.getResult()) {
+        assertFalse(users.getResult().isEmpty());
+        for (UserTO user : users.getResult()) {
             assertNotNull(user);
         }
     }
 
     @Test
     public void issueSYNCOPE712() {
-        final PagedResult<RoleTO> matchingRoles = roleService.search(
+        PagedResult<RoleTO> matchingRoles = roleService.search(
                 SyncopeClient.getRoleSearchConditionBuilder().is("parent").equalTo(1L).query());
 
         assertNotNull(matchingRoles);
@@ -226,17 +227,16 @@ public class SearchTestITCase extends AbstractTest {
 
         assertFalse(usersWithType.isEmpty());
 
-        final PagedResult<UserTO> matchedUsers = userService.search(
+        PagedResult<UserTO> users = userService.search(
                 SyncopeClient.getUserSearchConditionBuilder().is("username").notNullValue().query(),
                 SyncopeClient.getOrderByClauseBuilder().asc("type").build());
 
-        assertTrue(matchedUsers.getResult().size() > usersWithType.size());
+        assertTrue(users.getResult().size() > usersWithType.size());
     }
 
     @Test
     public void issueSYNCOPE929() {
-        PagedResult<UserTO> matchingUsers = userService.search(
-                "(surname==Rossini,gender==M);surname==Bellini");
+        PagedResult<UserTO> matchingUsers = userService.search("(surname==Rossini,gender==M);surname==Bellini");
 
         assertNotNull(matchingUsers);
 
@@ -245,4 +245,12 @@ public class SearchTestITCase extends AbstractTest {
             assertTrue(user.getUsername().startsWith("bellini"));
         }
     }
+
+    @Test
+    public void issueSYNCOPE983() {
+        PagedResult<UserTO> users = userService.search(
+                SyncopeClient.getUserSearchConditionBuilder().is("surname").equalTo("*o*").query(),
+                SyncopeClient.getOrderByClauseBuilder().asc("surname").desc("username").build());
+        assertNotEquals(0, users.getTotalCount());
+    }
 }