[SYNCOPE-1288] Task lists are now correctly sortable
authorskylark17 <matteo.alessandroni@tirasa.net>
Thu, 5 Apr 2018 13:59:48 +0000 (15:59 +0200)
committerskylark17 <matteo.alessandroni@tirasa.net>
Thu, 5 Apr 2018 14:39:18 +0000 (16:39 +0200)
client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java
client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java
client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java
core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java
core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java
fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java

index 21b1563..5109da3 100644 (file)
@@ -108,11 +108,11 @@ public abstract class ReportDirectoryPanel
         columns.add(new PropertyColumn<>(new StringResourceModel("name", this), "name", "name"));
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("lastExec", this), "lastExec", "lastExec"));
+                new StringResourceModel("lastExec", this), null, "lastExec"));
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("nextExec", this), "nextExec", "nextExec"));
-
+                new StringResourceModel("nextExec", this), null, "nextExec"));
+        
         columns.add(new DatePropertyColumn<>(
                 new StringResourceModel("start", this), "start", "start"));
 
index f7f0b2e..504e767 100644 (file)
@@ -117,10 +117,10 @@ public abstract class ProvisioningTaskDirectoryPanel<T extends ProvisioningTaskT
         }
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("lastExec", this), "lastExec", "lastExec"));
+                new StringResourceModel("lastExec", this), null, "lastExec"));
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("nextExec", this), "nextExec", "nextExec"));
+                new StringResourceModel("nextExec", this), null, "nextExec"));
 
         columns.add(new PropertyColumn<>(
                 new StringResourceModel("latestExecStatus", this), "latestExecStatus", "latestExecStatus"));
index 824b778..bfc7eb6 100644 (file)
@@ -161,10 +161,10 @@ public abstract class SchedTaskDirectoryPanel<T extends SchedTaskTO>
         });
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("lastExec", this), "lastExec", "lastExec"));
+                new StringResourceModel("lastExec", this), null, "lastExec"));
 
         columns.add(new DatePropertyColumn<>(
-                new StringResourceModel("nextExec", this), "nextExec", "nextExec"));
+                new StringResourceModel("nextExec", this), null, "nextExec"));
 
         columns.add(new PropertyColumn<>(
                 new StringResourceModel("latestExecStatus", this), "latestExecStatus", "latestExecStatus"));
index d47ba5d..dd78ec3 100644 (file)
  */
 package org.apache.syncope.core.persistence.jpa.dao;
 
+import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Predicate;
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.ManyToOne;
+import javax.persistence.OneToMany;
 import javax.persistence.Query;
 import javax.persistence.TypedQuery;
 import org.apache.commons.lang3.StringUtils;
@@ -42,6 +48,7 @@ import org.apache.syncope.core.persistence.jpa.entity.task.JPAPushTask;
 import org.apache.syncope.core.persistence.jpa.entity.task.JPASchedTask;
 import org.apache.syncope.core.persistence.jpa.entity.task.JPAPullTask;
 import org.apache.syncope.core.persistence.jpa.entity.task.AbstractTask;
+import org.apache.syncope.core.persistence.jpa.entity.task.JPATaskExec;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
 import org.springframework.transaction.annotation.Transactional;
@@ -84,6 +91,36 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
         return result;
     }
 
+    private String getEntityTableName(final TaskType type) {
+        String result = null;
+
+        switch (type) {
+            case NOTIFICATION:
+                result = JPANotificationTask.class.getAnnotation(DiscriminatorValue.class).value();
+                break;
+
+            case PROPAGATION:
+                result = JPAPropagationTask.class.getAnnotation(DiscriminatorValue.class).value();
+                break;
+
+            case PUSH:
+                result = JPAPushTask.class.getAnnotation(DiscriminatorValue.class).value();
+                break;
+
+            case SCHEDULED:
+                result = JPASchedTask.class.getAnnotation(DiscriminatorValue.class).value();
+                break;
+
+            case PULL:
+                result = JPAPullTask.class.getAnnotation(DiscriminatorValue.class).value();
+                break;
+
+            default:
+        }
+
+        return result;
+    }
+
     @Transactional(readOnly = true)
     @SuppressWarnings("unchecked")
     @Override
@@ -131,7 +168,7 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
         return query.getResultList();
     }
 
-    private <T extends Task> StringBuilder buildFindAllQuery(final TaskType type) {
+    private <T extends Task> StringBuilder buildFindAllQueryJPA(final TaskType type) {
         StringBuilder builder = new StringBuilder("SELECT t FROM ").
                 append(getEntityReference(type).getSimpleName()).
                 append(" t WHERE ");
@@ -151,7 +188,7 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
     @Override
     @SuppressWarnings("unchecked")
     public <T extends Task> List<T> findToExec(final TaskType type) {
-        StringBuilder queryString = buildFindAllQuery(type).append("AND ");
+        StringBuilder queryString = buildFindAllQueryJPA(type).append("AND ");
 
         if (type == TaskType.NOTIFICATION) {
             queryString.append("t.executed = 0 ");
@@ -175,7 +212,9 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
             final ExternalResource resource,
             final Notification notification,
             final AnyTypeKind anyTypeKind,
-            final String entityKey) {
+            final String entityKey,
+            final boolean orderByTaskExecInfo,
+            final List<Object> queryParameters) {
 
         if (resource != null
                 && type != TaskType.PROPAGATION && type != TaskType.PUSH && type != TaskType.PULL) {
@@ -193,38 +232,135 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
             throw new IllegalArgumentException(type + " is not related to notifications");
         }
 
-        StringBuilder queryString = buildFindAllQuery(type);
+        StringBuilder queryString = new StringBuilder("SELECT ").
+                append(AbstractTask.TABLE).
+                append(".id FROM ").
+                append(AbstractTask.TABLE);
+        if (orderByTaskExecInfo) {
+            queryString.append(" LEFT OUTER JOIN ").
+                    append(JPATaskExec.TABLE).
+                    append(" ON ").
+                    append(AbstractTask.TABLE).
+                    append(".id = ").
+                    append(JPATaskExec.TABLE).
+                    append(".task_id");
+        }
+        queryString.append(" WHERE ").
+                append(AbstractTask.TABLE).
+                append(".DTYPE = ?1");
+        queryParameters.add(getEntityTableName(type));
+        if (type == TaskType.SCHEDULED) {
+            queryString.append(" AND ").
+                    append(AbstractTask.TABLE).
+                    append(".id NOT IN (SELECT ").append(AbstractTask.TABLE).append(".id FROM ").
+                    append(AbstractTask.TABLE).append(" WHERE ").
+                    append(AbstractTask.TABLE).append(".DTYPE = ?2)").
+                    append(" AND ").
+                    append(AbstractTask.TABLE).
+                    append(".id NOT IN (SELECT id FROM ").
+                    append(AbstractTask.TABLE).append(" WHERE ").
+                    append(AbstractTask.TABLE).append(".DTYPE = ?3)");
+
+            queryParameters.add(JPAPushTask.class.getAnnotation(DiscriminatorValue.class).value());
+            queryParameters.add(JPAPullTask.class.getAnnotation(DiscriminatorValue.class).value());
+        }
+        queryString.append(' ');
 
         if (resource != null) {
-            queryString.append("AND t.resource=:resource ");
+            queryParameters.add(resource.getKey());
+
+            queryString.append("AND ").
+                    append(AbstractTask.TABLE).
+                    append(".resource_id=?").append(queryParameters.size());
         }
         if (notification != null) {
-            queryString.append("AND t.notification=:notification ");
+            queryParameters.add(notification.getKey());
+
+            queryString.append("AND ").
+                    append(AbstractTask.TABLE).
+                    append(".notification_id=?").append(queryParameters.size());
         }
         if (anyTypeKind != null && entityKey != null) {
-            queryString.append("AND t.anyTypeKind=:anyTypeKind AND t.entityKey=:entityKey ");
+            queryParameters.add(anyTypeKind.name());
+            queryParameters.add(entityKey);
+
+            queryString.append("AND ").
+                    append(AbstractTask.TABLE).
+                    append(".anyTypeKind=?").append(queryParameters.size() - 1).
+                    append(" AND ").
+                    append(AbstractTask.TABLE).
+                    append(".entityKey=?").append(queryParameters.size());
         }
 
         return queryString;
     }
 
     private String toOrderByStatement(
-            final Class<? extends Task> beanClass, final List<OrderByClause> orderByClauses) {
+            final Class<? extends Task> beanClass,
+            final List<OrderByClause> orderByClauses,
+            final boolean orderByTaskExecInfo) {
 
         StringBuilder statement = new StringBuilder();
 
+        if (orderByTaskExecInfo) {
+            statement.append(" AND (").
+                    append(JPATaskExec.TABLE).
+                    append(".startDate IS NULL OR ").
+                    append(JPATaskExec.TABLE).
+                    append(".startDate = (SELECT MAX(").
+                    append(JPATaskExec.TABLE).
+                    append(".startDate) FROM ").
+                    append(JPATaskExec.TABLE).
+                    append(" WHERE ").
+                    append(AbstractTask.TABLE).
+                    append(".id = ").
+                    append(JPATaskExec.TABLE).
+                    append(".task_id))");
+        }
+        statement.append(" ORDER BY ");
+
+        StringBuilder subStatement = new StringBuilder();
         orderByClauses.forEach(clause -> {
             String field = clause.getField().trim();
-            if (ReflectionUtils.findField(beanClass, field) != null) {
-                statement.append("t.").append(field).append(' ').append(clause.getDirection().name());
+            String table = JPATaskExec.TABLE;
+            switch (field) {
+                case "latestExecStatus":
+                    field = "status";
+                    break;
+
+                case "start":
+                    field = "startDate";
+                    break;
+
+                case "end":
+                    field = "endDate";
+                    break;
+
+                default:
+                    Field beanField = ReflectionUtils.findField(beanClass, field);
+                    if (beanField != null
+                            && (beanField.getAnnotation(ManyToOne.class) != null
+                            || beanField.getAnnotation(OneToMany.class) != null)) {
+                        field += "_id";
+                    }
+                    table = AbstractTask.TABLE;
             }
+            subStatement.append(table).
+                    append(".").
+                    append(field).
+                    append(' ').
+                    append(clause.getDirection().name()).
+                    append(',');
         });
 
-        if (statement.length() == 0) {
-            statement.append("ORDER BY t.id DESC");
+        if (subStatement.length() == 0) {
+            statement.append(AbstractTask.TABLE).
+                    append(".id DESC");
         } else {
-            statement.insert(0, "ORDER BY ");
+            subStatement.deleteCharAt(subStatement.length() - 1);
+            statement.append(subStatement);
         }
+
         return statement.toString();
     }
 
@@ -240,19 +376,31 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
             final int itemsPerPage,
             final List<OrderByClause> orderByClauses) {
 
-        StringBuilder queryString = buildFindAllQuery(type, resource, notification, anyTypeKind, entityKey).
-                append(toOrderByStatement(getEntityReference(type), orderByClauses));
+        List<Object> queryParameters = new ArrayList<>();
 
-        Query query = entityManager().createQuery(queryString.toString());
-        if (resource != null) {
-            query.setParameter("resource", resource);
-        }
-        if (notification != null) {
-            query.setParameter("notification", notification);
-        }
-        if (anyTypeKind != null && entityKey != null) {
-            query.setParameter("anyTypeKind", anyTypeKind);
-            query.setParameter("entityKey", entityKey);
+        boolean orderByTaskExecInfo = orderByClauses.stream().anyMatch(new Predicate<OrderByClause>() {
+
+            @Override
+            public boolean test(final OrderByClause t) {
+                return t.getField().equals("start")
+                        || t.getField().equals("end")
+                        || t.getField().equals("latestExecStatus")
+                        || t.getField().equals("status");
+            }
+        });
+        StringBuilder queryString = buildFindAllQuery(type,
+                resource,
+                notification,
+                anyTypeKind,
+                entityKey,
+                orderByTaskExecInfo,
+                queryParameters).
+                append(toOrderByStatement(getEntityReference(type), orderByClauses, orderByTaskExecInfo));
+
+        Query query = entityManager().createNativeQuery(queryString.toString());
+
+        for (int i = 1; i <= queryParameters.size(); i++) {
+            query.setParameter(i, queryParameters.get(i - 1));
         }
 
         query.setFirstResult(itemsPerPage * (page <= 0 ? 0 : page - 1));
@@ -261,7 +409,7 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
             query.setMaxResults(itemsPerPage);
         }
 
-        return query.getResultList();
+        return buildResult(query.getResultList());
     }
 
     @Override
@@ -272,19 +420,18 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
             final AnyTypeKind anyTypeKind,
             final String entityKey) {
 
-        StringBuilder queryString = buildFindAllQuery(type, resource, notification, anyTypeKind, entityKey);
+        List<Object> queryParameters = new ArrayList<>();
 
-        Query query = entityManager().createQuery(StringUtils.replaceOnce(
-                queryString.toString(), "SELECT t", "SELECT COUNT(t)"));
-        if (resource != null) {
-            query.setParameter("resource", resource);
-        }
-        if (notification != null) {
-            query.setParameter("notification", notification);
-        }
-        if (anyTypeKind != null && entityKey != null) {
-            query.setParameter("anyTypeKind", anyTypeKind);
-            query.setParameter("entityKey", entityKey);
+        StringBuilder queryString =
+                buildFindAllQuery(type, resource, notification, anyTypeKind, entityKey, false, queryParameters);
+
+        Query query = entityManager().createNativeQuery(StringUtils.replaceOnce(
+                queryString.toString(),
+                "SELECT " + AbstractTask.TABLE + ".id",
+                "SELECT COUNT(" + AbstractTask.TABLE + ".id)"));
+
+        for (int i = 1; i <= queryParameters.size(); i++) {
+            query.setParameter(i, queryParameters.get(i - 1));
         }
 
         return ((Number) query.getSingleResult()).intValue();
@@ -322,4 +469,24 @@ public class JPATaskDAO extends AbstractDAO<Task> implements TaskDAO {
         findAll(type, resource, null, null, null, -1, -1, Collections.<OrderByClause>emptyList()).
                 stream().map(Entity::getKey).forEach(task -> delete(task));
     }
+
+    private <T extends Task> List<T> buildResult(final List<Object> raw) {
+        List<T> result = new ArrayList<>();
+
+        for (Object anyKey : raw) {
+            String actualKey = anyKey instanceof Object[]
+                    ? (String) ((Object[]) anyKey)[0]
+                    : ((String) anyKey);
+
+            @SuppressWarnings("unchecked")
+            T any = find(actualKey);
+            if (any == null) {
+                LOG.error("Could not find task with id {}, even if returned by native query", actualKey);
+            } else if (!result.contains(any)) {
+                result.add(any);
+            }
+        }
+
+        return result;
+    }
 }
index 5205ffe..0da3e39 100644 (file)
@@ -24,9 +24,11 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 import org.apache.syncope.common.lib.to.UserTO;
@@ -91,6 +93,21 @@ public class TaskTest extends AbstractTest {
     }
 
     @Test
+    public void readMultipleOrderBy() {
+        List<OrderByClause> orderByClauses = new ArrayList<>();
+        OrderByClause clause1 = new OrderByClause();
+        clause1.setField("start");
+        OrderByClause clause2 = new OrderByClause();
+        clause2.setField("latestExecStatus");
+        OrderByClause clause3 = new OrderByClause();
+        clause3.setField("connObjectKey");
+        orderByClauses.add(clause1);
+        orderByClauses.add(clause2);
+        orderByClauses.add(clause3);
+        assertFalse(taskDAO.findAll(TaskType.PROPAGATION, null, null, null, null, -1, -1, orderByClauses).isEmpty());
+    }
+
+    @Test
     public void save() {
         ExternalResource resource = resourceDAO.find("ws-target-resource-1");
         assertNotNull(resource);
index 551dac4..5c9944d 100644 (file)
@@ -24,9 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 import org.apache.commons.lang3.SerializationUtils;
+import org.apache.syncope.common.lib.patch.AttrPatch;
+import org.apache.syncope.common.lib.patch.UserPatch;
 import org.apache.syncope.common.lib.to.TaskTO;
 import org.apache.syncope.common.lib.to.AnyObjectTO;
 import org.apache.syncope.common.lib.to.AttrTO;
@@ -42,6 +46,7 @@ import org.apache.syncope.common.lib.to.ResourceTO;
 import org.apache.syncope.common.lib.to.UserTO;
 import org.apache.syncope.common.lib.types.AnyTypeKind;
 import org.apache.syncope.common.lib.types.MappingPurpose;
+import org.apache.syncope.common.lib.types.PatchOperation;
 import org.apache.syncope.common.lib.types.TaskType;
 import org.apache.syncope.common.rest.api.beans.ExecuteQuery;
 import org.apache.syncope.common.rest.api.beans.ExecQuery;
@@ -225,4 +230,76 @@ public class PropagationTaskITCase extends AbstractTaskITCase {
                 page(1).size(2).build());
         assertTrue(execs.getTotalCount() >= execs.getResult().size());
     }
+
+    @Test
+    public void issueSYNCOPE1288() {
+        // create a new user
+        UserTO userTO = UserITCase.getUniqueSampleTO("xxxyyy@xxx.xxx");
+        userTO.getResources().add(RESOURCE_NAME_LDAP);
+
+        userTO = createUser(userTO).getEntity();
+        assertNotNull(userTO);
+
+        // generate some PropagationTasks
+        for (int i = 0; i < 9; i++) {
+            UserPatch userPatch = new UserPatch();
+            userPatch.setKey(userTO.getKey());
+            userPatch.getPlainAttrs().add(new AttrPatch.Builder().operation(PatchOperation.ADD_REPLACE).
+                    attrTO(new AttrTO.Builder().schema("userId").value(
+                            "test" + getUUIDString() + i + "@test.com").build()).
+                    build());
+
+            userService.update(userPatch);
+        }
+
+        // ASC order
+        PagedResult<TaskTO> unorderedTasks = taskService.search(
+                new TaskQuery.Builder(TaskType.PROPAGATION).
+                        resource(RESOURCE_NAME_LDAP).
+                        entityKey(userTO.getKey()).
+                        anyTypeKind(AnyTypeKind.USER).
+                        page(1).
+                        size(10).
+                        build());
+        Collections.sort(unorderedTasks.getResult(), new Comparator<TaskTO>() {
+
+            @Override
+            public int compare(final TaskTO o1, final TaskTO o2) {
+                return o1.getStart().compareTo(o2.getStart());
+            }
+        });
+        assertNotNull(unorderedTasks);
+        assertFalse(unorderedTasks.getResult().isEmpty());
+        assertEquals(10, unorderedTasks.getResult().size());
+
+        PagedResult<TaskTO> orderedTasks = taskService.search(
+                new TaskQuery.Builder(TaskType.PROPAGATION).
+                        resource(RESOURCE_NAME_LDAP).
+                        entityKey(userTO.getKey()).
+                        anyTypeKind(AnyTypeKind.USER).
+                        page(1).
+                        size(10).
+                        orderBy("start").
+                        build());
+        assertNotNull(orderedTasks);
+        assertFalse(orderedTasks.getResult().isEmpty());
+        assertEquals(10, orderedTasks.getResult().size());
+
+        assertTrue(orderedTasks.getResult().equals(unorderedTasks.getResult()));
+
+        // DESC order
+        Collections.reverse(unorderedTasks.getResult());
+        orderedTasks = taskService.search(
+                new TaskQuery.Builder(TaskType.PROPAGATION).
+                        resource(RESOURCE_NAME_LDAP).
+                        entityKey(userTO.getKey()).
+                        anyTypeKind(AnyTypeKind.USER).
+                        page(1).
+                        size(10).
+                        orderBy("start DESC").
+                        build());
+
+        assertTrue(orderedTasks.getResult().equals(unorderedTasks.getResult()));
+    }
+
 }