You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by fm...@apache.org on 2018/12/20 23:20:43 UTC

[syncope] 01/03: [SYNCOPE-1419] provides the correct behavior in case of multivalue fields for json implementation as well

This is an automated email from the ASF dual-hosted git repository.

fmartelli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/syncope.git

commit b8114cf1cfadd6a9730adca4301663b4dd86db19
Author: fmartelli <fa...@gmail.com>
AuthorDate: Thu Dec 20 23:52:09 2018 +0100

    [SYNCOPE-1419] provides the correct behavior in case of multivalue fields for json implementation as well
---
 .../persistence/jpa/dao/PGJPAJSONAnySearchDAO.java | 177 +++++++++++----------
 .../core/persistence/jpa/dao/JPAAnySearchDAO.java  |  24 +--
 2 files changed, 111 insertions(+), 90 deletions(-)

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 1c465e9..884a40b 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
@@ -28,6 +28,7 @@ import org.apache.commons.lang3.tuple.Pair;
 import org.apache.syncope.common.lib.SyncopeConstants;
 import org.apache.syncope.common.lib.types.AnyTypeKind;
 import org.apache.syncope.common.lib.types.AttrSchemaType;
+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.entity.AnyUtils;
@@ -153,94 +154,106 @@ public class PGJPAJSONAnySearchDAO extends JPAAnySearchDAO {
             final PlainSchema schema,
             final AttributeCond cond,
             final boolean not,
-            final List<Object> parameters) {
+            final List<Object> parameters,
+            final SearchSupport svs) {
 
-        String key = key(schema.getType());
-        boolean lower = (schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum)
-                && (cond.getType() == AttributeCond.Type.IEQ || cond.getType() == AttributeCond.Type.ILIKE);
+        // This first branch is required for handling with not conditions given on multivalue fields (SYNCOPE-1419)
+        if (not && !(cond instanceof AnyCond)
+                && schema.isMultivalue()
+                && cond.getType() != AttributeCond.Type.ISNULL
+                && cond.getType() != AttributeCond.Type.ISNOTNULL) {
+            query.append("id NOT IN (SELECT DISTINCT any_id FROM ");
+            query.append(svs.field().name).append(" WHERE ");
+            fillAttrQuery(anyUtils, query, attrValue, schema, cond, false, parameters, svs);
+            query.append(")");
+        } else {
+            String key = key(schema.getType());
+            boolean lower = (schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum)
+                    && (cond.getType() == AttributeCond.Type.IEQ || cond.getType() == AttributeCond.Type.ILIKE);
+
+            if (!not && cond.getType() == AttributeCond.Type.EQ) {
+                PlainAttr<?> container = anyUtils.newPlainAttr();
+                container.setSchema(schema);
+                if (attrValue instanceof PlainAttrUniqueValue) {
+                    container.setUniqueValue((PlainAttrUniqueValue) attrValue);
+                } else {
+                    ((JSONPlainAttr) container).add(attrValue);
+                }
 
-        if (!not && cond.getType() == AttributeCond.Type.EQ) {
-            PlainAttr<?> container = anyUtils.newPlainAttr();
-            container.setSchema(schema);
-            if (attrValue instanceof PlainAttrUniqueValue) {
-                container.setUniqueValue((PlainAttrUniqueValue) attrValue);
+                query.append("plainAttrs @> '").
+                        append(POJOHelper.serialize(Arrays.asList(container))).
+                        append("'::jsonb");
             } else {
-                ((JSONPlainAttr) container).add(attrValue);
-            }
+                query.append("attrs ->> 'schema' = ?").append(setParameter(parameters, cond.getSchema())).
+                        append(" AND ").
+                        append(lower ? "LOWER(" : "").
+                        append(schema.isUniqueConstraint()
+                                ? "attrs -> 'uniqueValue'" : "attrValues").
+                        append(" ->> '").append(key).append("'").
+                        append(lower ? ")" : "");
+
+                switch (cond.getType()) {
+                    case LIKE:
+                    case ILIKE:
+                        if (not) {
+                            query.append("NOT ");
+                        }
+                        query.append(" LIKE ");
+                        break;
 
-            query.append("plainAttrs @> '").
-                    append(POJOHelper.serialize(Arrays.asList(container))).
-                    append("'::jsonb");
-        } else {
-            query.append("attrs ->> 'schema' = ?").append(setParameter(parameters, cond.getSchema())).
-                    append(" AND ").
-                    append(lower ? "LOWER(" : "").
-                    append(schema.isUniqueConstraint()
-                            ? "attrs -> 'uniqueValue'" : "attrValues").
-                    append(" ->> '").append(key).append("'").
-                    append(lower ? ")" : "");
-
-            switch (cond.getType()) {
-                case LIKE:
-                case ILIKE:
-                    if (not) {
-                        query.append("NOT ");
-                    }
-                    query.append(" LIKE ");
-                    break;
-
-                case GE:
-                    if (not) {
-                        query.append('<');
-                    } else {
-                        query.append(">=");
-                    }
-                    break;
+                    case GE:
+                        if (not) {
+                            query.append('<');
+                        } else {
+                            query.append(">=");
+                        }
+                        break;
 
-                case GT:
-                    if (not) {
-                        query.append("<=");
-                    } else {
-                        query.append('>');
-                    }
-                    break;
+                    case GT:
+                        if (not) {
+                            query.append("<=");
+                        } else {
+                            query.append('>');
+                        }
+                        break;
 
-                case LE:
-                    if (not) {
-                        query.append('>');
-                    } else {
-                        query.append("<=");
-                    }
-                    break;
+                    case LE:
+                        if (not) {
+                            query.append('>');
+                        } else {
+                            query.append("<=");
+                        }
+                        break;
 
-                case LT:
-                    if (not) {
-                        query.append(">=");
-                    } else {
-                        query.append('<');
-                    }
-                    break;
+                    case LT:
+                        if (not) {
+                            query.append(">=");
+                        } else {
+                            query.append('<');
+                        }
+                        break;
 
-                case EQ:
-                case IEQ:
-                default:
-                    if (not) {
-                        query.append('!');
-                    }
-                    query.append('=');
-            }
+                    case EQ:
+                    case IEQ:
+                    default:
+                        if (not) {
+                            query.append('!');
+                        }
+                        query.append('=');
+                }
 
-            String value = cond.getExpression();
-            if (schema.getType() == AttrSchemaType.Date) {
-                try {
-                    value = String.valueOf(DATE_FORMAT.parse(value).getTime());
-                } catch (ParseException e) {
-                    LOG.error("Could not parse {} as date", value, e);
+                String value = cond.getExpression();
+                if (schema.getType() == AttrSchemaType.Date) {
+                    try {
+                        value = String.valueOf(DATE_FORMAT.parse(value).getTime());
+                    } catch (ParseException e) {
+                        LOG.error("Could not parse {} as date", value, e);
+                    }
                 }
+                query.append(lower ? "LOWER(" : "").
+                        append("?").append(setParameter(parameters, value)).
+                        append(lower ? ")" : "");
             }
-            query.append(lower ? "LOWER(" : "").
-                    append("?").append(setParameter(parameters, value)).
-                    append(lower ? ")" : "");
         }
     }
 
@@ -267,8 +280,8 @@ public class PGJPAJSONAnySearchDAO extends JPAAnySearchDAO {
             }
         }
 
-        StringBuilder query = new StringBuilder("SELECT DISTINCT any_id FROM ").
-                append(svs.field().name).append(" WHERE ");
+        StringBuilder query = 
+                new StringBuilder("SELECT DISTINCT any_id FROM ").append(svs.field().name).append(" WHERE ");
         switch (cond.getType()) {
             case ISNOTNULL:
                 query.append("plainAttrs @> '[{\"schema\":\"").
@@ -285,8 +298,12 @@ public class PGJPAJSONAnySearchDAO extends JPAAnySearchDAO {
                 break;
 
             default:
+                if (not && !(cond instanceof AnyCond) && checked.getLeft().isMultivalue()) {
+                    query = new StringBuilder("SELECT DISTINCT id AS any_id FROM ").append(svs.table().name).
+                            append(" WHERE ");
+                }
                 fillAttrQuery(anyUtilsFactory.getInstance(svs.anyTypeKind),
-                        query, checked.getRight(), checked.getLeft(), cond, not, parameters);
+                        query, checked.getRight(), checked.getLeft(), cond, not, parameters, svs);
         }
 
         return query.toString();
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 da76f48..c669690 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
@@ -130,8 +130,8 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
         Pair<String, Set<String>> filter = getAdminRealmsFilter(adminRealms, svs, parameters);
 
         // 1. get the query string from the search condition
-        Pair<StringBuilder, Set<String>> queryInfo =
-                getQuery(buildEffectiveCond(cond, filter.getRight()), parameters, svs);
+        Pair<StringBuilder, Set<String>> queryInfo = getQuery(buildEffectiveCond(cond, filter.getRight()), parameters,
+                svs);
 
         StringBuilder queryString = queryInfo.getLeft();
 
@@ -167,8 +167,8 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
             Pair<String, Set<String>> filter = getAdminRealmsFilter(adminRealms, svs, parameters);
 
             // 1. get the query string from the search condition
-            Pair<StringBuilder, Set<String>> queryInfo =
-                    getQuery(buildEffectiveCond(cond, filter.getRight()), parameters, svs);
+            Pair<StringBuilder, Set<String>> queryInfo = getQuery(buildEffectiveCond(cond, filter.getRight()),
+                    parameters, svs);
 
             StringBuilder queryString = queryInfo.getLeft();
 
@@ -384,8 +384,8 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
                         orderByNonUniquePlainSchemas.add(schema.getKey());
                     }
                     if (orderByUniquePlainSchemas.size() > 1 || orderByNonUniquePlainSchemas.size() > 1) {
-                        SyncopeClientException invalidSearch =
-                                SyncopeClientException.build(ClientExceptionType.InvalidSearchExpression);
+                        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));
@@ -973,12 +973,16 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
                 break;
 
             default:
-                if (checked.getLeft().isUniqueConstraint()) {
-                    query.append(svs.asSearchViewSupport().uniqueAttr().name);
+                if (not && !(cond instanceof AnyCond) && checked.getLeft().isMultivalue()) {
+                    query.append(svs.field().name).append(" WHERE ");
                 } else {
-                    query.append(svs.asSearchViewSupport().attr().name);
+                    if (checked.getLeft().isUniqueConstraint()) {
+                        query.append(svs.asSearchViewSupport().uniqueAttr().name);
+                    } else {
+                        query.append(svs.asSearchViewSupport().attr().name);
+                    }
+                    query.append(" WHERE schema_id='").append(checked.getLeft().getKey());
                 }
-                query.append(" WHERE schema_id='").append(checked.getLeft().getKey());
                 fillAttrQuery(query, checked.getRight(), checked.getLeft(), cond, not, parameters, svs);
         }