From 504500de5092d45dfafb8d85d69b2edc4a081ac1 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 7 Jul 2017 21:17:19 +0300 Subject: [PATCH] fix(aio): activate ServiceWorker updates asap (#17699) Previouly, whenever a new ServiceWorker update was detected the user was prompted to update (with a notification). This turned out to be more distracting than helpful. Also, one would get notifications on all open browser tabs/windows and had to manually reload each one in order for the whole content (including the app) to be updated. This commit changes the update strategy as follows: - Whenever a new update is detected, it is immediately activated (and all tabs/windows will be notified). - Once an update is activated (regardless of whether the activation was initiated by the current tab/window or not), a flag will be set to do a "full page navigation" the next time the user navigates to a document. Benefits: - All tabs/windows are updated asap. - The updates are applied authomatically, without the user's needing to do anything. - The updates are applied in a way that: a. Ensures that the app and content versions are always compatible. b. Does not distract the user from their usual workflow. NOTE: The "full page navigation" may cause a flash (while the page is loading from scratch), but this is expected to be minimal, since at that point almost all necessary resources are cached by and served from the ServiceWorker. Fixes #17539 --- aio/src/app/app.component.spec.ts | 10 - aio/src/app/app.component.ts | 4 - aio/src/app/embedded/embedded.module.ts | 3 +- aio/src/app/shared/location.service.spec.ts | 26 ++- aio/src/app/shared/location.service.ts | 13 +- aio/src/app/sw-updates/global.value.spec.ts | 18 -- aio/src/app/sw-updates/global.value.ts | 8 - .../sw-update-notifications.service.spec.ts | 195 ---------------- .../sw-update-notifications.service.ts | 94 -------- aio/src/app/sw-updates/sw-updates.module.ts | 6 - .../app/sw-updates/sw-updates.service.spec.ts | 221 +++++++----------- aio/src/app/sw-updates/sw-updates.service.ts | 68 +++--- .../sw-update-notifications.service.ts | 4 - aio/src/testing/sw-updates.service.ts | 20 -- 14 files changed, 153 insertions(+), 537 deletions(-) delete mode 100644 aio/src/app/sw-updates/global.value.spec.ts delete mode 100644 aio/src/app/sw-updates/global.value.ts delete mode 100644 aio/src/app/sw-updates/sw-update-notifications.service.spec.ts delete mode 100644 aio/src/app/sw-updates/sw-update-notifications.service.ts delete mode 100644 aio/src/testing/sw-update-notifications.service.ts delete mode 100644 aio/src/testing/sw-updates.service.ts diff --git a/aio/src/app/app.component.spec.ts b/aio/src/app/app.component.spec.ts index 6f45d2cc5b..cc2a29b740 100644 --- a/aio/src/app/app.component.spec.ts +++ b/aio/src/app/app.component.spec.ts @@ -18,14 +18,12 @@ import { Logger } from 'app/shared/logger.service'; import { MockLocationService } from 'testing/location.service'; import { MockLogger } from 'testing/logger.service'; import { MockSearchService } from 'testing/search.service'; -import { MockSwUpdateNotificationsService } from 'testing/sw-update-notifications.service'; import { NavigationNode } from 'app/navigation/navigation.service'; import { ScrollService } from 'app/shared/scroll.service'; import { SearchBoxComponent } from 'app/search/search-box/search-box.component'; import { SearchResultsComponent } from 'app/search/search-results/search-results.component'; import { SearchService } from 'app/search/search.service'; import { SelectComponent, Option } from 'app/shared/select/select.component'; -import { SwUpdateNotificationsService } from 'app/sw-updates/sw-update-notifications.service'; import { TocComponent } from 'app/embedded/toc/toc.component'; import { TocItem, TocService } from 'app/shared/toc.service'; @@ -68,13 +66,6 @@ describe('AppComponent', () => { expect(component).toBeDefined(); }); - describe('ServiceWorker update notifications', () => { - it('should be enabled', () => { - const swUpdateNotifications = TestBed.get(SwUpdateNotificationsService) as SwUpdateNotificationsService; - expect(swUpdateNotifications.enable).toHaveBeenCalled(); - }); - }); - describe('hasFloatingToc', () => { it('should initially be true', () => { const fixture2 = TestBed.createComponent(AppComponent); @@ -904,7 +895,6 @@ function createTestingModule(initialUrl: string) { { provide: LocationService, useFactory: () => new MockLocationService(initialUrl) }, { provide: Logger, useClass: MockLogger }, { provide: SearchService, useClass: MockSearchService }, - { provide: SwUpdateNotificationsService, useClass: MockSwUpdateNotificationsService }, ] }); } diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts index 5dbecf8aa4..b8ce7922a4 100644 --- a/aio/src/app/app.component.ts +++ b/aio/src/app/app.component.ts @@ -11,7 +11,6 @@ import { ScrollService } from 'app/shared/scroll.service'; import { SearchResultsComponent } from 'app/search/search-results/search-results.component'; import { SearchBoxComponent } from 'app/search/search-box/search-box.component'; import { SearchService } from 'app/search/search.service'; -import { SwUpdateNotificationsService } from 'app/sw-updates/sw-update-notifications.service'; import { TocService } from 'app/shared/toc.service'; import { BehaviorSubject } from 'rxjs/BehaviorSubject'; @@ -106,7 +105,6 @@ export class AppComponent implements OnInit { private navigationService: NavigationService, private scrollService: ScrollService, private searchService: SearchService, - private swUpdateNotifications: SwUpdateNotificationsService, private tocService: TocService ) { } @@ -177,8 +175,6 @@ export class AppComponent implements OnInit { this.navigationService.versionInfo.subscribe( vi => this.versionInfo = vi ); - this.swUpdateNotifications.enable(); - const hasNonEmptyToc = this.tocService.tocList.map(tocList => tocList.length > 0); combineLatest(hasNonEmptyToc, this.showFloatingToc) .subscribe(([hasToc, showFloatingToc]) => this.hasFloatingToc = hasToc && showFloatingToc); diff --git a/aio/src/app/embedded/embedded.module.ts b/aio/src/app/embedded/embedded.module.ts index 5ec0f7d3e7..ac1426e33e 100644 --- a/aio/src/app/embedded/embedded.module.ts +++ b/aio/src/app/embedded/embedded.module.ts @@ -9,7 +9,7 @@ import { PrettyPrinter } from './code/pretty-printer.service'; // It is not enough just to import them inside the AppModule // Reusable components (used inside embedded components) -import { MdIconModule, MdTabsModule } from '@angular/material'; +import { MdIconModule, MdSnackBarModule, MdTabsModule } from '@angular/material'; import { CodeComponent } from './code/code.component'; import { SharedModule } from 'app/shared/shared.module'; @@ -42,6 +42,7 @@ export class EmbeddedComponents { imports: [ CommonModule, MdIconModule, + MdSnackBarModule, MdTabsModule, SharedModule ], diff --git a/aio/src/app/shared/location.service.spec.ts b/aio/src/app/shared/location.service.spec.ts index 55a78a92c6..8781721819 100644 --- a/aio/src/app/shared/location.service.spec.ts +++ b/aio/src/app/shared/location.service.spec.ts @@ -1,14 +1,17 @@ import { ReflectiveInjector } from '@angular/core'; import { Location, LocationStrategy, PlatformLocation } from '@angular/common'; import { MockLocationStrategy } from '@angular/common/testing'; +import { Subject } from 'rxjs/Subject'; import { GaService } from 'app/shared/ga.service'; +import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; import { LocationService } from './location.service'; describe('LocationService', () => { let injector: ReflectiveInjector; let location: MockLocationStrategy; let service: LocationService; + let swUpdates: MockSwUpdatesService; beforeEach(() => { injector = ReflectiveInjector.resolveAndCreate([ @@ -16,11 +19,13 @@ describe('LocationService', () => { Location, { provide: GaService, useClass: TestGaService }, { provide: LocationStrategy, useClass: MockLocationStrategy }, - { provide: PlatformLocation, useClass: MockPlatformLocation } + { provide: PlatformLocation, useClass: MockPlatformLocation }, + { provide: SwUpdatesService, useClass: MockSwUpdatesService } ]); location = injector.get(LocationStrategy); service = injector.get(LocationService); + swUpdates = injector.get(SwUpdatesService); }); describe('currentUrl', () => { @@ -289,6 +294,21 @@ describe('LocationService', () => { expect(goExternalSpy).toHaveBeenCalledWith(externalUrl); }); + it('should do a "full page navigation" if a ServiceWorker update has been activated', () => { + const goExternalSpy = spyOn(service, 'goExternal'); + + // Internal URL - No ServiceWorker update + service.go('some-internal-url'); + expect(goExternalSpy).not.toHaveBeenCalled(); + expect(location.path(true)).toEqual('some-internal-url'); + + // Internal URL - ServiceWorker update + swUpdates.updateActivated.next('foo'); + service.go('other-internal-url'); + expect(goExternalSpy).toHaveBeenCalledWith('other-internal-url'); + expect(location.path(true)).toEqual('some-internal-url'); + }); + it('should not update currentUrl for external url that starts with "http"', () => { let localUrl: string; spyOn(service, 'goExternal'); @@ -588,6 +608,10 @@ class MockPlatformLocation { replaceState = jasmine.createSpy('PlatformLocation.replaceState'); } +class MockSwUpdatesService { + updateActivated = new Subject(); +} + class TestGaService { locationChanged = jasmine.createSpy('locationChanged'); } diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts index 5f96ee2bc9..00d042ad70 100644 --- a/aio/src/app/shared/location.service.ts +++ b/aio/src/app/shared/location.service.ts @@ -6,37 +6,44 @@ import { ReplaySubject } from 'rxjs/ReplaySubject'; import 'rxjs/add/operator/do'; import { GaService } from 'app/shared/ga.service'; +import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; @Injectable() export class LocationService { private readonly urlParser = document.createElement('a'); private urlSubject = new ReplaySubject(1); + private swUpdateActivated = false; + currentUrl = this.urlSubject .map(url => this.stripSlashes(url)); currentPath = this.currentUrl .map(url => url.match(/[^?#]*/)[0]) // strip query and hash - .do(url => this.gaService.locationChanged(url)); + .do(path => this.gaService.locationChanged(path)); constructor( private gaService: GaService, private location: Location, - private platformLocation: PlatformLocation) { + private platformLocation: PlatformLocation, + swUpdates: SwUpdatesService) { this.urlSubject.next(location.path(true)); this.location.subscribe(state => { return this.urlSubject.next(state.url); }); + + swUpdates.updateActivated.subscribe(() => this.swUpdateActivated = true); } // TODO?: ignore if url-without-hash-or-search matches current location? go(url: string) { if (!url) { return; } url = this.stripSlashes(url); - if (/^http/.test(url)) { + if (/^http/.test(url) || this.swUpdateActivated) { // Has http protocol so leave the site + // (or do a "full page navigation" if a ServiceWorker update has been activated) this.goExternal(url); } else { this.location.go(url); diff --git a/aio/src/app/sw-updates/global.value.spec.ts b/aio/src/app/sw-updates/global.value.spec.ts deleted file mode 100644 index ff43fd55ef..0000000000 --- a/aio/src/app/sw-updates/global.value.spec.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { ReflectiveInjector } from '@angular/core'; - -import { Global, globalProvider } from './global.value'; - - -describe('Global', () => { - let value: any; - - beforeEach(() => { - const injector = ReflectiveInjector.resolveAndCreate([globalProvider]); - value = injector.get(Global); - }); - - - it('should be `window`', () => { - expect(value).toBe(window); - }); -}); diff --git a/aio/src/app/sw-updates/global.value.ts b/aio/src/app/sw-updates/global.value.ts deleted file mode 100644 index 91d1fe04a1..0000000000 --- a/aio/src/app/sw-updates/global.value.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { InjectionToken } from '@angular/core'; - - -export const Global = new InjectionToken('global'); -export const globalProvider = { provide: Global, useFactory: globalFactory }; -export function globalFactory() { - return window; -} diff --git a/aio/src/app/sw-updates/sw-update-notifications.service.spec.ts b/aio/src/app/sw-updates/sw-update-notifications.service.spec.ts deleted file mode 100644 index 58c1671aa8..0000000000 --- a/aio/src/app/sw-updates/sw-update-notifications.service.spec.ts +++ /dev/null @@ -1,195 +0,0 @@ -import { ReflectiveInjector } from '@angular/core'; -import { fakeAsync, flushMicrotasks } from '@angular/core/testing'; -import { MdSnackBar, MdSnackBarConfig } from '@angular/material'; -import { Observable } from 'rxjs/Observable'; -import { Subject } from 'rxjs/Subject'; - -import { MockSwUpdatesService } from 'testing/sw-updates.service'; -import { Global } from './global.value'; -import { SwUpdateNotificationsService } from './sw-update-notifications.service'; -import { SwUpdatesService } from './sw-updates.service'; - - -describe('SwUpdateNotificationsService', () => { - const UPDATE_AVAILABLE_MESSAGE = 'New update for angular.io is available.'; - const UPDATE_FAILED_MESSAGE = 'Update activation failed :('; - let injector: ReflectiveInjector; - let service: SwUpdateNotificationsService; - let swUpdates: MockSwUpdatesService; - let snackBar: MockMdSnackBar; - - // Helpers - const activateUpdate = success => { - swUpdates.$$isUpdateAvailableSubj.next(true); - snackBar.$$lastRef.$$onActionSubj.next(); - swUpdates.$$activateUpdateSubj.next(success); - - flushMicrotasks(); - }; - - beforeEach(() => { - injector = ReflectiveInjector.resolveAndCreate([ - { provide: Global, useClass: MockGlobal }, - { provide: MdSnackBar, useClass: MockMdSnackBar }, - { provide: SwUpdatesService, useClass: MockSwUpdatesService }, - SwUpdateNotificationsService - ]); - service = injector.get(SwUpdateNotificationsService); - swUpdates = injector.get(SwUpdatesService); - snackBar = injector.get(MdSnackBar); - }); - - - it('should create', () => { - expect(service).toBeTruthy(); - }); - - it('should not notify about available updates before being enabled', () => { - swUpdates.$$isUpdateAvailableSubj.next(true); - expect(snackBar.$$lastRef).toBeUndefined(); - }); - - describe('when enabled', () => { - beforeEach(() => service.enable()); - - - it('should not re-subscribe to updates if already enabled', () => { - spyOn(snackBar, 'open').and.callThrough(); - - service.enable(); - swUpdates.$$isUpdateAvailableSubj.next(true); - - expect(snackBar.open).toHaveBeenCalledTimes(1); - }); - - it('should notify when updates are available', () => { - expect(snackBar.$$lastRef).toBeUndefined(); - - swUpdates.$$isUpdateAvailableSubj.next(true); - - expect(snackBar.$$lastRef.$$message).toBe(UPDATE_AVAILABLE_MESSAGE); - expect(snackBar.$$lastRef.$$action).toBe('Update now'); - expect(snackBar.$$lastRef.$$config.duration).toBeUndefined(); - }); - - it('should not notify when updates are not available', () => { - swUpdates.$$isUpdateAvailableSubj.next(false); - expect(snackBar.$$lastRef).toBeUndefined(); - }); - - it('should activate the update when clicking on `Update now`', () => { - spyOn(swUpdates, 'activateUpdate').and.callThrough(); - - swUpdates.$$isUpdateAvailableSubj.next(true); - expect(swUpdates.activateUpdate).not.toHaveBeenCalled(); - - snackBar.$$lastRef.$$onActionSubj.next(); - expect(swUpdates.activateUpdate).toHaveBeenCalled(); - }); - - it('should reload the page after a successful activation', fakeAsync(() => { - const global = injector.get(Global); - - expect(global.location.reload).not.toHaveBeenCalled(); - - activateUpdate(true); - expect(global.location.reload).toHaveBeenCalled(); - })); - - it('should report a failed activation', fakeAsync(() => { - activateUpdate(false); - - expect(snackBar.$$lastRef.$$message).toBe(UPDATE_FAILED_MESSAGE); - expect(snackBar.$$lastRef.$$action).toBe('Dismiss'); - expect(snackBar.$$lastRef.$$config.duration).toBeGreaterThan(0); - })); - - it('should dismiss the failed activation snack-bar when clicking on `Dismiss`', fakeAsync(() => { - activateUpdate(false); - expect(snackBar.$$lastRef.$$dismissed).toBe(false); - - snackBar.$$lastRef.$$onActionSubj.next(); - expect(snackBar.$$lastRef.$$dismissed).toBe(true); - })); - }); - - describe('#disable()', () => { - beforeEach(() => service.enable()); - - - it('should dismiss open update notification', () => { - swUpdates.$$isUpdateAvailableSubj.next(true); - expect(snackBar.$$lastRef.$$message).toBe(UPDATE_AVAILABLE_MESSAGE); - expect(snackBar.$$lastRef.$$dismissed).toBe(false); - - service.disable(); - expect(snackBar.$$lastRef.$$dismissed).toBe(true); - }); - - it('should ignore further updates', () => { - service.disable(); - swUpdates.$$isUpdateAvailableSubj.next(true); - - expect(snackBar.$$lastRef).toBeUndefined(); - }); - - it('should not ignore further updates if re-enabled', () => { - service.disable(); - service.enable(); - expect(snackBar.$$lastRef).toBeUndefined(); - - swUpdates.$$isUpdateAvailableSubj.next(true); - expect(snackBar.$$lastRef.$$message).toBe(UPDATE_AVAILABLE_MESSAGE); - }); - - it('should not ignore pending updates if re-enabled', () => { - service.disable(); - swUpdates.isUpdateAvailable = Observable.of(true); - expect(snackBar.$$lastRef).toBeUndefined(); - - service.enable(); - expect(snackBar.$$lastRef.$$message).toBe(UPDATE_AVAILABLE_MESSAGE); - }); - }); -}); - -// Mocks -class MockGlobal { - location = { - reload: jasmine.createSpy('MockGlobal.location.reload') - }; -} - -class MockMdSnackBarRef { - $$afterDismissedSubj = new Subject(); - $$onActionSubj = new Subject(); - $$dismissed = false; - - constructor(public $$message: string, - public $$action: string, - public $$config: MdSnackBarConfig) {} - - afterDismissed() { - return this.$$afterDismissedSubj; - } - - dismiss() { - this.$$dismissed = true; - } - - onAction() { - return this.$$onActionSubj; - } -} - -class MockMdSnackBar { - $$lastRef: MockMdSnackBarRef; - - open(message: string, action: string = null, config: MdSnackBarConfig = {}): MockMdSnackBarRef { - if (this.$$lastRef && !this.$$lastRef.$$dismissed) { - this.$$lastRef.dismiss(); - } - - return this.$$lastRef = new MockMdSnackBarRef(message, action, config); - } -} diff --git a/aio/src/app/sw-updates/sw-update-notifications.service.ts b/aio/src/app/sw-updates/sw-update-notifications.service.ts deleted file mode 100644 index eb9ceaac8f..0000000000 --- a/aio/src/app/sw-updates/sw-update-notifications.service.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { Inject, Injectable } from '@angular/core'; -import { MdSnackBar, MdSnackBarConfig, MdSnackBarRef } from '@angular/material'; -import { Subject } from 'rxjs/Subject'; -import 'rxjs/add/operator/filter'; - -import { Global } from './global.value'; -import { SwUpdatesService } from './sw-updates.service'; - - -/** - * SwUpdateNotificationsService - * - * @description - * Once enabled: - * 1. Subscribes to ServiceWorker updates and prompts the user to update. - * 2. When the user confirms, it activates the update and reloads the page upon activation success. - * 3. Continues to listen for available ServiceWorker updates. - * - * @method - * `disable()` {() => void} - Dismiss any open notifications and stop listening for ServiceWorker - * updates. - * - * @method - * `enable()` {() => void} - Start listening for ServiceWorker updates. - */ -@Injectable() -export class SwUpdateNotificationsService { - private onDisable = new Subject(); - private snackBars: MdSnackBarRef[] = []; - private enabled = false; - - constructor(@Inject(Global) private global: any, - private snackBarService: MdSnackBar, - private swUpdates: SwUpdatesService) { - this.onDisable.subscribe(() => this.snackBars.forEach(sb => sb.dismiss())); - } - - disable() { - if (this.enabled) { - this.enabled = false; - this.onDisable.next(); - } - } - - enable() { - if (!this.enabled) { - this.enabled = true; - this.swUpdates.isUpdateAvailable - .filter(v => v) - .takeUntil(this.onDisable) - .subscribe(() => this.notifyForUpdate()); - } - } - - private activateUpdate() { - this.swUpdates.activateUpdate().then(success => { - if (success) { - this.onActivateSuccess(); - } else { - this.onActivateFailure(); - } - }); - } - - private notifyForUpdate() { - this.openSnackBar('New update for angular.io is available.', 'Update now') - .onAction().subscribe(() => this.activateUpdate()); - } - - private onActivateFailure() { - const snackBar = this.openSnackBar('Update activation failed :(', 'Dismiss', {duration: 5000}); - snackBar.onAction().subscribe(() => snackBar.dismiss()); - } - - private onActivateSuccess() { - this.reloadPage(); - } - - private openSnackBar(message: string, action?: string, config?: MdSnackBarConfig): MdSnackBarRef { - const snackBar = this.snackBarService.open(message, action, config); - snackBar.afterDismissed().subscribe(() => this.snackBars = this.snackBars.filter(sb => sb !== snackBar)); - - this.snackBars.push(snackBar); - - return snackBar; - } - - private reloadPage() { - const location = this.global && (this.global as Window).location; - if (location && location.reload) { - location.reload(); - } - } -} diff --git a/aio/src/app/sw-updates/sw-updates.module.ts b/aio/src/app/sw-updates/sw-updates.module.ts index e0aa61d9f2..573f64d3e7 100644 --- a/aio/src/app/sw-updates/sw-updates.module.ts +++ b/aio/src/app/sw-updates/sw-updates.module.ts @@ -1,20 +1,14 @@ import { NgModule } from '@angular/core'; -import { MdSnackBarModule } from '@angular/material'; import { ServiceWorkerModule } from '@angular/service-worker'; -import { globalProvider } from './global.value'; -import { SwUpdateNotificationsService } from './sw-update-notifications.service'; import { SwUpdatesService } from './sw-updates.service'; @NgModule({ imports: [ - MdSnackBarModule, ServiceWorkerModule ], providers: [ - globalProvider, - SwUpdateNotificationsService, SwUpdatesService ] }) diff --git a/aio/src/app/sw-updates/sw-updates.service.spec.ts b/aio/src/app/sw-updates/sw-updates.service.spec.ts index e66f3ae456..435e03c0fb 100644 --- a/aio/src/app/sw-updates/sw-updates.service.spec.ts +++ b/aio/src/app/sw-updates/sw-updates.service.spec.ts @@ -4,6 +4,7 @@ import { NgServiceWorker } from '@angular/service-worker'; import { Subject } from 'rxjs/Subject'; import 'rxjs/add/operator/take'; +import { Logger } from 'app/shared/logger.service'; import { SwUpdatesService } from './sw-updates.service'; describe('SwUpdatesService', () => { @@ -14,11 +15,13 @@ describe('SwUpdatesService', () => { // Helpers // NOTE: - // Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated - // inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't run - // `setup()` in a `beforeEach()` block. We use the `run()` helper to call it inside each test' zone. + // Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated + // inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't run + // `setup()` in a `beforeEach()` block. We use the `run()` helper to call `setup()` inside each + // test's zone. const setup = () => { injector = ReflectiveInjector.resolveAndCreate([ + { provide: Logger, useClass: MockLogger }, { provide: NgServiceWorker, useClass: MockNgServiceWorker }, SwUpdatesService ]); @@ -39,7 +42,7 @@ describe('SwUpdatesService', () => { expect(service).toBeTruthy(); })); - it('should immediatelly check for updates when instantiated', run(() => { + it('should immediately check for updates when instantiated', run(() => { expect(sw.checkForUpdate).toHaveBeenCalled(); })); @@ -51,9 +54,10 @@ describe('SwUpdatesService', () => { tick(checkInterval); expect(sw.checkForUpdate).toHaveBeenCalled(); + expect(sw.activateUpdate).not.toHaveBeenCalled(); }))); - it('should not schedule a new check if there is an update available', fakeAsync(run(() => { + it('should activate new updates immediately', fakeAsync(run(() => { sw.checkForUpdate.calls.reset(); sw.$$checkForUpdateSubj.next(true); @@ -61,153 +65,92 @@ describe('SwUpdatesService', () => { tick(checkInterval); expect(sw.checkForUpdate).not.toHaveBeenCalled(); + expect(sw.activateUpdate).toHaveBeenCalled(); }))); - describe('#activateUpdate()', () => { - it('should return a promise', run(() => { - expect(service.activateUpdate()).toEqual(jasmine.any(Promise)); - })); + it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', fakeAsync(run(() => { + sw.$$checkForUpdateSubj.next(true); + tick(checkInterval); - it('should call `NgServiceWorker.activateUpdate()`', run(() => { - expect(sw.activateUpdate).not.toHaveBeenCalled(); + expect(sw.activateUpdate).toHaveBeenCalledWith(null); + }))); - service.activateUpdate(); + it('should schedule a new check after activating the update', fakeAsync(run(() => { + sw.checkForUpdate.calls.reset(); + sw.$$checkForUpdateSubj.next(true); + + tick(checkInterval); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + sw.$$activateUpdateSubj.next(); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + tick(checkInterval); + expect(sw.checkForUpdate).toHaveBeenCalled(); + }))); + + it('should emit on `updateActivated` when an update has been activated', run(() => { + const activatedVersions: string[] = []; + service.updateActivated.subscribe(v => activatedVersions.push(v)); + + sw.$$updatesSubj.next({type: 'pending', version: 'foo'}); + sw.$$updatesSubj.next({type: 'activation', version: 'bar'}); + sw.$$updatesSubj.next({type: 'pending', version: 'baz'}); + sw.$$updatesSubj.next({type: 'activation', version: 'qux'}); + + expect(activatedVersions).toEqual(['bar', 'qux']); + })); + + describe('when destroyed', () => { + it('should not schedule a new check for update (after current check)', fakeAsync(run(() => { + sw.checkForUpdate.calls.reset(); + + service.ngOnDestroy(); + sw.$$checkForUpdateSubj.next(false); + tick(checkInterval); + + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + }))); + + it('should not schedule a new check for update (after activating an update)', fakeAsync(run(() => { + sw.checkForUpdate.calls.reset(); + + sw.$$checkForUpdateSubj.next(true); expect(sw.activateUpdate).toHaveBeenCalled(); + + service.ngOnDestroy(); + sw.$$activateUpdateSubj.next(); + tick(checkInterval); + + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + }))); + + it('should stop emitting on `updateActivated`', run(() => { + const activatedVersions: string[] = []; + service.updateActivated.subscribe(v => activatedVersions.push(v)); + + sw.$$updatesSubj.next({type: 'pending', version: 'foo'}); + sw.$$updatesSubj.next({type: 'activation', version: 'bar'}); + service.ngOnDestroy(); + sw.$$updatesSubj.next({type: 'pending', version: 'baz'}); + sw.$$updatesSubj.next({type: 'activation', version: 'qux'}); + + expect(activatedVersions).toEqual(['bar']); })); - - it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', run(() => { - (service.activateUpdate as Function)('foo'); - expect(sw.activateUpdate).toHaveBeenCalledWith(null); - })); - - it('should resolve the promise with the activation outcome', fakeAsync(run(() => { - let outcome; - - service.activateUpdate().then(v => outcome = v); - sw.$$activateUpdateSubj.next(true); - tick(); - expect(outcome).toBe(true); - - service.activateUpdate().then(v => outcome = v); - sw.$$activateUpdateSubj.next(false); - tick(); - expect(outcome).toBe(false); - }))); - - it('should schedule a new check (if the activation succeeded)', fakeAsync(run(() => { - sw.checkForUpdate.calls.reset(); - - service.activateUpdate(); - - tick(checkInterval); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - sw.$$activateUpdateSubj.next(true); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - tick(checkInterval); - expect(sw.checkForUpdate).toHaveBeenCalled(); - }))); - - it('should schedule a new check (if the activation failed)', fakeAsync(run(() => { - sw.checkForUpdate.calls.reset(); - - service.activateUpdate(); - - tick(checkInterval); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - sw.$$activateUpdateSubj.next(false); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - tick(checkInterval); - expect(sw.checkForUpdate).toHaveBeenCalled(); - }))); - }); - - describe('#isUpdateAvailable', () => { - let emittedValues: boolean[]; - - // Helpers - const withSubscription = specFn => () => { - emittedValues = []; - service.isUpdateAvailable.subscribe(v => emittedValues.push(v)); - specFn(); - }; - - - it('should emit `false/true` when there is/isn\'t an update available', - fakeAsync(run(withSubscription(() => { - expect(emittedValues).toEqual([]); - - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([false, true]); - }))) - ); - - it('should emit only when the value has changed', - fakeAsync(run(withSubscription(() => { - expect(emittedValues).toEqual([]); - - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - }))) - ); - - it('should emit `false` after a successful activation', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([true]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(true); - - expect(emittedValues).toEqual([true, false]); - }))) - ); - - it('should emit `false` after a failed activation', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([true]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(false); - - expect(emittedValues).toEqual([true, false]); - }))) - ); - - it('should not emit a new value after activation if already `false`', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(true); - - expect(emittedValues).toEqual([false]); - }))) - ); }); }); // Mocks +class MockLogger { + log = jasmine.createSpy('MockLogger.log'); +} + class MockNgServiceWorker { $$activateUpdateSubj = new Subject(); $$checkForUpdateSubj = new Subject(); + $$updatesSubj = new Subject<{type: string, version: string}>(); + + updates = this.$$updatesSubj.asObservable(); activateUpdate = jasmine.createSpy('MockNgServiceWorker.activateUpdate') .and.callFake(() => this.$$activateUpdateSubj.take(1)); diff --git a/aio/src/app/sw-updates/sw-updates.service.ts b/aio/src/app/sw-updates/sw-updates.service.ts index 60a9ef1fcb..1760a87030 100644 --- a/aio/src/app/sw-updates/sw-updates.service.ts +++ b/aio/src/app/sw-updates/sw-updates.service.ts @@ -1,14 +1,17 @@ -import { Injectable, OnDestroy } from '@angular/core'; +import { Inject, Injectable, OnDestroy } from '@angular/core'; import { NgServiceWorker } from '@angular/service-worker'; import { Observable } from 'rxjs/Observable'; -import { ReplaySubject } from 'rxjs/ReplaySubject'; import { Subject } from 'rxjs/Subject'; -import 'rxjs/add/operator/debounceTime'; -import 'rxjs/add/operator/distinctUntilChanged'; +import 'rxjs/add/observable/of'; import 'rxjs/add/operator/concat'; +import 'rxjs/add/operator/debounceTime'; +import 'rxjs/add/operator/filter'; +import 'rxjs/add/operator/map'; import 'rxjs/add/operator/startWith'; import 'rxjs/add/operator/take'; -import 'rxjs/add/operator/toPromise'; +import 'rxjs/add/operator/takeUntil'; + +import { Logger } from 'app/shared/logger.service'; /** @@ -17,60 +20,57 @@ import 'rxjs/add/operator/toPromise'; * @description * 1. Checks for available ServiceWorker updates once instantiated. * 2. As long as there is no update available, re-checks every 6 hours. - * 3. As soon as an update is detected, it waits until the update is activated, then starts checking - * again (every 6 hours). + * 3. As soon as an update is detected, it activates the update and notifies interested parties. + * 4. It continues to check for available updates. * * @property - * `isUpdateAvailable` {Observable} - Emit `true`/`false` to indicate updates being - * available or not. Remembers the last emitted value. Will only emit a new value if it is different - * than the last one. - * - * @method - * `activateUpdate()` {() => Promise} - Activate the latest available update. The returned - * promise resolves to `true` if an update was activated successfully and `false` if the activation - * failed (e.g. if there was no update to activate). + * `updateActivated` {Observable} - Emit the version hash whenever an update is activated. */ @Injectable() export class SwUpdatesService implements OnDestroy { private checkInterval = 1000 * 60 * 60 * 6; // 6 hours private onDestroy = new Subject(); private checkForUpdateSubj = new Subject(); - private isUpdateAvailableSubj = new ReplaySubject(1); - isUpdateAvailable = this.isUpdateAvailableSubj.distinctUntilChanged(); + updateActivated = this.sw.updates + .takeUntil(this.onDestroy) + .do(evt => this.log(`Update event: ${JSON.stringify(evt)}`)) + .filter(({type}) => type === 'activation') + .map(({version}) => version); - constructor(private sw: NgServiceWorker) { + constructor(private logger: Logger, private sw: NgServiceWorker) { this.checkForUpdateSubj .debounceTime(this.checkInterval) - .takeUntil(this.onDestroy) .startWith(null) - .subscribe(() => this.checkForUpdate()); - - this.isUpdateAvailableSubj - .filter(v => !v) .takeUntil(this.onDestroy) - .subscribe(() => this.checkForUpdateSubj.next()); + .subscribe(() => this.checkForUpdate()); } ngOnDestroy() { this.onDestroy.next(); } - activateUpdate(): Promise { - return new Promise(resolve => { - this.sw.activateUpdate(null) - // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137. - // TODO (gkalpak): Remove once #137 is fixed. - .concat(Observable.of(false)).take(1) - .do(() => this.isUpdateAvailableSubj.next(false)) - .subscribe(resolve); - }); + private activateUpdate() { + this.log('Activating update...'); + this.sw.activateUpdate(null) + .subscribe(() => this.scheduleCheckForUpdate()); } private checkForUpdate() { + this.log('Checking for update...'); this.sw.checkForUpdate() // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137. // TODO (gkalpak): Remove once #137 is fixed. .concat(Observable.of(false)).take(1) - .subscribe(v => this.isUpdateAvailableSubj.next(v)); + .do(v => this.log(`Update available: ${v}`)) + .subscribe(v => v ? this.activateUpdate() : this.scheduleCheckForUpdate()); + } + + private log(message: string) { + const timestamp = (new Date).toISOString(); + this.logger.log(`[SwUpdates - ${timestamp}]: ${message}`); + } + + private scheduleCheckForUpdate() { + this.checkForUpdateSubj.next(); } } diff --git a/aio/src/testing/sw-update-notifications.service.ts b/aio/src/testing/sw-update-notifications.service.ts deleted file mode 100644 index 30c0cefa1d..0000000000 --- a/aio/src/testing/sw-update-notifications.service.ts +++ /dev/null @@ -1,4 +0,0 @@ -export class MockSwUpdateNotificationsService { - enable = jasmine.createSpy('MockSwUpdateNotificationsService.enable'); - disable = jasmine.createSpy('MockSwUpdateNotificationsService.disable'); -} diff --git a/aio/src/testing/sw-updates.service.ts b/aio/src/testing/sw-updates.service.ts deleted file mode 100644 index 2553674d0f..0000000000 --- a/aio/src/testing/sw-updates.service.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { Subject } from 'rxjs/Subject'; -import 'rxjs/add/operator/distinctUntilChanged'; -import 'rxjs/add/operator/take'; - - -export class MockSwUpdatesService { - $$activateUpdateSubj = new Subject(); - $$isUpdateAvailableSubj = new Subject(); - isUpdateAvailable = this.$$isUpdateAvailableSubj.distinctUntilChanged(); - - activateUpdate(): Promise { - return new Promise(resolve => { - this.$$activateUpdateSubj - // Better simulate what actually happens with the real ServiceWorker. - .take(1) - .do(() => this.$$isUpdateAvailableSubj.next(false)) - .subscribe(resolve); - }); - } -}