You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Ryan Baxter <rb...@gmail.com> on 2012/02/16 22:57:16 UTC
Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It would be much easier for consumers of the common container to supply a list of allowed RPC service ids for this container in the config.
This addresses bug SHINDIG-1709.
https://issues.apache.org/jira/browse/SHINDIG-1709
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1245178
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
Diff: https://reviews.apache.org/r/3938/diff
Testing
-------
Tested in common container
Thanks,
Ryan
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Ryan Baxter <rb...@gmail.com>.
> On 2012-02-23 18:10:07, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java, line 51
> > <https://reviews.apache.org/r/3938/diff/3/?file=76150#file76150line51>
> >
> > Not sure if we need this annotation. Probably just add @since instead. And remember we are releasing 2.5.0 instead of 3.0.0 for next release.
Good point, I will fix that.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5311
-----------------------------------------------------------
On 2012-02-20 21:48:36, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 21:48:36)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Updated and added unit tests. Also tested in common container.
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5311
-----------------------------------------------------------
Ship it!
+1 with small nit
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java
<https://reviews.apache.org/r/3938/#comment11556>
Not sure if we need this annotation. Probably just add @since instead. And remember we are releasing 2.5.0 instead of 3.0.0 for next release.
- Henry
On 2012-02-20 21:48:36, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 21:48:36)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Updated and added unit tests. Also tested in common container.
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/
-----------------------------------------------------------
(Updated 2012-02-23 20:19:33.079998)
Review request for shindig.
Changes
-------
Updated based on Henry's comments.
Summary
-------
If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
This addresses bug SHINDIG-1709.
https://issues.apache.org/jira/browse/SHINDIG-1709
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1292933
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1292933
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1292933
Diff: https://reviews.apache.org/r/3938/diff
Testing
-------
Updated and added unit tests. Also tested in common container.
Thanks,
Ryan
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5307
-----------------------------------------------------------
Ship it!
LGTM
- Stanton
On 2012-02-20 21:48:36, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 21:48:36)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Updated and added unit tests. Also tested in common container.
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/
-----------------------------------------------------------
(Updated 2012-02-20 21:48:36.261908)
Review request for shindig.
Changes
-------
Updated implementation based on gadget admin features.
Summary (updated)
-------
If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
This addresses bug SHINDIG-1709.
https://issues.apache.org/jira/browse/SHINDIG-1709
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1245178
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1245178
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1245178
Diff: https://reviews.apache.org/r/3938/diff
Testing (updated)
-------
Updated and added unit tests. Also tested in common container.
Thanks,
Ryan
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Ryan Baxter <rb...@gmail.com>.
> On 2012-02-16 22:35:47, Stanton Sievers wrote:
> > I think this would make more sense to add to the existing gadget-admin.json file instead of into the container.js config. It would keep all of the "admin" stuff in one place.
> >
> > So today we have something like this in gadget-admin.json:
> >
> > {
> > "default" : {
> > "gadgets" : {
> > "http://www.google.com/ig/modules/horoscope.xml" : {
> > "features" : ["views", "tabs", "setprefs", "dynamic-height"],
> > "type" : "blacklist"
> > }
> > }
> > }
> > }
> >
> > You could just add another attribute that is a sibling to "gadgets" (name pending)
> >
> > {
> > "default" : {
> > "gadgets" : {
> > "http://www.google.com/ig/modules/horoscope.xml" : {
> > "features" : ["views", "tabs", "setprefs", "dynamic-height"],
> > "type" : "blacklist"
> > }
> > },
> > "additional_rpc_endpoints" : ["foobar"]
> > }
> > }
> >
> > You could read the new array on the server-side when processing the list of allowed endpoints from the feature.xmls before shipping that info off to the container.
Good thought Stanton!
I think I would like to modify your suggestion a bit though. I think in some scenarios it would be useful to specify the additional RPC endpoints on a per gadget basis, so I am envisioning something more like this
{
"default" : {
"gadgets" : {
"http://www.google.com/ig/modules/horoscope.xml" : {
"features" : {
"name" : ["views", "tabs", "setprefs", "dynamic-height"],
"type" : "blacklist"
},
"rpc" : {
"additionalEndpoints" : ["foobar"]
}
},
"*" : {
"rpc" : {
"additionalEndpoints" : ["helloworld"]
}
}
}
}
}
This format lets you specify additional endpoints to allow on a per gadget basis. You can use the * syntax to denote ones that should be allowed for all gadgets. What do you think?
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5172
-----------------------------------------------------------
On 2012-02-16 21:57:16, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-16 21:57:16)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It would be much easier for consumers of the common container to supply a list of allowed RPC service ids for this container in the config.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Tested in common container
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Ryan Baxter <rb...@gmail.com>.
> On 2012-02-16 22:35:47, Stanton Sievers wrote:
> > I think this would make more sense to add to the existing gadget-admin.json file instead of into the container.js config. It would keep all of the "admin" stuff in one place.
> >
> > So today we have something like this in gadget-admin.json:
> >
> > {
> > "default" : {
> > "gadgets" : {
> > "http://www.google.com/ig/modules/horoscope.xml" : {
> > "features" : ["views", "tabs", "setprefs", "dynamic-height"],
> > "type" : "blacklist"
> > }
> > }
> > }
> > }
> >
> > You could just add another attribute that is a sibling to "gadgets" (name pending)
> >
> > {
> > "default" : {
> > "gadgets" : {
> > "http://www.google.com/ig/modules/horoscope.xml" : {
> > "features" : ["views", "tabs", "setprefs", "dynamic-height"],
> > "type" : "blacklist"
> > }
> > },
> > "additional_rpc_endpoints" : ["foobar"]
> > }
> > }
> >
> > You could read the new array on the server-side when processing the list of allowed endpoints from the feature.xmls before shipping that info off to the container.
>
> Ryan Baxter wrote:
> Good thought Stanton!
>
> I think I would like to modify your suggestion a bit though. I think in some scenarios it would be useful to specify the additional RPC endpoints on a per gadget basis, so I am envisioning something more like this
>
> {
> "default" : {
> "gadgets" : {
> "http://www.google.com/ig/modules/horoscope.xml" : {
> "features" : {
> "name" : ["views", "tabs", "setprefs", "dynamic-height"],
> "type" : "blacklist"
> },
> "rpc" : {
> "additionalEndpoints" : ["foobar"]
> }
> },
> "*" : {
> "rpc" : {
> "additionalEndpoints" : ["helloworld"]
> }
> }
> }
> }
> }
>
> This format lets you specify additional endpoints to allow on a per gadget basis. You can use the * syntax to denote ones that should be allowed for all gadgets. What do you think?
One slight change to my example...
{
"default" : {
"gadgets" : {
"http://www.google.com/ig/modules/horoscope.xml" : {
"features" : {
"name" : ["views", "tabs", "setprefs", "dynamic-height"],
"type" : "blacklist"
},
"rpc" : {
"additionalEndpoints" : ["foobar"]
}
},
"http://*" : {
"rpc" : {
"additionalEndpoints" : ["helloworld"]
}
}
}
}
}
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5172
-----------------------------------------------------------
On 2012-02-20 21:48:36, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-20 21:48:36)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It be easier if containers could specify a list of additional RPC services to allow on a per gadget basis. It would also be nice if this was integrated into the gadget admin feature in Shindig already.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/gadget-admin.json 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminData.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/GadgetAdminStore.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/admin/RpcAdminData.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/BasicGadgetAdminStoreTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ContainerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/GadgetAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/RpcAdminDataTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/admin/ServerAdminDataTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Updated and added unit tests. Also tested in common container.
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: The RPC abritration code in the common container should
allow you to pass in a list of allowed RPC services for that container
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3938/#review5172
-----------------------------------------------------------
I think this would make more sense to add to the existing gadget-admin.json file instead of into the container.js config. It would keep all of the "admin" stuff in one place.
So today we have something like this in gadget-admin.json:
{
"default" : {
"gadgets" : {
"http://www.google.com/ig/modules/horoscope.xml" : {
"features" : ["views", "tabs", "setprefs", "dynamic-height"],
"type" : "blacklist"
}
}
}
}
You could just add another attribute that is a sibling to "gadgets" (name pending)
{
"default" : {
"gadgets" : {
"http://www.google.com/ig/modules/horoscope.xml" : {
"features" : ["views", "tabs", "setprefs", "dynamic-height"],
"type" : "blacklist"
}
},
"additional_rpc_endpoints" : ["foobar"]
}
}
You could read the new array on the server-side when processing the list of allowed endpoints from the feature.xmls before shipping that info off to the container.
- Stanton
On 2012-02-16 21:57:16, Ryan Baxter wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3938/
> -----------------------------------------------------------
>
> (Updated 2012-02-16 21:57:16)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> If you enable RPC arbitration in a container and you want to allow a set of RPC service ids regardless of whether the gadgets has features that uses them, it is very difficult to do so. The only way to do this today is to provide your own arbitrator function to the common container. Essentially this function will do the same thing as the default implementation in the common container except have a list of allowed RPC service ids. It would be much easier for consumers of the common container to supply a list of allowed RPC service ids for this container in the config.
>
>
> This addresses bug SHINDIG-1709.
> https://issues.apache.org/jira/browse/SHINDIG-1709
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/constant.js 1245178
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1245178
>
> Diff: https://reviews.apache.org/r/3938/diff
>
>
> Testing
> -------
>
> Tested in common container
>
>
> Thanks,
>
> Ryan
>
>