[discussion] allow Script Engines to also render components based on paths

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

[discussion] allow Script Engines to also render components based on paths

Radu Cotescu-3
Hello,

In SLING-4330 [0] Oliver suggests that Sightly should be configured to
render templates only from some search paths sub-paths, because he'd also
like to write some Thymeleaf templates in a project, where the Thymeleaf
script engine also binds itself to the *.html extension.

However, if we'd like scripting engines to process templates based on
allowed paths I think that this should happen at a higher level. AFAIK a
scripting engine registers itself for an extension, but that's about it.
The behaviour that determines which script engine will provide the
rendering for a resource is implemented by the SlingServletResolver, which
adapts the component's resource to a ScriptEngine.

The AdapterFactory that performs the adaptation is implemented by
SlingScriptAdapterFactory, which picks the scripting engine based solely on
extension. If we'd change the ScriptEngineFactories to also provide the
path patterns for which they register and we'd expose the patterns as a
configurable OSGi property I think we'd reach the goal of having scripting
engines render templates also based on paths.

WDYT?

Regards,
Radu

[0] - https://issues.apache.org/jira/browse/SLING-4330
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Felix Meschberger-3
Hi Radu

Thanks for starting this thread.

As I wrote in my other message [1], the ScriptEngine management is currently laid out to have script engines to use disjoint extensions. Having two engines with the same extension is not supported.

I also don’t really like to have multiple engines to have the same extension. And using „html“ as the extension of a script really is prone for such collisions.

Also I don’t like this path thing because it is very brittle and we have enough troubles with them in the Servlet Resolver already …

Lets see how Unix' exec() system call does it: It actually ignores the extension and looks for a magic file pattern at the beginning of the file „#!“.

So we could extend our ScriptResolution as well like this. If the file begins with a line of the form

   #!<engine-name><CR><LF>

then the named script engine is selected by its name, which now is considered to be unique. The selection only looks at the first two characters to decide whether to actually read the first line. If the first line is of this form, it is consumed and the script engine will not be able to read it — this way we also prevent script engines from failing due to incorrect commenting.

If a script does not start with „#!“ then the regular selection by extension kicks in.

WDYT ?

Regards
Felix

> Am 23.01.2015 um 11:17 schrieb Radu Cotescu <[hidden email]>:
>
> Hello,
>
> In SLING-4330 [0] Oliver suggests that Sightly should be configured to
> render templates only from some search paths sub-paths, because he'd also
> like to write some Thymeleaf templates in a project, where the Thymeleaf
> script engine also binds itself to the *.html extension.
>
> However, if we'd like scripting engines to process templates based on
> allowed paths I think that this should happen at a higher level. AFAIK a
> scripting engine registers itself for an extension, but that's about it.
> The behaviour that determines which script engine will provide the
> rendering for a resource is implemented by the SlingServletResolver, which
> adapts the component's resource to a ScriptEngine.
>
> The AdapterFactory that performs the adaptation is implemented by
> SlingScriptAdapterFactory, which picks the scripting engine based solely on
> extension. If we'd change the ScriptEngineFactories to also provide the
> path patterns for which they register and we'd expose the patterns as a
> configurable OSGi property I think we'd reach the goal of having scripting
> engines render templates also based on paths.
>
> WDYT?
>
> Regards,
> Radu
>
> [0] - https://issues.apache.org/jira/browse/SLING-4330

Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Bertrand Delacretaz
On Fri, Jan 23, 2015 at 11:30 AM, Felix Meschberger <[hidden email]> wrote:
> We could extend our ScriptResolution as well like this. If the file begins with a line of the form
>
>    #!<engine-name><CR><LF>

Why not. It's a bit costly as you need to read part of the script
during resolution but I suppose this can be cached.

I'd make the syntax specific to Sling then, maybe like

  #!sling:language thymeleaf

so that people know where to look for the meaning of that.

And of course make the engine's extensions configurable (I hope that's
already the case) so if someone wants to use .sly for Sightly and .tlf
for Thymeleaf that should be possible.

-Bertrand
Reply | Threaded
Open this post in threaded view
|

Re: Re: [discussion] allow Script Engines to also render components based on paths

Oliver Lietz
In reply to this post by Felix Meschberger-3
On Friday 23 January 2015 10:30:26 Felix Meschberger wrote:
> Hi Radu
>
> Thanks for starting this thread.
>
> As I wrote in my other message [1], the ScriptEngine management is currently
> laid out to have script engines to use disjoint extensions. Having two
> engines with the same extension is not supported.
 
> I also don’t really like to have multiple engines to have the same
> extension. And using „html“ as the extension of a script really is prone
> for such collisions.
 
> Also I don’t like this path thing because it is very brittle and we have
> enough troubles with them in the Servlet Resolver already …
 
> Lets see how Unix' exec() system call does it: It actually ignores the
> extension and looks for a magic file pattern at the beginning of the file
> „#!“.
 
> So we could extend our ScriptResolution as well like this. If the file
> begins with a line of the form
 
>    #!<engine-name><CR><LF>
>
> then the named script engine is selected by its name, which now is
> considered to be unique. The selection only looks at the first two
> characters to decide whether to actually read the first line. If the first
> line is of this form, it is consumed and the script engine will not be able
> to read it — this way we also prevent script engines from failing due to
> incorrect commenting.
 
> If a script does not start with „#!“ then the regular selection by extension
> kicks in.
 
> WDYT ?

Adding a Shebang to XML and HTML makes them invalid. So that won't work.

Regards,
O.

> Regards
> Felix
>
>
> > Am 23.01.2015 um 11:17 schrieb Radu Cotescu <[hidden email]>:
> >
> > Hello,
> >
> > In SLING-4330 [0] Oliver suggests that Sightly should be configured to
> > render templates only from some search paths sub-paths, because he'd also
> > like to write some Thymeleaf templates in a project, where the Thymeleaf
> > script engine also binds itself to the *.html extension.
> >
> > However, if we'd like scripting engines to process templates based on
> > allowed paths I think that this should happen at a higher level. AFAIK a
> > scripting engine registers itself for an extension, but that's about it.
> > The behaviour that determines which script engine will provide the
> > rendering for a resource is implemented by the SlingServletResolver,
> > which
> > adapts the component's resource to a ScriptEngine.
> >
> > The AdapterFactory that performs the adaptation is implemented by
> > SlingScriptAdapterFactory, which picks the scripting engine based solely
> > on
 extension. If we'd change the ScriptEngineFactories to also provide

> > the path patterns for which they register and we'd expose the patterns as
> > a configurable OSGi property I think we'd reach the goal of having
> > scripting engines render templates also based on paths.
> >
> > WDYT?
> >
> > Regards,
> > Radu
> >
> > [0] - https://issues.apache.org/jira/browse/SLING-4330
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Oliver Lietz
In reply to this post by Bertrand Delacretaz
On Friday 23 January 2015 11:52:20 Bertrand Delacretaz wrote:
> On Fri, Jan 23, 2015 at 11:30 AM, Felix Meschberger <[hidden email]>
wrote:

> > We could extend our ScriptResolution as well like this. If the file begins
> > with a line of the form>
> >    #!<engine-name><CR><LF>
>
> Why not. It's a bit costly as you need to read part of the script
> during resolution but I suppose this can be cached.
>
> I'd make the syntax specific to Sling then, maybe like
>
>   #!sling:language thymeleaf
>
> so that people know where to look for the meaning of that.
>
> And of course make the engine's extensions configurable (I hope that's
> already the case) so if someone wants to use .sly for Sightly and .tlf
> for Thymeleaf that should be possible.

That's the case for Thymeleaf.

Thymeleaf 3 comes with support for pure templates - HTML/XML without Thymeleaf
tags/attributes at all - so pure HTML/XML.

O.

> -Bertrand

Reply | Threaded
Open this post in threaded view
|

Re: Re: [discussion] allow Script Engines to also render components based on paths

Bertrand Delacretaz
In reply to this post by Oliver Lietz
On Fri, Jan 23, 2015 at 11:55 AM, Oliver Lietz <[hidden email]> wrote:
> ...Adding a Shebang to XML and HTML makes them invalid....

Good point - we can consider that notation say anywhere in the first
200 chars of the script, so

<?xml version="1.0" encoding="UTF-8"?>
<!-- sling:language: thymeleaf -->
<foo/>

would work, the pattern being "sling:language" followed by whitespace
and one word which is the language name.

-Bertrand
Reply | Threaded
Open this post in threaded view
|

Re: Re: [discussion] allow Script Engines to also render components based on paths

Bertrand Delacretaz
On Fri, Jan 23, 2015 at 12:15 PM, Bertrand Delacretaz
<[hidden email]> wrote:
> ... the pattern being "sling:language" followed by whitespace
> and one word which is the language name....

Or maybe "sling:script" to avoid confusion with human languages.

-Bertrand
Reply | Threaded
Open this post in threaded view
|

Re: Re: [discussion] allow Script Engines to also render components based on paths

Radu Cotescu-3
In reply to this post by Bertrand Delacretaz
On Fri, Jan 23, 2015 at 1:15 PM, Bertrand Delacretaz <[hidden email]
> wrote:

> Good point - we can consider that notation say anywhere in the first
> 200 chars of the script, so
>
> <?xml version="1.0" encoding="UTF-8"?>
> <!-- sling:language: thymeleaf -->
> <foo/>
>
> would work, the pattern being "sling:language" followed by whitespace
> and one word which is the language name.
>

This has the disadvantage that the Servlet Resolver would have to parse the
first n characters of the script and then the Script Engine would parse
again the whole file. We should not increase the number of resource reads,
if not really needed.

If we want to have a conflict resolution mechanism we could add a property
on component nodes (but they must be sling:Folder / nt:unstructured) that
would handle which of the multiple scripting engines registered for the
same extension should be used:

"sling:script" -> "html:thymeleaf"

This would essentially say that for the *.html scripts for this component,
Thymeleaf should be used.

However, this adds an unnecessary step for component developers. I think
the configurable patterns on the ScriptEngineFactories is simpler, as this
is essentially a deployment issue.

Whatever the approach is, if we want to allow multiple scripting engines to
use the same extension we need to refactor the code from the
SlingScriptEngineManager.
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Oliver Lietz
On Friday 23 January 2015 13:33:37 Radu Cotescu wrote:

> On Fri, Jan 23, 2015 at 1:15 PM, Bertrand Delacretaz <[hidden email]
> > wrote:
> >
> > Good point - we can consider that notation say anywhere in the first
> > 200 chars of the script, so
> >
> > <?xml version="1.0" encoding="UTF-8"?>
> > <!-- sling:language: thymeleaf -->
> > <foo/>
> >
> > would work, the pattern being "sling:language" followed by whitespace
> > and one word which is the language name.
>
> This has the disadvantage that the Servlet Resolver would have to parse the
> first n characters of the script and then the Script Engine would parse
> again the whole file. We should not increase the number of resource reads,
> if not really needed.
>
> If we want to have a conflict resolution mechanism we could add a property
> on component nodes (but they must be sling:Folder / nt:unstructured) that
> would handle which of the multiple scripting engines registered for the
> same extension should be used:
>
> "sling:script" -> "html:thymeleaf"
>
> This would essentially say that for the *.html scripts for this component,
> Thymeleaf should be used.
>
> However, this adds an unnecessary step for component developers. I think
> the configurable patterns on the ScriptEngineFactories is simpler, as this
> is essentially a deployment issue.

Totally agree.

> Whatever the approach is, if we want to allow multiple scripting engines to
> use the same extension we need to refactor the code from the
> SlingScriptEngineManager.

Why? The ScriptEngine itself can decide if it kicks in for a script based on
the paths patterns.

O.

Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Robert Munteanu-3
On Fri, Jan 23, 2015 at 1:59 PM, Oliver Lietz <[hidden email]> wrote:
>
>> Whatever the approach is, if we want to allow multiple scripting engines to
>> use the same extension we need to refactor the code from the
>> SlingScriptEngineManager.
>
> Why? The ScriptEngine itself can decide if it kicks in for a script based on
> the paths patterns.

If we configure various ScriptEngine implementations it becomes harder
to get an overview of what it configured where. It's even possible to
have conflicting patterns in two scripting engines.

Robert

--
Sent from my (old) computer
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Radu Cotescu-3
On Fri, Jan 23, 2015 at 2:08 PM, Robert Munteanu <[hidden email]> wrote:

> If we configure various ScriptEngine implementations it becomes harder
> to get an overview of what it configured where. It's even possible to
> have conflicting patterns in two scripting engines.
>

We could have a master component that provides the needed configuration for
all engines, simplifying the configuration issue you mentioned.
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Radu Cotescu-3
In reply to this post by Oliver Lietz
On Fri, Jan 23, 2015 at 1:59 PM, Oliver Lietz <[hidden email]> wrote:

> > Whatever the approach is, if we want to allow multiple scripting engines
> to
> > use the same extension we need to refactor the code from the
> > SlingScriptEngineManager.
>
> Why? The ScriptEngine itself can decide if it kicks in for a script based
> on
> the paths patterns.



Because currently the script engines are just template processors. The
SlingServletResolver is the component that maps a request to the script
engine that will provide the output, with the help of the
SlingScriptAdapterFactory and the SlingScriptEngineManager.
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Felix Meschberger-3
Hi

> Am 23.01.2015 um 13:29 schrieb Radu Cotescu <[hidden email]>:
>
> On Fri, Jan 23, 2015 at 1:59 PM, Oliver Lietz <[hidden email]> wrote:
>
>>> Whatever the approach is, if we want to allow multiple scripting engines
>> to
>>> use the same extension we need to refactor the code from the
>>> SlingScriptEngineManager.
>>
>> Why? The ScriptEngine itself can decide if it kicks in for a script based
>> on
>> the paths patterns.
>
>
>
> Because currently the script engines are just template processors. The
> SlingServletResolver is the component that maps a request to the script
> engine that will provide the output, with the help of the
> SlingScriptAdapterFactory and the SlingScriptEngineManager.

Exactly: The SlingScriptAdapterFactory checks the extension of the script and selects the ScriptEngine for this extension. Only one ScriptEngineFactory of a particular extension can be registered at any one time. So this one is select and called to evaluate the script. If the ScriptEngine decides to not actually evaluate it, nothing more happens.

We have to make sure we improve on the ScriptEngine selection mechanism. I don’t want to cycle through multiple ScriptEngine’s until one decides to handle….

Regards
Felix
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Felix Meschberger-3
In reply to this post by Oliver Lietz
Hi

> Am 23.01.2015 um 11:55 schrieb Oliver Lietz <[hidden email]>:
>
> On Friday 23 January 2015 10:30:26 Felix Meschberger wrote:
>> Hi Radu
>>
>> Thanks for starting this thread.
>>
>> As I wrote in my other message [1], the ScriptEngine management is currently
>> laid out to have script engines to use disjoint extensions. Having two
>> engines with the same extension is not supported.
>
>> I also don’t really like to have multiple engines to have the same
>> extension. And using „html“ as the extension of a script really is prone
>> for such collisions.
>
>> Also I don’t like this path thing because it is very brittle and we have
>> enough troubles with them in the Servlet Resolver already …
>
>> Lets see how Unix' exec() system call does it: It actually ignores the
>> extension and looks for a magic file pattern at the beginning of the file
>> „#!“.
>
>> So we could extend our ScriptResolution as well like this. If the file
>> begins with a line of the form
>
>>   #!<engine-name><CR><LF>
>>
>> then the named script engine is selected by its name, which now is
>> considered to be unique. The selection only looks at the first two
>> characters to decide whether to actually read the first line. If the first
>> line is of this form, it is consumed and the script engine will not be able
>> to read it — this way we also prevent script engines from failing due to
>> incorrect commenting.
>
>> If a script does not start with „#!“ then the regular selection by extension
>> kicks in.
>
>> WDYT ?
>
> Adding a Shebang to XML and HTML makes them invalid. So that won't work.

Technically yes, but from a processing perspective no: because the SlingScriptAdapterFactory will it it up and the ScriptEngine should not be able to see it anymore.

Regards
Felix

>
> Regards,
> O.
>
>> Regards
>> Felix
>>
>>
>>> Am 23.01.2015 um 11:17 schrieb Radu Cotescu <[hidden email]>:
>>>
>>> Hello,
>>>
>>> In SLING-4330 [0] Oliver suggests that Sightly should be configured to
>>> render templates only from some search paths sub-paths, because he'd also
>>> like to write some Thymeleaf templates in a project, where the Thymeleaf
>>> script engine also binds itself to the *.html extension.
>>>
>>> However, if we'd like scripting engines to process templates based on
>>> allowed paths I think that this should happen at a higher level. AFAIK a
>>> scripting engine registers itself for an extension, but that's about it.
>>> The behaviour that determines which script engine will provide the
>>> rendering for a resource is implemented by the SlingServletResolver,
>>> which
>>> adapts the component's resource to a ScriptEngine.
>>>
>>> The AdapterFactory that performs the adaptation is implemented by
>>> SlingScriptAdapterFactory, which picks the scripting engine based solely
>>> on
> extension. If we'd change the ScriptEngineFactories to also provide
>>> the path patterns for which they register and we'd expose the patterns as
>>> a configurable OSGi property I think we'd reach the goal of having
>>> scripting engines render templates also based on paths.
>>>
>>> WDYT?
>>>
>>> Regards,
>>> Radu
>>>
>>> [0] - https://issues.apache.org/jira/browse/SLING-4330
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Carsten Ziegeler
Am 23.01.15 um 06:04 schrieb Felix Meschberger:
>>
>> Adding a Shebang to XML and HTML makes them invalid. So that won't work.
>
> Technically yes, but from a processing perspective no: because the SlingScriptAdapterFactory will it it up and the ScriptEngine should not be able to see it anymore.
>
But wouldn't that mean that we potentially loose the tooling advantages
as the tools might not cope with the extra line at the beginning and
consider the file invalid?

Carsten
--
Carsten Ziegeler
Adobe Research Switzerland
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Carsten Ziegeler
In reply to this post by Felix Meschberger-3
What about adding an additional property to the script resource?

Carsten
--
Carsten Ziegeler
Adobe Research Switzerland
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Felix Meschberger-3
Reminds me of the Mac Resource Forks … :-)

It would be neat to be able to set this in/on the script itself without having to manage some out-of-band metadata.

Regards
Felix

> Am 23.01.2015 um 15:15 schrieb Carsten Ziegeler <[hidden email]>:
>
> What about adding an additional property to the script resource?
>
> Carsten
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

justzzzz
FWIW, I think it would be more natural to set this on the container
node, so if you have /apps/foo/bar/something.html, I'd set
/apps/foo/bar@sling:script = html:thymeleaf.

We could, however, support both per-script and per-container settings,
but I don't think it is necessary to go more than one level up.

Regards,
Justin

On Fri, Jan 23, 2015 at 9:18 AM, Felix Meschberger <[hidden email]> wrote:

> Reminds me of the Mac Resource Forks … :-)
>
> It would be neat to be able to set this in/on the script itself without having to manage some out-of-band metadata.
>
> Regards
> Felix
>
>> Am 23.01.2015 um 15:15 schrieb Carsten Ziegeler <[hidden email]>:
>>
>> What about adding an additional property to the script resource?
>>
>> Carsten
>> --
>> Carsten Ziegeler
>> Adobe Research Switzerland
>> [hidden email]
>
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

chetan mehrotra
In reply to this post by Carsten Ziegeler
On Fri, Jan 23, 2015 at 7:45 PM, Carsten Ziegeler <[hidden email]> wrote:
> What about adding an additional property to the script resource?

+1. This looks more safer and less disrupting as reading the property
is cheaper.

The script header approach looks nice but would require changes at
quite a few places

Chetan Mehrotra
Reply | Threaded
Open this post in threaded view
|

Re: [discussion] allow Script Engines to also render components based on paths

Radu Cotescu-3
On Fri, Jan 23, 2015 at 4:21 PM, Chetan Mehrotra <[hidden email]>
wrote:

> +1. This looks more safer and less disrupting as reading the property
> is cheaper.
>

But we'd need a mixin, as scripts are usually nt:file nodes. Right?
12