[seam-dev] Need for Common Annotation-attribute scanning/parsing utility (or CDI API/SPI)

[Dan Allen]

On Wed, Apr 14, 2010 at 8:34 AM, Pete Muir <pmuir at redhat.com> wrote:

> I think we need to take a step back and establish a couple of things:
>
> 1) are all the annotations we have suitable to be meta-annotations?
> 2) should it be possible to use meta-annotations anywhere, or just in
> certain places (on stereotypes)
> 3) should these annotations be usable via seam-xml?
>
> IMO the answers are
>
> 1) No. Not all annotations are suitable as meta-annotations. This is a
> principle clearly established by the CDI spec (which doesn't allow
> qualifiers etc. as meta-annotations). We should consider whether each
> annotation is suitable to be a meta-annotation or not.
>

+1 Constraint breed consistency.


> 2) In general no, only on stereotypes. There are a few reasons. In general,
> stereotypes were designed for exactly the purpose of stereotyping an action.
> The overhead on the user is minimal (I believe the concern that users will
> get confused by their annotation not working can be solved by tooling).
> Finally, if stereotypes are reused, the CDI container is capable of caching
> the recursive scan of the annotation hierarchy (which no extension can do as
> it doesn't know the lifecycle of the underlying class), and also applying
> any metadata specified through the CDI SPI (e.g. XML config) to the
> stereotype (something which is not required in other cases). This means the
> complexity of the checking the annotations is reduced to N (rather than the
> N^N we get with a recursive scan).
>
> IOW, taking the JMS case, it would be necessary to specify:
>
> @Stereotype
> @Retention(RUNTIME)
> @Target({ PARAMETER, METHOD, FIELD })
> @JmsDestination(jndiName = "/jms/my-topic")
> public @interface MyTopic {}
>

I think that's very fair. It certainly helps clarify the purpose.


>
> 3) Yes. This reduces the potential for user confusion, and makes it much
> more powerful.
>
> So, I would propose we create a new API in WeldExtensions that looks like:
>
>   /**
>    * Discover if a AnnotatedElement <b>element</b> has been annotated with
> <b>annotationType</b>. This
>    * also discovers annotations defined through a @{@link Stereotype} and
> the CDI SPI.
>    *
>    * @param element The element to inspect.
>    * @param annotationType
>    * @param metaAnnotation Whether the annotation may be used as a
> meta-annotation or not
>    *
>    * @return true if annotation is present either on the method itself.
> Returns false if the annotation
>    * is not present
>    * @throws IllegalArgumentException if element or annotationType is null
>    */
>   public static boolean isAnnotationPresent(AnnotatedElement element,
> Class<? extends Annotation> annotationType, boolean metaAnnotation);
>
>   /**
>    * Inspect AnnoatedElement <b>element</b> for a specific <b>type</b> of
> annotation. This
>    * also discovers annotations defined through a @ {@link Stereotype} and
> the CDI SPI.
>    *
>    * @param element The element to inspect
>    * @param annotationType The annotation type to check for
>    * @param metaAnnotation Whether the annotation may be used as a
> meta-annotation or not
>    *
>    * @return The annotation instance found on this method or null if no
> matching annotation was found.
>    * @throws IllegalArgumentException if element or annotationType is null
>    */
>   public static <A extends Annotation> A getAnnotation(AnnotatedElement
> element, final Class<A> annotationType, boolean metaAnnotation)
>
> This is somewhat different to Lincoln's original proposal (adjusted to add
> the meta-annotation stuff, aligned with the Java Reflection API naming
> conventions, removed the stuff about whether the annotation is on the
> declaring class as well - if this is really needed we can add a boolean flag
> for this I guess).
>
> Adam also raised the need to be able to inspect the annotations on an
> annotation element, which indicates we need another utility method:
>
> /**
>    * Inspect AnnoatedElement <b>element</b> for a specific <b>type</b> of
> annotation. This
>    * also discovers annotations defined through a @ {@link Stereotype} and
> the CDI SPI. Having found
>    * the annotation, it is inspected for a member with name
> <b>memberName</b>.
>    *
>    * @param element The element to inspect
>    * @param annotationType The annotation type to check for
>    * @param memberName The name of the member to look for
>    * @param expectedMemberType The expectedType of the member
>    * @param metaAnnotation Whether the annotation may be used as a
> meta-annotation or not
>    *
>    * @return The annotation member's value or null if no matching
> annotation was found
>    * @throws IllegalArgumentException if element, annotationType,
> memberName or expectedMemberType is null
>    * @throws IllegalArgumentException if the annotationType does not have a
> member of memberName
>    * @throws ClassCastException if the value of memberName cannot be cast
> to expectedMemberType
>    */
>   public static <V> V getAnnotationMemberValue(AnnotatedElement element,
> final Class<A> annotationType, String memberName, Class<V>
> expectedMemberType, boolean metaAnnotation)
>
> WDYT?


I'm not sure I understand why we need a separate method to get an annotation
member value. It requires specifying a string name. Why can't we simply
return the annotation instance and then allow the value to be read by
invoking the member method?

Consider

public @interface Begin {
   String id default "";
}

annotations.get(annotatedElement, Begin.class, true).id();

I'll admit I was excited about the more fluent method names that Lincoln had
in his code. Do we really need to have these long method names? Seems like
pomp and circumstance to me. I suppose that this is what Java has always
been about, so why take the garb off the queen? Boo.

-Dan

-- 
Dan Allen
Senior Software Engineer, Red Hat | Author of Seam in Action
Registered Linux User #231597

http://mojavelinux.com
http://mojavelinux.com/seaminaction
http://www.google.com/profiles/dan.j.allen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/seam-dev/attachments/20100414/caef0bd2/attachment-0001.html