Skip to content

Commit

Permalink
fix(service-worker): detect new version even if files are identical t…
Browse files Browse the repository at this point in the history
…o an old one

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
  • Loading branch information
gkalpak committed Sep 18, 2018
1 parent e952c65 commit d12a118
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 7 deletions.
1 change: 1 addition & 0 deletions packages/service-worker/config/src/generator.ts
Expand Up @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions packages/service-worker/config/test/generator_spec.ts
Expand Up @@ -11,6 +11,8 @@ 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',
Expand Down Expand Up @@ -71,6 +73,7 @@ import {MockFilesystem} from '../testing/mock';
res.then(config => {
expect(config).toEqual({
configVersion: 1,
timestamp: 1234567890123,
appData: {
test: true,
},
Expand Down Expand Up @@ -138,6 +141,7 @@ import {MockFilesystem} from '../testing/mock';
res.then(config => {
expect(config).toEqual({
configVersion: 1,
timestamp: 1234567890123,
appData: undefined,
index: '/test/index.html',
assetGroups: [],
Expand Down
2 changes: 2 additions & 0 deletions packages/service-worker/test/integration_spec.ts
Expand Up @@ -32,6 +32,7 @@ function obsToSinglePromise<T>(obs: Observable<T>): Promise<T> {

const manifest: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
appData: {version: '1'},
index: '/only.txt',
assetGroups: [{
Expand All @@ -47,6 +48,7 @@ const manifest: Manifest = {

const manifestUpdate: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
appData: {version: '2'},
index: '/only.txt',
assetGroups: [{
Expand Down
1 change: 1 addition & 0 deletions packages/service-worker/worker/src/manifest.ts
Expand Up @@ -12,6 +12,7 @@ export type ManifestHash = string;

export interface Manifest {
configVersion: number;
timestamp: number;
appData?: {[key: string]: string};
index: string;
assetGroups?: AssetGroupConfig[];
Expand Down
1 change: 1 addition & 0 deletions packages/service-worker/worker/test/data_spec.ts
Expand Up @@ -39,6 +39,7 @@ const distUpdate = new MockFileSystemBuilder()

const manifest: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
index: '/index.html',
assetGroups: [
{
Expand Down
37 changes: 31 additions & 6 deletions packages/service-worker/worker/test/happy_spec.ts
Expand Up @@ -50,6 +50,7 @@ const brokenFs = new MockFileSystemBuilder().addFile('/foo.txt', 'this is foo').

const brokenManifest: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
index: '/foo.txt',
assetGroups: [{
name: 'assets',
Expand All @@ -67,6 +68,7 @@ const brokenManifest: Manifest = {

const manifest: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
appData: {
version: 'original',
},
Expand Down Expand Up @@ -114,6 +116,7 @@ const manifest: Manifest = {

const manifestUpdate: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
appData: {
version: 'update',
},
Expand Down Expand Up @@ -166,12 +169,21 @@ 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()
Expand Down Expand Up @@ -309,6 +321,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;
Expand Down
9 changes: 8 additions & 1 deletion packages/service-worker/worker/testing/mock.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: [
{
Expand Down
1 change: 1 addition & 0 deletions packages/service-worker/worker/testing/scope.ts
Expand Up @@ -290,6 +290,7 @@ export class ConfigBuilder {
const hashTable = {};
return {
configVersion: 1,
timestamp: 1234567890123,
index: '/index.html', assetGroups,
navigationUrls: [], hashTable,
};
Expand Down

0 comments on commit d12a118

Please sign in to comment.