From 45b7086032b11c2aec84723d3c5ea1be921daf73 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 14 Nov 2016 13:46:05 -0500 Subject: [PATCH] compute: use common/Operation (#1773) --- packages/google-cloud-compute/package.json | 3 +- .../google-cloud-compute/src/operation.js | 81 +---- .../system-test/compute.js | 10 + .../google-cloud-compute/test/operation.js | 277 ++++-------------- 4 files changed, 69 insertions(+), 302 deletions(-) diff --git a/packages/google-cloud-compute/package.json b/packages/google-cloud-compute/package.json index 545502f3d13..1d154f7bbf9 100644 --- a/packages/google-cloud-compute/package.json +++ b/packages/google-cloud-compute/package.json @@ -50,14 +50,13 @@ "compute engine" ], "dependencies": { - "@google-cloud/common": "^0.7.0", + "@google-cloud/common": "^0.8.0", "arrify": "^1.0.0", "async": "^2.0.1", "create-error-class": "^3.0.2", "extend": "^3.0.0", "gce-images": "^0.3.0", "is": "^3.0.1", - "modelo": "^4.2.0", "string-format-obj": "^1.0.0" }, "devDependencies": { diff --git a/packages/google-cloud-compute/src/operation.js b/packages/google-cloud-compute/src/operation.js index 0881f21062d..94375814267 100644 --- a/packages/google-cloud-compute/src/operation.js +++ b/packages/google-cloud-compute/src/operation.js @@ -21,8 +21,7 @@ 'use strict'; var common = require('@google-cloud/common'); -var events = require('events'); -var modelo = require('modelo'); +var util = require('util'); /*! Developer Documentation * @@ -159,23 +158,17 @@ function Operation(scope, name) { get: true }; - common.ServiceObject.call(this, { + common.Operation.call(this, { parent: scope, baseUrl: isCompute ? '/global/operations' : '/operations', id: name, methods: methods }); - events.EventEmitter.call(this); - - this.completeListeners = 0; - this.hasActiveListeners = false; this.name = name; - - this.listenForEvents_(); } -modelo.inherits(Operation, common.ServiceObject, events.EventEmitter); +util.inherits(Operation, common.Operation); /** * Get the operation's metadata. For a detailed description of metadata see @@ -230,62 +223,6 @@ Operation.prototype.getMetadata = function(callback) { }); }; -/** - * Convenience method that wraps the `complete` and `error` events in a - * Promise. - * - * @return {promise} - * - * @example - * operation.promise().then(function(metadata) { - * // The operation is complete. - * }, function(err) { - * // An error occurred during the operation. - * }); - */ -Operation.prototype.promise = function() { - var self = this; - - return new self.Promise(function(resolve, reject) { - self - .on('error', reject) - .on('complete', function(metadata) { - resolve([metadata]); - }); - }); -}; - -/** - * Begin listening for events on the operation. This method keeps track of how - * many "complete" listeners are registered and removed, making sure polling is - * handled automatically. - * - * As long as there is one active "complete" listener, the connection is open. - * When there are no more listeners, the polling stops. - * - * @private - */ -Operation.prototype.listenForEvents_ = function() { - var self = this; - - this.on('newListener', function(event) { - if (event === 'complete') { - self.completeListeners++; - - if (!self.hasActiveListeners) { - self.hasActiveListeners = true; - self.startPolling_(); - } - } - }); - - this.on('removeListener', function(event) { - if (event === 'complete' && --self.completeListeners === 0) { - self.hasActiveListeners = false; - } - }); -}; - /** * Poll `getMetadata` to check the operation's status. This runs a loop to ping * the API on an interval. @@ -295,13 +232,9 @@ Operation.prototype.listenForEvents_ = function() { * * @private */ -Operation.prototype.startPolling_ = function() { +Operation.prototype.poll_ = function(callback) { var self = this; - if (!this.hasActiveListeners) { - return; - } - this.getMetadata(function(err, metadata, apiResponse) { // Parsing the response body will automatically create an ApiError object if // the operation failed. @@ -309,7 +242,7 @@ Operation.prototype.startPolling_ = function() { err = err || parsedHttpRespBody.err; if (err) { - self.emit('error', err); + callback(err); return; } @@ -319,12 +252,12 @@ Operation.prototype.startPolling_ = function() { } if (metadata.status !== 'DONE') { - setTimeout(self.startPolling_.bind(self), 500); + callback(); return; } self.status = metadata.status; - self.emit('complete', metadata); + callback(null, metadata); }); }; diff --git a/packages/google-cloud-compute/system-test/compute.js b/packages/google-cloud-compute/system-test/compute.js index 3116fcf762d..dea86b88159 100644 --- a/packages/google-cloud-compute/system-test/compute.js +++ b/packages/google-cloud-compute/system-test/compute.js @@ -253,6 +253,16 @@ describe('Compute', function() { }); }); }); + + it('should run operation as a promise', function() { + var snapshot = disk.snapshot(generateName('snapshot')); + + return snapshot.create() + .then(function(response) { + var operation = response[1]; + return operation.promise(); + }); + }); }); describe('firewalls', function() { diff --git a/packages/google-cloud-compute/test/operation.js b/packages/google-cloud-compute/test/operation.js index a6e12cce862..2fd60d3e099 100644 --- a/packages/google-cloud-compute/test/operation.js +++ b/packages/google-cloud-compute/test/operation.js @@ -18,17 +18,16 @@ var assert = require('assert'); var extend = require('extend'); -var nodeutil = require('util'); -var proxyquire = require('proxyquire'); -var ServiceObject = require('@google-cloud/common').ServiceObject; +var proxyquire = require('proxyquire').noPreserveCache(); var util = require('@google-cloud/common').util; -function FakeServiceObject() { +function FakeOperation() { this.calledWith_ = arguments; - ServiceObject.apply(this, arguments); } -nodeutil.inherits(FakeServiceObject, ServiceObject); +function FakeServiceObject() { + this.calledWith_ = arguments; +} var parseHttpRespBodyOverride = null; var promisified = false; @@ -59,6 +58,7 @@ describe('Operation', function() { before(function() { Operation = proxyquire('../src/operation.js', { '@google-cloud/common': { + Operation: FakeOperation, ServiceObject: FakeServiceObject, util: fakeUtil } @@ -70,17 +70,13 @@ describe('Operation', function() { operation = new Operation(SCOPE, OPERATION_NAME); }); - afterEach(function() { - operation.removeAllListeners(); - }); - describe('instantiation', function() { it('should localize the name', function() { assert.strictEqual(operation.name, OPERATION_NAME); }); - it('should inherit from ServiceObject', function() { - assert(operation instanceof ServiceObject); + it('should inherit from Operation', function() { + assert(operation instanceof FakeOperation); var calledWith = operation.calledWith_[0]; @@ -108,11 +104,6 @@ describe('Operation', function() { var calledWith = operation.calledWith_[0]; assert.strictEqual(calledWith.baseUrl, '/global/operations'); }); - - it('should correctly initialize variables', function() { - assert.strictEqual(operation.completeListeners, 0); - assert.strictEqual(operation.hasActiveListeners, false); - }); }); describe('getMetadata', function() { @@ -205,133 +196,17 @@ describe('Operation', function() { }); }); - describe('promise', function() { - beforeEach(function() { - operation.startPolling_ = util.noop; - }); - - it('should return an instance of the localized Promise', function() { - var FakePromise = operation.Promise = function() {}; - var promise = operation.promise(); - - assert(promise instanceof FakePromise); - }); - - it('should reject the promise if an error occurs', function() { - var error = new Error('err'); - - setImmediate(function() { - operation.emit('error', error); - }); - - return operation.promise().then(function() { - throw new Error('Promise should have been rejected.'); - }, function(err) { - assert.strictEqual(err, error); - }); - }); - - it('should resolve the promise on complete', function() { - var metadata = {}; - - setImmediate(function() { - operation.emit('complete', metadata); - }); - - return operation.promise().then(function(data) { - assert.deepEqual(data, [metadata]); - }); - }); - }); - - describe('listenForEvents_', function() { - beforeEach(function() { - operation.startPolling_ = util.noop; - }); - - it('should start polling when complete listener is bound', function(done) { - operation.startPolling_ = function() { - done(); - }; - - operation.on('complete', util.noop); - }); - - it('should track the number of listeners', function() { - assert.strictEqual(operation.completeListeners, 0); - - operation.on('complete', util.noop); - assert.strictEqual(operation.completeListeners, 1); - - operation.removeListener('complete', util.noop); - assert.strictEqual(operation.completeListeners, 0); - }); - - it('should only run a single pulling loop', function() { - var startPollingCallCount = 0; - - operation.startPolling_ = function() { - startPollingCallCount++; - }; - - operation.on('complete', util.noop); - operation.on('complete', util.noop); - - assert.strictEqual(startPollingCallCount, 1); - }); - - it('should close when no more message listeners are bound', function() { - operation.on('complete', util.noop); - operation.on('complete', util.noop); - assert.strictEqual(operation.hasActiveListeners, true); - - operation.removeListener('complete', util.noop); - assert.strictEqual(operation.hasActiveListeners, true); - - operation.removeListener('complete', util.noop); - assert.strictEqual(operation.hasActiveListeners, false); - }); - }); - - describe('startPolling_', function() { - var listenForEvents_; - var operation; - - before(function() { - listenForEvents_ = Operation.prototype.listenForEvents_; - }); - - after(function() { - Operation.prototype.listenForEvents_ = listenForEvents_; - }); - + describe('poll_', function() { beforeEach(function() { - Operation.prototype.listenForEvents_ = util.noop; - operation = new Operation(SCOPE, OPERATION_NAME); - operation.hasActiveListeners = true; + operation.emit = util.noop; }); - afterEach(function() { - operation.hasActiveListeners = false; - }); - - it('should not call getMetadata if no listeners', function(done) { - operation.hasActiveListeners = false; - - operation.getMetadata = done; // if called, test will fail. - - operation.startPolling_(); - done(); - }); - - it('should call getMetadata if listeners are registered', function(done) { - operation.hasActiveListeners = true; - + it('should call getMetadata', function(done) { operation.getMetadata = function() { done(); }; - operation.startPolling_(); + operation.poll_(assert.ifError); }); describe('API error', function() { @@ -344,12 +219,10 @@ describe('Operation', function() { }); it('should emit the error', function(done) { - operation.on('error', function(err) { + operation.poll_(function(err) { assert.strictEqual(err, error); done(); }); - - operation.startPolling_(); }); }); @@ -363,37 +236,23 @@ describe('Operation', function() { }; }); - it('should parse the response body', function(done) { - parseHttpRespBodyOverride = function(body) { - assert.strictEqual(body, apiResponse); - setImmediate(done); - return {}; - }; + it('should parse and return the response body', function(done) { + var parsedHttpRespBody = { err: {} }; - operation.startPolling_(); - }); - - it('should detect and emit the error', function(done) { parseHttpRespBodyOverride = function(body) { assert.strictEqual(body, apiResponse); - - return { - err: error - }; + return parsedHttpRespBody; }; - operation.on('error', function(err) { - assert.strictEqual(err, error); + operation.poll_(function(err) { + assert.strictEqual(err, parsedHttpRespBody.err); done(); }); - - operation.startPolling_(); }); }); - describe('operation pending', function() { - var apiResponse = { status: 'PENDING' }; - var setTimeoutCached = global.setTimeout; + describe('operation running', function() { + var apiResponse = { status: 'RUNNING' }; beforeEach(function() { operation.getMetadata = function(callback) { @@ -401,97 +260,63 @@ describe('Operation', function() { }; }); - after(function() { - global.setTimeout = setTimeoutCached; - }); - - it('should call startPolling_ after 500 ms', function(done) { - var startPolling_ = operation.startPolling_; - var startPollingCalled = false; + it('should update status', function(done) { + delete operation.status; - global.setTimeout = function(fn, timeoutMs) { - fn(); // should call startPolling_ - assert.strictEqual(timeoutMs, 500); - }; - - operation.startPolling_ = function() { - if (!startPollingCalled) { - // Call #1. - startPollingCalled = true; - startPolling_.apply(this, arguments); - return; - } + operation.poll_(function(err) { + assert.ifError(err); + assert.strictEqual(operation.status, apiResponse.status); + done(); + }); + }); - // This is from the setTimeout call. - assert.strictEqual(this, operation); + it('should emit the running event', function(done) { + operation.emit = function(eventName, metadata) { + assert.strictEqual(eventName, 'running'); + assert.strictEqual(metadata, apiResponse); done(); }; - operation.startPolling_(); + operation.poll_(assert.ifError); }); - }); - describe('operation complete', function() { - var apiResponse = { status: 'DONE' }; + it('should not emit running if already running', function(done) { + operation.emit = function(eventName) { + assert.strictEqual(eventName, 'running'); - beforeEach(function() { - operation.getMetadata = function(callback) { - callback(null, apiResponse, apiResponse); + operation.emit = done; // will fail test if called + operation.poll_(done); }; - }); - - it('should emit complete with metadata', function(done) { - operation.on('complete', function(metadata) { - assert.strictEqual(metadata, apiResponse); - done(); - }); - operation.startPolling_(); + operation.poll_(assert.ifError); }); }); - describe('operation status', function() { - var apiResponse; + describe('operation complete', function() { + var apiResponse = { status: 'DONE' }; beforeEach(function() { - apiResponse = { status: 'RUNNING' }; - operation.getMetadata = function(callback) { callback(null, apiResponse, apiResponse); }; }); - it('should emit the running event when running', function(done) { - operation.on('running', function(metadata) { - assert.strictEqual(metadata, apiResponse); + it('should update status', function(done) { + operation.status = 'PENDING'; + + operation.poll_(function(err) { + assert.ifError(err); + assert.strictEqual(operation.status, apiResponse.status); done(); }); - - operation.startPolling_(); }); - it('should only emit the running event once', function(done) { - var statusSteps = ['PENDING', 'RUNNING', 'RUNNING', 'DONE']; - var metadataCallCount = 0; - var runningCallCount = 0; - - this.timeout(2000); - - operation.getMetadata = function(callback) { - apiResponse.status = statusSteps[metadataCallCount++]; - callback(null, apiResponse, apiResponse); - }; - - operation.on('running', function() { - runningCallCount++; - }); - - operation.on('complete', function() { - assert.strictEqual(runningCallCount, 1); + it('should execute callback with metadata', function(done) { + operation.poll_(function(err, metadata) { + assert.ifError(err); + assert.strictEqual(metadata, apiResponse); done(); }); - - operation.startPolling_(); }); }); });