Hi Mike, I would lean toward making the property overwrite a separate import option for the same reasons you outlined below.
Regards, -Eric On Fri, Jul 30, 2010 at 12:00 PM, Mike Moulton <[email protected]> wrote: > I will work on a patch for this. > > Should we add ImportOptions.isPropertyOverwrite or should I use the > existing ImportOptions.isOverwrite for both nodes and properties? Seems to > me using the same option could be dangerous as isOverwrite currently causes > the node to be removed before adding the new node. Causing unintended > consequences if you only wished to modify properties, not replacing the node > altogether. > > Thoughts? > > -- Mike > > > On Jul 30, 2010, at 2:26 AM, Bertrand Delacretaz wrote: > > > Hi, > > > > On Fri, Jul 30, 2010 at 1:30 AM, Mike Moulton <[email protected]> > wrote: > >> I have the need to use the "import" operation provided by SLING-1172 to > modify a structure, or more specifically a property. Looking at the > DefaultContentCreator.java:311 it looks like property modification is turned > off for the path the ImportOperation.java takes through the codebase. > >> > >> Can someone, who knows the content loader codebase better, chime in on > what they think the level of effort might be here. Is this something that > should be supported by Sling or should I maintain my own codebase for > something like this? > > > > So basically you want to add an option to continue writing the > > property here, instead of returning? > > > > if (node.hasProperty(name) > > && !node.getProperty(name).isNew()) { > > return; > > } > > > > That looks simple enough, I think you'd just need to add an > > isPropertyOverwrite() method to ImportOptions and its implementations, > > and in the ImportOperation class (servlets.post bundle) set that > > option according to a request parameter. > > > > ImportOptions of part of a public API but hasn't been released yet, so > > I think we can still change it. > > > >> ...Is this something that should be supported by Sling or should I > maintain my own codebase for > >> something like this?... > > > > I'd be +1 on adding that feature if someone (hint, hint ;-) can > > provide a patch including tests. > > > > -Bertrand > >
