control non amp pages via amp-sw (#28)

* capability for non amp pages * fixing array finding * adding partial * adding a comment for type extension
ampproject · Jul 2, 2019 · 041625a · 041625a
1 parent 15f441b
commit 041625a
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 2 deletions.
diff --git a/src/modules/document-caching/AmpDocumentCachablePlugin.ts b/src/modules/document-caching/AmpDocumentCachablePlugin.ts
@@ -17,9 +17,24 @@
 // @ts-ignore
 import { Plugin } from 'workbox-cache-expiration';
 
+export interface AmpDocumentCachablePluginConfig {
+  maxEntries: Number;
+  maxAgeSeconds: Number;
+  allowedNonAMPPages: Array<RegExp>;
+}
+
 export default class AmpDocumentCachablePlugin extends Plugin {
-  constructor(config: any) {
-    super(config);
+  private allowedPages_: Array<RegExp>;
+
+  constructor(config: Partial<AmpDocumentCachablePluginConfig>) {
+    const { allowedNonAMPPages, ...pluginConfig } = config;
+    super(pluginConfig);
+    if (allowedNonAMPPages) {
+      if (!Array.isArray(allowedNonAMPPages)) {
+        throw new TypeError('allowedNonAMPPages should be an array of RegExp');
+      }
+      this.allowedPages_ = allowedNonAMPPages;
+    }
   }
   async cacheWillUpdate({
     response,
@@ -28,9 +43,22 @@ export default class AmpDocumentCachablePlugin extends Plugin {
   }): Promise<Response | null> {
     const clonedResponse = response.clone();
     const responseContentType = clonedResponse.headers.get('content-type');
+
     // TODO: implement header check as well as it'll be less work.
     if (responseContentType && responseContentType.includes('text/html')) {
       try {
+        // Check if the url qualifies for any explicitly allowed page.
+        if (this.allowedPages_) {
+          const foundUrlInAllowedPages = this.allowedPages_.some(
+            allowedPageRegExp => {
+              return allowedPageRegExp.test(clonedResponse.url);
+            },
+          );
+          if (foundUrlInAllowedPages) {
+            return response;
+          }
+        }
+
         const responseBody = await clonedResponse.text();
         // Check if the response is AMP HTML page, only then cache it.
         if (/<html\s[^>]*(⚡|amp)[^>]*>/.test(responseBody)) {

diff --git a/src/modules/document-caching/index.ts b/src/modules/document-caching/index.ts
@@ -24,12 +24,17 @@ import AmpNavigationRoute from './AmpNavigationRoute';
 import { AmpDocumentNetworkFirst } from './AmpDocumentNetworkFirst';
 import { AmpSwModule } from '../core/AmpSwModule';
 
+/* We cannot extend the following type with the type of AmpDocumentCachablePlugin
+ * as the keys of DocumentCaching and AmpDocumentCachablePlugin are different to
+ * allow more verbose name for amp-sw users.
+ */
 export type DocumentCachingOptions = {
   allowList?: Array<RegExp>;
   denyList?: Array<RegExp>;
   timeoutSeconds?: Number;
   maxDocumentsInCache?: Number;
   maxAgeSecondsforDocumentsInCache?: Number;
+  allowedNonAMPPages?: Array<RegExp>;
 };
 
 export class DocumentCachingModule implements AmpSwModule {
@@ -80,6 +85,7 @@ export class DocumentCachingModule implements AmpSwModule {
               maxAgeSeconds:
                 documentCachingOptions.maxAgeSecondsforDocumentsInCache ||
                 5 * 24 * 60 * 60,
+              allowedNonAMPPages: documentCachingOptions.allowedNonAMPPages,
             }),
           ],
           networkTimeoutSeconds: documentCachingOptions.timeoutSeconds,

diff --git a/test/modules/document-caching/document-caching-test.js b/test/modules/document-caching/document-caching-test.js
@@ -175,6 +175,7 @@ describe('Document caching module', function() {
       );
       expect(cachedData).to.be.null;
     });
+
     it('should cache the current page URL if its AMP page', async () => {
       const generatedSW = await buildSW({}, '/test/dist/amp-sw.js');
       await writeFile(serviceWorkerPath, generatedSW);
@@ -207,5 +208,46 @@ describe('Document caching module', function() {
       );
       expect(cachedData).to.not.be.null;
     });
+
+    it('should allow non AMP pages via config', async () => {
+      const generatedSW = await buildSW(
+        {
+          documentCachingOptions: {
+            allowedNonAMPPages: [/index.html/],
+          },
+        },
+        '/test/dist/amp-sw.js',
+      );
+      await writeFile(serviceWorkerPath, generatedSW);
+      await driver.get('http://localhost:6881/test/alternate.amp.html');
+      await driver.executeAsyncScript(cb => {
+        // install util scripts
+        const cleanupScript = document.createElement('script');
+        cleanupScript.src = '/test/test-utils/sw-test-cleanup.js';
+        document.body.appendChild(cleanupScript);
+        const waitScript = document.createElement('script');
+        waitScript.src = '/test/test-utils/wait-for-sw-state.js';
+        document.body.appendChild(waitScript);
+        cb();
+      });
+      await performCleanupAndWaitForSWActivation(
+        driver,
+        `/${serviceWorkerPath}`,
+        false,
+      );
+      await driver.get('http://localhost:6881/test/index.html');
+      let cachedData = await driver.executeAsyncScript(
+        async (cacheName, cb) => {
+          const cache = await caches.open(cacheName);
+          cb(
+            await cache.match(
+              new Request('http://localhost:6881/test/index.html'),
+            ),
+          );
+        },
+        cacheName,
+      );
+      expect(cachedData).to.not.be.null;
+    });
   });
 });