Fix versions filter in the instances management api (#222)

Fix versions filter in the instances management api.
When a version parameter is provided, the list instances management api was not removing entries with unmatching versions, the converter was setting null as version for those unmatching instances. This is due to the fact that the processDefinitionKey, which is the workflow id, was used for filtering. This has been replaced by the processDefinitionId which is the id of the matching versions instances.

workflow-bot-app/src/main/java/com/symphony/bdk/workflow/engine/camunda/monitoring/repository/WorkflowInstCmdaApiQueryRepository.java

@@ -4,7 +4,6 @@
 import com.symphony.bdk.workflow.converter.ObjectConverter;
 import com.symphony.bdk.workflow.monitoring.repository.WorkflowInstQueryRepository;
 import com.symphony.bdk.workflow.monitoring.repository.domain.WorkflowInstanceDomain;
-
 import org.apache.commons.lang3.StringUtils;
 import org.camunda.bpm.engine.HistoryService;
 import org.camunda.bpm.engine.RepositoryService;
@@ -19,14 +18,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 @Component
 public class WorkflowInstCmdaApiQueryRepository extends CamundaAbstractQueryRepository
-    implements WorkflowInstQueryRepository {
+        implements WorkflowInstQueryRepository {
   public WorkflowInstCmdaApiQueryRepository(RepositoryService repositoryService, HistoryService historyService,
-      RuntimeService runtimeService, ObjectConverter objectConverter) {
+                                            RuntimeService runtimeService, ObjectConverter objectConverter) {
     super(repositoryService, historyService, runtimeService, objectConverter);
   }
 
@@ -38,17 +38,29 @@ public WorkflowInstCmdaApiQueryRepository(RepositoryService repositoryService, H
    */
   @Override
   public List<WorkflowInstanceDomain> findAllById(String id) {
-    return this.findAllByIdAndVersion(id, null);
+    Map<String, String> processIdVersionTagMap = getProcessIdVersionMap(id, null);
+    List<HistoricProcessInstance> instances = historyService.createHistoricProcessInstanceQuery()
+            .processDefinitionKey(id)
+            .orderByProcessInstanceStartTime()
+            .asc()
+            .list();
+
+    return objectConverter.convertCollection(instances, processIdVersionTagMap, WorkflowInstanceDomain.class);
   }
 
   @Override
   public List<WorkflowInstanceDomain> findAllByIdAndVersion(String id, String version) {
     Map<String, String> processIdVersionTagMap = getProcessIdVersionMap(id, version);
-    List<HistoricProcessInstance> instances = historyService.createHistoricProcessInstanceQuery()
-        .processDefinitionKey(id)
-        .orderByProcessInstanceStartTime()
-        .asc()
-        .list();
+    List<HistoricProcessInstance> instances = new ArrayList<>();
+
+    processIdVersionTagMap.keySet().forEach(processDefinitionId -> {
+      instances.addAll(historyService.createHistoricProcessInstanceQuery()
+              .processDefinitionId(processDefinitionId)
+              .orderByProcessInstanceStartTime()
+              .asc()
+              .list());
+    });
+
     return objectConverter.convertCollection(instances, processIdVersionTagMap, WorkflowInstanceDomain.class);
   }
 
@@ -57,8 +69,8 @@ private Map<String, String> getProcessIdVersionMap(String id, String version) {
     Optional.ofNullable(version).ifPresent(definitionQuery::versionTag);
     List<ProcessDefinition> definitions = definitionQuery.list();
     return definitions.stream()
-        .filter(def -> StringUtils.isNotBlank(def.getVersionTag()))
-        .collect(Collectors.toMap(ProcessDefinition::getId, ProcessDefinition::getVersionTag));
+            .filter(def -> StringUtils.isNotBlank(def.getVersionTag()))
+            .collect(Collectors.toMap(ProcessDefinition::getId, ProcessDefinition::getVersionTag));
   }
 
   @Override
@@ -68,44 +80,106 @@ public List<WorkflowInstanceDomain> findAllByIdAndStatus(String id, StatusEnum s
 
   @Override
   public List<WorkflowInstanceDomain> findAllByIdAndStatusAndVersion(String id, StatusEnum status, String version) {
-    return queryByStatus(id, status, version,
-        () -> historyService.createHistoricProcessInstanceQuery().processDefinitionKey(id));
+    return query(id, status, version, () ->
+            historyService.createHistoricProcessInstanceQuery().processDefinitionKey(id));
   }
 
-  private List<WorkflowInstanceDomain> queryByStatus(String id, StatusEnum status, String version,
-      Supplier<HistoricProcessInstanceQuery> supplier) {
-    HistoricProcessInstanceQuery historicProcessInstanceQuery = supplier.get();
-
+  private List<WorkflowInstanceDomain> query(String id, StatusEnum status, String version,
+                                             Supplier<HistoricProcessInstanceQuery> supplier) {
     Map<String, String> processIdVersionTagMap = getProcessIdVersionMap(id, version);
+    List<HistoricProcessInstance> instances;
+
+    if (StringUtils.isBlank(version)) {
+      instances = queryByStatus(status, supplier.get());
+    } else {
+      instances = queryByStatusAndVersion(processIdVersionTagMap.keySet(), status, supplier.get());
+    }
+
+    return objectConverter.convertCollection(instances, processIdVersionTagMap, WorkflowInstanceDomain.class);
+  }
+
+  private List<HistoricProcessInstance> queryByStatus(StatusEnum status,
+                                                      HistoricProcessInstanceQuery historicProcessInstanceQuery) {
     List<HistoricProcessInstance> instances = new ArrayList<>();
     switch (status) {
       case COMPLETED:
         instances.addAll(historicProcessInstanceQuery.finished()
-            .orderByProcessInstanceStartTime()
-            .asc()
-            .list()
-            .stream()
-            .filter(
-                instance -> instance.getEndActivityId() != null && instance.getEndActivityId().startsWith("endEvent"))
-            .collect(Collectors.toList()));
+                .orderByProcessInstanceStartTime()
+                .asc()
+                .list()
+                .stream()
+                .filter(instance -> instance.getEndActivityId() != null
+                    && instance.getEndActivityId().startsWith("endEvent"))
+                .collect(Collectors.toList()));
         break;
+
       case FAILED:
         instances.addAll(historicProcessInstanceQuery.finished()
-            .orderByProcessInstanceStartTime()
-            .asc()
-            .list()
-            .stream()
-            .filter(
-                instance -> instance.getEndActivityId() != null && !instance.getEndActivityId().startsWith("endEvent"))
-            .collect(Collectors.toList()));
+                .orderByProcessInstanceStartTime()
+                .asc()
+                .list()
+                .stream()
+                .filter(instance -> instance.getEndActivityId() != null
+                    && !instance.getEndActivityId().startsWith("endEvent"))
+                .collect(Collectors.toList()));
         break;
+
       case PENDING:
         instances.addAll(historicProcessInstanceQuery.unfinished().orderByProcessInstanceStartTime().asc().list());
         break;
+
       default:
         break;
     }
 
-    return objectConverter.convertCollection(instances, processIdVersionTagMap, WorkflowInstanceDomain.class);
+    return instances;
+  }
+
+  private List<HistoricProcessInstance> queryByStatusAndVersion(
+      Set<String> processDefIds, StatusEnum status, HistoricProcessInstanceQuery historicProcessInstanceQuery) {
+    List<HistoricProcessInstance> instances = new ArrayList<>();
+    switch (status) {
+      case COMPLETED:
+        processDefIds.forEach(processDefinitionId ->
+                instances.addAll(historicProcessInstanceQuery.processDefinitionId(processDefinitionId)
+                        .finished()
+                        .orderByProcessInstanceStartTime()
+                        .asc()
+                        .list()
+                        .stream()
+                        .filter(
+                                instance -> instance.getEndActivityId() != null && instance.getEndActivityId()
+                                        .startsWith("endEvent"))
+                        .collect(Collectors.toList())));
+        break;
+
+      case FAILED:
+        processDefIds.forEach(processDefinitionId ->
+                instances.addAll(historicProcessInstanceQuery.processInstanceId(processDefinitionId)
+                        .finished()
+                        .orderByProcessInstanceStartTime()
+                        .asc()
+                        .list()
+                        .stream()
+                        .filter(
+                                instance -> instance.getEndActivityId() != null && !instance.getEndActivityId()
+                                        .startsWith("endEvent"))
+                        .collect(Collectors.toList())));
+        break;
+
+      case PENDING:
+        processDefIds.forEach(processDefinitionId ->
+                instances.addAll(historicProcessInstanceQuery.processDefinitionId(processDefinitionId)
+                        .unfinished()
+                        .orderByProcessInstanceStartTime()
+                        .asc()
+                        .list()));
+        break;
+
+      default:
+        break;
+    }
+
+    return instances;
   }
 }