jsr344-experts@javaserverfaces-spec-public.java.net

[jsr344-experts] Re: [jsr-344-experts] should really a composite component requires NamingContainer interface?

From: Blake Sullivan <blake.sullivan_at_oracle.com>
Date: Fri, 08 Jul 2011 09:35:01 -0700

On 7/7/11 4:55 PM, Leonardo Uribe wrote:
> Hi
>
> BS> The issue is that the cases that don't work aren't hacks. If anything,
> BS> they are the way we expect our users to use composite components. I
> BS> worry that by carving out an exception, we complicate the use of
> BS> composite components if the page author has to look at each composite
> BS> component to determine whether it happens to be a NamingContainer or not.
> BS>
>
> The case proposed is a hack, so if not all works, well that's expected.
>
> But after thinking about it, you are right about why an user has to
> worry if the composite component is or not a NamingContainer. I see it
> from other point of view. The user should not have to worry about if a
> component is NamingContainer or not. It should work like with jsf 1.1
> or 1.2. Components like h:form or h:dataTable or other special cases
> should be NamingContainer, but the remaining ones that does not
> "iterate" or don't require it by definition should not.
>
> In fact, I have an alternative idea: the one who should implement
> NamingContainer is not the composite component, instead it should be
> composite component panel, or the one that store the information
> everything inside cc:implementation under the special key
> "javax.faces.component.COMPOSITE_FACET_NAME". This could require some
> changes, but I can bet with that hack we can get rid the
> NamingContainer restriction for composite component and make it work
> for all cases.
It would be nice for it to appear to the page author that the composite
components are not NamingContainers when referencing the components that
they are passing to the composite component. However the implementation
is internally going to need a NamingContainer to allow the composite
component to be used more than once on the page if the composite
component definition contains any ids.

There is the additional issue of backwards compatibility, by shipping a
version with a leaky abstraction, we have made fixing the issue harder
as users may have code dependent on the old behavior.

There are also issues with the ids of the children passed into the
composite component definition, as they can conflict with the ids used
in the composite component definition itself. This is a trickier
problem to fix.

> BS> I agree that fixing findComponent is not trivial, however remembering
> BS> the original child locations isn't exactly rocket science either.
> BS>
> BS> Can you provide an example of where fixing findComponent would cause any
> BS> problems with invokeOnComponent or visitTree? As I wrote before, we
> BS> aren't modifying the generated clientIds for the relocated components so
> BS> I don't understand why a change to findComponent would have any effect
> BS> on clientId-based behavior.
>
> I checked it again and it is true that findComponent is "decoupled" to
> a certain level from clientId generation. There are some use cases to
> consider:
>
> 1. messages : call findComponent and then getClientId
> 2. ajax client behavior renderer : call findComponent and then getClientId
> 3. replacement for "binding" attribute: user code call
> invokeOnComponent or visitTree get the component instance directly and
> do something.
I am not particularly surprised. JSF has a terminology problem because
it doesn't have a name for the identifier that you pass to
findComponent. In Trinidad and ADF Faces, we call these scoped Ids.
There is a general problem that even at the framework level, developers
are confused regarding the difference between the two and when it is
safe to call getClientId() on a component (or in fact, to retrieve the
value of any EL-bound value on a component that is not currently in
scope). Anyway, if framework or application code is calling
findComponent() to retrieve a component instance that is not guaranteed
to be in the same component context, the behavior of calling
getClientId() on the component is undefined and always has been.

1) and 2) sound like bugs

3) In the more general case of getting values or otherwise messing with
a potential stamped instance of a component, #3 is correct as long as
the "do something" is performed inside the callback. In the
binding-replacement case, the user wants to affect all, possible stamps,
so they would only call one of the visiting methods if they happened to
already have a clientId, otherwise they would likely call findComponent().

This doesn't really have anything to do with composite components in and
of themselves, but points to general usability problems with component
and EL context. If we're confused, our users will definitely be
confused. For example, it is completely non-obvious that for EL-bound
attribute values, application developers need to use invokeOnComponent
or visitTree in order to retrieve the value.

The general rule is that all declarative markup is going to use
findComponent and it is up to us to figure out how to translate that
into the appropriate runtime behavior,

> The first two are not a problem,
Actually they are. You can't safely call findComponent() and then
getClientId() on a component in all cases.

> but 3 is a problem right now, because
> the user needs to worry about if a component is a composite component
> or not (is a NamingContainer or not), to then write the right
> expression.
That is an annoyance. The real bug is that the page author needs to
know whether his components will be inserted inside of an additional
NamingContainer inside of the composite component. This is an
encapsulation failure. Similarly, there are potential issues with the
EL of the component relocated to inside the composite component.
> I already did something related to store the child locations long time
> ago on MyFaces to solve "composite component state get lost" problem
> (see PostAddToViewEvent still inconsistent mails), but that code was
> replaced with another solution. It involves use a Listener to
> PostAddToViewEvent to store the child ids. The problem is any similar
> hack will only work on MyFaces because the current composite component
> implementation on Mojarra does not allow calls to getClientId inside
> PostAddToViewEvent listeners.
>
> In any case, it seems a change over findComponent / getClientId
> algorithm is unavoidable, but I don't have clear the details about
> which changes should be done.
>
> Leonardo Uribe
The first step is that UIComponent.findComponent() needs to specify that
it delegates to each parent to search its children (the current
implementation in Mojarra at least never delegates to any other
component). This will allow the composite component to affect the
behavior. I need to dig up the list of issues we were looking at for
the use of findComponent in the Trinidad composite components, as they
all apply to composite components in general.

-- Blake Sullivan
>> 2011/7/7 Leonardo Uribe<lu4242_at_gmail.com>:
>>> Hi
>>>
>>> This answer is from some days ago, but it wasn't sent to
>>> jsr344-experts list, so I resend it to continue the discussion on this
>>> list:
>>>
>>> Hi
>>>
>>> 2011/7/1 Blake Sullivan<blake.sullivan_at_oracle.com>:
>>>> Leonardo,
>>>>
>>>> I agree that you can get rid of the restriction in a very limited case, but
>>>> is it worthwhile to make such a change? The reason the composite components
>>>> are NamingContainers is to ensure isolation from the consuming page. Your
>>>> solution to avoiding id conflicts in this case was to not set any ids in
>>>> your composite component definition. This precludes the definition from
>>>> EVER using any id-based services, such as<f:ajax> within its definition.
>>>> In addition, your proposal does nothing to address abstraction failure
>>>> issues regarding whether a composite component implementation happens to use
>>>> NamingContainers as part of its implementation.
>>>>
>>> The intention is just allow the "exception", that theorically is
>>> working on MyFaces without change anything. It is clear there will be
>>> some hacks that just will not work, but the same is true for cases
>>> that right know depends on ui:include or facelets user tags. By
>>> default, composite components should be naming containers to work
>>> fully. Note this feature does not need any change on the spec.
>>>
>>>> I do not consider the initial findComponent() failure a bug. Page authors
>>>> using components need to know whether they are NamingContainers or not.
>>>> Good IDEs should be helping you out in this case. In many ways, it is
>>>> easier for page authors to remember whether a component is a composite
>>>> component or not if all of the composite components are NamingContainers.
>>>>
>>>> The abstraction failure is that findComponent() should always work in terms
>>>> of the document that the page author is working in. In the case of
>>>> composite components, this means that component references in the page
>>>> consuming the composite component should stay the same regardless of whether
>>>> a composite component uses NamingContainer in its implementation or not.
>>>>
>>> It sounds reasonable, but I don't see how can we do such change over
>>> findComponent() without degrade its performance. Things are more
>>> difficult if you consider nested composite components using
>>> cc:insertChildren, cc:insertFacet and templating tags. The problem is
>>> once the component tree is built, you lose the information related to
>>> the page where the component was defined. The only information saved
>>> is a marker to identify it on a further refresh (MARK_ID).
>>>
>>>> Fixing findComponent() should not break invokeOnComponent() and visitTree()
>>>> as we are not talking about changing the structure of the returned clientIds
>>>> for the child components from the consuming page after they have been
>>>> relocated into the composite component.
>>>>
>>> But an invokeOnComponent will not work too, without get the final
>>> clientId. The intention of do not use a NamingContainer deliberately
>>> is precisely to know beforehand the clientId of the component.
>>>
>>> The problem with use facelets user tags or facelets dynamic components
>>> is that all properties are strings that are put on a VariableMapper,
>>> which is a nasty hack (I already changed that terrible code in MyFaces
>>> to something better), and in practice there are some situations where
>>> JSF composite components cannot be used, specifically by the
>>> NamingContainer restriction.
>>>
>>> regards,
>>>
>>> Leonardo Uribe
>>>