faces icon indicating copy to clipboard operation
faces copied to clipboard

TCK Challenge: faces23/facelets ui:repeat begin/end tests rely on undocumented behavior

Open brideck opened this issue 2 years ago • 2 comments

Challenged Tests: ee.jakarta.tck.faces.test.servlet40.facelets.Spec1102IT#testSpec1102

TCK Version: Jakarta Faces 4.0.x

Tested Implementation: Open Liberty -- containing MyFaces 4.0

Description: The VDL for <ui:repeat/> is not particularly well-specified. As a result, Mojarra and MyFaces have made different implementation decisions that are exacerbated by the strange set of corner cases used in this TCK test. The areas of contention covered by these tests include:

  1. Negative values for the begin and end attributes

For the begin attribute, the VDL states "If value specified: Iteration begins at the item located at the specified index. First item of the collection has index 0. If value not specified: Iteration begins with index set at the specified value." The repeated use of the word index might lead one to believe that this should work like <c:forEach/> from Tags (as that's what this was designed after -- https://github.com/jakartaee/faces/issues/1102), and indeed the VDL for that tag is identical, only the references to items have all been updated to value (which both makes for confusing reading, since "value" is now used two different ways in the same sentence, and is problematic for reasons outlined in section 5 of this challenge).

The doc states that the first item in the collection should have index 0, so what do negative values mean in this context? Yet five of the eight variations in this test (repeat2 to repeat6) involve negative values for begin and/or end. According to the behavior seemingly specified, MyFaces ignores the negative values and starts with index 0. Mojarra, and by extension the expected answer in the TCK, allows the negative values and does something different.

Taking repeat2 as an example:

<span id="repeat2"><ui:repeat begin="-3" end="3" var="i">#{i}</ui:repeat></span>

Mojarra produces -3-2-10123, MyFaces produces 0123.

  1. begin attribute is greater than end attribute

Nothing in the documentation specifies what should happen in this case. Should the implementation loop through the collection in reverse? Should this be allowed at all? Again, Mojarra and MyFaces take a different tack.

<span id="repeat3"><ui:repeat begin="3" end="-3" var="i">#{i}</ui:repeat></span>

Mojarra loops backward, producing 3210-1-2-3. MyFaces rejects this configuration, resulting in an exception:

org.apache.myfaces.view.facelets.LocationAwareFacesException: begin cannot be greater than end
    at org.apache.myfaces.view.facelets.component.UIRepeat._validateAttributes(UIRepeat.java:968)
  1. Interactions between attributes: begin/offset and end/size

Per the discussion in https://github.com/jakartaee/faces/issues/1102 these attribute pairs are effectively synonymous with each other, so what happens when someone specifies both end and size with conflicting values? Again, nothing in the documentation specifies these cases. Does one take precedence over the other? Should the configuration be rejected?

<span id="repeat6"><ui:repeat begin="-3" end="3" step="2" size="2" var="i">#{i}</ui:repeat></span>

In the absence of a provided value (see section 5 of this challenge for more on this), Mojarra seems to define the collection by the begin and end elements and then proceeds to treat it as if it only has size elements in it, merely producing -3 in this case. Because of the added step attribute in this case, it's unclear exactly how MyFaces handles end vs. size, but it produces 02 as you would expect from an implementation that ignores the negative value and then steps by 2.

(See https://github.com/jakartaee/faces/issues/1713 for the start of a discussion about many attribute interactions in this space.)

  1. What is the default value for either begin or end when only one is provided?

The documentation is silent on what happens in either of these cases, yet one of them is tested in the TCK.

<span id="repeat8"><ui:repeat end="3" var="i">#{i}</ui:repeat></span>

Mojarra produces 0123, while MyFaces doesn't assume where to start and produces nothing.

  1. How is the case where no value is specified handled?

None of the variations in this test specify the value attribute. This could be a problem in and of itself, since the VDL says that value is a required attribute of <ui:repeat/>. Yet, by copy/pasting from <c:forEach/> in Tags (where the equivalent items attribute is not marked as required), a contradiction has been introduced in the documentation -- value is required, but begin (and end) claims to state how to handle the case where it's not been provided: "If value not specified: Iteration begins with index set at the specified value."

Again, Mojarra and MyFaces differ in behavior when no value is specified, and neither implementation enforces it as a required attribute.

Mojarra sees that no value is set and creates an Array with values from the begin to end (thus allowing negatives). Iteration now occurs over this new array object. In effect, the begin and end attributes are treated as literal values (much like a for loop would behave).

On the other hand, MyFaces uses an empty model when no value is specified.

Due to the lack of specificity in the documentation, the challenged test should be removed as invalid until things become better defined. Then the TCK can be updated to reflect the decisions that are made in this space.

brideck avatar Nov 10 '22 07:11 brideck

Accepted. The VDL on this is indeed very sloppy and needs revision.

BalusC avatar Nov 10 '22 11:11 BalusC

Thanks @BalusC for your input, I've labeled this issue challenge I also agree with your statement, and as such I'll label it as accepted as well.

pnicolucci avatar Nov 10 '22 12:11 pnicolucci

Please continue here to see redefinition of spec on these attributes https://github.com/jakartaee/faces/issues/1713

BalusC avatar Dec 10 '22 11:12 BalusC

Seems to be fixed for now. Remaining items will be done in new spec release.

arjantijms avatar Dec 13 '22 16:12 arjantijms