Skip to content
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

Create a CDI based API for custom converters #125

Open
OndroMih opened this issue Apr 13, 2017 · 9 comments
Open

Create a CDI based API for custom converters #125

OndroMih opened this issue Apr 13, 2017 · 9 comments
Assignees
Labels
use case 💡 An issue which illustrates a desired use case for the specification

Comments

@OndroMih
Copy link
Contributor

As mentioned in #124 , there's no CDI-based way to provide custom converters. They could be provided simply by adding a producer.

In similar way, we also don't have such API for ConfigSource and ConfigSourceProvider - each class that can be loaded via Service Loader should be at least considered to be loaded from a CDI producer (unless it's already too late when CDI producers are available)

@OndroMih OndroMih self-assigned this Apr 13, 2017
@Emily-Jiang
Copy link
Member

Emily-Jiang commented Apr 13, 2017

@OndrejM custom converters use the same pattern as config source, which are loaded via service loader. See converter javadoc.

Why do you need a CDI-based APIs for Converters?

@OndroMih
Copy link
Contributor Author

Service loader is not very friendly, especially when CDI is available. CDI producers are much more convenient to write than creating a service file. Developers prefer writing Java over textual files and referencing classes via full name. CDI producers are also checked at compile time, unlike service files, where it's easy to make a typo, which may often lead to hours of investigation.

@OndroMih
Copy link
Contributor Author

Simply, as a developer myself, I would see no reason why I would want to use service loader in a CDI container.

@struberg
Copy link
Contributor

@OndrejM Because the CDI container is probably not booted for a loooong time when you first access the Configuration ;)

We might have been able to go that way if we would have used the CDI-only approach (PR #40 I think) but that came with a few other drawbacks.

@OndroMih
Copy link
Contributor Author

@struberg I understand. But my point is not to replace the service loader, but provide an alternative way to define extensions. Most users would be application developers, which already can use the CDI container for everything.

@smillidge
Copy link
Contributor

I agree with Ondrej on this one. Currently ServiceLoader is somewhat limited. For example when implementing a custom ConfigSource there is no standard way of passing configuration to the ConfigSource on creation as ServiceLoader just instantiates an instance of the class. A CDI api could provide this configuration.

Configuration data is essential for a ConfigSource that talks to an external repository.

@Emily-Jiang
Copy link
Member

ok. In MP1.1, we can investigate the producer approach to supply converters and configsource.

@smillidge
Copy link
Contributor

smillidge commented Jul 26, 2017

Another option is annotation based definition similar to how JCA connectors can be deployed for example.

This doesn't have to depend on CDI but it could be specified to support ConfigProperty Injection by saying it should be created after all default config sources have been initialised so it can read its properties from default sources.

@ConfigSourceDefinition 
public FooConfigSource implements ConfigSource {

   @ConfigProperty(name="url", defaultValue="http://foo.bar.com")
   String url;

  ...  
}

@Emily-Jiang
Copy link
Member

Use case:
As a CDI developer, I want to use producer to supply a custom converter/config source/config prividers so that service loader is not mandatory.

@dmlloyd dmlloyd added the use case 💡 An issue which illustrates a desired use case for the specification label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Projects
None yet
Development

No branches or pull requests

6 participants