clean up makeAuthorizedRequest & test

stephenplusplus · stephenplusplus · commit cad6302b7876 · 2014-11-06T14:29:31.000-05:00
diff --git a/lib/bigquery/index.js b/lib/bigquery/index.js
@@ -109,7 +109,7 @@ function BigQuery(options) {
 
   options = options || {};
 
-  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest(this, {
+  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest({
     credentials: options.credentials,
     keyFile: options.keyFilename,
     scopes: SCOPES
diff --git a/lib/common/util.js b/lib/common/util.js
@@ -341,14 +341,11 @@ function makeWritableStream(dup, options, onComplete) {
 
 module.exports.makeWritableStream = makeWritableStream;
 
-var tokenRefreshAttempts = 0;
-
-function makeAuthorizedRequest(container, config) {
+function makeAuthorizedRequest(config) {
   var authorize = gsa(config);
 
   function makeRequest(reqOpts, callback) {
-    var that = this;
-
+    var tokenRefreshAttempts = 0;
     reqOpts.headers = reqOpts.headers || {};
 
     if (reqOpts.headers['User-Agent']) {
@@ -361,15 +358,13 @@ function makeAuthorizedRequest(container, config) {
       if (err) {
         if (err.code === 401 &&
             ++tokenRefreshAttempts <= MAX_TOKEN_REFRESH_ATTEMPTS) {
-          that.authorizeReq_(reqOpts, onAuthorizedRequest);
+          authorize(reqOpts, onAuthorizedRequest);
         } else {
           (callback.onAuthorized || callback)(err);
         }
         return;
       }
 
-      tokenRefreshAttempts = 0;
-
       if (callback.onAuthorized) {
         callback.onAuthorized(null, authorizedReqOpts);
       } else {
diff --git a/lib/datastore/dataset.js b/lib/datastore/dataset.js
@@ -93,7 +93,7 @@ function Dataset(options) {
 
   options = options || {};
 
-  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest(this, {
+  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest({
     credentials: options.credentials,
     keyFile: options.keyFilename,
     scopes: SCOPES
diff --git a/lib/pubsub/index.js b/lib/pubsub/index.js
@@ -109,7 +109,7 @@ var SCOPES = [
 function PubSub(options) {
   options = options || {};
 
-  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest(this, {
+  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest({
     credentials: options.credentials,
     keyFile: options.keyFilename,
     scopes: SCOPES
diff --git a/lib/storage/index.js b/lib/storage/index.js
@@ -101,7 +101,7 @@ function Storage(config) {
     return new Storage(config);
   }
 
-  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest(this, {
+  this.makeAuthorizedRequest_ = util.makeAuthorizedRequest({
     credentials: config.credentials,
     keyFile: config.keyFilename,
     scopes: SCOPES
diff --git a/test/common/util.js b/test/common/util.js
@@ -25,10 +25,20 @@ var request = require('request');
 var util = require('sandboxed-module')
   .require('../../lib/common/util', {
     requires: {
+      'google-service-account': fakeGsa,
       request: fakeRequest
     }
   });
 
+var gsa_Override;
+
+function fakeGsa() {
+  var args = [].slice.apply(arguments);
+  var results = (gsa_Override || util.noop).apply(null, args);
+  gsa_Override = null;
+  return results || { getCredentials: util.noop };
+}
+
 var request_Cached = request;
 var request_Override;
 
@@ -252,4 +262,132 @@ describe('common/util', function() {
       });
     });
   });
+
+  describe('makeAuthorizedRequest', function() {
+    it('should pass configuration to gsa', function(done) {
+      var config = { keyFile: 'key', scopes: [1, 2] };
+
+      gsa_Override = function(cfg) {
+        assert.deepEqual(cfg, config);
+        done();
+      };
+
+      util.makeAuthorizedRequest(config);
+    });
+
+    it('should return gsa.getCredentials function', function() {
+      var getCredentials = util.makeAuthorizedRequest().getCredentials;
+      assert.equal(typeof getCredentials, 'function');
+    });
+
+    describe('makeRequest', function() {
+      it('should add a user agent onto headers', function(done) {
+        gsa_Override = function() {
+          return function authorize(reqOpts) {
+            assert(reqOpts.headers['User-Agent'].indexOf('gcloud') > -1);
+            done();
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({});
+      });
+
+      it('should extend an existing user agent', function(done) {
+        gsa_Override = function() {
+          return function authorize(reqOpts) {
+            var index = reqOpts.headers['User-Agent'].indexOf('test; gcloud');
+            assert.equal(index, 0);
+            done();
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({ headers: { 'User-Agent': 'test' } });
+      });
+
+      it('should execute callback with error', function(done) {
+        var error = new Error('Error.');
+
+        gsa_Override = function() {
+          return function authorize(reqOpts, callback) {
+            callback(error);
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({}, function(err) {
+          assert.equal(err, error);
+          done();
+        });
+      });
+
+      it('should try to reconnect if token invalid', function(done) {
+        var attempts = 0;
+        var expectedAttempts = 2;
+        var error = { code: 401 };
+
+        gsa_Override = function() {
+          return function authorize(reqOpts, callback) {
+            attempts++;
+            callback(error);
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({}, function (err) {
+          assert.equal(attempts, expectedAttempts);
+          assert.equal(err, error);
+          done();
+        });
+      });
+
+      it('should execute the onauthorized callback', function(done) {
+        gsa_Override = function() {
+          return function authorize(reqOpts, callback) {
+            callback();
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({}, { onAuthorized: done });
+      });
+
+      it('should execute the onauthorized callback with error', function(done) {
+        var error = new Error('Error.');
+
+        gsa_Override = function() {
+          return function authorize(reqOpts, callback) {
+            callback(error);
+          };
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({}, {
+          onAuthorized: function(err) {
+            assert.equal(err, error);
+            done();
+          }
+        });
+      });
+
+      it('should make the authorized request', function(done) {
+        var authorizedReqOpts = { a: 'b', c: 'd' };
+
+        gsa_Override = function() {
+          return function authorize(reqOpts, callback) {
+            callback(null, authorizedReqOpts);
+          };
+        };
+
+        request_Override = function(reqOpts) {
+          assert.deepEqual(reqOpts, authorizedReqOpts);
+          done();
+        };
+
+        var makeRequest = util.makeAuthorizedRequest();
+        makeRequest({}, assert.ifError);
+      });
+    });
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ function Storage(config) {`
`101`	`101`	`return new Storage(config);`
`102`	`102`	`}`
`103`	`103`
`104`		`- this.makeAuthorizedRequest_ = util.makeAuthorizedRequest(this, {`
	`104`	`+ this.makeAuthorizedRequest_ = util.makeAuthorizedRequest({`
`105`	`105`	`credentials: config.credentials,`
`106`	`106`	`keyFile: config.keyFilename,`
`107`	`107`	`scopes: SCOPES`