Skip to content

Commit

Permalink
REST Data with Panache should not produce links when hal is disabled
Browse files Browse the repository at this point in the history
This is related to quarkusio#35167.
Even though HAL is disabled by default, it's still used to generate unexpected links and to set the location.

(cherry picked from commit 362f03a)
  • Loading branch information
Sgitario authored and gsmet committed Aug 9, 2023
1 parent 933c063 commit d2f3837
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 d2f3837

Please sign in to comment.