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

ReflectiveEndpointLoader doesn't detect methods declared in the superclass #47

Open
Memexurer opened this issue Jul 22, 2024 · 3 comments
Labels
question Further information is requested

Comments

@Memexurer
Copy link

Actual behavior (the bug)
Look at the example code - the only available endpoint is /ab/b; /ab/a doesn't work

Expected behavior
Endpoints should be loaded from the superclasses too

To Reproduce

public class TestEndpoints {
    public static class Endpoint1 {
        @Get("/a")
        public void a(Context ctx) {
            ctx.result("a");
        }
    }

    @Endpoints("/ab")
    public static class Endpoint2 extends Endpoint1 {
        @Get("/b")
        public void b(Context ctx) {
            ctx.result("b");
        }
    }
}

Additional context
This happens because getDeclaredMethods doesn't return methods declared in the superclasses. This could be fixed by changing getDeclaredMethods invocations inside ReflectiveEndpointLoader class with getMethods (but I think that would break compatibility with earlier versions of javalin-routing-extensions)

@dzikoysk dzikoysk added the question Further information is requested label Jul 22, 2024
Memexurer added a commit to Memexurer/javalin-routing-extensions that referenced this issue Jul 22, 2024
@dzikoysk
Copy link
Member

Hey, it seems to be a bit specific way to use the annotated api, so we just simply never tried to support it. Is there any particular reason why you're trying to introduce such abstraction at the controller level?

@Memexurer
Copy link
Author

Basically I have two classes with very similar endpoints, but only one endpoint is different in those two classes. So I just extracted these similar endpoints to another class which these two classes now extend.
But the problem now is, that this project doesn't support such class layout.

@dzikoysk
Copy link
Member

dzikoysk commented Jul 22, 2024

Maybe instead of going for straightforwrd inheritance, we could try to support it via the more explicit API. It' should be a bit more user-friendly - especially considering some bigger apps with a lot of endpoints. E.g.:

// as repatable annotation:
@Endpoints("/a")
@Endpoints("/b")
// or via new parameter:
@Endpoints(roots = {"/a", "/b"})
public class SharedEndpoints {
    @Get("/shared")
    public void a(Context ctx) {
        ctx.result("a or b");
    }
}

@Endpoints("/b")
public class BSpecificEndpoints {
    @Get("/extra")
    public void b(Context ctx) {
        ctx.result("b");
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants