Skip to content
Permalink
Browse files

fix(service-worker): detect new version even if files are identical t…

…o 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
  • Loading branch information...
gkalpak authored and AndrewKushnir committed Mar 5, 2019
1 parent c976b88 commit 56693339c2ff7a6574214e54f89e33136dbc478e
@@ -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),
@@ -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: [],
@@ -31,6 +31,7 @@ function obsToSinglePromise<T>(obs: Observable<T>): Promise<T> {

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: [{
@@ -12,6 +12,7 @@ export type ManifestHash = string;

export interface Manifest {
configVersion: number;
timestamp: number;
appData?: {[key: string]: string};
index: string;
assetGroups?: AssetGroupConfig[];
@@ -39,6 +39,7 @@ const distUpdate = new MockFileSystemBuilder()

const manifest: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
index: '/index.html',
assetGroups: [
{
@@ -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;
@@ -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: [
{
@@ -306,6 +306,7 @@ export class ConfigBuilder {
const hashTable = {};
return {
configVersion: 1,
timestamp: 1234567890123,
index: '/index.html', assetGroups,
navigationUrls: [], hashTable,
};

2 comments on commit 5669333

@ujjwal-kr

This comment has been minimized.

Copy link

ujjwal-kr replied Apr 23, 2019

Will this fix the update cache error?

@gkalpak

This comment has been minimized.

Copy link
Member Author

gkalpak replied Apr 23, 2019

What error is that?

Please sign in to comment.
You can’t perform that action at this time.