From 4c7231d8a552fc09fe9086547fdb524102be381e Mon Sep 17 00:00:00 2001 From: Mikko Tiihonen Date: Fri, 12 Dec 2014 13:01:58 +0200 Subject: [PATCH] Clean up WorkflowDefinitionResource code and add more tests for corner cases --- .../rest/v1/WorkflowDefinitionResource.java | 25 +++------ .../ListWorkflowDefinitionConverter.java | 3 +- .../v1/WorkflowDefinitionResourceTest.java | 51 ++++++++++++++----- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResource.java b/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResource.java index ce50e712d..e4e168b9d 100644 --- a/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResource.java +++ b/nflow-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 listWorkflowDefinitions(@QueryParam("type") String[] types) { List> definitions = workflowDefinitions.getWorkflowDefinitions(); - Set reqTypes = new LinkedHashSet<>(asList(types)); + Set reqTypes = new HashSet<>(asList(types)); Set foundTypes = new HashSet<>(); List response = new ArrayList<>(); for (WorkflowDefinition 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 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 missingTypes = new HashSet<>(reqTypes); - missingTypes.removeAll(foundTypes); - List storedDefinitions = workflowDefinitionDao.queryStoredWorkflowDefinitions(missingTypes); - for (StoredWorkflowDefinition storedDefinition : storedDefinitions) { - response.add(converter.convert(storedDefinition)); } } sort(response); diff --git a/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/converter/ListWorkflowDefinitionConverter.java b/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/converter/ListWorkflowDefinitionConverter.java index bf253134e..3b64ea5e5 100644 --- a/nflow-rest-api/src/main/java/com/nitorcreations/nflow/rest/v1/converter/ListWorkflowDefinitionConverter.java +++ b/nflow-rest-api/src/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 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/nflow-rest-api/src/test/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResourceTest.java b/nflow-rest-api/src/test/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResourceTest.java index 5eae6acd0..99d30bdd9 100644 --- a/nflow-rest-api/src/test/java/com/nitorcreations/nflow/rest/v1/WorkflowDefinitionResourceTest.java +++ b/nflow-rest-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> stringList; + @Mock + private WorkflowDefinition dummyDefinition; + @Mock + private ListWorkflowDefinitionResponse dummyResponse; private WorkflowDefinitionResource resource; @Before public void setup() { - @SuppressWarnings("unchecked") - WorkflowDefinition 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 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 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 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 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 ret = resource.listWorkflowDefinitions(new String[] { "new" }); + assertThat(ret, hasItems(storedResponseNew)); } @Test