-
Notifications
You must be signed in to change notification settings - Fork 43
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
Allow explicit class references when building routes #63
Allow explicit class references when building routes #63
Conversation
…when generating routes to allow external code to generate routes with reflection
People can use Requests, Responses and Parameters with specialized generic classes like |
Ah right, I forgot about that. I've only been using explicit response classes. I will update this PR later today to fix that plus add some more unit tests that will cover such cases. |
Some parts of this library still provide and use KClass instances, they should be (and eventually will be) changed to use KType as well. Don't hesitate to convert them if convenient. |
Alright, already got a working version with a mix of the two. I will replace it entirely then. |
…otherwise be erased without reified, change access of several routing methods with potential of incorrect use to internal
I've replaced the KClass usages with KType. I've also restricted access to several non-DSL methods to internal as they can now be used with non-matching generic types and KType which would result in errors (preHandle/handle). This should not affect the API at all. I've also removed the inlining of the route building logic, this is only executed once at application startup which means the performance impact of inlining these methods in almost non-existent however it leads to massive increase in generated classes which can actually make a negative performance and memory impact (I've experienced that myself with a application with around ~120 API endpoints). The tests are passing and I've added a few additional ones, but I'm not convinced that the tests are sufficient to ensure proper functionality. |
Alright, it looks good to me on a first glance, it didn't change any behaviour fundamentally as far as i can see. |
I tried to preserve the existing DSL as much as possible. |
You can annotate the types like this Be careful with package discovery through reflections, it tends to be really slow. I used to work with spring and it suffered greatly under that, with up to 15 minutes load times on larger-ish projects. Even the package scan i use for the 7-odd classes takes 200ms to load... I would get rid of it if it wasn't necessary to provide default modules automatically and conveniently. |
Ah I forgot about that one, I was thinking about annotations for method parameters. Perhaps it is worth supporting it as it would allow classes outside of the project to be used. I wouldn't use reflection for it, I would use class path scanning: https://github.com/classgraph/classgraph |
Alright, currently I use reflections, maybe classgraph is faster. i'll give it a shot. But that is what caused spring to take so long to initialise. |
I found it to be a very good alternative. Let me know if this PR needs any more changes. |
When building routes with reflection instead of manually registering them it is not possible to use reified DSL because it is not available at runtime using reflection. This can be fixed by only using reified for DSL methods and passing the class instance as parameters. This would allow external code to build routes from for example something like this:
There is no impact to the current DSL and there should be no performance impact, since this only affects the route building.