Hi Ed,
Just confirmed that your solution can't work, the reason is that the
restore part(the method restoreState() ) can't handle the state saved in
saveState() method of AttachedObjectListHolder. and in this case,
restoreState() is not so easy to modify.
The class AttachedObjectListHolder is not designed well and fragile to
work, for example, in comments, it says there is a 1:1 relationship
between restored state and the attachedObjects of
AttachedObjectListHolder, actually this is not correct cause the
attachedObjects is not always those restored by the state saved in
saveState().
I need to discuss with you about the solution if you wish.
I want to defend my previous solution here If the correct behavior is
that dynamically added validators should be kept and it only need one
line change:
In restoreState() method of class AttachedObjectListHolder:
this.attachedObjects.add(restored);
== change to the following ===>
add(restored)
Cause the add() method will call clearInitialState() to set the
initialState to false, this is the legacy logic, when initialState is
false, the attached RequireValidator is saved and kept for future restore.
Can you tell me why the initialState is set to false whenever a
validator added?
BR,
Zhijun
On 9/12/14, 8:45, zhijun Ren wrote:
> Hi Ed,
>
> I have tried this solution days before and it did not work.
>
> Cause the restore part also need to change accordingly.
>
> I will try to work out a feasible one according to your thought.
>
> BR,
> Zhijun
>
> On 9/12/14, 3:36, Edward Burns wrote:
>> Hello Zhijun,
>>
>> Thanks for your work on 3241-dynamicValidator.
>>
>> I attached another changebundle to the issue. Here is the header of the
>> changebundle:
>>
>> Please read through this changebundle, make the requested changes, and
>> try the tests. I didn't try the tests, so my change may require further
>> refinement.
>>
>> SECTION: Modified Files
>> ----------------------------
>> M test/servlet30/stateSaving/pom.xml
>>
>> - As mentioned by Manfred, when possible, we try to avoid adding a whole
>> new test app. Generally speaking, a new test app must be added if
>> either of the following conditions are true.
>>
>> * None of the existing are a good fit for this new test
>>
>> * The app requires per web-app configuration to test. For example, a
>> factory in the faces-config.xml, or some specific parameters in the
>> web.xml.
>>
>> I looked for a good fit in the existing tests here. I didn't find
>> one, but when I asked Manfred, he suggested
>> test/servlet30/facelets/core.
>>
>> Now, let's look at the new files:
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/src/main/webapp/WEB-INF/beans.xml
>>
>>
>> The tests in test/servlet30 must not have any CDI. As the name
>> implies, they are "Servlet 3.0" tests. So, do not include the
>> beans.xml in your commit.
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/nbactions.xml
>> test/servlet30/stateSaving/dynamicAddedValidator/pom.xml
>> test/servlet30/stateSaving/dynamicAddedValidator/src/main/java/com/sun/faces/test/servlet30/statesaving/dynamicaddedvalidator/DynamicValidatorBean.java
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/src/main/webapp/WEB-INF/glassfish-web.xml
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/src/main/webapp/WEB-INF/web.xml
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/src/main/webapp/index.xhtml
>>
>> test/servlet30/stateSaving/dynamicAddedValidator/src/test/java/com/sun/faces/test/servlet30/statesaving/dynamicaddedvalidator/Issue3241IT.java
>>
>>
>> Otherwise, looks great, just meld them into
>> test/servlet30/facelets/core and don't modify the pom.xml
>>
>> M
>> jsf-api/src/main/java/javax/faces/component/AttachedObjectListHolder.java
>>
>> - Now, I saw your changebundle posted 2014-09-11 07:45 JIRA time. I
>> don't agree with that fix. I have a different proposal.
>>
>> In AttachedObjectListHolder.saveState(), if we are in the initial
>> state, recognize that we still need to use a StateHolderSaver() to
>> wrap the attached object, even if it does implement StateHolder.
>>
>> This is accomplished with these changes:
>>
>> if (initialState) {
>> Object[] attachedObjects = new
>> Object[this.attachedObjects.size()];
>> boolean stateWritten = false;
>> for (int i = 0, len = attachedObjects.length; i< len;
>> i++) {
>> T attachedObject = this.attachedObjects.get(i);
>> if (attachedObject instanceof StateHolder) {
>> StateHolder sh = (StateHolder) attachedObject;
>> if (!sh.isTransient()) {
>> attachedObjects[i] = new
>> StateHolderSaver(context, sh);
>> stateWritten = true;
>> }
>> } else {
>> StateHolderSaver toSave = new
>> StateHolderSaver(context, attachedObject);
>> attachedObjects[i] = toSave;
>> stateWritten = true;
>> }
>> }
>>
>> Ed