From 43aeaf4cf6f1cc0783b6e5783993e63bd977d1e6 Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Fri, 15 Mar 2019 00:17:38 +1300 Subject: [PATCH] Adds unknown replication status When the user is offline but we know that all docs have been replicated to the server show "unknown" replication state. Also refactors the replication events. medic/medic#5501 --- webapp/src/css/inbox.less | 6 +- webapp/src/js/controllers/inbox.js | 84 +++++---- webapp/src/js/services/db-sync.js | 188 +++++++++----------- webapp/src/templates/partials/header.html | 11 +- webapp/tests/karma/unit/services/db-sync.js | 116 ++++++------ 5 files changed, 202 insertions(+), 203 deletions(-) diff --git a/webapp/src/css/inbox.less b/webapp/src/css/inbox.less index 77499d67a41..36a8a94cfba 100644 --- a/webapp/src/css/inbox.less +++ b/webapp/src/css/inbox.less @@ -280,8 +280,10 @@ body { } .header .dropdown-menu .sync-status { - .in_progress, - .not_required { + .no-click { + color: @label-color; + } + .success { color: @ok-color; } .required { diff --git a/webapp/src/js/controllers/inbox.js b/webapp/src/js/controllers/inbox.js index 34db919c233..627f77d425c 100644 --- a/webapp/src/js/controllers/inbox.js +++ b/webapp/src/js/controllers/inbox.js @@ -100,56 +100,66 @@ var _ = require('underscore'), document.title = title; }); + const SYNC_STATUS = { + inProgress: { + icon: 'fa-refresh', + key: 'sync.status.in_progress', + debounce: true + }, + success: { + icon: 'fa-check', + key: 'sync.status.not_required', + className: 'success' + }, + required: { + icon: 'fa-exclamation-triangle', + key: 'sync.status.required', + className: 'required' + }, + unknown: { + icon: 'fa-question-circle', + key: 'sync.status.unknown' + } + }; + $scope.replicationStatus = { disabled: false, lastSuccess: {}, lastTrigger: undefined, - current: 'unknown', - textKey: 'sync.status.unknown', - }; - var SYNC_ICON = { - in_progress: 'fa-refresh', - not_required: 'fa-check', - required: 'fa-exclamation-triangle', - unknown: 'fa-question-circle', - }; - - const updateReplicationStatus = status => { - if ($scope.replicationStatus.current !== status) { - $scope.replicationStatus.current = status; - $scope.replicationStatus.textKey = 'sync.status.' + status; - $scope.replicationStatus.icon = SYNC_ICON[status]; - } + current: SYNC_STATUS.unknown, }; - DBSync.addUpdateListener(update => { - if (update.disabled) { + DBSync.addUpdateListener(({ disabled, unknown, inProgress, to, from }) => { + if (disabled) { $scope.replicationStatus.disabled = true; - // admins have potentially too much data so bypass local pouch - $log.debug('You have administrative privileges; not replicating'); return; } - - // Listen for directedReplicationStatus to update replicationStatus.lastSuccess - var now = Date.now(); - if (update.directedReplicationStatus === 'success') { - $scope.replicationStatus.lastSuccess[update.direction] = now; + if (unknown) { + $scope.replicationStatus.current = SYNC_STATUS.unknown; return; } - - // Listen for aggregateReplicationStatus updates - const status = update.aggregateReplicationStatus; + const now = Date.now(); const lastTrigger = $scope.replicationStatus.lastTrigger; - if (status === 'not_required' || status === 'required') { - const delay = lastTrigger ? (now - lastTrigger) / 1000 : 'unknown'; - $log.info('Replication ended after ' + delay + ' seconds with status ' + status); - } else if (status === 'in_progress') { + const delay = lastTrigger ? (now - lastTrigger) / 1000 : 'unknown'; + if (inProgress) { + $scope.replicationStatus.current = SYNC_STATUS.inProgress; $scope.replicationStatus.lastTrigger = now; - const duration = lastTrigger ? (now - lastTrigger) / 1000 : 'unknown'; - $log.info('Replication started after ' + duration + ' seconds since previous attempt.'); + $log.info(`Replication started after ${delay} seconds since previous attempt`); + return; + } + if (to === 'success') { + $scope.replicationStatus.lastSuccess.to = now; + } + if (from === 'success') { + $scope.replicationStatus.lastSuccess.from = now; + } + if (to === 'required' || from === 'required') { + $log.info(`Replication failed after ${delay} seconds`); + $scope.replicationStatus.current = SYNC_STATUS.required; + } else { + $log.info(`Replication succeeded after ${delay} seconds`); + $scope.replicationStatus.current = SYNC_STATUS.success; } - - updateReplicationStatus(status); }); $window.addEventListener('online', () => DBSync.setOnlineStatus(true), false); @@ -158,7 +168,7 @@ var _ = require('underscore'), key: 'sync-status', callback: function() { if (!DBSync.isSyncInProgress()) { - updateReplicationStatus('required'); + $scope.replicationStatus.current = SYNC_STATUS.required; } }, }); diff --git a/webapp/src/js/services/db-sync.js b/webapp/src/js/services/db-sync.js index f4cd42cdbe9..35101e201ed 100644 --- a/webapp/src/js/services/db-sync.js +++ b/webapp/src/js/services/db-sync.js @@ -1,11 +1,9 @@ -const _ = require('underscore'), - READ_ONLY_TYPES = ['form', 'translations'], - READ_ONLY_IDS = ['resources', 'branding', 'service-worker-meta', 'zscore-charts', 'settings', 'partners'], - DDOC_PREFIX = ['_design/'], - SYNC_INTERVAL = 5 * 60 * 1000, // 5 minutes - META_SYNC_INTERVAL = 30 * 60 * 1000; // 30 minutes - +const READ_ONLY_TYPES = ['form', 'translations']; +const READ_ONLY_IDS = ['resources', 'branding', 'service-worker-meta', 'zscore-charts', 'settings', 'partners']; +const DDOC_PREFIX = ['_design/']; const LAST_REPLICATED_SEQ_KEY = require('../bootstrapper/purger').LAST_REPLICATED_SEQ_KEY; +const SYNC_INTERVAL = 5 * 60 * 1000; // 5 minutes +const META_SYNC_INTERVAL = 30 * 60 * 1000; // 30 minutes angular .module('inboxServices') @@ -29,11 +27,7 @@ angular meta: undefined, }; - var authenticationIssue = function(errors) { - return _.findWhere(errors, { status: 401 }); - }; - - var readOnlyFilter = function(doc) { + const readOnlyFilter = function(doc) { // Never replicate "purged" documents upwards const keys = Object.keys(doc); if (keys.length === 4 && @@ -52,107 +46,91 @@ angular ); }; - var getOptions = function(direction) { - var options = {}; - if (direction === 'to') { - options.checkpoint = 'source'; - options.filter = readOnlyFilter; + const DIRECTIONS = [ + { + name: 'to', + options: { + checkpoint: 'source', + filter: readOnlyFilter + }, + allowed: () => Auth('can_edit').then(() => true).catch(() => false) + }, + { + name: 'from', + options: {}, + allowed: () => $q.resolve(true) } - - return options; - }; - - var replicate = function(direction) { - var options = getOptions(direction); - var remote = DB({ remote: true }); - return DB() - .replicate[direction](remote, options) - .on('denied', function(err) { - // In theory this could be caused by 401s - // TODO: work out what `err` looks like and navigate to login - // when we detect it's a 401 - $log.error('Denied replicating ' + direction + ' remote server', err); - }) - .on('error', function(err) { - $log.error('Error replicating ' + direction + ' remote server', err); - }) - .on('complete', function(info) { - if (!info.ok && authenticationIssue(info.errors)) { - Session.navigateToLogin(); - } - }); - }; - - const replicateTo = () => { - const AUTH_FAILURE = {}; - return Auth('can_edit') - // not authorized to replicate to server - that's ok. Silently skip replication.to - .catch(() => AUTH_FAILURE) - .then(err => { - if (err !== AUTH_FAILURE) { - return replicate('to'); + ]; + + const replicate = function(direction) { + return direction.allowed() + .then(allowed => { + if (!allowed) { + // not authorized to replicate - that's ok, skip silently + return; } + const remote = DB({ remote: true }); + return DB() + .replicate[direction.name](remote, direction.options) + .on('denied', function(err) { + // In theory this could be caused by 401s + // TODO: work out what `err` looks like and navigate to login + // when we detect it's a 401 + $log.error(`Denied replicating ${direction.name} remote server`, err); + }) + .on('error', function(err) { + $log.error(`Error replicating ${direction.name} remote server`, err); + }) + .on('complete', function(info) { + const authError = !info.ok && + info.errors && + info.errors.some(error => error.status === 401); + if (authError) { + Session.navigateToLogin(); + } + }) + .then(() => { + return; // success + }) + .catch(err => { + $log.error(`Error replicating ${direction.name} remote server`, err); + return direction.name; + }); }); }; - const sendUpdateForDirectedReplication = (func, direction) => { - return func - .then(() => { - sendUpdate({ - direction, - directedReplicationStatus: 'success', - }); - }) - .catch(error => { - sendUpdate({ - direction, - directedReplicationStatus: 'failure', - error, - }); - return error; - }); - }; - - var syncMeta = function() { - var remote = DB({ meta: true, remote: true }); - var local = DB({ meta: true }); - local.sync(remote); - }; + const getCurrentSeq = () => DB().info().then(info => info.update_seq + ''); + const getLastSyncSeq = () => $window.localStorage.getItem(LAST_REPLICATED_SEQ_KEY); // inProgressSync prevents multiple concurrent replications let inProgressSync; + const sync = force => { if (!knownOnlineState && !force) { return $q.resolve(); } - /* - Controllers need the status of each directed replication (directedReplicationStatus) and the - status of the replication as a whole (aggregateReplicationStatus). - */ if (!inProgressSync) { inProgressSync = $q - .all([ - sendUpdateForDirectedReplication(replicate('from'), 'from'), - sendUpdateForDirectedReplication(replicateTo(), 'to'), - ]) - .then(results => { - const errors = _.filter(results, result => result); - if (errors.length > 0) { - $log.error('Error replicating remote server', errors); - sendUpdate({ - aggregateReplicationStatus: 'required', - error: errors, - }); - return; - } - - syncIsRecent = true; - sendUpdate({ aggregateReplicationStatus: 'not_required' }); - - return DB().info() - .then(dbInfo => { - $window.localStorage.setItem(LAST_REPLICATED_SEQ_KEY, dbInfo.update_seq); + .all(DIRECTIONS.map(direction => replicate(direction))) + .then(errs => { + return getCurrentSeq().then(currentSeq => { + errs = errs.filter(err => err); + let update = { to: 'success', from: 'success' }; + if (!errs.length) { + // no errors + syncIsRecent = true; + $window.localStorage.setItem(LAST_REPLICATED_SEQ_KEY, currentSeq); + } else if (currentSeq === getLastSyncSeq()) { + // no changes to send, but may have some to receive + update = { unknown: true }; + } else { + // definitely need to sync something + errs.forEach(err => { + update[err] = 'required'; + }); + } + sendUpdate(update); }); }) .finally(() => { @@ -160,14 +138,18 @@ angular }); } - sendUpdate({ aggregateReplicationStatus: 'in_progress' }); + sendUpdate({ inProgress: true }); return inProgressSync; }; + const syncMeta = function() { + const remote = DB({ meta: true, remote: true }); + const local = DB({ meta: true }); + local.sync(remote); + }; + const sendUpdate = update => { - _.forEach(updateListeners, listener => { - listener(update); - }); + updateListeners.forEach(listener => listener(update)); }; const resetSyncInterval = () => { @@ -220,6 +202,8 @@ angular */ sync: force => { if (Session.isOnlineOnly()) { + // online users have potentially too much data so bypass local pouch + $log.debug('You have administrative privileges; not replicating'); sendUpdate({ disabled: true }); return $q.resolve(); } diff --git a/webapp/src/templates/partials/header.html b/webapp/src/templates/partials/header.html index e6d3baa2e52..1f86234ff3c 100644 --- a/webapp/src/templates/partials/header.html +++ b/webapp/src/templates/partials/header.html @@ -92,17 +92,16 @@ -
  • - +
  • + sync.now -
  • - - - {{replicationStatus.textKey}} + + + {{replicationStatus.current.key}}
    • diff --git a/webapp/tests/karma/unit/services/db-sync.js b/webapp/tests/karma/unit/services/db-sync.js index 70d6fada29a..294cd5b556e 100644 --- a/webapp/tests/karma/unit/services/db-sync.js +++ b/webapp/tests/karma/unit/services/db-sync.js @@ -4,19 +4,21 @@ describe('DBSync service', () => { const { expect } = chai; - let service, - to, - from, - query, - allDocs, - info, - isOnlineOnly, - userCtx, - sync, - Auth, - recursiveOn, - $interval, - replicationResult; + let service; + let to; + let from; + let query; + let allDocs; + let info; + let isOnlineOnly; + let userCtx; + let sync; + let Auth; + let recursiveOn; + let $interval; + let replicationResult; + let getItem; + let setItem; beforeEach(() => { replicationResult = Q.resolve; @@ -33,16 +35,18 @@ describe('DBSync service', () => { query = sinon.stub(); allDocs = sinon.stub(); info = sinon.stub(); - info.returns(Q.resolve({update_seq: -99})); + info.returns(Q.resolve({ update_seq: 99 })); isOnlineOnly = sinon.stub(); userCtx = sinon.stub(); sync = sinon.stub(); Auth = sinon.stub(); + setItem = sinon.stub(); + getItem = sinon.stub(); module('inboxApp'); module($provide => { $provide.factory('DB', KarmaUtils.mockDB({ - replicate: { to: to, from: from }, + replicate: { to, from }, allDocs: allDocs, sync: sync, info: info @@ -53,6 +57,7 @@ describe('DBSync service', () => { userCtx: userCtx } ); $provide.value('Auth', Auth); + $provide.value('$window', { localStorage: { setItem, getItem } }); }); inject((_DBSync_, _$interval_) => { service = _DBSync_; @@ -87,14 +92,17 @@ describe('DBSync service', () => { }); }); - it('syncs automatically after interval', () => { + it('syncs automatically after interval', done => { isOnlineOnly.returns(false); Auth.returns(Q.resolve()); - return service.sync().then(() => { + service.sync().then(() => { expect(from.callCount).to.equal(1); $interval.flush(5 * 60 * 1000 + 1); - expect(from.callCount).to.equal(2); + setTimeout(() => { + expect(from.callCount).to.equal(2); + done(); + }); }); }); @@ -128,6 +136,23 @@ describe('DBSync service', () => { }); }); + it('error in replication with no docs to send results in "unknown" status', () => { + isOnlineOnly.returns(false); + Auth.returns(Q.resolve()); + + replicationResult = () => Q.reject('error'); + const onUpdate = sinon.stub(); + service.addUpdateListener(onUpdate); + info.returns(Q.resolve({ update_seq: 100 })); + getItem.returns('100'); + + return service.sync().then(() => { + expect(onUpdate.callCount).to.eq(2); + expect(onUpdate.args[0][0]).to.deep.eq({ inProgress: true }); + expect(onUpdate.args[1][0]).to.deep.eq({ unknown: true }); + }); + }); + it('error in replication results in "required" status', () => { isOnlineOnly.returns(false); Auth.returns(Q.resolve()); @@ -137,33 +162,18 @@ describe('DBSync service', () => { service.addUpdateListener(onUpdate); return service.sync().then(() => { - expect(onUpdate.callCount).to.eq(4); - expect(onUpdate.args[0][0]).to.deep.eq({ - aggregateReplicationStatus: 'in_progress', - }); - expect(onUpdate.args[1][0]).to.deep.eq({ - directedReplicationStatus: 'failure', - direction: 'from', - error: 'error', - }); - expect(onUpdate.args[2][0]).to.deep.eq({ - directedReplicationStatus: 'failure', - direction: 'to', - error: 'error', - }); - expect(onUpdate.args[3][0]).to.contain({ - aggregateReplicationStatus: 'required', - }); - expect(onUpdate.args[3][0].error).to.deep.eq(['error', 'error']); + expect(onUpdate.callCount).to.eq(2); + expect(onUpdate.args[0][0]).to.deep.eq({ inProgress: true }); + expect(onUpdate.args[1][0]).to.deep.eq({ to: 'required', from: 'required' }); }); }); - it('sync scenarios based on connectivity state', () => { + it('sync scenarios based on connectivity state', done => { isOnlineOnly.returns(false); Auth.returns(Q.resolve()); // sync with default online status - return service.sync().then(() => { + service.sync().then(() => { expect(from.callCount).to.equal(1); // go offline, don't attempt to sync @@ -173,10 +183,12 @@ describe('DBSync service', () => { // when you come back online eventually, sync immediately service.setOnlineStatus(true); - expect(from.callCount).to.equal(2); + + expect(from.callCount).to.equal(1); // wait for the inprogress sync to complete before continuing the test - return service.sync().then(() => { + service.sync().then(() => { + expect(from.callCount).to.equal(2); // don't sync if you quickly lose and regain connectivity @@ -186,7 +198,11 @@ describe('DBSync service', () => { // eventually, sync on the timer $interval.flush(5 * 60 * 1000 + 1); - expect(from.callCount).to.equal(3); + + setTimeout(() => { + expect(from.callCount).to.equal(3); + done(); + }); }); }); }); @@ -205,21 +221,9 @@ describe('DBSync service', () => { expect(from.args[0][1]).to.not.have.keys('filter', 'checkpoint'); expect(to.callCount).to.equal(0); - expect(onUpdate.callCount).to.eq(4); - expect(onUpdate.args[0][0]).to.deep.eq({ - aggregateReplicationStatus: 'in_progress', - }); - expect(onUpdate.args[1][0]).to.deep.eq({ - directedReplicationStatus: 'success', - direction: 'from', - }); - expect(onUpdate.args[2][0]).to.deep.eq({ - directedReplicationStatus: 'success', - direction: 'to', - }); - expect(onUpdate.args[3][0]).to.deep.eq({ - aggregateReplicationStatus: 'not_required', - }); + expect(onUpdate.callCount).to.eq(2); + expect(onUpdate.args[0][0]).to.deep.eq({ inProgress: true }); + expect(onUpdate.args[1][0]).to.deep.eq({ to: 'success', from: 'success' }); }); }); });