CHD-3035 - accept both documentReference and documentReferenceUri on case document upload

sjp-command/sjp-command-api/src/raml/json/schema/sjp.upload-case-document.json

@@ -10,7 +10,7 @@
       "format": "uri"
     }
   },
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "caseDocument"

sjp-command/sjp-command-api/src/raml/json/sjp.upload-case-document.json

@@ -1,3 +1,4 @@
 {
-  "caseDocument": "fc3f8d82-d200-4cab-a492-2796e6fdf42c"
+  "caseDocument": "fc3f8d82-d200-4cab-a492-2796e6fdf42c",
+  "caseDocumentUri": "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/plea.pdf"
 }

sjp-command/sjp-command-controller/src/raml/json/schema/sjp.command.upload-case-document.json

@@ -21,7 +21,7 @@
     "caseId",
     "caseDocumentType"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "caseDocument"

sjp-command/sjp-command-controller/src/raml/json/sjp.command.upload-case-document.json

@@ -1,5 +1,6 @@
 {
   "caseId": "6ac98e9c-66b2-4363-8a78-641dbec8bbd2",
   "caseDocument": "fc3f8d82-d200-4cab-a492-2796e6fdf42c",
+  "caseDocumentUri": "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/plea.pdf",
   "caseDocumentType": "PLEA"
 }

sjp-command/sjp-command-handler/src/main/java/uk/gov/moj/cpp/sjp/command/handler/UploadCaseDocumentHandler.java

@@ -31,9 +31,10 @@ public void handle(JsonEnvelope command) throws EventStreamException {
         final UUID caseId = getCaseId(payload);
         final String caseDocumentType = payload.getString("caseDocumentType");
 
-        // Exactly one of a file service id (caseDocument) or a blob uri (caseDocumentUri) is
-        // present - the command schema's oneOf enforces that, and JsonSchemaValidationInterceptor
-        // applies it on the way in, so this only has to pick whichever arrived.
+        // At least one of a file service id (caseDocument) or a blob uri (caseDocumentUri) is
+        // present - the command schema's anyOf enforces that - and both may arrive together, which
+        // is what staging-dvla sends. Pass through whatever came; the aggregate and the processor
+        // decide what each of them means.
         final String caseDocumentReference = valueOrNull(payload, CASE_DOCUMENT);
         final String caseDocumentUri = valueOrNull(payload, CASE_DOCUMENT_URI);
 

sjp-command/sjp-command-handler/src/raml/json/schema/sjp.command.upload-case-document.json

@@ -21,7 +21,7 @@
     "caseId",
     "caseDocumentType"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "caseDocument"

sjp-command/sjp-command-handler/src/raml/json/sjp.command.upload-case-document.json

@@ -1,5 +1,6 @@
 {
   "caseId": "6ac98e9c-66b2-4363-8a78-641dbec8bbd2",
   "caseDocument": "fc3f8d82-d200-4cab-a492-2796e6fdf42c",
+  "caseDocumentUri": "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/plea.pdf",
   "caseDocumentType": "PLEA"
 }

sjp-command/sjp-command-handler/src/test/java/uk/gov/moj/cpp/sjp/command/handler/UploadCaseDocumentHandlerTest.java

@@ -169,6 +169,29 @@ public void shouldRaiseUploadedEventCarryingTheUriWhenDocumentIsBlobAddressed()
 
 
 
+    @Test
+    public void shouldRaiseUploadedEventCarryingBothReferencesWhenTheCallerSuppliesBoth() throws EventStreamException {
+        final JsonEnvelope command = createCommand(payload -> payload
+                .add(CASE_DOCUMENT_REFERENCE_PROPERTY, DOCUMENT_REFERENCE.toString())
+                .add(CASE_DOCUMENT_URI_PROPERTY, DOCUMENT_URI));
+        when(eventSource.getStreamById(CASE_ID)).thenReturn(eventStream);
+        when(aggregateService.get(eventStream, CaseAggregate.class)).thenReturn(caseAggregate);
+
+        uploadCaseDocumentHandler.handle(command);
+
+        assertThat(eventStream, eventStreamAppendedWith(
+                streamContaining(
+                        jsonEnvelope(
+                                withMetadataEnvelopedFrom(command)
+                                        .withName("sjp.events.case-document-uploaded"),
+                                payloadIsJson(allOf(
+                                        withJsonPath("$.caseId", equalTo(CASE_ID.toString())),
+                                        withJsonPath("$.documentReference", equalTo(DOCUMENT_REFERENCE.toString())),
+                                        withJsonPath("$.documentReferenceUri", equalTo(DOCUMENT_URI))
+                                )))
+                )));
+    }
+
     private JsonEnvelope createCaseDocumentUploadCommand(final UUID caseId, final UUID caseDocumentReference, final String documentType) {
         final JsonObjectBuilder payload = createObjectBuilder()
                 .add(CASE_ID_PROPERTY, caseId.toString())

sjp-domain/sjp-domain-aggregate/src/main/java/uk/gov/moj/cpp/sjp/domain/aggregate/handler/CaseDocumentHandler.java

@@ -46,11 +46,12 @@ public Stream<Object> addCaseDocument(final UUID caseId,
     }
 
     /**
-     * @param documentReference    file service id of the document, or null when it is addressed by
-     *                             {@code documentReferenceUri}
-     * @param documentReferenceUri blob uri of the document, or null when it is addressed by
-     *                             {@code documentReference}. Exactly one of the two is set; the
-     *                             caller validates that.
+     * @param documentReference    file service id of the document, or the uuid derived from
+     *                             {@code documentReferenceUri} by the calling context. Null only
+     *                             when the caller supplied a uri and no id.
+     * @param documentReferenceUri blob uri of the document, or null when it is addressed by a file
+     *                             service id. At least one of the two is set - the command schema's
+     *                             {@code anyOf} enforces that - and both may be set together.
      */
     public Stream<Object> uploadCaseDocument(final UUID caseId,
                                              final UUID documentReference,
@@ -59,7 +60,9 @@ public Stream<Object> uploadCaseDocument(final UUID caseId,
                                              final CaseAggregateState state) {
 
         if (!state.hasGrantedApplication()) {
-            final Object reference = nonNull(documentReference) ? documentReference : documentReferenceUri;
+            // Both may be present now, so prefer the uri: it is the reference the calling context
+            // supplied and recognises, and the uuid is derived from it anyway.
+            final Object reference = nonNull(documentReferenceUri) ? documentReferenceUri : documentReference;
 
             if (state.isCaseReferredForCourtHearing()) {
                 LOGGER.warn("Case Document Upload rejected as case is referred to court for hearing: {}", reference);

sjp-domain/sjp-domain-aggregate/src/test/java/uk/gov/moj/cpp/sjp/domain/aggregate/handler/CaseDocumentHandlerTest.java

@@ -60,6 +60,30 @@ public void uploadCaseDocument_whenBlobAddressedAndRejected_shouldCarryTheUriOnT
         assertThat(rejected.getDescription(), containsString(documentReferenceUri));
     }
 
+    @Test
+    public void uploadCaseDocument_whenBothReferencesSupplied_shouldNameTheUriAndCarryBoth() {
+        final UUID caseId = UUID.randomUUID();
+        final UUID documentReference = UUID.randomUUID();
+        final String documentReferenceUri = "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/doc.pdf";
+        final CaseAggregateState state = mock(CaseAggregateState.class);
+
+        when(state.hasGrantedApplication()).thenReturn(false);
+        when(state.isCaseReferredForCourtHearing()).thenReturn(true);
+
+        final List<Object> events = CaseDocumentHandler.INSTANCE
+                .uploadCaseDocument(caseId, documentReference, documentReferenceUri, "type", state)
+                .collect(Collectors.toList());
+
+        final CaseDocumentUploadRejected rejected = (CaseDocumentUploadRejected) events.get(0);
+
+        // both travel on the event...
+        assertThat(rejected.getDocumentId(), is(documentReference));
+        assertThat(rejected.getDocumentReferenceUri(), is(documentReferenceUri));
+
+        // ...but the message names the uri, which is what the calling context recognises
+        assertThat(rejected.getDescription(), containsString(documentReferenceUri));
+    }
+
     @Test
     public void uploadCaseDocument_whenCaseNotManagedByAtcm_shouldReturnCaseDocumentUploadRejectedEvent() {
         UUID caseId = UUID.randomUUID();

sjp-event/sjp-event-processor/src/main/java/uk/gov/moj/cpp/sjp/event/processor/CaseDocumentProcessor.java

@@ -1,7 +1,6 @@
 package uk.gov.moj.cpp.sjp.event.processor;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.isNull;
 import static java.util.Objects.nonNull;
 import static java.util.UUID.nameUUIDFromBytes;
 import static java.util.UUID.randomUUID;
@@ -61,12 +60,13 @@ public void handleCaseDocumentUploaded(final JsonEnvelope caseDocumentUploadedEv
         final UUID caseId = UUID.fromString(payload.getString(CASE_ID));
         final String documentType = payload.getString(DOCUMENT_TYPE);
 
-        // Exactly one of these is set - the event schema's oneOf enforces that, and
-        // JsonSchemaValidationInterceptor applies it on the way in. A blob-addressed document is
-        // forwarded onward as a uri; SJP never reads the document itself either way.
+        // Both may now arrive together: staging-dvla derives the case document's uuid from the uri
+        // and sends the pair, so a uuid being present no longer means the document is file-service
+        // addressed. The uri is what says "blob" - test for that directly. Getting this backwards
+        // sends material a fileServiceId no file service has ever heard of.
         final String documentReference = valueOrNull(payload, DOCUMENT_REFERENCE);
         final String documentReferenceUri = valueOrNull(payload, DOCUMENT_REFERENCE_URI);
-        final boolean addressedByUri = isNull(documentReference);
+        final boolean addressedByUri = nonNull(documentReferenceUri);
 
         final JsonObjectBuilder fileUploadedEventPayload = createObjectBuilder()
                 .add(CASE_ID, caseId.toString());
@@ -76,15 +76,26 @@ public void handleCaseDocumentUploaded(final JsonEnvelope caseDocumentUploadedEv
                 .add(CASE_ID, caseId.toString())
                 .add(DOCUMENT_TYPE, documentType);
 
+        // The public event and the sjp metadata carry everything we were given. The metadata one
+        // matters most: handleMaterialAdded reads documentId back out of it to build
+        // sjp.command.add-case-document, so the uuid has to ride along or that hop falls back to
+        // deriving one and the caller's id is lost.
+        if (nonNull(documentReference)) {
+            fileUploadedEventPayload.add(DOCUMENT_ID, documentReference);
+            sjpMetadata.add(DOCUMENT_ID, documentReference);
+        }
         if (addressedByUri) {
             fileUploadedEventPayload.add(DOCUMENT_URI, documentReferenceUri);
-            // Material rejects a command carrying more than one file reference, so send only this one.
-            uploadFilePayload.add(FILE_URI, documentReferenceUri);
             sjpMetadata.add(DOCUMENT_URI, documentReferenceUri);
+        }
+
+        // Material is the exception: material.command.upload-file is an exclusive oneOf and its
+        // handler throws on more than one reference, so exactly one goes on that payload. The uri
+        // wins when there is one - it is the only form material can actually read.
+        if (addressedByUri) {
+            uploadFilePayload.add(FILE_URI, documentReferenceUri);
         } else {
-            fileUploadedEventPayload.add(DOCUMENT_ID, documentReference);
             uploadFilePayload.add(FILE_SERVICE_ID, documentReference);
-            sjpMetadata.add(DOCUMENT_ID, documentReference);
         }
 
         sender.send(enveloper.withMetadataFrom(caseDocumentUploadedEvent, "public.sjp.case-document-uploaded")
@@ -123,11 +134,11 @@ public void handleMaterialAdded(final JsonEnvelope materialAddedEvent) {
 
             LOGGER.info("Material {} is a {} for sjp case {}", materialId, documentType, caseId);
 
-            // A blob-addressed document has no file service id to become the case document's
-            // identity, and case_document.id is a uuid primary key. Derive a stable v3 uuid from
-            // the blob uri: the same uri always yields the same id, so a redelivered
-            // material.material-added is caught by the aggregate's duplicate check exactly as it is
-            // on the file-service path.
+            // Normally the id was supplied: staging-dvla derives it from the uri with this exact
+            // algorithm and sends it on the command. The derivation below is the fallback for a
+            // uri-only payload - a caller not yet updated, or a pre-change event replayed from the
+            // store. Keep the two implementations identical; see
+            // cpp-context-staging-dvla SystemDocGeneratorEventProcessor.
             final String caseDocumentId = getCaseDocumentId(documentId, documentUri);
 
             final JsonObjectBuilder payload = createObjectBuilder()

sjp-event/sjp-event-processor/src/test/java/uk/gov/moj/cpp/sjp/event/processor/CaseDocumentUploadedProcessorTest.java

@@ -10,6 +10,7 @@
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
@@ -200,6 +201,80 @@ public void shouldForwardTheUriToMaterialWhenDocumentIsBlobAddressed() {
         assertThat(sent.get(1).payloadAsJsonObject().containsKey("fileServiceId"), is(false));
     }
 
+    @Test
+    public void shouldHandleABlobAddressedDocumentWithoutError() {
+        final JsonObject payload = createObjectBuilder()
+                .add("caseId", caseId.toString())
+                .add("documentReferenceUri", DOCUMENT_URI)
+                .add("documentType", DOCUMENT_TYPE).build();
+
+        caseDocumentProcessor.handleCaseDocumentUploaded(createEnvelope("sjp.events.case-document-uploaded", payload));
+
+        verify(sender, times(2)).send(envelopeCaptor.capture());
+        assertThat(envelopeCaptor.getAllValues().get(0).metadata().name(), is("public.sjp.case-document-uploaded"));
+        assertThat(envelopeCaptor.getAllValues().get(1).metadata().name(), is("material.command.upload-file"));
+    }
+
+    @Test
+    public void shouldCarryBothReferencesOnwardWhenTheCallerSuppliesBoth() {
+        // The shape staging-dvla now sends: it derives the uuid from the uri itself and sends the
+        // pair, so a uuid being present must NOT be read as "file service addressed".
+        final JsonObject payload = createObjectBuilder()
+                .add("caseId", caseId.toString())
+                .add("documentReference", documentReference.toString())
+                .add("documentReferenceUri", DOCUMENT_URI)
+                .add("documentType", DOCUMENT_TYPE).build();
+
+        caseDocumentProcessor.handleCaseDocumentUploaded(createEnvelope("sjp.events.case-document-uploaded", payload));
+
+        verify(sender, times(2)).send(envelopeCaptor.capture());
+        final List<JsonEnvelope> sent = envelopeCaptor.getAllValues();
+
+        // the public event keeps everything we were given
+        assertThat(sent.get(0).payloadAsJsonObject().toString(), isJson(allOf(
+                withJsonPath("$.documentId", equalTo(documentReference.toString())),
+                withJsonPath("$.documentUri", equalTo(DOCUMENT_URI)))));
+
+        // material takes exactly one, and it has to be the uri - its handler throws on more than
+        // one reference, and a derived uuid means nothing to the file service.
+        assertThat(sent.get(1).metadata().name(), is("material.command.upload-file"));
+        assertThat(sent.get(1).payloadAsJsonObject().toString(), isJson(
+                withJsonPath("$.fileUri", equalTo(DOCUMENT_URI))));
+        assertThat(sent.get(1).payloadAsJsonObject().containsKey("fileServiceId"), is(false));
+
+        // and the sjp metadata carries both, so handleMaterialAdded uses the supplied uuid
+        assertThat(sent.get(1).metadata().asJsonObject().toString(), isJson(allOf(
+                withJsonPath("$.sjpMetadata.documentId", equalTo(documentReference.toString())),
+                withJsonPath("$.sjpMetadata.documentUri", equalTo(DOCUMENT_URI)))));
+    }
+
+    @Test
+    public void shouldUseTheSuppliedIdRatherThanDerivingOneWhenBothAreKnown() {
+        // The whole point of the change: a derived uuid is still a valid uuid, so a regression here
+        // would be invisible without pinning the supplied value explicitly.
+        final Metadata enriched = metadataFrom(
+                JsonObjects.createObjectBuilder(materialAddedMetadata.asJsonObject())
+                        .add("sjpMetadata", createObjectBuilder()
+                                .add("caseId", caseId.toString())
+                                .add("documentId", documentReference.toString())
+                                .add("documentUri", DOCUMENT_URI)
+                                .add("documentType", DOCUMENT_TYPE)
+                                .build()).build())
+                .build();
+
+        caseDocumentProcessor.handleMaterialAdded(envelopeFrom(enriched, materialAddedPayload));
+
+        verify(sender).send(envelopeCaptor.capture());
+
+        assertThat(envelopeCaptor.getValue().payloadAsJsonObject().toString(), isJson(allOf(
+                withJsonPath("$.id", equalTo(documentReference.toString())),
+                withJsonPath("$.documentUri", equalTo(DOCUMENT_URI)))));
+
+        // specifically NOT the value the fallback would have produced
+        assertThat(envelopeCaptor.getValue().payloadAsJsonObject().getString("id"),
+                is(not(nameUUIDFromBytes(DOCUMENT_URI.getBytes(UTF_8)).toString())));
+    }
+
     @Test
     public void shouldAddCaseDocumentForABlobAddressedDocumentWithAnIdDerivedFromTheUri() {
         // A blob-addressed document has no file service id, and case_document.id is a uuid primary

sjp-event/sjp-event-processor/src/yaml/json/public.sjp.case-document-uploaded.json

@@ -1,4 +1,5 @@
 {
   "documentId": "7e2f843e-d639-40b3-2611-8015f3a18958",
+  "documentUri": "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/plea.pdf",
   "caseId": "b62dc6aa-97e2-4883-98f8-2ab681a29d22"
 }

sjp-event/sjp-event-processor/src/yaml/json/schema/public.sjp.case-document-uploaded.json

@@ -17,7 +17,7 @@
   "required": [
     "caseId"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "documentId"

sjp-event/sjp-event-processor/src/yaml/json/schema/public.sjp.events.case-document-upload-rejected.json

@@ -17,7 +17,7 @@
   "required": [
     "description"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "documentId"

sjp-event/sjp-event-processor/src/yaml/json/schema/sjp.events.case-document-upload-rejected.json

@@ -17,7 +17,7 @@
   "required": [
     "description"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "documentId"

sjp-event/sjp-event-processor/src/yaml/json/schema/sjp.events.case-document-uploaded.json

@@ -23,7 +23,7 @@
     "caseId",
     "documentType"
   ],
-  "oneOf": [
+  "anyOf": [
     {
       "required": [
         "documentReference"

sjp-event/sjp-event-processor/src/yaml/json/sjp.events.case-document-uploaded.json

@@ -1,5 +1,6 @@
 {
   "caseId": "6ac98e9c-66b2-4363-8a78-641dbec8bbd2",
   "documentReference": "6ac98e9c-66b2-4363-8a78-641dbec8bbd1",
+  "documentReferenceUri": "https://sadevfilestore.blob.core.windows.net/stack-stagingdvla/generated/plea.pdf",
   "documentType": "PLEA"
 }