Hello Zhijun,
I'm sorry to have to make a modification to your commit in this way, but
I think it is the best way to take advantage of this "teachable moment"
[1] regarding JCP practices. Please accept my apologies if you already
know all this.
SECTION: Modified Files
----------------------------
M jsf-api/src/main/java/javax/faces/flow/FlowScoped.java
- Added text relating to JAVASERVERFACES_SPEC_PUBLIC-1361
* <p class="changed_added_2_3">When replacing (rather than decorating)
* the flow implementation with a custom {_at_link FlowHandler}
* implementation, it is necessary to also replace the CDI extension
* that implements the specified behavior regarding
* <code>FlowScoped</code> beans.</p>
M jsf-ri/src/main/java/com/sun/faces/flow/FlowHandlerImpl.java
M jsf-ri/src/main/java/com/sun/faces/flow/FlowCDIContext.java
- As indicated in my earlier changebundle, there already is a tight
coupling between FlowHandlerImpl and FlowCDI context: FlowHandlerImpl
call static methods flowEntered() and flowExited() on FlowCDIContext.
I want to solve JAVASERVERFACES-3680 by simply extending that coupling
to go the other way as well.
You had solved 3680 by adding a method on FlowHandlerImpl and
downcasting from within FlowCDIContext, as shown here:
if ( flowHandler instanceof FlowHandlerImpl) {
FlowHandlerImpl flowHandlerImpl = (FlowHandlerImpl)facesContext.getApplication().getFlowHandler();
While I appreciate your usage of the instanceof test to avoid a
ClassCastException, that approach will unnecessarily fail when the
FlowHandler is decorated. It should not fail in that case.
Consider this usage:
public class MyFlowHandlerFactory extends FlowHandlerFactoryWrapper<FlowHandlerFactory> {
private FlowHandlerFactory parent;
public MyFlowHandlerFactory(FlowHandlerFactory parent) {
this.parent = parent;
}
public FlowHandlerFactory getWrapped() {
return parent;
}
}
public class MyCustomFlowHandler extends FlowHandler {
private FlowHandler parent;
public MyCustomFlowHandler(FlowHandler parent) {
this.parent = parent;
}
public Map<Object, Object> getCurrentFlowScope() {
return parent.getCurrentFlowScope();
}
// ... similar for other methods. Just call return parent.method().
}
This approach would fail with your code, even though it really should
pass because the FlowHandler implementation is indeed just doing the
same thing as FlowHandlerImpl but is not an instance of
FlowHandlerImpl. My approach, calling the public static method on
FlowHandlerImpl, would be ok.
Now, all of this hinges upon this comment, which I added in
JAVASERVERFACES_SPEC_PUBLIC-1361:
Also add to the spec that when replacing (rather than decorating)
the FlowHandler, one must also replace the @FlowScoped CDI
context/extension. The technique of doing this replacement is still
an open question.
and which I included in the spec for FlowScoped above.
Please accept this modification to your commit. I think it's OK,
unless I'm missing something.
[1]
http://en.wikipedia.org/wiki/Teachable_moment
--
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 15 days til DevNexus 2015
| 25 days til JavaLand 2015
| 35 days til CONFESS 2015