beehive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carlin Rogers <carlin.rog...@gmail.com>
Subject Re: fix for BEEHIVE-1003
Date Wed, 16 Nov 2005 07:43:27 GMT
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 <richfeit@gmail.com> 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 <richfeit@gmail.com> 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 <richfeit@gmail.com> 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 <richfeit@gmail.com> 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 <ekoneil@gmail.com> 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: crogers@apache.org <crogers@apache.org>
> >>>>>>>>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: commits@beehive.apache.org
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>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
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message