ACC-2100: add RestController for relations by NielsCW · Pull Request #39 · xenit-eu/contentgrid-appserver

NielsCW · 2025-06-05T09:03:49Z

Add RelationRequestHandler, it implements the following methods from PropertyRequestHandler and PropertyItemRequestHandler:

getProperty: GET /{entity}/{sourceId}/{relation}
- redirect to target entity item if *-to-one relation
- redirect to target entity collection if *-to-many relation (query parameters present but don't work)
putProperty: PUT /{entity}/{sourceId}/{relation}
- set a *-to-one relation
postProperty: POST /{entity}/{sourceId}/{relation}
- add items to a *-to-many relation
deleteProperty: DELETE /{entity}/{sourceId}/{relation}
- clear a *-to-one relation
- clears all items from a *-to-many relation
getPropertyItem: GET /{entity}/{sourceId}/{relation}/{targetId}
- redirect to target entity item of a *-to-many relation
deletePropertyItem: DELETE /{entity}/{sourceId}/{relation}/{targetId}
- remove one item from a *-to-many relation

Add PropertyRestController, it manages all property endpoints (/{entity}/{id}/{property}) and delegates to the correct PropertyRequestHandler:

If the PropertyRequestHandler does not find the property, it throws a PropertyNotFoundException to indicate a next PropertyRequestHandler should handle the property. If all PropertyRequestHandler throw PropertyNotFoundException, the response is a 404 Not Found.
If the PropertyRequestHandler does not understand the media type, it throws an UnsupportedMediaTypeException to indicate a next PropertyRequestHandler should handle the property and media type. (Allows to support multiple media types for the same property type by creating multiple PropertyRequestHandlers.) If some PropertyRequestHandler throws UnsupportedMediaTypeException and none of the handlers returns a ResponseEntity, it is assumed that the property exists and the response is 415 Unsupported Media Type.
If it is able to execute without errors, it simply returns a ResponseEntity<Object>.
If it there are other errors while executing the request, it throws an exception with a http error code.

PropertyItemRestController is similar to PropertyRestController, the only difference is that it manages property item endpoints (/{entity}/{entityId}/{property}/{propertyId})

thijslemmens

As discussed.
Untangle the PropertyRestController from real controllers.
Move the controller mappings to EntityController. Use Factory + Strategy pattern for the different implementations (relation, content, normal property) ...

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

rschev

Personally, I think this has more indirection and complexity than we need. If someone new is reading the code, and they find the PropertyItem- or PropertyRestController, it's not clear at all which things handle these endpoints, and in what order. They'd have to start looking through the codebase for things that implement these Handler interfaces, but then that's an abstract class, you have to find what extends this abstract class, and then you still don't know what order these things are tried in.

It might have been a bit more "wordy", but I would have preferred to read, right in the restcontroller, something like:

if (contentPropertyRequestHandler.matches(entityName, property) {
    return contentPropertyRequestHandler.handle(entityName, instanceId, property);
} else if (xToOneRequestHandler.matches(entityName, property) {
    return xToOneRequestHandler.handle(entityName, instanceId, property);
} else if (xToManyRequestHandler.matches(entityName, property) {
    return xToManyRequestHandler.handle(entityName, instanceId, property);
} else {
    // return error
}

contentgrid-appserver-domain/src/main/java/com/contentgrid/appserver/domain/DatamodelApi.java

rschev · 2025-06-23T06:02:11Z

...main/java/com/contentgrid/appserver/rest/property/handler/XToManyRelationRequestHandler.java

+        datamodelApi.findById(application, property.getSourceEndPoint().getEntity(), instanceId)
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Entity with id %s not found".formatted(instanceId)));
+        var redirectUrl = linkTo(methodOn(EntityRestController.class).listEntity(application, targetPathSegment, 0,
+                Map.of(property.getTargetEndPoint().getName().getValue(), instanceId.toString()))).toUri(); // TODO: use RelationSearchFilter


RelationSearchFilter doesn't exist any more :)
We'll leave this for a next PR, I guess.

Now that I see this, I noticed that this throws a NullPointerException when property.getTargetEndPoint().getName() is null

rschev · 2025-07-01T09:24:28Z

contentgrid-appserver-app/src/main/resources/application.yml

+  mvc.problemdetails.enabled: true
+server:
+  port: ${PORT:8080}
+  max-http-request-header-size: 10MB


10MB is beefy, I reckon I can now paste the whole Lord of the Rings trilogy in a single header. What prompted this change?

I copy pasted this from v1 applications. There it was needed because the gateway adds the residual expression from opa as a thunx expression to the jwt token. And those expressions turned out to be beefy. It is definitely not needed for current state of v2 applications, but might be needed later on.

rschev · 2025-07-01T10:00:47Z

...main/java/com/contentgrid/appserver/rest/property/handler/XToManyRelationRequestHandler.java

+        var targetPathSegment = property.getTargetEndPoint().getEntity().getPathSegment();
+        datamodelApi.findById(application, property.getSourceEndPoint().getEntity(), instanceId)
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Entity with id %s not found".formatted(instanceId)));
+        // TODO: use FilterName of relation


I suppose it's not guaranteed that there is a search filter for this relation... Is this appserver's responsibility, or scribe?

it's for ticket ACC-2149 to find out how to represent it and how scribe can pass the name of the search filter.

rschev · 2025-07-01T11:26:39Z

contentgrid-appserver-rest/src/main/resources/messages.properties

@@ -0,0 +1,5 @@
+problemDetail.type.com.contentgrid.appserver.rest.exception.PropertyNotFoundException=https://contentgrid.cloud/problems/not-found/property
+problemDetail.title.com.contentgrid.appserver.rest.exception.PropertyNotFoundException=Property not found


Any particular reason why this is the only one to get a title?

The default title just says "Not found", I guess it is not really necessary to override it as long as the details contains a detailed message which property does not exist.

because the remarks are taken into account

Support text/uri-list request body types.

Adds: - POST /{entityName}/{sourceId}/{relationName} for adding items to *-to-many relations - PUT /{entityName}/{sourceId}/{relationName} for setting a *-to-one relation - DELETE /{entityName}/{sourceId}/{relationName} for deleting a relation - DELETE /{entityName}/{sourceId}/{relationName}/{targetId} for deleting an item from a *-to-many relation

…ith 'links'

…roperty types

…but different mediaType This is usefull when we want to support file upload with content-type 'multipart/form-data' and '*/*'.

…ne relation

…er and XToManyRelationRequestHandler

…n and UnsupportedMediaTypeException Let them extend ResponseStatusException and UnsupportedMediaTypeStatusException from spring web mvc. So that they will automatically be converted to problem details

…o-many relation

sonarqubecloud · 2025-07-04T12:52:56Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
71.9% Coverage on New Code
2.5% Duplication on New Code

See analysis details on SonarQube Cloud

NielsCW force-pushed the ACC-2100 branch 4 times, most recently from 9b0720b to 97815a4 Compare June 6, 2025 09:22

NielsCW marked this pull request as ready for review June 6, 2025 09:23

NielsCW requested a review from a team as a code owner June 6, 2025 09:23

NielsCW requested review from rschev, thijslemmens and vierbergenlars June 6, 2025 09:23

thijslemmens previously requested changes Jun 6, 2025

View reviewed changes

NielsCW force-pushed the ACC-2100 branch 2 times, most recently from 40267c9 to 1e224da Compare June 17, 2025 08:16

vierbergenlars reviewed Jun 17, 2025

View reviewed changes

...st/src/main/java/com/contentgrid/appserver/rest/property/handler/RelationRequestHandler.java Outdated Show resolved Hide resolved

...st/src/main/java/com/contentgrid/appserver/rest/property/handler/RelationRequestHandler.java Outdated Show resolved Hide resolved

NielsCW force-pushed the ACC-2100 branch from 2ec5678 to 1335805 Compare June 18, 2025 14:56

rschev reviewed Jun 23, 2025

View reviewed changes

NielsCW force-pushed the ACC-2100 branch 4 times, most recently from 29a1cbb to cfa48cd Compare June 25, 2025 13:19

rschev reviewed Jul 1, 2025

View reviewed changes

rschev approved these changes Jul 3, 2025

View reviewed changes

NielsCW added 8 commits July 4, 2025 14:46

ACC-2100 add RelationRestController

9e51316

ACC-2100 add UriListHttpMessageConverter

fc78c09

Support text/uri-list request body types.

ACC-2100 use DataModelApi in RelationRestController

79c258a

ACC-2100 rename relation methods in DatamodelApi to avoid confusing w…

f3cc441

…ith 'links'

ACC-2100 fix relation link

ce52451

ACC-2100 fix sonar issues

3e71198

ACC-2100 eliminate casts after calling Relation.inverse()

f5fb7e9

NielsCW added 24 commits July 4, 2025 14:46

ACC-2100 Add RelationRestControllerTest

e460953

ACC-2100 Add failure tests

edac3eb

ACC-2100 reset mocks to fix tests when running them all together

fe0653d

ACC-2100 use '%n' as newline instead of '\n'

5c84aa8

ACC-2100 add PropertyRestController + refactor RelationRestController

de4761e

ACC-2100 add PropertyRequestHandler hierarchy for handling multiple p…

27d7f77

…roperty types

ACC-2100 return '405 Method not allowed' when operation is not supported

0bb2fef

ACC-2100 fix sonar issues

8563787

ACC-2100 catch parseMediaType() errors

4998797

ACC-2100 support multiple PropertyRequestHandlers for same property, …

64acb37

…but different mediaType This is usefull when we want to support file upload with content-type 'multipart/form-data' and '*/*'.

ACC-2100 add tests for invalid content types

ff4734f

ACC-2100 return 404 when performing item resource request on a *-to-o…

2655aa1

…ne relation

ACC-2100 add javadoc

f1aba69

ACC-2100 split RelationRequestHandler into XToOneRelationRequestHandl…

6d1f7f9

…er and XToManyRelationRequestHandler

ACC-2100 use UriTemplateMatcher from contentgrid-hateoas

f2b2df4

ACC-2100 Always throw when encountering an error

ec0075a

ACC-2100 remove code duplication

5a04f21

ACC-2100 remove unused methods in DatamodelApi

2178e05

ACC-2100 refactor PropertyNotFoundException, IllegalMediaTypeExceptio…

76e9f68

…n and UnsupportedMediaTypeException Let them extend ResponseStatusException and UnsupportedMediaTypeStatusException from spring web mvc. So that they will automatically be converted to problem details

ACC-2100 add application.yml and enable problem details

98d0a80

ACC-2100 Return 501 Not implemented when following unidirectional *-t…

a5e6e81

…o-many relation

ACC-2100 add messages.properties to customize problem details

837e8da

ACC-2100 check problem details in tests

7de70dd

ACC-2100 add comments

c8d7bfc

NielsCW force-pushed the ACC-2100 branch from 3509667 to c8d7bfc Compare July 4, 2025 12:49

NielsCW merged commit 29cb54a into main Jul 4, 2025
3 checks passed

NielsCW deleted the ACC-2100 branch July 4, 2025 13:24

		@@ -0,0 +1,5 @@
		problemDetail.type.com.contentgrid.appserver.rest.exception.PropertyNotFoundException=https://contentgrid.cloud/problems/not-found/property
		problemDetail.title.com.contentgrid.appserver.rest.exception.PropertyNotFoundException=Property not found

Conversation

NielsCW commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thijslemmens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rschev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 4, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NielsCW commented Jun 5, 2025 •

edited

Loading