Repository: ambari Updated Branches: refs/heads/trunk 506bb8d18 -> a91890a81
AMBARI-15070. Graph scale behaviour is incorrect if time range is switched before graph data API call is complete (alexantonenko) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/a91890a8 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/a91890a8 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/a91890a8 Branch: refs/heads/trunk Commit: a91890a819e77638d5cb371e49a9a5d0fdebf4d1 Parents: 506bb8d Author: Alex Antonenko <[email protected]> Authored: Wed Feb 17 15:20:14 2016 +0200 Committer: Alex Antonenko <[email protected]> Committed: Wed Feb 17 17:46:45 2016 +0200 ---------------------------------------------------------------------- .../app/mixins/common/widgets/widget_mixin.js | 62 +++++++++++---- ambari-web/app/utils/ajax/ajax.js | 12 +++ .../app/views/common/chart/linear_time.js | 84 +++++++++++++++----- .../main/admin/stack_upgrade/versions_view.js | 6 +- .../test/mixins/common/widget_mixin_test.js | 18 ++++- ambari-web/test/utils/ajax/ajax_test.js | 32 ++++++++ .../test/views/common/chart/linear_time_test.js | 10 ++- 7 files changed, 179 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/app/mixins/common/widgets/widget_mixin.js ---------------------------------------------------------------------- diff --git a/ambari-web/app/mixins/common/widgets/widget_mixin.js b/ambari-web/app/mixins/common/widgets/widget_mixin.js index 6d65c33..13e55f2 100644 --- a/ambari-web/app/mixins/common/widgets/widget_mixin.js +++ b/ambari-web/app/mixins/common/widgets/widget_mixin.js @@ -120,9 +120,18 @@ App.WidgetMixin = Ember.Mixin.create({ startCallName: 'getHostComponentMetrics', successCallback: this.getHostComponentMetricsSuccessCallback, errorCallback: this.getMetricsErrorCallback, - completeCallback: function () { + completeCallback: function (xhr) { requestCounter--; if (requestCounter === 0) this.onMetricsLoaded(); + if (this.get('graphView')) { + var graph = this.get('childViews') && this.get('childViews').findProperty('runningRequests'); + if (graph) { + var requestsArrayName = graph.get('isPopup') ? 'runningPopupRequests' : 'runningRequests'; + graph.set(requestsArrayName, graph.get(requestsArrayName).reject(function (item) { + return item === xhr; + })); + } + } } }); } else { @@ -132,9 +141,18 @@ App.WidgetMixin = Ember.Mixin.create({ startCallName: 'getServiceComponentMetrics', successCallback: this.getMetricsSuccessCallback, errorCallback: this.getMetricsErrorCallback, - completeCallback: function () { + completeCallback: function (xhr) { requestCounter--; if (requestCounter === 0) this.onMetricsLoaded(); + if (this.get('graphView')) { + var graph = this.get('childViews') && this.get('childViews').findProperty('runningRequests'); + if (graph) { + var requestsArrayName = graph.get('isPopup') ? 'runningPopupRequests' : 'runningRequests'; + graph.set(requestsArrayName, graph.get(requestsArrayName).reject(function (item) { + return item === xhr; + })); + } + } } }); } @@ -192,7 +210,7 @@ App.WidgetMixin = Ember.Mixin.create({ * @returns {$.ajax} */ getServiceComponentMetrics: function (request) { - return App.ajax.send({ + var xhr = App.ajax.send({ name: 'widgets.serviceComponent.metrics.get', sender: this, data: { @@ -201,6 +219,14 @@ App.WidgetMixin = Ember.Mixin.create({ metricPaths: this.prepareMetricPaths(request.metric_paths) } }); + if (this.get('graphView')) { + var graph = this.get('childViews') && this.get('childViews').findProperty('runningRequests'); + if (graph) { + var requestsArrayName = graph.get('isPopup') ? 'runningPopupRequests' : 'runningRequests'; + graph.get(requestsArrayName).push(xhr); + } + } + return xhr; }, /** @@ -230,15 +256,21 @@ App.WidgetMixin = Ember.Mixin.create({ var metricPaths = this.prepareMetricPaths(request.metric_paths); if (metricPaths.length) { - return App.ajax.send({ - name: 'widgets.hostComponent.metrics.get', - sender: this, - data: { - componentName: request.component_name, - metricPaths: this.prepareMetricPaths(request.metric_paths), - hostComponentCriteria: this.computeHostComponentCriteria(request) - } - }); + var xhr = App.ajax.send({ + name: 'widgets.hostComponent.metrics.get', + sender: this, + data: { + componentName: request.component_name, + metricPaths: this.prepareMetricPaths(request.metric_paths), + hostComponentCriteria: this.computeHostComponentCriteria(request) + } + }), + graph = this.get('graphView') && this.get('childViews') && this.get('childViews').findProperty('runningRequests'); + if (graph) { + var requestsArrayName = graph.get('isPopup') ? 'runningPopupRequests' : 'runningRequests'; + graph.get(requestsArrayName).push(xhr); + } + return xhr; } return jQuery.Deferred().reject().promise(); }, @@ -300,7 +332,7 @@ App.WidgetMixin = Ember.Mixin.create({ * @param {string} errorThrown */ getMetricsErrorCallback: function (xhr, textStatus, errorThrown) { - if (this.get('graphView')) { + if (this.get('graphView') && !xhr.isForcedAbort) { var graph = this.get('childViews') && this.get('childViews').findProperty('_showMessage'); if (graph) { if (xhr.readyState == 4 && xhr.status) { @@ -778,9 +810,9 @@ App.WidgetLoadAggregator = Em.Object.create({ subRequest.errorCallback.call(subRequest.context, xhr, textStatus, errorThrown); } }, this); - }).always(function () { + }).always(function (xhr) { _request.subRequests.forEach(function (subRequest) { - subRequest.completeCallback.call(subRequest.context); + subRequest.completeCallback.call(subRequest.context, xhr); }, this); }); })(bulks[id]); http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/app/utils/ajax/ajax.js ---------------------------------------------------------------------- diff --git a/ambari-web/app/utils/ajax/ajax.js b/ambari-web/app/utils/ajax/ajax.js index f2174f3..3ccd01c 100644 --- a/ambari-web/app/utils/ajax/ajax.js +++ b/ambari-web/app/utils/ajax/ajax.js @@ -3054,6 +3054,18 @@ var ajax = Em.Object.extend({ */ defaultErrorKDCHandler: function(opt, msg) { return App.showInvalidKDCPopup(opt, msg); + }, + + /** + * Abort all requests stored in the certain array + * @param requestsArray + */ + abortRequests: function (requestsArray) { + requestsArray.forEach(function (xhr) { + xhr.isForcedAbort = true; + xhr.abort(); + }); + requestsArray.clear(); } }); http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/app/views/common/chart/linear_time.js ---------------------------------------------------------------------- diff --git a/ambari-web/app/views/common/chart/linear_time.js b/ambari-web/app/views/common/chart/linear_time.js index c7dd3f9..064bec3 100644 --- a/ambari-web/app/views/common/chart/linear_time.js +++ b/ambari-web/app/views/common/chart/linear_time.js @@ -167,6 +167,20 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { */ seriesTemplate: null, + /** + * Incomplete metrics requests + * @type {array} + * @default [] + */ + runningRequests: [], + + /** + * Incomplete metrics requests for detailed view + * @type {array} + * @default [] + */ + runningPopupRequests: [], + _containerSelector: Em.computed.format('#{0}-container', 'id'), _popupSelector: Em.computed.concat('', '_containerSelector', 'popupSuffix'), @@ -295,6 +309,7 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { this.$("[rel='ZoomInTooltip']").tooltip('destroy'); $(this.get('_containerSelector') + ' li.line').off(); $(this.get('_popupSelector') + ' li.line').off(); + App.ajax.abortRequests(this.get('runningRequests')); }, registerGraph: function () { @@ -307,16 +322,26 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { }, loadData: function () { - if (this.get('loadGroup') && !this.get('isPopup')) { + var self = this, + isPopup = this.get('isPopup'); + if (this.get('loadGroup') && !isPopup) { return App.ChartLinearTimeView.LoadAggregator.add(this, this.get('loadGroup')); } else { - return App.ajax.send({ - name: this.get('ajaxIndex'), - sender: this, - data: this.getDataForAjaxRequest(), - success: 'loadDataSuccessCallback', - error: 'loadDataErrorCallback' - }); + var requestsArrayName = isPopup ? 'runningPopupRequests' : 'runningRequests', + request = App.ajax.send({ + name: this.get('ajaxIndex'), + sender: this, + data: this.getDataForAjaxRequest(), + success: 'loadDataSuccessCallback', + error: 'loadDataErrorCallback', + callback: function () { + self.set(requestsArrayName, self.get(requestsArrayName).reject(function (item) { + return item === request; + })); + } + }); + this.get(requestsArrayName).push(request); + return request; } }, @@ -356,15 +381,17 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { }, loadDataErrorCallback: function (xhr, textStatus, errorThrown) { - this.set('isReady', true); - if (xhr.readyState == 4 && xhr.status) { - textStatus = xhr.status + " " + textStatus; + if (!xhr.isForcedAbort) { + this.set('isReady', true); + if (xhr.readyState == 4 && xhr.status) { + textStatus = xhr.status + " " + textStatus; + } + this._showMessage('warn', this.t('graphs.error.title'), this.t('graphs.error.message').format(textStatus, errorThrown)); + this.setProperties({ + hasData: false, + isExportButtonHidden: true + }); } - this._showMessage('warn', this.t('graphs.error.title'), this.t('graphs.error.message').format(textStatus, errorThrown)); - this.setProperties({ - hasData: false, - isExportButtonHidden: true - }); }, /** @@ -999,6 +1026,7 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { customDurationFormatted: targetView.get('customDurationFormatted'), isPopup: false }); + App.ajax.abortRequests(this.get('graph.runningPopupRequests')); this._super(); }, @@ -1034,6 +1062,7 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { this.set('childViews.firstObject.currentTimeRangeIndex', index); this.set('currentTimeIndex', index); self.set('currentTimeIndex', index); + App.ajax.abortRequests(this.get('graph.runningPopupRequests')); }, currentTimeIndex: self.get('currentTimeIndex'), @@ -1082,6 +1111,9 @@ App.ChartLinearTimeView = Ember.View.extend(App.ExportMetricsMixin, { customEndTime: customEndTime, customDurationFormatted: customDurationFormatted }); + if (index !== 8 || targetView.get('customStartTime') && targetView.get('customEndTime')) { + App.ajax.abortRequests(this.get('runningRequests')); + } }.observes('parentView.parentView.currentTimeRangeIndex', 'parentView.currentTimeRangeIndex', 'parentView.parentView.customStartTime', 'parentView.customStartTime', 'parentView.parentView.customEndTime', 'parentView.customEndTime'), timeUnitSeconds: 3600, timeUnitSecondsSetter: function () { @@ -1408,25 +1440,33 @@ App.ChartLinearTimeView.LoadAggregator = Em.Object.create({ (function (_request) { var fields = self.formatRequestData(_request); var hostName = (_request.context.get('content')) ? _request.context.get('content.hostName') : ""; - - App.ajax.send({ + var xhr = App.ajax.send({ name: _request.name, sender: _request.context, data: { fields: fields, hostName: hostName } - }).done(function (response) { + }); + + xhr.done(function (response) { console.time('==== runRequestsDone'); _request.subRequests.forEach(function (subRequest) { subRequest.context._refreshGraph.call(subRequest.context, response); }, this); console.timeEnd('==== runRequestsDone'); }).fail(function (jqXHR, textStatus, errorThrown) { - _request.subRequests.forEach(function (subRequest) { - subRequest.context.loadDataErrorCallback.call(subRequest.context, jqXHR, textStatus, errorThrown); - }, this); + if (!jqXHR.isForcedAbort) { + _request.subRequests.forEach(function (subRequest) { + subRequest.context.loadDataErrorCallback.call(subRequest.context, jqXHR, textStatus, errorThrown); + }, this); + } + }).always(function () { + _request.context.set('runningRequests', _request.context.get('runningRequests').reject(function (item) { + return item === xhr; + })); }); + _request.context.get('runningRequests').push(xhr); })(bulks[id]); } }, http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/app/views/main/admin/stack_upgrade/versions_view.js ---------------------------------------------------------------------- diff --git a/ambari-web/app/views/main/admin/stack_upgrade/versions_view.js b/ambari-web/app/views/main/admin/stack_upgrade/versions_view.js index d535300..3649513 100644 --- a/ambari-web/app/views/main/admin/stack_upgrade/versions_view.js +++ b/ambari-web/app/views/main/admin/stack_upgrade/versions_view.js @@ -199,12 +199,8 @@ App.MainAdminStackVersionsView = Em.View.extend({ * stop polling upgrade state */ willDestroyElement: function () { - var runningCheckRequests = this.get('controller.runningCheckRequests'); window.clearTimeout(this.get('updateTimer')); - runningCheckRequests.forEach(function (request) { - request.abort(); - }); - runningCheckRequests.clear(); + App.ajax.abortRequests(this.get('controller.runningCheckRequests')); }, /** http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/test/mixins/common/widget_mixin_test.js ---------------------------------------------------------------------- diff --git a/ambari-web/test/mixins/common/widget_mixin_test.js b/ambari-web/test/mixins/common/widget_mixin_test.js index e32026a..c70150d 100644 --- a/ambari-web/test/mixins/common/widget_mixin_test.js +++ b/ambari-web/test/mixins/common/widget_mixin_test.js @@ -394,6 +394,7 @@ describe('App.WidgetMixin', function () { cases = [ { graphView: null, + isForcedAbort: false, metricsLength: 1, showMessageCallCount: 0, isExportButtonHidden: false, @@ -401,6 +402,7 @@ describe('App.WidgetMixin', function () { }, { graphView: {}, + isForcedAbort: false, metricsLength: 1, showMessageCallCount: 0, isExportButtonHidden: false, @@ -409,6 +411,7 @@ describe('App.WidgetMixin', function () { { graphView: {}, childViews: [], + isForcedAbort: false, metricsLength: 1, showMessageCallCount: 0, isExportButtonHidden: false, @@ -417,6 +420,7 @@ describe('App.WidgetMixin', function () { { graphView: {}, childViews: [Em.Object.create({})], + isForcedAbort: false, metricsLength: 1, showMessageCallCount: 0, isExportButtonHidden: false, @@ -425,10 +429,20 @@ describe('App.WidgetMixin', function () { { graphView: {}, childViews: [Em.Object.create({}), view], + isForcedAbort: false, metricsLength: 0, showMessageCallCount: 1, isExportButtonHidden: true, title: 'graph view is available' + }, + { + graphView: {}, + childViews: [Em.Object.create({}), view], + isForcedAbort: true, + metricsLength: 1, + showMessageCallCount: 0, + isExportButtonHidden: false, + title: 'request is aborted' } ], messageCases = [ @@ -471,7 +485,9 @@ describe('App.WidgetMixin', function () { graphView: item.graphView, childViews: item.childViews }); - obj.getMetricsErrorCallback({}); + obj.getMetricsErrorCallback({ + isForcedAbort: item.isForcedAbort + }); }); it('metrics array', function () { http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/test/utils/ajax/ajax_test.js ---------------------------------------------------------------------- diff --git a/ambari-web/test/utils/ajax/ajax_test.js b/ambari-web/test/utils/ajax/ajax_test.js index b0749df..747ccff 100644 --- a/ambari-web/test/utils/ajax/ajax_test.js +++ b/ambari-web/test/utils/ajax/ajax_test.js @@ -162,4 +162,36 @@ describe('App.ajax', function() { }); }); }); + + describe('#abortRequests', function () { + + var xhr = { + abort: Em.K + }, + requests; + + beforeEach(function () { + sinon.spy(xhr, 'abort'); + xhr.isForcedAbort = false; + requests = [xhr, xhr]; + App.ajax.abortRequests(requests); + }); + + afterEach(function () { + xhr.abort.restore(); + }); + + it('should abort all requests', function () { + expect(xhr.abort.calledTwice).to.be.true; + }); + + it('should mark request as aborted', function () { + expect(xhr.isForcedAbort).to.be.true; + }); + + it('should clear requests array', function () { + expect(requests).to.have.length(0); + }); + + }); }); http://git-wip-us.apache.org/repos/asf/ambari/blob/a91890a8/ambari-web/test/views/common/chart/linear_time_test.js ---------------------------------------------------------------------- diff --git a/ambari-web/test/views/common/chart/linear_time_test.js b/ambari-web/test/views/common/chart/linear_time_test.js index 7d76520..c547249 100644 --- a/ambari-web/test/views/common/chart/linear_time_test.js +++ b/ambari-web/test/views/common/chart/linear_time_test.js @@ -543,7 +543,8 @@ describe('App.ChartLinearTimeView.LoadAggregator', function () { sinon.stub(App.ajax, 'send', function(){ return { done: Em.K, - fail: Em.K + fail: Em.K, + always: Em.K } }); }); @@ -552,7 +553,12 @@ describe('App.ChartLinearTimeView.LoadAggregator', function () { aggregator.formatRequestData.restore(); }); it("valid request is sent", function () { - var context = Em.Object.create({content: {hostName: 'host1'}}); + var context = Em.Object.create({ + content: { + hostName: 'host1' + }, + runningRequests: [] + }); var requests = { 'r1': { name: 'r1',
