On Mar 26, 2006, at 8:34 PM, Allen Gilliland wrote:
I have updated my backend refactoring proposal to include a bit
more implementation details.
http://rollerweblogger.org/wiki/Wiki.jsp?
page=Proposal_BackendRefactorings
I have basically listed all the manager classes which use the
database and then listed each method of those classes which
involves writing data.
I still stand behind my proposal that we should remove all
persistence specific logic from existing outside the business/
presentation layer. I am planning to begin work on this for April
and hopefully will get it completed for Roller 2.3
The more I think about this idea, the more I like it. I'd like to see
the Roller backend be a self-contained API and this appears to be the
way to achieve that.
But I'm still opposed to doing this work in the trunk. It involves
dramatic sweeping changes to the code base and requires that almost
every Struts action be rewritten. It should done done in a separate
branch and not merged back until it's been tested an evaluated.
Plus, I've got a couple questions about the plan:
1) You propose to remove the setUser() and getUser() methods from the
Roller interface. What is the plan for determining that the current
user has permission to perform an operation?
2) You mention that some operation are "single step persistence
operations which do not require special transaction considerations."
What does that mean in practice? Does each of these operations do
it's own begin() and then end with a commit() or rollback()? Will
these single step operations be called from the multi-step operations?
3) What is the plan for unit testing the changes to the Struts
Actions and other presentation classes?
4) How did you come up with the list of changes? I assume you
reviewed each of the Struts actions. Did you also review the
MetaWeblog API and Atom protocol implementations?
- Dave
-- Allen
Allen Gilliland wrote:
Anil Gangolli wrote:
Allen:
Your suggested pattern is workable, but you'll need to make begin
() and commit() conditional and smart about transaction
demarcation; it is a common pattern to conditionally begin a
transaction only if there isn't one on the current thread, and
you'll find that in many infrastructures (see my comments on
Section 2 in the design proposal).
I agree with this and believe it's fairly easy to do.
What happens if you don't have that logic? You'll find that the
operations that you choose to make transactional can't be easily
composed, which is a very awkward property to have. If you ever
need to compose two operations, you would need to write a third
method that includes all of the operations of the originals, and
not just calling the two original methods, but copying their
innards. I actually went through a period of trying to code apps
this way in the 90's before infrastructures like EJB started to
normalize the conditional demarcation patterns in Java. Lesson
learned.
You are correct, that is absolutely a possible ramification and we
could end up making it more difficult to properly wrap some steps
together in a transaction. However, I would also argue that the
reverse scenerio is true for our current setup using more open
transaction demarcation. Allowing business/presentation code to
control transaction demarcation allows for transactions to become
overly inclusive and actually end up including more operations
than they should.
The patterns for transaction demarcation in the dispatch layers
involve three parts: (1) demarcation for requests on a per-
request basis in a Filter or some other centralized dispatch
point (e.g. in Axis or Struts apps in extensions of these
servlets wrapping the entry points) (2) for scheduled tasks,
wrapping the run() method in a base class (3) providing a
mechanism to get an independent transaction for individual units
of work at smaller granularities (REQUIRES_NEW style semantics),
which I expect would be used very rarely in Roller. Patterns
like this work well for two-tier web apps like Roller where the
persistence container and the webapp container are the same.
They don't work (or require much stronger transaction
infrastructure) if you separate the webapp container from the
persistence container which does not apply in Roller's case.
In order to get a better grasp on some Hibernate details I am
currently skimming through "Hibernate in Action" and they say ...
"We merely observe that, in our opinion, overengineering has been
endemic in the Java community, and overly ambitious multilayered
architectures have significanly contributed to the cose of Java
development and to the perceived complexity of J2EE. On the other
hand, we do agree that a dedicated persistence layer is a sensible
choice for most applications and that persistence-related code
shouldn't be moxed with business logic or presentation." (page 296)
In their examples for designing a layered application (webapp
specifically) they do not provide transaction demarcation outside
of the DAO layer. Of course this is only a single opinion and
does not mean it's what's best for Roller, but I would say that my
opinions are more in line with the sentiments from the quote above.
Speaking more specifically about applying transaction demarcation
at points in the dispatch layer ... I think I still see this as
allowing persistence logic into the business/presentation layer.
The other reason why this makes me nervous is that I think it
creates some potential for two situations ...
1. catching a persistence exception too late. if the transaction
demarcation doesn't actually commit the transaction data until a
while after the app thinks the data has been written then we may
end up putting ourselves in a situation where we catch an
exception after we have already commited some of the response and
are no longer in a situation where we can respond appropriately.
This seems especially true if we consider each request/response
cycle as a single transaction and only flush/commit the data in a
filter as we exit the app.
2. commiting data changes we don't want, specifically users being
bad with their templates. I suppose our wrapper objects may take
care of the potential problem with users making changes to their
data that we don't want, but there may also be an issue of app
code doing the same thing. It has always seemed ideal to me to
only commit changes specifically when needed. If a filter simply
commits data at the end of the request/response cycle there is no
guarantee that someone hasn't modified an object that is not meant
to be saved back to the database.
Like your proposal, the dispatch-level patterns remove begin()
and commit() from app code, and we should end up with only a few
places that call them.
I like the idea of the dispatch-level pattern much more than our
current pattern. After I spend a bit more time on the proposal
and get things more flushed out I think I'll have a better idea of
which approaches make the most sense.
-- Allen
--a.
Allen Gilliland wrote:
On Thu, 2006-03-09 at 14:21, David M Johnson wrote:
On Mar 9, 2006, at 5:02 PM, Allen Gilliland wrote:
i feel like we are still a little disconnected here. my
approach will not alter anything about our ability to use the
open session in view pattern. the only difference i see is
the difference between these 2 code blocks ...
old way:
begin();
website.store();
commit();
new way:
WeblogManager wmgr = roller.getWeblogManager();
wmgr.saveWebsite(website);
But what if saving a website is just one step in a multi-part
transaction? OK, scratch that. If you start by doing the
analysis and designing one of the new Manager interfaces, I
think we'll have a much better understanding of this proposed
refactoring.
i'll just give the short answer now. usually things that are
multipart operations are still part of a single logical
operation. in fact, we already do this in some places. take a
look at the UserManager.createWebsite() method. that is a
logical method which wraps a series of persistence operations.
so the difference would be ...
old way (in struts action or servlet):
begin();
// setup website
website.store();
// setup template
template.store();
// setup category
category.store();
// etc etc
commit();
new way (logical method in manager class):
UserManager umgr = roller.getUserManager();
umgr.createWebsite(newWebsite, possible, other, data);
when i use the new way i only care wether or not the operation
succeeded and i don't have to think about the persistence
implications because they are hidden from me. now if it turns
out that we have way too many transactions that would need their
own logical method in a manager class then maybe this approach
won't work, but i believe that in most request/response cycles
the operations are pretty isolated and simple. in most of them
you are only creating/updating/deleting a single object at a time.
-- Allen
- Dave
