Shindig
  1. Shindig
  2. SHINDIG-1548

Shindig does not accept relative URLs for template libraries

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.5.0-beta1
    • Fix Version/s: 2.5.0-beta2
    • Component/s: Java
    • Labels:

      Description

      Section 15.1 of the OpenSocial Templating Spec (version 1.1 at <http://opensocial-resources.googlecode.com/svn/spec/1.1/OpenSocial-Templating.xml#rfc.section.15.1>, version 2.0 draft at <http://opensocial-resources.googlecode.com/svn/spec/2.0/OpenSocial-Templating.xml#rfc.section.15.1> states that, when interpreting the URL in a template library request like this:

      <Require feature="opensocial-templates">
        <Param name="requireLibrary">templates.xml</Param>
      </Require>                                    
      

      that "Relative URLs are interpreted in relation to the location of the Gadget Spec XML file." However, Shindig currently generates a /gadgets/makeRequest call and passes in the relative URL value ("templates.xml") and fails with an HTTP status of 400 (Bad Request) because this URL does not start with "http:" or "https:".

      1. 1548.patch
        3 kB
        Erik BI
      2. RelativeTemplateLibrary.xml
        1 kB
        Erik BI
      3. TestTemplateLibrary.xml
        0.3 kB
        Erik BI

        Activity

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

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/
        -----------------------------------------------------------

        Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

        Summary
        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:
        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.
        https://issues.apache.org/jira/browse/shindig-1548

        Diffs


        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing
        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7065
        -----------------------------------------------------------

        In general I think this is OK. Please follow the Shindig style guidelines when creating patches. Most notably you are using tabs instead of spaces and there is trailing whitespace, which you can see in the diff viewer in red. If you are using Eclipse you should be using the Shindig formatter which you can find here: http://svn.apache.org/repos/asf/shindig/trunk/etc/eclipse/

        • Stanton

        On 2012-04-20 02:53:53, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-20 02:53:53)

        Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7065 ----------------------------------------------------------- In general I think this is OK. Please follow the Shindig style guidelines when creating patches. Most notably you are using tabs instead of spaces and there is trailing whitespace, which you can see in the diff viewer in red. If you are using Eclipse you should be using the Shindig formatter which you can find here: http://svn.apache.org/repos/asf/shindig/trunk/etc/eclipse/ Stanton On 2012-04-20 02:53:53, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-20 02:53:53) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/
        -----------------------------------------------------------

        (Updated 2012-04-24 02:42:35.830930)

        Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

        Changes
        -------

        1. format code using shindig template
        2. change new ArrayList to Lists.newArrayList();

        Summary
        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:
        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.
        https://issues.apache.org/jira/browse/shindig-1548

        Diffs (updated)


        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing
        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-24 02:42:35.830930) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Changes ------- 1. format code using shindig template 2. change new ArrayList to Lists.newArrayList(); Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs (updated) http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/
        -----------------------------------------------------------

        (Updated 2012-04-24 09:18:47.335354)

        Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

        Changes
        -------

        exchange the .equals parameter, move the static final value as the caller to avoid the possible NullPointerException

        Summary
        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:
        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.
        https://issues.apache.org/jira/browse/shindig-1548

        Diffs (updated)


        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing
        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-24 09:18:47.335354) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Changes ------- exchange the .equals parameter, move the static final value as the caller to avoid the possible NullPointerException Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs (updated) http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7170
        -----------------------------------------------------------

        Ship it!

        LGTM. Can you add the Shindig dev list to the review?

        • Stanton

        On 2012-04-24 09:18:47, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-24 09:18:47)

        Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7170 ----------------------------------------------------------- Ship it! LGTM. Can you add the Shindig dev list to the review? Stanton On 2012-04-24 09:18:47, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-24 09:18:47) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/
        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27.056702)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary
        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:
        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.
        https://issues.apache.org/jira/browse/shindig-1548

        Diffs


        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing
        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27.056702) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7221
        -----------------------------------------------------------

        Need time to review the flow but would be nice to add test gadget you use to validate the fix.

        • Henry

        On 2012-04-25 02:27:27, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7221 ----------------------------------------------------------- Need time to review the flow but would be nice to add test gadget you use to validate the fix. Henry On 2012-04-25 02:27:27, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7243
        -----------------------------------------------------------

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java
        <https://reviews.apache.org/r/4818/#comment15976>

        This seems the wrong approach for me. Each ConfigContributor responsible to add additional config from server.

        In this case the CoreUtilConfigContributor is used to add additional logic to push config to init function in the core.util feature.

        Not sure how the opensocial-templates JS code could read this new rewritten requireLibrary param.

        • Henry

        On 2012-04-25 02:27:27, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7243 ----------------------------------------------------------- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java < https://reviews.apache.org/r/4818/#comment15976 > This seems the wrong approach for me. Each ConfigContributor responsible to add additional config from server. In this case the CoreUtilConfigContributor is used to add additional logic to push config to init function in the core.util feature. Not sure how the opensocial-templates JS code could read this new rewritten requireLibrary param. Henry On 2012-04-25 02:27:27, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7252
        -----------------------------------------------------------

        Ship it!

        +1 LGTM

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java
        <https://reviews.apache.org/r/4818/#comment16030>

        Looks like I was wrong. The core.util provides API to get the param for each feature.

        In this case opensocial-templates feature will call gadgets.util.getFeatureParameters(opensocial-templates') to get the requireLibrary param and its been fixed by the server code.

        • Henry

        On 2012-04-25 02:27:27, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7252 ----------------------------------------------------------- Ship it! +1 LGTM http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java < https://reviews.apache.org/r/4818/#comment16030 > Looks like I was wrong. The core.util provides API to get the param for each feature. In this case opensocial-templates feature will call gadgets.util.getFeatureParameters(opensocial-templates') to get the requireLibrary param and its been fixed by the server code. Henry On 2012-04-25 02:27:27, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7253
        -----------------------------------------------------------

        Actually could you also add the gadget.xml you used for testing this?

        • Henry

        On 2012-04-25 02:27:27, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7253 ----------------------------------------------------------- Actually could you also add the gadget.xml you used for testing this? Henry On 2012-04-25 02:27:27, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-04-26 06:29:45, Henry Saputra wrote:

        > Actually could you also add the gadget.xml you used for testing this?

        The way I tested it was changing a gadget to use relative url and check the library template request value in the request via firebug, so I can avoid writing a template library file:)
        Sure, I will write a test gadget&template library file.
        Thanks.

        • Erik

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/4818/#review7253
        -----------------------------------------------------------

        On 2012-04-25 02:27:27, Erik Bi wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/4818/

        -----------------------------------------------------------

        (Updated 2012-04-25 02:27:27)

        Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

        Summary

        -------

        per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400.

        Solution:

        At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one.

        This addresses bug shindig-1548.

        https://issues.apache.org/jira/browse/shindig-1548

        Diffs

        -----

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432

        Diff: https://reviews.apache.org/r/4818/diff

        Testing

        -------

        Thanks,

        Erik

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-04-26 06:29:45, Henry Saputra wrote: > Actually could you also add the gadget.xml you used for testing this? The way I tested it was changing a gadget to use relative url and check the library template request value in the request via firebug, so I can avoid writing a template library file: ) Sure, I will write a test gadget&template library file. Thanks. Erik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/#review7253 ----------------------------------------------------------- On 2012-04-25 02:27:27, Erik Bi wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4818/ ----------------------------------------------------------- (Updated 2012-04-25 02:27:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- per spec, Gadget should be able to use relative URL to define the template-libray, but right now when it uses this approach, client side will end up making a /gadgets/makeRequest call and passes in the relative URL value and fails with an HTTP status of 400. Solution: At client side, the opensocial-template feature will get the template-library URL value from global variable "features", so in this patch it updates the process of generating the init JS which will be passed back to client side to init the "features" variable(in Class "org.apache.shindig.gadgets.config.CoreUtilConfigContributor"), adds some specific logic to handle "template-library" parameter, convert the relative url to an absolute one. This addresses bug shindig-1548. https://issues.apache.org/jira/browse/shindig-1548 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/config/CoreUtilConfigContributor.java 1327432 Diff: https://reviews.apache.org/r/4818/diff Testing ------- Thanks, Erik
        Hide
        Henry Saputra added a comment -

        Any update about the test gadget for this?

        Show
        Henry Saputra added a comment - Any update about the test gadget for this?
        Hide
        Erik BI added a comment -

        Patch file for this issue

        Show
        Erik BI added a comment - Patch file for this issue
        Hide
        Erik BI added a comment -

        Test Gadget.
        For testing, please put the gadget definition xml and the template library xml file in same directory.

        Show
        Erik BI added a comment - Test Gadget. For testing, please put the gadget definition xml and the template library xml file in same directory.
        Hide
        Henry Saputra added a comment -

        +1

        If no other comment will commit the fix end of day today. Thanks much for the patch Erik.

        Show
        Henry Saputra added a comment - +1 If no other comment will commit the fix end of day today. Thanks much for the patch Erik.
        Hide
        Henry Saputra added a comment -

        Committed revision 1334192. Thanks so much for the patch.

        Show
        Henry Saputra added a comment - Committed revision 1334192. Thanks so much for the patch.
        Hide
        Erik BI added a comment -

        My pleasure, Henry _

        Show
        Erik BI added a comment - My pleasure, Henry _

          People

          • Assignee:
            Henry Saputra
            Reporter:
            Craig McClanahan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development