repeating annotations comments

[michael keith]

Okay. I'll give it a go and let you know.

-Mike

On 22/12/2012 1:03 AM, Joe Darcy wrote:
> Hi Mike,
> On 12/20/2012 7:48 PM, michael keith wrote:
>> Just to follow up on this, I did a quick survey and there are a 
>> number of EE 6 specs that make use of the idiomatic container 
>> pattern: EJB, JPA, JAXB, JAX-WS, JSF, Interceptors, Connectors, and 
>> Common Annotations. These are just the EE 6 specs, I didn't go 
>> through the ongoing EE 7 ones. This means that all of the 
>> implementations, IDE's, 3rd party tools and frameworks that process 
>> annotations from these specs will break, in many cases imperceptibly, 
>> if the semantics of the existing reflection methods get changed. As I 
>> mentioned earlier, while I was prepared to require that 
>> implementations change once we upgraded the specs to use repeatable 
>> annotations, I feel the fanout is too broad and the manner of 
>> breakage too subtle that the cost of abandoning compatibility is too 
>> high. If people feel there is confusion around the new methods acting 
>> differently from the old ones in the presence of repeatable 
>> annotations then perhaps we could go back to exploring the use of 
>> different names for the new ones.
>
> It would be very helpful for someone to verify that the idiomatic 
> container pattern used in the various EE specs can be successfully 
> retrofitted to use the formal container pattern in the current JDK 8 
> builds.  Several adjustments were made to the initial formal container 
> design to accommodate the actual usage patterns in EE.
>
> Thanks,
>
> -Joe
>
>>
>> -Mike
>>
>> On 17/12/2012 9:44 AM, Mike Keith wrote:
>>> Thanks for your comments, ideas and suggestions, Alex. I had more 
>>> answers but my draft got prematurely sent. More stuff below.
>>>
>>> -Mike
>>>
>>> On 2012-12-16, at 10:33 PM, Mike Keith<michael.keith at oracle.com>  
>>> wrote:
>>>
>>>> On 2012-12-14, at 8:41 PM, Alex Buckley<alex.buckley at oracle.com>  
>>>> wrote:
>>>>
>>>>> Hi Mike,
>>>>>
>>>>> Thanks for the quick feedback. (I know you've been playing with 
>>>>> the JDK8 b68 binary.)
>>>>>
>>>>> On 12/14/2012 1:59 PM, michael keith wrote:
>>>>>> 1. The getAnnotation(Class<T>) method is being changed to look 
>>>>>> through
>>>>>> container annotations. This method returns an Annotation, i.e. a 
>>>>>> single
>>>>>> instance, so applying it to repeated ones is necessarily going to 
>>>>>> have
>>>>>> to return only one. But having it look through container 
>>>>>> annotation ends
>>>>>> up not being very productive, and may be causing it to be
>>>>>> counter-productive.
>>>>>>
>>>>>> Look-through for this method is not productive because the call 
>>>>>> is being
>>>>>> made precisely in the legacy cases when only one annotation was
>>>>>> expected, but a container annotation will only be synthesized when
>>>>>> multiples are present. So look-through does not really help get 
>>>>>> "the"
>>>>>> expected annotation. The only case when it would be useful is in the
>>>>>> unlikely case when the container annotation is used explicitly and
>>>>>> contains a single contained annotation.
>>>>> I feel the author of the repeatable annotation type (who "opted 
>>>>> in") and the author of the multiple annotations (@Foo @Foo class C 
>>>>> ...) bear some responsibility when a legacy caller of 
>>>>> getAnnotation(Class<T>) sees one-out-of-many annotations. Someone 
>>>>> decided there is no more of "'the' expected annotation".
>>>> Sure, there is some responsibility to be shared by both of those 
>>>> parties.The problem is that the call succeeds at all. It should 
>>>> probably fail (eg return null) if there are multiples
>>>>
>>>>
>>>>>> It may be counter-productive for three reasons.
>>>>>> The first is because getting a random single annotation in the 
>>>>>> presence
>>>>>> of multiples is potentially dangerous since the calling code will
>>>>>> continue without knowing that multiples are present, so legacy 
>>>>>> code is
>>>>>> essentially broken, but will not actually fail. It will just be
>>>>>> semantically incorrect.
>>>>> Acknowledged.
>>>>>
>>>>>> The second is because I didn't see (and I may just have missed 
>>>>>> it) where
>>>>>> it was defined that if both T and TC were present that T would be
>>>>>> guaranteed to be returned and not some instance within TC. 
>>>>>> Example 1.2-3
>>>>>> shows this to be the case but I didn't see that it was specified
>>>>>> behavior rather than just because the T occurred before the TC. 
>>>>>> If the
>>>>>> example had been TC and then T would the instance of T be returned?
>>>>> If both @T and @TC are present, we have not yet specified [1] 
>>>>> whether get[Declared]Annotation(Class<T>) returns always the lone 
>>>>> @T v. always a containerized @T v. something different every time. 
>>>>> That's what your feedback is for :-)
>>>>>
>>>>> [1] 
>>>>> http://download.java.net/jdk8/docs/api/java/lang/reflect/AnnotatedElement.html#getAnnotation%28java.lang.Class%29
>>>>>
>>>>>> This could be important because existing code to handle this case
>>>>>> checks for the container annotation TC and then checks for the 
>>>>>> single
>>>>>> annotation T. However, if checking for T happens to look through TC
>>>>>> and return one of the contained annotations in TC instead of the 
>>>>>> lone
>>>>>> T annotation then the lone annotation would not get found.
>>>>> The implementation currently "prefers" a lone @T over a @T inside 
>>>>> @TC. If you have a declaration  @TC(...) @T class C {}  then 
>>>>> C.class.getAnnotation(T.class) will return the standalone @T. So 
>>>>> existing code to handle this case will still work.
>>>>>
>>>>>> The third is because all of the code that is currently out there 
>>>>>> that
>>>>>> needs to check for the previous case, i.e. both T and TC 
>>>>>> occurring, will
>>>>>> get a false positive in the simpler case of multiple instances of T
>>>>>> annotations (as per the second part of example 1.2-1 in the doc). 
>>>>>> The
>>>>>> getAnnotation(TC) will return the synthesized TC containing all 
>>>>>> of the
>>>>>> instances of T, and then getAnnotation(T) will return one of 
>>>>>> those same
>>>>>> instances again. Legacy code will assume that the result of
>>>>>> getAnnotation(T) is disjoint from the contained annotations of 
>>>>>> the TC
>>>>>> returned by getAnnotation(TC).
>>>>> Acknowledged.
>>>>>
>>>>>> 2. Given that get[Declared]Annotations() is going to change and 
>>>>>> return
>>>>>> multiple instances of the same annotation (rather than a single 
>>>>>> instance
>>>>>> of a container annotation) existing implementations are going to 
>>>>>> have to
>>>>>> be fixed since there is no backwards compatibility there at all.
>>>>> Acknowledged. The category of existing implementations which 
>>>>> assume there will never be more than one annotation of a given 
>>>>> type in the returned array is in for a surprise.
>>>>>
>>>>> But there is a category of existing implementations which will 
>>>>> continue to work fine. Suppose code calls 
>>>>> get[Declared]Annotations() and looks first for @T in the array; 
>>>>> only if @T is not found does the code look for @TC in the array. 
>>>>> This seems like rational behavior for a non-repeating world, as it 
>>>>> means one call to get annotations rather than 
>>>>> getAnnotation(T.class) plus getAnnotation(TC.class). Now, run the 
>>>>> code on SE 8 to get the new behavior of 
>>>>> get[Declared]Annotations(). The code will find an @T in the array 
>>>>> (the first of many, perhaps) and there will be no search for @TC 
>>>>> and everything will be fine. (Assume a top-level @T is returned 
>>>>> before containerized @T's.)
>>> I might be misunderstanding you, but I don't see how that category 
>>> of legacy application would continue to work. It still won't be 
>>> expecting multiples as it looks in the collection, so it will stop 
>>> looking once it finds the first one.
>>>
>>>>>> 3. In general, it appears that once an annotation is made 
>>>>>> repeatable the
>>>>>> annotation processing code is inevitably going to have to change in
>>>>>> order to handle the repeatability. We were willing to take that hit,
>>>>>> particularly since the new behavior requires opting in, and that 
>>>>>> means
>>>>>> that specs could do so at their own discretion. However, the code to
>>>>>> handle both the new and the old (i.e. be able to process both new
>>>>>> repeatable annotations as well as the older idiomatic container
>>>>>> annotations) is a little less intuitive to write than I had hoped it
>>>>>> would be. The easiest way might just be to create a Set of 
>>>>>> instances to
>>>>>> avoid duplicates. Not rocket science, just not what I would have 
>>>>>> expected.
>>>>> I'm not sure if you are proposing that AnnotatedElement should 
>>>>> return Set instances or your framework code should create Set 
>>>>> instances. By policy, core reflection does not depend on the rest 
>>>>> of the Java SE API, so uses array types as aggregates.
>>> No, sorry, I was just describing what the processor might have to 
>>> do, not implying that a Set would be returned by any of the 
>>> reflection calls.
>>>
>>>>> So, where are we? Joe and I have always recognized that changing 
>>>>> getAnnotation(Class<T>) and get[Declared]Annotations() to look 
>>>>> through official containers would affect legacy callers who assume 
>>>>> there is at most one @T. A legacy caller which allows for both @T 
>>>>> and @TC is perhaps hit harder than the rest - we weren't sure how 
>>>>> many legacy callers were this careful, but you imply it's 
>>>>> non-trivial.
>>> Yes, and the number of users that would actually make use of this 
>>> pattern is admittedly minuscule. Unfortunately, the processing code 
>>> generally ends up having to handle it anyway, so the pattern for 
>>> processing it is going to be widespread.
>>>
>>>>> Here's an idea. We pondered having the "existing" methods - 
>>>>> get[Declared]Annotation(Class<T>) and get[Declared]Annotations() - 
>>>>> be unaware of official containers, so they would return exactly 
>>>>> what's in the class file. Essentially this is perfect behavioral 
>>>>> compatibility. Meanwhile, the new 
>>>>> get[Declared]Annotations(Class<T>) methods would look through 
>>>>> official containers if present. How does that grab you?
>>>  From a compatibility perspective that would be great. Existing code 
>>> would be able to handle existing applications, and new code could 
>>> easily handle both old and new applications.
>>>
>>> I would just like to mention, though, that while we in EE-land would 
>>> like compatibility we are also sensitive to non-legacy users and 
>>> don't want to impose changes that too severely hamper the API or 
>>> render it unintuitive in the future. We are certainly willing to 
>>> make changes to code if necessary, but I was kind of hoping that we 
>>> wouldn't be in the situation of existing code apparently working, 
>>> but not really. I would be interested in hearing how others view the 
>>> idea of the typed version of get[Declared]Annotations behaving 
>>> differently than the legacy untyped one.
>