[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný updated an issue Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Matěj Novotný Affects Version/s: 4.0.0.Alpha2 Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Karl von Randow PRs are merged for both, 3.1 and master, thanks for reporting this and contributing! Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný updated WELD-2628 Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Matěj Novotný Status: Pull Request Sent Resolved Resolution: Done Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný updated an issue Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Matěj Novotný Git Pull Request: https://github.com/weld/core/pull/1999 , https://github.com/weld/core/pull/2002 Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný updated an issue Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Matěj Novotný Git Pull Request: https://github.com/weld/core/pull/1999 Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Issue was automatically transitioned when Karl von Randow created pull request #1999 in GitHub Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Karl von Randow Status: Open Pull Request Sent Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný updated an issue Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Change By: Matěj Novotný Fix Version/s: 3.1.5.Final Fix Version/s: 4.0.0.Alpha3 Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority I believe I've found the issue. It's the Integer.MIN_VALUE priorities on those two classes. Haha, nice, you got me there. I certainly didn't expect that I think I never saw a negative priority usage within CDI in the first place, but the spec doesn't seem to forbid it so this is a bug we need to fix. Karl von Randow would you be interested in submitting a PR containing a fix + test or should I do it? Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title
Karl von Randow edited a comment on WELD-2628
Re: AfterTypeDiscovery added alternative gets ineffective priority
[~manovotn] thanks again, especially as it seems there might be is something odd with my setup.I can _confirm_ that I do see the {{getAlternatives}} list containing two classes at the end that have {{Integer.MIN_VALUE}} priority, and others earlier in the list with {{100}} and {{200}}.I believe I've found the issue. It's the {{Integer.MIN_VALUE}} priorities on those two classes. Let me try to excuse myself by saying that those priorities are actually for JAX-RS (as the classes are Filters), and I'm using CXF-CDI so my classes are {{@Dependent}} but use {{@Priority}} for JAX-RS and aren't actually alternatives.The {{compareTo}} function in {{org.jboss.weld.bootstrap.enablement.Item}} uses {{return p1 - p2;}} as the return value, which hits an integer overflow when {{p2 == Integer.MIN_VALUE}}.I'm not clear on whether (large) negative priorities are legal in the CDI spec. They're certainly possible! Perhaps it could be considered a fix to change the {{compareTo}} method to avoid integer overflow with a few comparisons?
Add Comment
This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c)
___
weld-issues mailing list
weld-issues@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Karl von Randow commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Matěj Novotný thanks again, especially as it seems there might be something odd with my setup. I can confirm that I do see the getAlternatives list containing two classes at the end that have Integer.MIN_VALUE priority, and others earlier in the list with 100 and 200. I believe I've found the issue. It's the Integer.MIN_VALUE priorities on those two classes. Let me try to excuse myself by saying that those priorities are actually for JAX-RS (as the classes are Filters), and I'm using CXF-CDI so my classes are @Dependent but use @Priority for JAX-RS and aren't actually alternatives. The compareTo function in org.jboss.weld.bootstrap.enablement.Item uses return p1 - p2; as the return value, which hits an integer overflow when p2 == Integer.MIN_VALUE. I'm not clear on whether (large) negative priorities are legal in the CDI spec. They're certainly possible! Perhaps it could be considered a fix to change the compareTo method to avoid integer overflow with a few comparisons? Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Karl von Randow hmm, that sounds fishy, there are tests in the TCK that assert the ordering in this case (and Weld passes TCKs). For instance this test which uses 3 differently prioritized alternatives and asserts ordering. I will look at what's involved to create a test case demonstrating this... That would be great. Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Karl von Randow commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Matěj Novotný thank you very much for your quick and thorough replies. The list of alternatives accessed is not sorted by priority in Weld 3.1.4 in the case I have observed. Therefore I think Weld has a bug and does not sort by priority in ascending order. I will look at what's involved to create a test case demonstrating this... Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c) ___ weld-issues mailing list weld-issues@lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-issues
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Karl von Randow Staring a bit longer into the spec, there is the following statement about the collection of alternatives you are getting: getAlternatives() returns the ordered list of enabled alternatives for the application, sorted by priority in ascending order. Alternatives enabled for a bean archive are not included in the list. This ultimately means that the highest priority that exists in the list is the rightmost element. Hence anything you add as a next element into the list will become rightmost and will have the priority of the previously highest value plus some constant (in this case it should be +10). So this new element inevitably becomes the one with the highest priority in the whole list. So taking what you said: If prior to AfterTypeDiscovery the alternatives list already contains an alternative implementation of FooService with a priority (say 100), but it's not the last item in the alternatives list, then an Extension adds another alternative of FooService, that new alternative will get a priority 10 greater than the priority of the last alternative in the list, but not necessarily greater than 100, and therefore will not be chosen. In this case, if there is FooService with 100 and it is not last item in the list, then any last item has to have more then 100 priority and anything you add behind that will have even more than that and certainly more then 100. With this in mind, I am failing to see the issue you are having, could you maybe provide a reproducer or a more in depth description of what you are seeing? Add Comment This message was sent by Atlassian Jira (v7.13.8#713008-sha1:1606a5c)
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Matěj Novotný commented on WELD-2628 Re: AfterTypeDiscovery added alternative gets ineffective priority Hi, so you're talking about directly adding elements into the collection returned by AfterTypeDiscovery#getAlternatives(), right? CDI-437 is related, this area of spec is pretty underspecificied. May I ask what's your use case for this? It's just my curiosity really, I always thought it's way easier and actually well specified to use annotation transformation to enable alternatives in given order (if you have to do it programatically). I believe the order of items in the list of alternatives should be used to choose the alternative implementation. As stated in CDI-437, this is going to be against spec (no ambig. resolution exception or equal priorities) which is something we don't want. Especially if there are TCKs for it - which I am not sure is the case. Perhaps that means a renumbering of priorities on all alternatives is necessary (for when someone goes Integer.MAX_VALUE). If someone uses Integer.MAX_VALUE it doesn't seem to be meant to override it with higher value, I'd be careful there. Maybe in this edge cases we could assign the same value and then blow up with ambiguous resolution. Other than that, you shouldn't need to reshuffle the priority numbers. Perhaps the EnablementListView should consider the maximum priority of all entries in the alternatives list when added a new alternative. Hmm, this could easily catapult you to all alternatives having Integer.MAX_VALUE as soon as one has it. This probably depends on what use case should this scenario cover - if it is enabling alternatives for beans that have no other alternatives in place (which I think was the original idea judging by the design) then the number doesn't really matter so long as it's enabled. The problematic part is when there is some other alternative already in place in which case it is a best guess - you could say that all alternatives enabled this way are prioritized (for instance by having Integer.MAX_VALUE), but that kills the ordering. Personally, I'd rather see the specification change this and instead of giving users List>, it could use an abstraction which holds a reference to the class and the priority and won't let you add classes without stating their priority. But that's unlikely to happen due to backwards compatibility anyway... Martin Kouba what are your thoughts on this?
[weld-issues] [JBoss JIRA] (WELD-2628) AfterTypeDiscovery added alternative gets ineffective priority
Title: Message Title Karl von Randow created an issue Weld / WELD-2628 AfterTypeDiscovery added alternative gets ineffective priority Issue Type: Bug Affects Versions: 3.1.4.Final Assignee: Unassigned Components: Extensions Created: 29/May/20 3:04 PM Priority: Minor Reporter: Karl von Randow When an Extension adds an alternative in AfterTypeDiscovery, the alternative gets assigned a priority by org.jboss.weld.bootstrap.enablement.EnablementListView#getPriority based upon the previous item in the alternatives list (it adds 10). Note that it uses the previous one item only, so it assigns a priority greater than the currently last item, not greater than the priority of any other alternatives. If prior to AfterTypeDiscovery the alternatives list already contains an alternative implementation of FooService with a priority (say 100), but it's not the last item in the alternatives list, then an Extension adds another alternative of FooService, that new alternative will get a priority 10 greater than the priority of the last alternative in the list, but not necessarily greater than 100, and therefore will not be chosen. I believe the order of items in the list of alternatives should be used to choose the alternative implementation. Perhaps the EnablementListView should consider the maximum priority of all entries in the alternatives list when added a new alternative. Perhaps that means a renumbering of priorities on all alternatives is necessary (for when someone goes Integer.MAX_VALUE). I am happy to contribute a patch if I'm on the right track. This may be related to CDI-437.
