[Monitoring] Stop a new request when one is inflight (#27253) (#27341)

* Convert all pages to use the base controlller, then add logic in there to stop a new request when one is inflight * Reuse the promise * Undo logstash changes * Update in catch too * Add unit test * Fix cluster name showing up * Update broken test * Just use updateDataPromise
elastic · Dec 17, 2018 · 6d2b61f · 6d2b61f
1 parent 66eaefe
commit 6d2b61f
Show file tree

Hide file tree

Showing 21 changed files with 355 additions and 446 deletions.
diff --git a/x-pack/plugins/monitoring/public/components/elasticsearch/index/index.js b/x-pack/plugins/monitoring/public/components/elasticsearch/index/index.js
@@ -18,9 +18,9 @@ import { MonitoringTimeseriesContainer } from '../../chart';
 import { ShardAllocation } from '../shard_allocation/shard_allocation';
 
 export const Index = ({
+  scope,
   indexSummary,
   metrics,
-  scope,
   kbnUrl,
   ...props
 }) => {
@@ -51,7 +51,7 @@ export const Index = ({
             ))}
           </EuiFlexGrid>
           <EuiSpacer size="m"/>
-          <ShardAllocation scope={scope} kbnUrl={kbnUrl} type="index" />
+          <ShardAllocation scope={scope} {...props} kbnUrl={kbnUrl} type="index" />
         </EuiPageContent>
       </EuiPageBody>
     </EuiPage>

diff --git a/x-pack/plugins/monitoring/public/components/kibana/detail_status/index.js b/x-pack/plugins/monitoring/public/components/kibana/detail_status/index.js
@@ -21,6 +21,10 @@ function DetailStatusUI({ stats, intl }) {
 
   const metrics = [
     {
+      label: intl.formatMessage({
+        id: 'xpack.monitoring.kibana.detailStatus.transportAddressLabel',
+        defaultMessage: 'Transport Address'
+      }),
       value: transportAddress,
       'data-test-subj': 'transportAddress'
     },

diff --git a/x-pack/plugins/monitoring/public/components/logstash/detail_status/index.js b/x-pack/plugins/monitoring/public/components/logstash/detail_status/index.js
@@ -22,6 +22,9 @@ function DetailStatusUi({ stats, intl }) {
 
   const firstMetrics = [
     {
+      label: intl.formatMessage({
+        id: 'xpack.monitoring.logstash.detailStatus.transportAddressLabel', defaultMessage: 'Transport Address'
+      }),
       value: httpAddress,
       'data-test-subj': 'httpAddress'
     },

diff --git a/x-pack/plugins/monitoring/public/directives/all.js b/x-pack/plugins/monitoring/public/directives/all.js
@@ -7,7 +7,6 @@
 import './main';
 import './chart';
 import './sparkline';
-import './cluster/overview';
 import './cluster/listing';
 import './elasticsearch/cluster_status';
 import './elasticsearch/index_summary';

diff --git a/x-pack/plugins/monitoring/public/directives/cluster/overview/index.js b/x-pack/plugins/monitoring/public/directives/cluster/overview/index.js
diff --git a/x-pack/plugins/monitoring/public/directives/main/index.html b/x-pack/plugins/monitoring/public/directives/main/index.html
@@ -253,7 +253,7 @@ <h1 class="kuiTitle kuiFlexItem">{{ monitoringMain.pipelineId }}</h1>
         </div>
 
         <div ng-if="monitoringMain.inOverview" class="kuiLocalTabs" role="navigation">
-          <a class="kuiLocalTab" data-test-subj="clusterName">{{ cluster.cluster_name }}</a>
+          <a class="kuiLocalTab" data-test-subj="clusterName">{{ pageData.cluster_name }}</a>
         </div>
 
         <div ng-if="monitoringMain.inAlerts" class="kuiLocalTabs" role="navigation">

diff --git a/x-pack/plugins/monitoring/public/directives/main/index.js b/x-pack/plugins/monitoring/public/directives/main/index.js
@@ -101,6 +101,8 @@ uiModule.directive('monitoringMain', (breadcrumbs, license, kbnUrl, config) => {
         clusterName: get(scope, 'cluster.cluster_name')
       });
 
+      attributes.$observe('instance', instance => controller.instance = instance);
+      attributes.$observe('resolver', resolver => controller.resolver = resolver);
     }
   };
 });
diff --git a/x-pack/plugins/monitoring/public/views/__tests__/base_controller.js b/x-pack/plugins/monitoring/public/views/__tests__/base_controller.js
@@ -69,6 +69,25 @@ describe('MonitoringViewBaseController', function () {
     expect(executorService.start.calledOnce).to.be(true);
   });
 
+  it('does not allow for a new request if one is inflight', done => {
+    let counter = 0;
+    const opts = {
+      title: 'testo',
+      getPageData: () => Promise.resolve(++counter),
+      $injector,
+      $scope
+    };
+
+    const ctrl = new MonitoringViewBaseController(opts);
+    Promise.all([
+      ctrl.updateData(),
+      ctrl.updateData(),
+    ]).then(() => {
+      expect(counter).to.be(1);
+      done();
+    });
+  });
+
   describe('time filter', () => {
     it('enables timepicker and auto refresh #1', () => {
       expect(timefilter.isTimeRangeSelectorEnabled).to.be(true);

diff --git a/x-pack/plugins/monitoring/public/views/base_controller.js b/x-pack/plugins/monitoring/public/views/base_controller.js
@@ -75,7 +75,7 @@ export class MonitoringViewBaseController {
 
     titleService($scope.cluster, title);
 
-    this.data = { ...defaultData };
+    $scope.pageData = this.data = { ...defaultData };
     this._isDataInitialized = false;
     this.reactNodeId = reactNodeId;
 
@@ -96,12 +96,22 @@ export class MonitoringViewBaseController {
       timefilter.enableAutoRefreshSelector();
     }
 
+    this.updateDataPromise = null;
     this.updateData = () => {
+      if (this.updateDataPromise) {
+        // Do not sent another request if one is inflight
+        // See https://github.com/elastic/kibana/issues/24082
+        return this.updateDataPromise;
+      }
       const _api = apiUrlFn ? apiUrlFn() : api;
-      return _getPageData($injector, _api)
+      return this.updateDataPromise = _getPageData($injector, _api)
         .then(pageData => {
           this._isDataInitialized = true; // render will replace loading screen with the react component
-          this.data = pageData; // update the view's data with the fetch result
+          $scope.pageData = this.data = pageData; // update the view's data with the fetch result
+          this.updateDataPromise = null;
+        })
+        .catch(() => {
+          this.updateDataPromise = null;
         });
     };
     this.updateData();
@@ -126,6 +136,8 @@ export class MonitoringViewBaseController {
         mode: 'absolute'
       });
     };
+
+    this.setTitle = title => titleService($scope.cluster, title);
   }
 
   renderReact(component) {

diff --git a/x-pack/plugins/monitoring/public/views/cluster/overview/index.html b/x-pack/plugins/monitoring/public/views/cluster/overview/index.html
@@ -1,3 +1,3 @@
 <monitoring-main name="overview" data-test-subj="clusterOverviewContainer">
-  <monitoring-cluster-overview cluster="cluster"></monitoring-cluster-overview>
+  <div id="monitoringClusterOverviewApp"></div>
 </monitoring-main>
diff --git a/x-pack/plugins/monitoring/public/views/cluster/overview/index.js b/x-pack/plugins/monitoring/public/views/cluster/overview/index.js
@@ -3,11 +3,13 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-
+import React from 'react';
 import uiRoutes from 'ui/routes';
 import { routeInitProvider } from 'plugins/monitoring/lib/route_init';
 import template from './index.html';
-import { timefilter } from 'ui/timefilter';
+import { MonitoringViewBaseController } from '../../';
+import { Overview } from 'plugins/monitoring/components/cluster/overview';
+import { I18nProvider } from '@kbn/i18n/react';
 
 uiRoutes.when('/overview', {
   template,
@@ -21,28 +23,40 @@ uiRoutes.when('/overview', {
       return monitoringClusters(globalState.cluster_uuid, globalState.ccs);
     }
   },
-  controller($injector, $scope, i18n) {
-    timefilter.enableTimeRangeSelector();
-    timefilter.enableAutoRefreshSelector();
-
-    const $route = $injector.get('$route');
-    $scope.cluster = $route.current.locals.cluster;
+  controller: class extends MonitoringViewBaseController {
+    constructor($injector, $scope, i18n) {
+      const kbnUrl = $injector.get('kbnUrl');
+      const monitoringClusters = $injector.get('monitoringClusters');
+      const globalState = $injector.get('globalState');
 
-    const title = $injector.get('title');
-    title($scope.cluster, i18n('xpack.monitoring.cluster.overviewTitle', { defaultMessage: 'Overview' }));
+      super({
+        title: i18n('xpack.monitoring.cluster.overviewTitle', {
+          defaultMessage: 'Overview'
+        }),
+        defaultData: {},
+        getPageData: () => monitoringClusters(globalState.cluster_uuid, globalState.ccs),
+        reactNodeId: 'monitoringClusterOverviewApp',
+        $scope,
+        $injector
+      });
 
-    const $executor = $injector.get('$executor');
-    const monitoringClusters = $injector.get('monitoringClusters');
-    const globalState = $injector.get('globalState');
-    $executor.register({
-      execute: () => monitoringClusters(globalState.cluster_uuid, globalState.ccs),
-      handleResponse(cluster) {
-        $scope.cluster = cluster;
-      }
-    });
+      const changeUrl = target => {
+        $scope.$evalAsync(() => {
+          kbnUrl.changePath(target);
+        });
+      };
 
-    $executor.start($scope);
-
-    $scope.$on('$destroy', $executor.destroy);
+      $scope.$watch(() => this.data, data => {
+        this.renderReact(
+          <I18nProvider>
+            <Overview
+              cluster={data}
+              changeUrl={changeUrl}
+              showLicenseExpiration={true}
+            />
+          </I18nProvider>
+        );
+      });
+    }
   }
 });
diff --git a/x-pack/plugins/monitoring/public/views/elasticsearch/index/advanced/index.html b/x-pack/plugins/monitoring/public/views/elasticsearch/index/advanced/index.html
@@ -1,7 +1,7 @@
 <monitoring-main
   product="elasticsearch" name="indices"
-  instance="{{ indexName }}"
-  resolver="{{ indexName }}"
+  instance="{{ monitoringElasticsearchAdvancedIndexApp.indexName }}"
+  resolver="{{ monitoringElasticsearchAdvancedIndexApp.indexName }}"
   page="advanced"
 >
   <div id="monitoringElasticsearchAdvancedIndexApp"></div>

diff --git a/x-pack/plugins/monitoring/public/views/elasticsearch/index/advanced/index.js b/x-pack/plugins/monitoring/public/views/elasticsearch/index/advanced/index.js
@@ -8,16 +8,14 @@
  * Controller for Advanced Index Detail
  */
 import React from 'react';
-import { render } from 'react-dom';
-import { find } from 'lodash';
 import uiRoutes from 'ui/routes';
 import { ajaxErrorHandlersProvider } from 'plugins/monitoring/lib/ajax_error_handler';
 import { routeInitProvider } from 'plugins/monitoring/lib/route_init';
 import template from './index.html';
 import { timefilter } from 'ui/timefilter';
 import { AdvancedIndex } from '../../../../components/elasticsearch/index/advanced';
 import { I18nProvider } from '@kbn/i18n/react';
-import moment from 'moment';
+import { MonitoringViewBaseController } from '../../../base_controller';
 
 function getPageData($injector) {
   const globalState = $injector.get('globalState');
@@ -51,57 +49,39 @@ uiRoutes.when('/elasticsearch/indices/:index/advanced', {
     },
     pageData: getPageData
   },
-  controller($injector, $scope, i18n) {
-    timefilter.enableTimeRangeSelector();
-    timefilter.enableAutoRefreshSelector();
+  controllerAs: 'monitoringElasticsearchAdvancedIndexApp',
+  controller: class extends MonitoringViewBaseController {
+    constructor($injector, $scope, i18n) {
+      const $route = $injector.get('$route');
+      const indexName = $route.current.params.index;
 
-    const $route = $injector.get('$route');
-    const globalState = $injector.get('globalState');
-    $scope.cluster = find($route.current.locals.clusters, { cluster_uuid: globalState.cluster_uuid });
-    $scope.indexName = $route.current.params.index;
-    $scope.pageData = $route.current.locals.pageData;
-
-    const title = $injector.get('title');
-    const routeTitle = i18n('xpack.monitoring.elasticsearch.indices.advanced.routeTitle', {
-      defaultMessage: 'Elasticsearch - Indices - {indexName} - Advanced',
-      values: {
-        indexName: $scope.indexName
-      }
-    });
-
-    title($scope.cluster, routeTitle);
-
-    const $executor = $injector.get('$executor');
-    $executor.register({
-      execute: () => getPageData($injector),
-      handleResponse: (response) => $scope.pageData = response
-    });
-
-    $executor.start($scope);
+      super({
+        title: i18n('xpack.monitoring.elasticsearch.indices.advanced.routeTitle', {
+          defaultMessage: 'Elasticsearch - Indices - {indexName} - Advanced',
+          values: {
+            indexName,
+          }
+        }),
+        defaultData: {},
+        getPageData,
+        reactNodeId: 'monitoringElasticsearchAdvancedIndexApp',
+        $scope,
+        $injector
+      });
 
-    $scope.$on('$destroy', $executor.destroy);
+      this.indexName = indexName;
 
-    function onBrush({ xaxis }) {
-      timefilter.setTime({
-        from: moment(xaxis.from),
-        to: moment(xaxis.to),
-        mode: 'absolute',
+      $scope.$watch(() => this.data, data => {
+        this.renderReact(
+          <I18nProvider>
+            <AdvancedIndex
+              indexSummary={data.indexSummary}
+              metrics={data.metrics}
+              onBrush={this.onBrush}
+            />
+          </I18nProvider>
+        );
       });
     }
-
-    this.renderReact = () => {
-      render(
-        <I18nProvider>
-          <AdvancedIndex
-            indexSummary={$scope.pageData.indexSummary}
-            metrics={$scope.pageData.metrics}
-            onBrush={onBrush}
-          />
-        </I18nProvider>,
-        document.getElementById('monitoringElasticsearchAdvancedIndexApp')
-      );
-    };
-
-    $scope.$watch('pageData', this.renderReact);
   }
 });
diff --git a/x-pack/plugins/monitoring/public/views/elasticsearch/index/index.html b/x-pack/plugins/monitoring/public/views/elasticsearch/index/index.html
@@ -1,8 +1,8 @@
 <monitoring-main
   product="elasticsearch"
   name="indices"
-  instance="{{ indexName }}"
-  resolver="{{ indexName }}"
+  instance="{{ monitoringElasticsearchIndexApp.indexName }}"
+  resolver="{{ monitoringElasticsearchIndexApp.indexName }}"
   page="overview"
 >
   <div id="monitoringElasticsearchIndexApp"></div>