You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by Nicolas <nt...@gmail.com> on 2007/02/14 18:26:23 UTC
Adding two iterator methods to PersistenceManager interface
Hi,
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.
Iterating directly at the persistence manager level allows efficicient
optimisation based on the underlying data-model and not according to the
graph model of the repository. Ie: optimized SQL requests for the
DbPersistenceManager or direct file system traversal for the file based
persistence layers. This way iterating on the content of a repository will
be a lot faster.
/**
* Returns an iterator over the PropertyState accessible by this
persistence manager.
* There are no guarantees concerning the order in which the elements
are returned
* @return
*/
Iterator iteratorPropertyState();
/**
* Returns an iterator over the NodeState accessible by this persistence
manager.
* There are no guarantees concerning the order in which the elements
are returned
* @return
*/
Iterator iteratorNodeState();
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.
On my side, I will be able to implement a low XML format for the backup tool
solving different issues at the same time: versionning constraints and
importXML high overhead. From a more general point of view, it will allow
easier and more performant traversal of content of a repository.
If you agree on this addition, I will put on JIRA in the next few days the
codes for your opinion.
Do you agree on this update and this process?
BR,
Nico
my blog! http://www.deviant-abstraction.net !!
Re: Adding two iterator methods to PersistenceManager interface
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On 2/14/07, Nicolas <nt...@gmail.com> wrote:
> Instead of:
> interface VisitablePersistenceManager implements Persistencemanager
>
> I think you meant:
>
> interface VisitablePersistenceManager extends Persistencemanager
>
> Am I correct?
Yep, my mistake.
> 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?
Agreed, good point.
> BTW in which package should the interface be located?
It's not directly related to persistence managers, so o.a.j.core.state
along with the other ItemState stuff would be best.
BR,
Jukka Zitting
Re: Adding two iterator methods to PersistenceManager interface
Posted by Nicolas <nt...@gmail.com>.
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 <ju...@gmail.com> wrote:
>
> Hi,
>
> On 2/14/07, Nicolas <nt...@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
>
Re: Adding two iterator methods to PersistenceManager interface
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On 2/14/07, Nicolas <nt...@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