From e42717adac3dc7914c5cda38475ad944658e9731 Mon Sep 17 00:00:00 2001 From: Tomasz Michalak Date: Mon, 23 Mar 2020 11:28:45 +0100 Subject: [PATCH 01/10] #134 Fragment Event Consumer refactor. --- .../api/FragmentExecutionLogConsumer.java | 2 +- .../consumer/EventLogConverterTest.java | 4 +--- .../consumer/MetadataConverterTest.java | 22 +++++++++---------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java index 9d49f2e5..1dfd5cfc 100644 --- a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java +++ b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java @@ -28,7 +28,7 @@ public interface FragmentExecutionLogConsumer { /** * Gets a list of processed and unprocessed fragments (execution logs). * - * @param request - original request data + * @param request - original request data * @param executions - list of fragment execution logs */ void accept(ClientRequest request, List executions); diff --git a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/EventLogConverterTest.java b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/EventLogConverterTest.java index 4397230a..1c6d2055 100644 --- a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/EventLogConverterTest.java +++ b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/EventLogConverterTest.java @@ -21,14 +21,13 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import io.knotx.fragments.api.Fragment; -import io.knotx.fragments.engine.api.EventLogEntry; import io.knotx.fragments.api.FragmentResult; +import io.knotx.fragments.engine.api.EventLogEntry; import io.knotx.fragments.handler.consumer.api.model.LoggedNodeStatus; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import java.util.Arrays; import java.util.Collections; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -45,7 +44,6 @@ void fillWithEmptyLog() { NodeExecutionData result = tested.getExecutionData(NODE_ID); assertEquals(LoggedNodeStatus.UNPROCESSED, result.getStatus()); - assertEquals(LoggedNodeStatus.UNPROCESSED, result.getStatus()); } @Test diff --git a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java index cc74bb19..3e9f2d4d 100644 --- a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java +++ b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java @@ -59,7 +59,7 @@ void shouldProduceEmptyJsonWhenNoMetadataProvided() { JsonObject output = tested.getExecutionLog().toJson(); - assertJsonEquals(new JsonObject(), output); + KnotxAssertions.assertJsonEquals(new JsonObject(), output); } @Test @@ -71,7 +71,7 @@ void shouldProduceOnlyProcessingInfoWhenNoMetadataProvided() { JsonObject expected = jsonForNotDescribedNode(ROOT_NODE); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -83,7 +83,7 @@ void shouldProduceCorrectJsonForOneNodeMetadata() { JsonObject expected = jsonForNode(ROOT_NODE, "custom"); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -95,7 +95,7 @@ void shouldProduceCorrectJsonForOneActionNodeMetadata() { JsonObject expected = jsonForActionNode(ROOT_NODE); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -112,7 +112,7 @@ void shouldProduceCorrectJsonForTwoNodesWithTransition() { .put("on", new JsonObject() .put(SUCCESS_TRANSITION, jsonForNode("node-A", "factory-A"))); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -142,7 +142,7 @@ void shouldProduceCorrectJsonForMissingNodeCase() { .put("transition", ERROR_TRANSITION) .put("invocations", new JsonArray())); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -165,7 +165,7 @@ void shouldProduceCorrectJsonForSuccessTransitionWithoutNextNode() { .put("transition", SUCCESS_TRANSITION) .put("invocations", wrap(simpleNodeLog()))); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -189,7 +189,7 @@ void shouldProduceCorrectJsonForCompositeNodeWithSimpleNodes() { jsonForNode("node-C", "factory-C") ))); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -213,7 +213,7 @@ void shouldProduceCorrectJsonForCompositeNodeWithSimpleNodesNotAllDescribed() { jsonForNotDescribedNode("node-C") ))); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -260,7 +260,7 @@ void shouldProduceCorrectJsonForComplexGraphWithFullMetadata() { ))) )); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } @Test @@ -342,7 +342,7 @@ void shouldProduceCorrectJsonForComplexGraphWithFullMetadataWithEventLog() { ))) )); - assertJsonEquals(expected, output); + KnotxAssertions.assertJsonEquals(expected, output); } private EventLog createEventLog(EventLogEntry... entries) { From 928965e542a54b7423f92926dee48b02ed0683f4 Mon Sep 17 00:00:00 2001 From: Oskar Jerzyk Date: Thu, 26 Mar 2020 09:51:50 +0100 Subject: [PATCH 02/10] JSON consumer implemented --- .../FragmentExecutionLogConsumerFactory.java | 4 +- .../html/FragmentHtmlBodyWriterFactory.java | 14 +- .../JsonFragmentsHandlerConsumerFactory.java | 73 ++++++- ...onFragmentsHandlerConsumerFactoryTest.java | 192 +++++++++++++++++- 4 files changed, 267 insertions(+), 16 deletions(-) diff --git a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumerFactory.java b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumerFactory.java index 4c5a2b6b..7823ec25 100644 --- a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumerFactory.java +++ b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumerFactory.java @@ -18,8 +18,8 @@ import io.vertx.core.json.JsonObject; /** - * The {@link FragmentExecutionLogConsumer} factory interface that enables dynamic implementation binding - * using SPI. + * The {@link FragmentExecutionLogConsumer} factory interface that enables dynamic implementation + * binding using SPI. */ public interface FragmentExecutionLogConsumerFactory { diff --git a/handler/consumer/html/src/main/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactory.java b/handler/consumer/html/src/main/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactory.java index 5e6b919e..631d91bc 100644 --- a/handler/consumer/html/src/main/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactory.java +++ b/handler/consumer/html/src/main/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactory.java @@ -20,7 +20,6 @@ import io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory; import io.knotx.fragments.handler.consumer.api.model.FragmentExecutionLog; import io.knotx.server.api.context.ClientRequest; -import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import java.util.Collections; import java.util.List; @@ -96,14 +95,11 @@ private boolean isSupported(FragmentExecutionLog executionData) { } private Set getSupportedTypes(JsonObject config) { - if (config.containsKey(FRAGMENT_TYPES_OPTIONS)) { - JsonArray fragmentTypes = config.getJsonArray(FRAGMENT_TYPES_OPTIONS); - return StreamSupport.stream(fragmentTypes.spliterator(), false) - .map(Object::toString) - .collect(Collectors.toSet()); - } else { - return Collections.emptySet(); - } + return Optional.ofNullable(config.getJsonArray(FRAGMENT_TYPES_OPTIONS)) + .map(fragmentTypes -> StreamSupport.stream(fragmentTypes.spliterator(), false) + .map(Object::toString) + .collect(Collectors.toSet())) + .orElse(Collections.emptySet()); } private String getConditionHeader(JsonObject config) { diff --git a/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java b/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java index ff864940..9fca9bcd 100644 --- a/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java +++ b/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java @@ -15,20 +15,87 @@ */ package io.knotx.fragments.handler.consumer.json; +import static java.lang.Boolean.FALSE; + import io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumer; import io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory; +import io.knotx.fragments.handler.consumer.api.model.FragmentExecutionLog; +import io.knotx.server.api.context.ClientRequest; import io.vertx.core.json.JsonObject; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; -// TODO fix #134 public class JsonFragmentsHandlerConsumerFactory implements FragmentExecutionLogConsumerFactory { + private static final String PARAM_OPTION = "param"; + private static final String KNOTX_FRAGMENT = "_knotx_fragment"; + static final String FRAGMENT_TYPES_OPTIONS = "fragmentTypes"; + static final String HEADER_OPTION = "header"; + static final String CONDITION_OPTION = "condition"; + @Override public String getName() { - return "json"; + return "fragmentJsonBodyWriter"; } @Override public FragmentExecutionLogConsumer create(JsonObject config) { - return null; + return new FragmentExecutionLogConsumer() { + + private Set supportedTypes = getSupportedTypes(config); + private String requestHeader = getConditionHeader(config); + private String requestParam = getConditionParam(config); + + @Override + public void accept(ClientRequest request, List executions) { + if (containsHeader(request) || containsParam(request)) { + executions.stream() + .filter(this::isSupported) + .forEach(this::appendExecutionDataToFragmentBody); + } + } + + private boolean isSupported(FragmentExecutionLog executionData) { + return supportedTypes.contains(executionData.getFragment().getType()); + } + + private boolean containsHeader(ClientRequest request) { + return Optional.ofNullable(requestHeader) + .map(header -> request.getHeaders().contains(header)) + .orElse(FALSE); + } + + private boolean containsParam(ClientRequest request) { + return Optional.ofNullable(requestParam) + .map(param -> request.getParams().contains(param)) + .orElse(FALSE); + } + + private void appendExecutionDataToFragmentBody(FragmentExecutionLog executionData) { + JsonObject fragmentBody = new JsonObject().put(KNOTX_FRAGMENT, executionData.toJson()) + .mergeIn(new JsonObject(executionData.getFragment().getBody())); + executionData.getFragment().setBody(fragmentBody.toString()); + } + }; + } + + private Set getSupportedTypes(JsonObject config) { + return Optional.ofNullable(config.getJsonArray(FRAGMENT_TYPES_OPTIONS)) + .map(fragmentTypes -> StreamSupport.stream(fragmentTypes.spliterator(), false) + .map(Object::toString) + .collect(Collectors.toSet())) + .orElse(Collections.emptySet()); + } + + private String getConditionHeader(JsonObject config) { + return config.getJsonObject(CONDITION_OPTION, new JsonObject()).getString(HEADER_OPTION); + } + + private String getConditionParam(JsonObject config) { + return config.getJsonObject(CONDITION_OPTION, new JsonObject()).getString(PARAM_OPTION); } } diff --git a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java index f8dca48b..6987fcea 100644 --- a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java +++ b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java @@ -15,9 +15,197 @@ */ package io.knotx.fragments.handler.consumer.json; -import static org.junit.jupiter.api.Assertions.*; +import static io.knotx.fragments.engine.api.EventLogEntry.NodeStatus.UNPROCESSED; +import static io.knotx.fragments.handler.consumer.json.JsonFragmentsHandlerConsumerFactory.CONDITION_OPTION; +import static io.knotx.fragments.handler.consumer.json.JsonFragmentsHandlerConsumerFactory.FRAGMENT_TYPES_OPTIONS; +import static io.knotx.fragments.handler.consumer.json.JsonFragmentsHandlerConsumerFactory.HEADER_OPTION; +import static io.knotx.junit5.assertions.KnotxAssertions.assertJsonEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.google.common.collect.ImmutableList; +import io.knotx.fragments.api.Fragment; +import io.knotx.fragments.engine.api.FragmentEvent; +import io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumer; +import io.knotx.fragments.handler.consumer.api.model.FragmentExecutionLog; +import io.knotx.server.api.context.ClientRequest; +import io.vertx.core.json.JsonArray; +import io.vertx.core.json.JsonObject; +import io.vertx.reactivex.core.MultiMap; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; -// TODO fix #134 class JsonFragmentsHandlerConsumerFactoryTest { + private static final String EXPECTED_FRAGMENT_TYPE = "snippet"; + private static final String EXPECTED_HEADER = "x-knotx-debug"; + private static final String EXPECTED_PARAM = "debug"; + private static final String PARAM_OPTION = "param"; + private static final String HTML_TYPE = "html"; + private static final String FRAGMENT_BODY_JSON = "{\"user\": \"admin\"}"; + private static final String FRAGMENT_BODY_HTML = "\"
body
\""; + private static final String USER_KEY = "user"; + private static final String KNOTX_FRAGMENT_KEY = "_knotx_fragment"; + + @Test + @DisplayName("Fragment body should not be modified when condition not configured") + void expectFragmentBodyNotModifiedWhenConditionNotConfigured() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); + tested.accept(new ClientRequest() + .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertEquals(copy, original); + } + + @Test + @DisplayName("Fragment body should not be modified when supported fragments do not contain fragment type") + void expectFragmentBodyNotModifiedWhenSupportedFragmentsDoNotContainFragmentType() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(HTML_TYPE, new JsonObject(), FRAGMENT_BODY_HTML)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(CONDITION_OPTION, new JsonObject().put(PARAM_OPTION, EXPECTED_PARAM))); + tested.accept(new ClientRequest() + .setParams(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertEquals(copy, original); + } + + @Test + @DisplayName("Fragment should not be modified when supported fragments do not contain fragment type") + void expectFragmentNotModifiedWhenSupportedFragmentsDoNotContainFragmentType() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(HTML_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(FRAGMENT_TYPES_OPTIONS, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)) + .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); + tested.accept(new ClientRequest() + .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + //then + assertEquals(copy, original); + } + + @Test + @DisplayName("Fragment should be modified when header condition and supported types configured") + void expectFragmentModifiedWhenHederConditionAndSupportedTypedConfigured() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(FRAGMENT_TYPES_OPTIONS, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)) + .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); + tested.accept(new ClientRequest() + .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertNotEquals(copy, original); + } + + @Test + @DisplayName("Fragment should be modified when param condition and supported types configured") + void expectFragmentModifiedWhenParamConditionAndSupportedTypesConfigured() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(FRAGMENT_TYPES_OPTIONS, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)) + .put(CONDITION_OPTION, new JsonObject().put(PARAM_OPTION, EXPECTED_PARAM))); + tested.accept(new ClientRequest() + .setParams(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_PARAM, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertNotEquals(copy, original); + } + + @Test + @DisplayName("Execution log entry should be properly merged into existing fragment body") + void expectExecutionLogEntryProperlyMergedIntoFragmentBody() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(FRAGMENT_TYPES_OPTIONS, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)) + .put(CONDITION_OPTION, new JsonObject().put(PARAM_OPTION, EXPECTED_PARAM))); + tested.accept(new ClientRequest() + .setParams(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_PARAM, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertNotEquals(copy, original); + JsonObject fragmentBody = new JsonObject(original.getFragment().getBody()); + assertTrue(fragmentBody.containsKey(USER_KEY)); + assertTrue(fragmentBody.containsKey(KNOTX_FRAGMENT_KEY)); + } + + @Test + @DisplayName("Execution log entry should contain proper fragment details") + void expectFragmentDetailsInExecutionLogEntry() { + //given + FragmentEvent original = new FragmentEvent( + new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); + FragmentEvent copy = new FragmentEvent(original.toJson()); + + JsonObject expectedLog = new JsonObject() + .put("startTime", 0) + .put("finishTime", 0) + .put("status", UNPROCESSED) + .put("fragment", new JsonObject() + .put("id", original.getFragment().getId()) + .put("type", "snippet") + .put("body", FRAGMENT_BODY_JSON)) + .put("graph", new JsonObject()); + + //when + FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() + .create(new JsonObject() + .put(FRAGMENT_TYPES_OPTIONS, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)) + .put(CONDITION_OPTION, new JsonObject().put(PARAM_OPTION, EXPECTED_PARAM))); + tested.accept(new ClientRequest() + .setParams(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_PARAM, "true")), + ImmutableList.of(FragmentExecutionLog.newInstance(original))); + + //then + assertNotEquals(copy, original); + JsonObject fragmentBody = new JsonObject(original.getFragment().getBody()); + assertJsonEquals(expectedLog, fragmentBody.getJsonObject(KNOTX_FRAGMENT_KEY)); + assertEquals(new JsonObject(FRAGMENT_BODY_JSON).getString(USER_KEY), + fragmentBody.getString(USER_KEY)); + } } \ No newline at end of file From 82870be407b025c7ba135468486e2ba01bc3a32b Mon Sep 17 00:00:00 2001 From: Oskar Jerzyk Date: Tue, 31 Mar 2020 16:03:08 +0200 Subject: [PATCH 03/10] META-INF moved --- ...ments.handler.consumer.api.FragmentExecutionLogConsumerFactory | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename handler/consumer/json/src/{ => main}/resources/META-INF/services/io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory (100%) diff --git a/handler/consumer/json/src/resources/META-INF/services/io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory b/handler/consumer/json/src/main/resources/META-INF/services/io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory similarity index 100% rename from handler/consumer/json/src/resources/META-INF/services/io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory rename to handler/consumer/json/src/main/resources/META-INF/services/io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory From fff058eb4f3b2881b988d6b0e19d0772037aaa7b Mon Sep 17 00:00:00 2001 From: Oskar Jerzyk Date: Wed, 1 Apr 2020 09:44:04 +0200 Subject: [PATCH 04/10] CR fixes applied --- .../api/FragmentExecutionLogConsumer.java | 2 +- handler/consumer/json/build.gradle.kts | 1 + .../JsonFragmentsHandlerConsumerFactory.java | 6 +++ ...onFragmentsHandlerConsumerFactoryTest.java | 50 ++++++++----------- .../consumer/MetadataConverterTest.java | 23 ++++----- 5 files changed, 40 insertions(+), 42 deletions(-) diff --git a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java index 1dfd5cfc..9d49f2e5 100644 --- a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java +++ b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/FragmentExecutionLogConsumer.java @@ -28,7 +28,7 @@ public interface FragmentExecutionLogConsumer { /** * Gets a list of processed and unprocessed fragments (execution logs). * - * @param request - original request data + * @param request - original request data * @param executions - list of fragment execution logs */ void accept(ClientRequest request, List executions); diff --git a/handler/consumer/json/build.gradle.kts b/handler/consumer/json/build.gradle.kts index 6d9a71e0..44beb075 100644 --- a/handler/consumer/json/build.gradle.kts +++ b/handler/consumer/json/build.gradle.kts @@ -37,6 +37,7 @@ dependencies { implementation(group = "org.apache.commons", name = "commons-lang3") implementation(group = "com.google.guava", name = "guava") + testImplementation("org.junit.jupiter:junit-jupiter-params") testImplementation(group = "org.mockito", name = "mockito-core") testImplementation(group = "org.mockito", name = "mockito-junit-jupiter") } diff --git a/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java b/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java index 9fca9bcd..42e8db4e 100644 --- a/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java +++ b/handler/consumer/json/src/main/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactory.java @@ -21,6 +21,7 @@ import io.knotx.fragments.handler.consumer.api.FragmentExecutionLogConsumerFactory; import io.knotx.fragments.handler.consumer.api.model.FragmentExecutionLog; import io.knotx.server.api.context.ClientRequest; +import io.vertx.core.json.DecodeException; import io.vertx.core.json.JsonObject; import java.util.Collections; import java.util.List; @@ -60,6 +61,11 @@ public void accept(ClientRequest request, List executions) } private boolean isSupported(FragmentExecutionLog executionData) { + try { + new JsonObject(executionData.getFragment().getBody()); + } catch (DecodeException e) { + return false; + } return supportedTypes.contains(executionData.getFragment().getType()); } diff --git a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java index 6987fcea..f4534e59 100644 --- a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java +++ b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java @@ -33,8 +33,12 @@ import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import io.vertx.reactivex.core.MultiMap; +import java.util.stream.Stream; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class JsonFragmentsHandlerConsumerFactoryTest { @@ -48,43 +52,31 @@ class JsonFragmentsHandlerConsumerFactoryTest { private static final String USER_KEY = "user"; private static final String KNOTX_FRAGMENT_KEY = "_knotx_fragment"; - @Test - @DisplayName("Fragment body should not be modified when condition not configured") - void expectFragmentBodyNotModifiedWhenConditionNotConfigured() { - //given - FragmentEvent original = new FragmentEvent( - new Fragment(EXPECTED_FRAGMENT_TYPE, new JsonObject(), FRAGMENT_BODY_JSON)); - FragmentEvent copy = new FragmentEvent(original.toJson()); - - //when - FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() - .create(new JsonObject() - .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); - tested.accept(new ClientRequest() - .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), - ImmutableList.of(FragmentExecutionLog.newInstance(original))); - - //then - assertEquals(copy, original); + private static Stream provideFragmentConsumerConfiguration() { + return Stream.of( + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_JSON), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_HTML), + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_HTML), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_JSON) + ); } - @Test - @DisplayName("Fragment body should not be modified when supported fragments do not contain fragment type") - void expectFragmentBodyNotModifiedWhenSupportedFragmentsDoNotContainFragmentType() { - //given + @ParameterizedTest + @MethodSource("provideFragmentConsumerConfiguration") + @DisplayName("Fragment body should not be modified when invalid configuration provided") + void fragmentBodyShouldNotBeModifiedWhenInvalidConfigurationProvided(String fragmentType, + String fragmentBody) { FragmentEvent original = new FragmentEvent( - new Fragment(HTML_TYPE, new JsonObject(), FRAGMENT_BODY_HTML)); + new Fragment(fragmentType, new JsonObject(), fragmentBody)); FragmentEvent copy = new FragmentEvent(original.toJson()); - //when FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() .create(new JsonObject() - .put(CONDITION_OPTION, new JsonObject().put(PARAM_OPTION, EXPECTED_PARAM))); + .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); tested.accept(new ClientRequest() - .setParams(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), + .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), ImmutableList.of(FragmentExecutionLog.newInstance(original))); - //then assertEquals(copy, original); } @@ -109,7 +101,7 @@ void expectFragmentNotModifiedWhenSupportedFragmentsDoNotContainFragmentType() { } @Test - @DisplayName("Fragment should be modified when header condition and supported types configured") + @DisplayName("Fragment should be modified when header condition and fragment type match") void expectFragmentModifiedWhenHederConditionAndSupportedTypedConfigured() { //given FragmentEvent original = new FragmentEvent( @@ -130,7 +122,7 @@ void expectFragmentModifiedWhenHederConditionAndSupportedTypedConfigured() { } @Test - @DisplayName("Fragment should be modified when param condition and supported types configured") + @DisplayName("Fragment should be modified when param condition and fragment type match") void expectFragmentModifiedWhenParamConditionAndSupportedTypesConfigured() { //given FragmentEvent original = new FragmentEvent( diff --git a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java index 3e9f2d4d..e196618e 100644 --- a/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java +++ b/handler/core/src/test/java/io/knotx/fragments/handler/consumer/MetadataConverterTest.java @@ -31,7 +31,6 @@ import io.knotx.fragments.handler.api.metadata.TaskMetadata; import io.knotx.fragments.handler.consumer.api.model.GraphNodeOperationLog; import io.knotx.fragments.handler.consumer.api.model.LoggedNodeStatus; -import io.knotx.junit5.assertions.KnotxAssertions; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import java.util.Arrays; @@ -59,7 +58,7 @@ void shouldProduceEmptyJsonWhenNoMetadataProvided() { JsonObject output = tested.getExecutionLog().toJson(); - KnotxAssertions.assertJsonEquals(new JsonObject(), output); + assertJsonEquals(new JsonObject(), output); } @Test @@ -71,7 +70,7 @@ void shouldProduceOnlyProcessingInfoWhenNoMetadataProvided() { JsonObject expected = jsonForNotDescribedNode(ROOT_NODE); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -83,7 +82,7 @@ void shouldProduceCorrectJsonForOneNodeMetadata() { JsonObject expected = jsonForNode(ROOT_NODE, "custom"); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -95,7 +94,7 @@ void shouldProduceCorrectJsonForOneActionNodeMetadata() { JsonObject expected = jsonForActionNode(ROOT_NODE); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -112,7 +111,7 @@ void shouldProduceCorrectJsonForTwoNodesWithTransition() { .put("on", new JsonObject() .put(SUCCESS_TRANSITION, jsonForNode("node-A", "factory-A"))); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -142,7 +141,7 @@ void shouldProduceCorrectJsonForMissingNodeCase() { .put("transition", ERROR_TRANSITION) .put("invocations", new JsonArray())); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -165,7 +164,7 @@ void shouldProduceCorrectJsonForSuccessTransitionWithoutNextNode() { .put("transition", SUCCESS_TRANSITION) .put("invocations", wrap(simpleNodeLog()))); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -189,7 +188,7 @@ void shouldProduceCorrectJsonForCompositeNodeWithSimpleNodes() { jsonForNode("node-C", "factory-C") ))); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -213,7 +212,7 @@ void shouldProduceCorrectJsonForCompositeNodeWithSimpleNodesNotAllDescribed() { jsonForNotDescribedNode("node-C") ))); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -260,7 +259,7 @@ void shouldProduceCorrectJsonForComplexGraphWithFullMetadata() { ))) )); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } @Test @@ -342,7 +341,7 @@ void shouldProduceCorrectJsonForComplexGraphWithFullMetadataWithEventLog() { ))) )); - KnotxAssertions.assertJsonEquals(expected, output); + assertJsonEquals(expected, output); } private EventLog createEventLog(EventLogEntry... entries) { From 1ef52fc41a1f4a52c0c1668684aca4e7bbfc35be Mon Sep 17 00:00:00 2001 From: Oskar Jerzyk Date: Mon, 6 Apr 2020 12:03:25 +0200 Subject: [PATCH 05/10] tests updated --- ...onFragmentsHandlerConsumerFactoryTest.java | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java index f4534e59..7bad36d0 100644 --- a/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java +++ b/handler/consumer/json/src/test/java/io/knotx/fragments/handler/consumer/json/JsonFragmentsHandlerConsumerFactoryTest.java @@ -51,28 +51,39 @@ class JsonFragmentsHandlerConsumerFactoryTest { private static final String FRAGMENT_BODY_HTML = "\"
body
\""; private static final String USER_KEY = "user"; private static final String KNOTX_FRAGMENT_KEY = "_knotx_fragment"; - - private static Stream provideFragmentConsumerConfiguration() { - return Stream.of( - Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_JSON), - Arguments.of(HTML_TYPE, FRAGMENT_BODY_HTML), - Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_HTML), - Arguments.of(HTML_TYPE, FRAGMENT_BODY_JSON) + private static final String FRAGMENT_TYPES = "fragmentTypes"; + private static final String UNSUPPORTED = "unsupported"; + + private static Stream provideFragmentTypeAndBody() { + return Stream.of( //fragmentType, fragmentBody, supportedTypes + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_JSON, new JsonArray()), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_HTML, new JsonArray()), + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_HTML, new JsonArray()), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_JSON, new JsonArray()), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_HTML, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)), + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_HTML, + new JsonArray().add(EXPECTED_FRAGMENT_TYPE)), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_JSON, new JsonArray().add(EXPECTED_FRAGMENT_TYPE)), + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_JSON, new JsonArray().add(UNSUPPORTED)), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_HTML, new JsonArray().add(UNSUPPORTED)), + Arguments.of(EXPECTED_FRAGMENT_TYPE, FRAGMENT_BODY_HTML, new JsonArray().add(UNSUPPORTED)), + Arguments.of(HTML_TYPE, FRAGMENT_BODY_JSON, new JsonArray().add(UNSUPPORTED)) ); } @ParameterizedTest - @MethodSource("provideFragmentConsumerConfiguration") - @DisplayName("Fragment body should not be modified when invalid configuration provided") + @MethodSource("provideFragmentTypeAndBody") + @DisplayName("Fragment body should not be modified when no supported methods specified in configuration") void fragmentBodyShouldNotBeModifiedWhenInvalidConfigurationProvided(String fragmentType, - String fragmentBody) { + String fragmentBody, JsonArray supportedTypes) { FragmentEvent original = new FragmentEvent( new Fragment(fragmentType, new JsonObject(), fragmentBody)); FragmentEvent copy = new FragmentEvent(original.toJson()); FragmentExecutionLogConsumer tested = new JsonFragmentsHandlerConsumerFactory() .create(new JsonObject() - .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER))); + .put(CONDITION_OPTION, new JsonObject().put(HEADER_OPTION, EXPECTED_HEADER)) + .put(FRAGMENT_TYPES, supportedTypes)); tested.accept(new ClientRequest() .setHeaders(MultiMap.caseInsensitiveMultiMap().add(EXPECTED_HEADER, "true")), ImmutableList.of(FragmentExecutionLog.newInstance(original))); From 7adc957726ee4249db5072d702200e23e06cf226 Mon Sep 17 00:00:00 2001 From: Oskar Jerzyk Date: Wed, 8 Apr 2020 10:45:57 +0200 Subject: [PATCH 06/10] CR fixes applied --- .../api/docs/asciidoc/dataobjects.adoc | 4 +- .../api/model/FragmentExecutionLog.java | 12 ++-- .../FragmentHtmlBodyWriterFactoryTest.java | 12 +--- .../JsonFragmentsHandlerConsumerFactory.java | 17 +++-- ...onFragmentsHandlerConsumerFactoryTest.java | 70 ++++++------------- .../task/factory/taskWithNodeLabels.conf | 10 +++ 6 files changed, 52 insertions(+), 73 deletions(-) create mode 100644 handler/core/src/test/resources/task/factory/taskWithNodeLabels.conf diff --git a/handler/consumer/api/docs/asciidoc/dataobjects.adoc b/handler/consumer/api/docs/asciidoc/dataobjects.adoc index 4e6fef70..710f365a 100644 --- a/handler/consumer/api/docs/asciidoc/dataobjects.adoc +++ b/handler/consumer/api/docs/asciidoc/dataobjects.adoc @@ -12,7 +12,7 @@ [frame="topbot"] |=== ^|Name | Type ^| Description -|[[finishTime]]`@finishTime`|`Number (long)`|+++ +|[[finishTime]]`@finishTime`|`Number (Long)`|+++ Node processing finish time. It is timestamp value. +++ |[[fragment]]`@fragment`|`link:dataobjects.html#Fragment[Fragment]`|+++ @@ -22,7 +22,7 @@ Node processing finish time. It is timestamp value. |[[graph]]`@graph`|`link:dataobjects.html#GraphNodeExecutionLog[GraphNodeExecutionLog]`|+++ Task evaluation details. +++ -|[[startTime]]`@startTime`|`Number (long)`|+++ +|[[startTime]]`@startTime`|`Number (Long)`|+++ Node processing start time. It is timestamp value. +++ |[[status]]`@status`|`link:enums.html#Status[Status]`|+++ diff --git a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/model/FragmentExecutionLog.java b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/model/FragmentExecutionLog.java index e19948d0..d6369104 100644 --- a/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/model/FragmentExecutionLog.java +++ b/handler/consumer/api/src/main/java/io/knotx/fragments/handler/consumer/api/model/FragmentExecutionLog.java @@ -29,8 +29,8 @@ public class FragmentExecutionLog { private Fragment fragment; private FragmentEvent.Status status = Status.UNPROCESSED; - private long startTime = 0; - private long finishTime = 0; + private Long startTime = 0L; + private Long finishTime = 0L; private GraphNodeExecutionLog graph = null; public static FragmentExecutionLog newInstance(FragmentEvent fragmentEvent, @@ -96,11 +96,11 @@ public FragmentExecutionLog setStatus(Status status) { * * @return node processing start time */ - public long getStartTime() { + public Long getStartTime() { return startTime; } - public FragmentExecutionLog setStartTime(long startTime) { + public FragmentExecutionLog setStartTime(Long startTime) { this.startTime = startTime; return this; } @@ -110,11 +110,11 @@ public FragmentExecutionLog setStartTime(long startTime) { * * @return node processing finish time */ - public long getFinishTime() { + public Long getFinishTime() { return finishTime; } - public FragmentExecutionLog setFinishTime(long finishTime) { + public FragmentExecutionLog setFinishTime(Long finishTime) { this.finishTime = finishTime; return this; } diff --git a/handler/consumer/html/src/test/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactoryTest.java b/handler/consumer/html/src/test/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactoryTest.java index 21d021e4..4db15f0f 100644 --- a/handler/consumer/html/src/test/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactoryTest.java +++ b/handler/consumer/html/src/test/java/io/knotx/fragments/handler/consumer/html/FragmentHtmlBodyWriterFactoryTest.java @@ -183,14 +183,7 @@ void expectFragmentBodyContainsDebugScript() { Fragment fragment = new Fragment("snippet", new JsonObject(), body); FragmentEvent event = new FragmentEvent(fragment); - JsonObject expectedLog = new JsonObject() - .put("startTime", 0) - .put("finishTime", 0) - .put("status", "UNPROCESSED") - .put("fragment", new JsonObject() - .put("id", fragment.getId()) - .put("type", "snippet")) - .put("graph", new JsonObject()); + JsonObject expectedLog = FragmentExecutionLog.newInstance(event).toJson(); String scriptRegexp = "