Shindig
  1. Shindig
  2. SHINDIG-1709

The RPC abritration code in the common container should allow you to pass in a list of allowed RPC services for that container.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5.0-beta1
    • Component/s: None
    • Labels:
      None

      Description

      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.

        Activity

        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        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" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > }

        > }

        > }

        > }

        >

        > You could just add another attribute that is a sibling to "gadgets" (name pending)

        >

        > {

        > "default" : {

        > "gadgets" : {

        > "http://www.google.com/ig/modules/horoscope.xml" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > }

        > },

        > "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

        Show
        jiraposter@reviews.apache.org added a comment - 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" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > } > } > } > } > > You could just add another attribute that is a sibling to "gadgets" (name pending) > > { > "default" : { > "gadgets" : { > "http://www.google.com/ig/modules/horoscope.xml" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > } > }, > "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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        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" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > }

        > }

        > }

        > }

        >

        > You could just add another attribute that is a sibling to "gadgets" (name pending)

        >

        > {

        > "default" : {

        > "gadgets" : {

        > "http://www.google.com/ig/modules/horoscope.xml" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > }

        > },

        > "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" : { bq. "name" : ["views", "tabs", "setprefs", "dynamic-height"], bq. "type" : "blacklist" bq. },

        "rpc" : { bq. "additionalEndpoints" : ["foobar"] bq. }

        },

        "*" : {

        "rpc" : { bq. "additionalEndpoints" : ["helloworld"] bq. }

        }

        }

        }

        }

        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

        Show
        jiraposter@reviews.apache.org added a comment - 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" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > } > } > } > } > > You could just add another attribute that is a sibling to "gadgets" (name pending) > > { > "default" : { > "gadgets" : { > "http://www.google.com/ig/modules/horoscope.xml" : { bq. > "features" : ["views", "tabs", "setprefs", "dynamic-height"], bq. > "type" : "blacklist" bq. > } > }, > "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" : { bq. "name" : ["views", "tabs", "setprefs", "dynamic-height"], bq. "type" : "blacklist" bq. }, "rpc" : { bq. "additionalEndpoints" : ["foobar"] bq. } }, "*" : { "rpc" : { bq. "additionalEndpoints" : ["helloworld"] bq. } } } } } 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        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

        Show
        jiraposter@reviews.apache.org added a comment - 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
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        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

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- 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
        Hide
        Ryan Baxter added a comment -

        Committed revision 1292940.

        Show
        Ryan Baxter added a comment - Committed revision 1292940.
        Hide
        Paul Lindner added a comment -

        part of 2.5.0-beta1 release.

        Show
        Paul Lindner added a comment - part of 2.5.0-beta1 release.

          People

          • Assignee:
            Ryan Baxter
            Reporter:
            Ryan Baxter
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development