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 (#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 5669333
Show file tree
Hide file tree
Showing 8 changed files with 44 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 @@ -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',
Expand Down Expand Up @@ -70,6 +72,7 @@ describe('Generator', () => {
res.then(config => {
expect(config).toEqual({
configVersion: 1,
timestamp: 1234567890123,
appData: {
test: true,
},
Expand Down Expand Up @@ -137,6 +140,7 @@ describe('Generator', () => {
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 @@ -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: [{
Expand All @@ -46,6 +47,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
32 changes: 26 additions & 6 deletions packages/service-worker/worker/test/happy_spec.ts
Expand Up @@ -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',
Expand Down Expand Up @@ -86,6 +87,7 @@ const manifestOld: ManifestV5 = {

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

const manifestUpdate: Manifest = {
configVersion: 1,
timestamp: 1234567890123,
appData: {
version: 'update',
},
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
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 @@ -306,6 +306,7 @@ export class ConfigBuilder {
const hashTable = {};
return {
configVersion: 1,
timestamp: 1234567890123,
index: '/index.html', assetGroups,
navigationUrls: [], hashTable,
};
Expand Down

4 comments on commit 5669333

@ujjwal-kr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this fix the update cache error?

@gkalpak
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What error is that?

@schankam
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has it been merged into the stable release yet ? I am using 7.3 and looks like I still got some issues with client app cache not refreshing at all on app update.

@gkalpak
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using 7.3

7.2.15 is the latest stable. There is no 7.3.x 😕

In any case, this commit has been included in the 7.2.8 release.

Please sign in to comment.