CR for SHINDIG-1525 Enable shindig to run on non-ROOT context path

Create new code review entry for Li Xu to fix the svn repo.

Please see details in the jira:
https://issues.apache.org/jira/browse/SHINDIG-1525

[henry.saputra, 2011-04-25 22:32:12]

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/container/gadget_holder.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/container/gadget_holder.js:90:
osapi.container.GadgetHolder.prototype.relayPath_ = null;
shouldnt this be defaulted to /gadgets/files/container/rpc_relay.html ?

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/container/init.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/container/init.js:33: 'jsonProxyUrl':
'http://%host%'+window.__CONTEXT_ROOT+'/gadgets/makeRequest',
Where is the window.__CONTEXT_ROOT defined?

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/shindig.uri/uri.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/shindig.uri/uri.js:230: setPath:
function(path) { if (path != null ) { path_ = (path[0] === '/' ? '' : '/') +
path; } return bundle; },
Need to check for undefined

http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java
(right):

http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java:127:
store.setDefaultCallbackUrl(config.getString(ContainerConfig.DEFAULT_CONTAINER,
"shindig.signing.global-callback-url"));
Not sure why you change this to look from container. The value is in the
shindig.properties.

[Li Xu, 2011-05-02 21:25:33]

Henry, thanks for reviewing the patch. I uploaded another patch for you to
review under here: http://codereview.appspot.com/4430047/
thanks.

On 2011/04/25 22:32:12, henry.saputra wrote:
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> File features/src/main/javascript/features/container/gadget_holder.js (right):
> 
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> features/src/main/javascript/features/container/gadget_holder.js:90:
> osapi.container.GadgetHolder.prototype.relayPath_ = null;
> shouldnt this be defaulted to /gadgets/files/container/rpc_relay.html ?
> 
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> File features/src/main/javascript/features/container/init.js (right):
> 
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> features/src/main/javascript/features/container/init.js:33: 'jsonProxyUrl':
> 'http://%host%'+window.__CONTEXT_ROOT+'/gadgets/makeRequest',
> Where is the window.__CONTEXT_ROOT defined?
> 
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> File features/src/main/javascript/features/shindig.uri/uri.js (right):
> 
>
http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
> features/src/main/javascript/features/shindig.uri/uri.js:230: setPath:
> function(path) { if (path != null ) { path_ = (path[0] === '/' ? '' : '/') +
> path; } return bundle; },
> Need to check for undefined
> 
>
http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
> File
> java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java
> (right):
> 
>
http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java:127:
>
store.setDefaultCallbackUrl(config.getString(ContainerConfig.DEFAULT_CONTAINER,
> "shindig.signing.global-callback-url"));
> Not sure why you change this to look from container. The value is in the
> shindig.properties.

[Li Xu, 2011-05-02 21:31:19]

thanks for reviewing the patch. please see reply below.

A new patch is uploaded under this issue:
http://codereview.appspot.com/4430047/

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/container/gadget_holder.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/container/gadget_holder.js:90:
osapi.container.GadgetHolder.prototype.relayPath_ = null;
On 2011/04/25 22:32:12, henry.saputra wrote:
> shouldnt this be defaulted to /gadgets/files/container/rpc_relay.html ?

Yes, it's defined in container.js so context root can be resolved on the server
end 
and available to common container on the client side.
  "container" : {
    "relayPath": "${CONTEXT_ROOT}/gadgets/files/container/rpc_relay.html"
  }

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/container/init.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/container/init.js:33: 'jsonProxyUrl':
'http://%host%'+window.__CONTEXT_ROOT+'/gadgets/makeRequest',
On 2011/04/25 22:32:12, henry.saputra wrote:
> Where is the window.__CONTEXT_ROOT defined?

window.__CONTEXT_ROOT is defined under index.jsp so it can be available to all
the client side js.
<script type="text/javascript">
window.__CONTEXT_ROOT = '<%=request.getContextPath()%>';
</script>
Please suggest if there's better place to hold this on client side

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
File features/src/main/javascript/features/shindig.uri/uri.js (right):

http://codereview.appspot.com/4423064/diff/1/features/src/main/javascript/fea...
features/src/main/javascript/features/shindig.uri/uri.js:230: setPath:
function(path) { if (path != null ) { path_ = (path[0] === '/' ? '' : '/') +
path; } return bundle; },
On 2011/04/25 22:32:12, henry.saputra wrote:
> Need to check for undefined

Done.

http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java
(right):

http://codereview.appspot.com/4423064/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/OAuthModule.java:127:
store.setDefaultCallbackUrl(config.getString(ContainerConfig.DEFAULT_CONTAINER,
"shindig.signing.global-callback-url"));
On 2011/04/25 22:32:12, henry.saputra wrote:
> Not sure why you change this to look from container. The value is in the
> shindig.properties.

thanks for catching this. It's missing from container.js in the patch submitted.
It supposed to be added  to container.js. so context_root can be resolved there
"shindig.signing.global-callback-url" :
"http://${SERVER_HOST}:${SERVER_PORT}/${CONTEXT_ROOT}gadgets/oauthcallback",