Re: [cdi-dev] Feedback - CDI bootstrap API (CDI-26)
by John D. Ament
Hmm.. comments in line.
On Wed, Mar 4, 2015 at 4:49 AM Antoine Sabot-Durand <
antoine(a)sabot-durand.net> wrote:
> Hi John,
>
> I think it could be a good idea to write down all of these to have a more
> stable doc for discussion. You should update the google doc with the result
> of this discussion.
>
> I agree with the following points in this thread :
>
> - Minimize the number of new Class / Interface. CDI and CDIProvider usage
> is still not very clear for end user so we should add the strict minimum
> and try to enhance existing API / SPI when possible
>
It seems odd to me that we're rehashing decisions made during the EG
meetings. Not putting it in CDI was discussed in several meetings at the
beginning of the year, and it seemed like the agreement was putting it in
CDI was a bad idea.
> - Be able to bootstrap CDI without returning BeanManager (env if the API
> give possibility to access it if needed). End user don’t need that : CDI
> app can start with an observer for instance
>
Agreed, but I think we need to provide some entry point to allow those who
aren't comfortable with programming with events to leverage it. Returning
the CDI instance makes that easier to do than returning the BeanManager.
>
> Something not dealt with but that we should keep in mind :
> - Providing a Java SE solution that could be easily used for a servlet
> bootstrap of CDI. I don’t know if we’ll standardize this but we definitely
> should keep this use case in mind
>
> and my bonus, it’s out of scope but I didn’t see anytime there that
> prevent this nice to have:
> - support the possibility to boot multiple BeanManager in Java SE.
>
>
We talked about this one as well on the EG, either this year or late last
year. I thought the decision at that time was that we wouldn't allow
multiple containers at once in SE.
> Antoine
>
>
> Le 1 mars 2015 à 15:13, John D. Ament <john.d.ament(a)gmail.com> a écrit :
>
>
> So, I think I've gathered enough feedback at this point, and seen some of
> the API changes. I'll hopefully be including some doc changes this week,
> but one question - do we want to start the SE specific stuff as its own
> asciidoc file?
>
> Changes made:
>
> - Changed return value to CDI<Object> to provide better capability out of
> the box.
> - Added AutoCloseable to CDIContainer, provided default implementation of
> calling shutdown.
> - Added synchronization support to the method body that retrieves the
> singleton instance (BTW, I'm not sure if this is even useful TBH as each
> impl, including the RI, needs to provide this class in its own format).
> - Made the params map typed to <String,Object>
>
> @Romain Your case isn't really supportable yet, until we have static
> injection support. You'd still have to have a managed version of Runner to
> work against.
>
> John
>
> On Sat, Feb 28, 2015 at 4:11 PM Romain Manni-Bucau <rmannibucau(a)gmail.com>
> wrote:
>
>> Yes but not the way to get an instance. Even Unmanaged does it.
>>
>> What can be awesome is to have static inject for it:
>>
>> public class Runner {
>>
>> @Inject
>> private static MyMain main;
>>
>> public static void main(String[] arg) {
>> try (CDIContainer c = CDIContainer.newContainer()) {
>> main.doWork();
>> }
>> }
>>
>> }
>>
>> And not a single additional line :).
>> Le 28 févr. 2015 19:05, "John D. Ament" <john.d.ament(a)gmail.com> a
>> écrit :
>>
>> Maybe I'm misreading, but I don't see us adding another API to do the
>>> same thing here - we're introducing new functionality.
>>>
>>> CDIContainer/Loader on startup/shutdown of the application
>>>
>>> CDI for runtime usage within the application to interact with the
>>> container.
>>>
>>> John
>>>
>>> On Fri, Feb 27, 2015 at 3:40 AM Romain Manni-Bucau <
>>> rmannibucau(a)gmail.com> wrote:
>>>
>>>> sure I fully agree excepted I think introducing yet another API to do
>>>> the same thing is not good so super tempting to skip it and wait for
>>>> feedbacks rather than introducing it eagerly.
>>>>
>>>>
>>>> Romain Manni-Bucau
>>>> @rmannibucau
>>>> http://www.tomitribe.com
>>>> http://rmannibucau.wordpress.com
>>>> https://github.com/rmannibucau
>>>>
>>>>
>>>> 2015-02-27 8:05 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com>:
>>>> > My point is that from the application perspective, the user obtains
>>>> one
>>>> > container handle for eventual shutdown (CDIContainer) and then looks
>>>> up a
>>>> > different container handle (CDI) that they can use for real work
>>>> (lookup /
>>>> > event dispatch / etc.) It would be cleaner if the container gave away
>>>> a
>>>> > single handle that can do all of that.
>>>> >
>>>> >
>>>> > On 02/26/2015 05:42 PM, Romain Manni-Bucau wrote:
>>>> >
>>>> > Not sure I get how a CDI instance can help.
>>>> >
>>>> > But container.getBeanManager() sounds nice is not a shortcut for
>>>> > CDI.current().getBm() otherwise it looks like duplication to me.
>>>> >
>>>> > Can we make container not contextual - dont think so? If so it makes
>>>> sense
>>>> > otherwise I fear it doesnt add much.
>>>> >
>>>> > Le 26 févr. 2015 16:19, "Jozef Hartinger" <jharting(a)redhat.com> a
>>>> écrit :
>>>> >>
>>>> >> I like the initialize + close() combination and the
>>>> try-with-resources
>>>> >> usage.
>>>> >> What looks weird to me is that at line one you obtain a container
>>>> handle:
>>>> >>
>>>> >> try (CDIContainer container = CDIContainer.newCDIContai...
>>>> >> CDI.current().getBeanManager() ...
>>>> >>
>>>> >> and then at line two you call a static method to perform a container
>>>> >> lookup :-/
>>>> >>
>>>> >> An API that allows you to use the container handle you already got
>>>> is way
>>>> >> better IMO, e.g.:
>>>> >>
>>>> >> try (CDIContainer container = CDIContainer.newCDIContai...
>>>> >> container.getBeanManager()
>>>> >>
>>>> >> If CDIContainer.newCDIContainer() returns an CDI instance or its
>>>> subclass,
>>>> >> we get this easily.
>>>> >>
>>>> >> On 02/26/2015 08:58 AM, Romain Manni-Bucau wrote:
>>>> >>>
>>>> >>> Hi guys
>>>> >>>
>>>> >>> why note keeping it simple?
>>>> >>>
>>>> >>> try (CDIContainer container = CDIContainer.newCDIContainer(/*
>>>> optional
>>>> >>> map to configure vendor features */)) {
>>>> >>> CDI.current().getBeanManager()....
>>>> >>> }
>>>> >>>
>>>> >>> Not sure the point having initialize() + having shutdown = close
>>>> >>> really makes the API more fluent and modern IMO.
>>>> >>>
>>>> >>> Also to be fully SE I guess provider() method would be needed even
>>>> if
>>>> >>> optional (SPI usage by default):
>>>> >>>
>>>> >>> try (CDIContainer container =
>>>> >>>
>>>> >>> CDIContainer.provider("org.jboss.weld.WeldCdiContainerProvider").
>>>> newCDIContainer())
>>>> >>> {
>>>> >>> CDI.current().getBeanManager()....
>>>> >>> }
>>>> >>>
>>>> >>> Finally I think having a kind of getInstance shortcut could be a
>>>> plus for
>>>> >>> SE:
>>>> >>>
>>>> >>> try (CDIContainer container = CDIContainer.newCDIContainer()) {
>>>> >>> container.newInstance(MyAppRunner.class /* optional
>>>> qualifiers */
>>>> >>> ).run(args);
>>>> >>> }
>>>> >>>
>>>> >>> Using container to get an instance would create the instance and
>>>> bind
>>>> >>> it to the container lifecycle (mainly for predestroy) avoiding this
>>>> >>> boilerplate code in all main which will surely only be used to
>>>> launch
>>>> >>> a soft.
>>>> >>>
>>>> >>> wdyt?
>>>> >>>
>>>> >>>
>>>> >>>
>>>> >>> Romain Manni-Bucau
>>>> >>> @rmannibucau
>>>> >>> http://www.tomitribe.com
>>>> >>> http://rmannibucau.wordpress.com
>>>> >>> https://github.com/rmannibucau
>>>> >>>
>>>> >>>
>>>> >>> 2015-02-26 8:32 GMT+01:00 Jozef Hartinger <jharting(a)redhat.com>:
>>>> >>>>
>>>> >>>> Comments inline
>>>> >>>>
>>>> >>>> On 02/25/2015 05:53 PM, John D. Ament wrote:
>>>> >>>>
>>>> >>>> Sorry Jozef, your email fell into the pits of google inbox's "smart
>>>> >>>> sorting"
>>>> >>>> features.
>>>> >>>>
>>>> >>>> On Thu, Feb 12, 2015 at 3:18 AM Jozef Hartinger <
>>>> jharting(a)redhat.com>
>>>> >>>> wrote:
>>>> >>>>>
>>>> >>>>> Hi John, comments inline:
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> On 02/11/2015 06:02 PM, John D. Ament wrote:
>>>> >>>>>
>>>> >>>>> Jozef,
>>>> >>>>>
>>>> >>>>> Most of what you see there is taken from the original doc, since
>>>> >>>>> everyone
>>>> >>>>> seemed to be in agreement. I think the map is just a safeguard
>>>> in case
>>>> >>>>> of
>>>> >>>>> additional boot options available in some implementations (e.g. I
>>>> think
>>>> >>>>> OWB/OpenEJB have some options.. currently OpenEJB supports an
>>>> embedded
>>>> >>>>> CDI
>>>> >>>>> boot mode).
>>>> >>>>>
>>>> >>>>> No, I am fine with the map. What I am questioning is the type of
>>>> the
>>>> >>>>> map.
>>>> >>>>> Usually, data structures with a similar purpose use Strings as
>>>> their
>>>> >>>>> keys.
>>>> >>>>> This applies to ServletContext attributes, InvocationContext data,
>>>> >>>>> Servlet
>>>> >>>>> request/session attributes and others. I am therefore wondering
>>>> whether
>>>> >>>>> there is a usecase for the proposed unbound key signature or not.
>>>> >>>>
>>>> >>>>
>>>> >>>> I think that's more of a placeholder, I was assuming it would be
>>>> >>>> Map<String,Object> once we clarify everything.
>>>> >>>>
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> We spoke a few times about BeanManager vs CDI. BeanManager was
>>>> >>>>> preferable
>>>> >>>>> since there's no easy way to get the the instance, CDI is easier
>>>> to get
>>>> >>>>> and
>>>> >>>>> more aligned with how you would get it. Usually people expect the
>>>> >>>>> BeanManager to be injected or available via JNDI, neither would
>>>> be the
>>>> >>>>> case
>>>> >>>>> here.
>>>> >>>>>
>>>> >>>>> If CDI 2.0 targets Java SE then this container initialization API
>>>> will
>>>> >>>>> become something that ordinary application developers use to
>>>> start/stop
>>>> >>>>> CDI
>>>> >>>>> in their applications. It therefore cannot be considered an SPI
>>>> but
>>>> >>>>> instead
>>>> >>>>> should be something easy to use. On the other hand, BeanManager is
>>>> >>>>> definitely an SPI. It is used in extension, frameworks and
>>>> generally
>>>> >>>>> for
>>>> >>>>> integration. Not much by applications directly. Therefore, I
>>>> don't see
>>>> >>>>> how
>>>> >>>>> the container bootstrap API and BeanManager fit together. IMO the
>>>> >>>>> bootstrap
>>>> >>>>> API should expose something that makes common tasks (obtaining a
>>>> >>>>> contextual
>>>> >>>>> reference and firing and event) easy, which the CDI class does.
>>>> >>>>>
>>>> >>>>> Plus do not forget that BeanManager can be obtained easily using
>>>> >>>>> CDI.getBeanManager().
>>>> >>>>
>>>> >>>>
>>>> >>>> I'm not disagreeing. There's a few things I'd consider:
>>>> >>>>
>>>> >>>> - Is this mostly for new apps or existing? If existing, it's
>>>> probably
>>>> >>>> using
>>>> >>>> some internal API, if new it can use whatever API we give.
>>>> >>>> - I don't want to return void, we should give some kind of
>>>> reference
>>>> >>>> into
>>>> >>>> the container when we're done booting.
>>>> >>>>
>>>> >>>> Agreed, we should not be returning void.
>>>> >>>>
>>>> >>>> - CDI is a one step retrievable reference, where as BeanManager is
>>>> a two
>>>> >>>> step reference. With that said, BeanManager makes more sense to
>>>> return
>>>> >>>> here. Another thought could be we invent some new class that has
>>>> both,
>>>> >>>> but
>>>> >>>> that's really redundant.
>>>> >>>>
>>>> >>>> Why do you think BeanManager makes more sense here? Especially
>>>> given the
>>>> >>>> assumption that application code is going to call this
>>>> init/shutdown
>>>> >>>> API, I
>>>> >>>> don't see BeanManager as making more sense.
>>>> >>>>
>>>> >>>>
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> Yes, this is the container start API. Sounds like you have some
>>>> good
>>>> >>>>> ideas for things like XML configuration or programmatic
>>>> configuration,
>>>> >>>>> both
>>>> >>>>> of which are being tracked under separate tickets. One idea
>>>> might be
>>>> >>>>> for an
>>>> >>>>> optional param in the map to control packages to scan/ignore, in
>>>> that
>>>> >>>>> map.
>>>> >>>>>
>>>> >>>>> I am wondering whether this configuration should be something
>>>> optional
>>>> >>>>> built on top of the bootstrap API or whether we should consider
>>>> making
>>>> >>>>> it
>>>> >>>>> mandatory. Either way, we cannot add the bootstrap API to the spec
>>>> >>>>> without
>>>> >>>>> explicitly defining how it behaves. My implicit assumption of the
>>>> >>>>> proposal
>>>> >>>>> is that the container is supposed to scan the entire classpath for
>>>> >>>>> explicit
>>>> >>>>> or implicit bean archives (including e.g. rt.jar), discover
>>>> beans, fire
>>>> >>>>> extensions, etc. This worries me as this default behavior is far
>>>> from
>>>> >>>>> being
>>>> >>>>> lightweight, which CDI for Java SE initially aimed to be.
>>>> >>>>
>>>> >>>>
>>>> >>>> Yes, the spec must be updated to reflect the behavior of SE mode.
>>>> I
>>>> >>>> plan to
>>>> >>>> get that completely into the google doc before opening any spec
>>>> changes
>>>> >>>> in a
>>>> >>>> PR.
>>>> >>>>
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> We didn't want to over load the CDI interface. It already does a
>>>> lot.
>>>> >>>>> This is really SPI code, CDI even though it's in the spi package
>>>> is
>>>> >>>>> used in
>>>> >>>>> a lot of application code.
>>>> >>>>>
>>>> >>>>> I would personally prefer to have it all in one place. Having
>>>> >>>>> CDIContainer, CDIContainerLoader, CDI and CDIProvider makes it
>>>> more
>>>> >>>>> difficult to know when to use what.
>>>> >>>>
>>>> >>>>
>>>> >>>> The problem is that most CDI (the interface) operations are
>>>> against a
>>>> >>>> running container. I think we spoke about leveraging CDIProvider
>>>> at one
>>>> >>>> point (in fact, I mistakenly called CDIContainer CDIProvider not
>>>> even
>>>> >>>> realizing it was there). I doubt that most app developers use it
>>>> >>>> currently,
>>>> >>>> there's not even a way to get a reference to it that I'm aware
>>>> of. It's
>>>> >>>> used by the implementor only.
>>>> >>>>
>>>> >>>> I don't think there's a conflict. CDI class would still only
>>>> provide
>>>> >>>> methods
>>>> >>>> to be run against a running container. The difference is that there
>>>> >>>> would be
>>>> >>>> additional static methods to get this running container (CDI
>>>> class) to
>>>> >>>> you
>>>> >>>> by starting the container.
>>>> >>>>
>>>> >>>> Either way, I agree that reusing CDIProvider is a must. There is no
>>>> >>>> reason
>>>> >>>> to define a new class for the same purpose.
>>>> >>>>
>>>> >>>>
>>>> >>>> I expect that my changes in the CDI spec around this will state,
>>>> along
>>>> >>>> the
>>>> >>>> lines of:
>>>> >>>>
>>>> >>>> To retrieve a CDIContainer to launch, do this:
>>>> >>>>
>>>> >>>> CDIContainer container = CDIContainerLocator.getCDIContainer();
>>>> >>>> container.initialize();
>>>> >>>> ... do work
>>>> >>>>
>>>> >>>> Once you want to shutdown the container, do this:
>>>> >>>>
>>>> >>>> container.shutdown();
>>>> >>>>
>>>> >>>> (we may want to consider implementing AutoCloseable, an oversight
>>>> on my
>>>> >>>> part)
>>>> >>>>
>>>> >>>> and then later on
>>>> >>>>
>>>> >>>> - What happens if I call CDIContainerLocator in an app server
>>>> >>>>
>>>> >>>> - It throws an IllegalStateException.
>>>> >>>>
>>>> >>>> - The container provides no beans of type CDIContainer, it is
>>>> managed
>>>> >>>> outside of the CDI container.
>>>> >>>>
>>>> >>>>
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> John
>>>> >>>>>
>>>> >>>>> On Wed Feb 11 2015 at 4:21:50 AM Jozef Hartinger <
>>>> jharting(a)redhat.com>
>>>> >>>>> wrote:
>>>> >>>>>>
>>>> >>>>>> Hi John, some thoughts:
>>>> >>>>>>
>>>> >>>>>> - instead of using BeanManager it makes more sense to me to
>>>> return a
>>>> >>>>>> CDI
>>>> >>>>>> instance, which is a more user-friendly API (and it also exposes
>>>> >>>>>> access to
>>>> >>>>>> BeanManager)
>>>> >>>>>> - is there a usecase for arbitrary keys of the "params" map or is
>>>> >>>>>> Map<String, ?> sufficient?
>>>> >>>>>> - if we could move the shutdown() method from CDIContainer to the
>>>> >>>>>> actual
>>>> >>>>>> container handle that we obtain from initialize(), that would
>>>> look
>>>> >>>>>> more
>>>> >>>>>> object-oriented
>>>> >>>>>> - what exactly is initialize() supposed to do? Is it supposed to
>>>> start
>>>> >>>>>> scanning the entire classpath for CDI beans? That could be a
>>>> problem
>>>> >>>>>> especially with spring-boot-like fat jars. I think we need an
>>>> API to
>>>> >>>>>> tell
>>>> >>>>>> the container which classes / packages to consider. Something
>>>> like
>>>> >>>>>> Guice's
>>>> >>>>>> binding API perhaps?
>>>> >>>>>>
>>>> >>>>>> - the proposal makes me wonder whether retrofitting this
>>>> functionality
>>>> >>>>>> to
>>>> >>>>>> the CDI class wouldn't be a better option. It could look like:
>>>> >>>>>>
>>>> >>>>>> CDI container = CDI.initialize();
>>>> >>>>>> container.select(Foo.class).get();
>>>> >>>>>> container.shutdown();
>>>> >>>>>>
>>>> >>>>>> compare it to:
>>>> >>>>>>
>>>> >>>>>> CDIContainer container = CDIContainerLoader. getCDIContainer();
>>>> >>>>>> BeanManager manager = container.initialize();
>>>> >>>>>> manager.getBeans(...);
>>>> >>>>>> container.shutdown(manager);
>>>> >>>>>>
>>>> >>>>>>
>>>> >>>>>> On 02/10/2015 06:58 PM, John D. Ament wrote:
>>>> >>>>>>
>>>> >>>>>> All,
>>>> >>>>>>
>>>> >>>>>> I have the updated API here, and wanted to solicit any final
>>>> feedback
>>>> >>>>>> before updating the google doc and spec pages.
>>>> >>>>>>
>>>> >>>>>>
>>>> >>>>>>
>>>> >>>>>> https://github.com/johnament/cdi/commit/2c362161e18dd521f8e8
>>>> 3c27151ddad467a1c01c
>>>> >>>>>>
>>>> >>>>>> Let me know your thoughts.
>>>> >>>>>>
>>>> >>>>>> Thanks,
>>>> >>>>>>
>>>> >>>>>> John
>>>> >>>>>>
>>>> >>>>>>
>>>> >>>>>> _______________________________________________
>>>> >>>>>> cdi-dev mailing list
>>>> >>>>>> cdi-dev(a)lists.jboss.org
>>>> >>>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>> >>>>>>
>>>> >>>>>> Note that for all code provided on this list, the provider
>>>> licenses
>>>> >>>>>> the
>>>> >>>>>> code under the Apache License, Version 2
>>>> >>>>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all
>>>> other ideas
>>>> >>>>>> provided on this list, the provider waives all patent and other
>>>> >>>>>> intellectual
>>>> >>>>>> property rights inherent in such information.
>>>> >>>>>>
>>>> >>>>>>
>>>> >>>>
>>>> >>>> _______________________________________________
>>>> >>>> cdi-dev mailing list
>>>> >>>> cdi-dev(a)lists.jboss.org
>>>> >>>> https://lists.jboss.org/mailman/listinfo/cdi-dev
>>>> >>>>
>>>> >>>> Note that for all code provided on this list, the provider
>>>> licenses the
>>>> >>>> code
>>>> >>>> under the Apache License, Version 2
>>>> >>>> (http://www.apache.org/licenses/LICENSE-2.0.html). For all other
>>>> ideas
>>>> >>>> provided on this list, the provider waives all patent and other
>>>> >>>> intellectual
>>>> >>>> property rights inherent in such information.
>>>> >>
>>>> >>
>>>> >
>>>>
>>> _______________________________________________
> cdi-dev mailing list
> cdi-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/cdi-dev
>
> Note that for all code provided on this list, the provider licenses the
> code under the Apache License, Version 2 (http://www.apache.org/
> licenses/LICENSE-2.0.html). For all other ideas provided on this list,
> the provider waives all patent and other intellectual property rights
> inherent in such information.
>
>
9 years, 10 months
[JBoss JIRA] (CDI-514) FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
by Jozef Hartinger (JIRA)
[ https://issues.jboss.org/browse/CDI-514?page=com.atlassian.jira.plugin.sy... ]
Jozef Hartinger commented on CDI-514:
-------------------------------------
If an invariant of a method is well-defined then the method should validate it.
As for why it is defined this way then I think it enforces consistency between what you can achieve using annotation on classes and what you can achieve programatically. Let's revisit this constraint for CDI 2.0
> FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
> --------------------------------------------------------------
>
> Key: CDI-514
> URL: https://issues.jboss.org/browse/CDI-514
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> testDuplicateBindingsToFireEventFails() tests for 2 Lifted literals with different values. But this is perfectly fine as value is NOT annotated as @Nonbinding. Thus the 2 literals are NOT equals according to CDI rules. They are essentially 2 different annotations...
> Plz remove this test. It also makes no sense to add all those very performance costly tests at runtime. The worst case which can happen is that the 2 annotations make no sense. But they don't break anything. Wheras checking all the nasty conditions each and every time is really mad from a performance aspect.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
Bean Discovery Mode in SE
by John D. Ament
All,
I'd like to propose in my doc changes for CDI-26 that if a classpath entry
does not contain a META-INF/beans.xml that it is treated as if it were
bean-discovery-mode=none, e.g. no beans will be scanned in that entry.
(BTW, I'm using classpath entry rather than archive in the document to
account for cases where someone does -cp
"./classes:./extra-classes:./lib/*" to define their classpath)
It's a bit different than how EE works, but I could imagine it causing
fewer headaches when running on SE classpaths.
Any thoughts/comments?
John
9 years, 10 months
[JBoss JIRA] (CDI-514) FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-514?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-514:
-----------------------------------
correctness? of what? This is currently preventing a perfectly valid use case. IF then it would need to be check for Qualifier equality (taking binding params into account).
But even if not - what happens if a user queries with n times the same qualifier? Nothing, nada, njente. There is just no impact! You will always get back the same Bean.
So it DOES break valid use cases and it DOES NOT prevent some broken cases -> totally useless, and we should fix this.
> FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
> --------------------------------------------------------------
>
> Key: CDI-514
> URL: https://issues.jboss.org/browse/CDI-514
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> testDuplicateBindingsToFireEventFails() tests for 2 Lifted literals with different values. But this is perfectly fine as value is NOT annotated as @Nonbinding. Thus the 2 literals are NOT equals according to CDI rules. They are essentially 2 different annotations...
> Plz remove this test. It also makes no sense to add all those very performance costly tests at runtime. The worst case which can happen is that the 2 annotations make no sense. But they don't break anything. Wheras checking all the nasty conditions each and every time is really mad from a performance aspect.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
Today's meeting agenda
by Antoine Sabot-Durand
Hi all,
I’d like to focus on Java SE support in today’s meeting. We had a lot of discussion around these api (thanks John, Jozef and Romain), now we can start going forward.
John already worked on a doc here : https://github.com/johnament/cdi/blob/CDI-26-docs/spec/src/main/doc/cdi-s...
The points I see to discuss here are :
- New API/SPI to introduce vs enhance exiting one (using CDIProvider vs CDIContainerLoader)
- Exposition of BeanManager in the SE boot API vs CDI
- Container shutdown
Your other points are welcome.
Antoine
9 years, 10 months
[JBoss JIRA] (CDI-514) FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
by Jozef Hartinger (JIRA)
[ https://issues.jboss.org/browse/CDI-514?page=com.atlassian.jira.plugin.sy... ]
Jozef Hartinger commented on CDI-514:
-------------------------------------
Right, for CDI 2.0 this will need to be revisited. As for the performance impact I don't really buy this. How much time does it take to do a instanceof or class identity check? Especially given that method is usually called with 0, 1 or 2 qualifiers? We should prefer correctness over dubious performance optimizations.
> FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
> --------------------------------------------------------------
>
> Key: CDI-514
> URL: https://issues.jboss.org/browse/CDI-514
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> testDuplicateBindingsToFireEventFails() tests for 2 Lifted literals with different values. But this is perfectly fine as value is NOT annotated as @Nonbinding. Thus the 2 literals are NOT equals according to CDI rules. They are essentially 2 different annotations...
> Plz remove this test. It also makes no sense to add all those very performance costly tests at runtime. The worst case which can happen is that the 2 annotations make no sense. But they don't break anything. Wheras checking all the nasty conditions each and every time is really mad from a performance aspect.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
[JBoss JIRA] (CDI-514) FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-514?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-514:
-----------------------------------
well thats not true with Java8 anymore...
Also testing this at runtime just doesn't help and just makes the container slower...
> FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
> --------------------------------------------------------------
>
> Key: CDI-514
> URL: https://issues.jboss.org/browse/CDI-514
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> testDuplicateBindingsToFireEventFails() tests for 2 Lifted literals with different values. But this is perfectly fine as value is NOT annotated as @Nonbinding. Thus the 2 literals are NOT equals according to CDI rules. They are essentially 2 different annotations...
> Plz remove this test. It also makes no sense to add all those very performance costly tests at runtime. The worst case which can happen is that the 2 annotations make no sense. But they don't break anything. Wheras checking all the nasty conditions each and every time is really mad from a performance aspect.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
[JBoss JIRA] (CDI-514) FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
by Jozef Hartinger (JIRA)
[ https://issues.jboss.org/browse/CDI-514?page=com.atlassian.jira.plugin.sy... ]
Jozef Hartinger commented on CDI-514:
-------------------------------------
Right, the spec is very clear about this and the TCK follows that.
As for the spec being "broken" it seems that the spec just enforces that the SPI usage is consistent with what the language allows regarding annotations (at most one annotation of a type on a given location).
> FireEventTest#testDuplicateBindingsToFireEventFails() is wrong
> --------------------------------------------------------------
>
> Key: CDI-514
> URL: https://issues.jboss.org/browse/CDI-514
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> testDuplicateBindingsToFireEventFails() tests for 2 Lifted literals with different values. But this is perfectly fine as value is NOT annotated as @Nonbinding. Thus the 2 literals are NOT equals according to CDI rules. They are essentially 2 different annotations...
> Plz remove this test. It also makes no sense to add all those very performance costly tests at runtime. The worst case which can happen is that the 2 annotations make no sense. But they don't break anything. Wheras checking all the nasty conditions each and every time is really mad from a performance aspect.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
[JBoss JIRA] (CDI-513) Clarify whether passivating pseudo-scopes are valid
by Sven Linstaedt (JIRA)
[ https://issues.jboss.org/browse/CDI-513?page=com.atlassian.jira.plugin.sy... ]
Sven Linstaedt edited comment on CDI-513 at 3/4/15 3:33 AM:
------------------------------------------------------------
...and having a PassivationCapable implementing bean returned by the container, which might be used by custom pseudo scopes.
was (Author: tzwoenn):
...and having a PassivationCapable implementing returned by the container, which might be used by custom pseudo scopes.
> Clarify whether passivating pseudo-scopes are valid
> ---------------------------------------------------
>
> Key: CDI-513
> URL: https://issues.jboss.org/browse/CDI-513
> Project: CDI Specification Issues
> Issue Type: Clarification
> Affects Versions: 1.2.Final
> Reporter: Mark Struberg
>
> On behalf of Jozef I copied this to a CDI ticket... See CDITCK-466
> I personally think it is clear as there is no single word which forbids this and there is a very vocal description about the single flags.
> -----
> AddingPassivatingScopeTest is illegal as addScope for passivating non-normalscopes is perfectly fine.
> There is nothing in the spec which says that a non-normalscope cannot be passivating.
> The practical use case for this is e.g. when bridging over to Spring. Those beans might need to get checked for Serializable BUT spring brings it's own proxies. So we do not need to wrap it into just another normalscoping proxy.
> Actually the test should come in 2 flavours:
> 1.) RomanEmpire being Serializable -> all fine must work
> 2.) RomainEmpire not Serializable -> DefinitionException
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 10 months
