Remove need to instantiate Application classes twice for OpenAPI support #2829
Conversation
Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
tjquinno
changed the title
| */ | ||
| public class OpenAPISupport implements Service { | ||
| public abstract class OpenAPISupport<T extends OpenAPISupport<T, B>, B extends OpenAPISupport.Builder<T, B>> implements Service { |
There was a problem hiding this comment.
I do not see the type B used anywhere. (it also should not be).
There was a problem hiding this comment.
The type B is used as the return type of Builder#me() and all the public builder methods defined on the abstract class. Otherwise there is no way for the builder methods that are implemented on the abstract class to return this correctly typed to the builder subclass without using unchecked casts. Or at least I have not seen one. If there is I would love to do away with me().
Returning the abstract Builder type instead of the specific subclass type B would lead to problems chaining calls of methods that are defined only on the subclass.
| private final OpenApiStaticFile openApiStaticFile; | ||
| private final Supplier<List<? extends IndexView>> indexViewsSupplier; | ||
|
|
||
| protected OpenAPISupport(Builder builder) { |
There was a problem hiding this comment.
Should be Builder<?, ?> so we do not have a Raw use of class with type parameters
There was a problem hiding this comment.
Yes, this should not use the raw type. For the moment I've changed it to Builder<T, B> to strongly type the parameter.
…ort (#2829) * Remove need to instantiate Application classes twice Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
…ort (helidon-io#2829) * Remove need to instantiate Application classes twice Signed-off-by: tim.quinn@oracle.com <tim.quinn@oracle.com>
Resolves #2799
If multiple
Applicationclasses exist in the user's app, we build separate OpenAPI models for each one. As part of that, we need to invoke eachApplication'sgetClassesandgetSingletonsmethods. The OpenAPI CDI extension needs to act before the server's CDI extension actually creates theApplicationinstances because the OpenAPI extension needs to adjust the routing to include the OpenAPI endpoint. But at that time, the server had not created theApplicationinstances. So to invokegetClassesandgetSingletonsthe OpenAPI CDI extension would use CDI to create unmanaged instances of eachApplication, invoke those methods, and then dispose of the unmanaged instances.This caused
@PostConstructmethods to be invoked twice, once for the unmanaged instances and again when the server created the "live" instances. Developers found this disconcerting.Avoiding the double-instantiation involves splitting the OpenAPI CDI extension's work into two steps: first, creating the
OpenAPISupportobject so its routing can be plugged in correctly, and second later on when theApplicationinstances are available for use in building the in-memory OpenAPI model.To do that involved refactoring the model-building logic from the
Builderclass to theOpenAPISupportclass.Allowing the MP CDI extension to prepare the model during start-up (rather than lazily upon first request for the OpenAPI document), but without exposing a
prepareModel()method aspublicinvolved creating SE and MP variants of theOpenAPISupportclass. The now-abstract superclassOpenAPISupportexposes aprotected prepareModel()method which the MP variant overrides, also asprotected, so the MP CDI extension in the same package can invoke it.And doing that in a type-safe way requires using type parameters on the
OpenAPISupportclass and its builder, which brought along another set of changes.As a result, there is a seemingly large number of changes required for a conceptually simple improvement in user experience, but doing it this way avoids expanding the public surface area more than we want.
Signed-off-by: tim.quinn@oracle.com tim.quinn@oracle.com