From 95ba4071f3522a6eecee06b7a8897ea6fe855b54 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Wed, 5 Oct 2022 16:11:19 -0700 Subject: [PATCH 01/20] Handle/cache offline-capable mode media requests in service worker --- .eslintrc.js | 187 +++++++++++++++ .prettierignore | 1 - app/controllers/offline-controller.js | 78 ++----- app/controllers/transformation-controller.js | 2 +- app/models/config-model.js | 17 ++ package.json | 1 + public/js/src/enketo-webform.js | 6 +- public/js/src/module/application-cache.js | 124 +++++----- public/js/src/module/controller-webform.js | 27 +-- public/js/src/module/form-cache.js | 212 ++---------------- public/js/src/module/media.js | 25 +-- .../src/module/offline-app-worker-partial.js | 120 ---------- public/js/src/module/offline-app-worker.js | 65 ++++++ 13 files changed, 382 insertions(+), 483 deletions(-) create mode 100644 .eslintrc.js delete mode 100644 public/js/src/module/offline-app-worker-partial.js create mode 100644 public/js/src/module/offline-app-worker.js diff --git a/.eslintrc.js b/.eslintrc.js new file mode 100644 index 000000000..97c56528b --- /dev/null +++ b/.eslintrc.js @@ -0,0 +1,187 @@ +const eslintConfigAirbnb = require('eslint-config-airbnb-base/rules/variables'); + +const serviceWorkerGlobals = eslintConfigAirbnb.rules[ + 'no-restricted-globals' +].filter((item) => item.name !== 'self' && item !== 'self'); + +module.exports = { + env: { + es6: true, + browser: true, + node: false, + }, + globals: { + Promise: true, + structuredClone: true, + }, + extends: ['airbnb', 'prettier'], + plugins: ['chai-friendly', 'jsdoc', 'prettier', 'unicorn'], + parserOptions: { + sourceType: 'module', + ecmaVersion: 2021, + }, + settings: { + jsdoc: { + tagNamePreference: { + returns: 'return', + }, + }, + }, + rules: { + 'prettier/prettier': 'error', + 'import/no-unresolved': [ + 'error', + { + ignore: [ + 'enketo/config', + 'enketo/widgets', + 'enketo/translator', + 'enketo/dialog', + 'enketo/file-manager', + ], + }, + ], + + 'react/destructuring-assignment': 'off', + + 'array-callback-return': 'warn', + 'consistent-return': 'warn', + 'global-require': 'warn', + 'import/order': 'warn', + 'import/extensions': 'warn', + 'no-param-reassign': 'warn', + 'no-plusplus': 'warn', + 'no-promise-executor-return': 'warn', + 'no-restricted-globals': 'warn', + 'no-restricted-syntax': 'warn', + 'no-return-assign': 'warn', + 'no-shadow': 'warn', + 'no-underscore-dangle': 'warn', + 'no-unused-expressions': 'warn', + 'no-use-before-define': [ + 'warn', + { + functions: false, + }, + ], + 'prefer-const': 'warn', + 'no-cond-assign': 'warn', + 'no-nested-ternary': 'warn', + 'prefer-destructuring': 'warn', + 'import/no-dynamic-require': 'warn', + 'prefer-promise-reject-errors': 'warn', + }, + overrides: [ + { + files: ['**/*.md'], + parser: 'markdown-eslint-parser', + rules: { + 'prettier/prettier': ['error', { parser: 'markdown' }], + }, + }, + + { + files: [ + 'app.js', + 'app/**/*.js', + '!app/views/**/*.js', + 'tools/redis-repl', + ], + env: { + browser: false, + node: true, + }, + ecmaFeatures: { + modules: false, + }, + }, + + { + files: [ + 'Gruntfile.js', + 'config/build.js', + 'scripts/build.js', + 'test/client/config/karma.conf.js', + 'test/server/**/*.js', + 'tools/**/*.js', + ], + env: { + browser: false, + node: true, + }, + ecmaFeatures: { + modules: false, + }, + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { devDependencies: true }, + ], + }, + }, + + { + files: [ + 'app/views/**/*.js', + 'public/js/src/**/*.js', + 'test/client/**/*.js', + ], + env: { + browser: true, + node: false, + }, + }, + + { + files: ['public/js/src/module/offline-app-worker.js'], + globals: { + self: true, + }, + rules: { + 'no-restricted-globals': serviceWorkerGlobals, + }, + }, + + { + files: ['test/client/**/*.js'], + env: { + mocha: true, + }, + globals: { + expect: true, + sinon: true, + }, + rules: { + 'no-console': 0, + 'import/no-extraneous-dependencies': [ + 'error', + { devDependencies: true }, + ], + }, + }, + + { + files: ['test/server/**/*.js'], + env: { + mocha: true, + }, + globals: { + expect: true, + sinon: true, + }, + rules: { + 'no-console': 0, + }, + }, + + { + files: ['**/*.mjs'], + parser: '@babel/eslint-parser', + parserOptions: { + sourceType: 'module', + ecmaVersion: 2021, + requireConfigFile: false, + }, + }, + ], +}; diff --git a/.prettierignore b/.prettierignore index d1a8c3126..7c09f188e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -2,7 +2,6 @@ .nyc* public/js/build/* -*/offline-app-worker-partial.js */node_modules/* docs/* test-coverage/* diff --git a/app/controllers/offline-controller.js b/app/controllers/offline-controller.js index 6598bd507..44cf9bc35 100644 --- a/app/controllers/offline-controller.js +++ b/app/controllers/offline-controller.js @@ -2,9 +2,6 @@ * @module offline-resources-controller */ -const fs = require('fs'); -const path = require('path'); -const crypto = require('crypto'); const express = require('express'); const router = express.Router(); @@ -15,6 +12,7 @@ const config = require('../models/config-model').server; module.exports = (app) => { app.use(`${app.get('base path')}/`, router); }; + router.get('/x/offline-app-worker.js', (req, res, next) => { if (config['offline enabled'] === false) { const error = new Error( @@ -23,59 +21,25 @@ router.get('/x/offline-app-worker.js', (req, res, next) => { error.status = 404; next(error); } else { - res.set('Content-Type', 'text/javascript').send(getScriptContent()); + // We add as few explicit resources as possible because the offline-app-worker can do this dynamically and that is preferred + // for easier maintenance of the offline launch feature. + const resources = config['themes supported'] + .flatMap((theme) => [ + `${config['base path']}${config['offline path']}/css/theme-${theme}.css`, + `${config['base path']}${config['offline path']}/css/theme-${theme}.print.css`, + ]) + .concat([ + `${config['base path']}${config['offline path']}/images/icon_180x180.png`, + ]); + + const link = resources + .map((resource) => `<${resource}>; rel="prefetch"`) + .join(', '); + + const script = config.offlineWorkerScript; + + res.set('Content-Type', 'text/javascript'); + res.set('Link', link); + res.send(script); } }); - -/** - * Assembles script contentå - */ -function getScriptContent() { - // Determining hash every time, is done to make development less painful (refreshing service worker) - // The partialScriptHash is not actually required but useful to see which offline-app-worker-partial.js is used during troubleshooting. - // by going to http://localhost:8005/x/offline-app-worker.js and comparing the version with the version shown in the side slider of the webform. - const partialOfflineAppWorkerScript = fs.readFileSync( - path.resolve( - config.root, - 'public/js/src/module/offline-app-worker-partial.js' - ), - 'utf8' - ); - const partialScriptHash = crypto - .createHash('md5') - .update(partialOfflineAppWorkerScript) - .digest('hex') - .substring(0, 7); - const configurationHash = crypto - .createHash('md5') - .update(JSON.stringify(config)) - .digest('hex') - .substring(0, 7); - const version = [config.version, configurationHash, partialScriptHash].join( - '-' - ); - // We add as few explicit resources as possible because the offline-app-worker can do this dynamically and that is preferred - // for easier maintenance of the offline launch feature. - const resources = config['themes supported'] - .reduce((accumulator, theme) => { - accumulator.push( - `${config['base path']}${config['offline path']}/css/theme-${theme}.css` - ); - accumulator.push( - `${config['base path']}${config['offline path']}/css/theme-${theme}.print.css` - ); - - return accumulator; - }, []) - .concat([ - `${config['base path']}${config['offline path']}/images/icon_180x180.png`, - ]); - - return ` -const version = '${version}'; -const resources = [ - '${resources.join("',\n '")}' -]; - -${partialOfflineAppWorkerScript}`; -} diff --git a/app/controllers/transformation-controller.js b/app/controllers/transformation-controller.js index 42f7a6b7b..c93cd0641 100644 --- a/app/controllers/transformation-controller.js +++ b/app/controllers/transformation-controller.js @@ -244,7 +244,7 @@ function _respond(res, survey) { * @return { string } - a hash */ function _getCombinedHash(survey) { - const FORCE_UPDATE = 1; + const FORCE_UPDATE = 2; const brandingHash = survey.account.branding && survey.account.branding.source ? utils.md5(survey.account.branding.source) diff --git a/app/models/config-model.js b/app/models/config-model.js index d5e29eafd..8f4c75e03 100644 --- a/app/models/config-model.js +++ b/app/models/config-model.js @@ -8,6 +8,7 @@ const mergeWith = require('lodash/mergeWith'); const path = require('path'); const fs = require('fs'); const url = require('url'); +const crypto = require('crypto'); const themePath = path.join(__dirname, '../../public/css'); const languagePath = path.join(__dirname, '../../locales/src'); @@ -342,6 +343,21 @@ if (config['id length'] < 4) { config['id length'] = 31; } +const offlineWorkerScript = fs.readFileSync( + path.resolve(config.root, 'public/js/src/module/offline-app-worker.js'), + 'utf8' +); + +const hashSource = Buffer.from([offlineWorkerScript, JSON.stringify(config)]); +const hash = crypto + .createHash('md5') + .update(hashSource) + .digest('hex') + .substring(0, 7); + +config.offlineVersion = [config.version, hash].join('-'); +config.offlineWorkerScript = offlineWorkerScript; + module.exports = { /** * @type { object } @@ -371,6 +387,7 @@ module.exports = { csrfCookieName: config['csrf cookie name'], excludeNonRelevant: config['exclude non-relevant'], experimentalOptimizations: config['experimental optimizations'], + offlineVersion: config.offlineVersion, }, getThemesSupported, }; diff --git a/package.json b/package.json index 136fed8e6..8bc636028 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "esbuild": "^0.12.29", "esbuild-plugin-alias": "^0.1.2", "eslint-config-airbnb": "^19.0.4", + "eslint-config-airbnb-base": "^15.0.0", "eslint-config-prettier": "^8.5.0", "eslint-plugin-chai-friendly": "^0.7.2", "eslint-plugin-import": "^2.26.0", diff --git a/public/js/src/enketo-webform.js b/public/js/src/enketo-webform.js index 052f672a4..41116f636 100644 --- a/public/js/src/enketo-webform.js +++ b/public/js/src/enketo-webform.js @@ -44,7 +44,6 @@ if (settings.offline) { return formParts; }) - .then(formCache.updateMedia) .then(_setFormCacheEventHandlers) .catch(_showErrorOrAuthenticate); } else { @@ -167,7 +166,6 @@ function _setEmergencyHandlers() { */ function _addBranding(survey) { const brandImg = document.querySelector('.form-header__branding img'); - const attribute = settings.offline ? 'data-offline-src' : 'src'; if ( brandImg && @@ -175,9 +173,9 @@ function _addBranding(survey) { survey.branding.source && brandImg.src !== survey.branding.source ) { - brandImg.src = ''; - brandImg.setAttribute(attribute, survey.branding.source); + brandImg.src = survey.branding.source; } + brandImg.classList.remove('hide'); return survey; diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index 795d7333d..0a6345b26 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -5,68 +5,82 @@ import events from './event'; import settings from './settings'; -function init(survey) { - if ('serviceWorker' in navigator) { - window.addEventListener('load', () => { - navigator.serviceWorker - .register(`${settings.basePath}/x/offline-app-worker.js`) - .then( - (registration) => { - // Registration was successful - console.log( - 'Offline application service worker registration successful with scope: ', - registration.scope - ); - setInterval(() => { - console.log( - 'Checking for offline application cache service worker update' - ); - registration.update(); - }, 60 * 60 * 1000); - - if (registration.active) { - _reportOfflineLaunchCapable(true); - } - registration.addEventListener('updatefound', () => { - const newWorker = registration.installing; - - newWorker.addEventListener('statechange', () => { - if (newWorker.state === 'activated') { - console.log( - 'New offline application service worker activated!' - ); - document.dispatchEvent( - events.ApplicationUpdated() - ); - } - }); - }); - }, - (err) => { - // registration failed :( - console.error( - 'Offline application service worker registration failed: ', - err - ); - _reportOfflineLaunchCapable(true); +/** + * @typedef {import('../../../../app/models/survey-model').SurveyObject} Survey + */ + +/** + * @param {Survey} survey + */ +const init = async (survey) => { + try { + if ('serviceWorker' in navigator) { + const registration = await navigator.serviceWorker.register( + `${settings.basePath}/x/offline-app-worker.js` + ); + + // Registration was successful + console.log( + 'Offline application service worker registration successful with scope: ', + registration.scope + ); + setInterval(() => { + console.log( + 'Checking for offline application cache service worker update' + ); + registration.update(); + }, 60 * 60 * 1000); + + const currentActive = registration.active; + + if (currentActive != null) { + registration.addEventListener('updatefound', () => { + _reportOfflineLaunchCapable(false); + }); + + navigator.serviceWorker.addEventListener( + 'controllerchange', + () => { + window.location.reload(); } ); - }); - } else { - if (location.protocol.startsWith('http:')) { - console.error( - 'Service workers not supported on this http URL (insecure)' - ); + } + + await registration.update(); + + if (currentActive == null) { + window.location.reload(); + } else { + _reportOfflineLaunchCapable(true); + } } else { - console.error( - 'Service workers not supported on this browser. This form cannot launch online' - ); + if (location.protocol.startsWith('http:')) { + console.error( + 'Service workers not supported on this http URL (insecure)' + ); + } else { + console.error( + 'Service workers not supported on this browser. This form cannot launch online' + ); + } + + _reportOfflineLaunchCapable(false); } + } catch (error) { + // registration failed :( + const registrationError = Error( + `Offline application service worker registration failed: ${error.message}` + ); + + registrationError.stack = error.stack; + _reportOfflineLaunchCapable(false); + + throw registrationError; } - return Promise.resolve(survey); -} + return survey; +}; function _reportOfflineLaunchCapable(capable = true) { document.dispatchEvent(events.OfflineLaunchCapable({ capable })); diff --git a/public/js/src/module/controller-webform.js b/public/js/src/module/controller-webform.js index 42d7d3a66..ebeab1dc9 100644 --- a/public/js/src/module/controller-webform.js +++ b/public/js/src/module/controller-webform.js @@ -18,7 +18,6 @@ import { } from './translator'; import records from './records-queue'; import encryptor from './encryptor'; -import formCache from './form-cache'; import { getLastSavedRecord, populateLastSavedInstances } from './last-saved'; import { replaceMediaSources, replaceModelMediaSources } from './media'; @@ -205,25 +204,7 @@ function _checkAutoSavedRecord() { * @property {boolean} [isOffline] */ -/** - * Controller function to reset to the initial state of a form. - * - * Note: Previously this function accepted a boolean `confirmed` parameter, presumably - * intending to block the reset behavior until user confirmation of save. But this - * parameter was always passed as `true`. Relatedly, the `FormReset` event fired here - * previously indirectly triggered `formCache.updateMedia` method, but it was triggered - * with stale `survey` state, overwriting any changes to `survey.externalData` - * referencing last-saved instances. - * - * That event listener has been removed in favor of calling `updateMedia` directly with - * the current state of `survey` in offline mode. This change is being called out in - * case the removal of that event listener impacts downstream forks. - * - * @param {Survey} survey - * @param {ResetFormOptions} [options] - * @return {Promise} - */ -function _resetForm(survey, options = {}) { +function _resetForm(survey) { return getLastSavedRecord(survey.enketoId) .then((lastSavedRecord) => populateLastSavedInstances(survey, lastSavedRecord) @@ -246,10 +227,6 @@ function _resetForm(survey, options = {}) { form.view.html.dispatchEvent(events.FormReset()); - if (options.isOffline) { - formCache.updateMedia(survey); - } - if (records) { records.setActive(null); } @@ -308,8 +285,6 @@ function _loadRecord(survey, instanceId, confirmed) { form.view.html.dispatchEvent(events.FormReset()); - formCache.updateMedia(survey); - form.recordName = record.name; records.setActive(record.instanceId); diff --git a/public/js/src/module/form-cache.js b/public/js/src/module/form-cache.js index 761d65156..34666103a 100644 --- a/public/js/src/module/form-cache.js +++ b/public/js/src/module/form-cache.js @@ -43,7 +43,7 @@ function init(survey) { return set(survey); }) .then(_processDynamicData) - .then(_setUpdateIntervals); + .then(setUpdateIntervals); } /** @@ -66,16 +66,6 @@ function get({ enketoId }) { ); } -/** - * @param {Survey} survey - * @return {Promise} - */ -function prepareOfflineSurvey(survey) { - return Promise.resolve(_swapMediaSrc(survey)).then( - _addBinaryDefaultsAndUpdateModel - ); -} - /** * @param {Survey} survey * @return {Promise} @@ -93,7 +83,7 @@ const updateSurveyCache = (survey) => function set(survey) { return connection .getFormParts(survey) - .then(prepareOfflineSurvey) + .then(addBinaryDefaultsAndUpdateModel) .then(store.survey.set); } @@ -175,56 +165,27 @@ function _processDynamicData(survey) { * @param {Survey} survey * @return {Promise} */ -function _setUpdateIntervals(survey) { +const setUpdateIntervals = async (survey) => { hash = survey.hash; // Check for form update upon loading. + await updateCache(survey); + // Note that for large Xforms where the XSL transformation takes more than 30 seconds, // the first update make take 20 minutes to propagate to the browser of the very first user(s) // that open the form right after the XForm update. + setTimeout(() => { - _updateCache(survey); + updateCache(survey); }, CACHE_UPDATE_INITIAL_DELAY); + // check for form update every 20 minutes setInterval(() => { - _updateCache(survey); + updateCache(survey); }, CACHE_UPDATE_INTERVAL); - return Promise.resolve(survey); -} - -/** - * Handles loading form media for newly added repeats. - * - * @param { Survey } survey - survey object - * @return { Promise } - */ -function _setRepeatListener(survey) { - // Instantiate only once, after loadMedia has been completed (once) - document - .querySelector('form.or') - .addEventListener(events.AddRepeat().type, (event) => { - _loadMedia(survey, [event.target]); - }); - - return Promise.resolve(survey); -} - -/** - * Changes src attributes in view to data-offline-src to facilitate loading those resources - * from the browser storage. - * - * @param { Survey } survey - survey object - * @return { Survey } - */ -function _swapMediaSrc(survey) { - survey.form = survey.form.replace( - /(src="[^"]*")/g, - 'data-offline-$1 src=""' - ); - return survey; -} +}; /** * Loads all default binary files and adds them to the survey object. It removes the src @@ -233,7 +194,7 @@ function _swapMediaSrc(survey) { * @param { Survey } survey - survey object * @return { Promise } */ -function _addBinaryDefaultsAndUpdateModel(survey) { +function addBinaryDefaultsAndUpdateModel(survey) { // The mechanism for default binary files is as follows: // 1. They are stored as binaryDefaults in the resources table with the key being comprised of the VALUE (i.e. jr:// url) // 2. Filemanager.getFileUrl will determine whether to load from (survey) resources of (record) files @@ -300,157 +261,11 @@ function updateMaxSubmissionSize(survey) { return Promise.resolve(survey); } -/** - * Loads survey resources either from the store or via HTTP (and stores them). - * - * @param { Survey } survey - survey object - * @return { Promise } - */ -function updateMedia(survey) { - const formElement = document.querySelector('form.or'); - - replaceMediaSources(formElement, survey.media, { - isOffline: true, - }); - - const containers = [formElement]; - const formHeader = document.querySelector('.form-header'); - if (formHeader) { - containers.push(formHeader); - } - - return _loadMedia(survey, containers) - .then((resources) => { - // if all resources were already in the database, _loadMedia returned undefined - if (resources) { - // Filter out the failed requests (undefined) - survey.resources = resources.filter((resource) => !!resource); - - // Store any resources that are now available for this form. - console.log('Survey media has been updated. Updating cache.'); - return updateSurveyCache(survey); - } - return survey; - }) - .then(_setRepeatListener) - .catch((error) => { - console.error('updateMedia failed', error); - - // Let the flow continue. - return survey; - }); -} - -/** - * To be used with Promise.all if you want the results to be returned even if some - * have failed. Failed tasks will return undefined. - * - * @param { Promise } task - [description] - * @return { object } [description] - */ -function _reflect(task) { - return task.then( - (response) => response, - (error) => { - console.error(error); - } - ); -} - -/** - * @typedef Resource - * @property {string} url URL to resource - * @property {Blob} item resource as Blob - */ - -/** - * Loads and displays survey resources either from the store or via HTTP. - * - * @param { Survey } survey - survey object - * @param { [Element]} targetContainers - HTML container elements to load media into - * @return { Promise<[Resource] | undefined> } - */ -function _loadMedia(survey, targetContainers) { - let updated = false; - - const requests = []; - _getElementsGroupedBySrc(targetContainers).forEach((elements) => { - const src = elements[0].dataset.offlineSrc; - - const request = store.survey.resource - .get(survey.enketoId, src) - .then(async (resource) => { - if (!resource || !resource.item) { - // no need to try/catch here as we don't care about catching failures - const downloaded = await connection.getMediaFile(src); - // only when successful - updated = true; - return downloaded; - } - return resource; - }) - // render the media - .then((resource) => { - if (resource.item) { - // create a resourceURL - const resourceUrl = URL.createObjectURL(resource.item); - // add this resourceURL as the src for all elements in the group - elements.forEach((element) => { - element.src = resourceUrl; - }); - } - return resource; - }) - .catch((error) => { - console.error(error); - }); - - requests.push(request); - }); - - return Promise.all(requests.map(_reflect)).then((resources) => { - if (updated) { - return resources; - } - }); -} - -function _getElementsGroupedBySrc(containers) { - const groupedElements = []; - const urls = {}; - let els = []; - - containers.forEach( - (container) => - (els = els.concat([ - ...container.querySelectorAll('[data-offline-src]'), - ])) - ); - - els.forEach((el) => { - if (!urls[el.dataset.offlineSrc]) { - const src = el.dataset.offlineSrc; - const group = els.filter((e) => { - if (e.dataset.offlineSrc === src) { - // remove from $els to improve performance - // els = els.filter( es => !es.matches( `[data-offline-src="${src}"]` ) ); - return true; - } - }); - - urls[src] = true; - groupedElements.push(group); - } - }); - - return groupedElements; -} - /** * @param {Survey} survey * @return {Promise} */ -function _updateCache(survey) { +function updateCache(survey) { console.log('Checking for survey update...'); return connection @@ -474,7 +289,7 @@ function _updateCache(survey) { return formParts; }) - .then(prepareOfflineSurvey) + .then(addBinaryDefaultsAndUpdateModel) .then(updateSurveyCache) .then((result) => { // set the hash so that subsequent update checks won't redownload the form @@ -536,7 +351,6 @@ export default { init, get, updateMaxSubmissionSize, - updateMedia, remove, flush, CACHE_UPDATE_INITIAL_DELAY, diff --git a/public/js/src/module/media.js b/public/js/src/module/media.js index 7f4aba1eb..88b450177 100644 --- a/public/js/src/module/media.js +++ b/public/js/src/module/media.js @@ -1,31 +1,20 @@ -/** - * @typedef ReplaceMediaOptions - * @property {boolean} isOffline - */ - /** * @param {Element} rootElement * @param {Record} [media] - * @param {ReplaceMediaOptions} [options] */ -export const replaceMediaSources = (rootElement, media = {}, options = {}) => { - const sourceElements = rootElement.querySelectorAll( - '[src^="jr:"], [data-offline-src^="jr:"]' - ); +export const replaceMediaSources = (rootElement, media = {}) => { + const sourceElements = rootElement.querySelectorAll('[src^="jr:"]'); const isHTML = rootElement instanceof HTMLElement; sourceElements.forEach((element) => { - const offlineSrc = isHTML ? element.dataset.offlineSrc : null; const source = ( - isHTML ? offlineSrc ?? element.src : element.getAttribute('src') + isHTML ? element.src : element.getAttribute('src') )?.trim(); const fileName = source.replace(/.*\/([^/]+)$/, '$1'); const replacement = media[fileName]; if (replacement != null) { - if (offlineSrc != null) { - element.dataset.offlineSrc = replacement; - } else if (isHTML) { + if (isHTML) { element.src = replacement; } else { element.setAttribute('src', replacement); @@ -42,11 +31,7 @@ export const replaceMediaSources = (rootElement, media = {}, options = {}) => { if (formLogoContainer.firstElementChild == null) { const formLogoImg = document.createElement('img'); - if (options.isOffline) { - formLogoImg.dataset.offlineSrc = formLogoURL; - } else { - formLogoImg.src = formLogoURL; - } + formLogoImg.src = formLogoURL; formLogoImg.alt = 'form logo'; formLogoContainer.append(formLogoImg); diff --git a/public/js/src/module/offline-app-worker-partial.js b/public/js/src/module/offline-app-worker-partial.js deleted file mode 100644 index a3d76eb29..000000000 --- a/public/js/src/module/offline-app-worker-partial.js +++ /dev/null @@ -1,120 +0,0 @@ -/** - * The version, resources variables above are dynamically prepended by the offline-controller. - */ - -const CACHES = [`enketo-common_${version}`]; - -self.addEventListener('install', (event) => { - self.skipWaiting(); - // Perform install steps - event.waitUntil( - caches - .open(CACHES[0]) - .then((cache) => { - console.log('Opened cache'); - - // To bypass any HTTP caching, always obtain resource from network - return cache.addAll( - resources.map( - (resource) => new Request(resource, { cache: 'reload' }) - ) - ); - }) - .catch((e) => { - console.log('Service worker install error', e); - }) - ); -}); - -self.addEventListener('activate', (event) => { - console.log('activated!'); - // Delete old resource caches - event.waitUntil( - caches - .keys() - .then((keys) => - Promise.all( - keys.map((key) => { - if (!CACHES.includes(key)) { - console.log('deleting cache', key); - - return caches.delete(key); - } - }) - ) - ) - .then(() => { - console.log(`${version} now ready to handle fetches!`); - }) - ); -}); - -self.addEventListener('fetch', (event) => { - event.respondWith( - caches.match(event.request).then((response) => { - if (response) { - console.log('returning cached response for', event.request.url); - - return response; - } - - // TODO: we have a fallback page we could serve when offline, but tbc if that is actually useful at all - - // To bypass any HTTP caching, always obtain resource from network - return fetch(event.request, { - credentials: 'same-origin', - cache: 'reload', - }) - .then((response) => { - const isScopedResource = event.request.url.includes('/x/'); - const isTranslation = - event.request.url.includes('/locales/build/'); - const isServiceWorkerScript = - event.request.url === self.location.href; - - // The second clause prevents confusing logging when opening the service worker directly in a separate tab. - if ( - isScopedResource && - !isServiceWorkerScript && - !isTranslation - ) { - console.error( - 'Resource missing from cache?', - event.request.url - ); - } - - // Check if we received a valid response - if ( - !response || - response.status !== 200 || - response.type !== 'basic' || - !isScopedResource || - isServiceWorkerScript - ) { - return response; - } - - // Cache any additional loaded languages - const responseToCache = response.clone(); - - // Cache any non-English language files that are requested - // Also, if the cache didn't get built correctly using the explicit resources list, - // just cache whatever is scoped dynamically to self-heal the cache. - caches.open(CACHES[0]).then((cache) => { - console.log( - 'Dynamically adding resource to cache', - event.request.url - ); - cache.put(event.request, responseToCache); - }); - - return response; - }) - .catch((e) => { - // Let fail silently - console.log('Failed to fetch resource', event.request, e); - }); - }) - ); -}); diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js new file mode 100644 index 000000000..b05fd4a5f --- /dev/null +++ b/public/js/src/module/offline-app-worker.js @@ -0,0 +1,65 @@ +const CACHES = [`enketo-common`]; + +self.addEventListener('install', () => { + self.skipWaiting(); +}); + +/** + * @param {RequestInfo} request + * @param {RequestInit} options + */ +const tryFetch = async (request, options) => { + try { + return await fetch(request, options); + } catch (error) { + return new Response(JSON.stringify({ message: error.message }), { + status: 500, + statusText: 'Internal Server Error', + headers: { + 'content-type': 'application/json', + }, + }); + } +}; + +/** + * @param {Request} request + */ +const onFetch = async (request) => { + const [{ value: response }, { value: cached }] = await Promise.allSettled([ + tryFetch(request, { + credentials: 'same-origin', + cache: 'reload', + }), + caches.match(request), + ]); + + if ( + cached != null && + (response == null || + response.status !== 200 || + response.type !== 'basic') + ) { + return cached; + } + + if (response == null || response.status !== 200) { + return response; + } + + // Cache any additional loaded languages + const responseToCache = response.clone(); + + // Cache any non-English language files that are requested + // Also, if the cache didn't get built correctly using the explicit resources list, + // just cache whatever is scoped dynamically to self-heal the cache. + caches.open(CACHES[0]).then((cache) => { + cache.put(request, responseToCache); + }); + + return response; +}; + +self.addEventListener('fetch', (event) => { + event.respondWith(onFetch(event.request)); +}); From c94eef611829175672891e5aa341187a00128311 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Thu, 6 Oct 2022 12:13:59 -0700 Subject: [PATCH 02/20] Remove outdated tests for offline media --- test/client/form-cache.spec.js | 126 --------------------------------- test/client/media.spec.js | 16 +---- 2 files changed, 3 insertions(+), 139 deletions(-) diff --git a/test/client/form-cache.spec.js b/test/client/form-cache.spec.js index 160f3575b..04c314abd 100644 --- a/test/client/form-cache.spec.js +++ b/test/client/form-cache.spec.js @@ -60,9 +60,6 @@ describe('Client Form Cache', () => { /** @type {GetFormPartsStubResult} */ let getFormPartsStubResult; - /** @type {SinonStub} */ - let getFileSpy; - /** @type {SinonFakeTimers} */ let timers; @@ -118,15 +115,6 @@ describe('Client Form Cache', () => { }) ); - getFileSpy = sandbox.stub(connection, 'getMediaFile').callsFake((url) => - Promise.resolve({ - url, - item: new Blob(['babdf'], { - type: 'image/png', - }), - }) - ); - store.init().then(done, done); }); @@ -156,26 +144,6 @@ describe('Client Form Cache', () => { .then(done, done); }); - it('will call connection.getMediaFile to obtain form resources', (done) => { - survey.enketoId = '20'; - formCache - .init(survey) - .then((result) => { - const currentForm = document.querySelector('form.or'); - const form = document - .createRange() - .createContextualFragment(result.form); - - currentForm.parentNode.replaceChild(form, currentForm); - - return formCache.updateMedia(result); - }) - .then(() => { - expect(getFileSpy).to.have.been.calledWith(url1); - }) - .then(done, done); - }); - it('will populate the cache upon initialization', (done) => { survey.enketoId = '30'; formCache @@ -196,18 +164,6 @@ describe('Client Form Cache', () => { }) .then(done, done); }); - - it('will empty src attributes and copy the original value to a data-offline-src attribute ', (done) => { - survey.enketoId = '40'; - formCache - .init(survey) - .then((result) => { - expect(result.form) - .to.contain('src=""') - .and.to.contain(`data-offline-src="${url1}"`); - }) - .then(done, done); - }); }); describe('form cache updates', () => { @@ -275,87 +231,5 @@ describe('Client Form Cache', () => { }) .then(done, done); }); - - describe('form media (only) cache updates', () => { - let resultSurvey; - - /** @type {SinonSpy} */ - let storeSurveyUpdateSpy; - - beforeEach((done) => { - getFileSpy.restore(); - getFileSpy = sandbox - .stub(connection, 'getMediaFile') - .callsFake(() => Promise.reject(new Error('Fail!'))); - - storeSurveyUpdateSpy = sandbox.spy(store.survey, 'update'); - - survey.enketoId = '200'; - formCache - .init(survey) - .then((result) => { - const currentForm = document.querySelector('form.or'); - const form = document - .createRange() - .createContextualFragment(result.form); - - currentForm.parentNode.replaceChild(form, currentForm); - - resultSurvey = result; - return formCache.updateMedia(result); - }) - .then(() => { - getFileSpy.restore(); - getFileSpy = sandbox - .stub(connection, 'getMediaFile') - .callsFake((url) => - Promise.resolve({ - url, - item: new Blob(['babdf'], { - type: 'image/png', - }), - }) - ); - }) - .then(done, done); - }); - - afterEach(() => { - storeSurveyUpdateSpy.restore(); - getFileSpy.restore(); - }); - - it('will re-attempt to download failed media files (at next load) and update the cache', (done) => { - expect(getFileSpy).to.not.have.been.called; - expect(storeSurveyUpdateSpy).to.not.have.been.called; - - // simulate re-opening a cached form by calling updateMedia again - formCache - .updateMedia(resultSurvey) - .then(() => { - // another attempt is made to download the previously-failed media file - expect(getFileSpy).to.have.been.calledOnce; - expect(getFileSpy).to.have.been.calledWith(url1); - // and to cache it when successful - expect(storeSurveyUpdateSpy).to.have.been.calledOnce; - }) - .then(done, done); - }); - - it('will not re-attempt to download and update again after the cache is complete', (done) => { - // simulate re-opening a cached form by calling updateMedia again - formCache - .updateMedia(resultSurvey) - .then(formCache.updateMedia) - .then(formCache.updateMedia) - .then(() => { - // Despite 3 calls the media file was only downloaded once, - // and the cache was updated only once. - expect(getFileSpy).to.have.been.calledOnce; - expect(storeSurveyUpdateSpy).to.have.been.calledOnce; - }) - .then(done, done); - }); - }); }); }); diff --git a/test/client/media.spec.js b/test/client/media.spec.js index a746708ba..e15ff6616 100644 --- a/test/client/media.spec.js +++ b/test/client/media.spec.js @@ -97,7 +97,7 @@ describe('Media replacement', () => { ); }); - it('replaces jr: URLs in a form with sources swapped for offline-capable mode', () => { + it('replaces jr: URLs in a form with sources previously swapped for offline-capable mode', () => { const sourceElements = formRoot.querySelectorAll('[src]'); sourceElements.forEach((element) => { @@ -109,10 +109,10 @@ describe('Media replacement', () => { const img = formRoot.querySelector('label img'); const audio = formRoot.querySelector('audio'); - expect(img.dataset.offlineSrc).to.equal( + expect(img.src).to.equal( 'https://example.com/-/media/get/0/WXMDbc0H/c0f15ee04dacb1db7cc60797285ff1c8/an%20image.jpg' ); - expect(audio.dataset.offlineSrc).to.equal( + expect(audio.src).to.equal( 'https://example.com/-/media/get/0/WXMDbc0H/c0f15ee04dacb1db7cc60797285ff1c8/a%20song.mp3' ); }); @@ -127,16 +127,6 @@ describe('Media replacement', () => { ); }); - it('appends a form logo with an offline source attribute if present in the media mapping', () => { - replaceMediaSources(formRoot, media, { isOffline: true }); - - const formLogo = formRoot.querySelector('.form-logo img'); - - expect(formLogo.dataset.offlineSrc).to.equal( - 'https://example.com/-/media/get/0/WXMDbc0H/c0f15ee04dacb1db7cc60797285ff1c8/form_logo.png' - ); - }); - it('replaces jr: URLs in `src` attributes in a model when the `modelRoot` property is set', () => { const enketoForm = { model: {}, From d8ebca99b7dc1d68b9b89c482074e503460a71a6 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Thu, 6 Oct 2022 14:59:22 -0700 Subject: [PATCH 03/20] Handle previously cached forms with data-offline-src attributes --- app/controllers/transformation-controller.js | 2 +- public/js/src/module/media.js | 21 +++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/controllers/transformation-controller.js b/app/controllers/transformation-controller.js index c93cd0641..42f7a6b7b 100644 --- a/app/controllers/transformation-controller.js +++ b/app/controllers/transformation-controller.js @@ -244,7 +244,7 @@ function _respond(res, survey) { * @return { string } - a hash */ function _getCombinedHash(survey) { - const FORCE_UPDATE = 2; + const FORCE_UPDATE = 1; const brandingHash = survey.account.branding && survey.account.branding.source ? utils.md5(survey.account.branding.source) diff --git a/public/js/src/module/media.js b/public/js/src/module/media.js index 88b450177..96eed7db0 100644 --- a/public/js/src/module/media.js +++ b/public/js/src/module/media.js @@ -1,11 +1,30 @@ +/** + * This is a stopgap measure to support forms previously cached with + * `data-offline-src` attributes. It can be removed when forms are + * loaded by the service worker. + * + * @param {HTMLElement} rootElement + */ +const reviveOfflineMediaSources = (rootElement) => { + rootElement.querySelectorAll('[data-offline-src]').forEach((element) => { + element.src = element.dataset.offlineSrc; + delete element.dataset.offlineSrc; + }); +}; + /** * @param {Element} rootElement * @param {Record} [media] */ export const replaceMediaSources = (rootElement, media = {}) => { - const sourceElements = rootElement.querySelectorAll('[src^="jr:"]'); const isHTML = rootElement instanceof HTMLElement; + if (isHTML) { + reviveOfflineMediaSources(rootElement); + } + + const sourceElements = rootElement.querySelectorAll('[src^="jr:"]'); + sourceElements.forEach((element) => { const source = ( isHTML ? element.src : element.getAttribute('src') From c58d19980dd9d0c4a585419857d55b9e6dbfddb7 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Thu, 6 Oct 2022 16:06:09 -0700 Subject: [PATCH 04/20] Test applicaiton-cache.js --- public/js/src/module/application-cache.js | 32 ++++- test/client/application-cache.spec.js | 168 ++++++++++++++++++++++ 2 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 test/client/application-cache.spec.js diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index 0a6345b26..c1157e213 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -5,6 +5,28 @@ import events from './event'; import settings from './settings'; +/** + * @private + * + * Used only for mocking `window.reload` in tests. + */ +const location = { + get protocol() { + return window.location.protocol; + }, + + reload() { + window.location.reload(); + }, +}; + +/** + * @private + * + * Exported only for testing. + */ +const UPDATE_REGISTRATION_INTERVAL = 60 * 60 * 1000; + /** * @typedef {import('../../../../app/models/survey-model').SurveyObject} Survey */ @@ -14,7 +36,7 @@ import settings from './settings'; */ const init = async (survey) => { try { - if ('serviceWorker' in navigator) { + if (navigator.serviceWorker != null) { const registration = await navigator.serviceWorker.register( `${settings.basePath}/x/offline-app-worker.js` ); @@ -29,7 +51,7 @@ const init = async (survey) => { 'Checking for offline application cache service worker update' ); registration.update(); - }, 60 * 60 * 1000); + }, UPDATE_REGISTRATION_INTERVAL); const currentActive = registration.active; @@ -41,7 +63,7 @@ const init = async (survey) => { navigator.serviceWorker.addEventListener( 'controllerchange', () => { - window.location.reload(); + location.reload(); } ); } @@ -49,7 +71,7 @@ const init = async (survey) => { await registration.update(); if (currentActive == null) { - window.location.reload(); + location.reload(); } else { _reportOfflineLaunchCapable(true); } @@ -88,6 +110,8 @@ function _reportOfflineLaunchCapable(capable = true) { export default { init, + location, + UPDATE_REGISTRATION_INTERVAL, get serviceWorkerScriptUrl() { if ( 'serviceWorker' in navigator && diff --git a/test/client/application-cache.spec.js b/test/client/application-cache.spec.js new file mode 100644 index 000000000..b01ef7051 --- /dev/null +++ b/test/client/application-cache.spec.js @@ -0,0 +1,168 @@ +import applicationCache from '../../public/js/src/module/application-cache'; +import events from '../../public/js/src/module/event'; +import settings from '../../public/js/src/module/settings'; + +describe('Application Cache', () => { + const basePath = '-'; + const offlineLaunchCapableType = events.OfflineLaunchCapable().type; + + /** @type {ServiceWorker | null} */ + let activeServiceWorker; + + /** @type {sinon.SinonSandbox} */ + let sandbox; + + /** @type {sinon.SinonFakeTimers} */ + let timers; + + /** @type {sinon.SinonFake} */ + let offlineLaunchCapableListener; + + /** @type {sinon.SinonStub} */ + let reloadStub; + + /** @type {sinon.SinonStub} */ + let registrationStub; + + /** @type {sinon.SinonFake} */ + let registrationUpdateFake; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + timers = sandbox.useFakeTimers(Date.now()); + + offlineLaunchCapableListener = sinon.fake(); + + document.addEventListener( + offlineLaunchCapableType, + offlineLaunchCapableListener + ); + + activeServiceWorker = null; + + registrationUpdateFake = sandbox.fake(() => Promise.resolve()); + + registrationStub = sandbox + .stub(navigator.serviceWorker, 'register') + .callsFake(() => + Promise.resolve({ + addEventListener() {}, + active: activeServiceWorker, + update: registrationUpdateFake, + }) + ); + reloadStub = sandbox + .stub(applicationCache.location, 'reload') + .callsFake(() => {}); + + if (!('basePath' in settings)) { + settings.basePath = undefined; + } + + sandbox.stub(settings, 'basePath').value(basePath); + }); + + afterEach(() => { + document.removeEventListener( + offlineLaunchCapableType, + offlineLaunchCapableListener + ); + timers.restore(); + sandbox.restore(); + }); + + it('registers the service worker script', async () => { + await applicationCache.init(); + + expect(registrationStub).to.have.been.calledWith( + `${basePath}/x/offline-app-worker.js` + ); + }); + + it('reloads immediately after registering the service worker for the first time', async () => { + await applicationCache.init(); + + expect(reloadStub).to.have.been.called; + }); + + it('does not reload immediately after registering the service worker for subsequent times', async () => { + activeServiceWorker = {}; + + await applicationCache.init(); + + expect(reloadStub).not.to.have.been.called; + }); + + it('reports offline capability after registering the service worker for subsequent times', async () => { + activeServiceWorker = {}; + + await applicationCache.init(); + + expect(offlineLaunchCapableListener).to.have.been.calledWith( + events.OfflineLaunchCapable({ capable: true }) + ); + }); + + it('reports offline capability is not available when service workers are not available', async () => { + activeServiceWorker = {}; + + sandbox.stub(navigator, 'serviceWorker').value(null); + + await applicationCache.init(); + + expect(offlineLaunchCapableListener).to.have.been.calledWith( + events.OfflineLaunchCapable({ capable: false }) + ); + }); + + it('reports offline capability is not available when registration throws an error', async () => { + activeServiceWorker = {}; + + const error = new Error('Something bad'); + + registrationStub.callsFake(() => Promise.reject(error)); + + /** @type {Error} */ + let caught; + + try { + await applicationCache.init(); + } catch (error) { + caught = error; + } + + expect(offlineLaunchCapableListener).to.have.been.calledWith( + events.OfflineLaunchCapable({ capable: false }) + ); + expect(caught instanceof Error).to.equal(true); + expect(caught.message).to.include(error.message); + expect(caught.stack).to.equal(error.stack); + }); + + it('reloads when an updated service worker becomes active', async () => { + activeServiceWorker = {}; + await applicationCache.init(); + + expect(applicationCache.location.reload).not.to.have.been.called; + + navigator.serviceWorker.dispatchEvent(new Event('controllerchange')); + + expect(applicationCache.location.reload).to.have.been.called; + }); + + it('checks for updates immediately after registration', async () => { + await applicationCache.init(); + + expect(registrationUpdateFake).to.have.been.calledOnce; + }); + + it('checks for updates immediately after registration', async () => { + await applicationCache.init(); + + expect(registrationUpdateFake).to.have.been.calledOnce; + + timers.tick(applicationCache.UPDATE_REGISTRATION_INTERVAL); + + expect(registrationUpdateFake).to.have.been.calledTwice; + }); +}); From 02327cb77bf3989f7fac078308c1328d27e3ed61 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 10:26:00 -0700 Subject: [PATCH 05/20] Fix: always serve current state of offline worker script This only affects dev, but otherwise requires a server restart to make any service worker changes --- app/controllers/offline-controller.js | 3 ++- app/models/config-model.js | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/controllers/offline-controller.js b/app/controllers/offline-controller.js index 44cf9bc35..53c8d5396 100644 --- a/app/controllers/offline-controller.js +++ b/app/controllers/offline-controller.js @@ -2,6 +2,7 @@ * @module offline-resources-controller */ +const fs = require('fs'); const express = require('express'); const router = express.Router(); @@ -36,7 +37,7 @@ router.get('/x/offline-app-worker.js', (req, res, next) => { .map((resource) => `<${resource}>; rel="prefetch"`) .join(', '); - const script = config.offlineWorkerScript; + const script = fs.readFileSync(config.offlineWorkerPath, 'utf-8'); res.set('Content-Type', 'text/javascript'); res.set('Link', link); diff --git a/app/models/config-model.js b/app/models/config-model.js index 8f4c75e03..2dc189d4a 100644 --- a/app/models/config-model.js +++ b/app/models/config-model.js @@ -343,10 +343,11 @@ if (config['id length'] < 4) { config['id length'] = 31; } -const offlineWorkerScript = fs.readFileSync( - path.resolve(config.root, 'public/js/src/module/offline-app-worker.js'), - 'utf8' +config.offlineWorkerPath = path.resolve( + config.root, + 'public/js/src/module/offline-app-worker.js' ); +const offlineWorkerScript = fs.readFileSync(config.offlineWorkerPath, 'utf-8'); const hashSource = Buffer.from([offlineWorkerScript, JSON.stringify(config)]); const hash = crypto @@ -356,7 +357,6 @@ const hash = crypto .substring(0, 7); config.offlineVersion = [config.version, hash].join('-'); -config.offlineWorkerScript = offlineWorkerScript; module.exports = { /** From 61da52215bce20cd661c87e58c50798c354ae457 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 10:29:09 -0700 Subject: [PATCH 06/20] Small service worker fixes - Claim clients immediately on activation. It's not clear this completely eliminates the need for a reload, but it does get part of the way there. - Don't attempt to cache responses for non-GET requests --- public/js/src/module/offline-app-worker.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index b05fd4a5f..5c3aab109 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -1,9 +1,13 @@ -const CACHES = [`enketo-common`]; +const CACHES = ['enketo-common']; self.addEventListener('install', () => { self.skipWaiting(); }); +self.addEventListener('activate', (event) => { + event.waitUntil(self.clients.claim()); +}); + /** * @param {RequestInfo} request * @param {RequestInit} options @@ -43,16 +47,16 @@ const onFetch = async (request) => { return cached; } - if (response == null || response.status !== 200) { + if ( + request.method !== 'GET' || + response == null || + response.status !== 200 + ) { return response; } - // Cache any additional loaded languages const responseToCache = response.clone(); - // Cache any non-English language files that are requested - // Also, if the cache didn't get built correctly using the explicit resources list, - // just cache whatever is scoped dynamically to self-heal the cache. caches.open(CACHES[0]).then((cache) => { cache.put(request, responseToCache); }); From 7e23a6e5ad661ec6597d96ff2777491380466e4d Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 10:37:33 -0700 Subject: [PATCH 07/20] Fix: remove stale versioned caches Also, no reason for an array of keys which only ever has a length of one --- public/js/src/module/offline-app-worker.js | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index 5c3aab109..e82dadc93 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -1,11 +1,28 @@ -const CACHES = ['enketo-common']; +const CACHE_KEY = 'enketo-common'; self.addEventListener('install', () => { self.skipWaiting(); }); +const removeStaleCaches = async () => { + const keys = await caches.keys(); + + await Promise.all( + keys.map((key) => { + if (key !== CACHE_KEY) { + caches.delete(key); + } + }) + ); +}; + +const onActivate = async () => { + await self.clients.claim(); + await removeStaleCaches(); +}; + self.addEventListener('activate', (event) => { - event.waitUntil(self.clients.claim()); + event.waitUntil(onActivate()); }); /** @@ -57,7 +74,7 @@ const onFetch = async (request) => { const responseToCache = response.clone(); - caches.open(CACHES[0]).then((cache) => { + caches.open(CACHE_KEY).then((cache) => { cache.put(request, responseToCache); }); From bff15df085b5859bc7b8ac71c93eb83c1cd6fba5 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 11:24:31 -0700 Subject: [PATCH 08/20] Fix: prefetch is a hint, manually request linked resources --- public/js/src/module/offline-app-worker.js | 96 ++++++++++++++++------ 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index e82dadc93..bd20e75b5 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -1,7 +1,68 @@ const CACHE_KEY = 'enketo-common'; -self.addEventListener('install', () => { +/** + * @param {RequestInfo} request + */ +const tryFetch = async (request) => { + try { + return await fetch(request, { + credentials: 'same-origin', + cache: 'reload', + }); + } catch (error) { + return new Response(JSON.stringify({ message: error.message }), { + status: 500, + statusText: 'Internal Server Error', + headers: { + 'content-type': 'application/json', + }, + }); + } +}; + +/** + * @param {Request} request + * @param {Response} response + */ +const cacheResponse = (request, response) => { + caches.open(CACHE_KEY).then((cache) => { + cache.put(request, response); + }); +}; + +/** @type {string[]} */ +const prefetchURLs = []; + +/** + * @param {Response} response + */ +const setPrefetchURLs = (response) => { + const linkHeader = response.headers.get('link'); + + if (linkHeader == null) { + return; + } + + const prefetchLinks = linkHeader.matchAll(/<([^>]+)>;\s*rel="prefetch"/g); + + for (const match of prefetchLinks) { + prefetchURLs.push(match[1]); + } +}; + +const cachePrefetchURLs = () => + Promise.all( + prefetchURLs.map(async (url) => { + const request = new Request(url); + const response = await tryFetch(request); + + cacheResponse(request, response.clone()); + }) + ); + +self.addEventListener('install', (event) => { self.skipWaiting(); + event.waitUntil(cachePrefetchURLs()); }); const removeStaleCaches = async () => { @@ -25,33 +86,12 @@ self.addEventListener('activate', (event) => { event.waitUntil(onActivate()); }); -/** - * @param {RequestInfo} request - * @param {RequestInit} options - */ -const tryFetch = async (request, options) => { - try { - return await fetch(request, options); - } catch (error) { - return new Response(JSON.stringify({ message: error.message }), { - status: 500, - statusText: 'Internal Server Error', - headers: { - 'content-type': 'application/json', - }, - }); - } -}; - /** * @param {Request} request */ const onFetch = async (request) => { const [{ value: response }, { value: cached }] = await Promise.allSettled([ - tryFetch(request, { - credentials: 'same-origin', - cache: 'reload', - }), + tryFetch(request), caches.match(request), ]); @@ -72,11 +112,13 @@ const onFetch = async (request) => { return response; } - const responseToCache = response.clone(); + const isServiceWorkerScript = request.url === self.location.href; - caches.open(CACHE_KEY).then((cache) => { - cache.put(request, responseToCache); - }); + cacheResponse(request, response.clone()); + + if (isServiceWorkerScript) { + setPrefetchURLs(response.clone()); + } return response; }; From afe3ee32e256823f58f4465bdb949e83005a34cd Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 11:25:10 -0700 Subject: [PATCH 09/20] Allow for-of statements in ESLint --- .eslintrc.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 97c56528b..d44f00c01 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,9 +1,19 @@ -const eslintConfigAirbnb = require('eslint-config-airbnb-base/rules/variables'); +const baseVariablesConfig = require('eslint-config-airbnb-base/rules/variables'); +const baseStyleConfig = require('eslint-config-airbnb-base/rules/style'); -const serviceWorkerGlobals = eslintConfigAirbnb.rules[ +const serviceWorkerGlobals = baseVariablesConfig.rules[ 'no-restricted-globals' ].filter((item) => item.name !== 'self' && item !== 'self'); +const baseNoRestrictedSyntax = + baseStyleConfig.rules['no-restricted-syntax'].slice(1); +const noRestrictedSyntax = [ + 'warn', + ...baseNoRestrictedSyntax.filter( + (rule) => rule.selector !== 'ForOfStatement' + ), +]; + module.exports = { env: { es6: true, @@ -53,7 +63,7 @@ module.exports = { 'no-plusplus': 'warn', 'no-promise-executor-return': 'warn', 'no-restricted-globals': 'warn', - 'no-restricted-syntax': 'warn', + 'no-restricted-syntax': noRestrictedSyntax, 'no-return-assign': 'warn', 'no-shadow': 'warn', 'no-underscore-dangle': 'warn', From 34911429efc6c778f97cec52f5996d65b5cb063b Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 13:01:28 -0700 Subject: [PATCH 10/20] Fix: always use the same cache key when requesting the page itself This should not vary between different forms. This actually fixes a bug in both the current implementation as well as the previous changes on this branch: 1. Request offline-capable Form A while online 2. Request offline-capable Form B while online 3. Any changes which would trigger a service worker update 4. Request offline-capable Form A again while online, which updates the service worker and wipes out the previous cache 5. Request offline-capable Form B while OFFLINE, which will fail because its cached HTML has been removed Unless or until there is some meaningful difference in the HTML served for different forms, this change should be safe because the HTML and service worker will both be re-cached at the same time --- public/js/src/module/offline-app-worker.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index bd20e75b5..121ef06dd 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -90,9 +90,16 @@ self.addEventListener('activate', (event) => { * @param {Request} request */ const onFetch = async (request) => { + const { url } = request; + const isServiceWorkerScript = url === self.location.href; + const isPageRequest = url === request.referrer; + const cacheKey = isPageRequest + ? new URL('/x/', self.location.href) + : request; + const [{ value: response }, { value: cached }] = await Promise.allSettled([ tryFetch(request), - caches.match(request), + caches.match(cacheKey), ]); if ( @@ -112,9 +119,7 @@ const onFetch = async (request) => { return response; } - const isServiceWorkerScript = request.url === self.location.href; - - cacheResponse(request, response.clone()); + cacheResponse(cacheKey, response.clone()); if (isServiceWorkerScript) { setPrefetchURLs(response.clone()); From 853985ca345d058c856d4df4bf12a50c8f79464c Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 8 Oct 2022 14:31:21 -0700 Subject: [PATCH 11/20] Preserve HTML caches, reduce network use on load --- .eslintrc.js | 7 + Gruntfile.js | 2 +- app/models/config-model.js | 15 +- config/build.js | 10 +- package.json | 3 +- public/js/src/module/application-cache.js | 13 +- public/js/src/module/offline-app-worker.js | 158 +++++++++++---------- public/js/src/module/utils.js | 6 +- test/client/application-cache.spec.js | 13 +- test/client/utils.spec.js | 2 + 10 files changed, 125 insertions(+), 104 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index d44f00c01..6ae46a797 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -142,6 +142,13 @@ module.exports = { }, }, + { + files: ['public/js/src/**/*.js'], + globals: { + ENV: true, + }, + }, + { files: ['public/js/src/module/offline-app-worker.js'], globals: { diff --git a/Gruntfile.js b/Gruntfile.js index 582a1a9c4..c0b277b9f 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -108,7 +108,7 @@ module.exports = (grunt) => { 'find locales -name "translation-combined.json" -delete && rm -fr locales/??', }, 'clean-js': { - command: 'rm -f public/js/build/* && rm -f public/js/*.js', + command: 'rm -rf public/js/build/* && rm -f public/js/*.js', }, translation: { command: diff --git a/app/models/config-model.js b/app/models/config-model.js index 2dc189d4a..bd3e27df6 100644 --- a/app/models/config-model.js +++ b/app/models/config-model.js @@ -8,7 +8,6 @@ const mergeWith = require('lodash/mergeWith'); const path = require('path'); const fs = require('fs'); const url = require('url'); -const crypto = require('crypto'); const themePath = path.join(__dirname, '../../public/css'); const languagePath = path.join(__dirname, '../../locales/src'); @@ -345,18 +344,8 @@ if (config['id length'] < 4) { config.offlineWorkerPath = path.resolve( config.root, - 'public/js/src/module/offline-app-worker.js' + 'public/js/build/module/offline-app-worker.js' ); -const offlineWorkerScript = fs.readFileSync(config.offlineWorkerPath, 'utf-8'); - -const hashSource = Buffer.from([offlineWorkerScript, JSON.stringify(config)]); -const hash = crypto - .createHash('md5') - .update(hashSource) - .digest('hex') - .substring(0, 7); - -config.offlineVersion = [config.version, hash].join('-'); module.exports = { /** @@ -387,7 +376,7 @@ module.exports = { csrfCookieName: config['csrf cookie name'], excludeNonRelevant: config['exclude non-relevant'], experimentalOptimizations: config['experimental optimizations'], - offlineVersion: config.offlineVersion, + version: config.version, }, getThemesSupported, }; diff --git a/config/build.js b/config/build.js index fa9437f5f..4b5719761 100644 --- a/config/build.js +++ b/config/build.js @@ -5,14 +5,16 @@ const pkg = require('../package.json'); const cwd = process.cwd(); const entryPoints = pkg.entries.map((entry) => path.resolve(cwd, entry)); - -const isProduction = process.env.NODE_ENV === 'production'; +const { NODE_ENV } = process.env; module.exports = { bundle: true, + define: { + ENV: JSON.stringify(NODE_ENV ?? 'production'), + }, entryPoints, format: 'iife', - minify: isProduction, + minify: true, outdir: path.resolve(cwd, './public/js/build'), plugins: [ alias( @@ -24,6 +26,6 @@ module.exports = { ) ), ], - sourcemap: isProduction ? false : 'inline', + sourcemap: NODE_ENV === 'production' ? false : 'inline', target: ['chrome89', 'edge89', 'firefox90', 'safari13'], }; diff --git a/package.json b/package.json index 8bc636028..c468c22ca 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,8 @@ "public/js/src/enketo-webform.js", "public/js/src/enketo-webform-edit.js", "public/js/src/enketo-webform-view.js", - "public/js/src/enketo-offline-fallback.js" + "public/js/src/enketo-offline-fallback.js", + "public/js/src/module/offline-app-worker.js" ], "volta": { "node": "16.6.1", diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index c1157e213..4b3ba3972 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -37,8 +37,13 @@ const UPDATE_REGISTRATION_INTERVAL = 60 * 60 * 1000; const init = async (survey) => { try { if (navigator.serviceWorker != null) { + const workerPath = `${settings.basePath}/x/offline-app-worker.js`; + const workerURL = new URL(workerPath, window.location.href); + + workerURL.searchParams.set('version', settings.version); + const registration = await navigator.serviceWorker.register( - `${settings.basePath}/x/offline-app-worker.js` + workerURL ); // Registration was successful @@ -68,7 +73,11 @@ const init = async (survey) => { ); } - await registration.update(); + try { + registration.update(); + } catch { + // Probably offline + } if (currentActive == null) { location.reload(); diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index 121ef06dd..368284874 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -1,80 +1,60 @@ -const CACHE_KEY = 'enketo-common'; +const STATIC_CACHE = 'enketo-common'; +const FORMS_CACHE = 'enketo-forms'; /** - * @param {RequestInfo} request + * @param {string} url */ -const tryFetch = async (request) => { - try { - return await fetch(request, { - credentials: 'same-origin', - cache: 'reload', - }); - } catch (error) { - return new Response(JSON.stringify({ message: error.message }), { - status: 500, - statusText: 'Internal Server Error', - headers: { - 'content-type': 'application/json', - }, - }); +const cacheName = (url) => { + if ( + url === '/favicon.ico' || + url.endsWith('/x/') || + /\/x\/((css|fonts|images|js|locales)\/|offline-app-worker.js)/.test(url) + ) { + return STATIC_CACHE; } + + return FORMS_CACHE; }; /** - * @param {Request} request + * @param {Request | string} key * @param {Response} response */ -const cacheResponse = (request, response) => { - caches.open(CACHE_KEY).then((cache) => { - cache.put(request, response); - }); -}; +const cacheResponse = async (key, response) => { + const clone = response.clone(); + const cache = await caches.open(cacheName(key.url ?? key)); + + await cache.put(key, clone); -/** @type {string[]} */ -const prefetchURLs = []; + return response; +}; /** * @param {Response} response */ -const setPrefetchURLs = (response) => { - const linkHeader = response.headers.get('link'); +const cachePrefetchURLs = async (response) => { + const linkHeader = response.headers.get('link') ?? ''; + const prefetchURLs = [ + ...linkHeader.matchAll(/<([^>]+)>;\s*rel="prefetch"/g), + ].map(([, url]) => url); - if (linkHeader == null) { - return; - } - - const prefetchLinks = linkHeader.matchAll(/<([^>]+)>;\s*rel="prefetch"/g); + const cache = await caches.open(STATIC_CACHE); - for (const match of prefetchLinks) { - prefetchURLs.push(match[1]); - } + await Promise.allSettled(prefetchURLs.map((url) => cache.add(url))); }; -const cachePrefetchURLs = () => - Promise.all( - prefetchURLs.map(async (url) => { - const request = new Request(url); - const response = await tryFetch(request); - - cacheResponse(request, response.clone()); - }) - ); - -self.addEventListener('install', (event) => { +self.addEventListener('install', () => { self.skipWaiting(); - event.waitUntil(cachePrefetchURLs()); }); const removeStaleCaches = async () => { - const keys = await caches.keys(); - - await Promise.all( - keys.map((key) => { - if (key !== CACHE_KEY) { - caches.delete(key); - } - }) - ); + const cacheStorageKeys = await caches.keys(); + + cacheStorageKeys.forEach((key) => { + if (key !== FORMS_CACHE) { + caches.delete(key); + } + }); }; const onActivate = async () => { @@ -86,48 +66,72 @@ self.addEventListener('activate', (event) => { event.waitUntil(onActivate()); }); +const FETCH_OPTIONS = { + cache: 'reload', + credentials: 'same-origin', +}; + /** * @param {Request} request */ const onFetch = async (request) => { - const { url } = request; - const isServiceWorkerScript = url === self.location.href; - const isPageRequest = url === request.referrer; - const cacheKey = isPageRequest - ? new URL('/x/', self.location.href) - : request; + const { method, referrer, url } = request; - const [{ value: response }, { value: cached }] = await Promise.allSettled([ - tryFetch(request), - caches.match(cacheKey), - ]); + if (method !== 'GET') { + return fetch(request, FETCH_OPTIONS); + } - if ( - cached != null && - (response == null || - response.status !== 200 || - response.type !== 'basic') - ) { - return cached; + const isFormPageRequest = + url.includes('/x/') && (referrer === '' || referrer === url); + const cacheKey = isFormPageRequest ? url.replace(/\/x\/.*/, '/x/') : url; + const cached = await caches.match(cacheKey); + + let response = cached; + + if (response == null || ENV === 'development') { + try { + response = await fetch(request, FETCH_OPTIONS); + } catch { + // Probably offline + } } if ( - request.method !== 'GET' || response == null || - response.status !== 200 + response.status !== 200 || + response.type !== 'basic' ) { return response; } - cacheResponse(cacheKey, response.clone()); + if (isFormPageRequest) { + const { status } = response.clone(); + + if (status === 204) { + return caches.match(cacheKey); + } + + await cacheResponse(url, new Response(null, { status: 204 })); + } + + const isServiceWorkerScript = url === self.location.href; if (isServiceWorkerScript) { - setPrefetchURLs(response.clone()); + cachePrefetchURLs(response); } + await cacheResponse(cacheKey, response.clone()); + return response; }; +const { origin } = self.location; + self.addEventListener('fetch', (event) => { - event.respondWith(onFetch(event.request)); + const { request } = event; + const requestURL = new URL(request.url); + + if (requestURL.origin === origin) { + event.respondWith(onFetch(request)); + } }); diff --git a/public/js/src/module/utils.js b/public/js/src/module/utils.js index 0009939ba..7d321cecc 100644 --- a/public/js/src/module/utils.js +++ b/public/js/src/module/utils.js @@ -245,9 +245,13 @@ function _throwInvalidXmlNodeName(name) { ); } +/** + * @typedef {Record} URLLike + */ + /** * - * @param { string } path - location.pathname in a browser + * @param {URLLike | string} url - location.pathname in a browser */ function getEnketoId(path) { path = path.endsWith('/') ? path.substring(0, path.length - 1) : path; diff --git a/test/client/application-cache.spec.js b/test/client/application-cache.spec.js index b01ef7051..77ecc9bfc 100644 --- a/test/client/application-cache.spec.js +++ b/test/client/application-cache.spec.js @@ -4,6 +4,7 @@ import settings from '../../public/js/src/module/settings'; describe('Application Cache', () => { const basePath = '-'; + const version = `1.2.3-BADB3D`; const offlineLaunchCapableType = events.OfflineLaunchCapable().type; /** @type {ServiceWorker | null} */ @@ -55,11 +56,10 @@ describe('Application Cache', () => { .stub(applicationCache.location, 'reload') .callsFake(() => {}); - if (!('basePath' in settings)) { - settings.basePath = undefined; - } - + settings.basePath ??= undefined; + settings.version ??= undefined; sandbox.stub(settings, 'basePath').value(basePath); + sandbox.stub(settings, 'version').value(version); }); afterEach(() => { @@ -75,7 +75,10 @@ describe('Application Cache', () => { await applicationCache.init(); expect(registrationStub).to.have.been.calledWith( - `${basePath}/x/offline-app-worker.js` + new URL( + `${basePath}/x/offline-app-worker.js?version=${version}`, + window.location.href + ) ); }); diff --git a/test/client/utils.spec.js b/test/client/utils.spec.js index 235ada253..f5e8ea712 100644 --- a/test/client/utils.spec.js +++ b/test/client/utils.spec.js @@ -395,6 +395,8 @@ describe('Client Utilities', () => { '/edit/i/abcd/', '/xform/abcd', '/xform/abcd/', + '/x/abcd', + '/x/abcd/', ].forEach((test) => { it('extracts the id "abcd" correctly', () => { expect(utils.getEnketoId(test)).to.equal('abcd'); From 2636925dd54743344fb6444b4c4727b2e3e6f3ae Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 15 Oct 2022 11:06:03 -0700 Subject: [PATCH 12/20] Exclude build artifacts from ESLint and Prettier --- .eslintrc.js | 1 + .prettierignore | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6ae46a797..16e1ee4bc 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -37,6 +37,7 @@ module.exports = { }, }, }, + ignorePatterns: ['public/js/build/**/*'], rules: { 'prettier/prettier': 'error', 'import/no-unresolved': [ diff --git a/.prettierignore b/.prettierignore index 7c09f188e..109b0d908 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,7 +1,7 @@ # Ignore artifacts: .nyc* -public/js/build/* +public/js/build/**/* */node_modules/* docs/* test-coverage/* From 4e25bceb0abbaea0201e232974903eb808be4ae3 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Sat, 15 Oct 2022 11:32:25 -0700 Subject: [PATCH 13/20] Show update banner rather than reload if update is detected after load --- public/js/src/module/application-cache.js | 9 +++- test/client/application-cache.spec.js | 53 ++++++++++++++++++++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index 4b3ba3972..682838f2e 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -46,12 +46,15 @@ const init = async (survey) => { workerURL ); + let isInitialUpdateCheck = true; + // Registration was successful console.log( 'Offline application service worker registration successful with scope: ', registration.scope ); setInterval(() => { + isInitialUpdateCheck = false; console.log( 'Checking for offline application cache service worker update' ); @@ -68,7 +71,11 @@ const init = async (survey) => { navigator.serviceWorker.addEventListener( 'controllerchange', () => { - location.reload(); + if (isInitialUpdateCheck) { + location.reload(); + } else { + document.dispatchEvent(events.ApplicationUpdated()); + } } ); } diff --git a/test/client/application-cache.spec.js b/test/client/application-cache.spec.js index 77ecc9bfc..c2e7f8656 100644 --- a/test/client/application-cache.spec.js +++ b/test/client/application-cache.spec.js @@ -2,9 +2,11 @@ import applicationCache from '../../public/js/src/module/application-cache'; import events from '../../public/js/src/module/event'; import settings from '../../public/js/src/module/settings'; -describe('Application Cache', () => { +describe('Application cache initialization (offline service worker registration)', () => { const basePath = '-'; const version = `1.2.3-BADB3D`; + const applicationUpdatedEvent = events.ApplicationUpdated(); + const applicationUpdatedType = applicationUpdatedEvent.type; const offlineLaunchCapableType = events.OfflineLaunchCapable().type; /** @type {ServiceWorker | null} */ @@ -28,6 +30,9 @@ describe('Application Cache', () => { /** @type {sinon.SinonFake} */ let registrationUpdateFake; + /** @type {Function | null} */ + let controllerChangeListener; + beforeEach(() => { sandbox = sinon.createSandbox(); timers = sandbox.useFakeTimers(Date.now()); @@ -60,6 +65,25 @@ describe('Application Cache', () => { settings.version ??= undefined; sandbox.stub(settings, 'basePath').value(basePath); sandbox.stub(settings, 'version').value(version); + + const addControllerChangeListener = + navigator.serviceWorker.addEventListener; + + controllerChangeListener = null; + + sandbox + .stub(navigator.serviceWorker, 'addEventListener') + .callsFake((type, listener) => { + if (type === 'controllerchange') { + expect(controllerChangeListener).to.equal(null); + controllerChangeListener = listener; + } + addControllerChangeListener.call( + navigator.serviceWorker, + type, + listener + ); + }); }); afterEach(() => { @@ -67,6 +91,15 @@ describe('Application Cache', () => { offlineLaunchCapableType, offlineLaunchCapableListener ); + + if (controllerChangeListener != null) { + navigator.serviceWorker.removeEventListener( + 'controllerchange', + controllerChangeListener + ); + } + + timers.reset(); timers.restore(); sandbox.restore(); }); @@ -142,7 +175,7 @@ describe('Application Cache', () => { expect(caught.stack).to.equal(error.stack); }); - it('reloads when an updated service worker becomes active', async () => { + it('reloads when an updated service worker becomes active on load', async () => { activeServiceWorker = {}; await applicationCache.init(); @@ -168,4 +201,20 @@ describe('Application Cache', () => { expect(registrationUpdateFake).to.have.been.calledTwice; }); + + it('notifies the user, rather than reloading, when the service worker has changed', async () => { + activeServiceWorker = {}; + await applicationCache.init(); + + timers.tick(applicationCache.UPDATE_REGISTRATION_INTERVAL); + + const listener = sandbox.fake(); + + document.addEventListener(applicationUpdatedType, listener); + navigator.serviceWorker.dispatchEvent(new Event('controllerchange')); + document.removeEventListener(applicationUpdatedType, listener); + + expect(reloadStub).not.to.have.been.called; + expect(listener).to.have.been.calledOnceWith(applicationUpdatedEvent); + }); }); From a1a420729e254c59235fc23d971e696e1a630817 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Wed, 19 Oct 2022 14:13:01 -0700 Subject: [PATCH 14/20] Document current offline caching and network behavior --- tutorials/37-offline.md | 55 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 tutorials/37-offline.md diff --git a/tutorials/37-offline.md b/tutorials/37-offline.md new file mode 100644 index 000000000..3fb8e4f4e --- /dev/null +++ b/tutorials/37-offline.md @@ -0,0 +1,55 @@ +Enketo may be [configured](./tutorial-10-configure.html#offline-enabled) to support offline access to forms. When this configuration is set to `true`, accessing a form in offline-capable mode _while online_ will allow that form to be available for subsequent requests _while offline_. + +**Note:** unless specified otherwise, all of the caching and storage discussed below refers to storage _on an individual user's device and browser_. Also unless specified otherwise, caching is not used when the Enketo deployment is not configured to support offline forms, or when the user accesses a form in online-only mode (which is the default). + +### Offline-capable mode + +#### Service Worker and Cache Storage + +The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API), which intercepts some network requests in order to [cache](https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage) certain resources as they're requested. These resources include: + +- The Service Worker script itself +- The Enketo app's static assests (core JavaScript and CSS) +- CSS supplied by [configured themes](./tutorial-10-configure.html#themes-supported) +- The Enketo app icon, which may be displayed if a user bookmarks the app or registers it as a mobile web app +- The initial HTML which begins loading the app and the requested form + +This cache is cleared and rebuilt, after a brief delay when the user again accesses a form in offline-capable mode _while online_, and when the Enketo service is (re)started with changes to the version specified in `package.json` or the Service Worker script (typically following a new release or during development), or with changes to the configured themes. Apart from those cases, onced cached these resources will always be retrieved from Cache Storage without performing a network request. + +**Important note:** after such an update, previously cached forms must be requested in offline-capable mode _while online_ to be re-added to the cache. + +#### Form and form attachments + +When loading a form in offline-capable mode for the first time, the following resources are requested and cached in IndexedDB for future access while offline: + +- The form definition itself, [transformed](https://github.com/enketo/enketo-transformer) into a `survey` format which can be consumed by [Enketo Core](https://github.com/enketo/enketo-core) +- Media attachments used by the form +- [External secondary instances](https://getodk.github.io/xforms-spec/#secondary-instances---external) used by the form + +Once cached, if a user requests this form again in offline-capable mode _while online_, a request is made after a short delay to determine whether the form or its associated resources have changed. If they have changed, the above requests are performed again to update the IndexedDB cache. + +If the form and its associated resources **have not** changed, they will always be retrieved from IndexedDB without performing a network request. + +#### Submissions and submission attachments + +When a user submits a form entry in offline-capable mode, the submission (`record`) and any media files submitted by the user are also added to the IndexedDB cache, acting as a queue of submissions to send when the user is online. After a brief delay, if the user is online at that time, those submissions are sent and removed from the queue. If the user is not online at that time, the app will periodically check online status to determine whether it can process the queue. + +#### Draft and auto-saved submissions + +As a user fills a form, they may choose to save the submission as a draft, to be completed and submitted at a later time. These are also stored in IndexedDB as `record`s, but they will not be queued for submission until complete. A draft is also automatically saved as the user makes changes to a submission in progress, allowing an incomplete submission to be recovered if the page is reloaded or reopened. + +### Caveats + +- Service Workers may not be available in certain environments, such as a browser's private or incognito mode. In those conditions, a user will not be able to access forms offline. + +- As noted above, forms cached prior to an Enketo update will be removed from the cache when the Service Worker is updated. + +- Browsers vary in how long they preserve storage for Service Workers, Cache Storage, and data stored in IndexedDB. They also vary in how much storage is allowed for a given site/app. Users may also manually clear storage. All offline caches should be assumed to be temporary. + +### Related caching and use of browser storage in Enketo + +There are two cases where Enketo uses, or allows to be used, caching and browser storage in _both_ online-only and offline-capable modes: + +- Forms which reference the [last-saved virtual endpoint](https://getodk.github.io/xforms-spec/#virtual-endpoints) as a secondary instance store the user's most recent submission in IndexedDB in a manner similar to storage of offline submissions and drafts. + +- Resources which would normally be cached by the browser according to their response headers. From 4e717f8df9a9191ae0b95157e760164584447db6 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Fri, 21 Oct 2022 11:14:37 -0700 Subject: [PATCH 15/20] Document updated offline/caching behavior # Conflicts: # tutorials/37-offline.md # Conflicts: # tutorials/37-offline.md --- tutorials/37-offline.md | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tutorials/37-offline.md b/tutorials/37-offline.md index 3fb8e4f4e..ad7aa1267 100644 --- a/tutorials/37-offline.md +++ b/tutorials/37-offline.md @@ -6,25 +6,34 @@ Enketo may be [configured](./tutorial-10-configure.html#offline-enabled) to supp #### Service Worker and Cache Storage -The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API), which intercepts some network requests in order to [cache](https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage) certain resources as they're requested. These resources include: +The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API), which intercepts some network requests in order to [cache](https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage) certain resources as they're requested. The following resources are stored in one of two caches: -- The Service Worker script itself -- The Enketo app's static assests (core JavaScript and CSS) -- CSS supplied by [configured themes](./tutorial-10-configure.html#themes-supported) -- The Enketo app icon, which may be displayed if a user bookmarks the app or registers it as a mobile web app -- The initial HTML which begins loading the app and the requested form +- Common static assets used by Enketo to serve and render forms: -This cache is cleared and rebuilt, after a brief delay when the user again accesses a form in offline-capable mode _while online_, and when the Enketo service is (re)started with changes to the version specified in `package.json` or the Service Worker script (typically following a new release or during development), or with changes to the configured themes. Apart from those cases, onced cached these resources will always be retrieved from Cache Storage without performing a network request. + - The Service Worker script itself + - The Enketo app's static assests (core JavaScript and CSS) + - CSS supplied by [configured themes](./tutorial-10-configure.html#themes-supported) + - The Enketo app icon, which may be displayed if a user bookmarks the app or registers it as a mobile web app + - The initial HTML which begins loading the app and the requested form -**Important note:** after such an update, previously cached forms must be requested in offline-capable mode _while online_ to be re-added to the cache. + This cache is cleared and rebuilt in production, after a brief delay when the user again accesses a form in offline-capable mode _while online_, and when the Enketo service is (re)started with changes to the version specified in `package.json` or the Service Worker script (typically following a new release or during development), or with changes to the configured themes. -#### Form and form attachments +- Resources associated with individual forms: + + - Media attachments used by the form + - [External secondary instances](https://getodk.github.io/xforms-spec/#secondary-instances---external) used by the form + + The resources in this cache will be updated when Enketo detects that they have changed. + +For production Enketo deployments, these resources will always be retrieved from the cache if present, without performing additional network requests other than to determine whether they have been updated. To aid maintenance and improvement of Enketo's offline functionality, the requests _are_ performed in development mode. + +**Important note:** forms cached prior to release of [these changes to media and HTML caching](https://github.com/enketo/enketo-express/pull/465) must be requested in offline-capable mode while online to be re-added to the cache. Forms cached after that update will no longer have this issue. + +#### IndexedDB storage When loading a form in offline-capable mode for the first time, the following resources are requested and cached in IndexedDB for future access while offline: - The form definition itself, [transformed](https://github.com/enketo/enketo-transformer) into a `survey` format which can be consumed by [Enketo Core](https://github.com/enketo/enketo-core) -- Media attachments used by the form -- [External secondary instances](https://getodk.github.io/xforms-spec/#secondary-instances---external) used by the form Once cached, if a user requests this form again in offline-capable mode _while online_, a request is made after a short delay to determine whether the form or its associated resources have changed. If they have changed, the above requests are performed again to update the IndexedDB cache. @@ -42,7 +51,7 @@ As a user fills a form, they may choose to save the submission as a draft, to be - Service Workers may not be available in certain environments, such as a browser's private or incognito mode. In those conditions, a user will not be able to access forms offline. -- As noted above, forms cached prior to an Enketo update will be removed from the cache when the Service Worker is updated. +- As noted above, forms cached prior to the current caching behavior will not be available until they are re-added to the cache. - Browsers vary in how long they preserve storage for Service Workers, Cache Storage, and data stored in IndexedDB. They also vary in how much storage is allowed for a given site/app. Users may also manually clear storage. All offline caches should be assumed to be temporary. From 5de6d66ee5c5fb42e8af2ce0649d33fb48f65a97 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Fri, 21 Oct 2022 11:15:52 -0700 Subject: [PATCH 16/20] Remove blocking on initial form cache update And unnecessary try/catch block around initial non-blocking servicer worker update check --- public/js/src/module/application-cache.js | 6 +----- public/js/src/module/form-cache.js | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index 682838f2e..9caef79a6 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -80,11 +80,7 @@ const init = async (survey) => { ); } - try { - registration.update(); - } catch { - // Probably offline - } + registration.update(); if (currentActive == null) { location.reload(); diff --git a/public/js/src/module/form-cache.js b/public/js/src/module/form-cache.js index 34666103a..7f0f201a8 100644 --- a/public/js/src/module/form-cache.js +++ b/public/js/src/module/form-cache.js @@ -169,7 +169,7 @@ const setUpdateIntervals = async (survey) => { hash = survey.hash; // Check for form update upon loading. - await updateCache(survey); + updateCache(survey); // Note that for large Xforms where the XSL transformation takes more than 30 seconds, // the first update make take 20 minutes to propagate to the browser of the very first user(s) From 10996fb9f1e7adbffe4ce68dcb65d6154928bfea Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Fri, 21 Oct 2022 14:38:54 -0700 Subject: [PATCH 17/20] Show banner/don't reload if service worker update takes >500ms --- public/js/src/module/application-cache.js | 17 ++++++++++++++--- test/client/application-cache.spec.js | 10 +++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/public/js/src/module/application-cache.js b/public/js/src/module/application-cache.js index 9caef79a6..3d10dc0c6 100644 --- a/public/js/src/module/application-cache.js +++ b/public/js/src/module/application-cache.js @@ -20,6 +20,13 @@ const location = { }, }; +/** + * @private + * + * Exported only for testing. + */ +const RELOAD_ON_UPDATE_TIMEOUT = 500; + /** * @private * @@ -46,7 +53,11 @@ const init = async (survey) => { workerURL ); - let isInitialUpdateCheck = true; + let reloadOnUpdate = true; + + setTimeout(() => { + reloadOnUpdate = false; + }, RELOAD_ON_UPDATE_TIMEOUT); // Registration was successful console.log( @@ -54,7 +65,6 @@ const init = async (survey) => { registration.scope ); setInterval(() => { - isInitialUpdateCheck = false; console.log( 'Checking for offline application cache service worker update' ); @@ -71,7 +81,7 @@ const init = async (survey) => { navigator.serviceWorker.addEventListener( 'controllerchange', () => { - if (isInitialUpdateCheck) { + if (reloadOnUpdate) { location.reload(); } else { document.dispatchEvent(events.ApplicationUpdated()); @@ -123,6 +133,7 @@ function _reportOfflineLaunchCapable(capable = true) { export default { init, location, + RELOAD_ON_UPDATE_TIMEOUT, UPDATE_REGISTRATION_INTERVAL, get serviceWorkerScriptUrl() { if ( diff --git a/test/client/application-cache.spec.js b/test/client/application-cache.spec.js index c2e7f8656..8912bbe41 100644 --- a/test/client/application-cache.spec.js +++ b/test/client/application-cache.spec.js @@ -192,7 +192,7 @@ describe('Application cache initialization (offline service worker registration) expect(registrationUpdateFake).to.have.been.calledOnce; }); - it('checks for updates immediately after registration', async () => { + it('checks for updates periodically', async () => { await applicationCache.init(); expect(registrationUpdateFake).to.have.been.calledOnce; @@ -200,13 +200,17 @@ describe('Application cache initialization (offline service worker registration) timers.tick(applicationCache.UPDATE_REGISTRATION_INTERVAL); expect(registrationUpdateFake).to.have.been.calledTwice; + + timers.tick(applicationCache.UPDATE_REGISTRATION_INTERVAL); + + expect(registrationUpdateFake).to.have.been.calledThrice; }); - it('notifies the user, rather than reloading, when the service worker has changed', async () => { + it('notifies the user, rather than reloading, when a service worker update is detected some time after the page is loaded', async () => { activeServiceWorker = {}; await applicationCache.init(); - timers.tick(applicationCache.UPDATE_REGISTRATION_INTERVAL); + timers.tick(applicationCache.RELOAD_ON_UPDATE_TIMEOUT); const listener = sandbox.fake(); From e9263c9a5f8860d82ba8f1d0782d36327337b4d5 Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Fri, 21 Oct 2022 14:39:09 -0700 Subject: [PATCH 18/20] Add comments explaining use of /x/ cache key for HTML --- public/js/src/module/offline-app-worker.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index 368284874..8166aae93 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -83,7 +83,17 @@ const onFetch = async (request) => { const isFormPageRequest = url.includes('/x/') && (referrer === '' || referrer === url); + + /** + * A response for the form page initial HTML is always cached with the + * same key: `https://example.com/basePath/x/`. This ensures that forms + * previously cached before a future service worker update will still + * be available after that update. + * + * @see {@link https://github.com/enketo/enketo-express/issues/470} + */ const cacheKey = isFormPageRequest ? url.replace(/\/x\/.*/, '/x/') : url; + const cached = await caches.match(cacheKey); let response = cached; @@ -104,6 +114,15 @@ const onFetch = async (request) => { return response; } + /** + * In addition to storing the form page initial HTML with a single + * cache key, we store a sentinel 204 response for each individual + * cached form page URL. This ensures we don't load forms cached + * prior to introducing this caching strategy, as their attachments + * will not yet have been cached. + * + * @see {cacheKey} + */ if (isFormPageRequest) { const { status } = response.clone(); From a1f6b7a086ada8f434a3b5da38c55012874f08ea Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Fri, 21 Oct 2022 14:39:37 -0700 Subject: [PATCH 19/20] Clarify CacheStorage Cache API & Cache keys used --- tutorials/37-offline.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tutorials/37-offline.md b/tutorials/37-offline.md index ad7aa1267..b44edfec0 100644 --- a/tutorials/37-offline.md +++ b/tutorials/37-offline.md @@ -6,9 +6,9 @@ Enketo may be [configured](./tutorial-10-configure.html#offline-enabled) to supp #### Service Worker and Cache Storage -The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API), which intercepts some network requests in order to [cache](https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage) certain resources as they're requested. The following resources are stored in one of two caches: +The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API), which intercepts some network requests in order to cache certain resources in [CacheStorage](https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage) as they're requested. The following resources are stored in one of two Caches: -- Common static assets used by Enketo to serve and render forms: +- `enketo-common`: common static assets used by Enketo to serve and render forms: - The Service Worker script itself - The Enketo app's static assests (core JavaScript and CSS) @@ -16,18 +16,18 @@ The Enketo app registers a [Service Worker](https://developer.mozilla.org/en-US/ - The Enketo app icon, which may be displayed if a user bookmarks the app or registers it as a mobile web app - The initial HTML which begins loading the app and the requested form - This cache is cleared and rebuilt in production, after a brief delay when the user again accesses a form in offline-capable mode _while online_, and when the Enketo service is (re)started with changes to the version specified in `package.json` or the Service Worker script (typically following a new release or during development), or with changes to the configured themes. + This Cache is cleared and rebuilt in production, after a brief delay when the user again accesses a form in offline-capable mode _while online_, and when the Enketo service is (re)started with changes to the version specified in `package.json` or the Service Worker script (typically following a new release or during development), or with changes to the configured themes. -- Resources associated with individual forms: +- `enketo-forms`: resources associated with individual forms: - Media attachments used by the form - [External secondary instances](https://getodk.github.io/xforms-spec/#secondary-instances---external) used by the form - The resources in this cache will be updated when Enketo detects that they have changed. + The resources in this Cache will be updated when Enketo detects that they have changed. -For production Enketo deployments, these resources will always be retrieved from the cache if present, without performing additional network requests other than to determine whether they have been updated. To aid maintenance and improvement of Enketo's offline functionality, the requests _are_ performed in development mode. +For production Enketo deployments, these resources will always be retrieved from CacheStorage if present, without performing additional network requests other than to determine whether they have been updated. To aid maintenance and improvement of Enketo's offline functionality, the requests _are_ performed in development mode. -**Important note:** forms cached prior to release of [these changes to media and HTML caching](https://github.com/enketo/enketo-express/pull/465) must be requested in offline-capable mode while online to be re-added to the cache. Forms cached after that update will no longer have this issue. +**Important note:** forms cached prior to release of [these changes to media and HTML caching](https://github.com/enketo/enketo-express/pull/465) must be requested in offline-capable mode while online to be re-added to CacheStorage. Forms cached after that update will no longer have this issue. #### IndexedDB storage From 1c39cecff56d82172659ebcf99f480ad4ab7eaaf Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Mon, 7 Nov 2022 13:59:43 -0500 Subject: [PATCH 20/20] Fix: don't treat direct requests for non-form HTML URLs as form requests --- public/js/src/module/offline-app-worker.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/public/js/src/module/offline-app-worker.js b/public/js/src/module/offline-app-worker.js index 8166aae93..744ad79ce 100644 --- a/public/js/src/module/offline-app-worker.js +++ b/public/js/src/module/offline-app-worker.js @@ -81,8 +81,10 @@ const onFetch = async (request) => { return fetch(request, FETCH_OPTIONS); } + const { pathname } = new URL(url); const isFormPageRequest = - url.includes('/x/') && (referrer === '' || referrer === url); + /\/x\/[^/]+\/?$/.test(pathname) && + (referrer === '' || referrer === url); /** * A response for the form page initial HTML is always cached with the @@ -130,7 +132,10 @@ const onFetch = async (request) => { return caches.match(cacheKey); } - await cacheResponse(url, new Response(null, { status: 204 })); + await cacheResponse( + url, + new Response(null, { status: 204, statusText: 'No Content' }) + ); } const isServiceWorkerScript = url === self.location.href;