diff --git a/package-lock.json b/package-lock.json index 9bd954fe37..bc41b80046 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43037,6 +43037,7 @@ "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", + "@overleaf/promise-utils": "*", "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "async": "^3.2.2", @@ -51636,6 +51637,7 @@ "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", + "@overleaf/promise-utils": "*", "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "async": "^3.2.2", diff --git a/services/project-history/app/js/ErrorRecorder.js b/services/project-history/app/js/ErrorRecorder.js index ab04eee063..570110dbf9 100644 --- a/services/project-history/app/js/ErrorRecorder.js +++ b/services/project-history/app/js/ErrorRecorder.js @@ -305,6 +305,11 @@ export function getFailures(callback) { export const promises = { getFailedProjects: promisify(getFailedProjects), - record: promisify(record), getFailureRecord: promisify(getFailureRecord), + getLastFailure: promisify(getLastFailure), + getFailuresByType: promisify(getFailuresByType), + getFailures: promisify(getFailures), + record: promisify(record), + recordSyncStart: promisify(recordSyncStart), + setForceDebug: promisify(setForceDebug), } diff --git a/services/project-history/app/js/HashManager.js b/services/project-history/app/js/HashManager.js index 898ad2f43c..f1d3b0a6bc 100644 --- a/services/project-history/app/js/HashManager.js +++ b/services/project-history/app/js/HashManager.js @@ -10,6 +10,7 @@ * DS207: Consider shorter variations of null checks * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ +import { promisify } from 'util' import fs from 'fs' import crypto from 'crypto' import OError from '@overleaf/o-error' @@ -51,3 +52,7 @@ export function _getBlobHash(fsPath, callback) { }) }) } + +export const promises = { + _getBlobHash: promisify(_getBlobHash), +} diff --git a/services/project-history/app/js/HistoryStoreManager.js b/services/project-history/app/js/HistoryStoreManager.js index 232e24c52a..16ba1ac79b 100644 --- a/services/project-history/app/js/HistoryStoreManager.js +++ b/services/project-history/app/js/HistoryStoreManager.js @@ -1,9 +1,9 @@ +import { promisify } from 'util' import fs from 'fs' import request from 'request' import stream from 'stream' import logger from '@overleaf/logger' import _ from 'lodash' -import BPromise from 'bluebird' import { URL } from 'url' import OError from '@overleaf/o-error' import Settings from '@overleaf/settings' @@ -354,7 +354,7 @@ export function deleteProject(projectId, callback) { ) } -const getProjectBlobAsync = BPromise.promisify(getProjectBlob) +const getProjectBlobAsync = promisify(getProjectBlob) class BlobStore { constructor(projectId) { @@ -435,3 +435,15 @@ function _requestHistoryService(options, callback) { } }) } + +export const promises = { + getMostRecentChunk: promisify(getMostRecentChunk), + getChunkAtVersion: promisify(getChunkAtVersion), + getMostRecentVersion: promisify(getMostRecentVersion), + getProjectBlob: promisify(getProjectBlob), + getProjectBlobStream: promisify(getProjectBlobStream), + sendChanges: promisify(sendChanges), + createBlobForUpdate: promisify(createBlobForUpdate), + initializeProject: promisify(initializeProject), + deleteProject: promisify(deleteProject), +} diff --git a/services/project-history/app/js/LockManager.js b/services/project-history/app/js/LockManager.js index 562eebb0ff..ffd750a291 100644 --- a/services/project-history/app/js/LockManager.js +++ b/services/project-history/app/js/LockManager.js @@ -7,6 +7,7 @@ * DS207: Consider shorter variations of null checks * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ +import { promisify } from 'util' import async from 'async' import metrics from '@overleaf/metrics' import Settings from '@overleaf/settings' @@ -273,3 +274,41 @@ class Lock { }) } } + +/** + * Promisified version of runWithLock. + * + * @param {string} key + * @param {(extendLock: Function) => Promise} runner + */ +async function runWithLockPromises(key, runner) { + const runnerCb = (extendLock, callback) => { + const extendLockPromises = promisify(extendLock) + runner(extendLockPromises) + .then(result => { + callback(null, result) + }) + .catch(err => { + callback(err) + }) + } + + return new Promise((resolve, reject) => { + runWithLock(key, runnerCb, (err, result) => { + if (err) { + reject(err) + } else { + resolve(result) + } + }) + }) +} + +export const promises = { + tryLock: promisify(tryLock), + extendLock: promisify(extendLock), + getLock: promisify(getLock), + checkLock: promisify(checkLock), + releaseLock: promisify(releaseLock), + runWithLock: runWithLockPromises, +} diff --git a/services/project-history/app/js/SnapshotManager.js b/services/project-history/app/js/SnapshotManager.js index fa291d83b1..80ef9b2c2e 100644 --- a/services/project-history/app/js/SnapshotManager.js +++ b/services/project-history/app/js/SnapshotManager.js @@ -1,4 +1,6 @@ // @ts-check + +import { promisify } from 'util' import Core from 'overleaf-editor-core' import { Readable as StringStream } from 'stream' import BPromise from 'bluebird' @@ -171,3 +173,9 @@ function _loadFilesLimit(snapshot, kind, blobStore) { { concurrency: MAX_REQUESTS } ) } + +export const promises = { + getFileSnapshotStream: promisify(getFileSnapshotStream), + getProjectSnapshot: promisify(getProjectSnapshot), + getLatestSnapshot: promisify(getLatestSnapshot), +} diff --git a/services/project-history/app/js/SyncManager.js b/services/project-history/app/js/SyncManager.js index fcf72f738f..18161e3148 100644 --- a/services/project-history/app/js/SyncManager.js +++ b/services/project-history/app/js/SyncManager.js @@ -1,6 +1,6 @@ import _ from 'lodash' -import async from 'async' -import { promisify } from 'util' +import { callbackify, promisify } from 'util' +import { callbackifyMultiResult } from '@overleaf/promise-utils' import Settings from '@overleaf/settings' import logger from '@overleaf/logger' import Metrics from '@overleaf/metrics' @@ -27,7 +27,7 @@ const keys = Settings.redis.lock.key_schema // To add expiresAt field to existing entries in collection (choose a suitable future expiry date): // db.projectHistorySyncState.updateMany({resyncProjectStructure: false, resyncDocContents: [], expiresAt: {$exists:false}}, {$set: {expiresAt: new Date("2019-07-01")}}) -export function startResync(projectId, options, callback) { +async function startResync(projectId, options = {}) { // We have three options here // // 1. If we update mongo before making the call to web then there's a @@ -39,114 +39,71 @@ export function startResync(projectId, options, callback) { // after, causing all updates to be ignored from then on // // 3. We can wrap everything in a project lock - if (typeof options === 'function') { - callback = options - options = {} - } - Metrics.inc('project_history_resync') - LockManager.runWithLock( - keys.projectHistoryLock({ project_id: projectId }), - (extendLock, releaseLock) => - _startResyncWithoutLock(projectId, options, releaseLock), - function (error) { - if (error) { - OError.tag(error) - // record error in starting sync ("sync ongoing") - ErrorRecorder.record(projectId, -1, error, () => callback(error)) - } else { - callback() + try { + await LockManager.promises.runWithLock( + keys.projectHistoryLock({ project_id: projectId }), + async extendLock => { + await _startResyncWithoutLock(projectId, options) } + ) + } catch (error) { + // record error in starting sync ("sync ongoing") + try { + await ErrorRecorder.promises.record(projectId, -1, error) + } catch (err) { + // swallow any error thrown by ErrorRecorder.record() } - ) -} - -export function startHardResync(projectId, options, callback) { - if (typeof options === 'function') { - callback = options - options = {} + throw error } +} +async function startHardResync(projectId, options = {}) { Metrics.inc('project_history_hard_resync') - LockManager.runWithLock( - keys.projectHistoryLock({ project_id: projectId }), - (extendLock, releaseLock) => - clearResyncState(projectId, function (err) { - if (err) { - return releaseLock(OError.tag(err)) - } - RedisManager.clearFirstOpTimestamp(projectId, function (err) { - if (err) { - return releaseLock(OError.tag(err)) - } - RedisManager.destroyDocUpdatesQueue(projectId, function (err) { - if (err) { - return releaseLock(OError.tag(err)) - } - _startResyncWithoutLock(projectId, options, releaseLock) - }) - }) - }), - function (error) { - if (error) { - OError.tag(error) - // record error in starting sync ("sync ongoing") - ErrorRecorder.record(projectId, -1, error, () => callback(error)) - } else { - callback() + try { + await LockManager.promises.runWithLock( + keys.projectHistoryLock({ project_id: projectId }), + async extendLock => { + await clearResyncState(projectId) + await RedisManager.promises.clearFirstOpTimestamp(projectId) + await RedisManager.promises.destroyDocUpdatesQueue(projectId) + await _startResyncWithoutLock(projectId, options) } - } - ) + ) + } catch (error) { + // record error in starting sync ("sync ongoing") + await ErrorRecorder.promises.record(projectId, -1, error) + throw error + } } -function _startResyncWithoutLock(projectId, options, callback) { - ErrorRecorder.recordSyncStart(projectId, function (error) { - if (error) { - return callback(OError.tag(error)) - } +async function _startResyncWithoutLock(projectId, options) { + await ErrorRecorder.promises.recordSyncStart(projectId) - _getResyncState(projectId, function (error, syncState) { - if (error) { - return callback(OError.tag(error)) - } + const syncState = await _getResyncState(projectId) + if (syncState.isSyncOngoing()) { + throw new OError('sync ongoing') + } + syncState.setOrigin(options.origin || { kind: 'history-resync' }) + syncState.startProjectStructureSync() - if (syncState.isSyncOngoing()) { - return callback(new OError('sync ongoing')) - } + await WebApiManager.promises.requestResync(projectId) + await setResyncState(projectId, syncState) +} - syncState.setOrigin(options.origin || { kind: 'history-resync' }) - syncState.startProjectStructureSync() - - WebApiManager.requestResync(projectId, function (error) { - if (error) { - return callback(OError.tag(error)) - } - - setResyncState(projectId, syncState, callback) - }) - }) +async function _getResyncState(projectId) { + const rawSyncState = await db.projectHistorySyncState.findOne({ + project_id: new ObjectId(projectId.toString()), }) + const syncState = SyncState.fromRaw(projectId, rawSyncState) + return syncState } -function _getResyncState(projectId, callback) { - db.projectHistorySyncState.findOne( - { - project_id: new ObjectId(projectId.toString()), - }, - function (error, rawSyncState) { - if (error) { - return callback(OError.tag(error)) - } - const syncState = SyncState.fromRaw(projectId, rawSyncState) - callback(null, syncState) - } - ) -} - -export function setResyncState(projectId, syncState, callback) { +async function setResyncState(projectId, syncState) { + // skip if syncState is null (i.e. unchanged) if (syncState == null) { - return callback() - } // skip if syncState is null (i.e. unchanged) + return + } const update = { $set: syncState.toRaw(), $push: { @@ -158,142 +115,103 @@ export function setResyncState(projectId, syncState, callback) { }, $currentDate: { lastUpdated: true }, } + // handle different cases if (syncState.isSyncOngoing()) { - // starting a new sync + // starting a new sync; prevent the entry expiring while sync is in ongoing update.$inc = { resyncCount: 1 } - update.$unset = { expiresAt: true } // prevent the entry expiring while sync is in ongoing + update.$unset = { expiresAt: true } } else { - // successful completion of existing sync + // successful completion of existing sync; set the entry to expire in the + // future update.$set.expiresAt = new Date( Date.now() + EXPIRE_RESYNC_HISTORY_INTERVAL_MS - ) // set the entry to expire in the future + ) } + // apply the update - db.projectHistorySyncState.updateOne( - { - project_id: new ObjectId(projectId), - }, + await db.projectHistorySyncState.updateOne( + { project_id: new ObjectId(projectId) }, update, - { - upsert: true, - }, - callback + { upsert: true } ) } -export function clearResyncState(projectId, callback) { - db.projectHistorySyncState.deleteOne( - { - project_id: new ObjectId(projectId.toString()), - }, - callback - ) -} - -export function skipUpdatesDuringSync(projectId, updates, callback) { - _getResyncState(projectId, function (error, syncState) { - if (error) { - return callback(OError.tag(error)) - } - - if (!syncState.isSyncOngoing()) { - logger.debug({ projectId }, 'not skipping updates: no resync in progress') - return callback(null, updates) // don't return synsState when unchanged - } - - const filteredUpdates = [] - - for (const update of updates) { - try { - syncState.updateState(update) - } catch (error1) { - error = OError.tag(error1) - if (error instanceof SyncError) { - return callback(error) - } else { - throw error - } - } - - const shouldSkipUpdate = syncState.shouldSkipUpdate(update) - if (!shouldSkipUpdate) { - filteredUpdates.push(update) - } else { - logger.debug({ projectId, update }, 'skipping update due to resync') - } - } - - callback(null, filteredUpdates, syncState) +async function clearResyncState(projectId) { + await db.projectHistorySyncState.deleteOne({ + project_id: new ObjectId(projectId.toString()), }) } -export function expandSyncUpdates( +async function skipUpdatesDuringSync(projectId, updates) { + const syncState = await _getResyncState(projectId) + if (!syncState.isSyncOngoing()) { + logger.debug({ projectId }, 'not skipping updates: no resync in progress') + // don't return syncState when unchanged + return { updates, syncState: null } + } + + const filteredUpdates = [] + + for (const update of updates) { + syncState.updateState(update) + const shouldSkipUpdate = syncState.shouldSkipUpdate(update) + if (!shouldSkipUpdate) { + filteredUpdates.push(update) + } else { + logger.debug({ projectId, update }, 'skipping update due to resync') + } + } + return { updates: filteredUpdates, syncState } +} + +async function expandSyncUpdates( projectId, projectHistoryId, updates, - extendLock, - callback + extendLock ) { const areSyncUpdatesQueued = _.some(updates, 'resyncProjectStructure') || _.some(updates, 'resyncDocContent') if (!areSyncUpdatesQueued) { logger.debug({ projectId }, 'no resync updates to expand') - return callback(null, updates) + return updates } - _getResyncState(projectId, (error, syncState) => { - if (error) { - return callback(OError.tag(error)) - } + const syncState = await _getResyncState(projectId) - // compute the current snapshot from the most recent chunk - SnapshotManager.getLatestSnapshot( + // compute the current snapshot from the most recent chunk + const snapshotFiles = await SnapshotManager.promises.getLatestSnapshot( + projectId, + projectHistoryId + ) + + // check if snapshot files are valid + const invalidFiles = _.pickBy( + snapshotFiles, + (v, k) => v == null || typeof v.isEditable !== 'function' + ) + if (_.size(invalidFiles) > 0) { + throw new SyncError('file is missing isEditable method', { projectId, - projectHistoryId, - (error, snapshotFiles) => { - if (error) { - return callback(OError.tag(error)) - } - // check if snapshot files are valid - const invalidFiles = _.pickBy( - snapshotFiles, - (v, k) => v == null || typeof v.isEditable !== 'function' - ) - if (_.size(invalidFiles) > 0) { - return callback( - new SyncError('file is missing isEditable method', { - projectId, - invalidFiles, - }) - ) - } - const expander = new SyncUpdateExpander( - projectId, - snapshotFiles, - syncState.origin - ) - // expand updates asynchronously to avoid blocking - const handleUpdate = ( - update, - cb // n.b. lock manager calls cb asynchronously - ) => - expander.expandUpdate(update, error => { - if (error) { - return cb(OError.tag(error)) - } - extendLock(cb) - }) - async.eachSeries(updates, handleUpdate, error => { - if (error) { - return callback(OError.tag(error)) - } - callback(null, expander.getExpandedUpdates()) - }) - } - ) - }) + invalidFiles, + }) + } + + const expander = new SyncUpdateExpander( + projectId, + snapshotFiles, + syncState.origin + ) + + // expand updates asynchronously to avoid blocking + for (const update of updates) { + await expander.expandUpdate(update) + await extendLock() + } + + return expander.getExpandedUpdates() } class SyncState { @@ -456,7 +374,7 @@ class SyncUpdateExpander { return !matchedExpectedFile } - expandUpdate(update, cb) { + async expandUpdate(update) { if (update.resyncProjectStructure != null) { logger.debug( { projectId: this.projectId, update }, @@ -523,16 +441,14 @@ class SyncUpdateExpander { expectedBinaryFiles, persistedBinaryFiles ) - cb() } else if (update.resyncDocContent != null) { logger.debug( { projectId: this.projectId, update }, 'expanding resyncDocContent update' ) - this.queueTextOpForOutOfSyncContents(update, cb) + await this.queueTextOpForOutOfSyncContents(update) } else { this.expandedUpdates.push(update) - cb() } } @@ -637,13 +553,13 @@ class SyncUpdateExpander { } } - queueTextOpForOutOfSyncContents(update, cb) { + async queueTextOpForOutOfSyncContents(update) { const pathname = UpdateTranslator._convertPathname(update.path) const snapshotFile = this.files[pathname] const expectedFile = update.resyncDocContent if (!snapshotFile) { - return cb(new OError('unrecognised file: not in snapshot')) + throw new OError('unrecognised file: not in snapshot') } // Compare hashes to see if the persisted file matches the expected content. @@ -664,7 +580,7 @@ class SyncUpdateExpander { { projectId: this.projectId, persistedHash, expectedHash }, 'skipping diff because hashes match and persisted file has no ops' ) - return cb() + return } } else { logger.debug('cannot compare hashes, will retrieve content') @@ -672,79 +588,104 @@ class SyncUpdateExpander { const expectedContent = update.resyncDocContent.content - const computeDiff = (persistedContent, cb) => { - let op - logger.debug( - { projectId: this.projectId, persistedContent, expectedContent }, - 'diffing doc contents' - ) - try { - op = UpdateCompressor.diffAsShareJsOps( - persistedContent, - expectedContent - ) - } catch (error) { - return cb( - OError.tag(error, 'error from diffAsShareJsOps', { - projectId: this.projectId, - persistedContent, - expectedContent, - }) - ) - } - if (op.length === 0) { - return cb() - } - update = { - doc: update.doc, - op, - meta: { - resync: true, - origin: this.origin, - ts: update.meta.ts, - pathname, - doc_length: persistedContent.length, - }, - } - logger.debug( - { projectId: this.projectId, diffCount: op.length }, - 'doc contents differ' - ) - this.expandedUpdates.push(update) - Metrics.inc('project_history_resync_operation', 1, { - status: 'update text file contents', - }) - cb() - } - + let persistedContent // compute the difference between the expected and persisted content if (snapshotFile.load != null) { - WebApiManager.getHistoryId(this.projectId, (err, historyId) => { - if (err) { - return cb(OError.tag(err)) - } - const loadFile = snapshotFile.load( - 'eager', - HistoryStoreManager.getBlobStore(historyId) - ) - loadFile - .then(file => computeDiff(file.getContent(), cb)) - .catch(err => cb(err)) // error loading file or computing diff - }) + const historyId = await WebApiManager.promises.getHistoryId( + this.projectId + ) + const file = await snapshotFile.load( + 'eager', + HistoryStoreManager.getBlobStore(historyId) + ) + persistedContent = file.getContent() } else if (snapshotFile.content != null) { // use dummy content from queueAddOpsForMissingFiles for added missing files - computeDiff(snapshotFile.content, cb) + persistedContent = snapshotFile.content } else { - cb(new OError('unrecognised file')) + throw new OError('unrecognised file') } + + let op + logger.debug( + { projectId: this.projectId, persistedContent, expectedContent }, + 'diffing doc contents' + ) + try { + op = UpdateCompressor.diffAsShareJsOps(persistedContent, expectedContent) + } catch (error) { + throw OError.tag(error, 'error from diffAsShareJsOps', { + projectId: this.projectId, + persistedContent, + expectedContent, + }) + } + if (op.length === 0) { + return + } + update = { + doc: update.doc, + op, + meta: { + resync: true, + origin: this.origin, + ts: update.meta.ts, + pathname, + doc_length: persistedContent.length, + }, + } + logger.debug( + { projectId: this.projectId, diffCount: op.length }, + 'doc contents differ' + ) + this.expandedUpdates.push(update) + Metrics.inc('project_history_resync_operation', 1, { + status: 'update text file contents', + }) } } -export const promises = { - startResync: promisify(startResync), - startHardResync: promisify(startHardResync), - setResyncState: promisify(setResyncState), - clearResyncState: promisify(clearResyncState), - skipUpdatesDuringSync: promisify(skipUpdatesDuringSync), - expandSyncUpdates: promisify(expandSyncUpdates), +// EXPORTS + +const startResyncCb = callbackify(startResync) +const startHardResyncCb = callbackify(startHardResync) +const setResyncStateCb = callbackify(setResyncState) +const clearResyncStateCb = callbackify(clearResyncState) +const skipUpdatesDuringSyncCb = callbackifyMultiResult(skipUpdatesDuringSync, [ + 'updates', + 'syncState', +]) +const expandSyncUpdatesCb = ( + projectId, + projectHistoryId, + updates, + extendLock, + callback +) => { + const extendLockPromises = promisify(extendLock) + expandSyncUpdates(projectId, projectHistoryId, updates, extendLockPromises) + .then(result => { + callback(null, result) + }) + .catch(err => { + callback(err) + }) +} + +export { + startResyncCb as startResync, + startHardResyncCb as startHardResync, + setResyncStateCb as setResyncState, + clearResyncStateCb as clearResyncState, + skipUpdatesDuringSyncCb as skipUpdatesDuringSync, + expandSyncUpdatesCb as expandSyncUpdates, +} + +export const promises = { + startResync, + startHardResync, + setResyncState, + clearResyncState, + skipUpdatesDuringSync, + expandSyncUpdates, } diff --git a/services/project-history/package.json b/services/project-history/package.json index 818f95573a..5c21d05713 100644 --- a/services/project-history/package.json +++ b/services/project-history/package.json @@ -22,6 +22,7 @@ "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", + "@overleaf/promise-utils": "*", "@overleaf/redis-wrapper": "*", "@overleaf/settings": "*", "async": "^3.2.2", diff --git a/services/project-history/test/acceptance/js/SyncTests.js b/services/project-history/test/acceptance/js/SyncTests.js index 0a6f18fb9a..7412df955c 100644 --- a/services/project-history/test/acceptance/js/SyncTests.js +++ b/services/project-history/test/acceptance/js/SyncTests.js @@ -162,7 +162,7 @@ describe('Syncing with web and doc-updater', function () { ], error => { if (error) { - throw error + return done(error) } assert( createBlob.isDone(), diff --git a/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js b/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js index 42ea4986ae..f249e77829 100644 --- a/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js +++ b/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js @@ -39,47 +39,65 @@ describe('SyncManager', function () { this.syncState = { origin: { kind: 'history-resync' } } this.db = { projectHistorySyncState: { - findOne: sinon.stub().yields(null, this.syncState), - updateOne: sinon.stub().yields(), + findOne: sinon.stub().resolves(this.syncState), + updateOne: sinon.stub().resolves(), }, } - this.callback = sinon.stub() - this.extendLock = sinon.stub().yields() + this.extendLock = sinon.stub().resolves() + this.LockManager = { - runWithLock: sinon.spy((key, runner, callback) => { - runner(this.extendLock, callback) - }), + promises: { + runWithLock: sinon.stub().callsFake(async (key, runner) => { + await runner(this.extendLock) + }), + }, } + this.UpdateCompressor = { diffAsShareJsOps: sinon.stub(), } + this.UpdateTranslator = { isTextUpdate: sinon.stub(), _convertPathname: sinon.stub(), } + this.WebApiManager = { - getHistoryId: sinon.stub(), - requestResync: sinon.stub().yields(), + promises: { + getHistoryId: sinon.stub(), + requestResync: sinon.stub().resolves(), + }, } - this.WebApiManager.getHistoryId + this.WebApiManager.promises.getHistoryId .withArgs(this.projectId) - .yields(null, this.historyId) + .resolves(this.historyId) + this.ErrorRecorder = { - record: sinon.stub().yields(), - recordSyncStart: sinon.stub().yields(), + promises: { + record: sinon.stub().resolves(), + recordSyncStart: sinon.stub().resolves(), + }, } + this.RedisManager = {} + this.SnapshotManager = { - getLatestSnapshot: sinon.stub(), + promises: { + getLatestSnapshot: sinon.stub(), + }, } + this.HistoryStoreManager = { getBlobStore: sinon.stub(), _getBlobHashFromString: sinon.stub().returns('random-hash'), } + this.HashManager = { _getBlobHashFromString: sinon.stub(), } + this.Metrics = { inc: sinon.stub() } + this.Settings = { redis: { lock: { @@ -91,6 +109,7 @@ describe('SyncManager', function () { }, }, } + this.SyncManager = await esmock(MODULE_PATH, { '../../../../app/js/LockManager.js': this.LockManager, '../../../../app/js/UpdateCompressor.js': this.UpdateCompressor, @@ -113,13 +132,13 @@ describe('SyncManager', function () { describe('startResync', function () { describe('if a sync is not in progress', function () { - beforeEach(function () { - this.db.projectHistorySyncState.findOne.yields(null, {}) - this.SyncManager.startResync(this.projectId, this.callback) + beforeEach(async function () { + this.db.projectHistorySyncState.findOne.resolves({}) + await this.SyncManager.promises.startResync(this.projectId) }) it('takes the project lock', function () { - expect(this.LockManager.runWithLock).to.have.been.calledWith( + expect(this.LockManager.promises.runWithLock).to.have.been.calledWith( `ProjectHistoryLock:${this.projectId}` ) }) @@ -131,9 +150,9 @@ describe('SyncManager', function () { }) it('requests a resync from web', function () { - expect(this.WebApiManager.requestResync).to.have.been.calledWith( - this.projectId - ) + expect( + this.WebApiManager.promises.requestResync + ).to.have.been.calledWith(this.projectId) }) it('sets the sync state in mongo and prevents it expiring', function () { @@ -158,37 +177,31 @@ describe('SyncManager', function () { } ) }) - - it('calls the callback', function () { - expect(this.callback).to.have.been.called - }) }) describe('if project structure sync is in progress', function () { beforeEach(function () { const syncState = { resyncProjectStructure: true } - this.db.projectHistorySyncState.findOne.yields(null, syncState) - this.SyncManager.startResync(this.projectId, this.callback) + this.db.projectHistorySyncState.findOne.resolves(syncState) }) - it('returns an error if already syncing', function () { - expect(this.callback).to.have.been.calledWithMatch({ - message: 'sync ongoing', - }) + it('returns an error if already syncing', async function () { + await expect( + this.SyncManager.promises.startResync(this.projectId) + ).to.be.rejectedWith('sync ongoing') }) }) describe('if doc content sync in is progress', function () { - beforeEach(function () { + beforeEach(async function () { const syncState = { resyncDocContents: ['/foo.tex'] } - this.db.projectHistorySyncState.findOne.yields(null, syncState) - this.SyncManager.startResync(this.projectId, this.callback) + this.db.projectHistorySyncState.findOne.resolves(syncState) }) - it('returns an error if already syncing', function () { - expect(this.callback).to.have.been.calledWithMatch({ - message: 'sync ongoing', - }) + it('returns an error if already syncing', async function () { + await expect( + this.SyncManager.promises.startResync(this.projectId) + ).to.be.rejectedWith('sync ongoing') }) }) }) @@ -208,32 +221,25 @@ describe('SyncManager', function () { } }) - it('sets the sync state in mongo and prevents it expiring', function (done) { + it('sets the sync state in mongo and prevents it expiring', async function () { // SyncState is a private class of SyncManager // we know the interface however: - this.SyncManager.setResyncState( + await this.SyncManager.promises.setResyncState( this.projectId, - this.syncState, - error => { - expect(error).to.be.undefined - expect( - this.db.projectHistorySyncState.updateOne - ).to.have.been.calledWith( - { - project_id: new ObjectId(this.projectId), - }, - sinon.match({ - $set: this.syncState.toRaw(), - $currentDate: { lastUpdated: true }, - $inc: { resyncCount: 1 }, - $unset: { expiresAt: true }, - }), - { - upsert: true, - } - ) - done() - } + this.syncState + ) + + expect( + this.db.projectHistorySyncState.updateOne + ).to.have.been.calledWith( + { project_id: new ObjectId(this.projectId) }, + sinon.match({ + $set: this.syncState.toRaw(), + $currentDate: { lastUpdated: true }, + $inc: { resyncCount: 1 }, + $unset: { expiresAt: true }, + }), + { upsert: true } ) }) }) @@ -252,71 +258,58 @@ describe('SyncManager', function () { } }) - it('sets the sync state entry in mongo to expire', function (done) { - this.SyncManager.setResyncState( + it('sets the sync state entry in mongo to expire', async function () { + await this.SyncManager.promises.setResyncState( this.projectId, - this.syncState, - error => { - expect(error).to.be.undefined - expect( - this.db.projectHistorySyncState.updateOne - ).to.have.been.calledWith( - { - project_id: new ObjectId(this.projectId), - }, - sinon.match({ - $set: { - resyncProjectStructure: false, - resyncDocContents: [], - origin: { kind: 'history-resync' }, - expiresAt: new Date( - this.now.getTime() + 90 * 24 * 3600 * 1000 - ), - }, - $currentDate: { lastUpdated: true }, - }), - { - upsert: true, - } - ) - done() - } + this.syncState + ) + + expect( + this.db.projectHistorySyncState.updateOne + ).to.have.been.calledWith( + { project_id: new ObjectId(this.projectId) }, + sinon.match({ + $set: { + resyncProjectStructure: false, + resyncDocContents: [], + origin: { kind: 'history-resync' }, + expiresAt: new Date(this.now.getTime() + 90 * 24 * 3600 * 1000), + }, + $currentDate: { lastUpdated: true }, + }), + { upsert: true } ) }) }) describe('when the new sync state is null', function () { - it('does not update the sync state in mongo', function (done) { + it('does not update the sync state in mongo', async function () { // SyncState is a private class of SyncManager // we know the interface however: - this.SyncManager.setResyncState(this.projectId, null, error => { - expect(error).to.be.undefined - expect(this.db.projectHistorySyncState.updateOne).to.not.have.been - .called - done() - }) + await this.SyncManager.promises.setResyncState(this.projectId, null) + expect(this.db.projectHistorySyncState.updateOne).to.not.have.been + .called }) }) }) describe('skipUpdatesDuringSync', function () { describe('if a sync is not in progress', function () { - beforeEach(function () { - this.db.projectHistorySyncState.findOne.yields(null, {}) + beforeEach(async function () { + this.db.projectHistorySyncState.findOne.resolves({}) this.updates = ['some', 'mock', 'updates'] - this.SyncManager.skipUpdatesDuringSync( + this.result = await this.SyncManager.promises.skipUpdatesDuringSync( this.projectId, - this.updates, - this.callback + this.updates ) }) it('returns all updates', function () { - expect(this.callback).to.have.been.calledWith(null, this.updates) + expect(this.result.updates).to.deep.equal(this.updates) }) it('should not return any newSyncState', function () { - expect(this.callback).to.have.been.calledWithExactly(null, this.updates) + expect(this.result.syncState).to.be.null }) }) @@ -364,99 +357,93 @@ describe('SyncManager', function () { resyncDocContents: [], origin: { kind: 'history-resync' }, } - this.db.projectHistorySyncState.findOne.yields(null, syncState) + this.db.projectHistorySyncState.findOne.resolves(syncState) }) - it('remove updates before a project structure sync update', function () { + it('remove updates before a project structure sync update', async function () { const updates = [ this.renameUpdate, this.textUpdate, this.projectStructureSyncUpdate, ] - this.SyncManager.skipUpdatesDuringSync( - this.projectId, - updates, - (error, filteredUpdates, syncState) => { - expect(error).to.be.null - expect(filteredUpdates).to.deep.equal([ - this.projectStructureSyncUpdate, - ]) - expect(syncState.toRaw()).to.deep.equal({ - resyncProjectStructure: false, - resyncDocContents: ['new.tex'], - origin: { kind: 'history-resync' }, - }) - } - ) + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([this.projectStructureSyncUpdate]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: ['new.tex'], + origin: { kind: 'history-resync' }, + }) }) - it('allow project structure updates after project structure sync update', function () { + it('allow project structure updates after project structure sync update', async function () { const updates = [this.projectStructureSyncUpdate, this.renameUpdate] - this.SyncManager.skipUpdatesDuringSync( - this.projectId, - updates, - (error, filteredUpdates, syncState) => { - expect(error).to.be.null - expect(filteredUpdates).to.deep.equal([ - this.projectStructureSyncUpdate, - this.renameUpdate, - ]) - expect(syncState.toRaw()).to.deep.equal({ - resyncProjectStructure: false, - resyncDocContents: ['new.tex'], - origin: { kind: 'history-resync' }, - }) - } - ) + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([ + this.projectStructureSyncUpdate, + this.renameUpdate, + ]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: ['new.tex'], + origin: { kind: 'history-resync' }, + }) }) - it('remove text updates for a doc before doc sync update', function () { + it('remove text updates for a doc before doc sync update', async function () { const updates = [ this.projectStructureSyncUpdate, this.textUpdate, this.docContentSyncUpdate, ] - this.SyncManager.skipUpdatesDuringSync( - this.projectId, - updates, - (error, filteredUpdates, syncState) => { - expect(error).to.be.null - expect(filteredUpdates).to.deep.equal([ - this.projectStructureSyncUpdate, - this.docContentSyncUpdate, - ]) - expect(syncState.toRaw()).to.deep.equal({ - resyncProjectStructure: false, - resyncDocContents: [], - origin: { kind: 'history-resync' }, - }) - } - ) + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([ + this.projectStructureSyncUpdate, + this.docContentSyncUpdate, + ]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: [], + origin: { kind: 'history-resync' }, + }) }) - it('allow text updates for a doc after doc sync update', function () { + it('allow text updates for a doc after doc sync update', async function () { const updates = [ this.projectStructureSyncUpdate, this.docContentSyncUpdate, this.textUpdate, ] - this.SyncManager.skipUpdatesDuringSync( - this.projectId, - updates, - (error, filteredUpdates, syncState) => { - expect(error).to.be.null - expect(filteredUpdates).to.deep.equal([ - this.projectStructureSyncUpdate, - this.docContentSyncUpdate, - this.textUpdate, - ]) - expect(syncState.toRaw()).to.deep.equal({ - resyncProjectStructure: false, - resyncDocContents: [], - origin: { kind: 'history-resync' }, - }) - } - ) + const { updates: filteredUpdates, syncState } = + await this.SyncManager.promises.skipUpdatesDuringSync( + this.projectId, + updates + ) + + expect(filteredUpdates).to.deep.equal([ + this.projectStructureSyncUpdate, + this.docContentSyncUpdate, + this.textUpdate, + ]) + expect(syncState.toRaw()).to.deep.equal({ + resyncProjectStructure: false, + resyncDocContents: [], + origin: { kind: 'history-resync' }, + }) }) }) }) @@ -493,97 +480,92 @@ describe('SyncManager', function () { .returns('another.tex') this.UpdateTranslator._convertPathname.withArgs('1.png').returns('1.png') this.UpdateTranslator._convertPathname.withArgs('2.png').returns('2.png') - this.SnapshotManager.getLatestSnapshot.yields(null, this.fileMap) + this.SnapshotManager.promises.getLatestSnapshot.resolves(this.fileMap) }) - it('returns updates if no sync updates are queued', function () { + it('returns updates if no sync updates are queued', async function () { const updates = ['some', 'mock', 'updates'] - this.SyncManager.expandSyncUpdates( + const expandedUpdates = await this.SyncManager.promises.expandSyncUpdates( this.projectId, this.historyId, updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.equal(updates) - expect(this.SnapshotManager.getLatestSnapshot).to.not.have.been.called - expect(this.extendLock).to.not.have.been.called - } + this.extendLock ) + + expect(expandedUpdates).to.equal(updates) + expect(this.SnapshotManager.promises.getLatestSnapshot).to.not.have.been + .called + expect(this.extendLock).to.not.have.been.called }) describe('expanding project structure sync updates', function () { - it('queues nothing for expected docs and files', function () { + it('queues nothing for expected docs and files', async function () { const updates = [ resyncProjectStructureUpdate( [this.persistedDoc], [this.persistedFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + expect(expandedUpdates).to.deep.equal([]) + expect(this.extendLock).to.have.been.called }) - it('queues file removes for unexpected files', function () { + it('queues file removes for unexpected files', async function () { const updates = [resyncProjectStructureUpdate([this.persistedDoc], [])] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: this.persistedFile.path, - new_pathname: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: this.persistedFile.path, + new_pathname: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('queues doc removes for unexpected docs', function () { + it('queues doc removes for unexpected docs', async function () { const updates = [resyncProjectStructureUpdate([], [this.persistedFile])] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: this.persistedDoc.path, - new_pathname: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: this.persistedDoc.path, + new_pathname: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('queues file additions for missing files', function () { + it('queues file additions for missing files', async function () { const newFile = { path: '2.png', file: {}, @@ -595,31 +577,30 @@ describe('SyncManager', function () { [this.persistedFile, newFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: newFile.path, - file: newFile.file, - url: newFile.url, - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: newFile.path, + file: newFile.file, + url: newFile.url, + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('queues blank doc additions for missing docs', function () { + it('queues blank doc additions for missing docs', async function () { const newDoc = { path: 'another.tex', doc: new ObjectId().toString(), @@ -630,31 +611,30 @@ describe('SyncManager', function () { [this.persistedFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: newDoc.path, - doc: newDoc.doc, - docLines: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: newDoc.path, + doc: newDoc.doc, + docLines: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('removes and re-adds files if whether they are binary differs', function () { + it('removes and re-adds files if whether they are binary differs', async function () { const fileWichWasADoc = { path: this.persistedDoc.path, url: 'filestore/2.png', @@ -667,40 +647,39 @@ describe('SyncManager', function () { [fileWichWasADoc, this.persistedFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: fileWichWasADoc.path, - new_pathname: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - { - pathname: fileWichWasADoc.path, - file: fileWichWasADoc.file, - url: fileWichWasADoc.url, - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: fileWichWasADoc.path, + new_pathname: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + { + pathname: fileWichWasADoc.path, + file: fileWichWasADoc.file, + url: fileWichWasADoc.url, + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it("does not remove and re-add files if the expected file doesn't have a hash", function () { + it("does not remove and re-add files if the expected file doesn't have a hash", async function () { const fileWichWasADoc = { path: this.persistedDoc.path, url: 'filestore/2.png', @@ -712,20 +691,19 @@ describe('SyncManager', function () { [fileWichWasADoc, this.persistedFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([]) + expect(this.extendLock).to.have.been.called }) - it('does not remove and re-add editable files if there is a binary file with same hash', function () { + it('does not remove and re-add editable files if there is a binary file with same hash', async function () { const binaryFile = { file: Object().toString(), // The paths in the resyncProjectStructureUpdate must have a leading slash ('/') @@ -741,20 +719,19 @@ describe('SyncManager', function () { const updates = [ resyncProjectStructureUpdate([], [binaryFile, this.persistedFile]), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([]) + expect(this.extendLock).to.have.been.called }) - it('removes and re-adds binary files if they do not have same hash', function () { + it('removes and re-adds binary files if they do not have same hash', async function () { const persistedFileWithNewContent = { _hash: 'anotherhashvalue', hello: 'world', @@ -767,40 +744,39 @@ describe('SyncManager', function () { [persistedFileWithNewContent] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: persistedFileWithNewContent.path, - new_pathname: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - { - pathname: persistedFileWithNewContent.path, - file: persistedFileWithNewContent.file, - url: persistedFileWithNewContent.url, - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: persistedFileWithNewContent.path, + new_pathname: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + { + pathname: persistedFileWithNewContent.path, + file: persistedFileWithNewContent.file, + url: persistedFileWithNewContent.url, + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('preserves other updates', function () { + it('preserves other updates', async function () { const update = 'mock-update' const updates = [ update, @@ -809,22 +785,21 @@ describe('SyncManager', function () { [this.persistedFile] ), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([update]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([update]) + expect(this.extendLock).to.have.been.called }) }) describe('expanding doc contents sync updates', function () { - it('returns errors from diffAsShareJsOps', function () { + it('returns errors from diffAsShareJsOps', async function () { const diffError = new Error('test') this.UpdateCompressor.diffAsShareJsOps.throws(diffError) const updates = [ @@ -834,33 +809,30 @@ describe('SyncManager', function () { ), docContentSyncUpdate(this.persistedDoc, this.persistedDoc.content), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.equal(diffError) - expect(this.extendLock).to.have.been.called - } - ) + await expect( + this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + ).to.be.rejectedWith(diffError) + expect(this.extendLock).to.have.been.called }) - it('handles an update for a file that is missing from the snapshot', function () { + it('handles an update for a file that is missing from the snapshot', async function () { const updates = [docContentSyncUpdate('not-in-snapshot.txt', 'test')] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.exist - expect(error.message).to.equal('unrecognised file: not in snapshot') - } - ) + await expect( + this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + ).to.be.rejectedWith('unrecognised file: not in snapshot') }) - it('queues nothing for in docs whose contents is in sync', function () { + it('queues nothing for in docs whose contents is in sync', async function () { const updates = [ resyncProjectStructureUpdate( [this.persistedDoc], @@ -869,20 +841,19 @@ describe('SyncManager', function () { docContentSyncUpdate(this.persistedDoc, this.persistedDoc.content), ] this.UpdateCompressor.diffAsShareJsOps.returns([]) - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([]) + expect(this.extendLock).to.have.been.called }) - it('queues text updates for docs whose contents is out of sync', function () { + it('queues text updates for docs whose contents is out of sync', async function () { const updates = [ resyncProjectStructureUpdate( [this.persistedDoc], @@ -891,32 +862,31 @@ describe('SyncManager', function () { docContentSyncUpdate(this.persistedDoc, 'a'), ] this.UpdateCompressor.diffAsShareJsOps.returns([{ d: 'sdf', p: 1 }]) - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - doc: this.persistedDoc.doc, - op: [{ d: 'sdf', p: 1 }], - meta: { - pathname: this.persistedDoc.path, - doc_length: 4, - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + doc: this.persistedDoc.doc, + op: [{ d: 'sdf', p: 1 }], + meta: { + pathname: this.persistedDoc.path, + doc_length: 4, + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('queues text updates for docs created by project structure sync', function () { + it('queues text updates for docs created by project structure sync', async function () { this.UpdateCompressor.diffAsShareJsOps.returns([{ i: 'a', p: 0 }]) const newDoc = { path: 'another.tex', @@ -930,42 +900,41 @@ describe('SyncManager', function () { ), docContentSyncUpdate(newDoc, newDoc.content), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - pathname: newDoc.path, - doc: newDoc.doc, - docLines: '', - meta: { - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - { - doc: newDoc.doc, - op: [{ i: 'a', p: 0 }], - meta: { - pathname: newDoc.path, - doc_length: 0, - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + pathname: newDoc.path, + doc: newDoc.doc, + docLines: '', + meta: { + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + { + doc: newDoc.doc, + op: [{ i: 'a', p: 0 }], + meta: { + pathname: newDoc.path, + doc_length: 0, + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) - it('skips text updates for docs when hashes match', function () { + it('skips text updates for docs when hashes match', async function () { this.fileMap['main.tex'].getHash = sinon.stub().returns('special-hash') this.HashManager._getBlobHashFromString.returns('special-hash') const updates = [ @@ -975,20 +944,19 @@ describe('SyncManager', function () { ), docContentSyncUpdate(this.persistedDoc, 'hello'), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([]) + expect(this.extendLock).to.have.been.called }) - it('computes text updates for docs when hashes differ', function () { + it('computes text updates for docs when hashes differ', async function () { this.fileMap['main.tex'].getHash = sinon.stub().returns('first-hash') this.HashManager._getBlobHashFromString.returns('second-hash') this.UpdateCompressor.diffAsShareJsOps.returns([ @@ -1001,33 +969,32 @@ describe('SyncManager', function () { ), docContentSyncUpdate(this.persistedDoc, 'hello'), ] - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - expect(error).to.be.null - expect(expandedUpdates).to.deep.equal([ - { - doc: this.persistedDoc.doc, - op: [{ i: 'test diff', p: 0 }], - meta: { - pathname: this.persistedDoc.path, - doc_length: 4, - resync: true, - ts: timestamp, - origin: { kind: 'history-resync' }, - }, - }, - ]) - expect(this.extendLock).to.have.been.called - } - ) + const expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) + + expect(expandedUpdates).to.deep.equal([ + { + doc: this.persistedDoc.doc, + op: [{ i: 'test diff', p: 0 }], + meta: { + pathname: this.persistedDoc.path, + doc_length: 4, + resync: true, + ts: timestamp, + origin: { kind: 'history-resync' }, + }, + }, + ]) + expect(this.extendLock).to.have.been.called }) describe('for docs whose contents is out of sync', function () { - beforeEach(function (done) { + beforeEach(async function () { const updates = [ resyncProjectStructureUpdate( [this.persistedDoc], @@ -1038,21 +1005,16 @@ describe('SyncManager', function () { const file = { getContent: sinon.stub().returns('stored content') } this.fileMap['main.tex'].load = sinon.stub().resolves(file) this.UpdateCompressor.diffAsShareJsOps.returns([{ d: 'sdf', p: 1 }]) - this.SyncManager.expandSyncUpdates( - this.projectId, - this.historyId, - updates, - this.extendLock, - (error, expandedUpdates) => { - this.error = error - this.expandedUpdates = expandedUpdates - done() - } - ) + this.expandedUpdates = + await this.SyncManager.promises.expandSyncUpdates( + this.projectId, + this.historyId, + updates, + this.extendLock + ) }) it('loads content from the history service when needed', function () { - expect(this.error).to.be.null expect(this.expandedUpdates).to.deep.equal([ { doc: this.persistedDoc.doc,