dev@javaserverfaces.java.net

Re: Zhijun: review for 3129-dynamicValidator

From: zhijun Ren <ren.zhijun_at_oracle.com>
Date: Fri, 12 Sep 2014 08:45:08 +0800

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