All, Thanks for the help and thoughtful input. Yes, I'm good with option #1. I'll go ahead and revert my changes to FCF to extract the control container initialization. Then I'll go back and add some documentation to indicate the expected usage/contract. For now, I'll leave some of the test files and rework it in a follow up revision to add the initialize / uninitialize CCC calls.
Carlin On 11/15/05, Rich Feit <[EMAIL PROTECTED]> wrote: > > 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 > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > > >
