Explicitly check the Date: header of a cached response when maxAge is…

… set (#206)

* Explicitly check the Date: header of a cached response when maxAge is set

* Linting.

* Linting.

* Review feedback.

* Update testing dependencies

* Updates for new selenium-assistant syntax

* One more try at updating selenium-assistant syntax

* Update another changed path to sw-testing-helpers

lib/helpers.js

@@ -171,6 +171,33 @@ function validatePrecacheInput(items) {
   return items;
 }
 
+function isResponseFresh(response, maxAgeSeconds, now) {
+  // If we don't have a response, then it's not fresh.
+  if (!response) {
+    return false;
+  }
+
+  // Only bother checking the age of the response if maxAgeSeconds is set.
+  if (maxAgeSeconds) {
+    var dateHeader = response.headers.get('date');
+    // If there's no Date: header, then fall through and return true.
+    if (dateHeader) {
+      var parsedDate = new Date(dateHeader);
+      // If the Date: header was invalid for some reason, parsedDate.getTime()
+      // will return NaN, and the comparison will always be false. That means
+      // that an invalid date will be treated as if the response is fresh.
+      if ((parsedDate.getTime() + (maxAgeSeconds * 1000)) < now) {
+        // Only return false if all the other conditions are met.
+        return false;
+      }
+    }
+  }
+
+  // Fall back on returning true by default, to match the previous behavior in
+  // which we never bothered checking to see whether the response was fresh.
+  return true;
+}
+
 module.exports = {
   debug: debug,
   fetchAndCache: fetchAndCache,
@@ -179,5 +206,6 @@ module.exports = {
   cache: cache,
   uncache: uncache,
   precache: precache,
-  validatePrecacheInput: validatePrecacheInput
+  validatePrecacheInput: validatePrecacheInput,
+  isResponseFresh: isResponseFresh
 };

lib/strategies/cacheFirst.js

@@ -14,13 +14,17 @@
 	limitations under the License.
 */
 'use strict';
+var globalOptions = require('../options');
 var helpers = require('../helpers');
 
 function cacheFirst(request, values, options) {
+  options = options || {};
   helpers.debug('Strategy: cache first [' + request.url + ']', options);
   return helpers.openCache(options).then(function(cache) {
     return cache.match(request).then(function(response) {
-      if (response) {
+      var cacheOptions = options.cache || globalOptions.cache;
+      var now = Date.now();
+      if (helpers.isResponseFresh(response, cacheOptions.maxAgeSeconds, now)) {
         return response;
       }
 

lib/strategies/cacheOnly.js

@@ -14,12 +14,22 @@
 	limitations under the License.
 */
 'use strict';
+var globalOptions = require('../options');
 var helpers = require('../helpers');
 
 function cacheOnly(request, values, options) {
+  options = options || {};
   helpers.debug('Strategy: cache only [' + request.url + ']', options);
   return helpers.openCache(options).then(function(cache) {
-    return cache.match(request);
+    return cache.match(request).then(function(response) {
+      var cacheOptions = options.cache || globalOptions.cache;
+      var now = Date.now();
+      if (helpers.isResponseFresh(response, cacheOptions.maxAgeSeconds, now)) {
+        return response;
+      }
+
+      return undefined;
+    });
   });
 }
 

lib/strategies/networkFirst.js

@@ -36,11 +36,14 @@ function networkFirst(request, values, options) {
       var cacheWhenTimedOutPromise = new Promise(function(resolve) {
         timeoutId = setTimeout(function() {
           cache.match(request).then(function(response) {
-            if (response) {
-              // Only resolve this promise if there's a valid response in the
-              // cache. This ensures that we won't time out a network request
-              // unless there's a cached entry to fallback on, which is arguably
-              // the preferable behavior.
+            var cacheOptions = options.cache || globalOptions.cache;
+            // Only resolve this promise if there's a valid response in the
+            // cache. This ensures that we won't time out a network request
+            // unless there's a cached entry to fallback on, which is arguably
+            // the preferable behavior.
+            var now = Date.now();
+            var maxAgeSeconds = cacheOptions.maxAgeSeconds;
+            if (helpers.isResponseFresh(response, maxAgeSeconds, now)) {
               resolve(response);
             }
           });

package.json

@@ -20,12 +20,12 @@
   "devDependencies": {
     "browserify": "^13.1.0",
     "chai": "^3.4.1",
-    "chromedriver": "^2.24.1",
+    "chromedriver": "^2.27.2",
     "cookie-parser": "^1.4.1",
     "eslint": "^1.10.3",
     "eslint-config-google": "^0.3.0",
     "express": "^4.13.3",
-    "geckodriver": "^1.1.2",
+    "geckodriver": "^1.3.0",
     "gulp": "^3.9.0",
     "gulp-eslint": "^1.1.1",
     "gulp-gh-pages": "^0.5.4",
@@ -35,12 +35,12 @@
     "jsdoc": "^3.4.0",
     "jshint-stylish": "^2.1.0",
     "mocha": "^2.3.4",
-    "npm-publish-scripts": "^2.0.7",
-    "operadriver": "^0.2.2",
+    "npm-publish-scripts": "^3.0.14",
+    "operadriver": "^1.0.0",
     "qunitjs": "^1.20.0",
-    "selenium-assistant": "^1.0.0",
+    "selenium-assistant": "^5.0.2",
     "selenium-webdriver": "^3.0.0-beta-2",
-    "sw-testing-helpers": "0.1.4",
+    "sw-testing-helpers": "^1.0.1",
     "temp": "^0.8.3",
     "vinyl-buffer": "^1.0.0",
     "vinyl-source-stream": "^1.1.0",

test/automated-suite.js

@@ -83,7 +83,7 @@ describe('Test SW-Toolbox', function() {
       .then(testResults => {
         if (testResults.failed.length > 0) {
           const errorMessage = mochaUtils.prettyPrintErrors(
-            browserInfo.prettyName,
+            browserInfo.getPrettyName(),
             testResults
           );
 
@@ -95,35 +95,35 @@ describe('Test SW-Toolbox', function() {
 
   seleniumAssistant.printAvailableBrowserInfo();
 
-  const automatedBrowsers = seleniumAssistant.getAvailableBrowsers();
+  const automatedBrowsers = seleniumAssistant.getLocalBrowsers();
   automatedBrowsers.forEach(browserInfo => {
     if (process.env.TRAVIS || process.env.RELEASE_SCRIPT) {
       // Firefox before version 50 have issues that can't be duplicated outside
       // of the selenium test runner.
-      if (browserInfo.getSeleniumBrowserId() === 'firefox' &&
+      if (browserInfo.getId() === 'firefox' &&
         browserInfo.getVersionNumber() <= 50) {
         console.log('Skipping ' + browserInfo.getRawVersionString());
         return;
       }
 
-      if (browserInfo.getSeleniumBrowserId() === 'opera' &&
+      if (browserInfo.getId() === 'opera' &&
         browserInfo.getVersionNumber() <= 39) {
         console.log('Skipping ' + browserInfo.getRawVersionString());
         return;
       }
 
       // Chrome 54 is having some issues with the selenium :(
-      if (browserInfo.getSeleniumBrowserId() === 'chrome' &&
+      if (browserInfo.getId() === 'chrome' &&
         browserInfo.getVersionNumber() >= 54) {
         console.log('Skipping ' + browserInfo.getRawVersionString());
         return;
       }
 
       // Block browsers w/o Service Worker support from being included in the
       // tests on Travis
-      if (browserInfo.getSeleniumBrowserId() !== 'firefox' &&
-        browserInfo.getSeleniumBrowserId() !== 'chrome' &&
-        browserInfo.getSeleniumBrowserId() !== 'opera') {
+      if (browserInfo.getId() !== 'firefox' &&
+        browserInfo.getId() !== 'chrome' &&
+        browserInfo.getId() !== 'opera') {
         console.log('Not running tests on: ' + browserInfo.getPrettyName());
         return;
       }

test/browser-tests/index.html

@@ -34,8 +34,8 @@
   <script src="/node_modules/mocha/mocha.js"></script>
 
   <!-- sw-testing-helpers -->
-  <script src="/node_modules/sw-testing-helpers/browser/mocha-utils.js"></script>
-  <script src="/node_modules/sw-testing-helpers/browser/sw-utils.js"></script>
+  <script src="/node_modules/sw-testing-helpers/build/browser/mocha-utils.js"></script>
+  <script src="/node_modules/sw-testing-helpers/build/browser/sw-utils.js"></script>
 
   <!--
     Timeout is extended to ensure tests for max-cache-age

test/helpers/download-browsers.js

@@ -1,12 +1,12 @@
 const seleniumAssistant = require('selenium-assistant');
 
 const promises = [
-  seleniumAssistant.downloadBrowser('chrome', 'stable', true),
-  seleniumAssistant.downloadBrowser('chrome', 'beta', true),
-  seleniumAssistant.downloadBrowser('chrome', 'unstable', true),
-  seleniumAssistant.downloadBrowser('firefox', 'stable', true),
-  seleniumAssistant.downloadBrowser('firefox', 'beta', true),
-  seleniumAssistant.downloadBrowser('firefox', 'unstable', true)
+  seleniumAssistant.downloadLocalBrowser('chrome', 'stable', 48),
+  seleniumAssistant.downloadLocalBrowser('chrome', 'beta', 48),
+  seleniumAssistant.downloadLocalBrowser('chrome', 'unstable', 48),
+  seleniumAssistant.downloadLocalBrowser('firefox', 'stable', 48),
+  seleniumAssistant.downloadLocalBrowser('firefox', 'beta', 48),
+  seleniumAssistant.downloadLocalBrowser('firefox', 'unstable', 48)
 ];
 
 Promise.all(promises)