diff --git a/packages/service-worker/worker/src/app-version.ts b/packages/service-worker/worker/src/app-version.ts index 07cf0159ca..e02c4e1d44 100644 --- a/packages/service-worker/worker/src/app-version.ts +++ b/packages/service-worker/worker/src/app-version.ts @@ -284,11 +284,14 @@ export class AppVersion implements UpdateSource { } /** - * Erase this application version, by cleaning up all the caches. + * Return a list of the names of all caches used by this version. */ - async cleanup(): Promise { - await Promise.all(this.assetGroups.map(group => group.cleanup())); - await Promise.all(this.dataGroups.map(group => group.cleanup())); + async getCacheNames(): Promise { + const allGroupCacheNames = await Promise.all([ + ...this.assetGroups.map(group => group.getCacheNames()), + ...this.dataGroups.map(group => group.getCacheNames()), + ]); + return ([] as string[]).concat(...allGroupCacheNames); } /** diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index d04b57dfeb..f3beb8a421 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -9,6 +9,7 @@ import {Adapter, Context} from './adapter'; import {CacheState, NormalizedUrl, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {Database, Table} from './database'; +import {CacheTable} from './db-cache'; import {errorToString, SwCriticalError, SwUnrecoverableStateError} from './error'; import {IdleScheduler} from './idle'; import {AssetGroupConfig} from './manifest'; @@ -100,13 +101,14 @@ export abstract class AssetGroup { abstract initializeFully(updateFrom?: UpdateSource): Promise; /** - * Clean up all the cached data for this group. + * Return a list of the names of all caches used by this group. */ - async cleanup(): Promise { - await Promise.all([ - this.cache.then(cache => this.adapter.caches.delete(cache.name)), - this.metadata.then(metadata => this.db.delete(metadata.name)), + async getCacheNames(): Promise { + const [cache, metadata] = await Promise.all([ + this.cache, + this.metadata as Promise, ]); + return [cache.name, metadata.cacheName]; } /** diff --git a/packages/service-worker/worker/src/data.ts b/packages/service-worker/worker/src/data.ts index 4fa3a29ff3..ef41458a3c 100644 --- a/packages/service-worker/worker/src/data.ts +++ b/packages/service-worker/worker/src/data.ts @@ -8,6 +8,7 @@ import {Adapter, Context} from './adapter'; import {Database, Table} from './database'; +import {CacheTable} from './db-cache'; import {DebugHandler} from './debug'; import {DataGroupConfig} from './manifest'; import {NamedCache} from './named-cache-storage'; @@ -554,6 +555,17 @@ export class DataGroup { this.lruTable.then(table => this.db.delete(table.name)), ]); } + /** + * Return a list of the names of all caches used by this group. + */ + async getCacheNames(): Promise { + const [cache, ageTable, lruTable] = await Promise.all([ + this.cache, + this.ageTable as Promise, + this.lruTable as Promise, + ]); + return [cache.name, ageTable.cacheName, lruTable.cacheName]; + } /** * Clear the state of the cache for a particular resource. diff --git a/packages/service-worker/worker/src/db-cache.ts b/packages/service-worker/worker/src/db-cache.ts index f360879ec4..5ff1f9a678 100644 --- a/packages/service-worker/worker/src/db-cache.ts +++ b/packages/service-worker/worker/src/db-cache.ts @@ -8,6 +8,7 @@ import {Adapter} from './adapter'; import {Database, NotFound, Table} from './database'; +import {NamedCache} from './named-cache-storage'; /** @@ -51,8 +52,10 @@ export class CacheDatabase implements Database { * A `Table` backed by a `Cache`. */ export class CacheTable implements Table { + cacheName = this.cache.name; + constructor( - readonly name: string, private cache: Cache, private adapter: Adapter, + readonly name: string, private cache: NamedCache, private adapter: Adapter, private cacheQueryOptions?: CacheQueryOptions) {} private request(key: string): Request { diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index ea3f0798ab..cca26ae408 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -10,6 +10,7 @@ import {Adapter} from './adapter'; import {CacheState, Debuggable, DebugIdleState, DebugState, DebugVersion, NormalizedUrl, UpdateCacheStatus, UpdateSource} from './api'; import {AppVersion} from './app-version'; import {Database} from './database'; +import {CacheTable} from './db-cache'; import {DebugHandler} from './debug'; import {errorToString} from './error'; import {IdleScheduler} from './idle'; @@ -110,6 +111,9 @@ export class Driver implements Debuggable, UpdateSource { debugger: DebugHandler; + // A promise resolving to the control DB table. + private controlTable = this.db.open('control'); + constructor( private scope: ServiceWorkerGlobalScope, private adapter: Adapter, private db: Database) { // Set up all the event handlers that the SW needs. @@ -517,8 +521,7 @@ export class Driver implements Debuggable, UpdateSource { // the SW has run or the DB state has been wiped or is inconsistent. In that case, // load a fresh copy of the manifest and reset the state from scratch. - // Open up the DB table. - const table = await this.db.open('control'); + const table = await this.controlTable; // Attempt to load the needed state from the DB. If this fails, the catch {} block // will populate these variables with freshly constructed values. @@ -568,12 +571,7 @@ export class Driver implements Debuggable, UpdateSource { // Schedule cleaning up obsolete caches in the background. this.idle.schedule('init post-load (cleanup)', async () => { - try { - await this.cleanupCaches(); - } catch (err) { - // Nothing to do - cleanup failed. Just log it. - this.debugger.log(err, 'cleanupCaches @ init post-load'); - } + await this.cleanupCaches(); }); // Initialize the `versions` map by setting each hash to a new `AppVersion` instance @@ -901,8 +899,7 @@ export class Driver implements Debuggable, UpdateSource { * Synchronize the existing state to the underlying database. */ private async sync(): Promise { - // Open up the DB table. - const table = await this.db.open('control'); + const table = await this.controlTable; // Construct a serializable map of hashes to manifests. const manifests: ManifestMap = {}; @@ -931,56 +928,45 @@ export class Driver implements Debuggable, UpdateSource { } async cleanupCaches(): Promise { - // Query for all currently active clients, and list the client ids. This may skip - // some clients in the browser back-forward cache, but not much can be done about - // that. - const activeClients: ClientId[] = - (await this.scope.clients.matchAll()).map(client => client.id); + try { + // Query for all currently active clients, and list the client IDs. This may skip some clients + // in the browser back-forward cache, but not much can be done about that. + const activeClients = + new Set((await this.scope.clients.matchAll()).map(client => client.id)); - // A simple list of client ids that the SW has kept track of. Subtracting - // activeClients from this list will result in the set of client ids which are - // being tracked but are no longer used in the browser, and thus can be cleaned up. - const knownClients: ClientId[] = Array.from(this.clientVersionMap.keys()); + // A simple list of client IDs that the SW has kept track of. Subtracting `activeClients` from + // this list will result in the set of client IDs which are being tracked but are no longer + // used in the browser, and thus can be cleaned up. + const knownClients: ClientId[] = Array.from(this.clientVersionMap.keys()); - // Remove clients in the clientVersionMap that are no longer active. - knownClients.filter(id => activeClients.indexOf(id) === -1) - .forEach(id => this.clientVersionMap.delete(id)); + // Remove clients in the `clientVersionMap` that are no longer active. + const obsoleteClients = knownClients.filter(id => !activeClients.has(id)); + obsoleteClients.forEach(id => this.clientVersionMap.delete(id)); - // Next, determine the set of versions which are still used. All others can be - // removed. - const usedVersions = new Set(); - this.clientVersionMap.forEach((version, _) => usedVersions.add(version)); + // Next, determine the set of versions which are still used. All others can be removed. + const usedVersions = new Set(this.clientVersionMap.values()); - // Collect all obsolete versions by filtering out used versions from the set of all versions. - const obsoleteVersions = - Array.from(this.versions.keys()) - .filter(version => !usedVersions.has(version) && version !== this.latestHash); + // Collect all obsolete versions by filtering out used versions from the set of all versions. + const obsoleteVersions = + Array.from(this.versions.keys()) + .filter(version => !usedVersions.has(version) && version !== this.latestHash); - // Remove all the versions which are no longer used. - await obsoleteVersions.reduce(async (previous, version) => { - // Wait for the other cleanup operations to complete. - await previous; + // Remove all the versions which are no longer used. + obsoleteVersions.forEach(version => this.versions.delete(version)); - // Try to get past the failure of one particular version to clean up (this - // shouldn't happen, but handle it just in case). - try { - // Get ahold of the AppVersion for this particular hash. - const instance = this.versions.get(version)!; + // Commit all the changes to the saved state. + await this.sync(); - // Delete it from the canonical map. - this.versions.delete(version); - - // Clean it up. - await instance.cleanup(); - } catch (err) { - // Oh well? Not much that can be done here. These caches will be removed when - // the SW revs its format version, which happens from time to time. - this.debugger.log(err, `cleanupCaches - cleanup ${version}`); - } - }, Promise.resolve()); - - // Commit all the changes to the saved state. - await this.sync(); + // Delete all caches that are no longer needed. + const allCaches = await this.adapter.caches.keys(); + const usedCaches = new Set(await this.getCacheNames()); + const cachesToDelete = allCaches.filter(name => !usedCaches.has(name)); + await Promise.all(cachesToDelete.map(name => this.adapter.caches.delete(name))); + } catch (err) { + // Oh well? Not much that can be done here. These caches will be removed on the next attempt + // or when the SW revs its format version, which happens from time to time. + this.debugger.log(err, 'cleanupCaches'); + } } /** @@ -1150,4 +1136,12 @@ export class Driver implements Debuggable, UpdateSource { }); } } + + private async getCacheNames(): Promise { + const controlTable = await this.controlTable as CacheTable; + const appVersions = Array.from(this.versions.values()); + const appVersionCacheNames = + await Promise.all(appVersions.map(version => version.getCacheNames())); + return [controlTable.cacheName].concat(...appVersionCacheNames); + } } diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 7e15572fc8..a231095160 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -698,6 +698,42 @@ describe('Driver', () => { expect(hasOriginalCaches).toEqual(false); }); + it('cleans up properly when failing to load stored state', async () => { + // Initialize the SW and cache the original app-version. + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo'); + await driver.initialized; + + // Update and cache the updated app-version. + scope.updateServerState(serverUpdate); + expect(await driver.checkForUpdate()).toBeTrue(); + expect(await makeRequest(scope, '/foo.txt', 'newClient')).toBe('this is foo v2'); + + // Verify both app-versions are stored in the cache. + let cacheNames = await scope.caches.keys(); + let hasOriginalVersion = cacheNames.some(name => name.startsWith(`${manifestHash}:`)); + let hasUpdatedVersion = cacheNames.some(name => name.startsWith(`${manifestUpdateHash}:`)); + expect(hasOriginalVersion).withContext('Has caches for original version').toBeTrue(); + expect(hasUpdatedVersion).withContext('Has caches for updated version').toBeTrue(); + + // Simulate failing to load the stored state (and thus starting from an empty state). + scope.caches.delete('db:control'); + driver = new Driver(scope, scope, new CacheDatabase(scope)); + + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo v2'); + await driver.initialized; + + // Verify that the caches for the obsolete original version are cleaned up. + // await driver.cleanupCaches(); + scope.advance(6000); + await driver.idle.empty; + + cacheNames = await scope.caches.keys(); + hasOriginalVersion = cacheNames.some(name => name.startsWith(`${manifestHash}:`)); + hasUpdatedVersion = cacheNames.some(name => name.startsWith(`${manifestUpdateHash}:`)); + expect(hasOriginalVersion).withContext('Has caches for original version').toBeFalse(); + expect(hasUpdatedVersion).withContext('Has caches for updated version').toBeTrue(); + }); + it('shows notifications for push notifications', async () => { expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); await driver.initialized; @@ -1481,6 +1517,7 @@ describe('Driver', () => { // Another 6 seconds. scope.advance(6000); await driver.idle.empty; + await new Promise(resolve => setTimeout(resolve)); // Wait for async operations to complete. serverUpdate.assertSawRequestFor('/unhashed/a.txt'); // Now the new version of the resource should be served. @@ -1817,16 +1854,19 @@ describe('Driver', () => { 'ngsuu:active', 'not:ngsw:active', 'NgSw:StAgEd', - 'ngsw:/:active', - 'ngsw:/foo/:staged', + 'ngsw:/:db:control', + 'ngsw:/foo/:active', + 'ngsw:/bar/:staged', ]; const allCacheNames = oldSwCacheNames.concat(otherCacheNames); await Promise.all(allCacheNames.map(name => scope.caches.original.open(name))); - expect(await scope.caches.original.keys()).toEqual(allCacheNames); + expect(await scope.caches.original.keys()) + .toEqual(jasmine.arrayWithExactContents(allCacheNames)); await driver.cleanupOldSwCaches(); - expect(await scope.caches.original.keys()).toEqual(otherCacheNames); + expect(await scope.caches.original.keys()) + .toEqual(jasmine.arrayWithExactContents(otherCacheNames)); }); it('should delete other caches even if deleting one of them fails', async () => {