Hi, I like the 2nd idea better, as you get direct feedback when operating on the map, e.g. if a property can't be set.
however, the writing map methods are: - put() - putAll() - remove() - clear() and I would throw a IllegalStateException if they fail (clear() will most likely never succeed, since jcr:primaryType is not deletable :-) and there are also indirect operations that modify the map: - keySet().* writing ops - values().* writing ops - entrySet().* wrirting ops so given the complexity, I think it's better to delay the writeback until update() is called. You loose the fine granular information of what failed, but in my experience the user cannot really do anything when e.g. a setProperty() fails, but just retry. (and there is stil the workaround to update() after each put()) Additionally, the PersistException (or whatever update() throws) could have a 'getFailedOperations()' method that list the failed operations. regards, toby On Wed, Jul 18, 2012 at 12:30 AM, Carsten Ziegeler <[email protected]> wrote: > Hi, > > I'm currently working on CRUD support for Sling based on the resource > API. Current trunk contains already a version of it. > I'm wondering what the best api for modifying resources is. > > Right now, the idea is to adapt a resource to ModifiableValueMap which > is an extension of the ValueMap. Like the PersistableValueMap this map > collects all changes internally. It provides an update() method which > pushes the changes into the transient persistence layer (e.g. jcr > session). A call to the resource resolver finally persists all > transient changes (let's forget about different resource providers and > transactions for now) > This approach has the advantage to reuse the map interface to change > values, no exceptions are thrown on put and remove. - but it requires > to additional calls, update on the map and commit on the resource > resolver to get the changes persisted. > > I thought of changing this and push each change directly into the > transient store, so a call to put() or remove() on the map is directly > pushed through - this avoids the extra call to update(), however put() > and remove() have no checked exceptions declared. So we can either > throw undeclared exceptions (which I think is not really a good idea > in this case) or defer exception throwing until the commit call on the > resource resolver. But obviously this gets nasty if e.g. a put() is > not successful, doesn't throw an exception and one is doing a get on > the same property etc. > > So, final solution would be to add special methods like set and delete > which might throw PersistenceException - and avoid reuse of modifying > methods from the map interface. Simpler to use compared to the current > version in trunk but additional methods. > > Right now I'm undecided between the first and the last solution - with > a slight preference of the current version from trunk. > > But maybe there are other/better options? > > Regards > Carsten > -- > Carsten Ziegeler > [email protected]
