You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Mike Moulton <mi...@meltmedia.com> on 2010/07/30 01:30:46 UTC
Allow modify during "import" operation
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?
-- Mike
Re: Allow modify during "import" operation
Posted by Eric Norman <er...@gmail.com>.
I will try to find some time to review the patch in the next few days.
Regards,
Eric
On Aug 5, 2010 11:25 AM, "Simon Gaeremynck" <ga...@gmail.com> wrote:
I cannot take any credit for this.
It was Zach Thomas who did all the work and it would be totally awesome if
it got merged in. :-)
Cheers,
Simon
On 5 Aug 2010, at 20:18, Mike Moulton wrote:
> I would love to have this incorporated into the tr...
Re: Allow modify during "import" operation
Posted by Simon Gaeremynck <ga...@gmail.com>.
I cannot take any credit for this.
It was Zach Thomas who did all the work and it would be totally awesome if it got merged in. :-)
Cheers,
Simon
On 5 Aug 2010, at 20:18, Mike Moulton wrote:
> I would love to have this incorporated into the trunk. At this point my company is using a custom operation until the trunk fully supports merging.
>
> Thank you Simon for creating this patch as I never got to it.
>
> Regards,
> Mike
>
> On Aug 5, 2010, at 10:36 AM, Simon Gaeremynck wrote:
>
>> This issue has been reported as [1] SLING-1627 and a patch was provided.
>> Is it possible to merge this in as this is blocking a Sakai release.
>>
>> Regards,
>> Simon
>>
>> [1] https://issues.apache.org/jira/browse/SLING-1627
>>
>>
>> On 3 Aug 2010, at 20:06, Eric Norman wrote:
>>
>>> 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 <mi...@meltmedia.com> 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 <mi...@meltmedia.com>
>>>> 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
>>>>
>>>>
>>
>
Re: Allow modify during "import" operation
Posted by Mike Moulton <mi...@meltmedia.com>.
I would love to have this incorporated into the trunk. At this point my company is using a custom operation until the trunk fully supports merging.
Thank you Simon for creating this patch as I never got to it.
Regards,
Mike
On Aug 5, 2010, at 10:36 AM, Simon Gaeremynck wrote:
> This issue has been reported as [1] SLING-1627 and a patch was provided.
> Is it possible to merge this in as this is blocking a Sakai release.
>
> Regards,
> Simon
>
> [1] https://issues.apache.org/jira/browse/SLING-1627
>
>
> On 3 Aug 2010, at 20:06, Eric Norman wrote:
>
>> 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 <mi...@meltmedia.com> 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 <mi...@meltmedia.com>
>>> 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
>>>
>>>
>
Re: Allow modify during "import" operation
Posted by Ray Davis <ra...@media.berkeley.edu>.
Sakai is very happy. :) Thank you for the fix.
Best,
Ray
On 8/9/10 10:08 AM, Eric Norman wrote:
> Hi all,
>
> I reviewed the patch and checked in the changes with a few minor
> modifications. Please verify that that the changes will work for your use
> cases.
>
> Regards,
> Eric
>
> On Aug 6, 2010 12:41 AM, "Bertrand Delacretaz"<bd...@apache.org>
> wrote:
>
> On Thu, Aug 5, 2010 at 7:36 PM, Simon Gaeremynck<ga...@gmail.com>
> wrote:
>> This issue has bee...
>> ...
>
>> [1] https://issues.apache.org/jira/browse/SLING-1627
> Looking at that patch, Zach reused the existing
> ImportOptions.isOverwrite() flag, instead of defining a new
> isPropertyOverwrite() one as some of us suggested.
>
> I think that introduces an incompatible change, whereas defining a new
> option means that old code would continue to work unchanged.
>
> -Bertrand
>
Re: Allow modify during "import" operation
Posted by Eric Norman <er...@gmail.com>.
Hi all,
I reviewed the patch and checked in the changes with a few minor
modifications. Please verify that that the changes will work for your use
cases.
Regards,
Eric
On Aug 6, 2010 12:41 AM, "Bertrand Delacretaz" <bd...@apache.org>
wrote:
On Thu, Aug 5, 2010 at 7:36 PM, Simon Gaeremynck <ga...@gmail.com>
wrote:
> This issue has bee...
>...
> [1] https://issues.apache.org/jira/browse/SLING-1627
Looking at that patch, Zach reused the existing
ImportOptions.isOverwrite() flag, instead of defining a new
isPropertyOverwrite() one as some of us suggested.
I think that introduces an incompatible change, whereas defining a new
option means that old code would continue to work unchanged.
-Bertrand
Re: Allow modify during "import" operation
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Thu, Aug 5, 2010 at 7:36 PM, Simon Gaeremynck <ga...@gmail.com> wrote:
> This issue has been reported as [1] SLING-1627 and a patch was provided.
> Is it possible to merge this in as this is blocking a Sakai release.
>...
> [1] https://issues.apache.org/jira/browse/SLING-1627
Looking at that patch, Zach reused the existing
ImportOptions.isOverwrite() flag, instead of defining a new
isPropertyOverwrite() one as some of us suggested.
I think that introduces an incompatible change, whereas defining a new
option means that old code would continue to work unchanged.
-Bertrand
Re: Allow modify during "import" operation
Posted by Simon Gaeremynck <ga...@gmail.com>.
This issue has been reported as [1] SLING-1627 and a patch was provided.
Is it possible to merge this in as this is blocking a Sakai release.
Regards,
Simon
[1] https://issues.apache.org/jira/browse/SLING-1627
On 3 Aug 2010, at 20:06, Eric Norman wrote:
> 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 <mi...@meltmedia.com> 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 <mi...@meltmedia.com>
>> 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
>>
>>
Re: Allow modify during "import" operation
Posted by Eric Norman <er...@gmail.com>.
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 <mi...@meltmedia.com> 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 <mi...@meltmedia.com>
> 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
>
>
Re: Allow modify during "import" operation
Posted by Mike Moulton <mi...@meltmedia.com>.
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 <mi...@meltmedia.com> 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
Re: Allow modify during "import" operation
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,
On Fri, Jul 30, 2010 at 1:30 AM, Mike Moulton <mi...@meltmedia.com> 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