[GitHub] [ignite-3] valepakh commented on a diff in pull request #2216: IGNITE-19450 Add * char to latest version of unit in list command

["valepakh (via GitHub)" <gi...@apache.org>]

valepakh commented on code in PR #2216:
URL: https://github.com/apache/ignite-3/pull/2216#discussion_r1238593160


##########
modules/rest/src/main/java/org/apache/ignite/internal/rest/deployment/DeploymentManagementController.java:
##########
@@ -158,18 +156,15 @@ private static List<UnitStatus> fromUnitStatuses(List<UnitStatuses> statusesList
      * @return Unit statuses DTO.
      */
     private static @Nullable UnitStatus fromUnitStatuses(UnitStatuses statuses, Predicate<DeploymentStatus> statusFilter) {
-        Map<String, DeploymentStatus> versionToDeploymentStatus = new HashMap<>();
-        Set<Version> versions = statuses.versions();
-        for (Version version : versions) {
-            DeploymentStatus status = fromDeploymentStatus(statuses.status(version));
-            if (statusFilter.test(status)) {
-                versionToDeploymentStatus.put(version.render(), status);
-            }
-        }
-        if (versionToDeploymentStatus.isEmpty()) {
+        List<UnitVersionStatus> versionStatus = statuses.versionStatuses().stream()
+                .filter(unitVersionStatus -> statusFilter.test(fromDeploymentStatus(unitVersionStatus.getStatus())))
+                .map(e -> new UnitVersionStatus(e.getVersion().render(), fromDeploymentStatus(e.getStatus())))

Review Comment:
   Double conversion, maybe leave the `for` loop?



##########
modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/deployment/DeploymentManagementControllerTest.java:
##########
@@ -180,7 +179,7 @@ public void testList() {
         List<UnitStatus> list = list(id);
 
         List<String> versions = list.stream()
-                .flatMap(unitStatus -> unitStatus.versionToStatus().keySet().stream())
+                .flatMap(unitStatus -> unitStatus.versionToStatus().stream().map(UnitVersionStatus::getVersion))
                 .collect(Collectors.toList());
         assertThat(versions, containsInAnyOrder("1.0.0", "1.0.1", "1.1.1", "1.1.2", "1.2.1", "2.0.0"));

Review Comment:
   Now that the versions are sorted, this could be changed to
   ```suggestion
           assertThat(versions, contains("1.0.0", "1.0.1", "1.1.1", "1.1.2", "1.2.1", "2.0.0"));
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org