Merge pull request #799 from CJSCommonPlatform/make-stream-id-nullable

Make stream_id and event_number in event_error_log nullable
CJSCommonPlatform · Dec 17, 2019 · cc47077 · cc47077
2 parents 320585b + 9190141
commit cc47077
Show file tree

Hide file tree

Showing 8 changed files with 173 additions and 139 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,7 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to
 
 ## [Unreleased]
 
-## [6.5.0-M4] - 2019-12-17
+## [6.5.0-M5] - 2019-12-17
 - Update framework-api to 4.2.1
 - Add table in system database 'event_error_log' for storing errors with events that failed to process
 - New SystemErrorService for reporting system errors

diff --git a/.../main/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorService.java b/.../main/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorService.java
@@ -1,17 +1,14 @@
 package uk.gov.justice.services.framework.system.errors;
 
-import static java.lang.String.format;
 import static javax.transaction.Transactional.TxType.NOT_SUPPORTED;
 
 import uk.gov.justice.services.common.util.UtcClock;
-import uk.gov.justice.services.framework.system.errors.SystemErrorService;
 import uk.gov.justice.services.framework.utilities.exceptions.StackTraceProvider;
 import uk.gov.justice.services.messaging.JsonEnvelope;
 import uk.gov.justice.services.messaging.Metadata;
 import uk.gov.justice.services.system.domain.EventError;
 import uk.gov.justice.services.system.persistence.EventErrorLogRepository;
 
-import java.time.Clock;
 import java.util.Optional;
 import java.util.UUID;
 
@@ -20,9 +17,6 @@
 
 public class DefaultSystemErrorService implements SystemErrorService {
 
-    private static final Long MISSING_EVENT_NUMBER = -1L;
-    private static final String NO_COMMENT = "";
-
     @Inject
     private EventErrorLogRepository eventErrorLogRepository;
 
@@ -44,30 +38,22 @@ public void reportError(
         final UUID eventId = metadata.id();
         final String eventName = metadata.name();
         final Optional<Long> eventNumber = metadata.eventNumber();
+        final Optional<UUID> streamId = metadata.streamId();
 
         final EventError eventError = new EventError(
                 messageId,
                 componentName,
-                eventId,
                 eventName,
-                eventNumber.orElse(MISSING_EVENT_NUMBER),
+                eventId,
+                streamId,
+                eventNumber,
                 metadata.asJsonObject().toString(),
                 jsonEnvelope.payload().toString(),
                 exception.getMessage(),
                 stackTraceProvider.getStackTrace(exception),
-                clock.now(),
-                getComment(eventNumber)
+                clock.now()
         );
 
         eventErrorLogRepository.save(eventError);
     }
-
-    private String getComment(final Optional<Long> eventNumberOptional) {
-
-        if (eventNumberOptional.isPresent()) {
-            return NO_COMMENT;
-        }
-
-        return format("Event number is missing from event. Setting to %d instead", MISSING_EVENT_NUMBER);
-    }
 }
diff --git a/...c/main/java/uk/gov/justice/services/framework/system/errors/MissingStreamIdException.java b/...c/main/java/uk/gov/justice/services/framework/system/errors/MissingStreamIdException.java
@@ -0,0 +1,8 @@
+package uk.gov.justice.services.framework.system.errors;
+
+public class MissingStreamIdException extends RuntimeException {
+
+    public MissingStreamIdException(final String message) {
+        super(message);
+    }
+}
diff --git a/...t/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorServiceTest.java b/...t/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorServiceTest.java
@@ -1,10 +1,8 @@
 package uk.gov.justice.services.framework.system.errors;
 
-import static java.util.Optional.empty;
 import static java.util.Optional.of;
 import static java.util.UUID.randomUUID;
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -18,6 +16,7 @@
 import uk.gov.justice.services.system.persistence.EventErrorLogRepository;
 
 import java.time.ZonedDateTime;
+import java.util.Optional;
 import java.util.UUID;
 
 import javax.json.JsonObject;
@@ -34,8 +33,6 @@
 @RunWith(MockitoJUnitRunner.class)
 public class DefaultSystemErrorServiceTest {
 
-    private static final String AN_EMPTY_STRING = "";
-
     @Mock
     private EventErrorLogRepository eventErrorLogRepository;
 
@@ -59,8 +56,9 @@ public void shouldCreateEventErrorAndPersist() throws Exception {
         final String eventName = "context.events.an.event";
         final String errorMessage = "Help help we're all going to die";
         final Throwable exception = new NullPointerException(errorMessage);
-        final Long eventNumber = 239874L;
+        final Optional<Long> eventNumber = of(239874L);
         final UUID eventId = randomUUID();
+        final Optional<UUID> streamId = of(randomUUID());
         final String metadataJson = "{metadata: stuff}";
         final String payloadJson = "{payload: stuff}";
         final String stackTrace = "the stacktrace";
@@ -72,66 +70,10 @@ public void shouldCreateEventErrorAndPersist() throws Exception {
         final JsonValue payload = mock(JsonValue.class);
 
         when(jsonEnvelope.metadata()).thenReturn(metadata);
-        when(metadata.id()).thenReturn(eventId);
         when(metadata.name()).thenReturn(eventName);
-        when(metadata.eventNumber()).thenReturn(of(eventNumber));
-
-        when(metadata.asJsonObject()).thenReturn(metadataJsonObject);
-        when(jsonEnvelope.payload()).thenReturn(payload);
-
-        when(metadataJsonObject.toString()).thenReturn(metadataJson);
-        when(payload.toString()).thenReturn(payloadJson);
-        when(stackTraceProvider.getStackTrace(exception)).thenReturn(stackTrace);
-
-        when(clock.now()).thenReturn(erroredAt);
-
-        defaultSystemErrorService.reportError(
-                messageId,
-                componentName,
-                jsonEnvelope,
-                exception
-        );
-
-        verify(eventErrorLogRepository).save(eventErrorCaptor.capture());
-
-        final EventError eventError = eventErrorCaptor.getValue();
-
-        assertThat(eventError.getMessageId(), is(messageId));
-        assertThat(eventError.getComponent(), is(componentName));
-        assertThat(eventError.getEventId(), is(eventId));
-        assertThat(eventError.getEventName(), is(eventName));
-        assertThat(eventError.getEventNumber(), is(eventNumber));
-        assertThat(eventError.getMetadata(), is(metadataJson));
-        assertThat(eventError.getPayload(), is(payloadJson));
-        assertThat(eventError.getErrorMessage(), is(errorMessage));
-        assertThat(eventError.getStacktrace(), is(stackTrace));
-        assertThat(eventError.getErroredAt(), is(erroredAt));
-        assertThat(eventError.getComments(), is(AN_EMPTY_STRING));
-    }
-
-    @Test
-    public void shouldHandleMissingEventNumber() throws Exception {
-
-        final String messageId = "message id";
-        final String componentName = "EVENT_LISTENER";
-        final String eventName = "context.events.an.event";
-        final String errorMessage = "Help help we're all going to die";
-        final Throwable exception = new NullPointerException(errorMessage);
-        final UUID eventId = randomUUID();
-        final String metadataJson = "{metadata: stuff}";
-        final String payloadJson = "{payload: stuff}";
-        final String stackTrace = "the stacktrace";
-        final ZonedDateTime erroredAt = new UtcClock().now();
-
-        final JsonEnvelope jsonEnvelope = mock(JsonEnvelope.class);
-        final Metadata metadata = mock(Metadata.class);
-        final JsonObject metadataJsonObject = mock(JsonObject.class);
-        final JsonValue payload = mock(JsonValue.class);
-
-        when(jsonEnvelope.metadata()).thenReturn(metadata);
         when(metadata.id()).thenReturn(eventId);
-        when(metadata.name()).thenReturn(eventName);
-        when(metadata.eventNumber()).thenReturn(empty());
+        when(metadata.streamId()).thenReturn(streamId);
+        when(metadata.eventNumber()).thenReturn(eventNumber);
 
         when(metadata.asJsonObject()).thenReturn(metadataJsonObject);
         when(jsonEnvelope.payload()).thenReturn(payload);
@@ -156,13 +98,13 @@ public void shouldHandleMissingEventNumber() throws Exception {
         assertThat(eventError.getMessageId(), is(messageId));
         assertThat(eventError.getComponent(), is(componentName));
         assertThat(eventError.getEventName(), is(eventName));
-        assertThat(eventError.getErrorMessage(), is(errorMessage));
-        assertThat(eventError.getEventNumber(), is(-1L));
         assertThat(eventError.getEventId(), is(eventId));
+        assertThat(eventError.getStreamId(), is(streamId));
+        assertThat(eventError.getEventNumber(), is(eventNumber));
         assertThat(eventError.getMetadata(), is(metadataJson));
         assertThat(eventError.getPayload(), is(payloadJson));
+        assertThat(eventError.getErrorMessage(), is(errorMessage));
         assertThat(eventError.getStacktrace(), is(stackTrace));
         assertThat(eventError.getErroredAt(), is(erroredAt));
-        assertThat(eventError.getComments(), is("Event number is missing from event. Setting to -1 instead"));
     }
 }
diff --git a/...rc/main/resources/liquibase/framework-system-changesets/004-add-event-error-log-table.xml b/...rc/main/resources/liquibase/framework-system-changesets/004-add-event-error-log-table.xml
@@ -16,14 +16,17 @@
             <column name="component" type="TEXT">
                 <constraints nullable="false"/>
             </column>
-            <column name="event_id" type="uuid">
+            <column name="event_name" type="TEXT">
                 <constraints nullable="false"/>
             </column>
-            <column name="event_name" type="TEXT">
+            <column name="event_id" type="uuid">
                 <constraints nullable="false"/>
             </column>
+            <column name="stream_id" type="uuid">
+                <constraints nullable="true"/>
+            </column>
             <column name="event_number" type="BIGINT">
-                <constraints nullable="false"/>
+                <constraints nullable="true"/>
             </column>
             <column name="metadata" type="TEXT">
                 <constraints nullable="false"/>
@@ -40,9 +43,6 @@
             <column name="errored_at" type="TIMESTAMP WITH TIME ZONE">
                 <constraints nullable="false"/>
             </column>
-            <column name="comments" type="TEXT">
-                <constraints nullable="true"/>
-            </column>
         </createTable>
 
         <addPrimaryKey tableName="event_error_log" columnNames="message_id,component"/>

diff --git a/...rk-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java b/...rk-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java
@@ -2,45 +2,46 @@
 
 import java.time.ZonedDateTime;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.UUID;
 
 public class EventError {
 
     private final String messageId;
     private final String component;
-    private final UUID eventId;
     private final String eventName;
-    private final Long eventNumber;
+    private final UUID eventId;
+    private final Optional<UUID> streamId;
+    private final Optional<Long> eventNumber;
     private final String metadata;
     private final String payload;
     private final String errorMessage;
     private final String stacktrace;
     private final ZonedDateTime erroredAt;
-    private final String comments;
 
     public EventError(
             final String messageId,
             final String component,
-            final UUID eventId,
             final String eventName,
-            final Long eventNumber,
+            final UUID eventId,
+            final Optional<UUID> streamId,
+            final Optional<Long> eventNumber,
             final String metadata,
             final String payload,
             final String errorMessage,
             final String stacktrace,
-            final ZonedDateTime erroredAt,
-            final String comments) {
+            final ZonedDateTime erroredAt) {
         this.messageId = messageId;
         this.component = component;
-        this.eventId = eventId;
         this.eventName = eventName;
+        this.eventId = eventId;
+        this.streamId = streamId;
         this.eventNumber = eventNumber;
         this.metadata = metadata;
         this.payload = payload;
         this.errorMessage = errorMessage;
         this.stacktrace = stacktrace;
         this.erroredAt = erroredAt;
-        this.comments = comments;
     }
 
     public String getMessageId() {
@@ -51,15 +52,19 @@ public String getComponent() {
         return component;
     }
 
+    public String getEventName() {
+        return eventName;
+    }
+
     public UUID getEventId() {
         return eventId;
     }
 
-    public String getEventName() {
-        return eventName;
+    public Optional<UUID> getStreamId() {
+        return streamId;
     }
 
-    public Long getEventNumber() {
+    public Optional<Long> getEventNumber() {
         return eventNumber;
     }
 
@@ -83,45 +88,41 @@ public ZonedDateTime getErroredAt() {
         return erroredAt;
     }
 
-    public String getComments() {
-        return comments;
-    }
-
     @Override
     public boolean equals(final Object o) {
         if (this == o) return true;
         if (!(o instanceof EventError)) return false;
         final EventError that = (EventError) o;
         return Objects.equals(messageId, that.messageId) &&
                 Objects.equals(component, that.component) &&
-                Objects.equals(eventId, that.eventId) &&
                 Objects.equals(eventName, that.eventName) &&
+                Objects.equals(eventId, that.eventId) &&
+                Objects.equals(streamId, that.streamId) &&
                 Objects.equals(eventNumber, that.eventNumber) &&
                 Objects.equals(metadata, that.metadata) &&
                 Objects.equals(payload, that.payload) &&
                 Objects.equals(errorMessage, that.errorMessage) &&
-                Objects.equals(erroredAt, that.erroredAt) &&
-                Objects.equals(comments, that.comments);
+                Objects.equals(stacktrace, that.stacktrace) &&
+                Objects.equals(erroredAt, that.erroredAt);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(messageId, component, eventId, eventName, eventNumber, metadata, payload, errorMessage, stacktrace, erroredAt, comments);
+        return Objects.hash(messageId, component, eventName, eventId, streamId, eventNumber, metadata, payload, errorMessage, stacktrace, erroredAt);
     }
 
     @Override
     public String toString() {
         return "EventError{" +
                 "messageId='" + messageId + '\'' +
                 ", component='" + component + '\'' +
-                ", eventId=" + eventId +
                 ", eventName='" + eventName + '\'' +
+                ", eventId=" + eventId +
+                ", streamId=" + streamId +
                 ", eventNumber=" + eventNumber +
                 ", metadata='" + metadata + '\'' +
-                ", payload='" + payload + '\'' +
                 ", errorMessage='" + errorMessage + '\'' +
                 ", erroredAt=" + erroredAt +
-                ", comments='" + comments + '\'' +
                 '}';
     }
 }