Fixes quarkusio#38247 incorrect rel=self web link

bpasson · Jan 18, 2024 · 15b6a5e · 15b6a5e
1 parent 9750746
commit 15b6a5e
Show file tree

Hide file tree

Showing 20 changed files with 641 additions and 46 deletions.
diff --git a/docs/src/main/asciidoc/qute-reference.adoc b/docs/src/main/asciidoc/qute-reference.adoc
@@ -223,7 +223,7 @@ Likewise, a line that contains an _expression_ or a _non-whitespace character_ i
                           <3>
      {/for}               <4>
      </ul>
-   <body>
+   </body>
 </html>
 ----
 <1> This is a standalone line and will be removed.
@@ -240,7 +240,7 @@ Likewise, a line that contains an _expression_ or a _non-whitespace character_ i
        <li>Foo 100</li>
 
      </ul>
-   <body>
+   </body>
 </html>
 ----
 
@@ -258,7 +258,7 @@ In this case, all whitespace characters from a standalone line will be printed t
 
 
      </ul>
-   <body>
+   </body>
 </html>
 ----
 
@@ -273,7 +273,7 @@ In the `object.property` (dot notation) syntax, the `property` must be a <<ident
 In the `object[property_name]` (bracket notation) syntax, the `property_name` has to be a non-null <<literals,literal>> value.
 
 An expression can start with an optional namespace followed by a colon (`:`).
-A valid namespace consist of alphanumeric characters and underscores.
+A valid namespace consists of alphanumeric characters and underscores.
 Namespace expressions are resolved differently - see also <<expression_resolution>>.
 
 .Property Accessor Examples
@@ -336,7 +336,7 @@ You can learn more about virtual methods in the <<virtual_methods,following sect
 ==== Resolution
 
 The first part of the expression is always resolved against the <<current_context_object,current context object>>.
-If no result is found for the first part it's resolved against the parent context object (if available).
+If no result is found for the first part, it's resolved against the parent context object (if available).
 For an expression that starts with a namespace the current context object is found using all the available ``NamespaceResolver``s.
 For an expression that does not start with a namespace the current context object is *derived from the position* of the tag.
 All other parts of an expression are resolved using all ``ValueResolver``s against the result of the previous resolution.
@@ -1426,7 +1426,7 @@ template.data(foo).createUni().subscribe().with(System.out::println);
 `TemplateInstance.createMulti()` returns a new `Multi<String>` object.
 Each item represents a part/chunk of the rendered template.
 Again, `createMulti()` does not trigger rendering.
-Instead, every time a computation is triggered by a subscriber the template is rendered again.
+Instead, every time a computation is triggered by a subscriber, the template is rendered again.
 
 .`TemplateInstance.createMulti()` Example
 [source,java]

diff --git a/docs/src/main/asciidoc/resteasy-reactive.adoc b/docs/src/main/asciidoc/resteasy-reactive.adoc
@@ -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() {

diff --git a/...eployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java b/...eployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java
@@ -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;
@@ -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)) {
@@ -187,7 +160,8 @@ private RuntimeValue<GetterAccessorsContainer> implementPathParameterValueGetter
                         }
 
                         getterAccessorsContainerRecorder.addAccessor(getterAccessorsContainer,
-                                entityType, getterMetadata.getFieldName(), getterMetadata.getGetterAccessorName());
+                                entityType, parameterName,
+                                getterMetadata.getGetterAccessorName());
                     }
                 }
             }
@@ -196,6 +170,51 @@ 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 byIdAnnotation = new FieldInfoSupplier(
+                    c -> {
+                        FieldInfo persistenceId = null;
+                        for (FieldInfo field : c.fields()) {
+                            // prefer RestLinId over Id
+                            if (fieldAnnotatedWith(field, RestLinkId.class.getName())) {
+                                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);
+            if (byIdAnnotation.get() != null) {
+                fieldInfo = byIdAnnotation.get();
+            }
+        }
+        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`
      *
@@ -227,15 +246,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 an `@RestLinkId annotation");
         }
 
         // Id field not found but there's still hope

diff --git a/...ent/src/test/java/io/quarkus/resteasy/reactive/links/deployment/AbstractHalLinksTest.java b/...ent/src/test/java/io/quarkus/resteasy/reactive/links/deployment/AbstractHalLinksTest.java
@@ -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");
+    }
 }
diff --git a/.../src/test/java/io/quarkus/resteasy/reactive/links/deployment/HalLinksWithJacksonTest.java b/.../src/test/java/io/quarkus/resteasy/reactive/links/deployment/HalLinksWithJacksonTest.java
@@ -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())))

diff --git a/...nt/src/test/java/io/quarkus/resteasy/reactive/links/deployment/HalLinksWithJsonbTest.java b/...nt/src/test/java/io/quarkus/resteasy/reactive/links/deployment/HalLinksWithJsonbTest.java
@@ -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())))

diff --git a/...t/src/test/java/io/quarkus/resteasy/reactive/links/deployment/RestLinksInjectionTest.java b/...t/src/test/java/io/quarkus/resteasy/reactive/links/deployment/RestLinksInjectionTest.java
@@ -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;

diff --git a/...steasy/reactive/links/deployment/RestLinksWithFailureInjectionMultipleRestLinkIdTest.java b/...steasy/reactive/links/deployment/RestLinksWithFailureInjectionMultipleRestLinkIdTest.java
@@ -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();
+    }
+}
diff --git a/...java/io/quarkus/resteasy/reactive/links/deployment/RestLinksWithFailureInjectionTest.java b/...java/io/quarkus/resteasy/reactive/links/deployment/RestLinksWithFailureInjectionTest.java
@@ -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;
@@ -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 an `@RestLinkId annotation");
             });
 
     @Test
     void validationFailed() {
         // Should not be reached: verify
-        assertTrue(false);
+        fail();
     }
 }