You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Bertrand Delacretaz <bd...@apache.org> on 2013/09/05 08:56:00 UTC

Re: Adding a "usage" service property to BindingsValuesProvider ?

Hi Justin,

On Tuesday, August 6, 2013, Justin Edelson wrote:

> ...Whereas today you do:
> ScriptEngineManager.getEngineByExtension("ecma")
>
> You could do
> ScriptEngineManager.getEngineByExtension("ecma", ["healthcheck",
"workflow"]) ...

FYI, I'm getting back to this (SLING-3038) and the above won't work for my
use case, as I need to setup some initial Bindings that provide context
objects, before calling BindingValuesProvider.addBindings(...).

The scenario that I'm considering is:

String context = "healthcheck"; // I don't think we need multiple contexts??
ScriptEngine engine = scriptEngineManager.getEngineByExtension("foo");
Bindings b = engine.createBindings();
b.put("bar", new SomethingThatOtherBindingsNeed());
for(BindingValuesProvider bvp : someProvider.getBindings(engine, context)) {
  bvp.addBindings(b);
}

where someProvider is a new service that I'm afraid might be called
BindingValuesProviderProvider...suggestions welcome ;-)

In the above loop, some BVPs use the "bar" object to create their bound
objects, as we already do with the BVP that provides "currentNode" if
there's a "resource" bound when it's called.

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Fri, Sep 6, 2013 at 10:43 PM, Felix Meschberger <fm...@adobe.com> wrote:
> Hmm, why not turning this around and ammed the BindingsValueProvider interface ?
>
>> public interface BindingsValuesProvider2 extends BindingsValueProvider {
>>     void addBindings(Bindings bindings, ScriptEngineFactory, String context);
>> }
>>...

What we have now is clearer IMO:

BindingsValuesProvider always was and remains a simple provider of
Bindings, that can be marked via service properties to apply to
specific languages and contexts.

BindingsValuesProvidersByContext is a source of
BindingsValuesProvider, which takes context and languages into account
to select them. Clients can also select the BVPs themselves directly
based on their service properties, if that's more convenient or to
avoid having to require the latest scripting api.

What you're suggesting would be more confusing IMO, as having to use
instanceof is somewhat ugly.

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Felix Meschberger <fm...@adobe.com>.
Hmm, why not turning this around and ammed the BindingsValueProvider interface ?

> public interface BindingsValuesProvider2 extends BindingsValueProvider {
>     void addBindings(Bindings bindings, ScriptEngineFactory, String context);
> }
> 

The BindingsValuesProvider2 is a marker interface to the BindginsValuesProvider service interface. A consumer would do

> BindingsValuesProvider[] bvps = getProviders();
> foreach (BindingsValuesProvider bvp: bvps) {
>   if (bvp instanceof BindingsValuesProvider2) {
>      // use with scope
>   } else {
>      // use without scope
>   }
> }
> 

Regards
Felix

Am 06.09.2013 um 06:04 schrieb Bertrand Delacretaz:

> On Thu, Sep 5, 2013 at 1:28 PM, Bertrand Delacretaz
> <bd...@apache.org> wrote:
>> ...Maybe
>> 
>>  addContextBindings(Bindings b, ScriptEngineFactory f, String context)
>> ...
> 
> FYI I didn't do it like that in the end, as that would have caused
> more changes than I like to existing code. The new service API is at
> https://svn.apache.org/repos/asf/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/BindingsValuesProvidersByContext.java
> 
> Apart from that this is now done, see SLING-3038 for details,
> 
> -Bertrand


Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Felix Meschberger <fm...@adobe.com>.
Hi

What about Import-Package version ? Which is why I poke at the @ConsumerType annotation.

@ConsumerType: We promise not to change the API unless we increase the major version of the export

@ProviderType: We promise not to change the API unless we increase the minor version of the export

Regards
Felix

Am 09.09.2013 um 16:24 schrieb Bertrand Delacretaz:

> Hi,
> 
> On Mon, Sep 9, 2013 at 3:26 PM, Felix Meschberger <fm...@adobe.com> wrote:
>> Am 09.09.2013 um 11:45 schrieb Bertrand Delacretaz:
>>> ...done in revision 1521017
>> ...
>> You added a constant to the interface, so what will the export package version be ? and the compatibility consequences ? Is this interface @ConsumerType...
> 
> I have already bumped the api package version to 2.2.0, from 2.1.0, in
> http://svn.apache.org/r1520565 (in the api module pom), after adding
> the new BindingsValuesProvidersByContext interface.
> 
> I don't think adding new constants to BindingsValuesProviders has any
> consequences for implementing classes, they won't see any difference.
> or will they?
> 
> -Bertrand


Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Mon, Sep 9, 2013 at 3:26 PM, Felix Meschberger <fm...@adobe.com> wrote:
> Am 09.09.2013 um 11:45 schrieb Bertrand Delacretaz:
>>...done in revision 1521017
>...
> You added a constant to the interface, so what will the export package version be ? and the compatibility consequences ? Is this interface @ConsumerType...

I have already bumped the api package version to 2.2.0, from 2.1.0, in
http://svn.apache.org/r1520565 (in the api module pom), after adding
the new BindingsValuesProvidersByContext interface.

I don't think adding new constants to BindingsValuesProviders has any
consequences for implementing classes, they won't see any difference.
or will they?

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Felix Meschberger <fm...@adobe.com>.
Hi


Am 09.09.2013 um 11:45 schrieb Bertrand Delacretaz:

> On Fri, Sep 6, 2013 at 3:31 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> ...The constants should really be defined in the BVP class, this is the
>> service declaring them...
> 
> Good point, done in revision 1521017

Hmm. While this makes sense, doesn't it introduce versioning problems ? 

You added a constant to the interface, so what will the export package version be ? and the compatibility consequences ? Is this interface @ConsumerType

Regards
Felix



> 
>> 
>> ... I don't understand what you mean by "changes to existing code" - this is
>> new, isn't it?...
> 
> Yes but existing code does use BindingsValuesProvider, and
> BindingsValuesProvidersByContext returning a Collection causes less
> changes to that existing code than the addContextBindings method that
> we discussed. I don't think one is better than the other anyway, it's
> just slightly different styles.
> 
> -Bertrand


Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Fri, Sep 6, 2013 at 3:31 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...The constants should really be defined in the BVP class, this is the
> service declaring them...

Good point, done in revision 1521017

>
>... I don't understand what you mean by "changes to existing code" - this is
> new, isn't it?...

Yes but existing code does use BindingsValuesProvider, and
BindingsValuesProvidersByContext returning a Collection causes less
changes to that existing code than the addContextBindings method that
we discussed. I don't think one is better than the other anyway, it's
just slightly different styles.

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Carsten Ziegeler <cz...@apache.org>.
The constants should really be defined in the BVP class, this is the
service declaring them

I don't understand what you mean by "changes to existing code" - this is
new, isn't it?

Carsten


2013/9/6 Bertrand Delacretaz <bd...@apache.org>

> On Thu, Sep 5, 2013 at 1:28 PM, Bertrand Delacretaz
> <bd...@apache.org> wrote:
> > ...Maybe
> >
> >   addContextBindings(Bindings b, ScriptEngineFactory f, String context)
> > ...
>
> FYI I didn't do it like that in the end, as that would have caused
> more changes than I like to existing code. The new service API is at
>
> https://svn.apache.org/repos/asf/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/BindingsValuesProvidersByContext.java
>
> Apart from that this is now done, see SLING-3038 for details,
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Thu, Sep 5, 2013 at 1:28 PM, Bertrand Delacretaz
<bd...@apache.org> wrote:
> ...Maybe
>
>   addContextBindings(Bindings b, ScriptEngineFactory f, String context)
> ...

FYI I didn't do it like that in the end, as that would have caused
more changes than I like to existing code. The new service API is at
https://svn.apache.org/repos/asf/sling/trunk/bundles/scripting/api/src/main/java/org/apache/sling/scripting/api/BindingsValuesProvidersByContext.java

Apart from that this is now done, see SLING-3038 for details,

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Carsten Ziegeler <cz...@apache.org>.
As our api is in the api bundle, well api it is :)

The package version will be 2.2.0 as this is adding new api

Carsten


2013/9/5 Bertrand Delacretaz <bd...@apache.org>

> On Thursday, September 5, 2013, Carsten Ziegeler wrote:
>
> > If you need to know the language, can't we pass this as a string into the
> > service?...
>
> It's not only the language, some other ScriptEngineFactory metadata such as
> compatible.javax.script.name is used.
>
> Now, where should that new service interface go, scripting.api bundle or
> scripting.core?
>
> I'd say scripting.api, and we bump the API package's version to 2.1.1 (from
> the current 2.1.0).
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Thursday, September 5, 2013, Carsten Ziegeler wrote:

> If you need to know the language, can't we pass this as a string into the
> service?...

It's not only the language, some other ScriptEngineFactory metadata such as
compatible.javax.script.name is used.

Now, where should that new service interface go, scripting.api bundle or
scripting.core?

I'd say scripting.api, and we bump the API package's version to 2.1.1 (from
the current 2.1.0).

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Carsten Ziegeler <cz...@apache.org>.
If you need to know the language, can't we pass this as a string into the
service?
Other than that, add sounds good to me (don't know why I picked apply
anyway...)

Carsten


2013/9/5 Bertrand Delacretaz <bd...@apache.org>

> On Thu, Sep 5, 2013 at 10:14 AM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > What about defining a service (whatever name :) ) that has something
> like a:
> > applyBindings(Bindings b , String context)
> > method?...
>
> Maybe
>
>   addContextBindings(Bindings b, ScriptEngineFactory f, String context)
>
> As the ScriptEngineFactory is needed to know what the current language
> is, and "add" for consistent naming with the existing
> BindingsValuesProvider.addBindings(Bindings b) method.
>
> WDYT?
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Bertrand Delacretaz <bd...@apache.org>.
On Thu, Sep 5, 2013 at 10:14 AM, Carsten Ziegeler <cz...@apache.org> wrote:
> What about defining a service (whatever name :) ) that has something like a:
> applyBindings(Bindings b , String context)
> method?...

Maybe

  addContextBindings(Bindings b, ScriptEngineFactory f, String context)

As the ScriptEngineFactory is needed to know what the current language
is, and "add" for consistent naming with the existing
BindingsValuesProvider.addBindings(Bindings b) method.

WDYT?

-Bertrand

Re: Adding a "usage" service property to BindingsValuesProvider ?

Posted by Carsten Ziegeler <cz...@apache.org>.
What about defining a service (whatever name :) ) that has something like a:
applyBindings(Bindings b , String context)
method?

Internally this service can do whatever magic it has to do, like looking up
all BVPs with the specified context attribute set etc.
You prefill your bindings with the objs you think that should be there
before calling that service.

If you want to support multiple contexts, simply call this method more than
once with the different context names.

Carsten


2013/9/5 Bertrand Delacretaz <bd...@apache.org>

> Hi Justin,
>
> On Tuesday, August 6, 2013, Justin Edelson wrote:
>
> > ...Whereas today you do:
> > ScriptEngineManager.getEngineByExtension("ecma")
> >
> > You could do
> > ScriptEngineManager.getEngineByExtension("ecma", ["healthcheck",
> "workflow"]) ...
>
> FYI, I'm getting back to this (SLING-3038) and the above won't work for my
> use case, as I need to setup some initial Bindings that provide context
> objects, before calling BindingValuesProvider.addBindings(...).
>
> The scenario that I'm considering is:
>
> String context = "healthcheck"; // I don't think we need multiple
> contexts??
> ScriptEngine engine = scriptEngineManager.getEngineByExtension("foo");
> Bindings b = engine.createBindings();
> b.put("bar", new SomethingThatOtherBindingsNeed());
> for(BindingValuesProvider bvp : someProvider.getBindings(engine, context))
> {
>   bvp.addBindings(b);
> }
>
> where someProvider is a new service that I'm afraid might be called
> BindingValuesProviderProvider...suggestions welcome ;-)
>
> In the above loop, some BVPs use the "bar" object to create their bound
> objects, as we already do with the BVP that provides "currentNode" if
> there's a "resource" bound when it's called.
>
> -Bertrand
>



-- 
Carsten Ziegeler
cziegeler@apache.org