Re: [cdi-dev] This is StillDI

[Matej Novotny <manovotn@xxxxxxxxxx>]

----- Original Message -----
> From: "Ladislav Thon" <lthon@xxxxxxxxxx>
> To: "cdi developer discussions" <cdi-dev@xxxxxxxxxxx>
> Sent: Monday, March 8, 2021 3:36:43 PM
> Subject: Re: [cdi-dev] This is StillDI
> 
> Hi,
> 
> I wanted to better understand if my prototype implementation of proposed
> Build Compatible Extensions on top of Portable Extensions has any issues in
> practice. I turned into my favorite example, SmallRye Fault Tolerance, and
> reimplemented the existing Portable Extension with the new API. You can find
> it here if you're interested:
> https://github.com/Ladicek/smallrye-fault-tolerance/commits/cdi-lite It
> passes the entire MicroProfile Fault Tolerance TCK, and it passes the entire
> SmallRye Fault Tolerance test suite with a single exception (a test that
> uses Portable Extensions).
> 
> When I was prototyping the Build Compatible Extensions API on top of Quarkus,
> I did the same thing, so I can now compare.
> 
> In short: good progress is being made and the current Build Compatible
> Extensions API proposal seems pretty decent, though more work is needed.
> Long version follows.
> 
> I found 2 important things:
> 
> 1. Portable Extensions don't allow transforming annotations on annotations (=
> you don't get `ProcessAnnotatedType` for annotation types). This is possible
> in Quarkus. To address that, I created a new extension point
> `MetaAnnoations`, which can be used to turn existing annotations into
> qualifiers, interceptor bindings, stereotypes or scopes (scopes were already
> there in the form of `Contexts`, the rest is new). This is close to what
> Portable Extensions allow in `BeforeBeanDiscovery`.
> 
> 2. Sometimes, the extension runs some initialization code and stores state
> that other parts of the integrated framework want to access. That is a
> non-issue with Portable Extensions, because you can store the state in the
> extension instance and access it directly. With Build Compatible Extensions,
> we raise a barrier between the extension code (which can potentially execute
> during application build!) and the rest of the integrated framework. To
> cross that barrier, it is possible to use synthetic beans. Synthetic beans
> have to provide a creation function, which can run the initialization code
> (that would previously be in the extension). The synthetic bean instance
> would then store the state (and other parts of the framework would inject
> this bean instead of the extension instance). And since beans are
> instantiated on demand, it is possible to create a synthetic observer of the
> `@Initialized(ApplicationScoped.class) Object` event, which will trigger
> instantiation of the synthetic bean, thereby forcing the creation function
> to run.

>From earlier Quarkus days I can confirm that this is the general strategy that I saw a lot of integrating frameworks do to transfer their data from build time to runtime (and have them injectable).
Even if they don't really need the power of CDI extension to alter beans, creating a synth bean as their bootstrap metadata holder is a valid approach that I've seen multiple times.

> 
> The 2nd point is somewhat intricate, but once it clicks, it's fine. At least
> that's my experience. It also raises some questions:
> 
> 1. Is there a portable way to run some code during application startup? The
> `@Initialized(ApplicationScoped.class) Object` event is fired during
> container initialization, which can be during application build, so that's
> not it. I propose CDI standardizes an `AfterStartup` event (symmetrically to
> existing `BeforeShutdown`), which is guaranteed to be fired during
> application startup. I personally find `AfterStartup` and `BeforeShutdown` a
> lot more natural than `@Initialized(ApplicationScoped.class) Object` and
> `@Destroyed(ApplicationScoped.class) Object`. (Though to be perfectly
> honest, `BeforeShutdown` seems misnamed and, since all CDI contexts are
> invalid at that point in time, also quite useless.)

I am totally +1 for `AfterStartup` as a marker of "application started" milestone because what you say with `@Initialized` is a problem in build env.
However, it cannot be symmetrical to `BeforeShutdown` because that is a lifecycle event that you can observe in extensions and `AfterStartup` should instead be a standard CDI event that you can observe from within any CDI bean.
I am pretty sure that's what you meant, I just wanted to make it clear :)

> 
> This is easy to implement in a fully runtime environment, so I took the
> liberty of adding that to StillDI, even though it isn't part of the
> extension API.
> 
> 2. The in-extension part of initialization code (complemented by synthetic
> beans, as described above) often needs to know the full set of beans in the
> application. The `AppDeployment` API provides access to that, but there's
> currently nothing in the API that would let you know "all beans have been
> processed". Even though the API is push-based, both implementations I
> currently have (in Quarkus and on top of Portable Extensions) are actually
> synchronous, so I was able to make it work, but there's clearly something
> missing. At this point, I'm thinking we'll probably have to add something
> similar to Portable Extensions `ProcessBean` and `ProcessObserver`. That
> would be before the `@Synthesis` phase, and `@Synthesis` would then serve as
> the "all beans have been processed" point. I really wanted to avoid this,
> but it seems inevitable. Another option is to add a "all done" callback to
> the `AppDeployment` API.
> 

This is.... puzzling :-D
I more or less get what you're trying to do but I think it would be nice to hear from Micronaut folks whether or not this is blocking and if we aren't missing something.

> I currently have a huge TODO for this in the SmallRye Fault Tolerance
> implementation about this. Suggestions welcome.
> 
> LT
> 
> On Wed, Feb 24, 2021 at 4:59 PM Ladislav Thon < lthon@xxxxxxxxxx > wrote:
> 
> 
> Hi,
> 
> over last weeks, I've heard many people expressing a concern that what
> we're up to with CDI Lite, and especially with the new extension model,
> is "no longer CDI". I'm 100% sure that isn't anyone's intention, it
> certainly isn't mine.
> 
> On one of the recent CDI calls, I realized (again thanks Tomas Langer
> for the initial idea) that it's probably possible to implement the
> proposed Build Compatible Extensions API on top of existing Portable
> Extensions, proving that indeed this is still CDI, and indeed this can
> still be made a strict subset of CDI with a little help.
> 
> Here goes: https://github.com/Ladicek/StillDI
> 
> Note that the README isn't really meant to be end-user documentation.
> It's meant for CDI experts and mainly serves as a brief overview and a
> comparison between the 2 extension APIs.
> 
> As part of the work, I discovered a few discrepancies that we need to
> resolve, and fortunately, they are all really small. Specifically:
> 
> 1. The proposed Build Compatible Extensions API allows adding all
> subtypes of a given type to be scanned during bean discovery. Portable
> Extensions don't have that, and I think we should just remove this. The
> method to add one concrete type is obviously not an issue.
> 
> 2. The proposed Build Compatible Extensions API allows setting a
> priority of a synthetic bean (together with its "is alternative" flag).
> I propose we add this to Portable Extensions.
> 
> 3. The proposed Build Compatible Extensions API provides access to the
> "current" injection point when creating an instance of a synthetic bean,
> when its scope is @Dependent. This is possible with producer methods, so
> I don't see why it shouldn't be possible with synthetic beans. I propose
> we add this to Portable Extensions.
> 
> Other than that, I don't see anything that can't be losslessly
> translated between the 2 extension APIs, even though they are somewhat
> differently structured at certain places. (We might also want to align
> that structuring more, that is all open for debate.)
> 
> I'd be grateful for any kind of comments, preferrably here on this
> mailing list I guess (but don't really mind).
> 
> Thanks,
> 
> LT
> 
> _______________________________________________
> cdi-dev mailing list
> cdi-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/cdi-dev
> 
> 
> _______________________________________________
> cdi-dev mailing list
> cdi-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/cdi-dev
>