Cleanup messed up actionstatus sorting. We used sort param but had

hardcoded sorting in the reposirory. I added also reportAt based sorting. Signed-off-by: Kai Zimmermann <kai.zimmermann@bosch-si.com>
2016-03-03 18:54:50 +01:00
parent 19fe7e5a46
commit e7173aa846
8 changed files with 77 additions and 80 deletions
--- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/repository/ActionStatusFields.java
+++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/repository/ActionStatusFields.java
@@ -20,7 +20,12 @@ public enum ActionStatusFields implements FieldNameProvider {
    /**
     * The id field.
     */
-    ID("id");
+    ID("id"),
+
+    /**
+     * The reportedAt field.
+     */
+    REPORTED_AT("createdAt");

    private final String fieldName;

--- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ActionStatusRepository.java
+++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ActionStatusRepository.java
@@ -50,13 +50,6 @@ public interface ActionStatusRepository
     */
    Page<ActionStatus> findByAction(Pageable pageReq, Action action);

-    /**
-     * @param pageReq
-     * @param action
-     * @return
-     */
-    Page<ActionStatus> findByActionOrderByIdDesc(Pageable pageReq, Action action);
-
    /**
     * Finds all status updates for the defined action and target order by
     * {@link ActionStatus#getId()} desc including
@@ -71,6 +64,6 @@ public interface ActionStatusRepository
     * @return Page with found targets
     */
    @EntityGraph(value = "ActionStatus.withMessages", type = EntityGraphType.LOAD)
-    Page<ActionStatus> getByActionOrderByIdDesc(Pageable pageReq, Action action);
+    Page<ActionStatus> getByAction(Pageable pageReq, Action action);

 }
--- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java
+++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java
@@ -925,7 +925,7 @@ public class DeploymentManagement {

    /**
     * retrieves all the {@link ActionStatus} entries of the given
-     * {@link Action} and {@link Target} in the order latest first.
+     * {@link Action} and {@link Target}.
     *
     * @param pageReq
     *            pagination parameter
@@ -937,12 +937,12 @@ public class DeploymentManagement {
     * @return the corresponding {@link Page} of {@link ActionStatus}
     */
    @PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_TARGET)
-    public Page<ActionStatus> findActionStatusMessagesByActionInDescOrder(final Pageable pageReq, final Action action,
+    public Page<ActionStatus> findActionStatusByAction(final Pageable pageReq, final Action action,
            final boolean withMessages) {
        if (withMessages) {
-            return actionStatusRepository.getByActionOrderByIdDesc(pageReq, action);
+            return actionStatusRepository.getByAction(pageReq, action);
        } else {
-            return actionStatusRepository.findByActionOrderByIdDesc(pageReq, action);
+            return actionStatusRepository.findByAction(pageReq, action);
        }
    }

--- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java
+++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/ControllerManagementTest.java
@@ -70,8 +70,8 @@ public class ControllerManagementTest extends AbstractIntegrationTest {
                .isEqualTo(TargetUpdateStatus.IN_SYNC);

        assertThat(actionStatusRepository.findAll(pageReq).getNumberOfElements()).isEqualTo(3);
-        assertThat(deploymentManagement.findActionStatusMessagesByActionInDescOrder(pageReq, savedAction, false)
-                .getNumberOfElements()).isEqualTo(3);
+        assertThat(deploymentManagement.findActionStatusByAction(pageReq, savedAction, false).getNumberOfElements())
+                .isEqualTo(3);
    }

    @Test
--- a/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/PagingUtility.java
+++ b/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/PagingUtility.java
@@ -102,7 +102,7 @@ public final class PagingUtility {
            sorting = new Sort(SortUtility.parse(ActionStatusFields.class, sortParam));
        } else {
            // default sort
-            sorting = new Sort(Direction.ASC, ActionStatusFields.ID.getFieldName());
+            sorting = new Sort(Direction.DESC, ActionStatusFields.ID.getFieldName());
        }
        return sorting;
    }
--- a/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java
+++ b/hawkbit-rest-resource/src/main/java/org/eclipse/hawkbit/rest/resource/TargetResource.java
@@ -235,7 +235,7 @@ public class TargetResource implements TargetRestApi {
        final int sanitizedLimitParam = PagingUtility.sanitizePageLimitParam(pagingLimitParam);
        final Sort sorting = PagingUtility.sanitizeActionStatusSortParam(sortParam);

-        final Page<ActionStatus> statusList = this.deploymentManagement.findActionStatusMessagesByActionInDescOrder(
+        final Page<ActionStatus> statusList = this.deploymentManagement.findActionStatusByAction(
                new OffsetBasedPageRequest(sanitizedOffsetParam, sanitizedLimitParam, sorting), action, true);

        return new ResponseEntity<>(
--- a/hawkbit-rest-resource/src/test/java/org/eclipse/hawkbit/rest/resource/TargetResourceTest.java
+++ b/hawkbit-rest-resource/src/test/java/org/eclipse/hawkbit/rest/resource/TargetResourceTest.java
@@ -35,6 +35,7 @@ import org.eclipse.hawkbit.TestDataUtil;
 import org.eclipse.hawkbit.WithUser;
 import org.eclipse.hawkbit.exception.SpServerError;
 import org.eclipse.hawkbit.im.authentication.SpPermission;
+import org.eclipse.hawkbit.repository.ActionFields;
 import org.eclipse.hawkbit.repository.ActionStatusFields;
 import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException;
 import org.eclipse.hawkbit.repository.model.Action;
@@ -117,27 +118,24 @@ public class TargetResourceTest extends AbstractIntegrationTest {
                new ActionStatus(actions.get(0), Status.FINISHED, System.currentTimeMillis(), "testmessage"),
                actions.get(0));

-        final PageRequest pageRequest = new PageRequest(0, 1000, Direction.ASC, ActionStatusFields.ID.getFieldName());
+        final PageRequest pageRequest = new PageRequest(0, 1000, Direction.ASC, ActionFields.ID.getFieldName());
+        final ActionStatus status = deploymentManagement
+                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
+                .get(0).getActionStatus().stream().sorted((e1, e2) -> Long.compare(e2.getId(), e1.getId()))
+                .collect(Collectors.toList()).get(0);

-        // limit to 1 - first page -> standard cancel message
-        final Long reportAt = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(0).getCreatedAt();
-        final Long id = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(0).getId();
        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
                + RestConstants.TARGET_V1_ACTIONS + "/" + actions.get(0).getId() + "/status")
                        .param(RestConstants.REQUEST_PARAMETER_PAGING_LIMIT, String.valueOf(limitSize))
-                        .param(RestConstants.REQUEST_PARAMETER_SORTING, "ID:ASC"))
+                        .param(RestConstants.REQUEST_PARAMETER_SORTING, "ID:DESC"))
                .andExpect(status().isOk()).andDo(MockMvcResultPrinter.print())
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(3)))
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(limitSize)))
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_CONTENT, hasSize(limitSize)))
-                .andExpect(jsonPath("content.[0].id", equalTo(id.intValue())))
+                .andExpect(jsonPath("content.[0].id", equalTo(status.getId().intValue())))
                .andExpect(jsonPath("content.[0].type", equalTo("finished")))
                .andExpect(jsonPath("content.[0].messages", hasSize(1)))
-                .andExpect(jsonPath("content.[0].reportedAt", equalTo(reportAt)))
+                .andExpect(jsonPath("content.[0].reportedAt", equalTo(status.getCreatedAt().longValue())))
                .andExpect(jsonPath("content.[1].type", equalTo("canceling")));
    }

@@ -855,13 +853,40 @@ public class TargetResourceTest extends AbstractIntegrationTest {
    @Test
    @Description("Verfies that the API returns the status list with expected content.")
    public void getMultipleActionStatus() throws Exception {
+        final String knownTargetId = "targetId";
+        final Action action = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId).get(0);
+        // retrieve list in default descending order for actionstaus entries
+        final List<ActionStatus> actionStatus = action.getActionStatus().stream()
+                .sorted((e1, e2) -> Long.compare(e2.getId(), e1.getId())).collect(Collectors.toList());
+
+        // sort is default descending order, latest status first
+        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
+                + RestConstants.TARGET_V1_ACTIONS + "/" + action.getId() + "/" + RestConstants.TARGET_V1_ACTION_STATUS))
+                .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
+                .andExpect(jsonPath("content.[0].id", equalTo(actionStatus.get(0).getId().intValue())))
+                .andExpect(jsonPath("content.[0].type", equalTo("canceling")))
+                .andExpect(jsonPath("content.[0].messages", hasItem("manual cancelation requested")))
+                .andExpect(jsonPath("content.[0].reportedAt", equalTo(actionStatus.get(0).getCreatedAt())))
+                .andExpect(jsonPath("content.[1].id", equalTo(actionStatus.get(1).getId().intValue())))
+                .andExpect(jsonPath("content.[1].type", equalTo("running")))
+                .andExpect(jsonPath("content.[1].reportedAt", equalTo(actionStatus.get(1).getCreatedAt())))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(2)))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(2)))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_CONTENT, hasSize(2)));
+    }
+
+    @Test
+    @Description("Verfies that the API returns the status list with expected content sorted by reportedAt field.")
+    public void getMultipleActionStatusSortedByReportedAt() throws Exception {
        final String knownTargetId = "targetId";
        final Action action = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId).get(0);
        final List<ActionStatus> actionStatus = action.getActionStatus().stream()
                .sorted((e1, e2) -> Long.compare(e1.getId(), e2.getId())).collect(Collectors.toList());

+        // descending order
        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
-                + RestConstants.TARGET_V1_ACTIONS + "/" + action.getId() + "/" + RestConstants.TARGET_V1_ACTION_STATUS))
+                + RestConstants.TARGET_V1_ACTIONS + "/" + action.getId() + "/" + RestConstants.TARGET_V1_ACTION_STATUS)
+                        .param(RestConstants.REQUEST_PARAMETER_SORTING, "REPORTED_AT:DESC"))
                .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
                .andExpect(jsonPath("content.[0].id", equalTo(actionStatus.get(1).getId().intValue())))
                .andExpect(jsonPath("content.[0].type", equalTo("canceling")))
@@ -873,6 +898,22 @@ public class TargetResourceTest extends AbstractIntegrationTest {
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(2)))
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(2)))
                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_CONTENT, hasSize(2)));
+
+        // ascending order
+        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
+                + RestConstants.TARGET_V1_ACTIONS + "/" + action.getId() + "/" + RestConstants.TARGET_V1_ACTION_STATUS)
+                        .param(RestConstants.REQUEST_PARAMETER_SORTING, "REPORTED_AT:ASC"))
+                .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
+                .andExpect(jsonPath("content.[1].id", equalTo(actionStatus.get(1).getId().intValue())))
+                .andExpect(jsonPath("content.[1].type", equalTo("canceling")))
+                .andExpect(jsonPath("content.[1].messages", hasItem("manual cancelation requested")))
+                .andExpect(jsonPath("content.[1].reportedAt", equalTo(actionStatus.get(1).getCreatedAt())))
+                .andExpect(jsonPath("content.[0].id", equalTo(actionStatus.get(0).getId().intValue())))
+                .andExpect(jsonPath("content.[0].type", equalTo("running")))
+                .andExpect(jsonPath("content.[0].reportedAt", equalTo(actionStatus.get(0).getCreatedAt())))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(2)))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(2)))
+                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_CONTENT, hasSize(2)));
    }

    @Test
@@ -963,7 +1004,8 @@ public class TargetResourceTest extends AbstractIntegrationTest {
                + "?offset=0&limit=50&sort=id:DESC";
    }

-    private List<Action> generateTargetWithTwoUpdatesWithOneOverride(final String knownTargetId) {
+    private List<Action> generateTargetWithTwoUpdatesWithOneOverride(final String knownTargetId)
+            throws InterruptedException {

        final PageRequest pageRequest = new PageRequest(0, 100, Direction.ASC, ActionStatusFields.ID.getFieldName());

@@ -981,6 +1023,8 @@ public class TargetResourceTest extends AbstractIntegrationTest {
        final List<Target> updatedTargets = deploymentManagement.assignDistributionSet(one, targets)
                .getAssignedTargets();
        // 2nd update
+        // sleep 1ms to ensure that we can sort by reportedAt
+        Thread.sleep(1);
        deploymentManagement.assignDistributionSet(two, updatedTargets);

        // two updates, one cancelation
@@ -1007,54 +1051,6 @@ public class TargetResourceTest extends AbstractIntegrationTest {
                        equalTo(generateStatusreferenceLink(knownTargetId, actions.get(1)))));
    }

-    @Test
-    public void getActionStatusWithMultipleResultsWithPagingLimitRequestParameter() throws Exception {
-        final int limitSize = 1;
-        final String knownTargetId = "targetId";
-        final List<Action> actions = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId);
-        actions.get(0).setStatus(Status.RUNNING);
-        controllerManagament.addUpdateActionStatus(
-                new ActionStatus(actions.get(0), Status.RUNNING, System.currentTimeMillis(), "testmessage"),
-                actions.get(0));
-
-        final PageRequest pageRequest = new PageRequest(0, 1000, Direction.ASC, ActionStatusFields.ID.getFieldName());
-
-        // limit to 1 - first page -> standard cancel message
-        Long reportAt = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(0).getCreatedAt();
-        Long id = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(0).getId();
-        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
-                + RestConstants.TARGET_V1_ACTIONS + "/" + actions.get(0).getId() + "/status")
-                        .param(RestConstants.REQUEST_PARAMETER_PAGING_LIMIT, String.valueOf(limitSize)))
-                .andExpect(status().isOk()).andDo(MockMvcResultPrinter.print())
-                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(3)))
-                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(limitSize)))
-                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_CONTENT, hasSize(limitSize)))
-                .andExpect(jsonPath("content.[0].id", equalTo(id.intValue())))
-                .andExpect(jsonPath("content.[0].type", equalTo("running")))
-                .andExpect(jsonPath("content.[0].messages", hasSize(1)))
-                .andExpect(jsonPath("content.[0].reportedAt", equalTo(reportAt)));
-
-        // limit to 1 - first page -> added custom message
-        reportAt = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(1).getCreatedAt();
-        id = deploymentManagement
-                .findActionsByTarget(pageRequest, targetManagement.findTargetByControllerID(knownTargetId)).getContent()
-                .get(1).getCreatedAt();
-
-        mvc.perform(get(RestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
-                + RestConstants.TARGET_V1_ACTIONS + "/" + actions.get(0).getId() + "/status")
-                        .param(RestConstants.REQUEST_PARAMETER_PAGING_LIMIT, String.valueOf(limitSize))
-                        .param(RestConstants.REQUEST_PARAMETER_PAGING_OFFSET, String.valueOf(1)))
-                .andExpect(status().isOk()).andDo(MockMvcResultPrinter.print())
-                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_TOTAL, equalTo(3)))
-                .andExpect(jsonPath(JSON_PATH_PAGED_LIST_SIZE, equalTo(1)));
-    }
-
    @Test
    public void assignDistributionSetToTarget() throws Exception {

--- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/actionhistory/ActionHistoryTable.java
+++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/actionhistory/ActionHistoryTable.java
@@ -16,6 +16,7 @@ import java.util.StringJoiner;
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;

+import org.eclipse.hawkbit.repository.ActionStatusFields;
 import org.eclipse.hawkbit.repository.DeploymentManagement;
 import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException;
 import org.eclipse.hawkbit.repository.model.Action;
@@ -43,6 +44,8 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Direction;
 import org.vaadin.spring.events.EventBus;
 import org.vaadin.spring.events.EventScope;
 import org.vaadin.spring.events.annotation.EventBusListenerMethod;
@@ -417,9 +420,9 @@ public class ActionHistoryTable extends TreeTable implements Handler {

            final org.eclipse.hawkbit.repository.model.Action action = deploymentManagement
                    .findActionWithDetails(actionId);
-            final Pageable pageReq = new PageRequest(0, 1000);
+            final Pageable pageReq = new PageRequest(0, 1000, new Sort(Direction.ASC, ActionStatusFields.ID.getFieldName()));
            final Page<ActionStatus> actionStatusList = deploymentManagement
-                    .findActionStatusMessagesByActionInDescOrder(pageReq, action,
+                    .findActionStatusByAction(pageReq, action,
                            managementUIState.isActionHistoryMaximized());
            final List<ActionStatus> content = actionStatusList.getContent();
            /*