Hi,

Thanks for the hard work on this important topic.

Am 18.07.2012 um 09:30 schrieb Carsten Ziegeler:

> 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.

This "second level of indirection" -- calling update and then commit -- sounds 
like overhead.

> 
> 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.

According to the Map interface, we can throw any of 
UnsupportedOperationException, ClassCastException, NullPointerException, 
IllegalArgumentException wrapping the actual underlying cause. While this is 
not part of the ValueMap interface itself it is inherited from the Map 
interface.

So the exception handling part of the problem could really be solved IMHO.

The other part of the problem as noted by Tobias B. is very important, too: 
There are live set views on the Map (keys, values, entries (and their Entry 
objects)), which also have update methods to be handled. This might be solved 
by having these views be Unmodifiable.. (similar to what 
Collections.UnmodifiableMap does).


> 
> 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.

I think that is not required when considering wrapping persistence exceptions 
into one of the runtime exceptions declared by the Map interface.

> 
> 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?

As indicated above, my preference would be a "modified version of half-way 
proposal 2":

  * Map.put/putAll is reused pushing into the ResourceProvider's
    transient space directly (for JCR this directly uses Node.setProperty)
  * Any persistence exceptions are wrapped into one of the defined
    runtime exceptions (e.g. ValueFormatException wrapped into 
ClassCastException)
  * Set views are unmodifiable
  * To actually persist any changes the ResourceResolver.commit method is used

WDYT ?

Follow-up questions: Do all ModifyingResourceProvider implementations have to 
provide a transient space ? I am thinking of a Filesystem based implementation.

I will have some more general comments on the current trunk in a separate 
thread.

Regards
Felix

> 
> Regards
> Carsten
> -- 
> Carsten Ziegeler
> [email protected]

Reply via email to