+1 for jukkas proposal in general, thanks! i share michael's concerns WRT ContentTree.commit/refresh. as michael already noted, Item.refresh/save are deprecated so IMO we need not care to implement them 'properly'. the current jackrabbit implementation just delegates to Session.save/refresh which should be good enough for oak as well.
cheers stefan On Wed, Apr 18, 2012 at 11:16 AM, Michael Dürig <[email protected]> wrote: > > > On 18.4.12 8:13, Jukka Zitting wrote: >> >> Hi, >> >> On Wed, Apr 18, 2012 at 7:47 AM, Michael Dürig<[email protected]> wrote: >>>> >>>> - ContentSession.getCurrentContentTree() returns a snapshot of the whole >>>> tree. Wouldn't it be useful to be able to get snapshots of just a >>>> sub-tree? Like it is currently possible with NodeStore.branch()? >> >> >> Not sure if there's a strong use case for that. You can achieve the >> same thing by acquiring the root and traversing down to the subtree >> you want. I'd rather implement such things in a helper class than >> burden the API with non-orthogonal methods. > > > Ok works for me. > > >> >>>> - Having a refresh() method on ContentTree is troublesome. What would it >>>> refresh if you call it on a sub-tree? That sub-tree only? The underlying >>>> implementation would then effectively have to manage a MK revision for >>>> each node (i.e. SubTree) of the content tree. Worse, on commit, against >>>> which revision would such a tree be committed? In the current >>>> implementation the refresh method would go into the Branch class. >>> >>> >>> The same actually goes for the commit method. I think it is somewhat miss >>> placed on ContentTree too. What about introducing a ContentRoot extends >>> ContentTree class which contained said methods? An instance of that class >>> would then be returned from ContentSession.getCurrentContentTree(). >> >> >> Yep, I was a bit undecided on this point. I'm fine with putting these >> operations to ContentSession, for example like this: >> >> ContentSession session = ...; >> ContentTree tree = ...; >> tree = session.getCurrentContentTree(tree); // preserve changes >> from given tree >> tree = session.commit(tree); // commit changes and return new >> state of the tree > > > Ok then. I'd prefer that to having commit() and refresh() on ContentTree. > > >> >> The main reason I put the methods on ContentTree was to allow >> straightforward implementation of Item.refresh() and Item.save() in >> JCR, but I'm not sure if we even want to fully support those features >> at the Oak API level. > > > Right. However Item.save() and Item.refresh() are both deprecated now in > JCR. And given the complexity implementing these, I'd rather leave them out. > > Michael > >> >> BR, >> >> Jukka Zitting > >
