Looks good. One thing that has always bothered me is that 'opensocial-0.8' key in container.js. What does everyone think about renaming that just 'opensocial'?
On Wed, Jul 15, 2009 at 2:19 PM, <[email protected]> wrote: > Reviewers: louiscryan, > > Description: > jsonrpccontainer was hardcoding /social/rpc and /gadgets/api/rpc paths, > which made it hard to config. Eliminate that, and pull the paths > entirely from runtime config. > > Please review this at http://codereview.appspot.com/96045 > > Affected files: > config/container.js > features/src/main/javascript/features/opensocial-current/feature.xml > > features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js > > > Index: > features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js > =================================================================== > --- > features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js > (revision 793628) > +++ > features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js > (working copy) > @@ -23,9 +23,19 @@ > * @fileoverview JSON-RPC based opensocial container. > */ > > -var JsonRpcContainer = function(baseUrl, domain, supportedFieldsArray) { > +var JsonRpcContainer = function(configParams) { > opensocial.Container.call(this); > > + var path = configParams.path; > + // Path for social API calls > + this.path_ = path.replace("%host%", document.location.host); > + > + // Path for calls to invalidate > + var invalidatePath = configParams.invalidatePath; > + this.invalidatePath_ = invalidatePath.replace("%host%", > + document.location.host); > + > + var supportedFieldsArray = configParams.supportedFields; > var supportedFieldsMap = {}; > for (var objectType in supportedFieldsArray) { > if (supportedFieldsArray.hasOwnProperty(objectType)) { > @@ -37,8 +47,8 @@ > } > } > > - this.environment_ = new opensocial.Environment(domain, > supportedFieldsMap); > - this.baseUrl_ = baseUrl; > + this.environment_ = new opensocial.Environment(configParams.domain, > + supportedFieldsMap); > > this.securityToken_ = shindig.auth.getSecurityToken(); > > @@ -206,7 +216,7 @@ > "POST_DATA" : gadgets.json.stringify(jsonBatchData) > }; > > - var url = [this.baseUrl_, "/social/rpc"]; > + var url = [this.path_]; > var token = shindig.auth.getSecurityToken(); > if (token) { > url.push("?st=", encodeURIComponent(token)); > @@ -439,7 +449,7 @@ > "POST_DATA" : gadgets.json.stringify(rpc) > }; > > - var url = [this.baseUrl_, "/gadgets/api/rpc"]; > + var url = [this.invalidatePath_]; > var token = shindig.auth.getSecurityToken(); > if (token) { > url.push("?st=", encodeURIComponent(token)); > Index: features/src/main/javascript/features/opensocial-current/feature.xml > =================================================================== > --- features/src/main/javascript/features/opensocial-current/feature.xml > (revision 793628) > +++ features/src/main/javascript/features/opensocial-current/feature.xml > (working copy) > @@ -34,13 +34,8 @@ > > gadgets.config.register("opensocial-0.8", requiredConfig, > function(config) { > - var configParams = config["opensocial-0.8"]; > - var path = configParams.path; > - path = path.replace("%host%", document.location.host); > - > ShindigContainer = function() { > - JsonRpcContainer.call(this, path, > - configParams.domain, configParams.supportedFields); > + JsonRpcContainer.call(this, config["opensocial-0.8"]); > }; > ShindigContainer.inherits(JsonRpcContainer); > > Index: config/container.js > =================================================================== > --- config/container.js (revision 793628) > +++ config/container.js (working copy) > @@ -136,7 +136,9 @@ > "opensocial-0.8" : { > // Path to fetch opensocial data from > // Must be on the same domain as the gadget rendering server > - "path" : "http://%host%";, > + "path" : "http://%host%/social/rpc";, > + // Path to issue invalidate calls > + "invalidatePath" : "http://%host%/gadgets/api/rpc";, > "domain" : "shindig", > "enableCaja" : false, > "supportedFields" : { > > >
