You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Clemens Wyss <cl...@mysign.ch> on 2010/12/08 16:19:00 UTC
SlingPOSTServlet : Auto Checkin Nodes
"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?
AW: SlingPOSTServlet : Auto Checkin Nodes
Posted by Clemens Wyss <cl...@mysign.ch>.
Looks like I got it, last but not least due to the integration tests ;-)
Regarding the integration tests (in PostServletVersionableTests.java) I would apply two "extract method" refactorings (createVersionedNode and createNonVersionedNode, which differ only in the fact that params is submitted or not which could yet be extracted), but that's just cosmetics...
Thx
Clemens
> -----Ursprüngliche Nachricht-----
> Von: Justin Edelson [mailto:justinedelson@gmail.com] Im Auftrag von Justin
> Edelson
> Gesendet: Donnerstag, 9. Dezember 2010 00:52
> An: dev@sling.apache.org
> Betreff: Re: SlingPOSTServlet : Auto Checkin Nodes
>
>
>
> On Dec 8, 2010, at 11:10 AM, Felix Meschberger <fm...@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
Re: SlingPOSTServlet : Auto Checkin Nodes
Posted by Justin Edelson <ju...@justinedelson.com>.
On Dec 8, 2010, at 11:10 AM, Felix Meschberger <fm...@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
AW: SlingPOSTServlet : Auto Checkin Nodes
Posted by Clemens Wyss <cl...@mysign.ch>.
> I think auto-checkin should only be done if the node has been checked out as
> part of the modification operation. Thus:
What if auto-checkout is also enabled, too?
> -----Ursprüngliche Nachricht-----
> Von: Felix Meschberger [mailto:fmeschbe@gmail.com]
> Gesendet: Mittwoch, 8. Dezember 2010 17:10
> An: dev@sling.apache.org
> Betreff: Re: SlingPOSTServlet : Auto Checkin Nodes
>
> 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
Posted by Ian Boston <ie...@tfd.co.uk>.
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
Posted by Felix Meschberger <fm...@gmail.com>.
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