From 56693339c2ff7a6574214e54f89e33136dbc478e Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 5 Mar 2019 15:24:07 +0200 Subject: [PATCH] fix(service-worker): detect new version even if files are identical to an old one (#26006) Previously, if an app version contained the same files as an older version (e.g. making a change, then rolling it back), the SW would not detect it as the latest version (and update clients). This commit fixes it by adding a `timestamp` field in `ngsw.json`, which makes each build unique (with sufficiently high probability). Fixes #24338 PR Close #26006 --- .../service-worker/config/src/generator.ts | 1 + .../config/test/generator_spec.ts | 4 +++ .../service-worker/test/integration_spec.ts | 2 ++ .../service-worker/worker/src/manifest.ts | 1 + .../service-worker/worker/test/data_spec.ts | 1 + .../service-worker/worker/test/happy_spec.ts | 32 +++++++++++++++---- .../service-worker/worker/testing/mock.ts | 9 +++++- .../service-worker/worker/testing/scope.ts | 1 + 8 files changed, 44 insertions(+), 7 deletions(-) diff --git a/packages/service-worker/config/src/generator.ts b/packages/service-worker/config/src/generator.ts index fd6bf09c7417b..0de825a65f6d6 100644 --- a/packages/service-worker/config/src/generator.ts +++ b/packages/service-worker/config/src/generator.ts @@ -32,6 +32,7 @@ export class Generator { return { configVersion: 1, + timestamp: Date.now(), appData: config.appData, index: joinUrls(this.baseHref, config.index), assetGroups, dataGroups: this.processDataGroups(config), diff --git a/packages/service-worker/config/test/generator_spec.ts b/packages/service-worker/config/test/generator_spec.ts index 1f3ebad858283..f0a6117fad4b2 100644 --- a/packages/service-worker/config/test/generator_spec.ts +++ b/packages/service-worker/config/test/generator_spec.ts @@ -10,6 +10,8 @@ import {Generator} from '../src/generator'; import {MockFilesystem} from '../testing/mock'; describe('Generator', () => { + beforeEach(() => spyOn(Date, 'now').and.returnValue(1234567890123)); + it('generates a correct config', done => { const fs = new MockFilesystem({ '/index.html': 'This is a test', @@ -70,6 +72,7 @@ describe('Generator', () => { res.then(config => { expect(config).toEqual({ configVersion: 1, + timestamp: 1234567890123, appData: { test: true, }, @@ -137,6 +140,7 @@ describe('Generator', () => { res.then(config => { expect(config).toEqual({ configVersion: 1, + timestamp: 1234567890123, appData: undefined, index: '/test/index.html', assetGroups: [], diff --git a/packages/service-worker/test/integration_spec.ts b/packages/service-worker/test/integration_spec.ts index d56a791c521a0..44d70265f879b 100644 --- a/packages/service-worker/test/integration_spec.ts +++ b/packages/service-worker/test/integration_spec.ts @@ -31,6 +31,7 @@ function obsToSinglePromise(obs: Observable): Promise { const manifest: Manifest = { configVersion: 1, + timestamp: 1234567890123, appData: {version: '1'}, index: '/only.txt', assetGroups: [{ @@ -46,6 +47,7 @@ const manifest: Manifest = { const manifestUpdate: Manifest = { configVersion: 1, + timestamp: 1234567890123, appData: {version: '2'}, index: '/only.txt', assetGroups: [{ diff --git a/packages/service-worker/worker/src/manifest.ts b/packages/service-worker/worker/src/manifest.ts index 7800afa4885e6..ba028d6c21e80 100644 --- a/packages/service-worker/worker/src/manifest.ts +++ b/packages/service-worker/worker/src/manifest.ts @@ -12,6 +12,7 @@ export type ManifestHash = string; export interface Manifest { configVersion: number; + timestamp: number; appData?: {[key: string]: string}; index: string; assetGroups?: AssetGroupConfig[]; diff --git a/packages/service-worker/worker/test/data_spec.ts b/packages/service-worker/worker/test/data_spec.ts index cdfe16b7e21e5..1474dd12c5ae5 100644 --- a/packages/service-worker/worker/test/data_spec.ts +++ b/packages/service-worker/worker/test/data_spec.ts @@ -39,6 +39,7 @@ const distUpdate = new MockFileSystemBuilder() const manifest: Manifest = { configVersion: 1, + timestamp: 1234567890123, index: '/index.html', assetGroups: [ { diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 783b3a8cfe03c..a4c744b109078 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -51,6 +51,7 @@ const brokenFs = new MockFileSystemBuilder().addFile('/foo.txt', 'this is foo'). const brokenManifest: Manifest = { configVersion: 1, + timestamp: 1234567890123, index: '/foo.txt', assetGroups: [{ name: 'assets', @@ -86,6 +87,7 @@ const manifestOld: ManifestV5 = { const manifest: Manifest = { configVersion: 1, + timestamp: 1234567890123, appData: { version: 'original', }, @@ -133,6 +135,7 @@ const manifest: Manifest = { const manifestUpdate: Manifest = { configVersion: 1, + timestamp: 1234567890123, appData: { version: 'update', }, @@ -185,12 +188,16 @@ const manifestUpdate: Manifest = { hashTable: tmpHashTableForFs(distUpdate), }; -const server = new MockServerStateBuilder() - .withStaticFiles(dist) - .withManifest(manifest) - .withRedirect('/redirected.txt', '/redirect-target.txt', 'this was a redirect') - .withError('/error.txt') - .build(); +const serverBuilderBase = + new MockServerStateBuilder() + .withStaticFiles(dist) + .withRedirect('/redirected.txt', '/redirect-target.txt', 'this was a redirect') + .withError('/error.txt'); + +const server = serverBuilderBase.withManifest(manifest).build(); + +const serverRollback = + serverBuilderBase.withManifest({...manifest, timestamp: manifest.timestamp + 1}).build(); const serverUpdate = new MockServerStateBuilder() @@ -372,6 +379,19 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate)); serverUpdate.assertNoOtherRequests(); }); + async_it('detects new version even if only `manifest.timestamp` is different', async() => { + expect(await makeRequest(scope, '/foo.txt', 'newClient')).toEqual('this is foo'); + await driver.initialized; + + scope.updateServerState(serverUpdate); + expect(await driver.checkForUpdate()).toEqual(true); + expect(await makeRequest(scope, '/foo.txt', 'newerClient')).toEqual('this is foo v2'); + + scope.updateServerState(serverRollback); + expect(await driver.checkForUpdate()).toEqual(true); + expect(await makeRequest(scope, '/foo.txt', 'newestClient')).toEqual('this is foo'); + }); + async_it('updates a specific client to new content on request', async() => { expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); await driver.initialized; diff --git a/packages/service-worker/worker/testing/mock.ts b/packages/service-worker/worker/testing/mock.ts index 00f5b6d053cdd..907e3756148b9 100644 --- a/packages/service-worker/worker/testing/mock.ts +++ b/packages/service-worker/worker/testing/mock.ts @@ -88,7 +88,13 @@ export class MockServerStateBuilder { return this; } - build(): MockServerState { return new MockServerState(this.resources, this.errors); } + build(): MockServerState { + // Take a "snapshot" of the current `resources` and `errors`. + const resources = new Map(this.resources.entries()); + const errors = new Set(this.errors.values()); + + return new MockServerState(resources, errors); + } } export class MockServerState { @@ -187,6 +193,7 @@ export function tmpManifestSingleAssetGroup(fs: MockFileSystem): Manifest { files.forEach(path => { hashTable[path] = fs.lookup(path) !.hash; }); return { configVersion: 1, + timestamp: 1234567890123, index: '/index.html', assetGroups: [ { diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index 6b5127d947505..f357f4da41775 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -306,6 +306,7 @@ export class ConfigBuilder { const hashTable = {}; return { configVersion: 1, + timestamp: 1234567890123, index: '/index.html', assetGroups, navigationUrls: [], hashTable, };