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

4.x - ApplicationPath is not a bean defining annotation #8502

Closed
romain-grecourt opened this issue Mar 20, 2024 · 4 comments
Closed

4.x - ApplicationPath is not a bean defining annotation #8502

romain-grecourt opened this issue Mar 20, 2024 · 4 comments
Assignees
Labels
4.x Version 4.x jax-rs JAX-RS and Jersey related issues MP

Comments

@romain-grecourt
Copy link
Contributor

Environment Details

  • Helidon Version: 4.0.6
  • Helidon MP

Problem Description

@Path is a bean defining annotation but @ApplicationPath is not. When creating a JaxRs application annotated only with @ApplicationPath but not with @ApplicationScope it is discarded.

This can lead to incorrect behavior:

  • if the path specified is not /
  • if there are multiple applications each with a different path and different resource classes

Both @Path and @Provider are declared as bean defining annotations, most likely for convenience.

Is it an oversight that @ApplicationPath is not included ?

Steps to reproduce

public class Main {
    public static void main(String[] args) {
        io.helidon.Main.main(args);
    }

    @ApplicationPath("/")
    public static class App1 extends Application {

        @Override
        public Set<Class<?>> getClasses() {
            return Set.of(Resource.class);
        }

        @Path("/app1")
        public static class Resource {

            @GET
            public String get() {
                return "OK";
            }
        }
    }

    @ApplicationPath("/foo")
    public static class App2 extends Application {

        @Override
        public Set<Class<?>> getClasses() {
            return Set.of(Resource.class);
        }

        @Path("/app2")
        public static class Resource {

            @GET
            public String get() {
                return "OK";
            }
        }
    }
}
curl  http://localhost:8080/app2
OK
curl  http://localhost:8080/foo/app2
Not Found
@tomas-langer
Copy link
Member

ApplicationPath cannot be a bean defining annotation, as it is not required on an application. If we made it BDA, some applications would be picked up, some would be ignored, and that is quite a bad behavior.
In general application class without a BDA should be ignored (and it should be @ApplicationScoped, as nothing else makes sense)

@barchetta barchetta removed this from the 4.0.7 milestone Mar 28, 2024
@jefrajames
Copy link

There is an ambiguity with Helidon 4.0.7 when defining a RestApplication class annotated with ApplicationPath.
Not adding ApplicationScoped explicitely leads to an inconstency:

. the ApplicationPath value is taken into account by OpenApi when exposing the API documentation
. but it is not taken into account in real endpoints exposure.

So endpoints exposed by OpenApi (and ui) are not correct.

@romain-grecourt
Copy link
Contributor Author

We have agreed on making @ApplicationPath a BDA, and warning about applications not annotated with @ApplicationScoped.

We should also investigate using the Jandex index to detect application classes with no annotations.

@tjquinno
Copy link
Member

tjquinno commented Jun 3, 2024

Some background that might be useful...

The Helidon OpenAPI implementation layers on the SmallRye OpenAPI library.

Helidon's MP OpenAPI code finds out what applications Helidon has discovered (by invoking Helidon's JaxRsCdiExtension#applicationsToRun).

Helidon then asks the SmallRye library to create a separate OpenAPI model for each separate application. Helidon then merges the separate per-application OpenAPI models into a single server-wide model (using a SmallRye utility operation) which gives rise to the single OpenAPI document output at the /openapi endpoint which describes the entirety of the API presented to the outside world by the server and all of its constituent applications.

@m0mus m0mus added this to Backlog Aug 12, 2024
@m0mus m0mus moved this to Normal priority in Backlog Aug 12, 2024
@spericas spericas self-assigned this Dec 10, 2024
@github-project-automation github-project-automation bot moved this from Normal priority to Closed in Backlog Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x jax-rs JAX-RS and Jersey related issues MP
Projects
Archived in project
Development

No branches or pull requests

7 participants