From 915058996ea2b925dd4d9d14c590429c78f1849e Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Thu, 21 Jan 2021 13:35:57 +0200 Subject: [PATCH] Make nFlow REST object mapper fail on trailing tokens --- .../nflow/rest/config/RestConfiguration.java | 2 ++ .../ListWorkflowInstanceConverter.java | 21 +++++------ .../io/nflow/tests/StateVariablesTest.java | 36 +++++++++++++++---- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/nflow-rest-api-common/src/main/java/io/nflow/rest/config/RestConfiguration.java b/nflow-rest-api-common/src/main/java/io/nflow/rest/config/RestConfiguration.java index cf683fab7..008ec19b2 100644 --- a/nflow-rest-api-common/src/main/java/io/nflow/rest/config/RestConfiguration.java +++ b/nflow-rest-api-common/src/main/java/io/nflow/rest/config/RestConfiguration.java @@ -1,5 +1,6 @@ package io.nflow.rest.config; +import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_TRAILING_TOKENS; import static com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATES_AS_TIMESTAMPS; import javax.inject.Named; @@ -28,6 +29,7 @@ public class RestConfiguration { public ObjectMapper nflowRestObjectMapper(@NFlow ObjectMapper nflowObjectMapper) { ObjectMapper restObjectMapper = nflowObjectMapper.copy(); restObjectMapper.configure(WRITE_DATES_AS_TIMESTAMPS, false); + restObjectMapper.enable(FAIL_ON_TRAILING_TOKENS); return restObjectMapper; } } diff --git a/nflow-rest-api-common/src/main/java/io/nflow/rest/v1/converter/ListWorkflowInstanceConverter.java b/nflow-rest-api-common/src/main/java/io/nflow/rest/v1/converter/ListWorkflowInstanceConverter.java index adfb913c6..4abda84fe 100644 --- a/nflow-rest-api-common/src/main/java/io/nflow/rest/v1/converter/ListWorkflowInstanceConverter.java +++ b/nflow-rest-api-common/src/main/java/io/nflow/rest/v1/converter/ListWorkflowInstanceConverter.java @@ -1,10 +1,9 @@ package io.nflow.rest.v1.converter; +import static java.util.stream.Collectors.toMap; import static org.springframework.util.CollectionUtils.isEmpty; -import java.io.IOException; import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -15,6 +14,7 @@ import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.TextNode; @@ -75,21 +75,16 @@ private Map stateVariablesToJson(Map stateVariab if (isEmpty(stateVariables)) { return null; } - Map jsonStateVariables = new LinkedHashMap<>(stateVariables.size() * 2); - for (Entry entry : stateVariables.entrySet()) { - String key = entry.getKey(); - jsonStateVariables.put(key, stringToJson(key, entry.getValue())); - } - return jsonStateVariables; + return stateVariables.entrySet().stream().collect(toMap(Entry::getKey, this::stringToJson)); } - private JsonNode stringToJson(String key, String value) { + private JsonNode stringToJson(Entry entry) { try { - return nflowRestObjectMapper.readTree(value); - } catch (IOException e) { - logger.debug("Failed to parse state variable {} value as JSON, returning value as unparsed string: {}: {}", key, + return nflowRestObjectMapper.readTree(entry.getValue()); + } catch (JsonProcessingException e) { + logger.debug("Failed to parse state variable {} value as JSON, returning value as unparsed string: {}: {}", entry.getKey(), e.getClass().getSimpleName(), e.getMessage()); - return new TextNode(value); + return new TextNode(entry.getValue()); } } } diff --git a/nflow-tests/src/test/java/io/nflow/tests/StateVariablesTest.java b/nflow-tests/src/test/java/io/nflow/tests/StateVariablesTest.java index 07fc66e6b..1e968c320 100644 --- a/nflow-tests/src/test/java/io/nflow/tests/StateVariablesTest.java +++ b/nflow-tests/src/test/java/io/nflow/tests/StateVariablesTest.java @@ -1,8 +1,11 @@ package io.nflow.tests; +import static io.nflow.tests.demo.workflow.SimpleWorkflow.SIMPLE_WORKFLOW_TYPE; import static io.nflow.tests.demo.workflow.StateWorkflow.STATEVAR_QUERYTEST; +import static io.nflow.tests.demo.workflow.StateWorkflow.STATE_WORKFLOW_TYPE; import static java.time.Duration.ofSeconds; import static java.util.Collections.singletonMap; +import static java.util.UUID.randomUUID; import static javax.ws.rs.core.MediaType.APPLICATION_JSON_TYPE; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static org.apache.commons.lang3.StringUtils.repeat; @@ -17,7 +20,6 @@ import java.io.IOException; import java.util.List; import java.util.Map; -import java.util.UUID; import javax.ws.rs.core.Response; @@ -30,12 +32,14 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import io.nflow.engine.workflow.curated.CronWorkflow; import io.nflow.rest.v1.msg.Action; import io.nflow.rest.v1.msg.CreateWorkflowInstanceRequest; import io.nflow.rest.v1.msg.CreateWorkflowInstanceResponse; import io.nflow.rest.v1.msg.ErrorResponse; import io.nflow.rest.v1.msg.ListWorkflowInstanceResponse; import io.nflow.rest.v1.msg.UpdateWorkflowInstanceRequest; +import io.nflow.tests.demo.workflow.SimpleWorkflow; import io.nflow.tests.demo.workflow.StateWorkflow; import io.nflow.tests.demo.workflow.StateWorkflow.State; import io.nflow.tests.extension.NflowServerConfig; @@ -44,7 +48,9 @@ public class StateVariablesTest extends AbstractNflowTest { public static NflowServerConfig server = new NflowServerConfig.Builder() - .prop("nflow.workflow.state.variable.value.length", "8000").springContextClass(TestConfiguration.class).build(); + .prop("nflow.workflow.state.variable.value.length", "8000") + .springContextClass(TestConfiguration.class) + .build(); private static CreateWorkflowInstanceRequest createRequest; private static CreateWorkflowInstanceResponse createResponse; @@ -63,8 +69,8 @@ public StateWorkflow stateWorkflow() { @Order(1) public void createStateWorkflow() throws JsonProcessingException, IOException { createRequest = new CreateWorkflowInstanceRequest(); - createRequest.type = "stateWorkflow"; - createRequest.externalId = UUID.randomUUID().toString(); + createRequest.type = STATE_WORKFLOW_TYPE; + createRequest.externalId = randomUUID().toString(); createRequest.stateVariables.put("requestData", new ObjectMapper().readTree("{\"test\":5}")); createResponse = assertTimeoutPreemptively(ofSeconds(5), () -> createWorkflowInstance(createRequest)); assertThat(createResponse.id, notNullValue()); @@ -119,8 +125,8 @@ public void updateWorkflowWithTooLongStateVariableValueReturnsBadRequest() { @Order(5) public void insertWorkflowWithTooLongStateVariableValueReturnsBadRequest() { createRequest = new CreateWorkflowInstanceRequest(); - createRequest.type = "stateWorkflow"; - createRequest.externalId = UUID.randomUUID().toString(); + createRequest.type = STATE_WORKFLOW_TYPE; + createRequest.externalId = randomUUID().toString(); createRequest.stateVariables.put("requestData", repeat('a', 8001)); try (Response response = getInstanceResource().put(createRequest)) { @@ -152,6 +158,24 @@ public void queryWorkflowInstancesFindsInstanceWithCurrentStateVariableValue() { assertThat(instances.length, is(1)); } + @Test + @Order(8) + public void insertCronStateVariable() { + createRequest = new CreateWorkflowInstanceRequest(); + createRequest.type = SIMPLE_WORKFLOW_TYPE; + createRequest.externalId = randomUUID().toString(); + String cronSchedule = "0 0 * * * *"; + createRequest.stateVariables.put(CronWorkflow.VAR_SCHEDULE, cronSchedule); + + createResponse = assertTimeoutPreemptively(ofSeconds(5), () -> createWorkflowInstance(createRequest)); + assertThat(createResponse.id, notNullValue()); + + ListWorkflowInstanceResponse listResponse = getWorkflowInstanceWithTimeout(createResponse.id, + SimpleWorkflow.State.done.name(), ofSeconds(5)); + assertThat(listResponse.stateVariables.size(), is(1)); + assertThat(listResponse.stateVariables.get(CronWorkflow.VAR_SCHEDULE), is(cronSchedule)); + } + private void assertState(List actions, int index, State state, String variable1, String variable2) { Action action = actions.get(index); assertEquals(state.name(), action.state);