From ec66efdfe6232c90dfbda996e04dbfd26f013ec9 Mon Sep 17 00:00:00 2001 From: Dapeng Wang Date: Fri, 2 Jun 2023 14:29:13 +0200 Subject: [PATCH] feat: return 200 (not 204) for PUT requests to entity verticles --- docs/usage/verticles/EntityVerticle.md | 35 ++++ .../olingo/processor/EntityProcessor.java | 196 ++++++++++-------- .../endpoint/odata/ODataCreateEntityTest.java | 18 ++ .../endpoint/odata/ODataUpdateEntityTest.java | 71 +++++-- .../verticle/TestService3EntityVerticle.java | 12 +- 5 files changed, 229 insertions(+), 103 deletions(-) create mode 100644 docs/usage/verticles/EntityVerticle.md diff --git a/docs/usage/verticles/EntityVerticle.md b/docs/usage/verticles/EntityVerticle.md new file mode 100644 index 000000000..2ec40e476 --- /dev/null +++ b/docs/usage/verticles/EntityVerticle.md @@ -0,0 +1,35 @@ +# EntityVerticle + +# Create + +A `POST` request will be used to create a new entity. In this case, the `Future createData(DataQuery query, DataContext context)` method of the responsible `EntityVerticle` will be invoked. +According to the best-practices of REST-API, the response of such a POST-request should +* contain a `Location`-header with the location of the newly created entity +* return the entity representation in the response body + +To achieve this, a `EntityVertile` should in the `createEntity`-method +* set the `Location` in the response context data +``` +context.responseData().put("Location", /io.neonbee.test3.TestService3/TestCars(/unique-id)); +``` +* return a filled `EntityWrapper` +``` +return Future.succeededFuture(new EntityWrapper(TEST_ENTITY_SET_FQN, createEntity(ENTITY_DATA_1)); +``` +In this case, a 201 status code with the entity representation will be returned in the HTTP-response. +The response context data under the key `Location` will be set as HTTP-header as well. + +Just for backward-compatibility, a 204 status code will be returned, if no entity is returned in the `EntityWrapper` by the verticle. + +# Update + +A `PUT` request will be used to update an existing entity. In this case, the `Future updateData(DataQuery query, DataContext context)` method of the responsible `EntityVerticle` will be invoked. +According to the best-practices of REST-API, the response of such a PUT-request should return the entity representation in the response body + +To achieve this, a `EntityVertile` should return a filled `EntityWrapper` in the `updateEntity`-method +``` +return Future.succeededFuture(new EntityWrapper(TEST_ENTITY_SET_FQN, createEntity(ENTITY_DATA_1)); +``` +In this case, a 200 status code with the entity representation will be returned in the HTTP-response. + +Just for backward-compatibility, a 204 status code will be returned, if no entity is returned in the `EntityWrapper` by the verticle. diff --git a/src/main/java/io/neonbee/endpoint/odatav4/internal/olingo/processor/EntityProcessor.java b/src/main/java/io/neonbee/endpoint/odatav4/internal/olingo/processor/EntityProcessor.java index d2ca7bf22..7f9ce0c90 100644 --- a/src/main/java/io/neonbee/endpoint/odatav4/internal/olingo/processor/EntityProcessor.java +++ b/src/main/java/io/neonbee/endpoint/odatav4/internal/olingo/processor/EntityProcessor.java @@ -87,6 +87,73 @@ public EntityProcessor(Vertx vertx, RoutingContext routingContext, Promise super(vertx, routingContext, processPromise); } + static Entity findEntityByKeyPredicates(RoutingContext routingContext, UriResourceEntitySet uriResourceEntitySet, + List entities) throws ODataApplicationException { + if (entities == null || entities.isEmpty()) { + return null; + } + // Get the key predicates used to select a single entity out of the start entity set, or an empty map if + // not used. For OData services the canonical form of an absolute URI identifying a single Entity is + // formed by adding a single path segment to the service root URI. The path segment is made up of the + // name of the Service associated with the Entity followed by the key predicate identifying the Entry + // within the Entity Set (list of entities). + // The canonical key predicate for single-part keys consists only of the key property value without the + // key property name. For multi-part keys the key properties appear in the same order they appear in the + // key definition in the service metadata. + // See + // https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_CanonicalURL + // for details. + Map keyPredicates = uriResourceEntitySet.getKeyPredicates().stream() + .collect(Collectors.toUnmodifiableMap(UriParameter::getName, UriParameter::getText)); + // If key predicates were provided apply the filter to the list of entities received + + // The names of the key properties provided in the key predicate query + Set keyPropertyNames = keyPredicates.keySet(); + List foundEntities = entities.stream().filter(Objects::nonNull).filter(entity -> { + // Get the names of all properties of the entity + List propertyNames = entity.getProperties().stream().filter(Objects::nonNull).map(Property::getName) + .collect(Collectors.toUnmodifiableList()); + + // Check if the entity contains all key properties + return propertyNames.containsAll(keyPropertyNames) // + // Find the entities matching all keys + && keyPropertyNames.stream().filter(Objects::nonNull).allMatch(keyPropertyName -> { // + // Get the provided value of the key predicate + String keyPropertyValue = + EdmHelper.extractValueFromLiteral(routingContext, keyPredicates.get(keyPropertyName)); + + // Check if the entity's key property matches the key property from the key + // predicate by comparing the provided value with the current entity's value + try { + // Get the EdmPrimitiveTypeKind like Edm.String or Edm.Int32 of the key property + EdmPrimitiveTypeKind edmPrimitiveTypeKind = + EdmHelper.getEdmPrimitiveTypeKindByPropertyType(uriResourceEntitySet.getEntitySet() + .getEntityType().getProperty(keyPropertyName).getType().toString()); + + // Get the value of the key property of the current entity + Property property = entity.getProperty(keyPropertyName); + Object propertyValue = property.getValue(); + + // Compare the provided key property value with the entity's current value + return ENTITY_COMPARISON.comparePropertyValues(routingContext, propertyValue, + keyPropertyValue, edmPrimitiveTypeKind, keyPropertyName) == 0; + } catch (ODataApplicationException e) { + LOGGER.correlateWith(routingContext).error(e.getMessage(), e); + } + return false; + }); + }).collect(Collectors.toUnmodifiableList()); + if (foundEntities.size() == 1) { + return foundEntities.get(0); + } else if (foundEntities.size() > 1) { + throw new ODataApplicationException( + "Error during processing the request. More than one entity with the same ids (key properties) " + + "was found, but ids (key properties) have to be unique.", + INTERNAL_SERVER_ERROR.getStatusCode(), Locale.ENGLISH); + } + return null; + } + @Override public void init(OData odata, ServiceMetadata serviceMetadata) { this.odata = odata; @@ -113,18 +180,24 @@ public void createEntity(ODataRequest request, ODataResponse response, UriInfo u forwardRequest(request, CREATE, entity, uriInfo, vertx, routingContext, processPromise) .onSuccess(createdEntity -> { try { - ContextURL contextUrl = - ContextURL.with().entitySet(uriResourceEntitySet.getEntitySet()).build(); - EntitySerializerOptions opts = EntitySerializerOptions.with().contextURL(contextUrl).build(); - response.setContent(odata.createSerializer(responseFormat) - .entity(serviceMetadata, uriResourceEntitySet.getEntitySet().getEntityType(), - createdEntity.getEntity(), opts) - .getContent()); - response.setStatusCode(CREATED.getStatusCode()); - response.setHeader(HttpHeader.CONTENT_TYPE, responseFormat.toContentTypeString()); - Optional.ofNullable( - routingContext.get(ProcessorHelper.RESPONSE_HEADER_PREFIX + LOCATION_HEADER)) - .ifPresent(location -> response.setHeader(LOCATION_HEADER, location)); + if (createdEntity.getEntity() != null) { + ContextURL contextUrl = + ContextURL.with().entitySet(uriResourceEntitySet.getEntitySet()).build(); + EntitySerializerOptions opts = + EntitySerializerOptions.with().contextURL(contextUrl).build(); + response.setContent(odata.createSerializer(responseFormat) + .entity(serviceMetadata, uriResourceEntitySet.getEntitySet().getEntityType(), + createdEntity.getEntity(), opts) + .getContent()); + response.setStatusCode(CREATED.getStatusCode()); + response.setHeader(HttpHeader.CONTENT_TYPE, responseFormat.toContentTypeString()); + Optional.ofNullable( + routingContext + .get(ProcessorHelper.RESPONSE_HEADER_PREFIX + LOCATION_HEADER)) + .ifPresent(location -> response.setHeader(LOCATION_HEADER, location)); + } else { + response.setStatusCode(NO_CONTENT.getStatusCode()); + } processPromise.complete(); } catch (SerializerException e) { processPromise.fail(e); @@ -140,16 +213,28 @@ public void updateEntity(ODataRequest request, ODataResponse response, UriInfo u Entity entity = parseBody(request, uriResourceEntitySet, requestFormat); EdmHelper.addKeyPredicateValues(entity, uriResourceEntitySet, routingContext); - forwardRequest(request, UPDATE, entity, uriInfo, vertx, routingContext, processPromise).onSuccess(ew -> { - /* - * TODO: Upon successful completion, the service responds with either 200 OK (in this case, the response - * body MUST contain the resource updated), or 204 No Content (in this case the response body is empty). The - * client may request that the response SHOULD include a body by specifying a Prefer header with a value of - * return=representation, or by specifying the system query options $select or $expand. - */ - response.setStatusCode(NO_CONTENT.getStatusCode()); - processPromise.complete(); - }); + forwardRequest(request, UPDATE, entity, uriInfo, vertx, routingContext, processPromise) + .onSuccess(updatedEntity -> { + try { + if (updatedEntity.getEntity() != null) { + ContextURL contextUrl = + ContextURL.with().entitySet(uriResourceEntitySet.getEntitySet()).build(); + EntitySerializerOptions opts = + EntitySerializerOptions.with().contextURL(contextUrl).build(); + response.setContent(odata.createSerializer(responseFormat) + .entity(serviceMetadata, uriResourceEntitySet.getEntitySet().getEntityType(), + updatedEntity.getEntity(), opts) + .getContent()); + response.setStatusCode(OK.getStatusCode()); + response.setHeader(HttpHeader.CONTENT_TYPE, responseFormat.toContentTypeString()); + } else { + response.setStatusCode(NO_CONTENT.getStatusCode()); + } + processPromise.complete(); + } catch (SerializerException e) { + processPromise.fail(e); + } + }); } @Override @@ -219,71 +304,4 @@ private Handler handleReadEntityResult(UriInfo uriInfo, ODataResp } }; } - - static Entity findEntityByKeyPredicates(RoutingContext routingContext, UriResourceEntitySet uriResourceEntitySet, - List entities) throws ODataApplicationException { - if (entities == null || entities.isEmpty()) { - return null; - } - // Get the key predicates used to select a single entity out of the start entity set, or an empty map if - // not used. For OData services the canonical form of an absolute URI identifying a single Entity is - // formed by adding a single path segment to the service root URI. The path segment is made up of the - // name of the Service associated with the Entity followed by the key predicate identifying the Entry - // within the Entity Set (list of entities). - // The canonical key predicate for single-part keys consists only of the key property value without the - // key property name. For multi-part keys the key properties appear in the same order they appear in the - // key definition in the service metadata. - // See - // https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_CanonicalURL - // for details. - Map keyPredicates = uriResourceEntitySet.getKeyPredicates().stream() - .collect(Collectors.toUnmodifiableMap(UriParameter::getName, UriParameter::getText)); - // If key predicates were provided apply the filter to the list of entities received - - // The names of the key properties provided in the key predicate query - Set keyPropertyNames = keyPredicates.keySet(); - List foundEntities = entities.stream().filter(Objects::nonNull).filter(entity -> { - // Get the names of all properties of the entity - List propertyNames = entity.getProperties().stream().filter(Objects::nonNull).map(Property::getName) - .collect(Collectors.toUnmodifiableList()); - - // Check if the entity contains all key properties - return propertyNames.containsAll(keyPropertyNames) // - // Find the entities matching all keys - && keyPropertyNames.stream().filter(Objects::nonNull).allMatch(keyPropertyName -> { // - // Get the provided value of the key predicate - String keyPropertyValue = - EdmHelper.extractValueFromLiteral(routingContext, keyPredicates.get(keyPropertyName)); - - // Check if the entity's key property matches the key property from the key - // predicate by comparing the provided value with the current entity's value - try { - // Get the EdmPrimitiveTypeKind like Edm.String or Edm.Int32 of the key property - EdmPrimitiveTypeKind edmPrimitiveTypeKind = - EdmHelper.getEdmPrimitiveTypeKindByPropertyType(uriResourceEntitySet.getEntitySet() - .getEntityType().getProperty(keyPropertyName).getType().toString()); - - // Get the value of the key property of the current entity - Property property = entity.getProperty(keyPropertyName); - Object propertyValue = property.getValue(); - - // Compare the provided key property value with the entity's current value - return ENTITY_COMPARISON.comparePropertyValues(routingContext, propertyValue, - keyPropertyValue, edmPrimitiveTypeKind, keyPropertyName) == 0; - } catch (ODataApplicationException e) { - LOGGER.correlateWith(routingContext).error(e.getMessage(), e); - } - return false; - }); - }).collect(Collectors.toUnmodifiableList()); - if (foundEntities.size() == 1) { - return foundEntities.get(0); - } else if (foundEntities.size() > 1) { - throw new ODataApplicationException( - "Error during processing the request. More than one entity with the same ids (key properties) " - + "was found, but ids (key properties) have to be unique.", - INTERNAL_SERVER_ERROR.getStatusCode(), Locale.ENGLISH); - } - return null; - } } diff --git a/src/test/java/io/neonbee/test/endpoint/odata/ODataCreateEntityTest.java b/src/test/java/io/neonbee/test/endpoint/odata/ODataCreateEntityTest.java index 357fa7f10..1ba23f7e8 100644 --- a/src/test/java/io/neonbee/test/endpoint/odata/ODataCreateEntityTest.java +++ b/src/test/java/io/neonbee/test/endpoint/odata/ODataCreateEntityTest.java @@ -40,6 +40,7 @@ void setUp(VertxTestContext testContext) { void createEntityTest(VertxTestContext testContext) { ODataRequest oDataRequest = new ODataRequest(TEST_ENTITY_SET_FQN).setMethod(HttpMethod.POST) .setBody(ENTITY_DATA_1.toBuffer()) + .addHeader("expectResponseBody", "true") .addHeader(HttpHeaders.CONTENT_TYPE.toString(), MediaType.JSON_UTF_8.toString()); requestOData(oDataRequest).onComplete(testContext.succeeding(response -> { @@ -55,6 +56,23 @@ void createEntityTest(VertxTestContext testContext) { })); } + @Test + @DisplayName("Respond with 204 No Content for backward compatibility") + void createEntityTestWithNoResponse(VertxTestContext testContext) { + ODataRequest oDataRequest = new ODataRequest(TEST_ENTITY_SET_FQN).setMethod(HttpMethod.POST) + .setBody(ENTITY_DATA_1.toBuffer()) + .addHeader(HttpHeaders.CONTENT_TYPE.toString(), MediaType.JSON_UTF_8.toString()); + + requestOData(oDataRequest).onComplete(testContext.succeeding(response -> { + testContext.verify(() -> { + assertThat(response.statusCode()).isEqualTo(204); + assertThat(response.getHeader("Location")).isNull(); + assertThat(response.body()).isNull(); + }); + testContext.completeNow(); + })); + } + @Test @DisplayName("Respond with 400") void createEntityTestWithWrongPayload(VertxTestContext testContext) { diff --git a/src/test/java/io/neonbee/test/endpoint/odata/ODataUpdateEntityTest.java b/src/test/java/io/neonbee/test/endpoint/odata/ODataUpdateEntityTest.java index 8c60af032..b7a826216 100644 --- a/src/test/java/io/neonbee/test/endpoint/odata/ODataUpdateEntityTest.java +++ b/src/test/java/io/neonbee/test/endpoint/odata/ODataUpdateEntityTest.java @@ -1,6 +1,8 @@ package io.neonbee.test.endpoint.odata; import static com.google.common.truth.Truth.assertThat; +import static io.neonbee.test.endpoint.odata.verticle.TestService3EntityVerticle.ENTITY_DATA_1_UPDATED; +import static io.neonbee.test.endpoint.odata.verticle.TestService3EntityVerticle.TEST_ENTITY_SET_FQN; import java.nio.file.Path; import java.time.LocalDate; @@ -30,6 +32,7 @@ import io.neonbee.test.base.ODataEndpointTestBase; import io.neonbee.test.base.ODataRequest; import io.neonbee.test.endpoint.odata.verticle.TestService1EntityVerticle; +import io.neonbee.test.endpoint.odata.verticle.TestService3EntityVerticle; import io.neonbee.test.endpoint.odata.verticle.TestServiceCompoundKeyEntityVerticle; import io.neonbee.test.helper.EntityHelper; import io.vertx.core.Future; @@ -44,10 +47,23 @@ class ODataUpdateEntityTest extends ODataEndpointTestBase { private ODataRequest oDataRequest; + static Stream withHTTPMethods() { + return Stream.of(Arguments.of(HttpMethod.PUT), Arguments.of(HttpMethod.PATCH)); + } + + static Stream withHTTPMethodsAndBody() { + JsonObject putBody = TestService1EntityVerticle.EXPECTED_ENTITY_DATA_1.copy(); + putBody.remove("KeyPropertyString"); + JsonObject patchBody = new JsonObject().put("PropertyString", "New String"); + + return Stream.of(Arguments.of(HttpMethod.PUT, putBody), Arguments.of(HttpMethod.PATCH, patchBody)); + } + @Override protected List provideEntityModels() { return List.of(TestService1EntityVerticle.getDeclaredEntityModel(), - TestServiceCompoundKeyEntityVerticle.getDeclaredEntityModel()); + TestServiceCompoundKeyEntityVerticle.getDeclaredEntityModel(), + TestService3EntityVerticle.getDeclaredEntityModel()); } @BeforeEach @@ -56,10 +72,6 @@ void setUp() { .addHeader(HttpHeaders.CONTENT_TYPE.toString(), MediaType.JSON_UTF_8.toString()); } - static Stream withHTTPMethods() { - return Stream.of(Arguments.of(HttpMethod.PUT), Arguments.of(HttpMethod.PATCH)); - } - @ParameterizedTest(name = "{index}: with Method {0}") @MethodSource("withHTTPMethods") @DisplayName("Respond with 405 METHOD NOT ALLOWED if the HTTP request was sent to the entity set url instead of a dedicated entity") @@ -86,14 +98,6 @@ void updateEntityInvalidBodyTest(HttpMethod method, VertxTestContext testContext })); } - static Stream withHTTPMethodsAndBody() { - JsonObject putBody = TestService1EntityVerticle.EXPECTED_ENTITY_DATA_1.copy(); - putBody.remove("KeyPropertyString"); - JsonObject patchBody = new JsonObject().put("PropertyString", "New String"); - - return Stream.of(Arguments.of(HttpMethod.PUT, putBody), Arguments.of(HttpMethod.PATCH, patchBody)); - } - @ParameterizedTest(name = "{index}: with Method {0}") @MethodSource("withHTTPMethodsAndBody") @DisplayName("Respond with 204 NO CONTENT if an entity was successfully updated") @@ -118,6 +122,47 @@ void updateEntityTest(HttpMethod method, JsonObject body, VertxTestContext testC })); } + @Test + @DisplayName("Respond with 200 OK and entity in body") + void updateEntityTest(VertxTestContext testContext) { + ODataRequest oDataRequest = new ODataRequest(TEST_ENTITY_SET_FQN) + .setMethod(HttpMethod.PUT).setBody(ENTITY_DATA_1_UPDATED.toBuffer()) + .setKey(Map.of("ID", 0L)) + .addHeader("expectResponseBody", "true") + .addHeader(HttpHeaders.CONTENT_TYPE.toString(), ContentType.APPLICATION_JSON.toContentTypeString()); + + deployVerticle(new TestService3EntityVerticle()) + .compose(v -> requestOData(oDataRequest).onComplete(testContext.succeeding(response -> { + testContext.verify(() -> { + assertThat(response.statusCode()).isEqualTo(200); + JsonObject body = response.body().toJsonObject(); + assertThat(body.getString("ID")).isEqualTo(ENTITY_DATA_1_UPDATED.getString("ID")); + assertThat(body.getString("name")).isEqualTo(ENTITY_DATA_1_UPDATED.getString("name")); + assertThat(body.getString("description")) + .isEqualTo(ENTITY_DATA_1_UPDATED.getString("description")); + }); + testContext.completeNow(); + }))); + } + + @Test + @DisplayName("Respond with 204 empty body") + void updateEntityTestWithoutResponse(VertxTestContext testContext) { + ODataRequest oDataRequest = new ODataRequest(TEST_ENTITY_SET_FQN) + .setMethod(HttpMethod.PUT).setBody(ENTITY_DATA_1_UPDATED.toBuffer()) + .setKey(Map.of("ID", 0L)) + .addHeader(HttpHeaders.CONTENT_TYPE.toString(), ContentType.APPLICATION_JSON.toContentTypeString()); + + deployVerticle(new TestService3EntityVerticle()) + .compose(v -> requestOData(oDataRequest).onComplete(testContext.succeeding(response -> { + testContext.verify(() -> { + assertThat(response.statusCode()).isEqualTo(204); + assertThat(response.body()).isNull(); + }); + testContext.completeNow(); + }))); + } + @Test @DisplayName("Respond with 204 NO CONTENT if an entity with a compound key was successfully updated") void updateEntityWithCompoundKeyViaPutTest(VertxTestContext testContext) { diff --git a/src/test/java/io/neonbee/test/endpoint/odata/verticle/TestService3EntityVerticle.java b/src/test/java/io/neonbee/test/endpoint/odata/verticle/TestService3EntityVerticle.java index 0d629b60e..147547a07 100644 --- a/src/test/java/io/neonbee/test/endpoint/odata/verticle/TestService3EntityVerticle.java +++ b/src/test/java/io/neonbee/test/endpoint/odata/verticle/TestService3EntityVerticle.java @@ -25,6 +25,9 @@ public class TestService3EntityVerticle extends EntityVerticle { public static final JsonObject ENTITY_DATA_1 = new JsonObject().put("ID", 0).put("name", "Car 0").put("description", "This is Car 0"); + public static final JsonObject ENTITY_DATA_1_UPDATED = + new JsonObject().put("ID", 0).put("name", "Car 0").put("description", "This is Car 0 updated"); + public static final JsonObject ENTITY_DATA_2 = new JsonObject().put("ID", 1).put("name", "Car 1").put("description", "This is Car 1"); @@ -45,7 +48,14 @@ public Future retrieveData(DataQuery query, DataContext context) @Override public Future createData(DataQuery query, DataContext context) { context.responseData().put("Location", ENTITY_URL); - return Future.succeededFuture(new EntityWrapper(TEST_ENTITY_SET_FQN, createEntity(ENTITY_DATA_1))); + return Future.succeededFuture(new EntityWrapper(TEST_ENTITY_SET_FQN, + query.getHeaders().get("expectResponseBody") != null ? createEntity(ENTITY_DATA_1) : null)); + } + + @Override + public Future updateData(DataQuery query, DataContext context) { + return Future.succeededFuture(new EntityWrapper(TEST_ENTITY_SET_FQN, + query.getHeaders().get("expectResponseBody") != null ? createEntity(ENTITY_DATA_1_UPDATED) : null)); } public static Path getDeclaredEntityModel() {