mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
fix(service-worker): make ngsw.json generation deterministic and correct (#43679)
Previously, all asset-groups from `ngsw-config.json` were processed in parallel. For each asset-group, we retrieved all files for the current build, filtered out files that were already matched by other asset-groups, determined which of the remaining files belonged to the current asset-group and generated entries for the `ngsw.json` manifest. This process was susceptible to race conditions when there were files that would be matched by multiple asset-groups. This made the generation of the `ngsw.json` manifest non-deterministic and violated the rule that each file would belong to the first asset-group that matched it (based on the asset-groups' order of appearance in `ngsw-config.json`), thus leading to broken ServiceWorker behavior. This commit fixes it by ensuring that the generation process is deterministic and that asset-groups are processed in the proper order. NOTE 1: The generation process has been broken since the beginning, but we have only noticed this recently. This is possibly related to the CLI's switching from a virtual file system host (which has more consistent timing characteristics) to the Node.js built-in `fs.promises` in angular/angular-cli@d3bc530c10. NOTE 2: This commit also ensures that files in the `ngsw.json` hash-table are in alphabetic order. Previously, the files were added to the hash-table in blocks corresponding to each asset-group. This change is not necessary (i.e. the order of keys in the hash-table makes no difference in behavior), but it makes it easier to scan for a file (for example, for debugging purposes). PR Close #43679
This commit is contained in:
parent
0dc45446fe
commit
fddb50b597
2 changed files with 123 additions and 19 deletions
|
|
@ -9,7 +9,7 @@
|
|||
import {parseDurationToMs} from './duration';
|
||||
import {Filesystem} from './filesystem';
|
||||
import {globToRegex} from './glob';
|
||||
import {Config} from './in';
|
||||
import {AssetGroup, Config} from './in';
|
||||
|
||||
const DEFAULT_NAVIGATION_URLS = [
|
||||
'/**', // Include all URLs.
|
||||
|
|
@ -45,8 +45,13 @@ export class Generator {
|
|||
|
||||
private async processAssetGroups(config: Config, hashTable: {[file: string]: string|undefined}):
|
||||
Promise<Object[]> {
|
||||
// Retrieve all files of the build.
|
||||
const allFiles = await this.fs.list('/');
|
||||
const seenMap = new Set<string>();
|
||||
return Promise.all((config.assetGroups || []).map(async (group) => {
|
||||
const filesPerGroup = new Map<AssetGroup, string[]>();
|
||||
|
||||
// Computed which files belong to each asset-group.
|
||||
for (const group of (config.assetGroups || [])) {
|
||||
if ((group.resources as any).versionedFiles) {
|
||||
throw new Error(
|
||||
`Asset-group '${group.name}' in 'ngsw-config.json' uses the 'versionedFiles' option, ` +
|
||||
|
|
@ -54,27 +59,30 @@ export class Generator {
|
|||
}
|
||||
|
||||
const fileMatcher = globListToMatcher(group.resources.files || []);
|
||||
const allFiles = await this.fs.list('/');
|
||||
|
||||
const matchedFiles = allFiles.filter(fileMatcher).filter(file => !seenMap.has(file)).sort();
|
||||
|
||||
matchedFiles.forEach(file => seenMap.add(file));
|
||||
filesPerGroup.set(group, matchedFiles);
|
||||
}
|
||||
|
||||
// Add the hashes.
|
||||
await matchedFiles.reduce(async (previous, file) => {
|
||||
await previous;
|
||||
const hash = await this.fs.hash(file);
|
||||
hashTable[joinUrls(this.baseHref, file)] = hash;
|
||||
}, Promise.resolve());
|
||||
// Compute hashes for all matched files and add them to the hash-table.
|
||||
const allMatchedFiles = ([] as string[]).concat(...Array.from(filesPerGroup.values())).sort();
|
||||
const allMatchedHashes = await Promise.all(allMatchedFiles.map(file => this.fs.hash(file)));
|
||||
allMatchedFiles.forEach((file, idx) => {
|
||||
hashTable[joinUrls(this.baseHref, file)] = allMatchedHashes[idx];
|
||||
});
|
||||
|
||||
return {
|
||||
name: group.name,
|
||||
installMode: group.installMode || 'prefetch',
|
||||
updateMode: group.updateMode || group.installMode || 'prefetch',
|
||||
cacheQueryOptions: buildCacheQueryOptions(group.cacheQueryOptions),
|
||||
urls: matchedFiles.map(url => joinUrls(this.baseHref, url)),
|
||||
patterns: (group.resources.urls || []).map(url => urlToRegex(url, this.baseHref, true)),
|
||||
};
|
||||
}));
|
||||
// Generate and return the processed asset-groups.
|
||||
return Array.from(filesPerGroup.entries())
|
||||
.map(([group, matchedFiles]) => ({
|
||||
name: group.name,
|
||||
installMode: group.installMode || 'prefetch',
|
||||
updateMode: group.updateMode || group.installMode || 'prefetch',
|
||||
cacheQueryOptions: buildCacheQueryOptions(group.cacheQueryOptions),
|
||||
urls: matchedFiles.map(url => joinUrls(this.baseHref, url)),
|
||||
patterns:
|
||||
(group.resources.urls || []).map(url => urlToRegex(url, this.baseHref, true)),
|
||||
}));
|
||||
}
|
||||
|
||||
private processDataGroups(config: Config): Object[] {
|
||||
|
|
|
|||
|
|
@ -128,6 +128,102 @@ describe('Generator', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('assigns files to the first matching asset-group (unaffected by file-system access delays)',
|
||||
async () => {
|
||||
const fs = new MockFilesystem({
|
||||
'/index.html': 'This is a test',
|
||||
'/foo/script-1.js': 'This is script 1',
|
||||
'/foo/script-2.js': 'This is script 2',
|
||||
'/bar/script-3.js': 'This is script 3',
|
||||
'/bar/script-4.js': 'This is script 4',
|
||||
'/qux/script-5.js': 'This is script 5',
|
||||
});
|
||||
|
||||
// Simulate fluctuating file-system access delays.
|
||||
const allFiles = await fs.list('/');
|
||||
spyOn(fs, 'list')
|
||||
.and.returnValues(
|
||||
new Promise(resolve => setTimeout(resolve, 2000, allFiles.slice())),
|
||||
new Promise(resolve => setTimeout(resolve, 3000, allFiles.slice())),
|
||||
new Promise(resolve => setTimeout(resolve, 1000, allFiles.slice())),
|
||||
);
|
||||
|
||||
const gen = new Generator(fs, '');
|
||||
const config = await gen.process({
|
||||
index: '/index.html',
|
||||
assetGroups: [
|
||||
{
|
||||
name: 'group-foo',
|
||||
resources: {files: ['/foo/**/*.js']},
|
||||
},
|
||||
{
|
||||
name: 'group-bar',
|
||||
resources: {files: ['/bar/**/*.js']},
|
||||
},
|
||||
{
|
||||
name: 'group-fallback',
|
||||
resources: {files: ['/**/*.js']},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(config).toEqual({
|
||||
configVersion: 1,
|
||||
timestamp: 1234567890123,
|
||||
appData: undefined,
|
||||
index: '/index.html',
|
||||
assetGroups: [
|
||||
{
|
||||
name: 'group-foo',
|
||||
installMode: 'prefetch',
|
||||
updateMode: 'prefetch',
|
||||
cacheQueryOptions: {ignoreVary: true},
|
||||
urls: [
|
||||
'/foo/script-1.js',
|
||||
'/foo/script-2.js',
|
||||
],
|
||||
patterns: [],
|
||||
},
|
||||
{
|
||||
name: 'group-bar',
|
||||
installMode: 'prefetch',
|
||||
updateMode: 'prefetch',
|
||||
cacheQueryOptions: {ignoreVary: true},
|
||||
urls: [
|
||||
'/bar/script-3.js',
|
||||
'/bar/script-4.js',
|
||||
],
|
||||
patterns: [],
|
||||
},
|
||||
{
|
||||
name: 'group-fallback',
|
||||
installMode: 'prefetch',
|
||||
updateMode: 'prefetch',
|
||||
cacheQueryOptions: {ignoreVary: true},
|
||||
urls: [
|
||||
'/qux/script-5.js',
|
||||
],
|
||||
patterns: [],
|
||||
},
|
||||
],
|
||||
dataGroups: [],
|
||||
hashTable: {
|
||||
'/bar/script-3.js': 'bc0a9b488b5707757c491ddac66f56304310b6b1',
|
||||
'/bar/script-4.js': 'b7782e97a285f1f6e62feca842384babaa209040',
|
||||
'/foo/script-1.js': '3cf257d7ef7e991898f8506fd408cab4f0c2de91',
|
||||
'/foo/script-2.js': '9de2ba54065bb9d610bce51beec62e35bea870a7',
|
||||
'/qux/script-5.js': '3dceafdc0a1b429718e45fbf8e3005dd767892de'
|
||||
},
|
||||
navigationUrls: [
|
||||
{positive: true, regex: '^\\/.*$'},
|
||||
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*\\.[^/]*$'},
|
||||
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*$'},
|
||||
{positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*\\/.*$'},
|
||||
],
|
||||
navigationRequestStrategy: 'performance',
|
||||
});
|
||||
});
|
||||
|
||||
it('uses default `navigationUrls` if not provided', async () => {
|
||||
const fs = new MockFilesystem({
|
||||
'/index.html': 'This is a test',
|
||||
|
|
|
|||
Loading…
Reference in a new issue