Merge pull request #109 from erlichmen/master

The naive root + path might cause cases of double slashes e.g. http://path//relative-path.

.gitignore

@@ -1,3 +1,4 @@
 node_modules/*
 .cache
 b041042364d89046c003ca151a6254ef
+.idea

lib/requests.js

@@ -99,6 +99,40 @@ BaseRequest.prototype.handleResponse = function(opt_fn) {
   };
 };
 
+/**
+ * @protected
+ * create the upload url for the method
+ * @param {string} root root of the url e.g. http://myserver
+ * @param {Array} paths an array of string that will be joined to a single path
+ * @param queryParams query parameters of the path, we be also used to replace inline path parameters e.g. {param}
+ */
+BaseRequest.buildUri = function(root, paths, queryParams) {
+    // create a path is starts with a slash and ends without it.
+    // Preventing double slashes along the way
+    var path = "/" + paths.join('/').split('/').filter(function(p) {
+            return !!p;
+        }).join('/'),
+        fullPath = url.resolve(root, path);
+
+    queryParams = queryParams || {};
+    // replace path query parameters, if there are on the path
+    for (var paramName in queryParams) {
+        var param = '{' + paramName + '}';
+        if (fullPath.indexOf(param) >= 0) {
+            fullPath = fullPath.replace(param, queryParams[paramName]);
+            delete queryParams[paramName];
+        }
+    }
+
+    // append the other query parameters
+    if (Object.keys(queryParams).length > 0) {
+        fullPath += '?' + querystring.stringify(queryParams);
+    }
+
+    return fullPath;
+};
+
+
 /**
  * Constructs a new Request.
  * @constructor
@@ -136,29 +170,26 @@ util.inherits(Request, BaseRequest);
 
 /**
  * Generates path to the method with given params.
- * @param  {object} queryParams Query params.
- * @return {String}             The method's REST path.
+ * @param  {String} baseUri the base Uri of the method e.g. http://servername
+ * @param  {Array} paths array of string that will be joined into the method path
+ * @param  {object} [queryParams] Query params.
+ * @return {String} The method's REST path.
  */
-Request.prototype.generatePath = function(queryParams) {
-  var path = this.apiMeta.basePath + this.methodMeta.path;
+Request.prototype.generateMethodUri = function(baseUri, paths, queryParams) {
 
-  queryParams = queryParams || {};
-  if (this.apiKey) {
-    queryParams.key = this.apiKey;
-  }
+    paths.push(this.apiMeta.basePath);
+    paths.push(this.methodMeta.path);
 
-  // replace path query parameters, if there are on the path
-  for (var paramName in queryParams) {
-    if (path.indexOf('{' + paramName + '}') >= 0) {
-      path = path.replace('{' + paramName + '}', queryParams[paramName]);
-      delete queryParams[paramName];
+    queryParams = queryParams || {};
+    if (this.apiKey) {
+        queryParams.key = this.apiKey;
     }
-  }
-  // append the oher query parameters
-  if (Object.keys(queryParams).length > 0) {
-    path += '?' + querystring.stringify(queryParams);
-  }
-  return path;
+
+    return BaseRequest.buildUri(baseUri,  paths, queryParams);
+};
+
+Request.prototype.generatePath = function(params) {
+    return this.generateMethodUri('', [], params);
 };
 
 /**
@@ -169,14 +200,17 @@ Request.prototype.generatePayload = function(rootUrl) {
   return !this.media ? this.generateBody(rootUrl) : this.generateUploadBody(rootUrl);
 };
 
+
 /**
  * Generates request opts.
  * @return {object} The request object.
  */
 Request.prototype.generateBody = function(rootUrl) {
-  var path = this.generatePath(this.params);
+  var path = this.generatePath(this.params),
+      rootUrl = rootUrl || this.methodMeta.baseUri;
+
   return {
-    uri: rootUrl + path,
+    uri: this.generateMethodUri(rootUrl, [], this.params),
     path: path,
     method: this.methodMeta.httpMethod,
     json: this.body
@@ -190,16 +224,16 @@ Request.prototype.generateBody = function(rootUrl) {
  */
 Request.prototype.generateUploadBody = function(rootUrl) {
   var params = this.params || {};
-  rootUrl = rootUrl.replace(/\/+$/, ''); // Remove any trailing slashes.
+  rootUrl = rootUrl || this.methodMeta.baseUri
 
   if (!this.body) {
     params.uploadType = 'media';
 
     return {
-      uri: rootUrl + '/upload' + this.generatePath(params),
+      uri: this.generateMethodUri(rootUrl, ['/upload'], this.params),
       method: this.methodMeta.httpMethod,
       headers: {
-        'Content-Type': this.media.mimeType,
+        'Content-Type': this.media.mimeType
       },
       body: this.media.data
     };
@@ -209,7 +243,7 @@ Request.prototype.generateUploadBody = function(rootUrl) {
   params.uploadType = 'multipart';
 
   return {
-    uri: rootUrl + '/upload' + this.generatePath(params),
+    uri: this.generateMethodUri(rootUrl, ['/upload'], this.params),
     method: this.methodMeta.httpMethod,
     multipart: [{
       'Content-Type': 'application/json',
@@ -283,12 +317,15 @@ BatchRequest.prototype.add = function(request) {
 /**
  * @protected
  * Generates HTTP payload object.
- *
+ * @param {String} [rootUrl]
  * @return {object} Generated payload.
  */
 BatchRequest.prototype.generatePayload = function(rootUrl) {
   // todo: implement
   var multipart = [];
+
+  rootUrl = rootUrl || BaseRequest.BASE_URL;
+
   this.requests_.forEach(function(request, key) {
     var body = [];
     var payload = request.generatePayload(rootUrl);
@@ -307,7 +344,7 @@ BatchRequest.prototype.generatePayload = function(rootUrl) {
 
   return {
     method: 'POST',
-    uri: rootUrl + '/batch',
+    uri: BaseRequest.buildUri(rootUrl, ['/batch']),
     multipart: multipart,
     headers: {
       'content-type': 'multipart/mixed'
@@ -352,7 +389,7 @@ BatchRequest.prototype.handleResponse = function(opt_fn) {
 BatchRequest.prototype.execute = function(opt_callback) {
   opt_callback = opt_callback || function() {};
   var callback = this.handleResponse(opt_callback),
-      requestOpts = this.generatePayload(BaseRequest.BASE_URL);
+      requestOpts = this.generatePayload();
 
   if (this.authClient) {
     this.authClient.request(requestOpts, callback);

package.json

@@ -34,7 +34,7 @@
     "url": "0.7.9"
   },
   "scripts": {
-    "test": "mocha tests/* --reporter spec --timeout 40000"
+    "test": "mocha --reporter spec --timeout 40000"
   },
   "license": "Apache 2"
 }

tests/test.requests.js → test/test.requests.js

@@ -58,9 +58,9 @@ describe('Requests', function() {
         .execute(function(err, client) {
       var obj = { longUrl: 'http://someurl...' };
       var request = client.urlshortener.url.insert(obj);
-      var payload = request.generatePayload('root-url');
+      var payload = request.generatePayload('http://root-url');
 
-      assert.equal(payload.uri, 'root-url/urlshortener/v1/url');
+      assert.equal(payload.uri, 'http://root-url/urlshortener/v1/url');
       assert.equal(payload.method, 'POST');
       assert.equal(payload.json.longUrl, 'http://someurl...');
       done();
@@ -91,8 +91,8 @@ describe('Requests', function() {
         .execute(function(err, client) {
       var params = { shortUrl: 'a' };
       var request = client.urlshortener.url.get(params);
-      var payload = request.generatePayload('root-url');
-      assert.equal(payload.uri, 'root-url/urlshortener/v1/url?shortUrl=a');
+      var payload = request.generatePayload('http://root-url');
+      assert.equal(payload.uri, 'http://root-url/urlshortener/v1/url?shortUrl=a');
       assert.equal(payload.method, 'GET');
       done();
     });
@@ -195,9 +195,9 @@ describe('Requests', function() {
     googleapis.discover('drive', 'v2').execute(function(err, client){
       var req = client.drive.files.insert().withMedia('text/plain', 'hey');
 
-      var payload = req.generatePayload('root-url');
+      var payload = req.generatePayload('http://root-url');
       assert.equal(payload.method, 'POST');
-      assert.equal(payload.uri, 'root-url/upload/drive/v2/files?uploadType=media');
+      assert.equal(payload.uri, 'http://root-url/upload/drive/v2/files?uploadType=media');
       assert.equal(payload.headers['Content-Type'], 'text/plain');
       assert.equal(payload.body, 'hey');
       done();
@@ -211,9 +211,9 @@ describe('Requests', function() {
           .insert({ title: 'title' })
           .withMedia('text/plain', 'hey');
 
-      var payload = req.generatePayload('root-url');
+      var payload = req.generatePayload('http://root-url');
       assert.equal(payload.method, 'POST');
-      assert.equal(payload.uri, 'root-url/upload/drive/v2/files?uploadType=multipart');
+      assert.equal(payload.uri, 'http://root-url/upload/drive/v2/files?uploadType=multipart');
       assert.equal(payload.multipart[0]['Content-Type'], 'application/json');
       assert.equal(payload.multipart[0].body, '{"title":"title"}');
       assert.equal(payload.multipart[1]['Content-Type'], 'text/plain');