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
--
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 11 work days til JavaOne 2014
| 7 work days til start of JSF 2.3 and Servlet 4.0