fixes for memory leaks and multiple subscriptions (#3456)

* fix content action memory leaks

* memory leak fixes for demo shell

demo-shell/src/app/components/process-service/process-attachments.component.ts

@@ -15,13 +15,14 @@
  * limitations under the License.
  */
 
-import { Component, Input, OnChanges, OnInit, ViewChild } from '@angular/core';
+import { Component, Input, OnChanges, OnInit, ViewChild, OnDestroy } from '@angular/core';
 import { ProcessInstance, ProcessService ,
     ProcessAttachmentListComponent, ProcessUploadService } from '@alfresco/adf-process-services';
 import { UploadService } from '@alfresco/adf-core';
 import { AlfrescoApiService } from '@alfresco/adf-core';
 import { AppConfigService } from '@alfresco/adf-core';
 import { PreviewService } from '../../services/preview.service';
+import { Subscription } from 'rxjs/Subscription';
 
 export function processUploadServiceFactory(api: AlfrescoApiService, config: AppConfigService) {
     return new ProcessUploadService(api, config);
@@ -40,7 +41,7 @@ export function processUploadServiceFactory(api: AlfrescoApiService, config: App
     ]
 })
 
-export class ProcessAttachmentsComponent implements OnInit, OnChanges {
+export class ProcessAttachmentsComponent implements OnInit, OnChanges, OnDestroy {
 
     @ViewChild('processAttachList')
     processAttachList: ProcessAttachmentListComponent;
@@ -50,6 +51,8 @@ export class ProcessAttachmentsComponent implements OnInit, OnChanges {
 
     processInstance: ProcessInstance;
 
+    private subscriptions: Subscription[] = [];
+
     constructor(
         private uploadService: UploadService,
         private processService: ProcessService,
@@ -57,7 +60,11 @@ export class ProcessAttachmentsComponent implements OnInit, OnChanges {
     ) {}
 
     ngOnInit() {
-        this.uploadService.fileUploadComplete.subscribe(value => this.onFileUploadComplete(value.data));
+        this.subscriptions.push(
+            this.uploadService.fileUploadComplete.subscribe(
+                value => this.onFileUploadComplete(value.data)
+            )
+        );
     }
 
     ngOnChanges() {
@@ -69,6 +76,11 @@ export class ProcessAttachmentsComponent implements OnInit, OnChanges {
         }
     }
 
+    ngOnDestroy() {
+        this.subscriptions.forEach(subscription => subscription.unsubscribe());
+        this.subscriptions = [];
+    }
+
     onFileUploadComplete(content: any) {
         this.processAttachList.add(content);
     }

demo-shell/src/app/components/process-service/process-service.component.ts

@@ -137,7 +137,6 @@ export class ProcessServiceComponent implements AfterViewInit, OnDestroy, OnInit
     taskFilter: FilterRepresentationModel;
     report: any;
     processFilter: UserProcessInstanceFilterRepresentation;
-    sub: Subscription;
     blobFile: any;
     flag = true;
 
@@ -151,6 +150,8 @@ export class ProcessServiceComponent implements AfterViewInit, OnDestroy, OnInit
         new DemoFieldValidator()
     ];
 
+    private subscriptions: Subscription[] = [];
+
     constructor(private elementRef: ElementRef,
                 private route: ActivatedRoute,
                 private router: Router,
@@ -172,27 +173,26 @@ export class ProcessServiceComponent implements AfterViewInit, OnDestroy, OnInit
         // Uncomment this line to map 'custom_stencil_01' to local editor component
         formRenderingService.setComponentTypeResolver('custom_stencil_01', () => CustomStencil01, true);
 
-        formService.formLoaded.subscribe((e: FormEvent) => {
+        this.subscriptions.push(
-            this.logService.log(`Form loaded: ${e.form.id}`);
+            formService.formLoaded.subscribe((e: FormEvent) => {
-        });
+                this.logService.log(`Form loaded: ${e.form.id}`);
-
+            }),
-        formService.formFieldValueChanged.subscribe((e: FormFieldEvent) => {
+            formService.formFieldValueChanged.subscribe((e: FormFieldEvent) => {
-            this.logService.log(`Field value changed. Form: ${e.form.id}, Field: ${e.field.id}, Value: ${e.field.value}`);
+                this.logService.log(`Field value changed. Form: ${e.form.id}, Field: ${e.field.id}, Value: ${e.field.value}`);
-        });
+            }),
-
+            this.preferenceService.select(UserPreferenceValues.PaginationSize).subscribe((pageSize) => {
-        this.preferenceService.select(UserPreferenceValues.PaginationSize).subscribe((pageSize) => {
+                this.paginationPageSize = pageSize;
-            this.paginationPageSize = pageSize;
+            }),
-        });
+            formService.validateDynamicTableRow.subscribe(
-
+                (validateDynamicTableRowEvent: ValidateDynamicTableRowEvent) => {
-        formService.validateDynamicTableRow.subscribe(
+                    const row: DynamicTableRow = validateDynamicTableRowEvent.row;
-            (validateDynamicTableRowEvent: ValidateDynamicTableRowEvent) => {
+                    if (row && row.value && row.value.name === 'admin') {
-                const row: DynamicTableRow = validateDynamicTableRowEvent.row;
+                        validateDynamicTableRowEvent.summary.isValid = false;
-                if (row && row.value && row.value.name === 'admin') {
+                        validateDynamicTableRowEvent.summary.message = 'Sorry, wrong value. You cannot use "admin".';
-                    validateDynamicTableRowEvent.summary.isValid = false;
+                        validateDynamicTableRowEvent.preventDefault();
-                    validateDynamicTableRowEvent.summary.message = 'Sorry, wrong value. You cannot use "admin".';
+                    }
-                    validateDynamicTableRowEvent.preventDefault();
                 }
-            }
+            )
         );
 
         // Uncomment this block to see form event handling in action
@@ -208,7 +208,7 @@ export class ProcessServiceComponent implements AfterViewInit, OnDestroy, OnInit
         if (this.router.url.includes('processes')) {
             this.activeTab = this.tabs.processes;
         }
-        this.sub = this.route.params.subscribe(params => {
+        this.route.params.subscribe(params => {
             const applicationId = params['appId'];
 
             this.filterSelected = params['filterId'] ? { id: +params['filterId'] } : { index: 0 };
@@ -227,7 +227,8 @@ export class ProcessServiceComponent implements AfterViewInit, OnDestroy, OnInit
     }
 
     ngOnDestroy() {
-        this.sub.unsubscribe();
+        this.subscriptions.forEach(subscription => subscription.unsubscribe());
+        this.subscriptions = [];
     }
 
     onTaskFilterClick(filter: FilterRepresentationModel): void {

demo-shell/src/app/components/process-service/task-attachments.component.ts

@@ -15,10 +15,11 @@
  * limitations under the License.
  */
 
-import { Component, Input, OnChanges, OnInit, ViewChild } from '@angular/core';
+import { Component, Input, OnChanges, OnInit, ViewChild, OnDestroy } from '@angular/core';
 import { TaskListService, TaskAttachmentListComponent, TaskDetailsModel, TaskUploadService } from '@alfresco/adf-process-services';
 import { UploadService, AlfrescoApiService, AppConfigService } from '@alfresco/adf-core';
 import { PreviewService } from '../../services/preview.service';
+import { Subscription } from 'rxjs/Subscription';
 
 export function taskUploadServiceFactory(api: AlfrescoApiService, config: AppConfigService) {
     return new TaskUploadService(api, config);
@@ -37,7 +38,7 @@ export function taskUploadServiceFactory(api: AlfrescoApiService, config: AppCon
     ]
 })
 
-export class TaskAttachmentsComponent implements OnInit, OnChanges {
+export class TaskAttachmentsComponent implements OnInit, OnChanges, OnDestroy {
 
     @ViewChild('taskAttachList')
     taskAttachList: TaskAttachmentListComponent;
@@ -47,13 +48,19 @@ export class TaskAttachmentsComponent implements OnInit, OnChanges {
 
     taskDetails: any;
 
+    private subscriptions: Subscription[] = [];
+
     constructor(
         private uploadService: UploadService,
         private activitiTaskList: TaskListService,
         private preview: PreviewService) {}
 
     ngOnInit() {
-        this.uploadService.fileUploadComplete.subscribe(value => this.onFileUploadComplete(value.data));
+        this.subscriptions.push(
+            this.uploadService.fileUploadComplete.subscribe(
+                value => this.onFileUploadComplete(value.data)
+            )
+        );
     }
 
     ngOnChanges() {
@@ -65,6 +72,11 @@ export class TaskAttachmentsComponent implements OnInit, OnChanges {
         }
     }
 
+    ngOnDestroy() {
+        this.subscriptions.forEach(subscription => subscription.unsubscribe());
+        this.subscriptions = [];
+    }
+
     onFileUploadComplete(content: any) {
         this.taskAttachList.add(content);
     }

lib/content-services/document-list/components/content-action/content-action.component.ts

@@ -17,13 +17,14 @@
 
  /* tslint:disable:component-selector  */
 
-import { Component, EventEmitter, Input, OnInit, Output, OnChanges, SimpleChanges } from '@angular/core';
+import { Component, EventEmitter, Input, OnInit, Output, OnChanges, SimpleChanges, OnDestroy } from '@angular/core';
 
 import { ContentActionHandler } from '../../models/content-action.model';
 import { DocumentActionsService } from '../../services/document-actions.service';
 import { FolderActionsService } from '../../services/folder-actions.service';
 import { ContentActionModel, ContentActionTarget } from './../../models/content-action.model';
 import { ContentActionListComponent } from './content-action-list.component';
+import { Subscription } from 'rxjs/Subscription';
 
 @Component({
     selector: 'content-action',
@@ -33,7 +34,7 @@ import { ContentActionListComponent } from './content-action-list.component';
         FolderActionsService
     ]
 })
-export class ContentActionComponent implements OnInit, OnChanges {
+export class ContentActionComponent implements OnInit, OnChanges, OnDestroy {
 
     /** The title of the action as shown in the menu. */
     @Input()
@@ -51,7 +52,7 @@ export class ContentActionComponent implements OnInit, OnChanges {
     @Input()
     handler: string;
 
-    /** Type of item that the action appies to. Can be "document" or "folder" */
+    /** Type of item that the action applies to. Can be "document" or "folder" */
     @Input()
     target: string = ContentActionTarget.All;
 
@@ -90,6 +91,8 @@ export class ContentActionComponent implements OnInit, OnChanges {
     documentActionModel: ContentActionModel;
     folderActionModel: ContentActionModel;
 
+    private subscriptions: Subscription[] = [];
+
     constructor(
         private list: ContentActionListComponent,
         private documentActions: DocumentActionsService,
@@ -116,6 +119,14 @@ export class ContentActionComponent implements OnInit, OnChanges {
         }
     }
 
+    ngOnDestroy() {
+        this.subscriptions.forEach(subscription => subscription.unsubscribe());
+        this.subscriptions = [];
+
+        this.documentActionModel = null;
+        this.folderActionModel = null;
+    }
+
     register(model: ContentActionModel): boolean {
         if (this.list) {
             return this.list.registerAction(model);
@@ -149,40 +160,40 @@ export class ContentActionComponent implements OnInit, OnChanges {
 
     getSystemHandler(target: string, name: string): ContentActionHandler {
         if (target) {
-            let ltarget = target.toLowerCase();
+            target = target.toLowerCase();
 
-            if (ltarget === ContentActionTarget.Document) {
+            if (target === ContentActionTarget.Document) {
                 if (this.documentActions) {
-                    this.documentActions.permissionEvent.subscribe((permission) => {
+                    this.subscriptions.push(
-                        this.permissionEvent.emit(permission);
+                        this.documentActions.permissionEvent.subscribe(permission => {
-                    });
+                            this.permissionEvent.emit(permission);
-
+                        }),
-                    this.documentActions.error.subscribe((errors) => {
+                        this.documentActions.error.subscribe(errors => {
-                        this.error.emit(errors);
+                            this.error.emit(errors);
-                    });
+                        }),
-
+                        this.documentActions.success.subscribe(message => {
-                    this.documentActions.success.subscribe((message) => {
+                            this.success.emit(message);
-                        this.success.emit(message);
+                        })
-                    });
+                    );
 
                     return this.documentActions.getHandler(name);
                 }
                 return null;
             }
 
-            if (ltarget === ContentActionTarget.Folder) {
+            if (target === ContentActionTarget.Folder) {
                 if (this.folderActions) {
-                    this.folderActions.permissionEvent.subscribe((permission) => {
+                    this.subscriptions.push(
-                        this.permissionEvent.emit(permission);
+                        this.folderActions.permissionEvent.subscribe(permission => {
-                    });
+                            this.permissionEvent.emit(permission);
-
+                        }),
-                    this.folderActions.error.subscribe((errors) => {
+                        this.folderActions.error.subscribe(errors => {
-                        this.error.emit(errors);
+                            this.error.emit(errors);
-                    });
+                        }),
-
+                        this.folderActions.success.subscribe(message => {
-                    this.folderActions.success.subscribe((message) => {
+                            this.success.emit(message);
-                        this.success.emit(message);
+                        })
-                    });
+                    );
 
                     return this.folderActions.getHandler(name);
                 }

lib/core/viewer/components/viewer.component.ts

@@ -252,9 +252,9 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy {
     }
 
     ngOnInit() {
-        this.subscriptions = this.subscriptions.concat([
+        this.subscriptions.push(
             this.apiService.nodeUpdated.subscribe(node => this.onNodeUpdated(node))
-        ]);
+        );
     }
 
     ngOnDestroy() {