Clean up WorkflowDefinitionResource code and add more tests for corne…

…r cases
NitorCreations · Dec 12, 2014 · 4c7231d · 4c7231d
1 parent d208038
commit 4c7231d
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 33 deletions.
diff --git a/...w-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResource.java b/...w-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResource.java
@@ -6,7 +6,6 @@
 
 import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -58,30 +57,22 @@ public WorkflowDefinitionResource(WorkflowDefinitionService workflowDefinitions,
   @ApiOperation(value = "List workflow definitions", response = ListWorkflowDefinitionResponse.class, responseContainer = "List")
   public List<ListWorkflowDefinitionResponse> listWorkflowDefinitions(@QueryParam("type") String[] types) {
     List<WorkflowDefinition<? extends WorkflowState>> definitions = workflowDefinitions.getWorkflowDefinitions();
-    Set<String> reqTypes = new LinkedHashSet<>(asList(types));
+    Set<String> reqTypes = new HashSet<>(asList(types));
     Set<String> foundTypes = new HashSet<>();
     List<ListWorkflowDefinitionResponse> response = new ArrayList<>();
     for (WorkflowDefinition<? extends WorkflowState> definition : definitions) {
-      if (!reqTypes.isEmpty() && !reqTypes.contains(definition.getType())) {
-        continue;
+      if (reqTypes.isEmpty() || reqTypes.contains(definition.getType())) {
+        foundTypes.add(definition.getType());
+        response.add(converter.convert(definition));
       }
-      foundTypes.add(definition.getType());
-      response.add(converter.convert(definition));
     }
-    if (reqTypes.isEmpty()) {
+    if (reqTypes.isEmpty() || foundTypes.size() < reqTypes.size()) {
+      reqTypes.removeAll(foundTypes);
       List<StoredWorkflowDefinition> storedDefinitions = workflowDefinitionDao.queryStoredWorkflowDefinitions(reqTypes);
       for (StoredWorkflowDefinition storedDefinition : storedDefinitions) {
-        if (foundTypes.contains(storedDefinition.type)) {
-          continue;
+        if (!foundTypes.contains(storedDefinition.type)) {
+          response.add(converter.convert(storedDefinition));
         }
-        response.add(converter.convert(storedDefinition));
-      }
-    } else if (!foundTypes.containsAll(reqTypes)) {
-      Set<String> missingTypes = new HashSet<>(reqTypes);
-      missingTypes.removeAll(foundTypes);
-      List<StoredWorkflowDefinition> storedDefinitions = workflowDefinitionDao.queryStoredWorkflowDefinitions(missingTypes);
-      for (StoredWorkflowDefinition storedDefinition : storedDefinitions) {
-        response.add(converter.convert(storedDefinition));
       }
     }
     sort(response);

diff --git a/...main/java/com/nitorcreations/nflow/rest/v1/converter/ListWorkflowDefinitionConverter.java b/...main/java/com/nitorcreations/nflow/rest/v1/converter/ListWorkflowDefinitionConverter.java
@@ -2,7 +2,6 @@
 
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -69,7 +68,7 @@ public ListWorkflowDefinitionResponse convert(StoredWorkflowDefinition storedDef
     List<State> states = new ArrayList<>();
     for (StoredWorkflowDefinition.State state : storedDefinition.states) {
       State tmp = new State(state.id, state.type, state.description);
-      tmp.transitions = new LinkedHashSet<>(state.transitions);
+      tmp.transitions.addAll(state.transitions);
       tmp.onFailure = state.onFailure;
       states.add(tmp);
     }

diff --git a/...st-api/src/test/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResourceTest.java b/...st-api/src/test/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResourceTest.java
@@ -3,15 +3,18 @@
 import static com.nitorcreations.Matchers.containsElements;
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyMap;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.anyCollectionOf;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
 
@@ -47,15 +50,19 @@ public class WorkflowDefinitionResourceTest {
   private WorkflowDefinitionStatisticsConverter statisticsConverter;
   @Captor
   private ArgumentCaptor<Collection<String>> stringList;
+  @Mock
+  private WorkflowDefinition<? extends WorkflowState> dummyDefinition;
+  @Mock
+  private ListWorkflowDefinitionResponse dummyResponse;
 
   private WorkflowDefinitionResource resource;
 
   @Before
   public void setup() {
-    @SuppressWarnings("unchecked")
-    WorkflowDefinition<? extends WorkflowState> def = mock(WorkflowDefinition.class);
-    when(def.getType()).thenReturn("dummy");
-    doReturn(asList(def)).when(workflowDefinitions).getWorkflowDefinitions();
+    when(dummyDefinition.getType()).thenReturn("dummy");
+    doReturn(asList(dummyDefinition)).when(workflowDefinitions).getWorkflowDefinitions();
+    when(converter.convert(dummyDefinition)).thenReturn(dummyResponse);
+    dummyResponse.type = "dummy";
     Map<String, StateExecutionStatistics> stats = emptyMap();
     when(workflowDefinitions.getStatistics("dummy", null, null, null, null)).thenReturn(stats);
     when(statisticsConverter.convert(stats)).thenReturn(new WorkflowDefinitionStatisticsResponse());
@@ -66,6 +73,7 @@ public void setup() {
   public void listWorkflowDefinitionsFindsExistingDefinition() {
     Collection<ListWorkflowDefinitionResponse> ret = resource.listWorkflowDefinitions(new String[] { "dummy" });
     assertThat(ret.size(), is(1));
+    verify(workflowDefinitionDao, never()).queryStoredWorkflowDefinitions(anyCollectionOf(String.class));
   }
 
   @Test
@@ -86,16 +94,31 @@ public void listWorkflowDefinitionsFindsExistingDefinitionWithoutArguments() {
 
   @Test
   public void listWorkflowDefinitionsFindsExistingAndStoredDefinitionsWithoutArguments() {
-    ArrayList<StoredWorkflowDefinition> storedDefinitions = new ArrayList<>();
-    StoredWorkflowDefinition storedDefinition = mock(StoredWorkflowDefinition.class);
-    storedDefinitions.add(storedDefinition);
-    when(workflowDefinitionDao.queryStoredWorkflowDefinitions(anyCollectionOf(String.class))).thenReturn(storedDefinitions);
-    ListWorkflowDefinitionResponse storedDefinitionResponse = mock(ListWorkflowDefinitionResponse.class);
-    when(converter.convert(storedDefinition)).thenReturn(storedDefinitionResponse);
+    StoredWorkflowDefinition storedDefinitionDummy = mock(StoredWorkflowDefinition.class);
+    StoredWorkflowDefinition storedDefinitionNew = mock(StoredWorkflowDefinition.class);
+    when(workflowDefinitionDao.queryStoredWorkflowDefinitions(anyCollectionOf(String.class))).thenReturn(
+        asList(storedDefinitionDummy, storedDefinitionNew));
+    ListWorkflowDefinitionResponse storedResponseDummy = mock(ListWorkflowDefinitionResponse.class, "dbDummy");
+    ListWorkflowDefinitionResponse storedResponseNew = mock(ListWorkflowDefinitionResponse.class, "dbNew");
+    when(converter.convert(storedDefinitionDummy)).thenReturn(storedResponseDummy);
+    storedDefinitionDummy.type = "dummy";
+    when(converter.convert(storedDefinitionNew)).thenReturn(storedResponseNew);
+    storedDefinitionNew.type = "new";
     Collection<ListWorkflowDefinitionResponse> ret = resource.listWorkflowDefinitions(new String[] {});
-    assertThat(ret.size(), is(2));
-    verify(workflowDefinitionDao).queryStoredWorkflowDefinitions(stringList.capture());
-    assertThat(stringList.getValue().size(), is(0));
+    assertThat(ret, hasItems(storedResponseNew, dummyResponse));
+    assertThat(ret, not(hasItem(storedResponseDummy)));
+  }
+
+  @Test
+  public void listWorkflowDefinitionsFindsExistingAndStoredDefinitionsWithDbType() {
+    StoredWorkflowDefinition storedDefinitionNew = mock(StoredWorkflowDefinition.class);
+    when(workflowDefinitionDao.queryStoredWorkflowDefinitions(anyCollectionOf(String.class))).thenReturn(
+        asList(storedDefinitionNew));
+    ListWorkflowDefinitionResponse storedResponseNew = mock(ListWorkflowDefinitionResponse.class, "dbNew");
+    when(converter.convert(storedDefinitionNew)).thenReturn(storedResponseNew);
+    storedDefinitionNew.type = "new";
+    Collection<ListWorkflowDefinitionResponse> ret = resource.listWorkflowDefinitions(new String[] { "new" });
+    assertThat(ret, hasItems(storedResponseNew));
   }
 
   @Test