AW: SlingPOSTServlet : Auto Checkin Nodes

2010-12-09 Thread Clemens Wyss
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:justinedel...@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 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


AW: SlingPOSTServlet : Auto Checkin Nodes

2010-12-08 Thread Clemens Wyss
 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:fmesc...@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