Skip to content

Commit

Permalink
Merge pull request #35181 from Sgitario/fix_hal_rest_data_panache
Browse files Browse the repository at this point in the history
REST Data with Panache should not produce links when hal is disabled
  • Loading branch information
Sgitario committed Aug 4, 2023
2 parents a3e6896 + 362f03a commit 57c036a
Show file tree
Hide file tree
Showing 14 changed files with 191 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.quarkus.hibernate.orm.rest.data.panache.deployment.entity;

import static io.restassured.RestAssured.given;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.restassured.response.Response;

class PanacheEntityResourceHalDisabledTest {

@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(Project.class, ProjectResource.class)
.addAsResource("application.properties"));

@Test
void shouldHalNotBeSupported() {
given().accept("application/hal+json")
.when().get("/group/projects/1")
.then().statusCode(406);
}

@Test
void shouldNotContainLocationAndLinks() {
Response response = given().accept("application/json")
.and().contentType("application/json")
.and().body("{\"name\": \"projectname\"}")
.when().post("/group/projects")
.thenReturn();
assertThat(response.statusCode()).isEqualTo(201);
assertThat(response.header("Location")).isBlank();
assertThat(response.getHeaders().getList("Link")).isEmpty();

response = given().accept("application/json")
.when().get("/group/projects/projectname")
.thenReturn();
assertThat(response.statusCode()).isEqualTo(200);
assertThat(response.header("Location")).isBlank();
assertThat(response.getHeaders().getList("Link")).isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.quarkus.hibernate.orm.rest.data.panache.deployment.entity;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;

import io.quarkus.hibernate.orm.panache.PanacheEntityBase;

@Entity
public class Project extends PanacheEntityBase {

@Id
public String name;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.quarkus.hibernate.orm.rest.data.panache.deployment.entity;

import io.quarkus.hibernate.orm.rest.data.panache.PanacheEntityResource;
import io.quarkus.rest.data.panache.ResourceProperties;

/**
* Having a path param in the path reproduces the issue of having HAL enabled spites it should be disabled by default.
*/
@ResourceProperties(path = "/{group}/projects")
public interface ProjectResource extends PanacheEntityResource<Project, String> {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.quarkus.hibernate.reactive.rest.data.panache.deployment.entity;

import static io.restassured.RestAssured.given;
import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.restassured.response.Response;

class PanacheEntityResourceHalDisabledTest {

@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(Project.class, ProjectResource.class)
.addAsResource("application.properties"));

@Test
void shouldHalNotBeSupported() {
given().accept("application/hal+json")
.when().get("/group/projects/1")
.then().statusCode(406);
}

@Test
void shouldNotContainLocationAndLinks() {
Response response = given().accept("application/json")
.and().contentType("application/json")
.and().body("{\"name\": \"projectname\"}")
.when().post("/group/projects")
.thenReturn();
assertThat(response.statusCode()).isEqualTo(201);
assertThat(response.header("Location")).isBlank();
assertThat(response.getHeaders().getList("Link")).isEmpty();

response = given().accept("application/json")
.when().get("/group/projects/projectname")
.thenReturn();
assertThat(response.statusCode()).isEqualTo(200);
assertThat(response.header("Location")).isBlank();
assertThat(response.getHeaders().getList("Link")).isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package io.quarkus.hibernate.reactive.rest.data.panache.deployment.entity;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;

import io.quarkus.hibernate.reactive.panache.PanacheEntityBase;

@Entity
public class Project extends PanacheEntityBase {

@Id
public String name;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.quarkus.hibernate.reactive.rest.data.panache.deployment.entity;

import io.quarkus.hibernate.reactive.rest.data.panache.PanacheEntityResource;
import io.quarkus.rest.data.panache.ResourceProperties;

/**
* Having a path param in the path reproduces the issue of having HAL enabled spites it should be disabled by default.
*/
@ResourceProperties(path = "/{group}/projects")
public interface ProjectResource extends PanacheEntityResource<Project, String> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
addContextAnnotation(methodCreator.getParameterAnnotations(1));
addConsumesAnnotation(methodCreator, APPLICATION_JSON);
addProducesJsonAnnotation(methodCreator, resourceProperties);
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
addOpenApiResponseAnnotation(methodCreator, Response.Status.CREATED, resourceMetadata.getEntityType());
addSecurityAnnotations(methodCreator, resourceProperties);
// Add parameter annotations
Expand All @@ -130,15 +130,15 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
ResultHandle entity = tryBlock.invokeVirtualMethod(
ofMethod(resourceMetadata.getResourceClass(), RESOURCE_METHOD_NAME, Object.class, Object.class),
resource, entityToSave);
tryBlock.returnValue(responseImplementor.created(tryBlock, entity));
tryBlock.returnValue(responseImplementor.created(tryBlock, entity, resourceProperties));
tryBlock.close();
} else {
ResultHandle uniEntity = methodCreator.invokeVirtualMethod(
ofMethod(resourceMetadata.getResourceClass(), RESOURCE_METHOD_NAME, Uni.class, Object.class),
resource, entityToSave);

methodCreator.returnValue(UniImplementor.map(methodCreator, uniEntity, EXCEPTION_MESSAGE,
(body, item) -> body.returnValue(responseImplementor.created(body, item))));
(body, item) -> body.returnValue(responseImplementor.created(body, item, resourceProperties))));
}

methodCreator.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
if (!isResteasyClassic()) {
// We only add the Links annotation in Resteasy Reactive because Resteasy Classic ignores the REL parameter:
// it always uses "list" for GET methods, so it interferes with the list implementation.
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
}

ResultHandle resource = methodCreator.readInstanceField(resourceField, methodCreator.getThis());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
addPathAnnotation(methodCreator, appendToPath(resourceProperties.getPath(RESOURCE_METHOD_NAME), "{id}"));
addDeleteAnnotation(methodCreator);
addPathParamAnnotation(methodCreator.getParameterAnnotations(0), "id");
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
addMethodAnnotations(methodCreator, resourceProperties.getMethodAnnotations(RESOURCE_METHOD_NAME));
addOpenApiResponseAnnotation(methodCreator, Response.Status.NO_CONTENT);
addSecurityAnnotations(methodCreator, resourceProperties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
addSecurityAnnotations(methodCreator, resourceProperties);

addPathParamAnnotation(methodCreator.getParameterAnnotations(0), "id");
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);

ResultHandle resource = methodCreator.readInstanceField(resourceField, methodCreator.getThis());
ResultHandle id = methodCreator.getMethodParam(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private void implementPaged(ClassCreator classCreator, ResourceMetadata resource
addGetAnnotation(methodCreator);
addPathAnnotation(methodCreator, resourceProperties.getPath(RESOURCE_METHOD_NAME));
addProducesJsonAnnotation(methodCreator, resourceProperties);
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
addMethodAnnotations(methodCreator, resourceProperties.getMethodAnnotations(RESOURCE_METHOD_NAME));
addOpenApiResponseAnnotation(methodCreator, Response.Status.OK, resourceMetadata.getEntityType(), true);
addSecurityAnnotations(methodCreator, resourceProperties);
Expand Down Expand Up @@ -280,7 +280,7 @@ private void implementNotPaged(ClassCreator classCreator, ResourceMetadata resou
addGetAnnotation(methodCreator);
addPathAnnotation(methodCreator, resourceProperties.getPath(RESOURCE_METHOD_NAME));
addProducesJsonAnnotation(methodCreator, resourceProperties);
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
addMethodAnnotations(methodCreator, resourceProperties.getMethodAnnotations(RESOURCE_METHOD_NAME));
addOpenApiResponseAnnotation(methodCreator, Response.Status.OK, resourceMetadata.getEntityType(), true);
addSecurityAnnotations(methodCreator, resourceProperties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,20 +100,23 @@ protected void addDeleteAnnotation(AnnotatedElement element) {
element.addAnnotation(DELETE.class);
}

protected void addLinksAnnotation(AnnotatedElement element, String entityClassName, String rel) {
if (isResteasyClassic()) {
AnnotationCreator linkResource = element.addAnnotation("org.jboss.resteasy.links.LinkResource");
linkResource.addValue("entityClassName", entityClassName);
linkResource.addValue("rel", rel);
} else {
AnnotationCreator linkResource = element.addAnnotation("io.quarkus.resteasy.reactive.links.RestLink");
Class<?> entityClass;
try {
entityClass = Thread.currentThread().getContextClassLoader().loadClass(entityClassName);
linkResource.addValue("entityType", entityClass);
protected void addLinksAnnotation(AnnotatedElement element, ResourceProperties resourceProperties, String entityClassName,
String rel) {
if (resourceProperties.isHal()) {
if (isResteasyClassic()) {
AnnotationCreator linkResource = element.addAnnotation("org.jboss.resteasy.links.LinkResource");
linkResource.addValue("entityClassName", entityClassName);
linkResource.addValue("rel", rel);
} catch (ClassNotFoundException e) {
LOGGER.error("Unable to create links for entity: '" + entityClassName + "'", e);
} else {
AnnotationCreator linkResource = element.addAnnotation("io.quarkus.resteasy.reactive.links.RestLink");
Class<?> entityClass;
try {
entityClass = Thread.currentThread().getContextClassLoader().loadClass(entityClassName);
linkResource.addValue("entityType", entityClass);
linkResource.addValue("rel", rel);
} catch (ClassNotFoundException e) {
LOGGER.error("Unable to create links for entity: '" + entityClassName + "'", e);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
addContextAnnotation(methodCreator.getParameterAnnotations(2));
addConsumesAnnotation(methodCreator, APPLICATION_JSON);
addProducesJsonAnnotation(methodCreator, resourceProperties);
addLinksAnnotation(methodCreator, resourceMetadata.getEntityType(), REL);
addLinksAnnotation(methodCreator, resourceProperties, resourceMetadata.getEntityType(), REL);
addMethodAnnotations(methodCreator, resourceProperties.getMethodAnnotations(RESOURCE_UPDATE_METHOD_NAME));
addOpenApiResponseAnnotation(methodCreator, Response.Status.CREATED, resourceMetadata.getEntityType());
addSecurityAnnotations(methodCreator, resourceProperties);
Expand All @@ -162,9 +162,9 @@ protected void implementInternal(ClassCreator classCreator, ResourceMetadata res
ResultHandle entityToSave = methodCreator.getMethodParam(1);

if (isNotReactivePanache()) {
implementClassicVersion(methodCreator, resourceMetadata, resource, id, entityToSave);
implementClassicVersion(methodCreator, resourceMetadata, resourceProperties, resource, id, entityToSave);
} else {
implementReactiveVersion(methodCreator, resourceMetadata, resource, id, entityToSave);
implementReactiveVersion(methodCreator, resourceMetadata, resourceProperties, resource, id, entityToSave);
}

methodCreator.close();
Expand All @@ -175,8 +175,8 @@ protected String getResourceMethodName() {
return RESOURCE_UPDATE_METHOD_NAME;
}

private void implementReactiveVersion(MethodCreator methodCreator, ResourceMetadata resourceMetadata, ResultHandle resource,
ResultHandle id, ResultHandle entityToSave) {
private void implementReactiveVersion(MethodCreator methodCreator, ResourceMetadata resourceMetadata,
ResourceProperties resourceProperties, ResultHandle resource, ResultHandle id, ResultHandle entityToSave) {
ResultHandle uniResponse = methodCreator.invokeVirtualMethod(
ofMethod(resourceMetadata.getResourceClass(), RESOURCE_GET_METHOD_NAME, Uni.class, Object.class),
resource, id);
Expand All @@ -193,15 +193,16 @@ private void implementReactiveVersion(MethodCreator methodCreator, ResourceMetad
(updateBody, itemUpdated) -> {
BranchResult ifEntityIsNew = updateBody.ifNull(itemWasFound);
ifEntityIsNew.trueBranch()
.returnValue(responseImplementor.created(ifEntityIsNew.trueBranch(), itemUpdated));
.returnValue(responseImplementor.created(ifEntityIsNew.trueBranch(), itemUpdated,
resourceProperties));
ifEntityIsNew.falseBranch()
.returnValue(responseImplementor.noContent(ifEntityIsNew.falseBranch()));
}));
}));
}

private void implementClassicVersion(MethodCreator methodCreator, ResourceMetadata resourceMetadata, ResultHandle resource,
ResultHandle id, ResultHandle entityToSave) {
private void implementClassicVersion(MethodCreator methodCreator, ResourceMetadata resourceMetadata,
ResourceProperties resourceProperties, ResultHandle resource, ResultHandle id, ResultHandle entityToSave) {
// Invoke resource methods inside a supplier function which will be given to an update executor.
// For ORM, this update executor will have the @Transactional annotation to make
// sure that all database operations are executed in a single transaction.
Expand All @@ -214,7 +215,8 @@ private void implementClassicVersion(MethodCreator methodCreator, ResourceMetada
updateExecutor, updateFunction);

BranchResult createdNewEntity = tryBlock.ifNotNull(newEntity);
createdNewEntity.trueBranch().returnValue(responseImplementor.created(createdNewEntity.trueBranch(), newEntity));
createdNewEntity.trueBranch()
.returnValue(responseImplementor.created(createdNewEntity.trueBranch(), newEntity, resourceProperties));
createdNewEntity.falseBranch().returnValue(responseImplementor.noContent(createdNewEntity.falseBranch()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.gizmo.ResultHandle;
import io.quarkus.hal.HalService;
import io.quarkus.rest.data.panache.deployment.properties.ResourceProperties;
import io.quarkus.resteasy.links.runtime.hal.ResteasyHalService;
import io.quarkus.resteasy.reactive.links.runtime.hal.ResteasyReactiveHalService;

Expand All @@ -44,17 +45,12 @@ public ResultHandle ok(BytecodeCreator creator, ResultHandle entity, ResultHandl
return creator.invokeVirtualMethod(ofMethod(ResponseBuilder.class, "build", Response.class), builder);
}

public ResultHandle created(BytecodeCreator creator, ResultHandle entity) {
return created(creator, entity, getEntityUrl(creator, entity));
}
public ResultHandle created(BytecodeCreator creator, ResultHandle entity, ResourceProperties resourceProperties) {
if (resourceProperties.isHal()) {
return doCreated(creator, entity, getEntityUrl(creator, entity));
}

public ResultHandle created(BytecodeCreator creator, ResultHandle entity, ResultHandle location) {
ResultHandle builder = getResponseBuilder(creator, Response.Status.CREATED.getStatusCode());
creator.invokeVirtualMethod(
ofMethod(ResponseBuilder.class, "entity", ResponseBuilder.class, Object.class), builder, entity);
creator.invokeVirtualMethod(
ofMethod(ResponseBuilder.class, "location", ResponseBuilder.class, URI.class), builder, location);
return creator.invokeVirtualMethod(ofMethod(ResponseBuilder.class, "build", Response.class), builder);
return doCreated(creator, entity, null);
}

public ResultHandle getEntityUrl(BytecodeCreator creator, ResultHandle entity) {
Expand Down Expand Up @@ -89,6 +85,18 @@ public ResultHandle notFoundException(BytecodeCreator creator) {
creator.load(Response.Status.NOT_FOUND.getStatusCode()));
}

private ResultHandle doCreated(BytecodeCreator creator, ResultHandle entity, ResultHandle location) {
ResultHandle builder = getResponseBuilder(creator, Response.Status.CREATED.getStatusCode());
creator.invokeVirtualMethod(
ofMethod(ResponseBuilder.class, "entity", ResponseBuilder.class, Object.class), builder, entity);
if (location != null) {
creator.invokeVirtualMethod(
ofMethod(ResponseBuilder.class, "location", ResponseBuilder.class, URI.class), builder, location);
}

return creator.invokeVirtualMethod(ofMethod(ResponseBuilder.class, "build", Response.class), builder);
}

private ResultHandle status(BytecodeCreator creator, int status) {
ResultHandle builder = getResponseBuilder(creator, status);
return creator.invokeVirtualMethod(ofMethod(ResponseBuilder.class, "build", Response.class), builder);
Expand Down

0 comments on commit 57c036a

Please sign in to comment.