You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Vidar Ramdal <vi...@idium.no> on 2010/02/11 17:02:00 UTC
JcrModifiablePropertyMap.save() makes atomic operations difficult
JcrModifiablePropertyMap works by storing all changes in an internal
cache. Upon save() the cached changes are written to the
javax.jcr.Node, and the node is save()d.
This makes sequences such as this impossible:
try {
// Modify a property through PersistableValueMap
PersistableValueMap props = resource.adaptTo(PersistableValueMap.class);
props.put("prop", value);
props.save();
// Do some other, unrelated changes
Node sibling = resource.adaptTo(Node.class).getParent().addNode("child");
sibling.doSomeChange(); // This may throw an exception
session.save();
} catch(Exception e) {
session.refresh(false); // Cancel the pending changes of this session
}
The idea is to save all changes to the repository in one single
session.save(). If anything fails, the session is reverted.
This doesn't work because the properties of PersistableValueMap is not
written to the node until PersistableValueMap.save(), and by then, it
is too late to revert the session, as the changes of
PersistableValueMap are already persisted.
We could make such operations easier by introducing a new method
PersistableValueMap.write() (in lack of a better name).
In case of JcrModifiablePropertyMap, most of save() would be moved to
write(), leaving out node.save().
So that JcrModifiablePropertyMap.save() would be:
public void save() throws PersistenceException {
this.write();
node.save();
}
In the code example above, replacing props.save() with props.write()
should give the desired functionality.
WDYT?
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Carsten Ziegeler <cz...@apache.org>.
Vidar Ramdal wrote:
> On Thu, Feb 11, 2010 at 9:49 PM, Julian Sedding <js...@day.com> wrote:
>> As Carsten mentioned, it starts getting tricky when the
>> write-capabilities of the Sling API are mixed with those of JCR. AFAIU
>> the JcrModifiablePropertyMap is for convenience and thus optimized and
>> limited for the use-case of setting properties on a single node.
>>
>> Would your problem be solved by a static helper method, accepting a
>> Node and a Map/ValueMap to achieve the same thing, similar convenience
>> and purely on the JCR API? I suppose this could go into the
>> JcrResourceUtil. I.e. something like
>> JcrResourceUtil.setProperties(Node, Map<String, Object>), which does
>> not execute session.save().
>
> That wouldn't help in my case, I think. The problem is not that
> JcrModifiablePropertyMap calls session.save() (it doesn't), but it
> calls node.save() which makes a session.refresh() useless.
Node.save() and node.refresh() are deprecated with jcr 2.0 :)
> I admit the problem *is* a bit far-fetched, but I'd like to have a
> completely resource-agnostic method of updating properties. So that
> any resource that adapts to ModifiablePropertyMap can be updated.
Yepp, I completly agree.
Now, as a node.save() and node.refresh() are deprecated, I think the way
the JcrModifiablePropertyMap currently works is very good, because you
can revert changes on the resource - regardless how the repository works
underneath.
One day our resource resolver might support crud operations completly,
but we're not there yet - maybe, we could add a save() method to the
resource resolver and this saves all changes - including the ones in the
JcrModifiablePropertyMaps (and any other resource providers).
Not sure, if this is a good idea :)
Carsten
--
Carsten Ziegeler
cziegeler@apache.org
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Vidar Ramdal <vi...@idium.no>.
On Thu, Feb 11, 2010 at 9:49 PM, Julian Sedding <js...@day.com> wrote:
> As Carsten mentioned, it starts getting tricky when the
> write-capabilities of the Sling API are mixed with those of JCR. AFAIU
> the JcrModifiablePropertyMap is for convenience and thus optimized and
> limited for the use-case of setting properties on a single node.
>
> Would your problem be solved by a static helper method, accepting a
> Node and a Map/ValueMap to achieve the same thing, similar convenience
> and purely on the JCR API? I suppose this could go into the
> JcrResourceUtil. I.e. something like
> JcrResourceUtil.setProperties(Node, Map<String, Object>), which does
> not execute session.save().
That wouldn't help in my case, I think. The problem is not that
JcrModifiablePropertyMap calls session.save() (it doesn't), but it
calls node.save() which makes a session.refresh() useless.
I admit the problem *is* a bit far-fetched, but I'd like to have a
completely resource-agnostic method of updating properties. So that
any resource that adapts to ModifiablePropertyMap can be updated.
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Julian Sedding <js...@day.com>.
As Carsten mentioned, it starts getting tricky when the
write-capabilities of the Sling API are mixed with those of JCR. AFAIU
the JcrModifiablePropertyMap is for convenience and thus optimized and
limited for the use-case of setting properties on a single node.
Would your problem be solved by a static helper method, accepting a
Node and a Map/ValueMap to achieve the same thing, similar convenience
and purely on the JCR API? I suppose this could go into the
JcrResourceUtil. I.e. something like
JcrResourceUtil.setProperties(Node, Map<String, Object>), which does
not execute session.save().
WDYT?
Regards
Julian
On Thu, Feb 11, 2010 at 9:22 PM, Vidar Ramdal <vi...@idium.no> wrote:
> On Thu, Feb 11, 2010 at 8:40 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> Vidar Ramdal wrote:
>>> JcrModifiablePropertyMap works by storing all changes in an internal
>>> cache. Upon save() the cached changes are written to the
>>> javax.jcr.Node, and the node is save()d.
>>>
>>> This makes sequences such as this impossible:
>>> try {
>>> // Modify a property through PersistableValueMap
>>> PersistableValueMap props = resource.adaptTo(PersistableValueMap.class);
>>> props.put("prop", value);
>>> props.save();
>>> // Do some other, unrelated changes
>>> Node sibling = resource.adaptTo(Node.class).getParent().addNode("child");
>>> sibling.doSomeChange(); // This may throw an exception
>>> session.save();
>>> } catch(Exception e) {
>>> session.refresh(false); // Cancel the pending changes of this session
>>> }
>>>
>>> The idea is to save all changes to the repository in one single
>>> session.save(). If anything fails, the session is reverted.
>>> This doesn't work because the properties of PersistableValueMap is not
>>> written to the node until PersistableValueMap.save(), and by then, it
>>> is too late to revert the session, as the changes of
>>> PersistableValueMap are already persisted.
>>>
>>> We could make such operations easier by introducing a new method
>>> PersistableValueMap.write() (in lack of a better name).
>>> In case of JcrModifiablePropertyMap, most of save() would be moved to
>>> write(), leaving out node.save().
>>>
>>> So that JcrModifiablePropertyMap.save() would be:
>>> public void save() throws PersistenceException {
>>> this.write();
>>> node.save();
>>> }
>>>
>>> In the code example above, replacing props.save() with props.write()
>>> should give the desired functionality.
>>>
>> I see the problem but I'm not sure what the best way of doing things is.
>> I think the property map uses the cache for an easy revert - if we would
>> do the revert differently (like caching the previous values) we could
>> directly set the properties on a modification and therefore a session
>> save would save these changes as well.
>> However if you call session save the property map is not aware of this
>> and does not get updated correctly.
>
> Exactly.
>
>> So I think as soon as you mix the two worlds - the sling resource api
>> with value maps - and the jcr api - direct access to nodes and the
>> session - you sooner or later get into problems.
>
> I was hoping for a little later :)
>
>> Therefore I would suggest to use one or the other.
>
> Yes, but then the problem is that the resource API (which I prefer for
> a lot of tasks) does not have the notion of a session, so atomic
> operations as in the code above is not possible using resources.
>
> Could we have some kind of ResourceModificationSession? Hmmm ...
>
> --
> Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
> Sommerrogata 13-15, N-0255 Oslo, Norway
> + 47 22 00 84 00 / +47 21 531941, ext 2070
>
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Vidar Ramdal <vi...@idium.no>.
On Thu, Feb 11, 2010 at 8:40 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Vidar Ramdal wrote:
>> JcrModifiablePropertyMap works by storing all changes in an internal
>> cache. Upon save() the cached changes are written to the
>> javax.jcr.Node, and the node is save()d.
>>
>> This makes sequences such as this impossible:
>> try {
>> // Modify a property through PersistableValueMap
>> PersistableValueMap props = resource.adaptTo(PersistableValueMap.class);
>> props.put("prop", value);
>> props.save();
>> // Do some other, unrelated changes
>> Node sibling = resource.adaptTo(Node.class).getParent().addNode("child");
>> sibling.doSomeChange(); // This may throw an exception
>> session.save();
>> } catch(Exception e) {
>> session.refresh(false); // Cancel the pending changes of this session
>> }
>>
>> The idea is to save all changes to the repository in one single
>> session.save(). If anything fails, the session is reverted.
>> This doesn't work because the properties of PersistableValueMap is not
>> written to the node until PersistableValueMap.save(), and by then, it
>> is too late to revert the session, as the changes of
>> PersistableValueMap are already persisted.
>>
>> We could make such operations easier by introducing a new method
>> PersistableValueMap.write() (in lack of a better name).
>> In case of JcrModifiablePropertyMap, most of save() would be moved to
>> write(), leaving out node.save().
>>
>> So that JcrModifiablePropertyMap.save() would be:
>> public void save() throws PersistenceException {
>> this.write();
>> node.save();
>> }
>>
>> In the code example above, replacing props.save() with props.write()
>> should give the desired functionality.
>>
> I see the problem but I'm not sure what the best way of doing things is.
> I think the property map uses the cache for an easy revert - if we would
> do the revert differently (like caching the previous values) we could
> directly set the properties on a modification and therefore a session
> save would save these changes as well.
> However if you call session save the property map is not aware of this
> and does not get updated correctly.
Exactly.
> So I think as soon as you mix the two worlds - the sling resource api
> with value maps - and the jcr api - direct access to nodes and the
> session - you sooner or later get into problems.
I was hoping for a little later :)
> Therefore I would suggest to use one or the other.
Yes, but then the problem is that the resource API (which I prefer for
a lot of tasks) does not have the notion of a session, so atomic
operations as in the code above is not possible using resources.
Could we have some kind of ResourceModificationSession? Hmmm ...
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Carsten Ziegeler <cz...@apache.org>.
Vidar Ramdal wrote:
> JcrModifiablePropertyMap works by storing all changes in an internal
> cache. Upon save() the cached changes are written to the
> javax.jcr.Node, and the node is save()d.
>
> This makes sequences such as this impossible:
> try {
> // Modify a property through PersistableValueMap
> PersistableValueMap props = resource.adaptTo(PersistableValueMap.class);
> props.put("prop", value);
> props.save();
> // Do some other, unrelated changes
> Node sibling = resource.adaptTo(Node.class).getParent().addNode("child");
> sibling.doSomeChange(); // This may throw an exception
> session.save();
> } catch(Exception e) {
> session.refresh(false); // Cancel the pending changes of this session
> }
>
> The idea is to save all changes to the repository in one single
> session.save(). If anything fails, the session is reverted.
> This doesn't work because the properties of PersistableValueMap is not
> written to the node until PersistableValueMap.save(), and by then, it
> is too late to revert the session, as the changes of
> PersistableValueMap are already persisted.
>
> We could make such operations easier by introducing a new method
> PersistableValueMap.write() (in lack of a better name).
> In case of JcrModifiablePropertyMap, most of save() would be moved to
> write(), leaving out node.save().
>
> So that JcrModifiablePropertyMap.save() would be:
> public void save() throws PersistenceException {
> this.write();
> node.save();
> }
>
> In the code example above, replacing props.save() with props.write()
> should give the desired functionality.
>
I see the problem but I'm not sure what the best way of doing things is.
I think the property map uses the cache for an easy revert - if we would
do the revert differently (like caching the previous values) we could
directly set the properties on a modification and therefore a session
save would save these changes as well.
However if you call session save the property map is not aware of this
and does not get updated correctly.
So I think as soon as you mix the two worlds - the sling resource api
with value maps - and the jcr api - direct access to nodes and the
session - you sooner or later get into problems. Therefore I would
suggest to use one or the other.
Carsten
--
Carsten Ziegeler
cziegeler@apache.org
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Vidar Ramdal <vi...@idium.no>.
On Mon, Feb 15, 2010 at 1:41 PM, Vidar Ramdal <vi...@idium.no> wrote:
> If we all agree that this is a viable path forward, I'll file a JIRA for it.
As there were no objections so far, I created SLING-1391 to track this.
https://issues.apache.org/jira/browse/SLING-1391
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Vidar Ramdal <vi...@idium.no>.
On Fri, Feb 12, 2010 at 10:32 PM, Felix Meschberger <fm...@gmail.com> wrote:
> Sorry to chime in late...
>
> I see the issues of Vidar and these are caused by the
> PersistableValueMap introducing its own transient space ontop of the JCR
> transient space.
>
> When the PersistableValueMap transient space is saved (save() call), the
> data is written to the JCR transient space and then directly to the JCR
> persistent space by calling the JCR save method.
>
> I see the problem. In fact, even if the Sling API would allow for full
> CRUD, particularly creating resources, Vidar's use case cannot be
> implemented using the PersistableValueMap.
>
> How about this:
>
> * Enable the PersistableValueMap to write through to the JCR
> transient space on put().
>
> * Write-through is configurable on a global level (in the
> JcrResourceResolver providing the PersistableValueMap
> instances upon adapt()). By default it is switched off
> (backwards compatiblity).
>
> * Write-through can also be switched on/off on a per-instance
> level of the PersistableValueMap object (setWriteThrough(boolean))
>
> * PersistableValueMap.save() first writes back local changes
> not already stored using write-through and then calls node.save()
>
> * PersistableValueMap.reset() drops local changes not already stored
> using write-through and then calls node.refresh(false)
>
> * When we finally do full CRUD in the ResourceResolver, we should
> add save/reset methods, which would call Session.save() or
> refresh(false), resp. For now the Session methods must be called
> directly.
>
> WDYT ?
Makes a lot of sense. I suppose the JCR write-through does not affect
performance too much.
I really like the configurable setting on JcrResourceResolver. As
Carsten says, Node.save() is deprecated by JCR 2.0, so we should
probably start avoiding it already.
If we all agree that this is a viable path forward, I'll file a JIRA for it.
--
Vidar S. Ramdal <vi...@idium.no> - http://www.idium.no
Sommerrogata 13-15, N-0255 Oslo, Norway
+ 47 22 00 84 00 / +47 21 531941, ext 2070
Re: JcrModifiablePropertyMap.save() makes atomic operations difficult
Posted by Felix Meschberger <fm...@gmail.com>.
Hi,
Sorry to chime in late...
I see the issues of Vidar and these are caused by the
PersistableValueMap introducing its own transient space ontop of the JCR
transient space.
When the PersistableValueMap transient space is saved (save() call), the
data is written to the JCR transient space and then directly to the JCR
persistent space by calling the JCR save method.
I see the problem. In fact, even if the Sling API would allow for full
CRUD, particularly creating resources, Vidar's use case cannot be
implemented using the PersistableValueMap.
How about this:
* Enable the PersistableValueMap to write through to the JCR
transient space on put().
* Write-through is configurable on a global level (in the
JcrResourceResolver providing the PersistableValueMap
instances upon adapt()). By default it is switched off
(backwards compatiblity).
* Write-through can also be switched on/off on a per-instance
level of the PersistableValueMap object (setWriteThrough(boolean))
* PersistableValueMap.save() first writes back local changes
not already stored using write-through and then calls node.save()
* PersistableValueMap.reset() drops local changes not already stored
using write-through and then calls node.refresh(false)
* When we finally do full CRUD in the ResourceResolver, we should
add save/reset methods, which would call Session.save() or
refresh(false), resp. For now the Session methods must be called
directly.
WDYT ?
Regards
Felix
On 11.02.2010 17:02, Vidar Ramdal wrote:
> JcrModifiablePropertyMap works by storing all changes in an internal
> cache. Upon save() the cached changes are written to the
> javax.jcr.Node, and the node is save()d.
>
> This makes sequences such as this impossible:
> try {
> // Modify a property through PersistableValueMap
> PersistableValueMap props = resource.adaptTo(PersistableValueMap.class);
> props.put("prop", value);
> props.save();
> // Do some other, unrelated changes
> Node sibling = resource.adaptTo(Node.class).getParent().addNode("child");
> sibling.doSomeChange(); // This may throw an exception
> session.save();
> } catch(Exception e) {
> session.refresh(false); // Cancel the pending changes of this session
> }
>
> The idea is to save all changes to the repository in one single
> session.save(). If anything fails, the session is reverted.
> This doesn't work because the properties of PersistableValueMap is not
> written to the node until PersistableValueMap.save(), and by then, it
> is too late to revert the session, as the changes of
> PersistableValueMap are already persisted.
>
> We could make such operations easier by introducing a new method
> PersistableValueMap.write() (in lack of a better name).
> In case of JcrModifiablePropertyMap, most of save() would be moved to
> write(), leaving out node.save().
>
> So that JcrModifiablePropertyMap.save() would be:
> public void save() throws PersistenceException {
> this.write();
> node.save();
> }
>
> In the code example above, replacing props.save() with props.write()
> should give the desired functionality.
>
> WDYT?
>