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 2022/02/05 12:18:34 UTC
[syncope] 02/02: [SYNCOPE-1663] Now returning appropriate error
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
commit c2f2cac38400503946051722b332866759133a52
Author: Francesco Chicchiriccò <il...@apache.org>
AuthorDate: Sat Feb 5 13:16:01 2022 +0100
[SYNCOPE-1663] Now returning appropriate error
---
.../common/lib/types/ClientExceptionType.java | 2 +-
.../persistence/api/search/FilterConverter.java | 2 +-
.../api/search/SearchCondConverter.java | 2 +-
.../api/search/FilterConverterTest.java | 14 +++---
.../api/search/SearchCondConverterTest.java | 1 -
.../persistence/jpa/dao/MyJPAJSONAnySearchDAO.java | 7 +--
.../persistence/jpa/dao/PGJPAJSONAnySearchDAO.java | 3 +-
.../persistence/jpa/dao/AbstractAnySearchDAO.java | 24 +++------
.../core/persistence/jpa/dao/JPAAnySearchDAO.java | 53 ++++----------------
.../core/persistence/jpa/outer/AnySearchTest.java | 2 +-
.../java/data/DynRealmDataBinderImpl.java | 2 +-
.../java/data/GroupDataBinderImpl.java | 2 +-
.../provisioning/java/data/RoleDataBinderImpl.java | 2 +-
.../core/rest/cxf/service/AbstractAnyService.java | 28 +++++++----
.../core/rest/cxf/service/AbstractServiceImpl.java | 2 +-
.../rest/cxf/service/AnyObjectServiceImpl.java | 2 +-
.../cxf/service/ReconciliationServiceImpl.java | 2 +-
.../core/rest/cxf/service/ResourceServiceImpl.java | 2 +-
.../jpa/dao/ElasticsearchAnySearchDAO.java | 57 ++++------------------
.../org/apache/syncope/fit/core/GroupITCase.java | 6 ++-
.../org/apache/syncope/fit/core/SearchITCase.java | 42 +++++++++++-----
21 files changed, 100 insertions(+), 157 deletions(-)
diff --git a/common/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java b/common/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
index 8a8f744..7796897 100644
--- a/common/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
+++ b/common/lib/src/main/java/org/apache/syncope/common/lib/types/ClientExceptionType.java
@@ -51,7 +51,7 @@ public enum ClientExceptionType {
InvalidAnyObject(Response.Status.BAD_REQUEST),
InvalidGroup(Response.Status.BAD_REQUEST),
InvalidSchemaDefinition(Response.Status.BAD_REQUEST),
- InvalidSearchExpression(Response.Status.BAD_REQUEST),
+ InvalidSearchParameters(Response.Status.BAD_REQUEST),
InvalidPageOrSize(Response.Status.BAD_REQUEST),
InvalidPropagationTaskExecReport(Response.Status.BAD_REQUEST),
InvalidPlainSchema(Response.Status.BAD_REQUEST),
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
index 124a478..d12589d 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/FilterConverter.java
@@ -51,7 +51,7 @@ public final class FilterConverter {
return visitor.getQuery();
} catch (Exception e) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
index 8a0198f..cf0ce63 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SearchCondConverter.java
@@ -55,7 +55,7 @@ public final class SearchCondConverter {
return visitor.getQuery();
} catch (Exception e) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
index d314db0..84f73d8 100644
--- a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
+++ b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/FilterConverterTest.java
@@ -117,7 +117,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -130,7 +130,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -143,7 +143,7 @@ public class FilterConverterTest {
FilterConverter.convert(fiql);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -178,7 +178,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.DYNREALMS + "==realm");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -188,7 +188,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.DYNREALMS + "!=realm");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -198,7 +198,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.RESOURCES + "==resource");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -208,7 +208,7 @@ public class FilterConverterTest {
FilterConverter.convert(SpecialAttr.RESOURCES + "!=resource");
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
diff --git a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
index eafb313..111aa67 100644
--- a/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
+++ b/core/persistence-api/src/test/java/org/apache/syncope/core/persistence/api/search/SearchCondConverterTest.java
@@ -329,5 +329,4 @@ public class SearchCondConverterTest {
assertEquals(SearchCond.getLeaf(cond), SearchCondConverter.convert(VISITOR, fiql));
}
-
}
diff --git a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
index 5eddaf3..f570c24 100644
--- a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
+++ b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/MyJPAJSONAnySearchDAO.java
@@ -228,12 +228,7 @@ public class MyJPAJSONAnySearchDAO extends JPAAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, svs.anyTypeKind);
// normalize NULL / NOT NULL checks
if (not) {
diff --git a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
index 3ecc9c6..c22771c 100644
--- a/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
+++ b/core/persistence-jpa-json/src/main/java/org/apache/syncope/core/persistence/jpa/dao/PGJPAJSONAnySearchDAO.java
@@ -592,8 +592,7 @@ public class PGJPAJSONAnySearchDAO extends JPAAnySearchDAO {
Realm realm = realmDAO.findByFullPath(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm full path: {}", cond.getExpression());
- return EMPTY_QUERY;
+ throw new IllegalArgumentException("Invalid Realm full path: " + cond.getExpression());
}
cond.setExpression(realm.getKey());
}
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
index 14592e3..1ee4126 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractAnySearchDAO.java
@@ -178,8 +178,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
PlainSchema schema = schemaDAO.find(cond.getSchema());
if (schema == null) {
- LOG.warn("Ignoring invalid schema '{}'", cond.getSchema());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Invalid schema " + cond.getSchema());
}
PlainAttrValue attrValue = schema.isUniqueConstraint()
@@ -194,8 +193,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
((JPAPlainSchema) schema).validator().validate(cond.getExpression(), attrValue);
}
} catch (ValidationException e) {
- LOG.error("Could not validate expression '" + cond.getExpression() + "'", e);
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not validate expression " + cond.getExpression());
}
return Pair.of(schema, attrValue);
@@ -210,8 +208,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
Field anyField = anyUtils.getField(computed.getSchema());
if (anyField == null) {
- LOG.warn("Ignoring invalid field '{}'", computed.getSchema());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Invalid schema " + computed.getSchema());
}
// Keeps track of difference between entity's getKey() and JPA @Id fields
if ("key".equals(computed.getSchema())) {
@@ -257,8 +254,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
try {
((JPAPlainSchema) schema).validator().validate(computed.getExpression(), attrValue);
} catch (ValidationException e) {
- LOG.error("Could not validate expression '" + computed.getExpression() + "'", e);
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not validate expression " + computed.getExpression());
}
}
@@ -275,8 +271,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
map(Collections::singletonList).orElseGet(() -> Collections.emptyList())
: groupDAO.findKeysByNamePattern(cond.getGroup());
if (matching.isEmpty()) {
- LOG.error("Could not find group(s) for '{}'", cond.getGroup());
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find group(s) for " + cond.getGroup());
}
return matching;
@@ -291,8 +286,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
rightAnyObjectKey = anyObject == null ? null : anyObject.getKey();
}
if (rightAnyObjectKey == null) {
- LOG.error("Could not find any object for '" + cond.getAnyObject() + "'");
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find any object for " + cond.getAnyObject());
}
return rightAnyObjectKey;
@@ -301,8 +295,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
protected Realm check(final AssignableCond cond) {
Realm realm = realmDAO.findByFullPath(cond.getRealmFullPath());
if (realm == null) {
- LOG.error("Could not find realm for '" + cond.getRealmFullPath() + "'");
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find realm for " + cond.getRealmFullPath());
}
return realm;
@@ -320,8 +313,7 @@ public abstract class AbstractAnySearchDAO extends AbstractDAO<Any<?>> implement
memberKey = member == null ? null : member.getKey();
}
if (memberKey == null) {
- LOG.error("Could not find user or any object for '" + cond.getMember() + "'");
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Could not find user or any object for " + cond.getMember());
}
return memberKey;
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 65bf645..042a688 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
@@ -65,8 +65,6 @@ import org.apache.syncope.core.persistence.api.entity.Realm;
*/
public class JPAAnySearchDAO extends AbstractAnySearchDAO {
- protected static final String EMPTY_QUERY = "SELECT any_id FROM user_search WHERE 1=2";
-
protected String buildAdminRealmsFilter(
final Set<String> realmKeys,
final SearchSupport svs,
@@ -228,9 +226,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
if (parameters.get(i) instanceof Date) {
query.setParameter(i + 1, (Date) parameters.get(i), TemporalType.TIMESTAMP);
} else if (parameters.get(i) instanceof Boolean) {
- query.setParameter(i + 1, ((Boolean) parameters.get(i))
- ? 1
- : 0);
+ query.setParameter(i + 1, ((Boolean) parameters.get(i)) ? 1 : 0);
} else {
query.setParameter(i + 1, parameters.get(i));
}
@@ -443,7 +439,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
}
if (orderByUniquePlainSchemas.size() > 1 || orderByNonUniquePlainSchemas.size() > 1) {
SyncopeClientException invalidSearch =
- SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
invalidSearch.getElements().add("Order by more than one attribute is not allowed; "
+ "remove one from " + (orderByUniquePlainSchemas.size() > 1
? orderByUniquePlainSchemas : orderByNonUniquePlainSchemas));
@@ -643,12 +639,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- String rightAnyObjectKey;
- try {
- rightAnyObjectKey = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ String rightAnyObjectKey = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
@@ -673,12 +664,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- List<String> groupKeys;
- try {
- groupKeys = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ List<String> groupKeys = check(cond);
String where = groupKeys.stream().
map(key -> "group_id=?" + setParameter(parameters, key)).
@@ -840,12 +826,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Realm realm;
- try {
- realm = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Realm realm = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE (");
@@ -871,12 +852,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- String memberKey;
- try {
- memberKey = check(cond);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ String memberKey = check(cond);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
@@ -1044,12 +1020,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
final List<Object> parameters,
final SearchSupport svs) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, svs.anyTypeKind);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ");
switch (cond.getType()) {
@@ -1100,18 +1071,12 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
Realm realm = realmDAO.findByFullPath(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm full path: {}", cond.getExpression());
- return EMPTY_QUERY;
+ throw new IllegalArgumentException("Invalid Realm full path: " + cond.getExpression());
}
cond.setExpression(realm.getKey());
}
- Triple<PlainSchema, PlainAttrValue, AnyCond> checked;
- try {
- checked = check(cond, svs.anyTypeKind);
- } catch (IllegalArgumentException e) {
- return EMPTY_QUERY;
- }
+ Triple<PlainSchema, PlainAttrValue, AnyCond> checked = check(cond, svs.anyTypeKind);
StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
append(svs.field().name).append(" WHERE ");
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 74a654e..0f92bca 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
@@ -188,7 +188,7 @@ public class AnySearchTest extends AbstractTest {
searchDAO.search(searchCondition, orderByClauses, AnyTypeKind.USER);
fail();
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
index 2f5cf0e..c5cc9d8 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/DynRealmDataBinderImpl.java
@@ -57,7 +57,7 @@ public class DynRealmDataBinderImpl implements DynRealmDataBinder {
private void setDynMembership(final DynRealm dynRealm, final AnyType anyType, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
index 1771735..7cf5ba0 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/GroupDataBinderImpl.java
@@ -70,7 +70,7 @@ public class GroupDataBinderImpl extends AbstractAnyDataBinder implements GroupD
private void setDynMembership(final Group group, final AnyType anyType, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
index bfe3059..544a902 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/RoleDataBinderImpl.java
@@ -68,7 +68,7 @@ public class RoleDataBinderImpl implements RoleDataBinder {
private void setDynMembership(final Role role, final String dynMembershipFIQL) {
SearchCond dynMembershipCond = SearchCondConverter.convert(searchCondVisitor, dynMembershipFIQL);
if (!dynMembershipCond.isValid()) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(dynMembershipFIQL);
throw sce;
}
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
index f857cc1..d6ec1d8 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractAnyService.java
@@ -27,7 +27,9 @@ import java.util.stream.Collectors;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.lang3.tuple.Pair;
+import org.apache.syncope.common.lib.SyncopeClientException;
import org.apache.syncope.common.lib.SyncopeConstants;
import org.apache.syncope.common.lib.patch.AnyPatch;
import org.apache.syncope.common.lib.patch.AssociationPatch;
@@ -38,6 +40,7 @@ import org.apache.syncope.common.lib.to.AnyTO;
import org.apache.syncope.common.lib.to.AttrTO;
import org.apache.syncope.common.lib.to.PagedResult;
import org.apache.syncope.common.lib.to.ProvisioningResult;
+import org.apache.syncope.common.lib.types.ClientExceptionType;
import org.apache.syncope.common.lib.types.PatchOperation;
import org.apache.syncope.common.lib.types.ResourceAssociationAction;
import org.apache.syncope.common.lib.types.ResourceDeassociationAction;
@@ -130,15 +133,22 @@ public abstract class AbstractAnyService<TO extends AnyTO, P extends AnyPatch>
? null
: getSearchCond(anyQuery.getFiql(), realm);
- Pair<Integer, List<TO>> result = getAnyLogic().search(
- searchCond,
- anyQuery.getPage(),
- anyQuery.getSize(),
- getOrderByClauses(anyQuery.getOrderBy()),
- isAssignableCond ? SyncopeConstants.ROOT_REALM : realm,
- anyQuery.getDetails());
-
- return buildPagedResult(result.getRight(), anyQuery.getPage(), anyQuery.getSize(), result.getLeft());
+ try {
+ Pair<Integer, List<TO>> result = getAnyLogic().search(
+ searchCond,
+ anyQuery.getPage(),
+ anyQuery.getSize(),
+ getOrderByClauses(anyQuery.getOrderBy()),
+ isAssignableCond ? SyncopeConstants.ROOT_REALM : realm,
+ anyQuery.getDetails());
+
+ return buildPagedResult(result.getRight(), anyQuery.getPage(), anyQuery.getSize(), result.getLeft());
+ } catch (IllegalArgumentException e) {
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
+ sce.getElements().add(anyQuery.getFiql());
+ sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
+ throw sce;
+ }
}
protected Date findLastChange(final String key) {
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractServiceImpl.java b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractServiceImpl.java
index c1a7c7f..f93a68d 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractServiceImpl.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AbstractServiceImpl.java
@@ -174,7 +174,7 @@ abstract class AbstractServiceImpl implements JAXRSService {
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", fiql, e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(fiql);
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
index fd2ead6..07e79d1 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/AnyObjectServiceImpl.java
@@ -91,7 +91,7 @@ public class AnyObjectServiceImpl extends AbstractAnyService<AnyObjectTO, AnyObj
if (StringUtils.isBlank(anyQuery.getFiql())
|| -1 == anyQuery.getFiql().indexOf(SpecialAttr.TYPE.toString())) {
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(SpecialAttr.TYPE.toString() + " is required in the FIQL string");
throw sce;
}
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
index 87a1a73..51b8ce1 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ReconciliationServiceImpl.java
@@ -85,7 +85,7 @@ public class ReconciliationServiceImpl extends AbstractServiceImpl implements Re
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", reconQuery.getFiql(), e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(reconQuery.getFiql());
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
index 9b80531..c68615d 100644
--- a/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
+++ b/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/service/ResourceServiceImpl.java
@@ -125,7 +125,7 @@ public class ResourceServiceImpl extends AbstractServiceImpl implements Resource
} catch (Exception e) {
LOG.error("Invalid FIQL expression: {}", query.getFiql(), e);
- SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+ SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.InvalidSearchParameters);
sce.getElements().add(query.getFiql());
sce.getElements().add(ExceptionUtils.getRootCauseMessage(e));
throw sce;
diff --git a/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java b/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
index 16eb4d4..c11bb4d 100644
--- a/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
+++ b/ext/elasticsearch/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/ElasticsearchAnySearchDAO.java
@@ -66,8 +66,6 @@ import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.client.core.CountRequest;
import org.elasticsearch.index.query.DisMaxQueryBuilder;
-import org.elasticsearch.index.query.MatchAllQueryBuilder;
-import org.elasticsearch.index.query.MatchNoneQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.SearchHit;
@@ -82,10 +80,6 @@ import org.springframework.beans.factory.annotation.Autowired;
*/
public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
- protected static final QueryBuilder MATCH_NONE_QUERY_BUILDER = new MatchNoneQueryBuilder();
-
- protected static final QueryBuilder MATCH_ALL_QUERY_BUILDER = new MatchAllQueryBuilder();
-
protected static final char[] ELASTICSEARCH_REGEX_CHARS = new char[] {
'.', '?', '+', '*', '|', '{', '}', '[', ']', '(', ')', '"', '\\', '&' };
@@ -334,7 +328,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
if (builder == null) {
- builder = MATCH_NONE_QUERY_BUILDER;
+ throw new IllegalArgumentException("Cannot construct QueryBuilder");
}
if (cond.getType() == SearchCond.Type.NOT_LEAF) {
@@ -369,23 +363,13 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected QueryBuilder getQueryBuilder(final RelationshipCond cond) {
- String rightAnyObjectKey;
- try {
- rightAnyObjectKey = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ String rightAnyObjectKey = check(cond);
return QueryBuilders.termQuery("relationships", rightAnyObjectKey);
}
protected QueryBuilder getQueryBuilder(final MembershipCond cond) {
- List<String> groupKeys;
- try {
- groupKeys = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ List<String> groupKeys = check(cond);
if (groupKeys.size() == 1) {
return QueryBuilders.termQuery("memberships", groupKeys.get(0));
@@ -397,12 +381,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected QueryBuilder getQueryBuilder(final AssignableCond cond) {
- Realm realm;
- try {
- realm = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ Realm realm = check(cond);
DisMaxQueryBuilder builder = QueryBuilders.disMaxQuery();
if (cond.isFromGroup()) {
@@ -431,12 +410,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected QueryBuilder getQueryBuilder(final MemberCond cond) {
- String memberKey;
- try {
- memberKey = check(cond);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ String memberKey = check(cond);
return QueryBuilders.termQuery("members", memberKey);
}
@@ -454,7 +428,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
? attrValue.getDateValue().getTime()
: attrValue.getValue();
- QueryBuilder builder = MATCH_NONE_QUERY_BUILDER;
+ QueryBuilder builder = null;
switch (cond.getType()) {
case ISNOTNULL:
@@ -517,12 +491,7 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
}
protected QueryBuilder getQueryBuilder(final AttrCond cond, final AnyTypeKind kind) {
- Pair<PlainSchema, PlainAttrValue> checked;
- try {
- checked = check(cond, kind);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ Pair<PlainSchema, PlainAttrValue> checked = check(cond, kind);
return fillAttrQuery(checked.getLeft(), checked.getRight(), cond);
}
@@ -533,23 +502,17 @@ public class ElasticsearchAnySearchDAO extends AbstractAnySearchDAO {
Realm realm = realmDAO.find(cond.getExpression());
if (realm == null) {
- LOG.warn("Invalid Realm key: {}", cond.getExpression());
- return MATCH_NONE_QUERY_BUILDER;
+ throw new IllegalArgumentException("Invalid Realm key: " + cond.getExpression());
}
cond.setExpression(realm.getFullPath());
}
- Triple<PlainSchema, PlainAttrValue, AnyCond> checked;
- try {
- checked = check(cond, kind);
- } catch (IllegalArgumentException e) {
- return MATCH_NONE_QUERY_BUILDER;
- }
+ Triple<PlainSchema, PlainAttrValue, AnyCond> checked = check(cond, kind);
return fillAttrQuery(checked.getLeft(), checked.getMiddle(), checked.getRight());
}
protected QueryBuilder getQueryBuilderForCustomConds(final SearchCond cond, final AnyTypeKind kind) {
- return MATCH_ALL_QUERY_BUILDER;
+ return null;
}
}
diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
index 6a7e68d..ada06db 100644
--- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
+++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/GroupITCase.java
@@ -248,10 +248,12 @@ public class GroupITCase extends AbstractITCase {
@Test
public void patch() {
GroupTO original = getBasicSampleTO("patch");
- original.setUDynMembershipCond("(($groups==3;$resources!=ws-target-resource-1);aLong==1)");
+ original.setUDynMembershipCond(
+ "(($groups==ebf97068-aa4b-4a85-9f01-680e8c4cf227;$resources!=ws-target-resource-1);aLong==1)");
original.getADynMembershipConds().put(
PRINTER,
- "(($groups==7;cool==ss);$resources==ws-target-resource-2);$type==PRINTER");
+ "(($groups==ece66293-8f31-4a84-8e8d-23da36e70846;cool==ss);$resources==ws-target-resource-2);"
+ + "$type==PRINTER");
GroupTO created = createGroup(original).getEntity();
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 b22bce3..030b3d0 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
@@ -422,12 +422,15 @@ public class SearchITCase extends AbstractITCase {
userTO = createUser(userTO).getEntity();
assertNotNull(userTO.getSecurityQuestion());
- PagedResult<UserTO> matchingUsers = userService.search(
- new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
- fiql(SyncopeClient.getUserSearchConditionBuilder().
- is("securityAnswer").equalTo(securityAnswer).query()).build());
- assertNotNull(matchingUsers);
- assertTrue(matchingUsers.getResult().isEmpty());
+ try {
+ userService.search(
+ new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql(SyncopeClient.getUserSearchConditionBuilder().
+ is("securityAnswer").equalTo(securityAnswer).query()).build());
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
+ }
}
@Test
@@ -761,7 +764,7 @@ public class SearchITCase extends AbstractITCase {
fail();
}
} catch (SyncopeClientException e) {
- assertEquals(ClientExceptionType.InvalidSearchExpression, e.getType());
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
}
}
@@ -787,11 +790,26 @@ public class SearchITCase extends AbstractITCase {
@Test
public void issueSYNCOPE1648() {
- PagedResult<UserTO> matching = userService.search(
- new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
- fiql(SyncopeClient.getUserSearchConditionBuilder().
- is("username").notEqualTo("verdi").query()).
- build());
+ PagedResult<UserTO> matching = userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql(SyncopeClient.getUserSearchConditionBuilder().
+ is("username").notEqualTo("verdi").query()).build());
assertTrue(matching.getResult().stream().noneMatch(user -> "verdi".equals(user.getUsername())));
}
+
+ @Test
+ public void issueSYNCOPE1663() {
+ try {
+ userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql("lastChangeDate=ge=2022-01-25T17:00:06Z").build());
+ fail();
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidSearchParameters, e.getType());
+ assertTrue(e.getElements().stream().
+ anyMatch(elem -> elem.contains("Could not validate expression 2022-01-25T17:00:06Z")));
+ }
+
+ PagedResult<UserTO> matching = userService.search(new AnyQuery.Builder().realm(SyncopeConstants.ROOT_REALM).
+ fiql("lastChangeDate=ge=2022-01-25T17:00:06+0000").build());
+ assertNotNull(matching);
+ }
}