Repository: syncope Updated Branches: refs/heads/master a8ae113d9 -> 51c60a7a1
[SYNCOPE-1288] Task lists are now correctly sortable Project: http://git-wip-us.apache.org/repos/asf/syncope/repo Commit: http://git-wip-us.apache.org/repos/asf/syncope/commit/51c60a7a Tree: http://git-wip-us.apache.org/repos/asf/syncope/tree/51c60a7a Diff: http://git-wip-us.apache.org/repos/asf/syncope/diff/51c60a7a Branch: refs/heads/master Commit: 51c60a7a134f7680b7f604f34b1a9c4acc5ddab6 Parents: a8ae113 Author: skylark17 <matteo.alessandr...@tirasa.net> Authored: Thu Apr 5 15:59:48 2018 +0200 Committer: skylark17 <matteo.alessandr...@tirasa.net> Committed: Thu Apr 5 16:39:18 2018 +0200 ---------------------------------------------------------------------- .../console/reports/ReportDirectoryPanel.java | 6 +- .../tasks/ProvisioningTaskDirectoryPanel.java | 4 +- .../console/tasks/SchedTaskDirectoryPanel.java | 4 +- .../core/persistence/jpa/dao/JPATaskDAO.java | 243 ++++++++++++++++--- .../core/persistence/jpa/outer/TaskTest.java | 17 ++ .../syncope/fit/core/PropagationTaskITCase.java | 77 ++++++ 6 files changed, 306 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java ---------------------------------------------------------------------- diff --git a/client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java b/client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java index 21b1563..5109da3 100644 --- a/client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java +++ b/client/console/src/main/java/org/apache/syncope/client/console/reports/ReportDirectoryPanel.java @@ -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")); http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java ---------------------------------------------------------------------- diff --git a/client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java b/client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java index f7f0b2e..504e767 100644 --- a/client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java +++ b/client/console/src/main/java/org/apache/syncope/client/console/tasks/ProvisioningTaskDirectoryPanel.java @@ -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")); http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java ---------------------------------------------------------------------- diff --git a/client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java b/client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java index 824b778..bfc7eb6 100644 --- a/client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java +++ b/client/console/src/main/java/org/apache/syncope/client/console/tasks/SchedTaskDirectoryPanel.java @@ -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")); http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java ---------------------------------------------------------------------- diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java index d47ba5d..dd78ec3 100644 --- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java +++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPATaskDAO.java @@ -18,8 +18,14 @@ */ 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; + } } http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java ---------------------------------------------------------------------- diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java index 5205ffe..0da3e39 100644 --- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java +++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/TaskTest.java @@ -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); http://git-wip-us.apache.org/repos/asf/syncope/blob/51c60a7a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java ---------------------------------------------------------------------- diff --git a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java index 551dac4..5c9944d 100644 --- a/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java +++ b/fit/core-reference/src/test/java/org/apache/syncope/fit/core/PropagationTaskITCase.java @@ -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("xxx...@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())); + } + }
