beehive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eddie O'Neil <ekon...@gmail.com>
Subject Re: fix for BEEHIVE-1003
Date Tue, 15 Nov 2005 16:17:57 GMT
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
View raw message