add stream transformation support for linked events #25

MohamedFarouk-HMCTS · 2019-03-14T13:38:12Z

No description provided.

coveralls · 2019-03-18T13:16:59Z

Coverage decreased (-0.5%) to 91.64% when pulling 0cd9e44 on add-linked-event-support-for-transformations into 8055d70 on master.

MartinYSpasov · 2019-03-18T15:36:22Z

event-tool/src/main/java/uk/gov/justice/event/tool/StartTransformation.java

+
+        logger.info("-------------- Truncating the Linked Events Log after complete transformation --------------");
+
+        linkedEventStreamTransformationService.truncateLinkedEvents();


do you need to call truncateLinkedEvents() twice?

MartinYSpasov · 2019-03-18T15:40:59Z

event-tool/src/test/java/uk/gov/justice/event/tool/StartTransformationTest.java

@@ -158,19 +168,28 @@ public void shouldRemoveFinishedTaskForAllPasses() {

        startTransformation.taskDone(future3, null, null, null);
        assertThat(startTransformation.outstandingTasks.size(), is(0));
+
+//        verify(linkedEventStreamTransformationService).truncateLinkedEvents();


please uncomment/delete verify (although we should verify that truncateLinkedEvents() and populateLinkedEvents() are called )

MartinYSpasov · 2019-03-18T15:41:37Z

pom.xml

@@ -23,8 +23,9 @@
    <properties>
        <cpp.repo.name>stream-transformation-tool</cpp.repo.name>

-        <framework.version>5.0.4</framework.version>
-        <event-store.version>1.0.4</event-store.version>
+        <framework.version>5.1.2-SNAPSHOT</framework.version>


don't use snapshots please

waiting for the release version of frame work api

MartinYSpasov · 2019-03-18T15:46:18Z

...sformation-it/src/main/java/uk/gov/justice/framework/tools/transformation/DatabaseUtils.java

    private final LiquibaseUtil liquibaseUtil = new LiquibaseUtil();
-
+    DataSource dataSource = null;


why dataSource was moved outside of DatabaseUtils() constructor?

datasource required for other test classes in the same package

MartinYSpasov · 2019-03-18T15:51:20Z

...ormation-it/src/main/java/uk/gov/justice/framework/tools/transformation/EventLogBuilder.java

@@ -15,7 +16,6 @@
    private EventLogBuilder() {


Is it EventLogBuilder or just EventBuilder?

EventLogBuilder

MartinYSpasov · 2019-03-18T15:55:50Z

...c/main/java/uk/gov/justice/framework/tools/transformation/TestLinkedEventJdbcRepository.java

+
+import javax.sql.DataSource;
+
+public class TestLinkedEventJdbcRepository extends LinkedEventJdbcRepository {


I know this is a test helper class, but maybe it's worth considering moving this logic to framework or test-utils so can be used by other people as well

MartinYSpasov · 2019-03-18T15:57:06Z

...n-it/src/test/java/uk/gov/justice/framework/tools/transformation/StreamTransformationIT.java

    }

    @After
    public void cleanup() throws Exception {
-        databaseUtils.resetDatabase();
+        //   databaseUtils.resetDatabase();


remove cleanup() please

MartinYSpasov · 2019-03-18T16:00:48Z

...e/tools/eventsourcing/transformation/service/LinkedEventStreamTransformationServiceTest.java

+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class LinkedEventStreamTransformationServiceTest {


This doesn't seem to test anything.

allanmckenzie · 2019-03-18T16:04:43Z

event-tool/src/test/java/uk/gov/justice/event/tool/StartTransformationTest.java

@@ -95,6 +99,9 @@ public void shouldCreateTasksForEachStream() {

        assertThat(startTransformation.outstandingTasks.size(), is(2));
        assertTrue(startTransformation.allTasksCreated);
+
+//        verify(linkedEventStreamTransformationService).truncateLinkedEvents();
+//        verify(linkedEventStreamTransformationService).populateLinkedEvents();
    }


Commented out code can be removed

allanmckenzie · 2019-03-18T16:04:57Z

event-tool/src/test/java/uk/gov/justice/event/tool/StartTransformationTest.java

@@ -119,6 +126,9 @@ public void shouldRemoveFinishedTasks() {

        startTransformation.taskDone(future2, null, null, null);
        assertThat(startTransformation.outstandingTasks.size(), is(0));
+
+//        verify(linkedEventStreamTransformationService).truncateLinkedEvents();
+//        verify(linkedEventStreamTransformationService).populateLinkedEvents();
    }


Commented out code can be removed

allanmckenzie · 2019-03-18T16:07:39Z

...sformation-it/src/main/java/uk/gov/justice/framework/tools/transformation/DatabaseUtils.java

    private final LiquibaseUtil liquibaseUtil = new LiquibaseUtil();
-
+    DataSource dataSource = null;


Should be private (and final if possible)

private not final used outside constructor

allanmckenzie · 2019-03-18T16:07:46Z

...sformation-it/src/main/java/uk/gov/justice/framework/tools/transformation/DatabaseUtils.java

@@ -19,11 +20,11 @@

    private TestEventLogJdbcRepository eventLogJdbcRepository;
    private TestEventStreamJdbcRepository eventStreamJdbcRepository;
-
+    private EventFetcherRepository eventFetcherRepository = new EventFetcherRepository();


mapingo · 2019-03-18T16:08:18Z

CHANGELOG.md

@@ -5,6 +5,9 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to

 ## [Unreleased]

+## [5.2.0] - 2018-12-12


looks like the wrong date

mapingo · 2019-03-18T16:09:47Z

event-tool/src/test/java/uk/gov/justice/event/tool/StartTransformationTest.java

    }


    @Test(expected = IllegalArgumentException.class)
    public void shouldThrowExceptionWhenstreamsProcessedCountStepInfoIsNull() throws Exception {
        streamsProcessedCountStepInfo.set(startTransformation, null);
        startTransformation.go();
+//
+//        verify(linkedEventStreamTransformationService).truncateLinkedEvents();
+//        verify(linkedEventStreamTransformationService).populateLinkedEvents();


this comment code should be reinstated or deleted

done as mentioned by allan and martin

mapingo · 2019-03-18T16:11:41Z

stream-transformation-test/sample-transformations/pom.xml

+            <groupId>uk.gov.justice.framework-api</groupId>
+            <artifactId>framework-api-core</artifactId>
+            <version>${framework-api.version}</version>
+        </dependency>


If this is not a test dependency then it should be higher in the list

mapingo · 2019-03-18T16:12:27Z

stream-transformation-test/stream-transformation-it/pom.xml

+            <groupId>uk.gov.justice.event-store</groupId>
+            <artifactId>event-repository-jdbc</artifactId>
+            <version>${event-store.version}</version>
+        </dependency>


Non test dependencies above should be moved above the test dependency

mapingo · 2019-03-18T16:14:40Z

.../src/test/java/uk/gov/justice/framework/tools/transformation/StreamTransformationMoveIT.java

@@ -20,24 +21,25 @@
 public class StreamTransformationMoveIT {

    private static final UUID STREAM_ID = randomUUID();
+    private static final long STREAM_COUNT_REPORTING_INTERVAL = 10l;


uppercase L is easier to read - 10L

mapingo · 2019-03-18T16:19:36Z

...gov/justice/tools/eventsourcing/transformation/service/EventStreamTransformationService.java

@@ -75,7 +75,7 @@ public UUID transformEventStream(final UUID originalStreamId, final int pass) {

            if (action.isTransform()) {
                final Optional<UUID> newStreamId = eventTransformationStreamIdFilter.getEventTransformationStreamId(eventTransformations, jsonEnvelopeList);
-                if (newStreamId.isPresent()) {
+                if (newStreamId.isPresent() && !newStreamId.get().equals(originalStreamId)) {


Can this check be created as a method or value and have a descriptive name?

mapingo · 2019-03-20T10:33:00Z

pom.xml

-
-        <framework.version>5.0.4</framework.version>
-        <event-store.version>1.0.4</event-store.version>
+        <framework-api.version>4.0.0-M2</framework-api.version>


framework-api version is 3.1.1

mapingo · 2019-03-20T10:33:36Z

pom.xml

-        <framework.version>5.0.4</framework.version>
-        <event-store.version>1.0.4</event-store.version>
+        <framework-api.version>4.0.0-M2</framework-api.version>
+        <framework.version>6.0.0-M4</framework.version>


framework version is 5.1.2

mapingo · 2019-03-20T10:40:04Z

CHANGELOG.md

@@ -5,6 +5,9 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to

 ## [Unreleased]

+## [5.2.0] - 2019-03-18


Wrong date now

MohamedFarouk-HMCTS requested review from MartinYSpasov and allanmckenzie March 18, 2019 15:29

MartinYSpasov suggested changes Mar 18, 2019

View reviewed changes

allanmckenzie reviewed Mar 18, 2019

View reviewed changes

mapingo suggested changes Mar 18, 2019

View reviewed changes

mapingo suggested changes Mar 20, 2019

View reviewed changes

MohamedFarouk-HMCTS force-pushed the add-linked-event-support-for-transformations branch 2 times, most recently from 8221936 to a3a6b07 Compare March 20, 2019 13:55

mapingo approved these changes Mar 20, 2019

View reviewed changes

MohamedFarouk-HMCTS force-pushed the add-linked-event-support-for-transformations branch from a3a6b07 to 2a128b5 Compare March 20, 2019 14:20

MartinYSpasov approved these changes Mar 20, 2019

View reviewed changes

add stream transformation support for linked events

0cd9e44

MohamedFarouk-HMCTS force-pushed the add-linked-event-support-for-transformations branch from 2a128b5 to 0cd9e44 Compare March 20, 2019 14:56

MohamedFarouk-HMCTS merged commit f66b22f into master Mar 20, 2019

MohamedFarouk-HMCTS deleted the add-linked-event-support-for-transformations branch March 20, 2019 15:15


		logger.info("-------------- Truncating the Linked Events Log after complete transformation --------------");

		linkedEventStreamTransformationService.truncateLinkedEvents();

		private final LiquibaseUtil liquibaseUtil = new LiquibaseUtil();

		DataSource dataSource = null;


		import javax.sql.DataSource;

		public class TestLinkedEventJdbcRepository extends LinkedEventJdbcRepository {

		@@ -5,6 +5,9 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to

		## [Unreleased]

		## [5.2.0] - 2018-12-12

		@@ -5,6 +5,9 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to

		## [Unreleased]

		## [5.2.0] - 2019-03-18

add stream transformation support for linked events #25

add stream transformation support for linked events #25

Conversation

MohamedFarouk-HMCTS commented Mar 14, 2019

coveralls commented Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MohamedFarouk-HMCTS Mar 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 18, 2019 •

edited

Loading

MohamedFarouk-HMCTS Mar 18, 2019 •

edited

Loading