Tidy up subscriptions to event emitters on navigation away from page

packages/composer-playground/src/app/editor/editor.component.spec.ts

@@ -22,6 +22,7 @@ import { ModelFile, Script } from 'composer-common';
 import * as sinon from 'sinon';
 import * as chai from 'chai';
 
+import 'rxjs/add/operator/takeWhile';
 import * as fileSaver from 'file-saver';
 
 let should = chai.should();
@@ -103,15 +104,19 @@ describe('EditorComponent', () => {
             mockBusinessNetworkService.OPEN_SAMPLE = false;
             mockInitializationService.initialize.returns(Promise.resolve());
             mockClientService.businessNetworkChanged$ = {
-                subscribe: (callback) => {
-                    let noError = true;
-                    callback(noError);
-                }
+                takeWhile: sinon.stub().returns({
+                    subscribe: (callback) => {
+                        let noError = true;
+                        callback(noError);
+                    }
+                })
             };
             mockClientService.fileNameChanged$ = {
-                subscribe: (callback) => {
-                    callback('new-name');
-                }
+                takeWhile: sinon.stub().returns({
+                    subscribe: (callback) => {
+                        callback('new-name');
+                    }
+                })
             };
             mockEditorFilesValidate = sinon.stub(component, 'editorFilesValidate').returns(true);
         });
@@ -125,6 +130,7 @@ describe('EditorComponent', () => {
             let mockUpdateFiles = sinon.stub(component, 'updateFiles');
             let mockSetFile = sinon.stub(component, 'setCurrentFile');
             let mockSetIntialFile = sinon.stub(component, 'setInitialFile');
+
             component.ngOnInit();
 
             tick();
@@ -175,10 +181,12 @@ describe('EditorComponent', () => {
 
         it('should set noError to false when notified', fakeAsync(() => {
             mockClientService.businessNetworkChanged$ = {
-                subscribe: (callback) => {
-                    let noError = false;
-                    callback(noError);
-                }
+                takeWhile: sinon.stub().returns({
+                    subscribe: (callback) => {
+                        let noError = false;
+                        callback(noError);
+                    }
+                })
             };
 
             mockEditorFilesValidate.returns(false);
@@ -196,10 +204,12 @@ describe('EditorComponent', () => {
 
         it('should set noError and dirty to be true when notified', fakeAsync(() => {
             mockClientService.businessNetworkChanged$ = {
-                subscribe: (callback) => {
-                    let noError = true;
-                    callback(noError);
-                }
+                takeWhile: sinon.stub().returns({
+                    subscribe: (callback) => {
+                        let noError = true;
+                        callback(noError);
+                    }
+                })
             };
 
             let mockUpdatePackage = sinon.stub(component, 'updatePackageInfo');
@@ -292,6 +302,17 @@ describe('EditorComponent', () => {
         }));
     });
 
+    describe('ngOnDestroy', () => {
+
+        it('should set this.alive to be false', () => {
+
+            component['alive'] = true;
+            component.ngOnDestroy();
+            component['alive'].should.equal(false);
+
+        });
+    });
+
     describe('updatePackageInfo', () => {
         it('should set the package info', () => {
             let mockMetaData = {
@@ -511,31 +532,65 @@ describe('EditorComponent', () => {
     });
 
     describe('addScriptFile', () => {
-        it('should add a script file', () => {
+        it('should create and add a script file', () => {
+            let mockUpdateFiles = sinon.stub(component, 'updateFiles');
+            let mockSetCurrentFile = sinon.stub(component, 'setCurrentFile');
+
+            mockScriptFile.getIdentifier.returns('script');
+            mockScriptFile.id = 'script';
+            component['files'] = [mockScriptFile];
+
+            let scriptManagerMock = {
+                createScript: sinon.stub().returns(mockScriptFile),
+                addScript: sinon.stub(),
+                getScripts: sinon.stub().returns([]),
+            };
+
+            mockClientService.getBusinessNetwork.returns({
+                getScriptManager: sinon.stub().returns(scriptManagerMock)
+            });
+
+            component.addScriptFile();
+
+            scriptManagerMock.createScript.should.have.been.calledWith('lib/script.js', 'JS', `/**
+  * New script file
+  */`);
+
+            scriptManagerMock.addScript.should.have.been.called;
+            mockUpdateFiles.should.have.been.called;
+
+            mockSetCurrentFile.should.have.been.calledWith({id: 'script'});
+            component['dirty'].should.equal(true);
+        });
+
+        it('should create and add a script file with an incremented name', () => {
             let mockUpdateFiles = sinon.stub(component, 'updateFiles');
             let mockSetCurrentFile = sinon.stub(component, 'setCurrentFile');
 
             let mockScript0 = sinon.createStubInstance(Script);
-            mockScript0.getIdentifier.returns('script');
+            mockScript0.getIdentifier.returns('lib/script.js');
             mockScript0.id = 'script';
 
             let mockScript1 = sinon.createStubInstance(Script);
-            mockScript1.getIdentifier.returns('script0');
+            mockScript1.getIdentifier.returns('lib/script0.js');
             mockScript1.id = 'script0';
 
             let mockScript2 = sinon.createStubInstance(Script);
-            mockScript2.getIdentifier.returns('script2');
-            mockScript2.id = 'script2';
+            mockScript2.getIdentifier.returns('lib/script1.js');
+            mockScript2.id = 'script1';
 
-            component['addScriptFileName'] = 'script';
-            component['files'] = [mockScript0, mockScript1, mockScript2];
+            let mockScript3 = sinon.createStubInstance(Script);
+            mockScript3.getIdentifier.returns('lib/script3.js');
+            mockScript3.id = 'script3';
+
+            component['files'] = [mockScript0, mockScript1, mockScript2, mockScript3];
 
             mockScriptFile.getIdentifier.returns('script');
 
             let scriptManagerMock = {
                 createScript: sinon.stub().returns(mockScriptFile),
                 addScript: sinon.stub(),
-                getScripts: sinon.stub().returns([mockScript0, mockScript1, mockScript2]),
+                getScripts: sinon.stub().returns([mockScript0, mockScript1, mockScript2, mockScript3]),
             };
 
             mockClientService.getBusinessNetwork.returns({
@@ -544,7 +599,7 @@ describe('EditorComponent', () => {
 
             component.addScriptFile();
 
-            scriptManagerMock.createScript.should.have.been.calledWith('script.js', 'JS', `/**
+            scriptManagerMock.createScript.should.have.been.calledWith('lib/script2.js', 'JS', `/**
   * New script file
   */`);
 

packages/composer-playground/src/app/editor/editor.component.ts

@@ -1,4 +1,4 @@
-import { Component, OnInit } from '@angular/core';
+import { Component, OnInit, OnDestroy } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
 
@@ -15,6 +15,7 @@ import { EditorService } from '../services/editor.service';
 
 import { ModelFile, ScriptManager, ModelManager } from 'composer-common';
 
+import 'rxjs/add/operator/takeWhile';
 import { saveAs } from 'file-saver';
 
 @Component({
@@ -25,7 +26,7 @@ import { saveAs } from 'file-saver';
     ]
 })
 
-export class EditorComponent implements OnInit {
+export class EditorComponent implements OnInit, OnDestroy {
 
     private files: any = [];
     private currentFile: any = null;
@@ -49,6 +50,8 @@ export class EditorComponent implements OnInit {
     private inputPackageName; // This is the input 'Name' before the BND is updated
     private inputPackageVersion; // This is the input 'Version' before the BND is updated
 
+    private alive: boolean = true; // used to prevent memory leaks on subscribers within ngOnInit/ngOnDestory
+
     constructor(private adminService: AdminService,
                 private clientService: ClientService,
                 private initializationService: InitializationService,
@@ -70,8 +73,9 @@ export class EditorComponent implements OnInit {
 
         return this.initializationService.initialize()
         .then(() => {
-            this.clientService.businessNetworkChanged$.subscribe((noError) => {
-                this.updateFiles();
+
+            this.clientService.businessNetworkChanged$.takeWhile(() => this.alive)
+            .subscribe((noError) => {
                 if (this.editorFilesValidate() && noError) {
                     this.noError = noError;
                     this.dirty = true;
@@ -80,7 +84,8 @@ export class EditorComponent implements OnInit {
                 }
             });
 
-            this.clientService.fileNameChanged$.subscribe((newName) => {
+            this.clientService.fileNameChanged$.takeWhile(() => this.alive)
+            .subscribe((newName) => {
                 if (this.currentFile !== null) {
                     this.updateFiles();
                     let index = this.files.findIndex((file) => file.id === newName);
@@ -99,6 +104,10 @@ export class EditorComponent implements OnInit {
         });
     }
 
+    ngOnDestroy() {
+       this.alive = false;
+    }
+
     updatePackageInfo() {
         this.deployedPackageName = this.clientService.getMetaData().getName(); // Set Name
         this.deployedPackageVersion = this.clientService.getMetaData().getVersion(); // Set Version