Juha Lindfors wrote:
> Would seem to me that the current implementation just suffers from mixing
> too many responsibilities into one class. We need to separate the different
> roles of a persistence manager and that of an O/R mapper behind two
> different interfaces.
>
> Currently the JAWSPersistenceManager does both the callbacks to the
> EntityContainer and deals with the JDBC calls to the database (this was how
> it worked before this weekend's changes anyhow). I believe marc is correct
> to point out that this is not a good solution since it leaves the
> responsibility of the callbacks to the O/R mapper plugin authors (that are
> required to have the knowledge of jBoss to be able to provide container
> management part as well). In addition, this leaves a door open for writing
> "bad" plugins that can fuck up the container by not doing the required
> calls or doing them in unexpected ways. I think we should attempt to
> isolate the plugins more in order to prevent this. Otherwise we might end
> up having to review all the possible persistence plugins to make sure they
> do work correctly.
>
> So we should provide an implementation of a PersistenceManager that does
> the callbacks to the container in correct places and that requests the
> services of the O/R mappers when needed. This would require us to define an
> interface for the O/R mappers to implement, thus separating the
> "management" part of doing the callbacks from the JDBC stuff of mapping
> objects to relational data. JAWS would be one such mapper and it would get
> its requests of service from jBoss PersistenceManager whenever it requires
> the operations. The same manager could also delegate the O/R services to
> other modules, such as Cocobase, when they too implement this yet to be
> named interface (basically doing a wrapper that implements it, and not
> having to know about the container callbacks).
>
> This would separate the JDBC calls from container calls and still maintain
> container's role as a mediator that receives callbacks from the plugins (or
> at least from the managers that make use of the plugins).
Agree, with the addition that I have changed my mind and would rather go
with Marc's suggestion to put this code in EntityContainer. Since there
is (AFAICT) only one way to implement these callbacks it doesn't seem
necessary to have it in a separate place, which would introduce more
complexity to the code.
There are a few places that needs to be changed:
* For create the EntityContainer should do ejbCreate, and handle the BMP
and CMP cases differently. Then call PM (which may be an O/R mapper),
then call ejbPostCreate. The EntityContainer should be responsible for
hooking up the instance with a EJBObject from the ContainerInvoker
before returning.
* For finders the PM should be responsible for returning one or more
primary keys. Again, the EntityContainer hooks up these with EJBObjects
before returning
* Two methods, activate() and passivate() should be added to
EntityContainer (and StatefulSessionContainer) which call the PM and
then ejbActivate, and ejbPassivate and the PM (respectively). The caches
should be changed to call the EntityContainer to ask for
activation/passivation instead of calling the PM directly.
I think this should be enough. This would provide a good refactoring of
the code, and will make it easier and less error-prone to code PM's.
Thanks to Marc and Juha for catching this and providing an interesting
(although a bit heated) discussion on the subject.
Sounds ok? Marc, if you agree (and I think you do), can you please make
the above changes?
regards,
Rickard
--
Rickard �berg
@home: +46 13 177937
Email: [EMAIL PROTECTED]
http://www.telkel.com
http://www.jboss.org
http://www.dreambean.com
