From d380e93b82bbe203f9ded436db9f3de4be3ab2c4 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 6 Jul 2020 16:55:36 +0300 Subject: [PATCH] refactor(service-worker): move asset URL normalization to `Adapter` (#37922) This is in preparation of enabling the ServiceWorker to handle relative paths in `ngsw.json` (as discussed in #25055), which will require normalizing URLs in other parts of the ServiceWorker. PR Close #37922 --- packages/service-worker/worker/main.ts | 2 +- packages/service-worker/worker/src/adapter.ts | 42 ++++++++++++++++--- packages/service-worker/worker/src/assets.ts | 27 ++---------- .../service-worker/worker/testing/scope.ts | 24 ++++++----- 4 files changed, 54 insertions(+), 41 deletions(-) diff --git a/packages/service-worker/worker/main.ts b/packages/service-worker/worker/main.ts index 58ea288b72..6a600c63f0 100644 --- a/packages/service-worker/worker/main.ts +++ b/packages/service-worker/worker/main.ts @@ -12,5 +12,5 @@ import {Driver} from './src/driver'; const scope = self as any as ServiceWorkerGlobalScope; -const adapter = new Adapter(scope); +const adapter = new Adapter(scope.registration.scope); const driver = new Driver(scope, adapter, new CacheDatabase(scope, adapter)); diff --git a/packages/service-worker/worker/src/adapter.ts b/packages/service-worker/worker/src/adapter.ts index 3edd11498f..cebad8aeef 100644 --- a/packages/service-worker/worker/src/adapter.ts +++ b/packages/service-worker/worker/src/adapter.ts @@ -14,12 +14,18 @@ */ export class Adapter { readonly cacheNamePrefix: string; + private readonly origin: string; - constructor(scope: ServiceWorkerGlobalScope) { - // Suffixing `ngsw` with the baseHref to avoid clash of cache names - // for SWs with different scopes on the same domain. - const baseHref = this.parseUrl(scope.registration.scope).path; - this.cacheNamePrefix = 'ngsw:' + baseHref; + constructor(protected readonly scopeUrl: string) { + const parsedScopeUrl = this.parseUrl(this.scopeUrl); + + // Determine the origin from the registration scope. This is used to differentiate between + // relative and absolute URLs. + this.origin = parsedScopeUrl.origin; + + // Suffixing `ngsw` with the baseHref to avoid clash of cache names for SWs with different + // scopes on the same domain. + this.cacheNamePrefix = 'ngsw:' + parsedScopeUrl.path; } /** @@ -58,7 +64,31 @@ export class Adapter { } /** - * Extract the pathname of a URL. + * Get a normalized representation of a URL such as those found in the ServiceWorker's `ngsw.json` + * configuration. + * + * More specifically: + * 1. Resolve the URL relative to the ServiceWorker's scope. + * 2. If the URL is relative to the ServiceWorker's own origin, then only return the path part. + * Otherwise, return the full URL. + * + * @param url The raw request URL. + * @return A normalized representation of the URL. + */ + normalizeUrl(url: string): string { + // Check the URL's origin against the ServiceWorker's. + const parsed = this.parseUrl(url, this.scopeUrl); + if (parsed.origin === this.origin) { + // The URL is relative to the SW's origin: Return the path only. + return parsed.path; + } else { + // The URL is not relative to the SW's origin: Return the full URL. + return url; + } + } + + /** + * Parse a URL into its different parts, such as `origin`, `path` and `search`. */ parseUrl(url: string, relativeTo?: string): {origin: string, path: string, search: string} { // Workaround a Safari bug, see diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index 1ec7f7cd4e..ecebcbaf16 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -47,9 +47,6 @@ export abstract class AssetGroup { */ protected metadata: Promise; - - private origin: string; - constructor( protected scope: ServiceWorkerGlobalScope, protected adapter: Adapter, protected idle: IdleScheduler, protected config: AssetGroupConfig, @@ -67,10 +64,6 @@ export abstract class AssetGroup { // the timestamp of when it was added to the cache. this.metadata = this.db.open(`${this.prefix}:${this.config.name}:meta`, this.config.cacheQueryOptions); - - // Determine the origin from the registration scope. This is used to differentiate between - // relative and absolute URLs. - this.origin = this.adapter.parseUrl(this.scope.registration.scope).origin; } async cacheStatus(url: string): Promise { @@ -108,7 +101,7 @@ export abstract class AssetGroup { * Process a request for a given resource and return it, or return null if it's not available. */ async handleFetch(req: Request, ctx: Context): Promise { - const url = this.getConfigUrl(req.url); + const url = this.adapter.normalizeUrl(req.url); // Either the request matches one of the known resource URLs, one of the patterns for // dynamically matched URLs, or neither. Determine which is the case for this request in // order to decide how to handle it. @@ -156,18 +149,6 @@ export abstract class AssetGroup { } } - private getConfigUrl(url: string): string { - // If the URL is relative to the SW's own origin, then only consider the path relative to - // the domain root. Determine this by checking the URL's origin against the SW's. - const parsed = this.adapter.parseUrl(url, this.scope.registration.scope); - if (parsed.origin === this.origin) { - // The URL is relative to the SW's origin domain. - return parsed.path; - } else { - return url; - } - } - /** * Some resources are cached without a hash, meaning that their expiration is controlled * by HTTP caching headers. Check whether the given request/response pair is still valid @@ -373,7 +354,7 @@ export abstract class AssetGroup { * Load a particular asset from the network, accounting for hash validation. */ protected async cacheBustedFetchFromNetwork(req: Request): Promise { - const url = this.getConfigUrl(req.url); + const url = this.adapter.normalizeUrl(req.url); // If a hash is available for this resource, then compare the fetched version with the // canonical hash. Otherwise, the network version will have to be trusted. @@ -456,7 +437,7 @@ export abstract class AssetGroup { */ protected async maybeUpdate(updateFrom: UpdateSource, req: Request, cache: Cache): Promise { - const url = this.getConfigUrl(req.url); + const url = this.adapter.normalizeUrl(req.url); const meta = await this.metadata; // Check if this resource is hashed and already exists in the cache of a prior version. if (this.hashes.has(url)) { @@ -546,7 +527,7 @@ export class PrefetchAssetGroup extends AssetGroup { // First, narrow down the set of resources to those which are handled by this group. // Either it's a known URL, or it matches a given pattern. .filter( - url => this.config.urls.some(cacheUrl => cacheUrl === url) || + url => this.config.urls.indexOf(url) !== -1 || this.patterns.some(pattern => pattern.test(url))) // Finally, process each resource in turn. .reduce(async (previous, url) => { diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index 8cce576d5a..74a8d5604b 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -81,8 +81,7 @@ export class MockClients implements Clients { async claim(): Promise {} } -export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context { - readonly cacheNamePrefix: string; +export class SwTestHarness extends Adapter implements ServiceWorkerGlobalScope, Context { readonly clients = new MockClients(); private eventHandlers = new Map(); private skippedWaiting = true; @@ -98,7 +97,7 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context this.selfMessageQueue.push(msg); }, }, - scope: this.origin, + scope: this.scopeUrl, showNotification: (title: string, options: Object) => { this.notifications.push({title, options}); @@ -125,7 +124,11 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context return url && (typeof url.parse === 'function') && (typeof url.resolve === 'function'); } - time: number; + get time() { + return this.mockTime; + } + + private mockTime = Date.now(); private timers: { at: number, @@ -135,10 +138,8 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context }[] = []; constructor( - private server: MockServerState, readonly caches: MockCacheStorage, private origin: string) { - const baseHref = this.parseUrl(origin).path; - this.cacheNamePrefix = 'ngsw:' + baseHref; - this.time = Date.now(); + private server: MockServerState, readonly caches: MockCacheStorage, scopeUrl: string) { + super(scopeUrl); } async resolveSelfMessages(): Promise { @@ -170,6 +171,7 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context } return skippedWaiting; } + updateServerState(server?: MockServerState): void { this.server = server || EMPTY_SERVER_STATE; } @@ -281,7 +283,7 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context timeout(ms: number): Promise { const promise = new Promise(resolve => { this.timers.push({ - at: this.time + ms, + at: this.mockTime + ms, duration: ms, fn: resolve, fired: false, @@ -296,9 +298,9 @@ export class SwTestHarness implements ServiceWorkerGlobalScope, Adapter, Context } advance(by: number): void { - this.time += by; + this.mockTime += by; this.timers.filter(timer => !timer.fired) - .filter(timer => timer.at <= this.time) + .filter(timer => timer.at <= this.mockTime) .forEach(timer => { timer.fired = true; timer.fn();