search

jooby-project / jooby

The modular web framework for Java and Kotlin
https://jooby.io
Apache License 2.0
1.7k stars 198 forks source link

Add MvcMethod object for `setMvcMethod` and `getMvcMethod`. #3532

Open agentgt opened 2 days ago

agentgt commented 2 days ago

Currently setMvcMethod and getMvcMethod are a java.lang.reflect.Method.

The original impetus for set/getMvcMethod was for metrics and the actual java.lang.reflect.Method is not even needed but rather method name and signature.

I propose based on this comment: https://github.com/jooby-project/jooby/issues/3529#issuecomment-2353089543

That we replace get/setMvcMethod(io.jooby.MvcMethod method).

MvcMethod type would have enough metadata so that one could get the java.lang.reflect.Method dynamically if desired.

This will avoid reflective lookup during registration regardless of whether nor setMvcMethod is enabled or not.

I can do a PR for this for 3.4.0 depending on timeline of 3.4.0

agentgt commented 2 days ago

The other advantage to this is if one actually did want to call the method they can use the more modern and in theory faster MethodLookup API.

jknack commented 2 days ago

We can probably add it, but either this way or just enabling the option in jooby-apt will require you to update all your projects, no?

agentgt commented 2 days ago

Correct.

The code change is minor as that is one place but the compiler flag is not because of Maven limitations.

I’m proposing it because it is the right thing to do. We should not kick off reflection on initialization.

jknack commented 2 days ago

Ok, send a PR so we can release 3.4.0 with the change

agentgt commented 1 day ago

@jknack This probably a silly question but I assume we are still targeting jDK 17 for 3.4.0 correct? I'm fairly sure but just want to check.

jknack commented 1 day ago

absolutely. jooby 4.0 will be 21