users@javaserverfaces-spec-public.java.net

[jsr344-experts mirror] [jsr344-experts] Re: [1080-ComponentContextId] Blake: request for help

From: Edward Burns <edward.burns_at_oracle.com>
Date: Mon, 19 Mar 2012 18:56:38 -0700

>>>>> On Fri, 16 Mar 2012 17:10:57 -0700, Blake Sullivan <blake.sullivan_at_oracle.com> said:

B> On 3/16/12 7:34 AM, Edward Burns wrote:
KM> So, I suppose the question is really whether or not we should support
KM> the ability to use findComponent() and evaluate the properties outside
KM> of a tree traversal.
>>
B> We have never been able to support this in the general case, which is
B> why invokeOnComponent() was invented. We should improve the
B> documentation and should maybe throw an IllegalStateException when
B> attempting to evaluate EL attributes on a non-current component during
B> the development project stage.
>>
EB> Ok, I've made the following change to the documentation to
EB> UIComponent.findComponent():
EB>
EB> Make the second paragraph of the javadoc for that method be:
>>
EB> This method is not intended to be used with components that reside
EB> inside of an iterating component. To take action on a component inside
EB> of an iteration, or to find a component given a simple clientId, see
EB> invokeOnComponent.
>>
B> It's actually more basic than that--you can't perform any operation on a
B> component returned by findComponent() that depends directly or
B> indirectly on the component being in the correct traversal context. In
B> practice, this means that attempting to retrieve an EL-bound attribute
B> value is not safe. Iteration is only the most obvious of these
B> problems, and the one that exists in the spec components.

>> Here is the revised text.
>>
>> + *<p class="changed_added_2_2">WARNING: The found
>> + *<code>UIComponent</code> instance, if any, is returned
>> + *<strong>without</strong> regard for its tree traversal context.
>> + * Retrieving an EL-bound attribute from the component is not safe.
>> + * EL expressions can contain implicit objects, such as
>> + *<code>#{component}</code>, which assume they are being evaluated
>> + * within the scope of a tree traversal context. Evaluating
>> + * expressions with these kinds of implicit objects outside of a
>> + * tree traversal context produces undefined results. See {_at_link
>> + * #invokeOnComponent} for a method that<strong>does</strong>
>> + * correctly account for the tree traversal context when operating
>> + * on the found<code>UIComponent</code> instance. {_at_link #invokeOnComponent}
>> + * is also useful to find components given a simple<code>clientId</code>.
>>
>>
EB> Now, to comply with your recommendation to throw an
EB> IllegalStateException, let's look at section "5.6.2 ELResolver for
EB> Facelets and Programmatic Access". In there, you see a diagram showing
EB> our ELResolver chain. Blake, would the following comply with your
EB> recommendation?
>>
EB> Modify the table in section 5.6.2.1 so that for all ELResolver methods,
EB> if ProjectStage is development, and base is a UIComponent, throw an
EB> IllegalStateException if base is not equal to the component returned
EB> from UIComponent.getCurrentComponent()?
>>
B> Yes. And this should also ensure that implementation code that needs to
B> mess with the current component, is doing so correctly.

It turns out that implementing this check is not worth the complexity it
bring to the implementation. My time is better spent in trying to bring
your new API into the spec.

B> You need new component apis to manage the pushing and popping of the El
B> context as the components execute. Currently, with the JSF spec, if
B> application code executes while in an EL-context, that application code
B> is playing with fire if it then makes a call such as invokeOnComponent
B> or visitTree that further modifies the EL-context. This is because the
B> new modifications will be layered on top of the current modifications.
B> This is incorrect, we really want to pop the current context, assert the
B> new context, invoke the requisite code and then pop the new context and
B> reapply the old. If we can do the above more efficiently by only
B> applying the pops and pushes back to the lowest shared ancestor, all the
B> better. The Trinidad ComponentContextManager
B> <https://myfaces.apache.org/trinidad/trinidad-api/apidocs/org/apache/myfaces/trinidad/context/ComponentContextManager.html>
B> does this.

I'll see how hard this starts to look after trying to implement it.

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| homepage:               | http://ridingthecrest.com/