users@javaserverfaces-spec-public.java.net

[jsr372-experts mirror] [jsr372-experts] Re: Re: ui:repeat

From: Leonardo Uribe <leonardo.uribe_at_irian.at>
Date: Wed, 13 Jan 2016 22:30:59 -0500

Hi

Use a Repeater interface or any call to class.getName() or whatever doesn't
look good. Why the parent component requires to know the internal structure
of the children? If the code is right, such coupling is not necessary. The
only exception to the rule is the components that implements
EditableValueHolder interface (save values per row for setRowIndex(...)).

In few words, the fix proposed is an incomplete solution, which is not
satisfactory in my opinion. I think it will be always possible to find
cases where the code will break.

In MyFaces the code of ui:repeat was modified from the original code
provided by facelets, to work in the same way MyFaces UIData implementation
does.


The only difference we have for ui:repeat / h:datatable is:

https://issues.apache.org/jira/browse/MYFACES-3326
https://issues.apache.org/jira/browse/MYFACES-3702

In some cases it is necessary that ui:repeat or h:datatable does not
refresh or clean the model. For example, when there is a validation error,
or when there is an error. The code in myfaces looks like this:

    public void encodeBegin(FacesContext context) throws IOException
    {
        _initialDescendantComponentState = null;
        if (_isValidChilds && !hasErrorMessages(context))
        {
            // Clear the data model so that when rendering code calls
            // getDataModel a fresh model is fetched from the backing
            // bean via the value-binding.
            _dataModelMap.clear();

            // When the data model is cleared it is also necessary to
            // clear the saved row state, as there is an implicit 1:1
            // relation between objects in the _rowStates and the
            // corresponding DataModel element.
            _rowStates.clear();
        }
        super.encodeBegin(context);
    }

I remember some users reported small differences between how ui:repeat
works for Mojarra and MyFaces:

(dennis hoersch) see MYFACES-3702

---------------------------------------------------------------

The question is about the following JavaDoc of UIData?

void encodeBegin(FacesContext context)
In addition to the default behavior, ensure that any saved per-row state
for our child input components is discarded unless it is needed to rerender
the current page with errors.

I understand that sentence as it is talking about the row state, not the
data model. But I am not so deeply into the implementation and don't know
if there might be a dependency between the both.



Mojarra is implementing that encodeBegin() with

private void preEncode(FacesContext context) {
setDataModel(null); // re-evaluate even with server-side state saving
if (!keepSaved(context))
{ // check if context has errors
getStateHelper().remove(PropertyKeys.saved); // usage of 'saved' looks like
'_rowStates' in MyFaces }

}

Mojorra clears the dataModel in both UIData and UIRepeat in any case.
---------------------------------------------------------------



Sorry if this mail is becoming too complex to understand, the important is
there is a relation between the model and the state, and I guess
implementation of UIRepeat in Mojarra has some bugs, and the reason why
this is happening is because the code is not correctly aligned with that
idea. In cases where there is a nested ui:repeat and h:datatable or
whatever, one real UIRepeat instance could be used to render different
components, and each component has its own model, but it also has its own
state. The error Cagatay describe happens because in some cases Mojarra
code clears a model that doesn't have to and probably its associated state.

AT>> does anyone think it's a good idea to investigate if it's
necessary/useful to introduce a new iterating (base) component?

Useful yes, necessary not much, but it doesn't feel good to not be able to
create components that extends from UIRepeat. My opinion: Yes.

regards,

Leonardo Uribe


2016-01-13 16:10 GMT-05:00 arjan tijms <arjan.tijms_at_gmail.com>:

> Hi,
>
> On Wed, Jan 13, 2016 at 5:53 PM, Cagatay Civici <
> cagatay.civici_at_primetek.com.tr> wrote:
>
>> Following is from Mojarra that is used by UIData and UIRepeat;
>>
>> private Boolean isNestedWithinIterator() {
>> if (isNested == null) {
>> UIComponent parent = this;
>> while (null != (parent = parent.getParent())) {
>> if (parent instanceof UIData ||
>> parent.getClass().getName().endsWith("UIRepeat")) {
>> isNested = Boolean.TRUE;
>> break;
>> }
>> }
>> ...
>>
>> [...]
>> In terms of Spec, an interface like Repeater should solve it where we at
>> PF can implement as well for our components.
>>
>
> We have had comparable issues with that same fragment. I like the Repeater
> proposal. If we just let it be a marker interface and then have both UIData
> and UIRepeat implement it, then the check can become:
>
> if (parent instanceof Repeater ||
> parent.getClass().getName().endsWith("UIRepeat")) {
> isNested = Boolean.TRUE;
> break;
> }
>
> It still has the class name check, but I think that's a small price for
> the sake of backwards compatibility.
>
> This would be a relatively small change, in terms of coding effort trivial
> almost. Cagatay, would the above change fully fix the issue for you?
>
> Parallel to this, does anyone think it's a good idea to investigate if
> it's necessary/useful to introduce a new iterating (base) component? I took
> a few looks at the existing code of UIData for some of the above issues and
> I fear that fixing those while at the same time being 100% backwards
> compatible is undoable.
>
> Kind regards,
> Arjan Tijms
>
>
>
>
>
>> On Wednesday 13 January 2016 at 16:27, arjan tijms wrote:
>>
>> Hi,
>>
>> Interesting, indeed. There's quite an amount of issues in the spec
>> tracker that concern ui:repeat and the related h:dataTable. I planned to do
>> a JSF 2.3 wishlist part II, but never came around to doing that.
>>
>> See:
>>
>> - https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-236
>> - UIData needs review on a couple of fronts
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-935
>> - Alignment/extending of iterating ui components
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-963
>> - Provide a component for iterating over hierarchical data
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-965
>> - ui:repeat requires document "begin" and "end" properties
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-892
>> - add "maxRepeats" attribute to ui:repeat.
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1249
>> - Add a styleClass attribute to h:column
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-217
>> - Inconsistent column styles handling with h:dataTable
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-627
>> - ui:repeat varStatus incompletely specified
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-609
>> - Add rowStatePreserved property to UIRepeat, exactly the same as
>> UIData https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1263
>> - UIData (and related classes) doesn't respect contract for
>> UIComponent process
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1120
>> - UIRepeat supports Collection
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1103
>> - Add support for condition check like regular for-loop in ui:repeat
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1102
>> - Have DataModel implementations registerable
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1078
>> - h:datatable component to handle multiple entities lists
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-713
>> - UIRepeat et al are not specified
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-704
>> - Ajax inside a DataTable (getClientId append rowIndex)
>> https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-865
>>
>> Perhaps some of the inconsistencies Cagatay found are related to any of
>> those?
>>
>> Kind regards,
>> Arjan Tijms
>>
>>
>>
>>
>>
>> On Wed, Jan 13, 2016 at 3:15 PM, Kito Mann <kito.mann_at_virtua.com> wrote:
>>
>> Hello Cagatay,
>>
>> I noticed you just added a PrimeFaces repeat component
>> <https://github.com/primefaces/primefaces/issues/999> citing "inconsistencies
>> between ui:repeat implementations of mojarra and myfaces with our
>> components". Is there something we need to do with the spec to ensure that
>> the two implementations of ui:repeat behave more consistently?
>> ___
>>
>> Kito D. Mann | @kito99 | Author, JSF in Action
>> Web Components, Polymer, JSF, PrimeFaces, Java EE, and Liferay training
>> and consulting
>> Virtua, Inc. | virtua.tech
>> JSFCentral.com | @jsfcentral
>> +1 203-998-0403
>>
>> * Listen to the Enterprise Java Newscast: *http://
>> <http://blogs.jsfcentral.com/JSFNewscast/>enterprisejavanews.com
>> <http://ww.enterprisejavanews.com>*
>>
>>
>>
>>
>