Hi Andy,
in fact the implementation doesn't need setTag(). I used it because it was there. But I'm fine with simply deleting it now and enhancing the whole Tag/TagAttribute(s) thing in a later release to no longer force implementors of tag decorators to use non public API.
Ciao Frank
Am 12.03.2013 um 15:52 schrieb Andy Schwartz <andy.schwartz_at_oracle.com>:
> Gang -
>
> We have added the following two methods to TagAttribute (and similar methods to TagAttributes):
>
> Index: javax/faces/view/facelets/TagAttribute.java
> ===================================================================
> --- javax/faces/view/facelets/TagAttribute.java (revision 8845)
> +++ javax/faces/view/facelets/TagAttribute.java (revision 11719)
> @@ -64,8 +64,8 @@
>
>
> + /**
> + * <p class="changed_added_2_2">A reference to the Tag for which this class
> + * represents the attributes. For compatibility with previous implementations,
> + * an implementation is provided that returns {_at_code null}.</p>
> + *
> + * @since 2.2
> + *
> + * @return the {_at_link Tag} for which this class represents the attributes.
> + */
> + + public Tag getTag() {
> + return null;
> + }
> -
> + /**
> + * <p class="changed_added_2_2">Set a reference to the Tag for which
> + * this class represents the attributes. The VDL runtime must
> + * ensure that this method is called before any {_at_link
> + * FaceletHandler}s for this element are instantiated. For
> + * compatibility with previous implementations, a no-op
> + * implementation is provided.</p>
> + *
> + * @since 2.2
> + *
> + */
> + public void setTag(Tag tag) {
> + + }
>
> Prior to the addition of setTag(), TagAttribute exposes a read-only/immutable API. Introducing mutability into this contract seems like an undesirable change.
>
> I was wondering whether we can find some way to avoid this, eg. only expose the getter in the public API and instead pass the Tag into the impl class through some other means.
>
> However, while doing so I noticed that these methods don't seem to be needed at all
>
> The one place where Mojarra calls TagAttribute.getTag() is in the (internal) DefaultTagDecorator:
>
> > protected TagAttribute convertTagAttribute(TagAttribute attribute) {
> > Location location = attribute.getLocation();
> > String ns = attribute.getNamespace();
> > String myLocalName = attribute.getLocalName();
> > String qName;
> > String value = attribute.getValue();
> >
> > if (Namespace.jsf.uri.equals(attribute.getNamespace())) {
> > // make this a component attribute > qName = myLocalName;
> > ns = "";
> > } else {
> > if (ns.length() != 0 && !ns.equals(attribute.getTag().getNamespace(\ <--------- Here!
> > ))) {
> > // the attribute has a different namespace than the tag. preser\
> > ve it. > return attribute;
> > }
>
> The problem with this is: we've already got access to the Tag instance. convertTagAttribute() is called from convertAttributes(). This is called from convertTag(), which gets passed the Tag. Since this is all internal code in a package-private class, we can easily solve this by passing the Tag instance into convertTagAttributes().
>
> Why are we adding new public APIs rather than just refactoring this code? Unless I am missing some other use case, let's kill the new APIs and do the trivial implementation refactoring. (Same thing applies to TagAttributes.)
>
> BTW, although we should kill these new methods, they provide a good example of why adding "non-transparent" default implementations (such as returning null) to abstract classes are a bad idea. TagAttribute.getTag() javadoc states that:
>
> + * represents the attributes. For compatibility with previous implementations,
> + * an implementation is provided that returns {_at_code null}.</p>
>
> I suppose the idea is that, for those random folks who have implemented TagAttribute subclasses, they are now happy because they are not exposed to this new API requirement.
>
> However, take a look at the getTag() usage above. Is this default implementation going to help? Not really. It's simply going to lead to a much more difficult to diagnose NullPointerException.
>
> As someone who has extended just about every extension point that JSF offers (though not TagAttribute), I would much, much rather find out about this requirement at compile time. Providing a default implementation that returns null (and later NPEs!) provides an false sense of compatibility, but really just ends up hiding the requirement and wasting developer time.
>
> Andy
>