Re: [jsr-314-open-mirror] [jsr-314-open] composite components: targets attribute revisited

Hi guys, I added a prototype patch for this problem to the spec issue at [1]. The patch adds the following tags: - cc:implementsActionSource - cc:implementsEditableValueHolder - cc:implementsValueHolder - cc:insertClientBehavior In addition, the patch contains a systest for cc:implementsActionSource and cc:implementsEditableValueHolder. Everything works great and it really felt good writing the tests with these new tags. One thing did not work though: f:ajax. It seems that it has some hack for composite components which uses the targets attribute of cc:clientBehavior. Did not jump into deep here, but "normal" client behaviors are working. @David: OK, sorry I misunderstood your mail. Please take a look at the patch and also the usage examples in the systest. IMHO "implements" (or "insert" in the case of clientBehavior) fits really, really well here, because it tells us that the surrounding button implements the action source with the name XXX of the composite component. And yes, multiple action source implementations are possible. However we most certainly can discuss this ;) Frankly, it would be very, very great to have this in JSF 2.1. Regards, Jakob [1] https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=901 2010/10/31 David Geary <clarity.training(a)gmail.com&gt;: > 2010/10/30 Jakob Korherr <jakob.korherr(a)gmail.com&gt; >> >> Hi, >> >> Andy, I totally agree with you in all points mentioned in your >> previous mail (actually this just made my day). > > So do I. Perhaps I wasn't clear, but I agree that the problems outlined by > Andy need to be addressed. I also understand, and agree with, the fact that > targets is an OO misdemeanor, with an interface depending upon > implementation details. > I'm just not crazy about the proposed solution. Consider the following > example from earlier in this thread: > <composite:interface> > <composite:actionSource name="loginButton"/> > </composite:interface> > > <composite:implementation> > <h:commandButton> > <composite:implementsActionSource name="loginButton"/> > </h:commandButton> > </composite:implementation> > In the preceding markup, the button doesn't implement the named action > source, as advertised, because the action source is not an action source at > all, but rather a reference to one -- and it's the action source that the > button already implements! That bit of circular illogic hurts my head. It's > also a lot of typing. > And then there's the problem of using the same attribute name ("name") for > two different things in two closely related tags. At the very least, > shouldn't the "name" attribute of <composite:implementsActionSource> be > something else, maybe "source", for example? > Also, if we're going to use "implements", why must we have separate tags for > each type of source? Why not just <composite:implements > name="loginButton"/>? Or perhaps something more semantically correct: > <composite:interface> > <composite:actionSource name="loginButton"/> > </composite:interface> > > <composite:implementation> > <h:commandButton> > <composite:references source="loginButton" (or perhaps refersTo instead > of references) > </h:commandButton> > </composite:implementation> > Is it just possible to do this with component ids, and forgo the extra tag > altogether? > <composite:interface> > <composite:actionSource name="loginButton"/> > </composite:interface> > > <composite:implementation> > <h:commandButton id="loginButton"> > </h:commandButton> > </composite:implementation> > Finally, perhaps I missed this in the thread, but does this solution let a > composite component author bind multiple targets (sorry) to a single source? > Again, I agree with the problems that Andy so eloquently described. I'd just > like to see us take a little more time to think through the solution. > > david > >> >> And I most liked the >> following paragraph: >> >> >Seriously?? After reading this, is it any wonder that our users are >> > flailing on this? If anything in this very extended email discussion reeks >> > of arcane high priestdom-ness, this is it. Wouldn't life have been simpler >> > for our users if <composite:attribute> had no "targets" attribute >> > whatsoever, no special-casing for the "name" attribute and instead we simply >> > boiled this down to: "use #{cc.attrs} for everything"? >> >> IMHO: Yes, totally! >> >> The current solution really is internally inconsistent, and >> furthermore many developers (and as you mentioned: even EG members) >> are having trouble with it. I myself implemented a lot of this for >> MyFaces, but I still have problems when using it without looking at my >> cheat-sheet... >> >> And it's not just the inconsistency: the ID-reference approach really >> is something we should stay away from, because no-one actually knows >> which ID to use: full clientid? last part of the clientid? what about >> naming containers? or even worse: what about data tables or >> <ui:repeat> (because they add an index to the clientId)? Doing a >> back-reference to the interface section here seems just soooo much >> better. >> >> David, although I know you don't really like the OO reference here, >> but IMO the target approach is even worse than this: >> >> public interface MyInterface implemented-by Foo, Bar, Bas {} >> >> ... because even in this scenario you definitely would know the >> package of Foo, Bar and Bas (or in other words: you would know which >> client-id to use). >> >> However, I always know how to insert a facet into a composite >> component, because this approach is very intuitive. Thus I think the >> right thing to do is to add those tags for actionSource, valueHolder, >> editableValueHolder and clientBehavior too and to "get rid" of the >> targets-solution. >> >> In the case of actionSource, valueHolder and editableValueHolder I >> really like "implementsXXX", because it shows that the certain >> implementation component implements this functionality for/of the >> composite component. However in the case of clientBehavior, I think we >> should go with cc:insertClientBehavior, just as we did with >> cc:insertFacet, because facets and clientBehaviors are somehow related >> here.. >> >> To wrap this up again, I'd propose to add these tags for JSF 2.1 in >> order to make the life of our users a lot easier: >> >> <cc:implementsActionSource name="XXX" /> >> <cc:implementsEditableValueHolder name="XXX" /> >> <cc:implementsValueHolder name="XXX" /> >> <cc:insertClientBehavior name="XXX" /> >> >> Regards, >> Jakob >> >> >> >> 2010/10/30 Andy Schwartz <andy.schwartz(a)oracle.com&gt;: >> > On 10/29/10 11:06 PM, David Geary wrote: >> > >> > It seems that I am perhaps the only dissenting voice here, but I don't >> > care >> > for this solution. >> > To those of us that understand the rationale for removing targets and >> > adding >> > these cc:implements... tags, the new tags make perfect sense. But to the >> > uninitiated, they will be bewildering. What does it mean for a button to >> > "implements action source"? Buttons already implement action source. >> > >> > Ed also raised this concern. If we are going to continue on with this >> > approach (and I hope that we do), perhaps we need to come up with less >> > ambiguous names for these tags. >> > >> > >> > IMO, targets are much easier to understand, and to explain. >> > I understand the urge to remove the targets attribute based on their OO >> > impurity, >> > >> > OO purity may be part of it, though I believe that the a bigger reason >> > for >> > focusing on this area is that end users (and even EG members) seem to be >> > having difficulty with our existing APIs. >> > >> > but I think the solution could use some more thought. There are already >> > too >> > many arcane oddities in JSF, whose rationale is only intuited by high >> > priests, and I hate to see us adding more. >> > >> > The "high priest" analogy does not present an especially charitable view >> > of >> > what is happening here. We released an API in JSF 2.0 that users are >> > finding problematic. Expert group/community members have recognized >> > that >> > this is a problem and are attempting to devise a solution that would be >> > more >> > in line with how our users expect this to work. Perhaps we are off base >> > in >> > aspects of this solution, but the fact that we are looking to address a >> > problem area is a good thing. I don't think it is fair to apply the old >> > high priest/ivory tower motif here. >> > >> > My own take is that a big reason why our end users are having difficulty >> > with our current solution is that it is internally inconsistent. In >> > some >> > cases implementation tags refer to interface metadata (eg. >> > <composite:insertFacet>). In other cases interface metadata refers to >> > implementation tags (eg. <composite:actionSource>). In one case, that >> > in >> > retrospect seems embarrassingly convoluted, we have bi-directional >> > references between interface and implementation: <composite:attribute>, >> > which sometimes uses "targets" and other times is referenced via >> > #{cc.attrs}. >> > >> > Looking at the tag documentation for the <composite:attribute> "targets" >> > attribute: >> > >> > If this element has a method-signature attribute, the value of the >> > targets >> > attribute must be interpreted as a space (not tab) separated list of >> > client >> > ids (relative to the top level component) of components within the >> > <composite:implementation> section. Space is used as the delimiter for >> > compatibility with the IDREFS and NMTOKENS data types from the XML >> > Schema. >> > Each entry in the list must be interpreted as the id of an inner >> > component >> > to which the MethodExpression from the composite component tag in the >> > using >> > page must be applied. If this element has a method-signature attribute, >> > but >> > no targets attribute, the value of the name attribute is used as the >> > single >> > entry in the list. If the value of the name attribute is not one of the >> > special values listed in the description of the name attribute, targets >> > (or >> > its derived value) need not correspond to the id of an inner component. >> > >> > The "name" attribute documentation isn't especially reassuring: >> > >> > The name of the attribute as it must appear on the composite component >> > tag >> > in the using page. If the value of the name attribute is equal to >> > (without >> > the quotes) “action”, “actionListener”, “validator”, or >> > “valueChangeListener”, the action described in >> > ViewHandler.retargetMethodExpressions() must be taken to handle the >> > attribute. In these cases, the method-signature attribute, if present, >> > must >> > be ignored as its value is derived as described in >> > retargetMethodExpressions(). >> > >> > Seriously?? After reading this, is it any wonder that our users are >> > flailing on this? If anything in this very extended email discussion >> > reeks >> > of arcane high priestdom-ness, this is it. Wouldn't life have been >> > simpler >> > for our users if <composite:attribute> had no "targets" attribute >> > whatsoever, no special-casing for the "name" attribute and instead we >> > simply >> > boiled this down to: "use #{cc.attrs} for everything"? >> > >> > I realize that the <composite:attribute> use case is a bit different >> > from >> > >> > <composite:actionSource>/<composite:valueHolder>/<composite:editableValueHolder> >> > cases, so perhaps it would be sufficient to address the >> > <composite:attribute> insanity and stop there. However, I do think it >> > is >> > worthwhile to look at the full solution and see whether there is room >> > for >> > additional simplification/consistency. >> > >> > To me, <composite:actionSource> is not all that different from >> > <composite:facet>. The <composite:facet> API, like >> > <composite:actionSource>, could have just as easily used a target >> > approach. >> > Yet we decided to go with this: >> > >> > >> > <composite:interface> >> > <composite:facet name="foo"/> >> > <composite:interface> >> > >> > <composite:implementation> >> > <bar:someComp> >> > <composite:insertFacet name="foo"/> >> > </bar:someComp> >> > </composite:implementation> >> > >> > Instead of: >> > >> > >> > <composite:interface> >> > <composite:facet name="foo" target="mySomeComp"/> >> > <composite:interface> >> > >> > <composite:implementation> >> > <bar:someComp id="mySomeComp"/> >> > </composite:implementation> >> > >> > >> > Why did we go with <composite:insertFacet> instead of "target"? Which >> > approach is easier for users? If we had provided both of these >> > approaches >> > in 2.0, which would users prefer? >> > >> > In my experience I have found our id-referencing APIs to be a difficult >> > to >> > work with. Id references can be confusing. It isn't always clear >> > whether >> > relative or full ids should be used or what location relative ids are >> > relative to. The presence of naming containers influence the >> > specification >> > of id references in ways that can trip up unsuspecting users. Id >> > references >> > are fragile and can easily go stale when refactoring content. >> > >> > I believe that we had the right idea when we went with the >> > <composite:facet>/<composite:insertFacet> design. This solution >> > provides a >> > simple, clear way for the user to express their intentions without >> > suffering >> > any of the above drawbacks. >> > >> > While I admit that the <composite:actionSource> case is not identical to >> > <composite:facet>, the similarities seem strong. If JSF 2.0 had >> > included a >> > >> > <composite:actionSource>/<composite:insert/implements/exposes/whateverActionSource> >> > pair (like <composite:facet>/<composite:insertFacet>) and had not >> > included >> > any notion of "targets", I don't think that our users would have had >> > much >> > difficulty coping with this. If we had exclusively gone with a >> > "implementation references interface" approach (no references from >> > interface >> > to implementation), I think our users would have been altogether better >> > off. >> > >> > Of course, we cannot go back and time and undo what we've started in JSF >> > 2.0. It is less important for us to question whether our original >> > design >> > choices were correct than to decide whether it makes sense to introduce >> > new >> > APIs now. In my mind, this looks like the proposals that we have been >> > discussion are a step in the right direction. I like the consistency of >> > this new approach; it allows users to interact with all of our interface >> > metadata in a similar manner. I am certainly open to discussion on >> > this, >> > and I completely agree that we need to be careful when pondering new >> > APIs. >> > You are right that it is critical that we avoid unintentionally making >> > life >> > more difficult for our users, not just in this case, but for all of our >> > API >> > decisions. >> > >> > Andy >> > >> > >> >> >> >> -- >> Jakob Korherr >> >> blog: http://www.jakobk.com >> twitter: http://twitter.com/jakobkorherr >> work: http://www.irian.at > > -- Jakob Korherr blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at