gh-610 Make sure spinner is consistently applied

* Add cleanup for all Subscriptions inside Components

gh-610 Code review changes

Author: ghillert

ui/src/app/about/about.service.spec.ts

@@ -3,6 +3,7 @@ import { Observable } from 'rxjs/Observable';
 import { ErrorHandler } from '../shared/model/error-handler';
 import { Response, ResponseOptions } from '@angular/http';
 import { SharedAboutService } from '../shared/services/shared-about.service';
+import { BusyService } from '../shared/services/busy.service';
 
 describe('AboutService', () => {
 
@@ -106,9 +107,8 @@ describe('AboutService', () => {
       body: JSON.stringify(jsonData)
     }));
     this.mockHttp.get.and.returnValue(Observable.of(mockResponse));
-
     const errorHandler = new ErrorHandler();
-    this.sharedAboutService = new SharedAboutService(this.mockHttp, errorHandler);
+    this.sharedAboutService = new SharedAboutService(new BusyService(), this.mockHttp, errorHandler);
     this.aboutService = new AboutService(this.sharedAboutService, this.mockHttp, errorHandler);
   });
 

ui/src/app/analytics/dashboard/dashboard.component.html

@@ -1,4 +1,3 @@
-<div [ngBusy]="busy"></div>
 <button type="button" (click)="resetDashboard()"
         class="btn btn-default">Reset <span class="glyphicon glyphicon-trash"></span></button>
 <form #dashboardForm="ngForm" name="dashboardForm" role="form" novalidate>

ui/src/app/analytics/dashboard/dashboard.component.ts

@@ -1,13 +1,13 @@
 import { Component, OnInit, OnDestroy } from '@angular/core';
-import { Subscription } from 'rxjs/Subscription';
-
 import { ToastyService } from 'ng2-toasty';
 import { AnalyticsService } from '../analytics.service';
 
 import {
   AggregateCounter, AggregateCounterResolutionType, BaseCounter,
   DashboardItem, FieldValueCounterValue, MetricType
 } from '../model';
+import { takeUntil } from 'rxjs/operators';
+import { Subject } from 'rxjs/Subject';
 
 /**
  * The dashboard component provides
@@ -20,8 +20,8 @@ import {
 })
 export class DashboardComponent implements OnInit, OnDestroy {
 
-  busy: Subscription;
-  busyCounters: Subscription;
+  private ngUnsubscribe$: Subject<any> = new Subject();
+
   /**
    * Returns the {@link DashboardItem}s from the
    * {@link AnalyticsService}.
@@ -54,10 +54,13 @@ export class DashboardComponent implements OnInit, OnDestroy {
 
   /**
    * When the component is destroyed, make sure all pollers are
-   * stopped also.
+   * stopped also. Will also cleanup any {@link Subscription}s to prevent
+   * memory leaks. 
    */
   ngOnDestroy() {
     this.analyticsService.stopAllDashboardPollers();
+    this.ngUnsubscribe$.next();
+    this.ngUnsubscribe$.complete();
   }
 
   /**
@@ -115,6 +118,7 @@ export class DashboardComponent implements OnInit, OnDestroy {
     dashBoardItem.counter = undefined;
 
     dashBoardItem.countersIsLoading = this.analyticsService.getCountersForMetricType(metricType)
+      .pipe(takeUntil(this.ngUnsubscribe$))
       .subscribe(result => {
         dashBoardItem.counters = result.items;
       },

ui/src/app/app.component.html

@@ -36,6 +36,7 @@
 <div class="container dataflow-main-container">
   <div class='row'>
     <div class='col-sm-24'><ng2-toasty></ng2-toasty>
+      <ng-template [ngBusy]="{busy: busy, minDuration: 500}"></ng-template>
       <router-outlet></router-outlet>
     </div>
   </div>

ui/src/app/app.component.ts

@@ -8,6 +8,7 @@ import { SecurityInfo } from './shared/model/about/security-info.model';
 import { SharedAboutService } from './shared/services/shared-about.service';
 import { Observable } from 'rxjs/Observable';
 import { AboutInfo } from './shared/model/about/about-info.model';
+import { BusyService } from './shared/services/busy.service';
 
 @Component({
   selector: 'app-root',
@@ -19,10 +20,12 @@ export class AppComponent implements DoCheck, OnInit {
   public dataflowVersionInfo$: Observable<AboutInfo>;
 
   public isCollapsed = true;
+  public busy: any = [];
 
   constructor(private toastyConfig: ToastyConfig,
       private renderer: Renderer2,
       private authService: AuthService,
+      private busyService: BusyService,
       private sharedAboutService: SharedAboutService) {
     this.toastyConfig.theme = 'bootstrap';
     this.toastyConfig.limit = 5;
@@ -46,6 +49,26 @@ export class AppComponent implements DoCheck, OnInit {
       this.updateToasty();
     });
     this.updateToasty();
+
+    this.busyService.busyObjects$.forEach(busyObject => {
+      if (busyObject) {
+        while(busyObject.length > 0) {
+          /*
+           * Unfortunately, Angular2 Busy does not support
+           * "Overlapping Subscriptions" and does not work
+           * with mutable arrays. Ideally, good Spinner solution
+           * would be able to accept a BehaviorSubject<boolean> as input,
+           * so that we could manage the on/off state of the spinner on
+           * our end.
+           *
+           * see: https://github.com/devyumao/angular2-busy/issues/77
+           */
+          this.busy = [];
+          this.busy.push(busyObject.pop());
+        }
+      }
+    });
+
   }
 
   private updateToasty() {

ui/src/app/apps/app-details/app-details.component.html

@@ -1,4 +1,4 @@
-<h1 [ngBusy]="busy">Application Details</h1>
+<h1>Application Details</h1>
 
 <table class="table table-striped table-hover table-fixed" *ngIf="detailedAppRegistration">
   <tbody>

ui/src/app/apps/app-details/app-details.component.ts

@@ -1,4 +1,4 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnInit, OnDestroy } from '@angular/core';
 import { Router, ActivatedRoute, Params } from '@angular/router';
 
 import { AppsService } from '../apps.service';
@@ -8,6 +8,9 @@ import { ApplicationType, DetailedAppRegistration } from '../../shared/model';
 
 import { Subscription } from 'rxjs/Subscription';
 import 'rxjs/add/observable/of';
+import { Subject } from 'rxjs/Subject';
+import { BusyService } from '../../shared/services/busy.service';
+import { takeUntil } from 'rxjs/operators';
 
 /**
  * Provides details for an App Registration
@@ -17,13 +20,14 @@ import 'rxjs/add/observable/of';
 @Component({
   templateUrl: './app-details.component.html'
 })
-export class AppDetailsComponent implements OnInit {
+export class AppDetailsComponent implements OnInit, OnDestroy {
 
-  public detailedAppRegistration: DetailedAppRegistration;
+  private ngUnsubscribe$: Subject<any> = new Subject();
 
-  busy: Subscription;
+  public detailedAppRegistration: DetailedAppRegistration;
 
   constructor(
+    private busyService: BusyService,
     private route: ActivatedRoute,
     private appsService: AppsService,
     private toastyService: ToastyService,
@@ -39,15 +43,29 @@ export class AppDetailsComponent implements OnInit {
       const appType: ApplicationType = params['appType'] as ApplicationType;
 
       console.log(`Retrieving app registration details for ${appName} (${appType}).`);
-      this.busy = this.appsService.getAppInfo(appType, appName).subscribe(data => {
+      const busy = this.appsService.getAppInfo(appType, appName)
+      .pipe(takeUntil(this.ngUnsubscribe$))
+      .subscribe(data => {
         this.detailedAppRegistration = data;
       },
       error => {
         this.toastyService.error(error);
       });
+      setTimeout(()=>{
+        this.busyService.addSubscription(busy);
+      });
     });
   }
 
+  /**
+   * Will cleanup any {@link Subscription}s to prevent
+   * memory leaks.  
+   */
+  ngOnDestroy() {
+    this.ngUnsubscribe$.next();
+    this.ngUnsubscribe$.complete();
+  }
+
   goBack() {
     console.log('Back to apps page ...');
     this.router.navigate(['apps']);

ui/src/app/apps/apps-bulk-import.component.html

@@ -1,4 +1,4 @@
-<h1 [ngBusy]="busy">Bulk Import Applications</h1>
+<h1>Bulk Import Applications</h1>
 <p>
     Import and register applications in bulk. Simply provide a URI that points to
     the location of the <strong>properties file</strong> where the keys are formatted as

ui/src/app/apps/apps-bulk-import.component.ts

@@ -1,31 +1,36 @@
-import { Component, OnInit, OnChanges, ViewChild, ElementRef } from '@angular/core';
+import { Component, OnInit, OnChanges, ViewChild, ElementRef, OnDestroy } from '@angular/core';
 import { Subscription} from 'rxjs/Subscription';
 import { AppsService } from './apps.service';
 import { ToastyService } from 'ng2-toasty';
 import { Router } from '@angular/router';
 
 import { AppRegistrationImport } from './model/app-registration-import';
 import { PopoverDirective } from 'ngx-bootstrap/popover';
+import { Subject } from 'rxjs/Subject';
+import { BusyService } from '../shared/services/busy.service';
+import { takeUntil } from 'rxjs/operators';
 
 @Component({
   selector: 'app-apps',
   templateUrl: './apps-bulk-import.component.html'
 })
-export class AppsBulkImportComponent implements OnInit, OnChanges {
+export class AppsBulkImportComponent implements OnInit, OnChanges, OnDestroy {
+
+  private ngUnsubscribe$: Subject<any> = new Subject();
 
   @ViewChild('childPopover')
   public childPopover: PopoverDirective;
 
   public model = new AppRegistrationImport(false, [], '');
 
   apps: any;
-  busy: Subscription;
 
   contents: any;
   uriPattern = '^([a-z0-9-]+:\/\/)([\\w\\.:-]+)(\/[\\w\\.:-]+)*$';
 
   constructor(
     private appsService: AppsService,
+    private busyService: BusyService,
     private toastyService: ToastyService,
     private router: Router,
     private elementRef: ElementRef) { }
@@ -35,6 +40,15 @@ export class AppsBulkImportComponent implements OnInit, OnChanges {
     console.log(this.appsService.appRegistrations);
   }
 
+  /**
+   * Will cleanup any {@link Subscription}s to prevent
+   * memory leaks.  
+   */
+  ngOnDestroy() {
+    this.ngUnsubscribe$.next();
+    this.ngUnsubscribe$.complete();
+  }
+
   goBack() {
     console.log('Back to apps page ...');
     this.router.navigate(['apps']);
@@ -86,17 +100,21 @@ export class AppsBulkImportComponent implements OnInit, OnChanges {
           console.log('Importing apps using textarea values:\n' + this.model.appsProperties + ' (force: ' + this.model.force + ')');
       }
 
-      this.busy = this.appsService.bulkImportApps(this.model).subscribe(
+      const busy = this.appsService.bulkImportApps(this.model).subscribe(
         data => {
           console.log(data);
           this.toastyService.success('Apps Imported.');
-          this.busy = this.appsService.getApps(true).subscribe(
+          this.busyService.addSubscription(
+          this.appsService.getApps(true)
+          .pipe(takeUntil(this.ngUnsubscribe$))
+          .subscribe(
             appRegistrations => {
               console.log('Back to about page ...');
               this.router.navigate(['apps']);
             }
-          );
+          ));
         }
       );
+      this.busyService.addSubscription(busy);
   }
 }

ui/src/app/apps/apps-register/apps-register.component.html

@@ -1,4 +1,4 @@
-<h1 [ngBusy]="busy">Register Applications</h1>
+<h1>Register Applications</h1>
 <p>
     Register one or more applications by entering a <strong>Name</strong>,
     <strong>Type</strong> and <strong>App URI</strong> of the application Jar.