comments inline ...
Anil Gangolli wrote:
Allen:
What you've got is not the pattern I had in mind, but I can't say for
sure whether it is good or not. It is not a pattern I'm familiar with
at all, so I'm not sure.
Based on your explanation below I think that the alteration to the
current plan that I am proposing will meet very nicely with the pattern
you are used to. There are still a couple noticeable differences, but I
think those differences are minor and can probably be tried and altered
as we move forward depending on how we think things work out.
I've used Spring's basic dependency injection/bean factory support, but
never used Spring for transaction demarcation, so I don't have
first-hand experience; I take it Matt does, and likes it. He says he's
biased, but if that's just the bias of prior experience, I don't think
such bias needs to be discounted. Roller is not characteristically
different in structure from the typical webapp, and the typical webapp
built with AppFuse, so there's probably something to be noted.
I am not at all opposed to using Spring, however I don't think we should
commit to it right now. I think the work we are doing now is good for
Roller regardless of whether we use Spring or not. So I would like to
keep Spring as a consideration, but I don't plan to involve it right now.
I'm biased by my own experience. I've used the pattern below before
and I'm comfortable with it. I've described it in pieces but not as a
whole, so I'll try to do that here (see below). The only thing I really
feel strongly about is that we should adopt a conventional pattern and
not invent here. I feel the pattern below is conventional; described
here by me, but not invented by me. To me, the pattern in the new
backend branch feels mostly invented and feels ad hoc.
Certainly we are all biased by our own experiences, but I'm not sure
that the pattern I had proposed is invented or ad hoc. Admittedly I
have little experience with Hibernate and other ORM tools because I
haven't needed them in the past. I believe my pattern is a bit more
procedural and therefore doesn't work as well with an ORM tool like
Hibernate and that's why I am trying to revise my proposed pattern now.
The primary metric for "simplicity" I would use here is: a smaller
number of places where the code knows about and deals with Hibernate
Sessions or transactions at all. I think the pattern in the new backend
branch is not as good along this metric. I think the pattern below is
simpler and cleaner.
Matt's suggestion of using Spring's declarative (I'm assuming)
demarcation also follows an established convention, and achieves similar
or greater simplicity in this metric than my proposed pattern. There
are even fewer places that know about the Session or Transaction at
all. More does have to be expressed in configuration though. I
really like the parts of Spring I have used, and I'd like to use more of
Spring in Roller, but I still think we should defer a major jump to
Spring for a major architectural revision of Roller. I think the
pattern below is a good intermediate step. I can see going from the
pattern below to Spring later.
I agree. I think the pattern we are moving towards will apply
regardless of whether we decide to use Spring later or not. If we
decide to add Spring transaction management later it should be fairly easy.
What's similar about the pattern below and the one in the new backend
branch is that there is a flush/commit between model updating operations
and the view. That sounds like what you're referring to.
The similarity pretty much ends there though. The pattern below has no
transaction-aware code in the model layer code itself and, with proper
class structure, it has only a few places in the entire codebase where
there is any transaction-aware code at all. Most of the code in both
app and model layers knows nothing about the Hibernate Session or
transaction demarcation; it expects it but is not explicitly cognizant
of it.
With the right base classes, this pattern should have very few places in
the code that know about Sessions and transactions. I'm talking about a
total of something like 4 or 5 places, plus a few exceptional background
tasks that need to manage multiple transactions per execution on their own.
I believe I am advocating for essentially the same thing, with the one
exception of the Roller.flush() method. I will explain how I think this
method can be used below.
--a.
---------------------
Here is an attempt at a reasonably complete description of the pattern
(not my own pattern):
The pattern involves explicit code in the following six aspects, which
with proper class structure occur at only a few cut points in the codebase:
(1) Have a ServletFilter create the Hibernate Session and begin the
initial Transaction before chaining. Associate the Session with the
thread context. In a finally clause, the filter will always rollback the
transaction if it has not already been committed or rolled back. The
filter expects the app layer to handle commits on success.
This part I disagree on ever so slightly. I prefer that the Session and
Transaction be initialized lazily rather than explicitly opened on each
new request. That shouldn't change anything about the functionality or
complexity of the code, it would merely be moving the code to a slightly
different location.
I think the biggest reason to do this is that it seems silly to open a
Session/Transaction for requests that don't need one. This could
technically be considered a scalability issue since the # of requests
you can handle would be tied to the # of db connections you can support.
I don't think we lose anything by opening the Session/Transaction
lazily when they are actually needed.
I fully agree about the part where the Session is managed in a thread
context and we can continue using the SessionFilter that we have now
which explicitly closes any open Sessions at response time via a call to
Roller.release().
(2) The model layer does nothing at all about transaction demarcation.
It always assumes a transaction. You see no begin or commit code in the
model layer. No explicit cognizance of Session or transaction.
I have a comment about this below (4).
(3) For view web app operations, the app layer also does nothing at all
about transaction management. It always uses the current session and
transaction. No explicit cognizance of Session or transaction.
Agreed.
(4) For model-updating web app operations, the app layer gets the
Session and does a flush and commit at successful completion, just
before forwarding to the normal success view. After committing, a new
transaction is started immediately by that same code. On
exception/error, the app layer should do a rollback; it could propagate
the exception, but typically it needs to return a meanngful error
response view to the user. Explicit cognizance of the Session and
transaction is required in this location, but I think this can be
limited to one cut point in the code if the action classes are
structured properly. This code could be isolated in one base class that
is used for all model-updating web app actions and encapsulates logic to
commit and forwards to a regular view in the success case or rollback
and forward to an error view for the error/exception case.
This part I disagree on very slightly, for the first pass at least. I
think it is okay to ask our model objects (struts actions, servlets,
etc) to perform a simple Roller.flush() explicitly before finishing
their job and choosing their view.
I agree that it is a little more annoying to require a call to
Roller.flush() in each model operation rather than trying to set things
up so that all model operations get a flush automatically, but I think
that is a fairly small compromise. There are a couple decent reasons
why I think this is useful ...
1. what if a model operation wants to modify an object but not save it?
that would be impossible if the flush happens automatically.
2. what if we want to choose different options for the view based on a
variety of exception possibilities? we may have cases were we don't
treat all exceptions as equal.
It is also easier to go from the explicit flush() calls to your implicit
model than vice versa.
(5) For web service operations the servlet filter is still used, but a
flush/commit (and rollback for exception cases) is needed in the
service() method of the entry point servlet. You generally need to
return an error response to the caller but also rollback. For
Axis-based web services, this is typically done in extension of
AxisServlet. In Roller there is RollerXMLRPCServlet. If there are
other similar entry point servlets in Roller, they'd be needed there.
I think I still prefer the situation that I proposed where any time an
Exception is generated from a call to Roller.flush() that a rollback is
automatically applied. This way all that the model knows is that for
some reason their operation failed and technically any changes they made
did not happen. I think this is important because to me a rollback is
purely a persistence concept which shouldn't exist outside the
persistence layer.
(6) For background tasks, a base class for normal background tasks can
do the same thing that is done in the filter (in 1). Normal means only
one transaction is needed per execution. Some background task
operations will involve processing a potentially long list of individual
objects, and a problem with processing one object should not affect
others. In this pattern, the Hibernate Session/transaction management
required for this is done by the specific task code. The only place I
can think of where Roller might need this is during ping queue
processing, where each ping is handled separately. There might be
something similar in the feed processing task in Planet, where one
probably wants to handle each feed and possibly each entry from the feed
individually.
Building on the comment I made on (5), I am not sure I agree that the
external task should manage the Session/Transaction explicitly. I agree
that they need to manage calls to Roller.flush(), but everything else
should be handle implicitly by the given persistence implementation.
So when it is all said and done the details of what I am proposing would
be as follows.
We use the newbackend that I have outlined in my proposal and coded in
the roller-newbackend branch with these changes to come:
1. Remove all lines in XXXManager methods which trigger a commit. This
means that any call to an XXXManager method is not automatically committed.
2. Add a new flush() method to the Roller interface. This method will
trigger a commit on the persistence implementation and if an Exception
occurs a rollback will happen automatically.
3. Go back into the Roller model code and add in calls to
Roller.flush(). This will basically just replace all the current calls
to Roller.commit() in the current trunk.
I think that is all that is required to apply this modification to the
current backend refactoring proposal.
-- Allen
----- Original Message ----- From: "Matt Raible" <[EMAIL PROTECTED]>
To: <[email protected]>
Sent: Friday, April 14, 2006 11:53 PM
Subject: Re: new backend status and questions
Simplicity is king in my book. I still think there's a fair amount of
value in using Spring to define the transaction attributes for the
middle-tier. Of course, I'm biased. ;-)
Matt
On 4/14/06, Allen Gilliland <[EMAIL PROTECTED]> wrote:
okay, the status on the backend refactorings is pretty good and i've got
all the major functionality fixed up and unit tested. i've also been
testing things out from the GUI along the way to make sure the
presentation layer was working as well. everything should be working
except for folders/bookmarks and possibly some referers stuff.
now for the more important part ...
as i have been reworking things i have slowly been warming up to Anil's
constant pleas that we provide some sort of transaction demarcation
available outside of the business layer. i still believe strongly that
we should attempt to hide all knowledge of persistence actions from the
presentation layer, however, based on the way that Hibernate wants us to
do things I think that providing a simple flush method may be more
appropriate than the strict boundaries that i originally proposed.
while this will require me to rollback a couple of the changes i have
made i still think that most of the work is applicable, notably that we
have removed the PersistenceStrategy and given more freedom to
persistence implementors, removed persistence specific methods from our
domain model (save() and remove()), and that we have removed the
extraneous transaction methods (begin(), setUser(), getUser(), and
rollback()) from being accessible to the presentation layer.
so, what i would propose is that we add a new method to the Roller
interface, flush(), which will trigger a database flush for any changes
that have happened within the current running transaction. all other
transaction operations (begin() and rollback()) are handled behind the
scenes by the persistence implementation. transactions are
automatically started when a Session (i.e. Request) begins. after any
flush occurs a new transaction begins to prepare for further changes.
if there is ever a problem caused by a flush then a rollback happens
automatically. so the work flow would look like this ...
incoming request
session begins, transaction begins
objects are queried for and updated
flush()
-- optionally make more changes and flush() again
response committed
at the end of the day this model places a greater emphasis on having a
properly structured domain model, which i believe is a good thing.
so, thoughts anyone? Anil, does this sound more like what you had in
mind?
-- Allen
