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

ArC: make it possible to restrict the bean types of a synthetic bean #44887

Open
mkouba opened this issue Dec 3, 2024 · 8 comments · May be fixed by #44916
Open

ArC: make it possible to restrict the bean types of a synthetic bean #44887

mkouba opened this issue Dec 3, 2024 · 8 comments · May be fixed by #44916
Labels
area/arc Issue related to ARC (dependency injection) kind/enhancement New feature or request

Comments

@mkouba
Copy link
Contributor

mkouba commented Dec 3, 2024

Description

It should be possible to restrict the bean types of a synthetic bean with the same semantics as @jakarta.enterprise.inject.Typed.

Currently, it's not possible to create a synthetic alternative to the following producer method:

    @Produces
    @ApplicationScoped
    @DefaultBean
    @Unremovable
    @Typed({ Mutiny.SessionFactory.class, Implementor.class })
    public MutinySessionFactoryImpl mutinySessionFactory() {
        if (jpaConfig.getDeactivatedPersistenceUnitNames()
                .contains(HibernateReactive.DEFAULT_REACTIVE_PERSISTENCE_UNIT_NAME)) {
            throw new IllegalStateException(
                    "Cannot retrieve the Mutiny.SessionFactory for persistence unit "
                            + HibernateReactive.DEFAULT_REACTIVE_PERSISTENCE_UNIT_NAME
                            + ": Hibernate Reactive was deactivated through configuration properties");
        }
        return (MutinySessionFactoryImpl) emf.unwrap(Mutiny.SessionFactory.class);
    }

Implementation ideas

Add the restrictTypes() method to the configurator API.

@mkouba mkouba added the kind/enhancement New feature or request label Dec 3, 2024
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Dec 3, 2024
Copy link

quarkus-bot bot commented Dec 3, 2024

/cc @Ladicek (arc), @manovotn (arc)

@yrodiere
Copy link
Member

yrodiere commented Dec 3, 2024

I thought addTypes was doing exactly that... am I mistaken?

@mkouba
Copy link
Contributor Author

mkouba commented Dec 3, 2024

I thought addTypes was doing exactly that... am I mistaken?

Not really, addTypes() merely adds the types to the set of bean types. It's like a manual discovery of bean types. Whereas this issue is about the possibility to restrict the set of bean types. In the example above, we would like to remove the MutinySessionFactoryImpl.class from the set of bean types.

@Ladicek
Copy link
Contributor

Ladicek commented Dec 3, 2024

To me, the main question here is: should restrictTypes() work like @Typed in that it accepts a set of classes and modifies the set of bean types so that it only contains the types that correspond to given classes [1], or should it be a variant of addType() / types() which clears the set of bean types at the beginning and then adds those that have been given to it [2]?

[1]

registration.configure(DotName.createSimple("com.example.MyClass"))
    .addType(com.example.MyInterface.class)
    .addType(ParameterizedType.create(List.class, ClassType.create(String.class)))
    .restrictTypes(MyInterface.class, List.class)   

would mean that the bean types of the synth bean would be MyInterface and List<String> (and Object of course)

[2]

registration.configure(DotName.createSimple("com.example.MyClass"))
    .restrictTypes(ClassType.create(MyInterface.class), ParameterizedType.create(List.class, ClassType.create(String.class)))   

would mean that the bean types of the synth bean would be MyInterface and List<String> (and Object of course)

@mkouba
Copy link
Contributor Author

mkouba commented Dec 3, 2024

To me, the main question here is: should restrictTypes() work like @Typed in that it accepts a set of classes and modifies the set of bean types so that it only contains the types that correspond to given classes [1], or should it be a variant of addType() / types() which clears the set of bean types at the beginning and then adds those that have been given to it [2]?

[1]

registration.configure(DotName.createSimple("com.example.MyClass"))
.addType(com.example.MyInterface.class)
.addType(ParameterizedType.create(List.class, ClassType.create(String.class)))
.restrictTypes(MyInterface.class, List.class)

would mean that the bean types of the synth bean would be MyInterface and List<String> (and Object of course)

[2]

registration.configure(DotName.createSimple("com.example.MyClass"))
.restrictTypes(ClassType.create(MyInterface.class), ParameterizedType.create(List.class, ClassType.create(String.class)))

would mean that the bean types of the synth bean would be MyInterface and List<String> (and Object of course)

+1 for the option [1]. It's more clear. Also it could be easily combined with automatic discovery - see #44888.

@manovotn
Copy link
Contributor

manovotn commented Dec 3, 2024

I thought addTypes was doing exactly that... am I mistaken?

I think Yoann has a point here.
As a user I would expect that my addType invocation explicitly define the whole set of types and that's it.
IMO we should look into how to change ArC behavior to treat it as such instead of asking users to take an extra step.

Or is there a benefit of having two different bean type sets that I am missing?

@mkouba
Copy link
Contributor Author

mkouba commented Dec 3, 2024

I thought addTypes was doing exactly that... am I mistaken?

I think Yoann has a point here. As a user I would expect that my addType invocation explicitly define the whole set of types and that's it. IMO we should look into how to change ArC behavior to treat it as such instead of asking users to take an extra step.

Or is there a benefit of having two different bean type sets that I am missing?

You're right that addType() or types() should be enough. Unless you need to remove the implementation class from the set of bean types because it's added automatically in the SyntheticBeanBuildItem#configure() factory methods. We would have to introduce new methods that would set the implementation class but would not add this class to the set of bean types. That's of course possible but not very elegant either (I can see users asking why so many factory methods exist..).

It could be also very useful when used together with the addTransitiveTypeClosure(Type) method requested in #44888.

What I like about this approach is that it mimics the behavior of @Typed. So that anyone who knows @Typed should immediately understand what's going on.

@geoand
Copy link
Contributor

geoand commented Dec 3, 2024

I thought addTypes was doing exactly that... am I mistaken?

This is the first thing I thought of as well

@geoand geoand changed the title ArC: make it possible to restric the bean types of a synthetic bean ArC: make it possible to restrict the bean types of a synthetic bean Dec 3, 2024
@manovotn manovotn linked a pull request Dec 4, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants