faces icon indicating copy to clipboard operation
faces copied to clipboard

ui:repeat clarification on attributes, such as offset and size

Open volosied opened this issue 2 years ago • 12 comments

There are behavior difference between Mojarra and MyFaces regarding ui:repeat.

The VLDDoc is vague, in my opinion. The main one I'm concerned about if offset="1" size="2", but I would appreciate any feedback.

For example:

        <ui:repeat id="repeat" value="#{['B1', 'B2', 'B3', 'B4', 'B5']}" var="row"   ...  >
            <h:outputText id="outputText" value="#{row}" />
        </ui:repeat> 

        
        Myfaces 4.0.0-RC2 (would be the same as 3.0, 2.3) :
        offset="2" : B3B4B5
        size = "3" : B1B2B3
        offset="2" begin="1" : ""
        begin="1" size="1" : LocationAwareFacesException: begin cannot be greater than end
        offset="1" size="2" : B2B3


        Mojarra 4.0.0: 
        offset="2" : B3B4B5
        size = "3" : B1B2B3
        offset="2" begin="1" :B2B3B4B5
        begin="1" size="1" : ""
        offset="1" size="2" : B2

These are just a few differences, but there are some more I've found (1). In general, the specification is vague and this has results in differences between the handling. I'm confused overall now in the proper behavior. In the simple cases of offset and size, things behave as expected. However, mixing the two produce different results.

Where does offset occur? From the begin value? From the first item in the collection? Should it be disregarded in some scenarios? What should occur if begin is a negative value?

I have similar questions for size. What does size mean? Is the simply the number of iterations to take? Or does it mean something else? Where does it start? Is size an index or a value? How does it relate to offset, begin, the first item in the collection, and the actual length of the collection? How is size different from end?

For size, the spec also mentions: If this value is less than the actual size of the collection, a FacesException must be thrown. but either implementation throws an exception?

Thanks!

  1. When should an empty model bee created? Should negative indices be supported (i.e. begin="-2") when iterating an empty collection (i.e begin=-2 end=3? Which has precedence if both end and size are specified?

volosied avatar Nov 04 '22 19:11 volosied

Found some information here which I'll need to review:

  • https://softwarecave.org/2014/04/18/facelets-uirepeat-tag/

    • Article says size is the number of iterations. and offset the index from which the iteration should start. Using this, it looks like MyFaces is correct with it's offset="1" size="2" handling since it outputs B2B3 (two iterations)

    • However, even MyFaces breaks down using these definitions if you throw step into the mix. For example, the code below produces B2, but I would expect it to produce B2B4. (offset skips B1, loop starts at B2 and steps by 2 to B4. and ends there since the iteration occurred twice) I suppose the problem here is how size is set. If size is treated as an index and it's set to end ( i.e. end = size -1) then the loop would stop at B3 and never reaches B4.

      <ui:repeat id="repeat" value="#{['B1', 'B2', 'B3', 'B4', 'B5']}" var="row" offset="1" size="2" step="2" >
        <h:outputText id="outputText" value="#{row}" />
      </ui:repeat>
      
  • https://github.com/jakartaee/faces/issues/1058 :

    • Mentions that purpose of the size attribute is to specify the number of elements or items to iterate over in the overall collection
  • https://github.com/jakartaee/faces/issues/1102#issuecomment-518592825 :

    Whilst the ui:repeat offset/size attributes have the same effect as c:forEach begin/end, during iteration the ui:repeat will still check DataModel#isRowAvailable(). In other words, presence of the value attribute referring the desired amount of items is still required. This is not what is being requested here.
    
    Further I also noticed in the Mojarra ui:repeat source code that it actually supports the literal begin/end attributes. The offset is just an overriding alias for begin, and the end defaults to size when unspecified. As those attributes have a clearer meaning, I will also document those attributes." 
    
  • https://issues.apache.org/jira/browse/MYFACES-3034

    • "On Mojarra 2.0.4, the size attribute is taken to mean the position of the last element to retrieve, not the number of elements to retrieve."

Code

  • Mojarra: https://github.com/eclipse-ee4j/mojarra/blob/831088a38ae846df96b880a488653bdf5d281cb3/impl/src/main/java/com/sun/faces/facelets/component/UIRepeat.java#L511-L518

  • MyFaces: https://github.com/apache/myfaces/blob/89c747e85615e3f33265e664c8361789f38ea7db/impl/src/main/java/org/apache/myfaces/view/facelets/component/UIRepeat.java#L1014-L1027

volosied avatar Nov 05 '22 00:11 volosied

History

Initially, ui:repeat had only offset/size/step attributes. However, this didn't allow to iterate a predefined amount of times. I.e. the following attempt to render 10 divs didn't work because it still requires the value attribute to be set.

<ui:repeat offset="1" size="10" var="i">
    <div>div number #{i}</div>
</ui:repeat>

In JSF 2.2, begin/end was added via https://github.com/eclipse-ee4j/mojarra/issues/2244 allowing this to work based on how the <c:forEach> was implemented as per https://jakarta.ee/specifications/tags/1.2/tagdocs/c/foreach:

<ui:repeat begin="1" end="10" var="i">
    <div>div number #{i}</div>
</ui:repeat>

However, this was in hindsight not terribly well thought out. The primary mistake was that the begin attribute completely overlapped the existing offset attribute in the actual implementation. And the meaning of the size attribute collided with the end attribute. The original thought was that begin/end should only and only be used when the value attribute is not set, and that they should be ignored or even throw an exception when the value attribute is set. I think explicitly mentioning this in the spec would be the most cleanest way to go forward.

Current spec

  • begin: First item of the collection has index 0. If value not specified: Iteration begins with index set at the specified value.
  • end: If value specified: Iteration ends at the item located at the specified index (inclusive). If value not specified: Iteration ends when index reaches the specified value (inclusive).
  • offset: Read-write property setting the offset from the beginning of the collection from which to start the iteration. If not set, this offset is not considered and iteration will start at the beginning of the collection.
  • size: Read-write property setting the size of the collection to iterate. If this value is less than the actual size of the collection, a FacesException must be thrown.
  • step: Iteration will only process every step items of the collection, starting with the first one.

New spec proposal

Primary change: begin/end MAY NOT be used when value attribute is specified (even if it evaluates to null or empty) and the offset and size MUST require the value attribute (even if it evaluates to null or empty).

  • begin: If the value attribute is not specified: iteration begins with the specified number (inclusive) as item. If the value attribute is specified: a FacesException must be thrown. If the corresponding end attribute is not specified: use 0 as default. The corresponding end attribute may be less than the begin attribute: iteration will take place in a reversed manner.
  • end: If the value attribute is not specified: iteration ends with the specified number (inclusive) as item. If the value attribute is specified: a FacesException must be thrown. If the corresponding begin attribute is not specified: use 0 as default. The corresponding begin attribute may be greater than the end attribute: iteration will take place in a reversed manner.
  • offset: If the value attribute is specified: iteration begins with the specified number as index. If the value attribute is not specified: a FacesException must be thrown. If the offset attribute is not specified: use 0 as default. If the offset attribute is less than 0 or is greater than the size of the actual collection behind the value attribute: a FacesException must be thrown.
  • size: If the value attribute is specified: iteration ends when the specified number of times has been iterated (inclusive). If the value attribute is not specified: a FacesException must be thrown. If the size attribute is not specified: use the size of the actual collection behind the value attribute as default. If the sum of the size attribute and the offset attribute is less than 0 or is greater than the size of the actual collection behind the value attribute: a FacesException must be thrown. If the step attribute is specified: each skipped item must also be counted for the size attribute.
  • step: Iteration will only process every step items of the collection, starting with the first one. If the step attribute is less than 1: a FacesException must be thrown.

Optional: support a negative step. I.e. step="-1" will iterate backwards. This will be a new feature.

Future spec proposal

Merge offset into begin so that no FacesException will be thrown depending on the presence of the value attribute. But this will require a Major version release because of breaking changes. Also it should be looked at if end could be merged into size. But I think this will end up in less intuitive API.

TCK proposal

<ui:repeat begin="1" end="3" var="i"> #{i} </ui:repeat> must render 1 2 3
<ui:repeat begin="-3" end="3" var="i"> #{i} </ui:repeat> must render -3 -2 -1 0 1 2 3
<ui:repeat begin="3" end="-3" var="i"> #{i} </ui:repeat> must render 3 2 1 0 -1 -2 -3
<ui:repeat begin="-3" end="3" step="2" var="i"> #{i} </ui:repeat> must render -3 -1 1 3
<ui:repeat begin="-3" end="3" offset="1" var="i"> #{i} </ui:repeat> must throw FacesException (offset requires value attribute)
<ui:repeat begin="-3" end="3" size="2" var="i"> #{i} </ui:repeat> must throw FacesException (size requires value attribute)
<ui:repeat begin="3" end="3" var="i"> #{i} </ui:repeat> must render 3
<ui:repeat begin="3" var="i"> #{i} </ui:repeat> must render 3 2 1 0
<ui:repeat begin="-3" var="i"> #{i} </ui:repeat> must render -3 -2 -1 0
<ui:repeat begin="0" var="i"> #{i} </ui:repeat> must render 0
<ui:repeat end="3" var="i"> #{i} </ui:repeat> must render 0 1 2 3
<ui:repeat end="-3" var="i"> #{i} </ui:repeat> must render 0 -1 -2 -3
<ui:repeat end="0" var="i"> #{i} </ui:repeat> must render 0
<ui:repeat var="i"> #{i} </ui:repeat> must render nothing
<ui:repeat begin="1" end="3" value="#{['A', 'B', 'C', 'D', 'E']}" var="i"> #{i} </ui:repeat> must throw FacesException (begin and end disallow value attribute, instruct user to use offset/size instead)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="2"> #{i} </ui:repeat> must render C D E
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="-1"> #{i} </ui:repeat> must throw FacesException (offset less than 0)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="6"> #{i} </ui:repeat> must throw FacesException (offset greater than item count)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" size="2"> #{i} </ui:repeat> must render A B
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" size="-1"> #{i} </ui:repeat> must throw FacesException (size less than 0)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" size="6"> #{i} </ui:repeat> must throw FacesException (size greater than item count)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="2" size="2"> #{i} </ui:repeat> must render C D
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="2" size="4"> #{i} </ui:repeat> must throw FacesException (offset plus size greater than item count)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" step="2"> #{i} </ui:repeat> must render A C E
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" step="0"> #{i} </ui:repeat> must throw FacesException (step less than 1)
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="1" step="2"> #{i} </ui:repeat> must render B D
<ui:repeat value="#{['A', 'B', 'C', 'D', 'E']}" var="i" offset="1" step="2" size="2"> #{i} </ui:repeat> must render B

Thoughts? cc: @arjantijms and @tandraschko

BalusC avatar Dec 03 '22 13:12 BalusC

+1 for Faces 5 proposal with fusion of begin / end with offset / size

Less things to learn, less obscure logic ...

In this way it's straightforward like an old style loop with index ...

pizzi80 avatar Jul 03 '23 21:07 pizzi80

@arjantijms @tandraschko @volosied +1 or -1 for 4.1 spec update proposal? And the support for a negative step?

BalusC avatar Jul 22 '23 16:07 BalusC

Hi, a recent faces-dev email had discussions about changes between releases.

The 4.1 proposal here for ui:repeat looks fine to me, but it would introduce breaking changes via the new behavior (exceptions), and I'm not sure if that should go into 4.1, per the guidelines set in the document:

Minor version increments: Backwards compatible new behaviour.

Should this be pushed back into 5.0? @BalusC

volosied avatar Aug 02 '23 14:08 volosied

Ok sure.

BalusC avatar Aug 02 '23 14:08 BalusC

How about the support for a negative step?

Optional: support a negative step. I.e. step="-1" will iterate backwards. This will be a new feature.

This can be very useful at times. We now have the opportunity to implement it for 5.0.

BalusC avatar Oct 05 '23 10:10 BalusC