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

[syncope] branch 2_1_X updated (c36eef3 -> 012d351)

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

fmartelli pushed a change to branch 2_1_X
in repository https://gitbox.apache.org/repos/asf/syncope.git.


    from c36eef3  Upgrading Elasticsearch
     new 6a81655  [SYNCOPE-1419] provides the correct behavior in case of multivalue fields
     new 012d351  [SYNCOPE-1419] provides the correct behavior in case of multivalue fields for json implementation as well

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../persistence/jpa/dao/PGJPAJSONAnySearchDAO.java | 177 +++++++++--------
 .../core/persistence/jpa/dao/JPAAnySearchDAO.java  | 209 +++++++++++----------
 .../core/persistence/jpa/inner/AnySearchTest.java  |  14 ++
 3 files changed, 225 insertions(+), 175 deletions(-)


[syncope] 01/02: [SYNCOPE-1419] provides the correct behavior in case of multivalue fields

Posted by fm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6a8165568da35a39295b521958ce211e2ac2f219
Author: fmartelli <fa...@gmail.com>
AuthorDate: Thu Dec 20 15:19:16 2018 +0100

    [SYNCOPE-1419] provides the correct behavior in case of multivalue fields
---
 .../core/persistence/jpa/dao/JPAAnySearchDAO.java  | 209 +++++++++++----------
 .../core/persistence/jpa/inner/AnySearchTest.java  |  14 ++
 2 files changed, 128 insertions(+), 95 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 da76f48..26eccfd 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
@@ -834,111 +834,126 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
             final boolean not,
             final List<Object> parameters,
             final SearchSupport svs) {
+        // 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("any_id NOT IN (SELECT DISTINCT any_id FROM ");
+            if (schema.isUniqueConstraint()) {
+                query.append(svs.asSearchViewSupport().uniqueAttr().name);
+            } else {
+                query.append(svs.asSearchViewSupport().attr().name);
+            }
+            query.append(" WHERE schema_id='").append(schema.getKey());
+            fillAttrQuery(query, attrValue, schema, cond, false, parameters, svs);
+            query.append(")");
+        } else {
+            // activate ignoreCase only for EQ and LIKE operators
+            boolean ignoreCase = AttributeCond.Type.ILIKE == cond.getType() || AttributeCond.Type.IEQ == cond.getType();
 
-        // activate ignoreCase only for EQ and LIKE operators
-        boolean ignoreCase = AttributeCond.Type.ILIKE == cond.getType() || AttributeCond.Type.IEQ == cond.getType();
-
-        String column = (cond instanceof AnyCond) ? cond.getSchema() : svs.fieldName(schema.getType());
-        if ((schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum) && ignoreCase) {
-            column = "LOWER (" + column + ")";
-        }
-        if (!(cond instanceof AnyCond)) {
-            column = "' AND " + column;
-        }
-
-        switch (cond.getType()) {
-
-            case ISNULL:
-                query.append(column).append(not
-                        ? " IS NOT NULL"
-                        : " IS NULL");
-                break;
+            String column = (cond instanceof AnyCond) ? cond.getSchema() : svs.fieldName(schema.getType());
+            if ((schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum) && ignoreCase) {
+                column = "LOWER (" + column + ")";
+            }
+            if (!(cond instanceof AnyCond)) {
+                column = "' AND " + column;
+            }
 
-            case ISNOTNULL:
-                query.append(column).append(not
-                        ? " IS NULL"
-                        : " IS NOT NULL");
-                break;
+            switch (cond.getType()) {
+
+                case ISNULL:
+                    query.append(column).append(not
+                            ? " IS NOT NULL"
+                            : " IS NULL");
+                    break;
+
+                case ISNOTNULL:
+                    query.append(column).append(not
+                            ? " IS NULL"
+                            : " IS NOT NULL");
+                    break;
+
+                case ILIKE:
+                case LIKE:
+                    if (schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum) {
+                        query.append(column);
+                        if (not) {
+                            query.append(" NOT ");
+                        }
+                        query.append(" LIKE ");
+                        if (ignoreCase) {
+                            query.append("LOWER(?").append(setParameter(parameters, cond.getExpression())).append(')');
+                        } else {
+                            query.append('?').append(setParameter(parameters, cond.getExpression()));
+                        }
+                    } else {
+                        if (!(cond instanceof AnyCond)) {
+                            query.append("' AND");
+                        }
+                        query.append(" 1=2");
+                        LOG.error("LIKE is only compatible with string or enum schemas");
+                    }
+                    break;
 
-            case ILIKE:
-            case LIKE:
-                if (schema.getType() == AttrSchemaType.String || schema.getType() == AttrSchemaType.Enum) {
+                case IEQ:
+                case EQ:
                     query.append(column);
                     if (not) {
-                        query.append(" NOT ");
-                    }
-                    query.append(" LIKE ");
-                    if (ignoreCase) {
-                        query.append("LOWER(?").append(setParameter(parameters, cond.getExpression())).append(')');
+                        query.append("<>");
                     } else {
-                        query.append('?').append(setParameter(parameters, cond.getExpression()));
+                        query.append('=');
                     }
-                } else {
-                    if (!(cond instanceof AnyCond)) {
-                        query.append("' AND");
+                    if ((schema.getType() == AttrSchemaType.String
+                            || schema.getType() == AttrSchemaType.Enum) && ignoreCase) {
+                        query.append("LOWER(?").append(setParameter(parameters, attrValue.getValue())).append(')');
+                    } else {
+                        query.append('?').append(setParameter(parameters, attrValue.getValue()));
                     }
-                    query.append(" 1=2");
-                    LOG.error("LIKE is only compatible with string or enum schemas");
-                }
-                break;
+                    break;
 
-            case IEQ:
-            case EQ:
-                query.append(column);
-                if (not) {
-                    query.append("<>");
-                } else {
-                    query.append('=');
-                }
-                if ((schema.getType() == AttrSchemaType.String
-                        || schema.getType() == AttrSchemaType.Enum) && ignoreCase) {
-                    query.append("LOWER(?").append(setParameter(parameters, attrValue.getValue())).append(')');
-                } else {
+                case GE:
+                    query.append(column);
+                    if (not) {
+                        query.append('<');
+                    } else {
+                        query.append(">=");
+                    }
                     query.append('?').append(setParameter(parameters, attrValue.getValue()));
-                }
-                break;
-
-            case GE:
-                query.append(column);
-                if (not) {
-                    query.append('<');
-                } else {
-                    query.append(">=");
-                }
-                query.append('?').append(setParameter(parameters, attrValue.getValue()));
-                break;
+                    break;
 
-            case GT:
-                query.append(column);
-                if (not) {
-                    query.append("<=");
-                } else {
-                    query.append('>');
-                }
-                query.append('?').append(setParameter(parameters, attrValue.getValue()));
-                break;
+                case GT:
+                    query.append(column);
+                    if (not) {
+                        query.append("<=");
+                    } else {
+                        query.append('>');
+                    }
+                    query.append('?').append(setParameter(parameters, attrValue.getValue()));
+                    break;
 
-            case LE:
-                query.append(column);
-                if (not) {
-                    query.append('>');
-                } else {
-                    query.append("<=");
-                }
-                query.append('?').append(setParameter(parameters, attrValue.getValue()));
-                break;
+                case LE:
+                    query.append(column);
+                    if (not) {
+                        query.append('>');
+                    } else {
+                        query.append("<=");
+                    }
+                    query.append('?').append(setParameter(parameters, attrValue.getValue()));
+                    break;
 
-            case LT:
-                query.append(column);
-                if (not) {
-                    query.append(">=");
-                } else {
-                    query.append('<');
-                }
-                query.append('?').append(setParameter(parameters, attrValue.getValue()));
-                break;
+                case LT:
+                    query.append(column);
+                    if (not) {
+                        query.append(">=");
+                    } else {
+                        query.append('<');
+                    }
+                    query.append('?').append(setParameter(parameters, attrValue.getValue()));
+                    break;
 
-            default:
+                default:
+            }
         }
     }
 
@@ -973,12 +988,16 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
                 break;
 
             default:
-                if (checked.getLeft().isUniqueConstraint()) {
-                    query.append(svs.asSearchViewSupport().uniqueAttr().name);
+                if (not && 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);
         }
 
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/AnySearchTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/AnySearchTest.java
index 43cc944..8d8ea69 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/AnySearchTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/AnySearchTest.java
@@ -770,4 +770,18 @@ public class AnySearchTest extends AbstractTest {
                 searchDAO.count(SyncopeConstants.FULL_ADMIN_REALMS, searchCondition, AnyTypeKind.USER),
                 users.size());
     }
+
+    @Test
+    public void issueSYNCOPE1419() {
+        AttributeCond loginDateCond = new AttributeCond(AttributeCond.Type.EQ);
+        loginDateCond.setSchema("loginDate");
+        loginDateCond.setExpression("2009-05-26");
+
+        SearchCond cond = SearchCond.getNotLeafCond(loginDateCond);
+        assertTrue(cond.isValid());
+
+        List<User> users = searchDAO.search(cond, AnyTypeKind.USER);
+        assertNotNull(users);
+        assertEquals(4, users.size());
+    }
 }


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

Posted by fm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 012d351434061a13dff2c697e8f399e2a41d24fc
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  |   2 +-
 2 files changed, 98 insertions(+), 81 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 26eccfd..81c0376 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
@@ -988,7 +988,7 @@ public class JPAAnySearchDAO extends AbstractAnySearchDAO {
                 break;
 
             default:
-                if (not && checked.getLeft().isMultivalue()) {
+                if (not && !(cond instanceof AnyCond) && checked.getLeft().isMultivalue()) {
                     query.append(svs.field().name).append(" WHERE ");
                 } else {
                     if (checked.getLeft().isUniqueConstraint()) {