jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nicolas " <nto...@gmail.com>
Subject Re: Adding two iterator methods to PersistenceManager interface
Date Wed, 14 Feb 2007 21:45:00 GMT
Hi Jukka,

Thinking about it, it sounds obvious, which is usually a sign of a good
idea. Just one detail and one proposition:

Instead of:
   interface VisitablePersistenceManager implements Persistencemanager

I think you meant:

interface VisitablePersistenceManager extends Persistencemanager

Am I correct?

However, I would propose to decouple the Visitable interface from
PersistenceManager: visitor pattern are quite common so some other extension
will need it later. The interface would become:

   interface VisitableItemStateCollection

I think it is more generic and clearer.

A visitable persistence manager would implement two interfaces:
PersistenceManager and VisitableItemStateCollection. Do you agree?

BTW in which package should the interface be located?

Nicolas

On 2/14/07, Jukka Zitting <jukka.zitting@gmail.com> wrote:
>
> Hi,
>
> On 2/14/07, Nicolas <ntoper@gmail.com> wrote:
> > I wish to add two methods to PersistenceManager Interface. They would
> return
> > an Iterator object of the content of the persistence manager. They would
> be
> > optimized for iterating on all the NodeState and PropertyState stored in
> a
> > specific persistence layer.
>
> You tell us how but forgot to mention why. :-) The rationale for this
> change is to allow the backup tool to efficiently retrieve all the
> content within a persistence manager.
>
> Some comments:
>
> 1) I don't think you need two separate methods for this, as you could
> just request all ItemStates. Otherwise an implementation like the
> XMLPersistenceManager needs to traverse the underlying storage twice.
>
> 2) I now I originally suggested the Iterator approach, but after
> thinking more about this I think the Visitor pattern fits the
> requirements better. A returned Iterator gives the control of the
> operation to the client and might cause a returned ResultSet or
> something similar to be referenced indefinitely. The Visitor pattern
> guarantees that any resources required for the item state traversal
> are only needed for the duration of the method call. My
> counterproposal would thus be:
>
>     /**
>      * Accepts the given item state visitor to visit <em>all</em> the
> item states
>      * within this persistence manager. The order in which the item
> states are visited
>      * is not specified. The visitor should not try to call back to
> this persistence manager
>      * during the visit.
>      *
>      * @param visitor item state visitor
>      * @throws ItemSateException if the item states could not be traverses
>      */
>     void accept(ItemStateVisitor visitor) throws ItemStateException;
>
> with:
>
>     /**
>      * Item state visitor. Used to traverse all the item states within a
>      * persistence manager.
>      */
>     interface ItemStateVisitor {
>
>         /**
>          * Visits a node state.
>          *
>          * @param state node state
>          */
>         void visit(NodeState state);
>
>         /**
>          * Visits a property state.
>          *
>          * @param state property state
>          */
>         void visit(PropertyState state);
>
>     }
>
> This even solves quite elegantly the typing issue related to point 1)
> above.
>
> > Since I do not expect all persistence manager to implement this new
> > interface, I will write a fallback generic iterator method in
> > AbstractPersistenceManager. It will iterate on the repository graph
> (using
> > load and getChildNodeEntries methods). Hence, this feature wille be
> provided
> > to all persistence manager.
>
> 3) It's not guaranteed that all persistence managers extend the
> AbstractPersistenceManager class. It's better if you create an
> extension interface like VisitablePersistenceManager:
>
>     /**
>      * Persistence manager that supports traversal of the all item
> states by a visitor.
>      */
>     interface VisitablePersistenceManager implements Persistencemanager {
>
>         void accept(ItemStateVisitor visitor) throws ItemStateException;
>
>     }
>
> and use the fallback in case a persistence manager doesn't implement
> this extension:
>
>     ItemStateVisitor visitor = ...;
>     PersistenceManager manager = ...;
>     if (manager instanceof VisitablePersistenceManager) {
>         ((VisitablePersistenceManager) manager).accept(visitor);
>     } else {
>         // use the fallback implementation
>     }
>
> BR,
>
> Jukka Zitting
>

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