Re: [weld-dev] Validating for fields that are annotated with both EJB and Inject

2020-04-15 Thread Emily Jiang 
Matej,

Since @Inject can only work with a qualifier annotation not anything other
annotations, do you think adding a validation criteria to assert that is
acceptable?
For IDE validation, it is quite limited because a lot of work needs to be
done via runtime to figure out whether the 2nd annotation is a CDI
qualifier or not.

Thanks
Emily

On Tue, Apr 14, 2020 at 3:28 PM Benjamin Confino 
wrote:

> Hello weld
>
> I had a customer with an issue that I believe occurred because they
> annotated a field with both @Inject and @EJB. This has given me two
> questions:
>
> 1) Should weld throw an error when a field is annotated with both
> annotations?
>
> 2) If so is the correct way to add something like
>
>  if (ij.getAnnotated().isAnnotationPresent(Inject.class) &&
> ij.getAnnotated().isAnnotationPresent(EJB.class)) {
> throw ...
> }
>
> to Validator.validateInjectionPointForDefinitionErrors()
>
> If so I'm willing to create a pull request, all I'd need is for you to
> tell me what the error message should say and please point me to which test
> I should expand to cover this case.
>
> Regards
> Benjamin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/weld-dev



-- 
Thanks
Emily
___
weld-dev mailing list
weld-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-dev

Re: [weld-dev] Validating for fields that are annotated with both EJB and Inject

2020-04-15 Thread Martin Kouba 
Dne 15. 04. 20 v 12:42 Emily Jiang napsal(a):
> Matej,
> 
> Since @Inject can only work with a qualifier annotation not anything 
> other annotations, 

That's not correct. You can add any annotation to an injection point. 
I've seen it several times where a framework inspected the 
InjectionPoint to find certain annotations that have no meaning in CDI.


> do you think adding a validation criteria to assert 
> that is acceptable?
> For IDE validation, it is quite limited because a lot of work needs to 
> be done via runtime to figure out whether the 2nd annotation is a CDI 
> qualifier or not.
> 
> Thanks
> Emily
> 
> On Tue, Apr 14, 2020 at 3:28 PM Benjamin Confino  > wrote:
> 
> Hello weld
> 
> I had a customer with an issue that I believe occurred because they
> annotated a field with both @Inject and @EJB. This has given me two
> questions:
> 
> 1) Should weld throw an error when a field is annotated with both
> annotations?
> 
> 2) If so is the correct way to add something like
> 
>   if (ij.getAnnotated().isAnnotationPresent(Inject.class) &&
> ij.getAnnotated().isAnnotationPresent(EJB.class)) {
>      throw ...
> }
> 
> to Validator.validateInjectionPointForDefinitionErrors()
> 
> If so I'm willing to create a pull request, all I'd need is for you
> to tell me what the error message should say and please point me to
> which test I should expand to cover this case.
> 
> Regards
> Benjamin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with
> number 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6 3AU
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org 
> https://lists.jboss.org/mailman/listinfo/weld-dev
> 
> 
> 
> -- 
> Thanks
> Emily
> 
> 
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/weld-dev
> 

-- 
Martin Kouba
Senior Software Engineer
Red Hat, Czech Republic


___
weld-dev mailing list
weld-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-dev

Re: [weld-dev] Validating for fields that are annotated with both EJB and Inject

2020-04-15 Thread Matej Novotny 
Hello,

what you are describing looks like a user error. Basically, for a given 
scenario it can be uncertain who's handling injection and I can imagine some 
cases where there can be race between then two frameworks.

Side note - Validator doesn't have EJB dependency and we don't want to add it 
anywhere outside of EJB module. We would have to use WeldEjbValidator instead.

However, looking at specs, I didn't find any note on this so basically it is 
undefined and I don't think we should validate it because:
1) it is a definition error that can be caught even by IDE (= before app runs)
2) we do not know how EJB subsystem reacts to similar situation, different 
servers might have different solutions in place already
3) I would say this is a rare case and one where you usually realize what's 
wrong pretty fast unless you just keep slamming multiple annotations you don't 
understand onto fields ;-)

Similar scenario would emerge with Resource annotation or with 
com.google.inject.

Regards
Matej

- Original Message -
> From: "Benjamin Confino" 
> To: weld-dev@lists.jboss.org
> Sent: Tuesday, April 14, 2020 4:27:05 PM
> Subject: [weld-dev] Validating for fields that are annotated with both EJB
> and Inject
> 
> Hello weld
> 
> I had a customer with an issue that I believe occurred because they annotated
> a field with both @Inject and @EJB. This has given me two questions:
> 
> 1) Should weld throw an error when a field is annotated with both
> annotations?
> 
> 2) If so is the correct way to add something like
> 
> if (ij.getAnnotated().isAnnotationPresent(Inject.class) &&
> ij.getAnnotated().isAnnotationPresent(EJB.class)) {
> throw ...
> }
> 
> to Validator.validateInjectionPointForDefinitionErrors()
> 
> If so I'm willing to create a pull request, all I'd need is for you to tell
> me what the error message should say and please point me to which test I
> should expand to cover this case.
> 
> Regards
> Benjamin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> 
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/weld-dev
___
weld-dev mailing list
weld-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-dev