ACC-2100 return 404 when performing item resource request on a *-to-one relation

Author: NielsCW

contentgrid-appserver-rest/src/main/java/com/contentgrid/appserver/rest/property/handler/RelationRequestHandler.java

@@ -37,7 +37,6 @@ public class RelationRequestHandler extends AbstractPropertyItemRequestHandler<L
 
     private static final HttpMethod[] SINGLE_TARGET_PROPERTY_METHODS = {HttpMethod.GET, HttpMethod.HEAD, HttpMethod.PUT, HttpMethod.DELETE};
     private static final HttpMethod[] MULTI_TARGET_PROPERTY_METHODS = {HttpMethod.GET, HttpMethod.HEAD, HttpMethod.POST, HttpMethod.DELETE};
-    private static final HttpMethod[] SINGLE_TARGET_PROPERTY_ITEM_METHODS = {HttpMethod.GET, HttpMethod.HEAD};
     private static final HttpMethod[] MULTI_TARGET_PROPERTY_ITEM_METHODS = {HttpMethod.GET, HttpMethod.HEAD, HttpMethod.DELETE};
 
     private static HttpMethod[] supportedPropertyMethods(Relation relation) {
@@ -48,11 +47,15 @@ private static HttpMethod[] supportedPropertyMethods(Relation relation) {
         }
     }
 
-    private static HttpMethod[] supportedPropertyItemMethods(Relation relation) {
+    private static ResponseEntity<Object> propertyItemUnsupportedMethodResponse(Relation relation) {
         if (relation instanceof OneToManyRelation || relation instanceof ManyToManyRelation) {
-            return MULTI_TARGET_PROPERTY_ITEM_METHODS;
+            // *-to-many relation
+            return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
+                    .allow(MULTI_TARGET_PROPERTY_ITEM_METHODS)
+                    .build();
         } else {
-            return SINGLE_TARGET_PROPERTY_ITEM_METHODS;
+            // *-to-one relation
+            return ResponseEntity.notFound().build();
         }
     }
 
@@ -245,15 +248,15 @@ protected ResponseEntity<Object> getPropertyItem(
             Relation property,
             EntityId itemId
     ) {
-        try {
-            if (datamodelApi.hasRelationTarget(application, property, instanceId, itemId)) {
-                var uri = linkTo(methodOn(EntityRestController.class).getEntity(application, entity.getPathSegment(), instanceId)).toUri();
-                return ResponseEntity.status(HttpStatus.FOUND).location(uri).build();
-            } else {
-                return ResponseEntity.notFound().build();
-            }
-        } catch (EntityNotFoundException e) {
-            throw new ResponseStatusException(HttpStatus.NOT_FOUND, e.getMessage(), e);
+        if (!(property instanceof OneToManyRelation || property instanceof ManyToManyRelation)) {
+            return ResponseEntity.notFound().build();
+        }
+
+        if (datamodelApi.hasRelationTarget(application, property, instanceId, itemId)) {
+            var uri = linkTo(methodOn(EntityRestController.class).getEntity(application, property.getTargetEndPoint().getEntity().getPathSegment(), itemId)).toUri();
+            return ResponseEntity.status(HttpStatus.FOUND).location(uri).build();
+        } else {
+            return ResponseEntity.notFound().build();
         }
     }
 
@@ -266,9 +269,7 @@ protected ResponseEntity<Object> postPropertyItem(
             EntityId itemId,
             List<URI> body
     ) {
-        return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
-                .allow(supportedPropertyItemMethods(property))
-                .build();
+        return propertyItemUnsupportedMethodResponse(property);
     }
 
     @Override
@@ -280,9 +281,7 @@ protected ResponseEntity<Object> putPropertyItem(
             EntityId itemId,
             List<URI> body
     ) {
-        return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
-                .allow(supportedPropertyItemMethods(property))
-                .build();
+        return propertyItemUnsupportedMethodResponse(property);
     }
 
     @Override
@@ -294,9 +293,7 @@ protected ResponseEntity<Object> patchPropertyItem(
             EntityId itemId,
             List<URI> body
     ) {
-        return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
-                .allow(supportedPropertyItemMethods(property))
-                .build();
+        return propertyItemUnsupportedMethodResponse(property);
     }
 
     @Override
@@ -308,9 +305,7 @@ protected ResponseEntity<Object> deletePropertyItem(
             EntityId itemId
     ) {
         if (!(property instanceof OneToManyRelation || property instanceof ManyToManyRelation)) {
-            return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED)
-                    .allow(SINGLE_TARGET_PROPERTY_ITEM_METHODS)
-                    .build();
+            return ResponseEntity.notFound().build();
         }
 
         try {

contentgrid-appserver-rest/src/test/java/com/contentgrid/appserver/rest/property/handler/RelationRequestHandlerTest.java

@@ -164,6 +164,38 @@ void followManyToManyRelation() throws Exception {
                     .findById(TestApplication.APPLICATION, TestApplication.PRODUCT, PRODUCT_ID);
         }
 
+        @Test
+        void followOneToManyRelationItem() throws Exception {
+            var targetId = EntityId.of(UUID.randomUUID());
+
+            Mockito.doReturn(true).when(datamodelApi)
+                    .hasRelationTarget(TestApplication.APPLICATION, TestApplication.PERSON_INVOICES, PERSON_ID, targetId);
+
+            mockMvc.perform(get("/persons/{sourceId}/invoices/{targetId}", PERSON_ID, targetId))
+                    .andExpect(status().isFound())
+                    .andExpect(
+                            header().string(HttpHeaders.LOCATION, "http://localhost/invoices/%s".formatted(targetId)));
+
+            Mockito.verify(datamodelApi)
+                    .hasRelationTarget(TestApplication.APPLICATION, TestApplication.PERSON_INVOICES, PERSON_ID, targetId);
+        }
+
+        @Test
+        void followManyToManyRelationItem() throws Exception {
+            var targetId = EntityId.of(UUID.randomUUID());
+
+            Mockito.doReturn(true).when(datamodelApi)
+                    .hasRelationTarget(TestApplication.APPLICATION, TestApplication.PRODUCT_INVOICES, PRODUCT_ID, targetId);
+
+            mockMvc.perform(get("/products/{sourceId}/invoices/{targetId}", PRODUCT_ID, targetId))
+                    .andExpect(status().isFound())
+                    .andExpect(
+                            header().string(HttpHeaders.LOCATION, "http://localhost/invoices/%s".formatted(targetId)));
+
+            Mockito.verify(datamodelApi)
+                    .hasRelationTarget(TestApplication.APPLICATION, TestApplication.PRODUCT_INVOICES, PRODUCT_ID, targetId);
+        }
+
         @Test
         void setOneToOneRelation() throws Exception {
             var targetId = EntityId.of(UUID.randomUUID());
@@ -315,8 +347,9 @@ static Stream<String> invalidContentType() {
 
         @ParameterizedTest
         @CsvSource({
-                "/invoices/01234567-89ab-cdef-0123-456789abcdef/previous", // non-existent relation
-                "/invoice/01234567-89ab-cdef-0123-456789abcdef/previous-invoice", // non-existent entity
+                "/invoices/01234567-89ab-cdef-0123-456789abcdef/non-existing", // non-existing relation
+                "/non-existing/01234567-89ab-cdef-0123-456789abcdef/previous-invoice", // non-existing entity
+                "/invoices/01234567-89ab-cdef-0123-456789abcdef/non-existing/01234567-89ab-cdef-0123-456789abcdef", // non-existing relation
         })
         void followRelationInvalidUrl(String url) throws Exception {
             mockMvc.perform(get(url))
@@ -410,12 +443,8 @@ static Stream<Arguments> unsupportedMethod() {
                     Arguments.of(HttpMethod.PATCH, "/invoices/%s/previous-invoice".formatted(INVOICE_ID)),
                     // property item endpoint
                     Arguments.of(HttpMethod.POST, "/persons/%s/invoices/%s".formatted(PERSON_ID, targetId)),
-                    Arguments.of(HttpMethod.POST, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
                     Arguments.of(HttpMethod.PUT, "/persons/%s/invoices/%s".formatted(PERSON_ID, targetId)),
-                    Arguments.of(HttpMethod.PUT, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
-                    Arguments.of(HttpMethod.PATCH, "/persons/%s/invoices/%s".formatted(PERSON_ID, targetId)),
-                    Arguments.of(HttpMethod.PATCH, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
-                    Arguments.of(HttpMethod.DELETE, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId))
+                    Arguments.of(HttpMethod.PATCH, "/persons/%s/invoices/%s".formatted(PERSON_ID, targetId))
             );
         }
 
@@ -434,6 +463,30 @@ void unsupportedMethod(HttpMethod method, String url) throws Exception {
                     .andExpect(header().string(HttpHeaders.ALLOW, not(containsString(method.name()))));
         }
 
+        static Stream<Arguments> unsupportedUrl() {
+            var targetId = EntityId.of(UUID.randomUUID());
+            return Stream.of(
+                    // property item endpoint of *-to-one relation
+                    Arguments.of(HttpMethod.GET, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
+                    Arguments.of(HttpMethod.POST, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
+                    Arguments.of(HttpMethod.PUT, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
+                    Arguments.of(HttpMethod.PATCH, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId)),
+                    Arguments.of(HttpMethod.DELETE, "/invoices/%s/previous-invoice/%s".formatted(INVOICE_ID, targetId))
+            );
+        }
+
+        @ParameterizedTest
+        @MethodSource
+        void unsupportedUrl(HttpMethod method, String url) throws Exception {
+            var requestBuilder = request(method, url);
+            if (Set.of(HttpMethod.POST, HttpMethod.PUT, HttpMethod.PATCH).contains(method)) {
+                requestBuilder = requestBuilder.contentType("text/uri-list")
+                        .content("http://localhost/invoices/%s%n".formatted(UUID.randomUUID()));
+            }
+            mockMvc.perform(requestBuilder)
+                    .andExpect(status().isNotFound());
+        }
+
     }
 
     @Nested
@@ -470,6 +523,18 @@ void followToManyRelationSourceIdNotFound() throws Exception {
                     .andExpect(status().isNotFound());
         }
 
+        @Test
+        void followToManyRelationItemSourceIdOrTargetIdNotFound() throws Exception {
+            var targetId = EntityId.of(UUID.randomUUID());
+
+            // Returns false if sourceId or targetId does not exist, or if there is no relation between sourceId and targetId
+            Mockito.doReturn(false).when(datamodelApi)
+                    .hasRelationTarget(TestApplication.APPLICATION, TestApplication.PERSON_INVOICES, PERSON_ID, targetId);
+
+            mockMvc.perform(get("/persons/{sourceId}/invoices/{targetId}", PERSON_ID, targetId))
+                    .andExpect(status().isNotFound());
+        }
+
         @Test
         void setRelationEntityIdNotFound() throws Exception {
             var targetId = EntityId.of(UUID.randomUUID());