From f05854345598a2be3978557d41c39e34c3f61b65 Mon Sep 17 00:00:00 2001 From: Qinyuan Wan Date: Tue, 22 Mar 2016 16:47:05 -0700 Subject: [PATCH 1/3] fix async patch and post run --- .../ms-rest-azure/lib/azureServiceClient.js | 201 ++++++++---------- .../NodeJS/ms-rest-azure/lib/pollingState.js | 65 +++--- 2 files changed, 128 insertions(+), 138 deletions(-) diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js index ae1798dc5e56a..4b967ca5cd373 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js @@ -32,15 +32,15 @@ function AzureServiceClient(credentials, options) { if (!credentials) { throw new Error('Azure clients require credentials.'); } - + AzureServiceClient['super_'].call(this, credentials, options); - + this.acceptLanguage = 'en-US'; this.generateClientRequestId = true; this.longRunningOperationRetryTimeout = 30; if (!options) options = {}; - + if (options.acceptLanguage !== null && options.acceptLanguage !== undefined) { this.acceptLanguage = options.acceptLanguage; } @@ -62,68 +62,73 @@ util.inherits(AzureServiceClient, msRest.ServiceClient); * @param {object} [options] * @param {object} [options.customHeaders] headers that will be added to request */ -AzureServiceClient.prototype.getPutOrPatchOperationResult = function (resultOfInitialRequest, options, callback) { +AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfInitialRequest, options, callback) { var self = this; - if(!callback && typeof options === 'function') { + if (!callback && typeof options === 'function') { callback = options; options = null; } if (!callback) { throw new Error('Missing callback'); } - + if (!resultOfInitialRequest) { return callback(new Error('Missing resultOfInitialRequest parameter')); } - + + if (!resultOfInitialRequest.response) { + return callback(new Error('Missing resultOfInitialRequest.response')); + } + if (resultOfInitialRequest.response.statusCode !== 200 && - resultOfInitialRequest.response.statusCode !== 201 && - resultOfInitialRequest.response.statusCode !== 202) { - return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', + resultOfInitialRequest.response.statusCode !== 201 && + resultOfInitialRequest.response.statusCode !== 202 && + resultOfInitialRequest.response.statusCode !== 204) { + return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', resultOfInitialRequest.response.statusCode))); } + var pollingState = null; try { pollingState = new PollingState(resultOfInitialRequest, this.longRunningOperationRetryTimeout); } catch (error) { callback(error); } - var resourceUrl = resultOfInitialRequest.request.url; this._options = options; - + async.whilst( - //while condition - function () { - var finished = [LroStates.Succeeded, LroStates.Failed, LroStates.Canceled].some(function (e) { - return pollingState.status === e; + function() { + var finished = [LroStates.Succeeded, LroStates.Failed, LroStates.Canceled].some(function(e) { + return e === pollingState.status; }); return !finished; }, - //while loop body - function (callback) { - setTimeout(function () { + function(callback) { + setTimeout(function() { if (pollingState.azureAsyncOperationHeaderLink) { - self._updateStateFromAzureAsyncOperationHeader(pollingState, false, function (err) { + self._updateStateFromAzureAsyncOperationHeader(pollingState, true, function(err) { return callback(err); }); } else if (pollingState.locationHeaderLink) { - self._updateStateFromLocationHeaderOnPut(pollingState, function (err) { + self._updateStateFromLocationHeader(pollingState, function(err) { return callback(err); }); - } else { - self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function (err) { + } else if (resultOfInitialRequest.request.method === "PUT") { + self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err); }); + } else { + return callback(new Error('Location header is missing from long running operation.')); } }, pollingState.getTimeout()); }, //when done - function (err) { + function(err) { if (pollingState.status === LroStates.Succeeded) { - if (!pollingState.resource) { - self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function (err) { + if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== "DELETE" && resultOfInitialRequest.request.method !== "POST") { + self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err, pollingState.getOperationResponse()); }); } else { @@ -142,9 +147,9 @@ AzureServiceClient.prototype.getPutOrPatchOperationResult = function (resultOfIn * @param {object} [options] * @param {object} [options.customHeaders] headers that will be added to request */ -AzureServiceClient.prototype.getPostOrDeleteOperationResult = function (resultOfInitialRequest, options, callback) { +AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfInitialRequest, options, callback) { var self = this; - + if (!callback && typeof options === 'function') { callback = options; options = null; @@ -152,45 +157,52 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function (resultOf if (!callback) { throw new Error('Missing callback'); } - + if (!resultOfInitialRequest) { return callback(new Error('Missing resultOfInitialRequest parameter')); } - + if (!resultOfInitialRequest.response) { return callback(new Error('Missing resultOfInitialRequest.response')); } - + if (resultOfInitialRequest.response.statusCode !== 200 && - resultOfInitialRequest.response.statusCode !== 202 && - resultOfInitialRequest.response.statusCode !== 204) { - return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', + resultOfInitialRequest.response.statusCode !== 201 && + resultOfInitialRequest.response.statusCode !== 202 && + resultOfInitialRequest.response.statusCode !== 204) { + return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', resultOfInitialRequest.response.statusCode))); } - + var pollingState = null; try { pollingState = new PollingState(resultOfInitialRequest, this.longRunningOperationRetryTimeout); } catch (error) { callback(error); } + + var resourceUrl = resultOfInitialRequest.request.url; this._options = options; async.whilst( - function () { - var finished = [LroStates.Succeeded, LroStates.Failed, LroStates.Canceled].some(function (e) { + function() { + var finished = [LroStates.Succeeded, LroStates.Failed, LroStates.Canceled].some(function(e) { return e === pollingState.status; }); return !finished; }, - function (callback) { - setTimeout(function () { + function(callback) { + setTimeout(function() { if (pollingState.azureAsyncOperationHeaderLink) { - self._updateStateFromAzureAsyncOperationHeader(pollingState, true, function (err) { + self._updateStateFromAzureAsyncOperationHeader(pollingState, true, function(err) { return callback(err); }); } else if (pollingState.locationHeaderLink) { - self._updateStateFromLocationHeaderOnPostOrDelete(pollingState, function (err) { + self._updateStateFromLocationHeader(pollingState, function(err) { + return callback(err); + }); + } else if (resultOfInitialRequest.request.method === "PUT") { + self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err); }); } else { @@ -198,9 +210,16 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function (resultOf } }, pollingState.getTimeout()); }, - function (err) { - if (pollingState.status === LroStates.Succeeded ) { - return callback(null, pollingState.getOperationResponse()); + //when done + function(err) { + if (pollingState.status === LroStates.Succeeded) { + if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== "DELETE" && resultOfInitialRequest.request.method !== "POST") { + self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { + return callback(err, pollingState.getOperationResponse()); + }); + } else { + return callback(null, pollingState.getOperationResponse()); + } } else { return callback(pollingState.getCloudError(err)); } @@ -212,14 +231,14 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function (resultOf * @param {object} [pollingState] - The object to persist current operation state. * @param {boolean} [inPostOrDelete] - Invoked by Post Or Delete operation. */ -AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = function (pollingState, inPostOrDelete, callback) { - this._getStatus(pollingState.azureAsyncOperationHeaderLink, function (err, result) { +AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = function(pollingState, inPostOrDelete, callback) { + this._getStatus(pollingState.azureAsyncOperationHeaderLink, function(err, result) { if (err) return callback(err); - + if (!result.body || !result.body.status) { return callback(new Error('The response from long running operation does not contain a body.')); } - + pollingState.status = result.body.status; pollingState.error = result.body.error; pollingState.response = result.response; @@ -233,36 +252,25 @@ AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = functio }; /** - * Retrieve PUT operation status by polling from 'location' header. + * Retrieve PUT/POST/PATCH/DELETE operation status by polling from 'location' header. * @param {object} [pollingState] - The object to persist current operation state. */ -AzureServiceClient.prototype._updateStateFromLocationHeaderOnPut = function (pollingState, callback) { - this._getStatus(pollingState.locationHeaderLink, function (err, result) { +AzureServiceClient.prototype._updateStateFromLocationHeader = function(pollingState, callback) { + this._getStatus(pollingState.locationHeaderLink, function(err, result) { if (err) return callback(err); - + pollingState.updateResponse(result.response); pollingState.request = result.request; - + var statusCode = result.response.statusCode; if (statusCode === 202) { pollingState.status = LroStates.InProgress; - } - else if (statusCode === 200 || - statusCode === 201) { - - if (!result.body) { - return callback(new Error('The response from long running operation does not contain a body.')); - } - - // In 202 pattern on PUT ProvisioningState may not be present in - // the response. In that case the assumption is the status is Succeeded. - if (result.body.properties && result.body.properties.provisioningState) { - pollingState.status = result.body.properties.provisioningState; - } - else { - pollingState.status = LroStates.Succeeded; - } - + } else if (statusCode === 200 || + statusCode === 201 || + statusCode === 204) { + + pollingState.status = LroStates.Succeeded; + pollingState.error = { code: pollingState.Status, message: util.format('Long running operation failed with status \'%s\'.', pollingState.status) @@ -273,59 +281,34 @@ AzureServiceClient.prototype._updateStateFromLocationHeaderOnPut = function (pol }); }; -/** - * Retrieve POST or DELETE operation status by polling from 'location' header. - * @param {object} [pollingState] - The object to persist current operation state. - */ -AzureServiceClient.prototype._updateStateFromLocationHeaderOnPostOrDelete = function (pollingState, callback) { - this._getStatus(pollingState.locationHeaderLink, function (err, result) { - if (err) return callback(err); - - pollingState.updateResponse(result.response); - pollingState.request = result.request; - - var statusCode = result.response.statusCode; - if (statusCode === 202) { - pollingState.status = LroStates.InProgress; - } - else if (statusCode === 200 || - statusCode === 201 || - statusCode === 204) { - pollingState.status = LroStates.Succeeded; - pollingState.resource = result.body; - } - callback(null); - }); -}; - /** * Polling for resource status. * @param {function} [resourceUrl] - The url of resource. * @param {object} [pollingState] - The object to persist current operation state. */ -AzureServiceClient.prototype._updateStateFromGetResourceOperation = function (resourceUrl, pollingState, callback) { - this._getStatus(resourceUrl, function (err, result) { +AzureServiceClient.prototype._updateStateFromGetResourceOperation = function(resourceUrl, pollingState, callback) { + this._getStatus(resourceUrl, function(err, result) { if (err) return callback(err); if (!result.body) { return callback(new Error('The response from long running operation does not contain a body.')); } - + if (result.body.properties && result.body.properties.provisioningState) { pollingState.status = result.body.properties.provisioningState; } else { pollingState.status = LroStates.Succeeded; } - + //we might not throw an error, but initialize here just in case. pollingState.error = { code: pollingState.status, message: util.format('Long running operation failed with status \'%s\'.', pollingState.status) }; - + pollingState.updateResponse(result.response); pollingState.request = result.request; pollingState.resource = result.body; - + //nothing to return, the 'pollingState' has all the info we care. callback(null); }); @@ -335,21 +318,21 @@ AzureServiceClient.prototype._updateStateFromGetResourceOperation = function (re * Retrieve operation status by querying the operation URL. * @param {string} [operationUrl] - URL used to poll operation result. */ -AzureServiceClient.prototype._getStatus = function (operationUrl, callback) { +AzureServiceClient.prototype._getStatus = function(operationUrl, callback) { var self = this; if (!operationUrl) { return callback(new Error('operationUrl cannot be null.')); } - + // Construct URL var requestUrl = operationUrl.replace(' ', '%20'); - + // Create HTTP transport objects var httpRequest = new WebResource(); httpRequest.method = 'GET'; httpRequest.headers = {}; httpRequest.url = requestUrl; - if(this._options) { + if (this._options) { for (var headerName in this._options['customHeaders']) { if (this._options['customHeaders'].hasOwnProperty(headerName)) { httpRequest.headers[headerName] = this._options['customHeaders'][headerName]; @@ -357,13 +340,13 @@ AzureServiceClient.prototype._getStatus = function (operationUrl, callback) { } } // Send Request - return self.pipeline(httpRequest, function (err, response, responseBody) { + return self.pipeline(httpRequest, function(err, response, responseBody) { if (err) { return callback(err); } var statusCode = response.statusCode; if (statusCode !== 200 && statusCode !== 201 && statusCode !== 202 && statusCode !== 204) { - var error = new Error(util.format('Invalid status code with response body "%s" occurred ' + + var error = new Error(util.format('Invalid status code with response body "%s" occurred ' + 'when polling for operation status.', responseBody)); error.statusCode = response.statusCode; error.request = msRest.stripRequest(httpRequest); @@ -388,8 +371,8 @@ AzureServiceClient.prototype._getStatus = function (operationUrl, callback) { try { result.body = JSON.parse(responseBody); } catch (deserializationError) { - var parseError = new Error(util.format('Error "%s" occurred in deserializing the response body - "%s" -' + - ' when polling for operation status.', deserializationError, responseBody)); + var parseError = new Error(util.format('Error "%s" occurred in deserializing the response body - "%s" -' + + ' when polling for operation status.', deserializationError, responseBody)); parseError.request = msRest.stripRequest(httpRequest); parseError.response = msRest.stripResponse(response); parseError.body = responseBody; diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js b/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js index 87a68bf6dfc25..0d7c48fd1df0b 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js @@ -25,39 +25,46 @@ function PollingState(resultOfInitialRequest, retryTimeout) { this.request = resultOfInitialRequest.request; //Parse response.body & assign it as the resource try { - if (resultOfInitialRequest.body && - typeof resultOfInitialRequest.body.valueOf() === 'string' && - resultOfInitialRequest.body.length > 0) { + if (resultOfInitialRequest.body && + typeof resultOfInitialRequest.body.valueOf() === 'string' && + resultOfInitialRequest.body.length > 0) { this.resource = JSON.parse(resultOfInitialRequest.body); } else { this.resource = resultOfInitialRequest.body; - } + } } catch (error) { - var deserializationError = new Error(util.format('Error "%s" occurred in parsing the responseBody ' + + var deserializationError = new Error(util.format('Error "%s" occurred in parsing the responseBody ' + 'while creating the PollingState for Long Running Operation- "%s"', error, resultOfInitialRequest.body)); deserializationError.request = resultOfInitialRequest.request; deserializationError.response = resultOfInitialRequest.response; throw deserializationError; } - - if (this.resource && this.resource.properties && this.resource.properties.provisioningState) { - this.status = this.resource.properties.provisioningState; - } else { - switch (this.response.statusCode) { - case 202: - this.status = LroStates.InProgress; - break; - case 204: - case 201: - case 200: - this.status = LroStates.Succeeded; - break; + switch (this.response.statusCode) { + case 202: + this.status = LroStates.InProgress; + break; - default: - this.status = LroStates.Failed; - break; - } + case 204: + this.status = LroStates.Succeeded; + break; + case 201: + if (this.resource && this.resource.properties && this.resource.properties.provisioningState) { + this.status = this.resource.properties.provisioningState; + } else { + this.status = LroStates.InProgress; + } + break; + case 200: + if (this.resource && this.resource.properties && this.resource.properties.provisioningState) { + this.status = this.resource.properties.provisioningState; + } else { + this.status = LroStates.Succeeded; + } + break; + default: + this.status = LroStates.Failed; + break; } } @@ -65,7 +72,7 @@ function PollingState(resultOfInitialRequest, retryTimeout) { * Gets timeout in milliseconds. * @returns {number} timeout */ -PollingState.prototype.getTimeout = function () { +PollingState.prototype.getTimeout = function() { if (this._retryTimeout || this._retryTimeout === 0) { return this._retryTimeout * 1000; } @@ -79,13 +86,13 @@ PollingState.prototype.getTimeout = function () { * Update cached data using the provided response object * @param {object} [response] - provider response object. */ -PollingState.prototype.updateResponse = function (response) { +PollingState.prototype.updateResponse = function(response) { this.response = response; if (response && response.headers) { if (response.headers['azure-asyncoperation']) { this.azureAsyncOperationHeaderLink = response.headers['azure-asyncoperation']; } - + if (response.headers['location']) { this.locationHeaderLink = response.headers['location']; } @@ -96,7 +103,7 @@ PollingState.prototype.updateResponse = function (response) { * Returns long running operation result. * @returns {object} HttpOperationResponse */ -PollingState.prototype.getOperationResponse = function () { +PollingState.prototype.getOperationResponse = function() { var result = new msRest.HttpOperationResponse(); result.request = this.request; result.response = this.response; @@ -112,7 +119,7 @@ PollingState.prototype.getOperationResponse = function () { * Returns an Error on operation failure. * @returns {object} Error */ -PollingState.prototype.getCloudError = function (err) { +PollingState.prototype.getCloudError = function(err) { var errMsg; var errCode; @@ -126,11 +133,11 @@ PollingState.prototype.getCloudError = function (err) { parsedResponse = JSON.parse(this.response.body); } } catch (err) { - error.message = util.format('Error "%s" occurred while deserializing the error ' + + error.message = util.format('Error "%s" occurred while deserializing the error ' + 'message "%s" for long running operation.', err.message, this.response.body); return error; } - + if (err && err.message) { errMsg = util.format('Long running operation failed with error: \'%s\'.', err.message); } else { From b789377b723611732d63266140f6592fd817d837 Mon Sep 17 00:00:00 2001 From: Qinyuan Wan Date: Wed, 23 Mar 2016 10:43:45 -0700 Subject: [PATCH 2/3] add test for fixing the post/patch async operation --- .../ms-rest-azure/lib/azureServiceClient.js | 8 +-- .../test/azureServiceClientTests.js | 50 ++++++++++++++++++- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js index 4b967ca5cd373..4a7ce94cf713a 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js @@ -115,7 +115,7 @@ AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfIni self._updateStateFromLocationHeader(pollingState, function(err) { return callback(err); }); - } else if (resultOfInitialRequest.request.method === "PUT") { + } else if (resultOfInitialRequest.request.method === 'PUT') { self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err); }); @@ -127,7 +127,7 @@ AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfIni //when done function(err) { if (pollingState.status === LroStates.Succeeded) { - if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== "DELETE" && resultOfInitialRequest.request.method !== "POST") { + if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== 'DELETE' && resultOfInitialRequest.request.method !== 'POST') { self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err, pollingState.getOperationResponse()); }); @@ -201,7 +201,7 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfI self._updateStateFromLocationHeader(pollingState, function(err) { return callback(err); }); - } else if (resultOfInitialRequest.request.method === "PUT") { + } else if (resultOfInitialRequest.request.method === 'PUT') { self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err); }); @@ -213,7 +213,7 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfI //when done function(err) { if (pollingState.status === LroStates.Succeeded) { - if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== "DELETE" && resultOfInitialRequest.request.method !== "POST") { + if ((pollingState.azureAsyncOperationHeaderLink || !pollingState.resource) && resultOfInitialRequest.request.method !== 'DELETE' && resultOfInitialRequest.request.method !== 'POST') { self._updateStateFromGetResourceOperation(resourceUrl, pollingState, function(err) { return callback(err, pollingState.getOperationResponse()); }); diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js b/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js index e28f44bf5ad68..bab92dc1e627c 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js @@ -156,11 +156,56 @@ describe('AzureServiceClient', function () { }); }); + describe('Patch', function () { + resultOfInitialRequest.response.statusCode = 202; + resultOfInitialRequest.body.properties.provisioningState = LroStates.Succeeded; + + it('works by polling from azure-asyncoperation header', function (done) { + resultOfInitialRequest.response.headers['azure-asyncoperation'] = ''; + resultOfInitialRequest.response.headers['location'] = urlFromLocationHeader_Return200; + client.getPutOrPatchOperationResult(resultOfInitialRequest, function (err, result) { + should.not.exist(err); + JSON.parse(result.body).name.should.equal(testResourceName); + should.exist(result.response.randomFieldFromPollLocationHeader); + done(); + }); + }); + + it('works by polling from location header', function (done) { + resultOfInitialRequest.response.headers['azure-asyncoperation'] = urlFromAzureAsyncOPHeader_Return200; + resultOfInitialRequest.response.headers['location'] = ''; + client.getPutOrPatchOperationResult(resultOfInitialRequest, function (err, result) { + should.not.exist(err); + JSON.parse(result.body).name.should.equal(testResourceName); + done(); + }); + }); + + it('returns error if failed to poll from the azure-asyncoperation header', function (done) { + resultOfInitialRequest.response.headers['azure-asyncoperation'] = url_ReturnError; + resultOfInitialRequest.response.headers['location'] = ''; + client.getPutOrPatchOperationResult(resultOfInitialRequest, function (err, result) { + err.message.should.containEql(testError); + done(); + }); + }); + + it('returns error if failed to poll from the location header', function (done) { + resultOfInitialRequest.response.headers['azure-asyncoperation'] = ''; + resultOfInitialRequest.response.headers['location'] = url_ReturnError; + client.getPutOrPatchOperationResult(resultOfInitialRequest, function (err, result) { + err.message.should.containEql(testError); + done(); + }); + }); + }); + describe('Post-or-Delete', function () { resultOfInitialRequest.response.statusCode = 202; - + resultOfInitialRequest.body.properties.provisioningState = LroStates.Succeeded; + it('throw on not Lro related status code', function (done) { - client.getPostOrDeleteOperationResult({ response: { statusCode: 201 } }, function (err, result) { + client.getPostOrDeleteOperationResult({ response: { statusCode: 203 }, request: {url: url_resource}}, function (err, result) { err.message.should.containEql('Unexpected polling status code from long running operation'); done(); }); @@ -169,6 +214,7 @@ describe('AzureServiceClient', function () { it('works by polling from the azure-asyncoperation header', function (done) { resultOfInitialRequest.response.headers['azure-asyncoperation'] = urlFromAzureAsyncOPHeader_Return200; resultOfInitialRequest.response.headers['location'] = ''; + resultOfInitialRequest.request.method = 'POST'; client.getPostOrDeleteOperationResult(resultOfInitialRequest, function (err, result) { should.not.exist(err); should.exist(result.response.randomFieldFromPollAsyncOpHeader); From 9c7e28b27cdc5b49900ce005b1c4c6a06290264c Mon Sep 17 00:00:00 2001 From: Qinyuan Wan Date: Thu, 24 Mar 2016 10:09:33 -0700 Subject: [PATCH 3/3] code revise --- .../ms-rest-azure/lib/azureServiceClient.js | 83 ++++++++++--------- .../NodeJS/ms-rest-azure/lib/pollingState.js | 26 +++--- .../test/azureServiceClientTests.js | 4 +- 3 files changed, 60 insertions(+), 53 deletions(-) diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js index 4a7ce94cf713a..dabedd8d3217c 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/lib/azureServiceClient.js @@ -32,15 +32,15 @@ function AzureServiceClient(credentials, options) { if (!credentials) { throw new Error('Azure clients require credentials.'); } - + AzureServiceClient['super_'].call(this, credentials, options); - + this.acceptLanguage = 'en-US'; this.generateClientRequestId = true; this.longRunningOperationRetryTimeout = 30; if (!options) options = {}; - + if (options.acceptLanguage !== null && options.acceptLanguage !== undefined) { this.acceptLanguage = options.acceptLanguage; } @@ -62,7 +62,7 @@ util.inherits(AzureServiceClient, msRest.ServiceClient); * @param {object} [options] * @param {object} [options.customHeaders] headers that will be added to request */ -AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfInitialRequest, options, callback) { +AzureServiceClient.prototype.getPutOrPatchOperationResult = function (resultOfInitialRequest, options, callback) { var self = this; if (!callback && typeof options === 'function') { @@ -81,12 +81,10 @@ AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfIni return callback(new Error('Missing resultOfInitialRequest.response')); } - if (resultOfInitialRequest.response.statusCode !== 200 && - resultOfInitialRequest.response.statusCode !== 201 && - resultOfInitialRequest.response.statusCode !== 202 && - resultOfInitialRequest.response.statusCode !== 204) { - return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', - resultOfInitialRequest.response.statusCode))); + if (this._checkInitialRequestResponseStatusCodeFailed(resultOfInitialRequest)) { + return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\' for method \'%s\'', + resultOfInitialRequest.response.statusCode, + resultOfInitialRequest.request.method))); } var pollingState = null; @@ -147,7 +145,7 @@ AzureServiceClient.prototype.getPutOrPatchOperationResult = function(resultOfIni * @param {object} [options] * @param {object} [options.customHeaders] headers that will be added to request */ -AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfInitialRequest, options, callback) { +AzureServiceClient.prototype.getPostOrDeleteOperationResult = function (resultOfInitialRequest, options, callback) { var self = this; if (!callback && typeof options === 'function') { @@ -166,12 +164,10 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfI return callback(new Error('Missing resultOfInitialRequest.response')); } - if (resultOfInitialRequest.response.statusCode !== 200 && - resultOfInitialRequest.response.statusCode !== 201 && - resultOfInitialRequest.response.statusCode !== 202 && - resultOfInitialRequest.response.statusCode !== 204) { - return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\'', - resultOfInitialRequest.response.statusCode))); + if (this._checkInitialRequestResponseStatusCodeFailed(resultOfInitialRequest)) { + return callback(new Error(util.format('Unexpected polling status code from long running operation \'%s\' for method \'%s\'', + resultOfInitialRequest.response.statusCode, + resultOfInitialRequest.request.method))); } var pollingState = null; @@ -180,7 +176,6 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfI } catch (error) { callback(error); } - var resourceUrl = resultOfInitialRequest.request.url; this._options = options; @@ -226,19 +221,31 @@ AzureServiceClient.prototype.getPostOrDeleteOperationResult = function(resultOfI }); }; +AzureServiceClient.prototype._checkInitialRequestResponseStatusCodeFailed = function (initialRequest) { + if (initialRequest.response.statusCode === 200 || + initialRequest.response.statusCode === 202 || + (initialRequest.response.statusCode === 201 && initialRequest.request.method === 'PUT') || + (initialRequest.response.statusCode === 204 && initialRequest.request.method === 'DELETE')) { + return false; + } else { + return true; + } +}; + + /** * Retrieve operation status by polling from 'azure-asyncoperation' header. * @param {object} [pollingState] - The object to persist current operation state. * @param {boolean} [inPostOrDelete] - Invoked by Post Or Delete operation. */ -AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = function(pollingState, inPostOrDelete, callback) { - this._getStatus(pollingState.azureAsyncOperationHeaderLink, function(err, result) { +AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = function (pollingState, inPostOrDelete, callback) { + this._getStatus(pollingState.azureAsyncOperationHeaderLink, function (err, result) { if (err) return callback(err); - + if (!result.body || !result.body.status) { return callback(new Error('The response from long running operation does not contain a body.')); } - + pollingState.status = result.body.status; pollingState.error = result.body.error; pollingState.response = result.response; @@ -252,10 +259,10 @@ AzureServiceClient.prototype._updateStateFromAzureAsyncOperationHeader = functio }; /** - * Retrieve PUT/POST/PATCH/DELETE operation status by polling from 'location' header. + * Retrieve PUT operation status by polling from 'location' header. * @param {object} [pollingState] - The object to persist current operation state. */ -AzureServiceClient.prototype._updateStateFromLocationHeader = function(pollingState, callback) { +AzureServiceClient.prototype._updateStateFromLocationHeader = function (pollingState, callback) { this._getStatus(pollingState.locationHeaderLink, function(err, result) { if (err) return callback(err); @@ -286,29 +293,29 @@ AzureServiceClient.prototype._updateStateFromLocationHeader = function(pollingSt * @param {function} [resourceUrl] - The url of resource. * @param {object} [pollingState] - The object to persist current operation state. */ -AzureServiceClient.prototype._updateStateFromGetResourceOperation = function(resourceUrl, pollingState, callback) { - this._getStatus(resourceUrl, function(err, result) { +AzureServiceClient.prototype._updateStateFromGetResourceOperation = function (resourceUrl, pollingState, callback) { + this._getStatus(resourceUrl, function (err, result) { if (err) return callback(err); if (!result.body) { return callback(new Error('The response from long running operation does not contain a body.')); } - + if (result.body.properties && result.body.properties.provisioningState) { pollingState.status = result.body.properties.provisioningState; } else { pollingState.status = LroStates.Succeeded; } - + //we might not throw an error, but initialize here just in case. pollingState.error = { code: pollingState.status, message: util.format('Long running operation failed with status \'%s\'.', pollingState.status) }; - + pollingState.updateResponse(result.response); pollingState.request = result.request; pollingState.resource = result.body; - + //nothing to return, the 'pollingState' has all the info we care. callback(null); }); @@ -318,21 +325,21 @@ AzureServiceClient.prototype._updateStateFromGetResourceOperation = function(res * Retrieve operation status by querying the operation URL. * @param {string} [operationUrl] - URL used to poll operation result. */ -AzureServiceClient.prototype._getStatus = function(operationUrl, callback) { +AzureServiceClient.prototype._getStatus = function (operationUrl, callback) { var self = this; if (!operationUrl) { return callback(new Error('operationUrl cannot be null.')); } - + // Construct URL var requestUrl = operationUrl.replace(' ', '%20'); - + // Create HTTP transport objects var httpRequest = new WebResource(); httpRequest.method = 'GET'; httpRequest.headers = {}; httpRequest.url = requestUrl; - if (this._options) { + if(this._options) { for (var headerName in this._options['customHeaders']) { if (this._options['customHeaders'].hasOwnProperty(headerName)) { httpRequest.headers[headerName] = this._options['customHeaders'][headerName]; @@ -340,13 +347,13 @@ AzureServiceClient.prototype._getStatus = function(operationUrl, callback) { } } // Send Request - return self.pipeline(httpRequest, function(err, response, responseBody) { + return self.pipeline(httpRequest, function (err, response, responseBody) { if (err) { return callback(err); } var statusCode = response.statusCode; if (statusCode !== 200 && statusCode !== 201 && statusCode !== 202 && statusCode !== 204) { - var error = new Error(util.format('Invalid status code with response body "%s" occurred ' + + var error = new Error(util.format('Invalid status code with response body "%s" occurred ' + 'when polling for operation status.', responseBody)); error.statusCode = response.statusCode; error.request = msRest.stripRequest(httpRequest); @@ -371,8 +378,8 @@ AzureServiceClient.prototype._getStatus = function(operationUrl, callback) { try { result.body = JSON.parse(responseBody); } catch (deserializationError) { - var parseError = new Error(util.format('Error "%s" occurred in deserializing the response body - "%s" -' + - ' when polling for operation status.', deserializationError, responseBody)); + var parseError = new Error(util.format('Error "%s" occurred in deserializing the response body - "%s" -' + + ' when polling for operation status.', deserializationError, responseBody)); parseError.request = msRest.stripRequest(httpRequest); parseError.response = msRest.stripResponse(response); parseError.body = responseBody; diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js b/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js index 0d7c48fd1df0b..114eedf25c573 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/lib/pollingState.js @@ -25,21 +25,21 @@ function PollingState(resultOfInitialRequest, retryTimeout) { this.request = resultOfInitialRequest.request; //Parse response.body & assign it as the resource try { - if (resultOfInitialRequest.body && - typeof resultOfInitialRequest.body.valueOf() === 'string' && - resultOfInitialRequest.body.length > 0) { + if (resultOfInitialRequest.body && + typeof resultOfInitialRequest.body.valueOf() === 'string' && + resultOfInitialRequest.body.length > 0) { this.resource = JSON.parse(resultOfInitialRequest.body); } else { this.resource = resultOfInitialRequest.body; - } + } } catch (error) { - var deserializationError = new Error(util.format('Error "%s" occurred in parsing the responseBody ' + + var deserializationError = new Error(util.format('Error "%s" occurred in parsing the responseBody ' + 'while creating the PollingState for Long Running Operation- "%s"', error, resultOfInitialRequest.body)); deserializationError.request = resultOfInitialRequest.request; deserializationError.response = resultOfInitialRequest.response; throw deserializationError; } - + switch (this.response.statusCode) { case 202: this.status = LroStates.InProgress; @@ -72,7 +72,7 @@ function PollingState(resultOfInitialRequest, retryTimeout) { * Gets timeout in milliseconds. * @returns {number} timeout */ -PollingState.prototype.getTimeout = function() { +PollingState.prototype.getTimeout = function () { if (this._retryTimeout || this._retryTimeout === 0) { return this._retryTimeout * 1000; } @@ -86,13 +86,13 @@ PollingState.prototype.getTimeout = function() { * Update cached data using the provided response object * @param {object} [response] - provider response object. */ -PollingState.prototype.updateResponse = function(response) { +PollingState.prototype.updateResponse = function (response) { this.response = response; if (response && response.headers) { if (response.headers['azure-asyncoperation']) { this.azureAsyncOperationHeaderLink = response.headers['azure-asyncoperation']; } - + if (response.headers['location']) { this.locationHeaderLink = response.headers['location']; } @@ -103,7 +103,7 @@ PollingState.prototype.updateResponse = function(response) { * Returns long running operation result. * @returns {object} HttpOperationResponse */ -PollingState.prototype.getOperationResponse = function() { +PollingState.prototype.getOperationResponse = function () { var result = new msRest.HttpOperationResponse(); result.request = this.request; result.response = this.response; @@ -119,7 +119,7 @@ PollingState.prototype.getOperationResponse = function() { * Returns an Error on operation failure. * @returns {object} Error */ -PollingState.prototype.getCloudError = function(err) { +PollingState.prototype.getCloudError = function (err) { var errMsg; var errCode; @@ -133,11 +133,11 @@ PollingState.prototype.getCloudError = function(err) { parsedResponse = JSON.parse(this.response.body); } } catch (err) { - error.message = util.format('Error "%s" occurred while deserializing the error ' + + error.message = util.format('Error "%s" occurred while deserializing the error ' + 'message "%s" for long running operation.', err.message, this.response.body); return error; } - + if (err && err.message) { errMsg = util.format('Long running operation failed with error: \'%s\'.', err.message); } else { diff --git a/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js b/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js index bab92dc1e627c..f358540bb6aae 100644 --- a/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js +++ b/ClientRuntimes/NodeJS/ms-rest-azure/test/azureServiceClientTests.js @@ -92,7 +92,7 @@ describe('AzureServiceClient', function () { describe('Put', function () { resultOfInitialRequest.response.statusCode = 201; - + it('throw on not Lro related status code', function (done) { client.getPutOrPatchOperationResult({ response: {statusCode: 10000}, request: { url:"http://foo" }}, function (err, result) { err.message.should.containEql('Unexpected polling status code from long running operation'); @@ -205,7 +205,7 @@ describe('AzureServiceClient', function () { resultOfInitialRequest.body.properties.provisioningState = LroStates.Succeeded; it('throw on not Lro related status code', function (done) { - client.getPostOrDeleteOperationResult({ response: { statusCode: 203 }, request: {url: url_resource}}, function (err, result) { + client.getPostOrDeleteOperationResult({ response: { statusCode: 201 }, request: {url: url_resource, method: 'POST'}}, function (err, result) { err.message.should.containEql('Unexpected polling status code from long running operation'); done(); });