OK, based on the feedback from you and Kyle, I think #1 is a reasonable way to go. This makes using FCF outside of a normal Page Flow request more complex, but in this scenario it's probably better for the ServletBeanContext management to be explicit -- this allows use of the returned page flow controller in *any* way that the caller sees fit (including Carlin's original test case, I think).
Carlin, does this (option #1) seem reasonable to you? Rich Eddie O'Neil wrote: >Rich-- > > Yeah; I think that this captures where we are pretty well. > > First on the ServletBeanContext issue -- in thinking about it more, >what we're doing with normal action requests (through the >PageFlowServlet or filters) seems fine. ServletBeanContext has >beginContext / endContext methods that allow reassociating the request >/ response / servlet context with a context. So, reusing that >instance seems like a fine thing to do. > > On the FlowControllerFactory (FCF), my thoughts are that there are >two options which involve loosely coupling the Control lifecycle with >that call. Could be done in two ways: > >1) extract control container initialization out of the FCF. This >would require the user of the FCF API to initialize a >ControlContainerContext if needed before calling the FCF. This is >sort of what the PF filter and servlet do today. This would have the >lifecycle look like: > > - initialize CCC > - initialize FCF > - unit of work > - uninitialize CCC > >2) leave control container initialization inside of the FCF but allow >this to be configurable. This allows breaking the binary dependence >between Controls and JPF. The issue with this is that the CCC >lifecycle is started inside of the FCF, but the CCC still needs to be >"ended" outside of the FCF. This would have the lifecycle look like: > > - initialize FCF > - initialize CCC > - unit of work > - uninitialize CCC > > >Option 1 is more symmetric; option 2 is more automatic with the >implicit contract that the CCC.endContext method be called after the >unit of work. > > Guess that this is a log way of saying that I'd go for (1) and break >the control lifecycle out of the FCF. :) > > As for Carlin's test, I'll leave that to your collective best >judgment; if the use-case is invalid, that certainly makes it easier. >:) Depending on how we change JavaControlUtils, we might need to add >the initialize / uninitialize CCC calls above as well. > >Thanks for taking a look at this... > >Eddie > > >On 11/14/05, Rich Feit <[EMAIL PROTECTED]> wrote: > > >>OK, here's my take (this is after an offline chat with Eddie -- I >>*think* we're on the same page, but if not, let me know). >> >> - We actually store a ControlBeanContext (the ServletBeanContext) in >>the user session, so that when we're calling beginContext/endContext in >>JavaControlUtils, we're calling it on the *same* instance of the >>context, even across multiple requests. It sounds like this is >>something we could take a second look at across the board (not just in >>this case). >> >> - It's possible that we need to reexamine the semantics around >>FlowControllerFactory. Right now, we guarantee that you can get a >>working page flow instance with a single call. Question is, do we want >>to extract (not just abstract) Controls logic from >>FlowControllerFactory? Would it be OK to require that the appropriate >>Controls context is set up and torn down independent of the call to >>get/create a page flow? My assumption has always been that >>FlowControllerFactory should guarantee that the returned page flow >>controller is set up, but I'm open to rethinking this. >> >> - Carlin's test is doing something that we don't support, which is >>calling methods on a page flow instance directly after getting it from >>FlowControllerFactory. I didn't pick up on this the first time around. >>The issue is that we don't get a chance to run the right setup code >>before the page flow instance is accessed. >> >>Carlin, as a first step, would you agree with changing the test so that >>it runs an action through PageFlowUtils.strutsLookup(), rather than >>invoking a method through reflection? This would guarantee that the >>right Page Flow lifecycle is run before any member control is accessed. >>It's also the way a Portal framework would run an action (although it >>will likely change in some manner when do the work for JSR 168 support). >> >>Let me know what you think... >> >>Thanks, >>Rich >> >>Eddie O'Neil wrote: >> >> >> >>> Very close. :) >>> >>> I'm assuming that this works as in the test that Carlin committed which >>> does: >>> >>>- deploy webapp >>>- start request directly to a Servlet >>>- go get a Page Flow from the FlowControllerFactory >>>- unit of work >>> - call the control in onCreate or call the control in some action >>>- end request >>> >>>What actually happens under the covers is this: >>> >>>- deploy webapp >>>- initiate request directly to a Servlet that uses the FCFactory >>>- go get a Page Flow from the FlowControllerFactory >>> - JavaControlUtils doesn't find a ControlContainerContext >>> - JavaControlUtils creates a ControlContainerContext >>> - @Control annotated fields are initialized with ControlBean instances >>> - JavaControlUtils uninitializes the ControlContainerContext >>> - onCreate is called on the JPF >>> - method invokes a method on the @Control annotated field >>> - any usages of the CCC returned by >>>ControlThreadContext.getContext() will NPE >>>- unit of work >>> - any usages of the CCC returned by >>>ControlThreadContext.getContext() will NPE >>>- end request >>> >>>I interpret the contract of the ControlContainerContext to be such >>>that the ControlContainerContext exists for the duration of an >>>execution scope. In the case of a JPF, this is the duration of a >>>ServletRequest. Suppose that you could argue that it's something else >>>-- one JPF action, one page, etc. >>> >>>The issue of calling beginContext / endContext on multiple >>>ServletBeanContext objects is that there would be two or more of these >>>while executing one Page Flow. The adverse affect of doing this is >>>that: >>> >>>- it's architecturally weird. Controls are inherently nested, and in >>>the current implementation, the root of the tree is being removed. >>>I'm not sure it's even possible to replace the root, and if it was, >>>it's not clear how that fits into the control lifecycle. >>>- any services / resources bound into the CCC need to be re-bound for >>>the contract with the contained controls to be valid >>> >>>I'm sure that there are other complex usage scenarios that I'm not >>>thinking of that could cause problems here, but that's my high-level >>>take. >>> >>> Hope that helps clarify things... >>> >>>Eddie >>> >>> >>> >>> >>>On 11/14/05, Rich Feit <[EMAIL PROTECTED]> wrote: >>> >>> >>> >>> >>>>OK, just to make sure I understand the issue, here's what I know: >>>> >>>> - If you go to FlowControllerFactory and get a page flow instance >>>>*outside of a direct request to that page flow*, it'll look up the >>>>ServletBeanContext from the session and call beginContext, initialize >>>>the page flow, and call endContext. >>>> >>>> - If, later in the same request (but not necessarily on the same >>>>code path), you run an action on the page flow by calling >>>>PageFlowUtils.strutsLookup(), it'll look up the ServletBeanContext from >>>>the session, call beginContext, run the action, and call endContext. >>>> >>>>...and here's what I don't know: >>>> >>>> - What's the adverse affect of possibly calling >>>>beginContext/endContext on ServletBeanContext twice within the same >>>>request? Is this really not a legitimate thing to do? >>>> >>>>Again, I'm just trying to understand here. I'm not necessarily arguing >>>>against this -- I might be missing the reason that this is an invalid >>>>thing to do. >>>> >>>>Rich >>>> >>>>Eddie O'Neil wrote: >>>> >>>> >>>> >>>> >>>> >>>>>I just reopened BEEHIVE-1003 and will attach a control demonstrating >>>>>the problem shortly. Basically, the problem is that the JPF Control >>>>>Container creates the ControlContainerContext (just a >>>>>ServletBeanContext) for long enough to initialize the controls -- but >>>>>that's it. In this case, the CCC needs to exist for the duration of >>>>>the interaction with the JPF. >>>>> >>>>>To fix it, the runtime needs to move from this initialization model: >>>>> >>>>>- initialize control context >>>>>- initialize controls >>>>>- uninitialize controls >>>>>- perform some unit of work >>>>> >>>>>to this one: >>>>> >>>>>- initialize control context >>>>>- initialize controls >>>>>- perform some unit of work like running actions, pages, etc >>>>>- uninitialize control context >>>>> >>>>>As I mentioned in the bug, I'll add a test that ensures that the >>>>>ServletBeanContext works in the usual JPF use-case -- not involving >>>>>direct invocation of the FlowControllerFactory. >>>>> >>>>>One other thing...it would be great if the test could uninitialize >>>>>the Page Flow and controls at the end of the Servlet in order to have >>>>>a full, end-to-end test of the lifecycle running with factory based >>>>>creation. >>>>> >>>>>Hope that helps explain things better... >>>>> >>>>>Eddie >>>>> >>>>> >>>>> >>>>>On 11/11/05, Rich Feit <[EMAIL PROTECTED]> wrote: >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>>All the public methods in FlowControllerFactory for creating page flows >>>>>>also store the page flow in the session. It's pretty similar to hitting >>>>>>the page flow directly. When another page flow is hit, or when the >>>>>>session expires, the controls in the page flow will be cleaned up. >>>>>> >>>>>>Rich >>>>>> >>>>>>Carlin Rogers wrote: >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>>Eddie, >>>>>>> >>>>>>>Thanks for the help. The scenario/interest for doing this is the case of >>>>>>>something like a portal that creates an instance of a page flow using the >>>>>>>FlowControllerFactory. >>>>>>> >>>>>>>Good questions about the contract to go through the entire life cycle of >>>>>>>the >>>>>>>controller in this case and the concern for leaks. I'm not sure about the >>>>>>>answers... Maybe I can defer to Rich here. >>>>>>> >>>>>>>Yes, I can definitely look at improving the test to drive the page flow >>>>>>>through its life cycle. >>>>>>> >>>>>>>Regards, >>>>>>>Carlin >>>>>>> >>>>>>>On 11/11/05, Eddie O'Neil <[EMAIL PROTECTED]> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>>Carlin-- >>>>>>>> >>>>>>>>I've been looking at this fix and at the test and am curious about a >>>>>>>>couple of things. First, why is this an interesting thing to do -- >>>>>>>>ie, what's the use case? If a controller is created outside of the >>>>>>>>runtime, isn't there a contract that says that the flow controller >>>>>>>>will be driven through its entire lifecycle? >>>>>>>> >>>>>>>>The problem is that simply by creating the page flow, controls will >>>>>>>>be initialized, but without driving the controller through its >>>>>>>>lifecycle, they're never torn down. So, what's the contract between >>>>>>>>the caller and the JPF instance here? I've not tried this specific >>>>>>>>case, but if the control being used from the test accessed a DB, it >>>>>>>>would leak ResultSets in this case. Similar things could be true for >>>>>>>>other control types. >>>>>>>> >>>>>>>>Also, can we expand the test to ensure that the right thing happens >>>>>>>>to the control bean context when the JPF *is* driven through its >>>>>>>>lifecycle? >>>>>>>> >>>>>>>>Thanks... >>>>>>>> >>>>>>>>Eddie >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>---------- Forwarded message ---------- >>>>>>>>From: [EMAIL PROTECTED] <[EMAIL PROTECTED]> >>>>>>>>Date: Nov 11, 2005 12:37 AM >>>>>>>>Subject: svn commit: r332482 - in /beehive/trunk/netui: >>>>>>>>src/pageflow/org/apache/beehive/netui/pageflow/internal/ >>>>>>>>test/webapps/drt/coreWeb/WEB-INF/ >>>>>>>>test/webapps/drt/coreWeb/miniTests/createPageFlow/ >>>>>>>>test/webapps/drt/src/miniTests/createPageFlow/ test/webapps/drt... >>>>>>>>To: [email protected] >>>>>>>> >>>>>>>> >>>>>>>>Author: crogers >>>>>>>>Date: Thu Nov 10 23:37:13 2005 >>>>>>>>New Revision: 332482 >>>>>>>> >>>>>>>>URL: http://svn.apache.org/viewcvs?rev=332482&view=rev >>>>>>>>Log: >>>>>>>>Fix for http://issues.apache.org/jira/browse/BEEHIVE-1003 : Calling >>>>>>>>FlowControllerFactory.createPageFlow() outside of page flow request >>>>>>>>processor and page filter does not initialize controls correctly. >>>>>>>> >>>>>>>>tests: drt, bvt in netui (WinXP) >>>>>>>> >>>>>>>> >>>>>>>>Added: >>>>>>>>beehive/trunk/netui/test/webapps/drt/coreWeb/miniTests/createPageFlow/ >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/coreWeb/miniTests/createPageFlow/Controller.java >>>>>>>>(with props) >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/coreWeb/miniTests/createPageFlow/test.jsp >>>>>>>>(with props) >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/ >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/CreatePageFlowServlet.java >>>>>>>>(with props) >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/controls/ >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/controls/Portfolio.java >>>>>>>>(with props) >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/controls/PortfolioControl.java >>>>>>>>(with props) >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/controls/PortfolioControlImpl.java >>>>>>>>(with props) >>>>>>>> >>>>>>>>beehive/trunk/netui/test/webapps/drt/src/miniTests/createPageFlow/controls/Stock.java >>>>>>>>(with props) >>>>>>>>beehive/trunk/netui/test/webapps/drt/testRecorder/tests/CreatePageFlow.xml >>>>>>>>(with props) >>>>>>>>Modified: >>>>>>>> >>>>>>>>beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/internal/JavaControlUtils.java >>>>>>>>beehive/trunk/netui/test/webapps/drt/coreWeb/WEB-INF/web.xml >>>>>>>>beehive/trunk/netui/test/webapps/drt/testRecorder/config/testRecorder- >>>>>>>>tests.xml >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>>>> >>> >>> >>> > > >
