Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes incorrect rel=self web link #38265

Merged
merged 4 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/src/main/asciidoc/resteasy-reactive.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,8 @@ Assuming a `Record` looks like:
----
public class Record {

// the class must contain/inherit either and `id` field or an `@Id` annotated field
// The class must contain/inherit either and `id` field, an `@Id` or `@RestLinkId` annotated field.
// When resolving the id the order of preference is: `@RestLinkId` > `@Id` > `id` field.
private int id;

public Record() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.resteasy.reactive.common.deployment.JaxRsResourceIndexBuildItem;
import io.quarkus.resteasy.reactive.links.RestLinkId;
import io.quarkus.resteasy.reactive.links.runtime.GetterAccessorsContainer;
import io.quarkus.resteasy.reactive.links.runtime.GetterAccessorsContainerRecorder;
import io.quarkus.resteasy.reactive.links.runtime.LinkInfo;
Expand Down Expand Up @@ -150,35 +151,7 @@ private RuntimeValue<GetterAccessorsContainer> implementPathParameterValueGetter
validateClassHasFieldId(index, entityType);

for (String parameterName : linkInfo.getPathParameters()) {
FieldInfoSupplier byParamName = new FieldInfoSupplier(c -> c.field(parameterName), className, index);

// We implement a getter inside a class that has the required field.
// We later map that getter's accessor with an entity type.
// If a field is inside a parent class, the getter accessor will be mapped to each subclass which
// has REST links that need access to that field.
FieldInfo fieldInfo = byParamName.get();
if ((fieldInfo == null) && parameterName.equals("id")) {
// this is a special case where we want to go through the fields of the class
// and see if any is annotated with any sort of @Id annotation
// N.B. as this module does not depend on any other module that could supply this @Id annotation
// (like Panache), we need this general lookup
FieldInfoSupplier byIdAnnotation = new FieldInfoSupplier(
c -> {
for (FieldInfo field : c.fields()) {
List<AnnotationInstance> annotationInstances = field.annotations();
for (AnnotationInstance annotationInstance : annotationInstances) {
if (annotationInstance.name().toString().endsWith("persistence.Id")) {
return field;
}
}
}
return null;
},
className,
index);
fieldInfo = byIdAnnotation.get();
}

FieldInfo fieldInfo = resolveField(index, parameterName, className);
if (fieldInfo != null) {
GetterMetadata getterMetadata = new GetterMetadata(fieldInfo);
if (!implementedGetters.contains(getterMetadata)) {
Expand All @@ -187,7 +160,8 @@ private RuntimeValue<GetterAccessorsContainer> implementPathParameterValueGetter
}

getterAccessorsContainerRecorder.addAccessor(getterAccessorsContainer,
entityType, getterMetadata.getFieldName(), getterMetadata.getGetterAccessorName());
entityType, parameterName,
getterMetadata.getGetterAccessorName());
}
}
}
Expand All @@ -196,6 +170,52 @@ private RuntimeValue<GetterAccessorsContainer> implementPathParameterValueGetter
return getterAccessorsContainer;
}

private FieldInfo resolveField(IndexView index, String parameterName, DotName className) {
FieldInfoSupplier byParamName = new FieldInfoSupplier(c -> c.field(parameterName), className, index);

// check if we have field matching the name
FieldInfo fieldInfo = byParamName.get();
if (parameterName.equals("id")) {
// this is a special case where we want to go through the fields of the class
// and see if any is annotated with any sort of @persistence.Id/@RestLinkId annotation
// N.B. as this module does not depend on any other module that could supply this @Id annotation
// (like Panache), we need this general lookup
// the order of preference for the annotations is @RestLinkId > @persistence.Id > id
FieldInfoSupplier byAnnotation = new FieldInfoSupplier(
c -> {
FieldInfo persistenceId = null;
for (FieldInfo field : c.fields()) {
FroMage marked this conversation as resolved.
Show resolved Hide resolved
// prefer RestLinId over Id
if (field.hasAnnotation(RestLinkId.class)) {
return field;
}
// keep the first found @persistence.Id annotation in case not @RestLinkId is found
if (fieldAnnotatedWith(field, "persistence.Id") && persistenceId == null) {
persistenceId = field;
}
}
return persistenceId;
},
className,
index);
FieldInfo annotatedField = byAnnotation.get();
if (annotatedField != null) {
fieldInfo = annotatedField;
}
}
return fieldInfo;
}

private boolean fieldAnnotatedWith(FieldInfo field, String annotation) {
List<AnnotationInstance> annotationInstances = field.annotations();
for (AnnotationInstance annotationInstance : annotationInstances) {
if (annotationInstance.name().toString().endsWith(annotation)) {
return true;
}
}
return false;
}

/**
* Validates if the given classname contains a field `id` or annotated with `@Id`
*
Expand Down Expand Up @@ -227,15 +247,26 @@ private void validateRec(IndexView index, String entityType, ClassInfo classInfo
.filter(a -> a.name().toString().endsWith("persistence.Id"))
.toList();

List<AnnotationInstance> fieldsAnnotatedWithRestLinkId = classInfo.fields().stream()
.flatMap(f -> f.annotations(RestLinkId.class).stream())
.toList();

// @RestLinkId annotation count > 1 is not allowed
if (fieldsAnnotatedWithRestLinkId.size() > 1) {
throw new IllegalStateException("Cannot generate web links for the class " + entityType +
" because it has multiple fields annotated with `@RestLinkId`, where a maximum of one is allowed");
}

// Id field found, break the loop
if (!fieldsNamedId.isEmpty() || !fieldsAnnotatedWithId.isEmpty())
if (!fieldsNamedId.isEmpty() || !fieldsAnnotatedWithId.isEmpty() || !fieldsAnnotatedWithRestLinkId.isEmpty()) {
return;
}

// Id field not found and hope is gone
DotName superClassName = classInfo.superName();
if (superClassName == null) {
throw new IllegalStateException("Cannot generate web links for the class " + entityType +
" because is either missing an `id` field or a field with an `@Id` annotation");
" because it is either missing an `id` field, a field with an `@Id` annotation or a field with a `@RestLinkId annotation");
}

// Id field not found but there's still hope
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,66 @@ void shouldGetHalLinksForInstance() {

assertThat(response.body().jsonPath().getString("_links.list.href")).endsWith("/records");
}

@Test
void shouldGetHalLinksForIdAndPersistenceIdAndRestLinkId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-id-and-persistence-id-and-rest-link-id/100")
.thenReturn();

assertThat(response.body()
.jsonPath()
.getString("_links.self.href")).endsWith("/records/with-id-and-persistence-id-and-rest-link-id/100");
}

@Test
void shouldGetHalLinksForIdAndPersistenceId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-id-and-persistence-id/10")
.thenReturn();

assertThat(response.body()
.jsonPath()
.getString("_links.self.href")).endsWith("/records/with-id-and-persistence-id/10");
}

@Test
void shouldGetHalLinksForIdAndRestLinkId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-id-and-rest-link-id/100")
.thenReturn();

assertThat(response.body()
.jsonPath()
.getString("_links.self.href")).endsWith("/records/with-id-and-rest-link-id/100");
}

@Test
void shouldGetHalLinksForPersistenceIdAndRestLinkId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-persistence-id-and-rest-link-id/100")
.thenReturn();

assertThat(response.body()
.jsonPath()
.getString("_links.self.href")).endsWith("/records/with-persistence-id-and-rest-link-id/100");
}

@Test
void shouldGetHalLinksForPersistenceId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-persistence-id/10")
.thenReturn();

assertThat(response.body().jsonPath().getString("_links.self.href")).endsWith("/records/with-persistence-id/10");
}

@Test
void shouldGetHalLinksForRestLinkId() {
Response response = given().accept(RestMediaType.APPLICATION_HAL_JSON)
.get("/records/with-rest-link-id/100")
.thenReturn();

assertThat(response.body().jsonPath().getString("_links.self.href")).endsWith("/records/with-rest-link-id/100");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ public class HalLinksWithJacksonTest extends AbstractHalLinksTest {
@RegisterExtension
static final QuarkusProdModeTest TEST = new QuarkusProdModeTest()
.withApplicationRoot((jar) -> jar
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class))
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class,
TestRecordWithIdAndPersistenceIdAndRestLinkId.class, TestRecordWithIdAndRestLinkId.class,
TestRecordWithIdAndPersistenceId.class, TestRecordWithPersistenceId.class,
TestRecordWithRestLinkId.class, TestRecordWithPersistenceIdAndRestLinkId.class))
.setForcedDependencies(List.of(
Dependency.of("io.quarkus", "quarkus-resteasy-reactive-jackson", Version.getVersion()),
Dependency.of("io.quarkus", "quarkus-hal", Version.getVersion())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ public class HalLinksWithJsonbTest extends AbstractHalLinksTest {
@RegisterExtension
static final QuarkusProdModeTest TEST = new QuarkusProdModeTest()
.withApplicationRoot((jar) -> jar
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class))
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class,
TestRecordWithIdAndPersistenceIdAndRestLinkId.class, TestRecordWithIdAndRestLinkId.class,
TestRecordWithIdAndPersistenceId.class, TestRecordWithPersistenceId.class,
TestRecordWithRestLinkId.class, TestRecordWithPersistenceIdAndRestLinkId.class))
.setForcedDependencies(List.of(
Dependency.of("io.quarkus", "quarkus-resteasy-reactive-jsonb", Version.getVersion()),
Dependency.of("io.quarkus", "quarkus-hal", Version.getVersion())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ public class RestLinksInjectionTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class));
.addClasses(AbstractId.class, AbstractEntity.class, TestRecord.class, TestResource.class,
TestRecordWithIdAndPersistenceIdAndRestLinkId.class, TestRecordWithIdAndRestLinkId.class,
TestRecordWithIdAndPersistenceId.class, TestRecordWithPersistenceId.class,
TestRecordWithRestLinkId.class, TestRecordWithPersistenceIdAndRestLinkId.class));

@TestHTTPResource("records")
String recordsUrl;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.quarkus.resteasy.reactive.links.deployment;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

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

import io.quarkus.runtime.util.ExceptionUtil;
import io.quarkus.test.QuarkusUnitTest;

public class RestLinksWithFailureInjectionMultipleRestLinkIdTest {

@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.withApplicationRoot(
jar -> jar.addClasses(TestRecordMultipleRestLinkIds.class, TestResourceMultipleRestLinkIds.class))
.assertException(t -> {
Throwable rootCause = ExceptionUtil.getRootCause(t);
assertThat(rootCause).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Cannot generate web links for the class " +
"io.quarkus.resteasy.reactive.links.deployment.TestRecordMultipleRestLinkIds" +
" because it has multiple fields annotated with `@RestLinkId`, where a maximum of one is allowed");
});

@Test
void validationFailed() {
// Should not be reached: verify
fail();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.quarkus.resteasy.reactive.links.deployment;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -17,13 +17,13 @@ public class RestLinksWithFailureInjectionTest {
Throwable rootCause = ExceptionUtil.getRootCause(t);
assertThat(rootCause).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Cannot generate web links for the class " +
"io.quarkus.resteasy.reactive.links.deployment.TestRecordNoId because is either " +
"missing an `id` field or a field with an `@Id` annotation");
"io.quarkus.resteasy.reactive.links.deployment.TestRecordNoId because it is " +
"either missing an `id` field, a field with an `@Id` annotation or a field with a `@RestLinkId annotation");
});

@Test
void validationFailed() {
// Should not be reached: verify
assertTrue(false);
fail();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package io.quarkus.resteasy.reactive.links.deployment;

import io.quarkus.resteasy.reactive.links.RestLinkId;

public class TestRecordMultipleRestLinkIds {

@RestLinkId
private long idOne;
@RestLinkId
private long idTwo;

private String name;

public TestRecordMultipleRestLinkIds() {
}

public TestRecordMultipleRestLinkIds(long idOne, long idTwo, String name) {
this.idOne = idOne;
this.idTwo = idTwo;
this.name = name;
}

public long getIdOne() {
return idOne;
}

public void setIdOne(long idOne) {
this.idOne = idOne;
}

public long getIdTwo() {
return idTwo;
}

public void setIdTwo(long idTwo) {
this.idTwo = idTwo;
}

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package io.quarkus.resteasy.reactive.links.deployment;

import io.quarkus.resteasy.reactive.links.deployment.persistence.Id;
FroMage marked this conversation as resolved.
Show resolved Hide resolved

public class TestRecordWithIdAndPersistenceId {

@Id
private int persistenceId;
private int id;
private String name;

public TestRecordWithIdAndPersistenceId() {
}

public TestRecordWithIdAndPersistenceId(int persistenceId, int id, String value) {
this.persistenceId = persistenceId;
this.id = id;
this.name = value;
}

public int getPersistenceId() {
return persistenceId;
}

public void setPersistenceId(int persistenceId) {
this.persistenceId = persistenceId;
}

public int getId() {
return id;
}

public void setId(int id) {
this.id = id;
}

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}
}
Loading
Loading