Hi Leonardo,

thanks for the detailed explanation.

I disabling the PPS  for now.
We are using tobago where the problem occurs, and because tobago did not use PPS in the partial rendering implementation this could easily disabled.

Regards,
    Volker



2014-04-11 17:11 GMT+02:00 Leonardo Uribe <lu4242@gmail.com>:
Hi

The problem of c:forEach is a long story. I'll try to make a summary. This tag
originally came from JSP, and then it was somewhat "copied" into facelets,
together with other tags. But the copy in facelets had a problem: it doesn't
store the "model" internally (JSP variant does). Instead, the model is
reapplied each time a refresh over the page is done, usually before render
response phase by facelets. The effect of the refresh is usually remove or
add the necessary components to synchronize the model. Since in JSF 1.2 the
whole component tree is saved in the state, nobody noticed anything. Well,
some guys saw the problem. See:

http://drewdev.blogspot.co.at/2008/08/cforeach-with-jsf-could-ruin-your-day.html

Andrew Robinson did a partial solution in tr:forEach, but I'm afraid the
only solution that will work reliable is fix c:forEach.

But with the introduction of JSF 2 Partial State Saving (PSS) algorithm, it
was changed the way how the component tree was saved and restored. This
introduced some new problems:

1. The generated ids were not stable enough in JSF 1.2, and PSS requires
stable ids, which in other words means the same ids are generated to the
component each time facelets algorithm is applied. Content generated by
c:forEach is not stable enough, so you have situations where the state
just get lost.

2. Now the component state is split in two: the initial state and the delta
state, but c:forEach is a facelet tag that is not bound to a component
counterpart, so it doesn't have an associated state to be split.

3. In JSF 1.2 it was only one refresh point (before render response), but
JSF 2 PSS added one extra point where facelets algorithm is executed
(restore view phase). Please note if PSS is disabled, this is not called.

The final result is we have not just one problem but a set of different
problems all over the same facelet tag. We finally solved the problem in
MyFaces 2.2.2 after a lot of struggle. The code could be backport into 2.1.x,
but at this stage it was considered too risky, because it can introduce
instability to existing applications. We can consider the backport but only
after some more releases of 2.2.x branch.

There is a set of well known workarounds, so you can just apply one of them
and problem solved. It is too hard to explain in detail every change done in
the algorithm, it took a lot of time and effort, but the important thing to
remind is the new code in 2.2.2 copies the model as in JSP to create the
initial state. So, the model needs to be Serializable and if it is not, it
fallback to the old algorithm, so that's why the code works after you make
DynamicTab implements Serializable interface. That's the recommended
solution.

Why it works in Mojarra? it doesn't work, it apparently work, but it doesn't.
Try this change:

  <h:panelGroup>
  <h:inputText value="begin"/>
  <c:forEach items="#{tabProvider.tabs}" var="tab">
    <h:inputText value="#{tab.outValue}"/>
    <br/>
  </c:forEach>
  <h:inputText value="end"/>
  </h:panelGroup>

The generated markup is this (Mojarra 2.1.28):

<input type="text" name="j_idt6" value="begin" />
    <input type="text" name="j_idt7" value="Content of dynamic Tab number 0" />
    <br />
    <input type="text" name="j_idt9" value="Content of dynamic Tab number 1" />
    <br />
    <input type="text" name="j_idt11" value="Content of dynamic Tab number 2" />
    <br />
<input type="text" name="j_idt13" value="end" />

Can you see the problem? The id generation is completely broken! Why?
because you can put a c:if before the c:forEach and change the way how the ids
are generated, and in that way alter the state and have some nasty exception
like duplicate ids or "state get lost" problems. That's a "Chronicle of a
Death Foretold". If you make a click on the button, you'll see this:

<input type="text" name="j_idt6" value="begin" />
    <input type="text" name="j_idt7" value="Content of dynamic Tab number 0" />
    <br />
    <input type="text" name="j_idt9" value="Content of dynamic Tab number 1" />
    <br />
    <input type="text" name="j_idt11" value="Content of dynamic Tab number 2" />
    <br />
    <input type="text" name="j_idt15" value="Content of dynamic Tab number 3" />
    <br />
<input type="text" name="j_idt13" value="end" />

Yes, it works, but that suggest they are internally doing a fallback to JSF 1.2
state saving over that portion, which is just the same thing as we do with
org.apache.myfaces.REFRESH_TRANSIENT_BUILD_ON_PSS_PRESERVE_STATE. This has
obviously a cost over the state size.

The problem is a fallback to JSF 1.2 is not a satisfactory solution.
We have already
heard about that. State gets bigger, slow performance, because the
implementation
needs to make complex calculations over the state, components that loses the
state, unexpected exceptions of all kinds, that nobody knows how to reproduce it
and how to fix them. It could work in the simple cases, but if you
start to add more
and more things to the page, sooner or later the page will break.

If you don't believe me take a look at this error:

https://java.net/jira/browse/JAVASERVERFACES-3241

With that report only, not even the wizard Mandrake can know what the
hell happened.
This is not a rant. This is "old history", something that we already
realized many years
ago. See this mail:

https://java.net/projects/javaserverfaces-spec-public/lists/jsr344-experts/archive/2011-10/message/2

Now what we know what is wrong with c:forEach and that there are workarounds for
this stuff, we can ask the big question:

Why the same code fails in MyFaces?

It fails because MyFaces applies PSS algorithm the way it supposed to be by
JSF spec. When you add a new tab, the component is marked to be restored
fully, so the first time it doesn't happen anything, but the new tab is
saved fully into the state, but when the algorithm restores the view, it
tries to calculate the initial state and then it add the added tab, but
it was added twice because the initial state changed, and one of the base
principles is that the initial state does not change across requests. The
underlying problem is c:forEach should store the model of the first request
processed so PSS algorithm can still be valid. But that can only be done if
the model is Serializable.

So, theoretically the default mode of PSS algorithm should implement the
solution proposed and already added in MyFaces 2.2.2. From the beginning
we have known that PSS algorithm is not 100% fail safe, but we also know
the only remaining case is this one.

Can we fix this case? even if it is not the best solution?

What we can do is add some lines of code that checks for the duplicate when
the component is added and use the saved version of the component instead.
More than a fix is a "damage control" alternative. But it sounds good,
because the check can be done quickly in the nearby, where the component
is supposed to be, and you can activate the case, just checking if the
component is managed by facelets.

I'll try the fix and if works, I'll commit it on all branches (2.0.x, 2.1.x
and 2.2.x).

regards,

Leonardo

2014-04-11 14:06 GMT+02:00 Leonardo Uribe <lu4242@gmail.com>:
> Hi
>
> It seems the example works under this configurations on 2.1.x branch:
>
> 1. Set javax.faces.PARTIAL_STATE_SAVING to false.
>
> or
>
> 2. Add the page in javax.faces.FULL_STATE_SAVING_VIEW_IDS (disable PSS
> on the related page only).
>
> or
>
> 3. Set org.apache.myfaces.REFRESH_TRANSIENT_BUILD_ON_PSS_PRESERVE_STATE to true.
>
> The reason is simple, in all previous cases what we are doing is mark
> that part of the tree to be saved fully. I'll give a detailed
> explanation later, I just wanted to give the workarounds for the time
> being.
>
> regards,
>
> Leonardo Uribe
>
>
> 2014-04-10 19:21 GMT+02:00 Leonardo Uribe <lu4242@gmail.com>:
>> Hi
>>
>> I was able to reproduce the problem. But I can confirm the problem is
>> gone once you use 2.2.3 and make DynamicTab implements Serializable.
>> I'll try to find if there is a way to fix it somehow in 2.1.x, so it
>> can be "less broken".
>>
>> regards,
>>
>> Leonardo
>>
>> 2014-04-10 17:05 GMT+02:00 Volker Weber <v.weber@inexso.de>:
>>> Hi,
>>>
>>> we are facing a duplicate id exception after adding a component in
>>> applicationPhase to to view.
>>>
>>> We are using c:forEach and add an item to the iterated list in the
>>> actionListener.
>>>
>>> Saving this view is done without problem, but restoring this saved view
>>> results in a view containing the new component twice! Which results in a
>>> duplicate id exception when saving this restored view again.
>>>
>>> I found that this new component on creation is marked as
>>> "oam.COMPONENT_ADDED_AFTER_BUILD_VIEW.
>>> On restore view the facelet is used to recreate the view, which is
>>> including the new component because it is now included in the forEach
>>> iteration.
>>> After recreating the view with the facelet the view is extended with the
>>> components which were marked with "oam.COMPONENT_ADDED_AFTER_BUILD_VIEW
>>> when storing the view.
>>>
>>> At this point the component is duplicated in the view.
>>>
>>> The Problem is in
>>> org.apache.myfaces.view.facelets.DefaultFaceletsStateManagementStrategy
>>> (row 437 to 451 in 2.1.10 sources), the component is added to the view
>>> regardless if there is already a component with same id.
>>>
>>> I have a example app to reproduce this Problem can downloaded here
>>> http://www.inexso.net/files/myfaces-dynamic-test.zip .
>>>
>>> Run this with "mvn jetty:run" to reproduce this.
>>>
>>> Running this with "mvn -Dcontainer=jetty-mojarra jetty:run" did not give
>>> me any error.
>>>
>>>
>>> Regards,
>>>   Volker
>>>
>>>
>>> --
>>> Volker Weber
>>> -Leiter Software-Entwicklung-
>>>
>>> inexso - information exchange solutions GmbH
>>> Ofener Str. 30      | 26121 Oldenburg
>>> www.inexso.de
>>>
>>> Firmensitz: Oldenburg | Amtsgericht Oldenburg HRB 205251
>>> Geschäftsführer: Stefan Schulte, Michael Terschüren
>>>
>>>



--
inexso - information exchange solutions GmbH
Ofener Straße 30 | 26121 Oldenburg
www.inexso.de