perf(core/executions): consider pipeline filters when fetching execut…

…ions (#4509)

app/scripts/modules/core/src/application/service/applicationDataSource.ts

@@ -1,4 +1,4 @@
-import { ILogService, IPromise, IQService, IScope } from 'angular';
+import { IDeferred, ILogService, IPromise, IQService, IScope } from 'angular';
 import { get } from 'lodash';
 import { UIRouter } from '@uirouter/core';
 import { Subject, Subscription } from 'rxjs';
@@ -258,10 +258,23 @@ export class ApplicationDataSource implements IDataSourceConfig {
    */
   public lastRefresh: number;
 
-  private refreshStream: Subject<any> = new Subject();
+  private refreshStream: Subject<void> = new Subject();
 
   private refreshFailureStream: Subject<any> = new Subject();
 
+  /**
+   * Simple counter used to track the most recent refresh call to avoid data stomping
+   * when multiple force refresh calls occur
+   * (will go away when we switch from Promises to Observables)
+   */
+  private currentLoadCall = 0;
+
+  /**
+   * Dumb queue to fire when the most recent refresh call finishes
+   * (will go away when we switch from Promises to Observables)
+   */
+  private refreshQueue: IDeferred<void>[] = [];
+
   /**
    * Called when a method mutates some item in the data source's data, e.g. when a running execution is updated
    * independent of the execution data source's refresh cycle
@@ -365,8 +378,8 @@ export class ApplicationDataSource implements IDataSourceConfig {
    *
    * @returns {IPromise<T>}
    */
-  public ready(): IPromise<any> {
-    const deferred = this.$q.defer();
+  public ready(): IPromise<void> {
+    const deferred = this.$q.defer<void>();
     if (this.disabled || this.loaded || (this.lazy && !this.active)) {
       deferred.resolve();
     } else if (this.loadFailure) {
@@ -394,15 +407,17 @@ export class ApplicationDataSource implements IDataSourceConfig {
    * to true, and the promise will resolve immediately.
    *
    * If the data source is lazy and its "active" flag is set to false, any existing data will be cleared, the "loaded"
-   * flag will be set to false, adn the promise will resolve immediately.
+   * flag will be set to false, and the promise will resolve immediately.
    *
    * If the data source is in the process of loading, the promise will resolve immediately. This behavior can be
    * overridden by calling "refresh(true)".
    *
    * @param forceRefresh
    * @returns {any}
    */
-  public refresh(forceRefresh?: boolean): IPromise<any> {
+  public refresh(forceRefresh?: boolean): IPromise<void> {
+    const deferred = this.$q.defer<void>();
+    this.refreshQueue.push(deferred);
     if (!this.loader || this.disabled) {
       this.data.length = 0;
       this.loading = false;
@@ -419,8 +434,17 @@ export class ApplicationDataSource implements IDataSourceConfig {
       return this.$q.when(null);
     }
     this.loading = true;
-    return this.loader(this.application)
+
+    this.currentLoadCall += 1;
+    const loadCall = this.currentLoadCall;
+    this.loader(this.application)
       .then((result) => {
+        if (loadCall < this.currentLoadCall) {
+          // discard, more recent call has come in
+          // TODO: this will all be cleaner with Observables
+          this.$log.debug(`Discarding load #${loadCall} for ${this.key} - current is #${this.currentLoadCall}`);
+          return;
+        }
         this.onLoad(this.application, result).then(data => {
           if (data) {
             this.data = data;
@@ -435,13 +459,21 @@ export class ApplicationDataSource implements IDataSourceConfig {
           this.addAlerts();
           this.dataUpdated();
         });
+        this.refreshQueue.forEach(d => d.resolve());
+        this.refreshQueue.length = 0;
       })
       .catch((rejection) => {
-        this.$log.warn(`Error retrieving ${this.key}`, rejection);
-        this.loading = false;
-        this.loadFailure = true;
-        this.refreshFailureStream.next(rejection);
+        if (loadCall === this.currentLoadCall) {
+          this.$log.warn(`Error retrieving ${this.key}`, rejection);
+          this.loading = false;
+          this.loadFailure = true;
+          this.refreshFailureStream.next(rejection);
+          // resolve, don't reject - the refreshFailureStream and loadFailure flags signal the rejection
+          this.refreshQueue.forEach(d => d.resolve(rejection));
+          this.refreshQueue.length = 0;
+        }
       });
+    return deferred.promise;
   }
 
   /**

app/scripts/modules/core/src/pipeline/config/pipelineConfigurer.js

@@ -373,7 +373,7 @@ module.exports = angular.module('spinnaker.core.pipeline.config.pipelineConfigur
     };
 
     this.getPipelineExecutions = () => {
-      executionService.getExecutionsForConfigIds($scope.pipeline.application, $scope.pipeline.id, 5)
+      executionService.getExecutionsForConfigIds($scope.pipeline.application, [$scope.pipeline.id], 5)
         .then(executions => {
           executions.forEach(execution => executionsTransformer.addBuildInfo(execution));
           $scope.pipelineExecutions = executions;

app/scripts/modules/core/src/pipeline/config/triggers/pipeline/pipelineTriggerOptions.directive.js

@@ -54,7 +54,7 @@ module.exports = angular
         return;
       }
       this.viewState.executionsLoading = true;
-      executionService.getExecutions(command.trigger.application, {statuses: []})
+      executionService.getExecutions(command.trigger.application)
         .then(executionLoadSuccess, executionLoadFailure);
     };
 

app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx

@@ -181,7 +181,11 @@ export class Execution extends React.Component<IExecutionProps, IExecutionState>
         executionService.getExecution(execution.id).then((updated: IExecution) => {
           if (this.mounted) {
             const dataSource = application.getDataSource(this.props.dataSourceKey);
-            executionService.updateExecution(application, updated, dataSource);
+            // if we are already in the middle of a refresh, leave the running execution alone,
+            // lest we trigger a dataUpdated event and clear the reloadingForFilters flag
+            if (!this.props.application.executions.reloadingForFilters) {
+              executionService.updateExecution(application, updated, dataSource);
+            }
             executionService.removeCompletedExecutionsFromRunningData(application);
           }
           refreshing = false;

app/scripts/modules/core/src/pipeline/filter/ExecutionFilters.tsx

@@ -78,13 +78,13 @@ export class ExecutionFilters extends React.Component<IExecutionFiltersProps, IE
   private refreshExecutions(): void {
     ReactInjector.executionFilterModel.asFilterModel.applyParamsToUrl();
     this.props.application.executions.reloadingForFilters = true;
-    this.props.application.executions.refresh();
+    this.props.application.executions.refresh(true);
   }
 
   private clearFilters(): void {
     ReactGA.event({ category: 'Pipelines', action: `Filter: clear all (side nav)` });
     ReactInjector.executionFilterService.clearFilters();
-    ReactInjector.executionFilterService.updateExecutionGroups(this.props.application);
+    this.refreshExecutions();
   }
 
   private getPipelineNames(): string[] {
@@ -188,7 +188,7 @@ export class ExecutionFilters extends React.Component<IExecutionFiltersProps, IE
                 <Pipelines
                   names={pipelineNames}
                   dragEnabled={pipelineReorderEnabled}
-                  update={this.updateExecutionGroups}
+                  update={this.refreshExecutions}
                   onSortEnd={this.handleSortEnd}
                 />
                 { pipelineNames.length && (

app/scripts/modules/core/src/pipeline/pipeline.dataSource.js

@@ -21,7 +21,7 @@ module.exports = angular
     };
 
     let loadExecutions = (application) => {
-      return executionService.getExecutions(application.name);
+      return executionService.getExecutions(application.name, application);
     };
 
     let loadPipelineConfigs = (application) => {

app/scripts/modules/core/src/pipeline/service/execution.service.ts

@@ -11,6 +11,7 @@ import { PIPELINE_CONFIG_PROVIDER, PipelineConfigProvider } from 'core/pipeline/
 import { SETTINGS } from 'core/config/settings';
 import { ApplicationDataSource } from 'core/application/service/applicationDataSource';
 import { DebugWindow } from 'core/utils/consoleDebug';
+import { IPipeline } from 'core/domain/IPipeline';
 
 export class ExecutionService {
   public get activeStatuses(): string[] { return ['RUNNING', 'SUSPENDED', 'PAUSED', 'NOT_STARTED']; }
@@ -28,13 +29,16 @@ export class ExecutionService {
   }
 
     public getRunningExecutions(applicationName: string): IPromise<IExecution[]> {
-      return this.getFilteredExecutions(applicationName, { statuses: this.activeStatuses, limit: this.runningLimit });
+      return this.getFilteredExecutions(applicationName, this.activeStatuses, this.runningLimit);
     }
 
-    private getFilteredExecutions(applicationName: string, { statuses = Object.keys(pickBy(this.executionFilterModel.asFilterModel.sortFilter.status || {}, identity)), limit = this.executionFilterModel.asFilterModel.sortFilter.count } = {}): IPromise<IExecution[]> {
-      const statusString = statuses.map((status) => status.toUpperCase()).join(',') || null;
-      return this.API.one('applications', applicationName).all('pipelines').getList({ limit: limit, statuses: statusString })
-        .then((data: IExecution[]) => {
+    private getFilteredExecutions(applicationName: string, statuses: string[], limit: number, pipelineConfigIds: string[] = null): IPromise<IExecution[]> {
+        const statusString = statuses.map((status) => status.toUpperCase()).join(',') || null;
+        const call = pipelineConfigIds ?
+          this.API.all('executions').getList({ limit, pipelineConfigIds, statuses }) :
+          this.API.one('applications', applicationName).all('pipelines').getList({ limit, statuses: statusString, pipelineConfigIds });
+
+        return call.then((data: IExecution[]) => {
           if (data) {
             data.forEach((execution: IExecution) => this.cleanExecutionForDiffing(execution));
             return data;
@@ -43,8 +47,23 @@ export class ExecutionService {
         });
     }
 
-    public getExecutions(applicationName: string): IPromise<IExecution[]> {
-      return this.getFilteredExecutions(applicationName);
+  /**
+   * Returns a filtered list of executions for the given application
+   * @param {string} applicationName the name of the application
+   * @param {Application} application: if supplied, and pipeline parameters are present on the filter model, the
+   * application will be used to correlate and filter the retrieved executions to only include those pipelines
+   * @return {<IExecution[]>}
+   */
+    public getExecutions(applicationName: string, application: Application = null): IPromise<IExecution[]> {
+      const pipelines = Object.keys(this.executionFilterModel.asFilterModel.sortFilter.pipeline);
+      const statuses = Object.keys(pickBy(this.executionFilterModel.asFilterModel.sortFilter.status || {}, identity));
+      const limit = this.executionFilterModel.asFilterModel.sortFilter.count;
+      if (application && pipelines.length) {
+        return this.getConfigIdsFromFilterModel(application).then(pipelineConfigIds => {
+          return this.getFilteredExecutions(application.name, statuses, limit, pipelineConfigIds);
+        });
+      }
+      return this.getFilteredExecutions(applicationName, statuses, limit);
     }
 
     public getExecution(executionId: string): IPromise<IExecution> {
@@ -74,6 +93,18 @@ export class ExecutionService {
       });
     }
 
+    private getConfigIdsFromFilterModel(application: Application): IPromise<string[]> {
+      const pipelines = Object.keys(this.executionFilterModel.asFilterModel.sortFilter.pipeline);
+      application.pipelineConfigs.activate();
+      return application.pipelineConfigs.ready().then(() => {
+        const data = application.pipelineConfigs.data.concat(application.strategyConfigs.data);
+        return pipelines.map(p => {
+          const match = data.find((c: IPipeline) => c.name === p);
+          return match ? match.id : null;
+        }).filter(id => !!id);
+      });
+    }
+
     private cleanExecutionForDiffing(execution: IExecution): void {
       (execution.stages || []).forEach((stage: IExecutionStage) => this.removeInstances(stage));
       if (execution.trigger && execution.trigger.parentExecution) {
@@ -343,7 +374,7 @@ export class ExecutionService {
           application.executions.data.push(re);
         }
       });
-      if (updated) {
+      if (updated && !application.executions.reloadingForFilters) {
         application.executions.dataUpdated();
       }
     }
@@ -412,7 +443,7 @@ export class ExecutionService {
     }
 
     public getLastExecutionForApplicationByConfigId(appName: string, configId: string): IPromise<IExecution> {
-      return this.getFilteredExecutions(appName, {})
+      return this.getFilteredExecutions(appName, [], 1)
         .then((executions: IExecution[]) => {
           return executions.filter((execution) => {
             return execution.pipelineConfigId === configId;