Velocity 2.0 and DeprecatedCheckUberspector

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Velocity 2.0 and DeprecatedCheckUberspector

Claude Brisson
Hi.

I'm currently working on Velocity 2.0 packaging.

If that's OK with you, I would like to incorporate
DeprecatedCheckUberspector.java into Velocity, but I need a statement
from your part to be able to change its licence to Apache 2.0 (LGPL and
Apache 2.0 licences aren't compatible).

By the way, I take this opportunity to tell you that if there is another
specific part of xwiki-commons-velocity that you think should be
integrated on our side, or an important missing feature you'd like to
insist on, don't hesitate. I already integrated VELOCITY-825, for
instance, so String->Enum constant conversions are now handled by
Velocity. There may be other important conversion cases you'd like to
see handled.

Regards,

   Claude

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Sergiu Dumitriu-3
+1 from me.

On 07/21/2016 11:26 AM, Claude Brisson wrote:

> Hi.
>
> I'm currently working on Velocity 2.0 packaging.
>
> If that's OK with you, I would like to incorporate
> DeprecatedCheckUberspector.java into Velocity, but I need a statement
> from your part to be able to change its licence to Apache 2.0 (LGPL and
> Apache 2.0 licences aren't compatible).
>
> By the way, I take this opportunity to tell you that if there is another
> specific part of xwiki-commons-velocity that you think should be
> integrated on our side, or an important missing feature you'd like to
> insist on, don't hesitate. I already integrated VELOCITY-825, for
> instance, so String->Enum constant conversions are now handled by
> Velocity. There may be other important conversion cases you'd like to
> see handled.
>
> Regards,
>
>   Claude

--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

vmassol
Administrator
In reply to this post by Claude Brisson
Hi Claude,

> On 21 Jul 2016, at 17:26, Claude Brisson <[hidden email]> wrote:
>
> Hi.
>
> I'm currently working on Velocity 2.0 packaging.
>
> If that's OK with you, I would like to incorporate DeprecatedCheckUberspector.java into Velocity, but I need a statement from your part to be able to change its licence to Apache 2.0 (LGPL and Apache 2.0 licences aren't compatible).

Thanks for reaching out. A big +1 from me too. The more we can move to Velocity the better it is.

> By the way, I take this opportunity to tell you that if there is another specific part of xwiki-commons-velocity that you think should be integrated on our side, or an important missing feature you'd like to insist on, don't hesitate. I already integrated VELOCITY-825, for instance, so String->Enum constant conversions are now handled by Velocity. There may be other important conversion cases you'd like to see handled.

Ok cool.

On your side, if you see other issues that you’d be willing to integrate let us know.

Thanks
-Vincent

> Regards,
>
>  Claude

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Sergiu Dumitriu-3
One thing that would be nice to have is the custom whitespace filter
that the {{velocity}} macro has: see
http://extensions.xwiki.org/xwiki/bin/view/Extension/Velocity+Macro#HParametersdefinition
and
http://extensions.xwiki.org/xwiki/bin/view/Extension/Velocity+Macro+Filter

Basically, Velocity only supports literal whitespace processing, with
every space and newline going into the output, while we like to be able
to indent our sources without padding the output with lots of extra spaces.

On 07/22/2016 02:49 AM, Vincent Massol wrote:

> Hi Claude,
>
>> On 21 Jul 2016, at 17:26, Claude Brisson <[hidden email]> wrote:
>>
>> Hi.
>>
>> I'm currently working on Velocity 2.0 packaging.
>>
>> If that's OK with you, I would like to incorporate DeprecatedCheckUberspector.java into Velocity, but I need a statement from your part to be able to change its licence to Apache 2.0 (LGPL and Apache 2.0 licences aren't compatible).
>
> Thanks for reaching out. A big +1 from me too. The more we can move to Velocity the better it is.
>
>> By the way, I take this opportunity to tell you that if there is another specific part of xwiki-commons-velocity that you think should be integrated on our side, or an important missing feature you'd like to insist on, don't hesitate. I already integrated VELOCITY-825, for instance, so String->Enum constant conversions are now handled by Velocity. There may be other important conversion cases you'd like to see handled.
>
> Ok cool.
>
> On your side, if you see other issues that you’d be willing to integrate let us know.
>
> Thanks
> -Vincent
>
>> Regards,
>>
>>  Claude

--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Thomas Mortagne
Administrator
In reply to this post by Claude Brisson
On Thu, Jul 21, 2016 at 5:26 PM, Claude Brisson <[hidden email]> wrote:
> Hi.
>
> I'm currently working on Velocity 2.0 packaging.

Great news :)

>
> If that's OK with you, I would like to incorporate
> DeprecatedCheckUberspector.java into Velocity, but I need a statement from
> your part to be able to change its licence to Apache 2.0 (LGPL and Apache
> 2.0 licences aren't compatible).

+1 for me

>
> By the way, I take this opportunity to tell you that if there is another
> specific part of xwiki-commons-velocity that you think should be integrated
> on our side, or an important missing feature you'd like to insist on, don't
> hesitate. I already integrated VELOCITY-825, for instance, so String->Enum
> constant conversions are now handled by Velocity. There may be other
> important conversion cases you'd like to see handled.

Note that XWiki goes far beyond String->Enum. When the signature of a
method cannot be found it search for method with similar number of
arguments and use xwiki-commons-properties module (similar to apache
beanutils) to try to convert the parameter it gets into the types of
the method parameters. We are using this a lot so it would probably be
a nice to have in Velocity (probably something based on beanutils that
could be extended on our side to use all xwiki-commons-properties
supported types). See
https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/MethodArgumentsUberspector.java.

Maybe it's not the case in Velocity 2.0 anymore, but restriction on
Class methods access is way too strong in 1.7 so we overwritten secure
introspector to accept more calls. See
https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/SecureIntrospector.java.

We implemented a directives based try/catch support for Velocity
recently. We don't use it much yet (too young) but we probably will.
Could be interesting on Velocity side. See
https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/TryCatchDirective.java.

>
> Regards,
>
>   Claude
>
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Claude Brisson
Thanks for your votes.

Regarding your suggestions:

  - velocity 2.0 will also go a little bit further than string -> enum,
as it will try to convert method arguments between all basic Java data
types (boolean, number, string).
  - it does this via a pluggable ConversionHandler that can easily be
extended, see [1]
  - I've added a public VelMethod.getMethod() getter, so it should be
easier to access methods

[1]
http://velocity.apache.org/engine/devel/developer-guide.html#method-arguments-conversions


   Claude

On 22/07/2016 09:52, Thomas Mortagne wrote:

> On Thu, Jul 21, 2016 at 5:26 PM, Claude Brisson <[hidden email]> wrote:
>> Hi.
>>
>> I'm currently working on Velocity 2.0 packaging.
> Great news :)
>
>> If that's OK with you, I would like to incorporate
>> DeprecatedCheckUberspector.java into Velocity, but I need a statement from
>> your part to be able to change its licence to Apache 2.0 (LGPL and Apache
>> 2.0 licences aren't compatible).
> +1 for me
>
>> By the way, I take this opportunity to tell you that if there is another
>> specific part of xwiki-commons-velocity that you think should be integrated
>> on our side, or an important missing feature you'd like to insist on, don't
>> hesitate. I already integrated VELOCITY-825, for instance, so String->Enum
>> constant conversions are now handled by Velocity. There may be other
>> important conversion cases you'd like to see handled.
> Note that XWiki goes far beyond String->Enum. When the signature of a
> method cannot be found it search for method with similar number of
> arguments and use xwiki-commons-properties module (similar to apache
> beanutils) to try to convert the parameter it gets into the types of
> the method parameters. We are using this a lot so it would probably be
> a nice to have in Velocity (probably something based on beanutils that
> could be extended on our side to use all xwiki-commons-properties
> supported types). See
> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/MethodArgumentsUberspector.java.
>
> Maybe it's not the case in Velocity 2.0 anymore, but restriction on
> Class methods access is way too strong in 1.7 so we overwritten secure
> introspector to accept more calls. See
> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/SecureIntrospector.java.
>
> We implemented a directives based try/catch support for Velocity
> recently. We don't use it much yet (too young) but we probably will.
> Could be interesting on Velocity side. See
> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/TryCatchDirective.java.
>
>> Regards,
>>
>>    Claude
>>
>> _______________________________________________
>> devs mailing list
>> [hidden email]
>> http://lists.xwiki.org/mailman/listinfo/devs
>
>

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Claude Brisson
In reply to this post by Sergiu Dumitriu-3
I agree whitespace filtering is a hot topic. I'll check those links, but
I'm not sure I'll be able to provide something for 2.0, though...

   Claude

On 22/07/2016 08:58, Sergiu Dumitriu wrote:

> One thing that would be nice to have is the custom whitespace filter
> that the {{velocity}} macro has: see
> http://extensions.xwiki.org/xwiki/bin/view/Extension/Velocity+Macro#HParametersdefinition
> and
> http://extensions.xwiki.org/xwiki/bin/view/Extension/Velocity+Macro+Filter
>
> Basically, Velocity only supports literal whitespace processing, with
> every space and newline going into the output, while we like to be able
> to indent our sources without padding the output with lots of extra spaces.
>
> On 07/22/2016 02:49 AM, Vincent Massol wrote:
>> Hi Claude,
>>
>>> On 21 Jul 2016, at 17:26, Claude Brisson <[hidden email]> wrote:
>>>
>>> Hi.
>>>
>>> I'm currently working on Velocity 2.0 packaging.
>>>
>>> If that's OK with you, I would like to incorporate DeprecatedCheckUberspector.java into Velocity, but I need a statement from your part to be able to change its licence to Apache 2.0 (LGPL and Apache 2.0 licences aren't compatible).
>> Thanks for reaching out. A big +1 from me too. The more we can move to Velocity the better it is.
>>
>>> By the way, I take this opportunity to tell you that if there is another specific part of xwiki-commons-velocity that you think should be integrated on our side, or an important missing feature you'd like to insist on, don't hesitate. I already integrated VELOCITY-825, for instance, so String->Enum constant conversions are now handled by Velocity. There may be other important conversion cases you'd like to see handled.
>> Ok cool.
>>
>> On your side, if you see other issues that you’d be willing to integrate let us know.
>>
>> Thanks
>> -Vincent
>>
>>> Regards,
>>>
>>>   Claude

_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Thomas Mortagne
Administrator
In reply to this post by Claude Brisson
Ok great, I added it to http://jira.xwiki.org/browse/XCOMMONS-1027 so
that we don't forget.

On Tue, Jul 26, 2016 at 6:47 PM, Claude Brisson <[hidden email]> wrote:

> Thanks for your votes.
>
> Regarding your suggestions:
>
>  - velocity 2.0 will also go a little bit further than string -> enum, as it
> will try to convert method arguments between all basic Java data types
> (boolean, number, string).
>  - it does this via a pluggable ConversionHandler that can easily be
> extended, see [1]
>  - I've added a public VelMethod.getMethod() getter, so it should be easier
> to access methods
>
> [1]
> http://velocity.apache.org/engine/devel/developer-guide.html#method-arguments-conversions
>
>
>   Claude
>
>
> On 22/07/2016 09:52, Thomas Mortagne wrote:
>>
>> On Thu, Jul 21, 2016 at 5:26 PM, Claude Brisson <[hidden email]>
>> wrote:
>>>
>>> Hi.
>>>
>>> I'm currently working on Velocity 2.0 packaging.
>>
>> Great news :)
>>
>>> If that's OK with you, I would like to incorporate
>>> DeprecatedCheckUberspector.java into Velocity, but I need a statement
>>> from
>>> your part to be able to change its licence to Apache 2.0 (LGPL and Apache
>>> 2.0 licences aren't compatible).
>>
>> +1 for me
>>
>>> By the way, I take this opportunity to tell you that if there is another
>>> specific part of xwiki-commons-velocity that you think should be
>>> integrated
>>> on our side, or an important missing feature you'd like to insist on,
>>> don't
>>> hesitate. I already integrated VELOCITY-825, for instance, so
>>> String->Enum
>>> constant conversions are now handled by Velocity. There may be other
>>> important conversion cases you'd like to see handled.
>>
>> Note that XWiki goes far beyond String->Enum. When the signature of a
>> method cannot be found it search for method with similar number of
>> arguments and use xwiki-commons-properties module (similar to apache
>> beanutils) to try to convert the parameter it gets into the types of
>> the method parameters. We are using this a lot so it would probably be
>> a nice to have in Velocity (probably something based on beanutils that
>> could be extended on our side to use all xwiki-commons-properties
>> supported types). See
>>
>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/MethodArgumentsUberspector.java.
>>
>> Maybe it's not the case in Velocity 2.0 anymore, but restriction on
>> Class methods access is way too strong in 1.7 so we overwritten secure
>> introspector to accept more calls. See
>>
>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/SecureIntrospector.java.
>>
>> We implemented a directives based try/catch support for Velocity
>> recently. We don't use it much yet (too young) but we probably will.
>> Could be interesting on Velocity side. See
>>
>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/TryCatchDirective.java.
>>
>>> Regards,
>>>
>>>    Claude
>>>
>>> _______________________________________________
>>> devs mailing list
>>> [hidden email]
>>> http://lists.xwiki.org/mailman/listinfo/devs
>>
>>
>>
>
> _______________________________________________
> devs mailing list
> [hidden email]
> http://lists.xwiki.org/mailman/listinfo/devs



--
Thomas Mortagne
_______________________________________________
devs mailing list
[hidden email]
http://lists.xwiki.org/mailman/listinfo/devs
Reply | Threaded
Open this post in threaded view
|

Re: Velocity 2.0 and DeprecatedCheckUberspector

Thomas Mortagne
Administrator
Hi Claude,

I (finally) started working on Velocity 2.0. You can see the work in
progress in https://github.com/xwiki/xwiki-commons/tree/feature-velocity2/xwiki-commons-core/xwiki-commons-velocity.

Good news first: the refactoring around macro/namespace handling with
the new Template class is great for us :)

Unfortunately we have a few important issues in the context of XWiki
since most extensions use Velocity and we don't want to break them as
much as possible:

= Blockers

The change in the way to handle macro parameters cause a major
regression for various scripts. Among other things we currently use
the following hack to make macros return stuff: (1) (see (2) for a few
examples of how it was used) and this is totally broken in Velocity
2.0. I started working on a setVariable directive to replace the old
#setVariable macro and have the same behavior then before to limit the
incidence of this change but I have a hard time finding the caller
variable name when called from a macro (the last test in (2)). The
current code is available on
https://github.com/xwiki/xwiki-commons/blob/feature-velocity2/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/internal/directive/SetVariableDirective.java,
if you have some suggestions that would be great ! I have a few ideas
to cover our return need without using this kind of hack but my only
concern right now is retro compatibility.

I found a few unplanned (at least not documented) regressions in Velocity 2 VTL:

* https://issues.apache.org/jira/browse/VELOCITY-896
* https://issues.apache.org/jira/browse/VELOCITY-897

= Things XWiki cannot really use in Velocity 2.0 after all

* the new ConversionHandler is actually too limited for us. I created
https://issues.apache.org/jira/browse/VELOCITY-892. Not a big deal,
we'll keep using our uberspector but would have been nice to delete a
bit of code on our side ;)



1: https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-web/src/main/webapp/templates/macros.vm#L1177
2: https://github.com/xwiki/xwiki-commons/blob/2f8f8be5149a3b9f62aff24ab0a499df68084656/xwiki-commons-core/xwiki-commons-velocity/src/test/java/org/xwiki/velocity/internal/DefaultVelocityEngineTest.java#L296

On Thu, Jul 28, 2016 at 10:50 AM, Thomas Mortagne
<[hidden email]> wrote:

> Ok great, I added it to http://jira.xwiki.org/browse/XCOMMONS-1027 so
> that we don't forget.
>
> On Tue, Jul 26, 2016 at 6:47 PM, Claude Brisson <[hidden email]> wrote:
>> Thanks for your votes.
>>
>> Regarding your suggestions:
>>
>>  - velocity 2.0 will also go a little bit further than string -> enum, as it
>> will try to convert method arguments between all basic Java data types
>> (boolean, number, string).
>>  - it does this via a pluggable ConversionHandler that can easily be
>> extended, see [1]
>>  - I've added a public VelMethod.getMethod() getter, so it should be easier
>> to access methods
>>
>> [1]
>> http://velocity.apache.org/engine/devel/developer-guide.html#method-arguments-conversions
>>
>>
>>   Claude
>>
>>
>> On 22/07/2016 09:52, Thomas Mortagne wrote:
>>>
>>> On Thu, Jul 21, 2016 at 5:26 PM, Claude Brisson <[hidden email]>
>>> wrote:
>>>>
>>>> Hi.
>>>>
>>>> I'm currently working on Velocity 2.0 packaging.
>>>
>>> Great news :)
>>>
>>>> If that's OK with you, I would like to incorporate
>>>> DeprecatedCheckUberspector.java into Velocity, but I need a statement
>>>> from
>>>> your part to be able to change its licence to Apache 2.0 (LGPL and Apache
>>>> 2.0 licences aren't compatible).
>>>
>>> +1 for me
>>>
>>>> By the way, I take this opportunity to tell you that if there is another
>>>> specific part of xwiki-commons-velocity that you think should be
>>>> integrated
>>>> on our side, or an important missing feature you'd like to insist on,
>>>> don't
>>>> hesitate. I already integrated VELOCITY-825, for instance, so
>>>> String->Enum
>>>> constant conversions are now handled by Velocity. There may be other
>>>> important conversion cases you'd like to see handled.
>>>
>>> Note that XWiki goes far beyond String->Enum. When the signature of a
>>> method cannot be found it search for method with similar number of
>>> arguments and use xwiki-commons-properties module (similar to apache
>>> beanutils) to try to convert the parameter it gets into the types of
>>> the method parameters. We are using this a lot so it would probably be
>>> a nice to have in Velocity (probably something based on beanutils that
>>> could be extended on our side to use all xwiki-commons-properties
>>> supported types). See
>>>
>>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/MethodArgumentsUberspector.java.
>>>
>>> Maybe it's not the case in Velocity 2.0 anymore, but restriction on
>>> Class methods access is way too strong in 1.7 so we overwritten secure
>>> introspector to accept more calls. See
>>>
>>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/SecureIntrospector.java.
>>>
>>> We implemented a directives based try/catch support for Velocity
>>> recently. We don't use it much yet (too young) but we probably will.
>>> Could be interesting on Velocity side. See
>>>
>>> https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/TryCatchDirective.java.
>>>
>>>> Regards,
>>>>
>>>>    Claude
>>>>
>>>> _______________________________________________
>>>> devs mailing list
>>>> [hidden email]
>>>> http://lists.xwiki.org/mailman/listinfo/devs
>>>
>>>
>>>
>>
>> _______________________________________________
>> devs mailing list
>> [hidden email]
>> http://lists.xwiki.org/mailman/listinfo/devs
>
>
>
> --
> Thomas Mortagne


Thanks,
--
Thomas Mortagne