Re: SlingPOSTServlet : Auto Checkin Nodes

[Felix Meschberger]

Hi,

Am Mittwoch, den 08.12.2010, 16:19 +0100 schrieb Clemens Wyss: 
 Auto Checkin Nodes  is activated (i.e. true). Hence I would expect that 
 when I modify (a property) of my mixin:versionable node, a new version is 
 created. Unfortunately this is not the case.
 
 Looking at AbstractSlingPostOperation#run only CREATE, CHECKOUT or CHECKIN 
 trigger a potential checkin, but not MODIFY. 
 Shouldn't we?:
 ...
 case MODIFY : 
 response.onModified(change.getSource());
 if ( versionableConfiguration.isAutoCheckin() ) {
 nodesToCheckin.add(change.getSource());
 }
 break;  
 ...
 maybe additionally check whether the node is really checked out?
 
 WDYT?

I think auto-checkin should only be done if the node has been checked
out as part of the modification operation. Thus:

-- if the node was checked-out before the op, then it must be
 checked-out after and no version must be created
-- if the node was checked-in before the op, and the node was
 checked out, then it should probably be checked in

And: we must be very carefull to not create backwards compatibility
issues around this automatic checkin/checkout.

Regards
Felix

Re: SlingPOSTServlet : Auto Checkin Nodes

[Ian Boston]

On 8 Dec 2010, at 16:10, Felix Meschberger wrote:

 Hi,
 
 Am Mittwoch, den 08.12.2010, 16:19 +0100 schrieb Clemens Wyss: 
 Auto Checkin Nodes  is activated (i.e. true). Hence I would expect that 
 when I modify (a property) of my mixin:versionable node, a new version is 
 created. Unfortunately this is not the case.
 
 Looking at AbstractSlingPostOperation#run only CREATE, CHECKOUT or CHECKIN 
 trigger a potential checkin, but not MODIFY. 
 Shouldn't we?:
 ...
 case MODIFY : 
response.onModified(change.getSource());
if ( versionableConfiguration.isAutoCheckin() ) {
nodesToCheckin.add(change.getSource());
}
break;  
 ...
 maybe additionally check whether the node is really checked out?
 
 WDYT?
 
 I think auto-checkin should only be done if the node has been checked
 out as part of the modification operation. Thus:
 
 -- if the node was checked-out before the op, then it must be
 checked-out after and no version must be created
 -- if the node was checked-in before the op, and the node was
 checked out, then it should probably be checked in
 
 And: we must be very carefull to not create backwards compatibility
 issues around this automatic checkin/checkout.

BTW, under load we have found adding mix:versionable to a node on creation 
creates significant locking problems as the creation of version history items 
tree takes quite a bit of JR resource. We have had to add mix:versionable only 
when absolutely needed.
Ian


 
 Regards
 Felix
 

Re: SlingPOSTServlet : Auto Checkin Nodes

[Justin Edelson]

On Dec 8, 2010, at 11:10 AM, Felix Meschberger fmesc...@gmail.com wrote:

 And: we must be very carefull to not create backwards compatibility
 issues around this automatic checkin/checkout.

And that's what integration tests are for.

Clemens - take a look at the integration tests. They should cover every 
supported scenario. The only difference is that the integration tests use 
parameters to specify the versioning configuration whereas I expect most users 
to configure this stuff in ConfigAdmin.

I can help you grok the test code if it isn't clear.

Justin