#10 CDI only proposal

[struberg]

This is a proposal which leverages CDI to create the Config.
Happy to explain the ideas.

[gunnarmorling]

Hey there, thanks for keeping pushing forward with this!

I'm a bit wondering about the usefulness of this method/class. If we want to mandate CDI (personally I don't think that's a good idea, but there seems to be a majority in favor of this), when would one use this static method? Wouldn't one rather obtain a Config via @Inject? And any non-CDI managed code wouldn't use this API anyways (e.g. I can't see any of the libraries I'm working on adding a dependency to CDI just for reading some config values), so I'm curious what's the use case for this static method.

[struberg]

Yes and yes.

1.) Yes, we can actually remove the whole ConfigProvider.
1.a User code would use @Inject Config (or even more fine granular future features, like the owner-style config you proposed).
1.b Exension developer can @observes Config or use beanManager.getExtension
1.c Non-CDI parts could still use CDI.current()...

2.) Yes, by mandating CDI in the core parts we loose adoption in every library where CDI is only optional. E.g. Hibernate or OpenJPA will never use it that way. Nor even any other EE component. Because in all of them CDI is only used IF available.
I tried to explain this point for months now, but it seems that people first need to see the mess before they get it (Brexit, Trump, anyone?)

[struberg]

Gunnar, I removed ConfigProvider and created a way to use @Inject Config in any user land code. Docs probably still needs updates.

[Emily-Jiang]

-1 on ConfigBean and ConfigExtension. ConfigExtension is implementation details. All implementation should provide config bean to be injected. No api is to be introduced.

[Emily-Jiang]

I am proposing to load the config sources automatically via introducing annotation and dynamic aspect on another issue.

[struberg]

@Emily-Jiang please create a new branch and show us how you would envision this. Please show us your working ideas!

There are 3 important use cases:
1.) Config must be accessible in Extensions. Even in BeforeBeanDiscovery!
2.) Config must be accessible from non CDI code parts like a JPA Listener. Of course also during startup.
3.) Config must obviously also be accessible in user code during the normal container runtime.

-1 on ConfigBean and ConfigExtension. ConfigExtension is implementation details.

The ConfigBean is already an implementation detail. And for accessing the Config outside of an Extension the ConfigExtension is absolutely necessary to keep in the API. I don't love this neither, but I could not find any better solution. Because getExension only works with the concrete Extension class and not with any interface, base class, etc.
In fact the proposed API is most probably already as lean as it gets as many implementation details are deferred to the impl!

I am proposing to load the config sources automatically via introducing annotation and dynamic aspect on another issue.

You mean you would like to introduce another manual class scanning? ^^
Because you will not be able to utilize ProcessAnnotatedType.

[OndroMih]

@struberg:

There are 3 important use cases:
1.) Config must be accessible in Extensions. Even in BeforeBeanDiscovery!
2.) Config must be accessible from non CDI code parts like a JPA Listener. Of course also during startup.
3.) Config must obviously also be accessible in user code during the normal container runtime.

I would agree that the above are important, but in reverse order. Why do we need to focus on extensions more than the user code, which will cover 99% of config usecases? I suggest focusing first on the user code API, something like Sabot and defer support for extensions to later.

And for accessing the Config outside of an Extension the ConfigExtension is absolutely necessary

I think that most of the usecases would be covered by the event fired from the ConfigExtension.buildConfig method, isn't that true? Any extension that depends on configuration can store its context and wait until it receives the event with the final configuration. I don't see any reason why we need to support anything else in the first version, and I'm strongly opposed to having ConfigExtension in the API.

But enough words, I'm playing with the CDI API now and will soon raise a PR to fuel the discussion and present my point of view. The impl in this PR looks good. The PR just doesn't provide any user level CDI API I expected. I don't agree that just injecting Config interface is enough to make developers life easy.

[struberg]

@OndrejM Of course @Inject Config already works out of the box!

But you really also need Configuraiton in Extensions. And sometimes even in BeforeBeanDiscovery. E.g. if you conditionally want to add AnnotatedTypes.
Here is a production example: https://github.com/apache/deltaspike/blob/573c1c88d217468d2589c17d665ae933cdd8eb1d/deltaspike/core/impl/src/main/java/org/apache/deltaspike/core/impl/resourceloader/ResourceLoaderExtension.java#L36

consider you want do to this conditionally depending on some configuation. We have such things quite some time in DeltaSpike
https://github.com/apache/deltaspike/blob/573c1c88d217468d2589c17d665ae933cdd8eb1d/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/TransactionContextExtension.java#L36

The other point is that you need a way to access your config from locations which are not directly linked to an Extension. In that case you need some way of 'factory lookup'.
By having the ConfigExtension in the API you can do

CDI.current().getBeanManager().getExtension(ConfigExension.class).getConfig()

Without any 'accessor' you are out of luck!

Because you can NOT inject a Config yet, and you can NOT use CDI.current().select(..) YET!

[Emily-Jiang]

@struberg All other places that cannot use Config injection should fall back to use ConfigProvide.getConfig. For CDI injection, we can support @Inject Config config. The configExtension is unnecessary. Take Transactional for an example, the only needed step is to put on the spec. The integration will implement it. As said multiple times, it is IMPLEMENTATION details, and it should not be part of API.

[struberg]

@Emily-Jiang where does ConfigProvider#getConfig() gets the info from? An own 'storage' or CDI.current() again?

I removed the ConfigProvider because the CDI.current() trick is already an official JavaEE API and while we need to support the use case of getting access to Config from outside of Extensions it's just a matter of documentation to tell the users what they should use instead.

In my opinion we should not introduce any API for functionality which already exists in a portable way.

Of course, If we do NOT use the ConfigurationExtension that we need the ConfigProvider + storage which the majority here seems to dislike (still not sure why btw, I really like the SE style basis)

[Emily-Jiang]

I have different perception on ConfigProvider + storage, which has wider usage than ConfigurationExtension. IIRC, most people wants to use injection, which is fine. Having the programmatical support makes the apis/spis strong and it is much easier to be understood.

[struberg]

@Emily-Jiang
This proposal has programmatic support! Simply use CDI.current().
It also solves the chicken-egg problem during container boot.

The thing which I do not like is that this now mandatory requires a CDI container. So it will not see adoption outside pure Java EE. Probably not even in libs which will run fine in JavaEE but do not require it (e.g. JPA, JSF, etc).

[Emily-Jiang]

@struberg The approach is no better than the existing one and it adds more mandatory dependencies for no apparent reason. It has less value, which makes the whole thing sound very awkward.

[struberg]

Functionally it provides exactly the same.
The only difference is that it mandatory requires CDI.

Again I'm also not a big fan IF we would aim for a JSR or standard. In which case I would aim for a CDI-indepenendent solution.
But for just using it in microprofile it is probably totally fine.

[Emily-Jiang]

-1 on putting any implementation in this repo. This repo should just have api, doc and tck. Any vendor can implement this but reside in different repos. All extension stuff are purely implementation details.

[kenfinnigan]

@Emily-Jiang why no implementation?

Isn't it better if the community collaborate on an implementation for a proposal, instead of each vendor implementing their own?

Even given a common implementation, it would still be up to a vendor as to whether they chose to use that or implement their own.

Without an implementation a proposal means nothing as it can't be used by consumers of MicroProfile.

[Emily-Jiang]

@kenfinnigan We can list a few implementations but the implementation may not be suitable to live in the MicroProfile. The reason on not storing the implementation here is that who will maintain the jira issues and fixes on the implementation. Take cdi for an example. CDI spec is one repo. Weld is owned under a different community. I suggest we should have api, doc and tck here. Impl should reside in a different area. We can list all impls in our doc.

[ebullient]

@kenfinnigan -- I think @Emily-Jiang's question about issue management is key .. is there a reason to keep api/doc/tck in one impl, and then a join implementation in another?

[struberg]

Well, the whole discussion should have a much broader spectrum

I agree that any implementation inside this repo does not make any sense IF we target a Specification. But with a Specification we would also create a standards approach. And this is what many involved Managers did explicitly deny. So if we don't want to create a standard but just 'innovate' (cit [1]) then the impl is fine.

Otherwise IF we create a standard, then again the org.eclipse.* package name is wrong as this would be vendor specific. (And of course Eclipse IS a vendor, albeit no commercial one).

Management: RUN FASTER
Developer: Which direction?
Management: I DON'T CARE

[OndroMih]

I wouldn't mind having an initial reference implementation inside the same repository. We can also create a separate repository for such implementation. However, later we could be forced to separate it as the same implementation may be used as a reference for more specifications to make us more productive in moving forward.
What about creating a separate repo for RIs, which would hold simple RIs in separate directories for all specs? Something like microprofile-refeference-impl?

[Emily-Jiang]

@struberg @OndrejM @ebullient Please see the thread discussion (https://groups.google.com/forum/#!topic/microprofile/1RIxSNqN8G0). Mark Little shares the same view as mine. We can let the implementations live outside microprofile and we can always link them.

[gunnarmorling]

I wouldn't mind having an initial reference implementation inside the same repository.

I think it's better if it's in a separate repo. Makes the split more apparent and helps to avoid any unwanted dependencies. Also API and impl are likely to be versioned separately.

What about creating a separate repo for RIs, which would hold simple RIs in separate directories for all specs? Something like microprofile-refeference-impl?

I'd rather have a dedicated repo per project/RI. The reason being that one typically wants to version them independently (e.g. for releasing a new bug fix release of the config RI), which is better doable if it's a separate repo (think of tagging, branching etc.).

[struberg]

+1 to gunnar and emily. I Can easily split it out. Will do if we agree to go the CDI route. Otherwise it is moot anyway as it is just in the PR on my personal repo.
My main goal was to experiment with it. It's easy to draft an API - but it's hard to create an API which really works well and is consistent! For doing that you need to actually implement it.
I've done this for the other design, so I know it works - but had no clue if my ideas for the purely CDI based solution would work in practice.

[struberg]

as discussed in the f2f meeting. nice idea but would require all users to mandatory depend on CDI. Thus we will follow the SE based route.