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

@Priority on producers #556

Closed
Ladicek opened this issue Nov 10, 2021 · 8 comments · Fixed by #699
Closed

@Priority on producers #556

Ladicek opened this issue Nov 10, 2021 · 8 comments · Fixed by #699
Milestone

Comments

@Ladicek
Copy link
Contributor

Ladicek commented Nov 10, 2021

Now that @Priority can be put on anything, we could (should?) support @Priority on producers (methods? fields? both?).

@mkouba
Copy link
Contributor

mkouba commented Nov 10, 2021

+1 for both, i.e. @Alternative producer methods and fields.

@manovotn
Copy link
Contributor

@Ladicek do you intent do include this in the next release or is it just planning issue?

@graemerocher
Copy link
Contributor

this is already supported in Micronaut btw

@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 10, 2021

I'm not proposing this for 4.0, just figured we should have a tracker for it.

@Ladicek
Copy link
Contributor Author

Ladicek commented May 24, 2023

I was thinking about this and realized that we need to deal with a few things.

Enablement

Currently, a producer is enabled if the declaring bean is enabled. We probably want to keep that? That is, if there's a producer that declares a @Priority, but the declaring bean is disabled, this producer will still be disabled.

Alternatively, a producer with @Priority could be enabled even if its declaring bean is disabled, but that would likely interfere with ambiguity resolution, specialization, and probably a few other things, so I'm not exactly a fan.

Ambiguity resolution

Currently, an @Alternative producer "inherits" @Priority from the bean class that declares the producer. We probably want that to keep working, but if the producer declares its own @Priority, we want that to win.

Examples

Declaring bean is not an alternative

@Dependent
class MyBean1 {
    @Alternative
    @Produces
    String producer1() { ... }

    @Alternative
    @Priority(1)
    @Produces
    String producer2() { ... }
}

@Dependent
@Priority(2)
class MyBean2 {
    @Alternative
    @Produces
    String producer3() { ... }

    @Alternative
    @Priority(3)
    @Produces
    String producer4() { ... }
}
  • producer1 is not enabled (and not selected)
  • producer2 is enabled, selected for application, priority 1
  • producer3 is enabled, selected for application, priority 2
  • producer4 is enabled, selected for application, priority 3

Declaring bean is an alternative

@Dependent
@Alternative
class MyBean3 {
    @Alternative
    @Produces
    String producer5() { ... }

    @Alternative
    @Priority(4)
    @Produces
    String producer6() { ... }
}

@Dependent
@Alternative
@Priority(5)
class MyBean2 {
    @Alternative
    @Produces
    String producer7() { ... }

    @Alternative
    @Priority(6)
    @Produces
    String producer8() { ... }
}
  • producer5 is not enabled (and not selected)
  • producer6 is not enabled (albeit it is selected for application, priority 4)
  • producer7 is enabled, selected for application, priority 5
  • producer8 is enabled, selected for application, priority 6

Thoughts?

Anything else we need to consider?

@manovotn
Copy link
Contributor

Alternatively, a producer with @priority could be enabled even if its declaring bean is disabled, but that would likely interfere with ambiguity resolution, specialization, and probably a few other things, so I'm not exactly a fan.

Not a fan either, you need an enabled bean to have a producer discovered on it. An exception might be static producer but I'm not sure that's worth the complexity.

Currently, an @Alternative producer "inherits" @priority from the bean class that declares the producer. We probably want that to keep working, but if the producer declares its own @priority, we want that to win.

Yes, I was thinking the same - this is akin to how bindings work for interceptors.

producer3 is enabled, selected for application, priority 2

Frankly speaking I always found this weird but I get why we want to preserve it. Would it be sensible to impose a deprecation and in some future version require that enabled producers should have the annotation on their respective method/field?
However, in this case there is still enablement via beans.xml which allows you per-class declaration IIRC (<alternatives>)...

producer6 is not enabled (albeit it is selected for application, priority 4)

I don't think it's even discovered because the bean declaring it isn't enabled (same for producer5 OFC).
See Bean Discovery:

For each enabled bean, the container must search the class for producer methods and fields, as defined in Producer methods and in Producer fields, including resources, and for disposer methods as defined in Disposer methods, and for observer methods.

@Ladicek
Copy link
Contributor Author

Ladicek commented May 25, 2023

Good point about discovery. The rest of the specification is written such that it doesn't really matter whether the producer is discovered or not -- if it would be discovered, it would still be disabled.

I also realized that by allowing @Priority on stereotypes in CDI 4.0, we actually already allowed putting priority directly on producers, so I'm thinking any changes we do here must directly follow from existing specification. It's probably also a good idea to experiment with how existing implementations work here.

@mkouba
Copy link
Contributor

mkouba commented May 25, 2023

Currently, a producer is enabled if the declaring bean is enabled. We probably want to keep that? That is, if there's a producer that declares a @Priority, but the declaring bean is disabled, this producer will still be disabled.

Makes sense. But frankly, this combination where both the declaring bean and the producer are @Alternative seems weird to me.

Currently, an @Alternative producer "inherits" @priority from the bean class that declares the producer. We probably want that to keep working, but if the producer declares its own @priority, we want that to win.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants