Merge pull request #9535 from overleaf/em-promisify-update-merger

Promisify UpdateMerger

GitOrigin-RevId: 5aca78225b524d95f0c47ae872df64edd2685b01
This commit is contained in:
Eric Mc Sween 2022-09-07 06:28:42 -04:00 committed by Copybot
parent 6b8dc91f9f
commit dc2097e684
2 changed files with 350 additions and 488 deletions

View file

@ -1,264 +1,165 @@
/* eslint-disable
camelcase,
n/handle-callback-err,
max-len,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* DS207: Consider shorter variations of null checks
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
let UpdateMerger
const OError = require('@overleaf/o-error')
const { callbackify } = require('util')
const _ = require('underscore')
const async = require('async')
const fs = require('fs')
const fsPromises = require('fs/promises')
const logger = require('@overleaf/logger')
const EditorController = require('../Editor/EditorController')
const FileTypeManager = require('../Uploads/FileTypeManager')
const FileWriter = require('../../infrastructure/FileWriter')
const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
const { promisifyAll } = require('../../util/promises')
module.exports = UpdateMerger = {
mergeUpdate(user_id, project_id, path, updateRequest, source, callback) {
if (callback == null) {
callback = function () {}
async function mergeUpdate(userId, projectId, path, updateRequest, source) {
const fsPath = await FileWriter.promises.writeStreamToDisk(
projectId,
updateRequest
)
try {
await _mergeUpdate(userId, projectId, path, fsPath, source)
} finally {
try {
await fsPromises.unlink(fsPath)
} catch (err) {
logger.err({ projectId, fsPath }, 'error deleting file')
}
return FileWriter.writeStreamToDisk(
project_id,
updateRequest,
function (err, fsPath) {
if (err != null) {
return callback(err)
}
return UpdateMerger._mergeUpdate(
user_id,
project_id,
path,
fsPath,
source,
mergeErr =>
fs.unlink(fsPath, function (deleteErr) {
if (deleteErr != null) {
logger.err({ project_id, fsPath }, 'error deleting file')
}
return callback(mergeErr)
})
)
}
)
},
_findExistingFileType(project_id, path, callback) {
ProjectEntityHandler.getAllEntities(
project_id,
function (err, { docs, files }) {
if (err != null) {
return callback(err)
}
let existingFileType = null
if (_.some(files, f => f.path === path)) {
existingFileType = 'file'
}
if (_.some(docs, d => d.path === path)) {
existingFileType = 'doc'
}
callback(null, existingFileType)
}
)
},
_determineFileType(project_id, path, fsPath, callback) {
if (callback == null) {
callback = function () {}
}
// check if there is an existing file with the same path (we either need
// to overwrite it or delete it)
UpdateMerger._findExistingFileType(
project_id,
path,
function (err, existingFileType) {
if (err) {
return callback(err)
}
// determine whether the update should create a doc or binary file
FileTypeManager.getType(
path,
fsPath,
existingFileType,
function (err, { binary, encoding }) {
if (err != null) {
return callback(err)
}
// If we receive a non-utf8 encoding, we won't be able to keep things in
// sync, so we'll treat non-utf8 files as binary
const isBinary = binary || encoding !== 'utf-8'
// Existing | Update | Action
// ---------|-----------|-------
// file | isBinary | existing-file
// file | !isBinary | existing-file
// doc | isBinary | new-file, delete-existing-doc
// doc | !isBinary | existing-doc
// null | isBinary | new-file
// null | !isBinary | new-doc
// if a binary file already exists, always keep it as a binary file
// even if the update looks like a text file
if (existingFileType === 'file') {
return callback(null, 'existing-file')
}
// if there is an existing doc, keep it as a doc except when the
// incoming update is binary. In that case delete the doc and replace
// it with a new file.
if (existingFileType === 'doc') {
if (isBinary) {
return callback(null, 'new-file', 'delete-existing-doc')
} else {
return callback(null, 'existing-doc')
}
}
// if there no existing file, create a file or doc as needed
return callback(null, isBinary ? 'new-file' : 'new-doc')
}
)
}
)
},
_mergeUpdate(user_id, project_id, path, fsPath, source, callback) {
if (callback == null) {
callback = function () {}
}
return UpdateMerger._determineFileType(
project_id,
path,
fsPath,
function (err, fileType, deleteOriginalEntity) {
if (err != null) {
return callback(err)
}
async.series(
[
function (cb) {
if (deleteOriginalEntity) {
// currently we only delete docs
UpdateMerger.deleteUpdate(user_id, project_id, path, source, cb)
} else {
cb()
}
},
function (cb) {
if (['existing-file', 'new-file'].includes(fileType)) {
return UpdateMerger.p.processFile(
project_id,
fsPath,
path,
source,
user_id,
cb
)
} else if (['existing-doc', 'new-doc'].includes(fileType)) {
return UpdateMerger.p.processDoc(
project_id,
user_id,
fsPath,
path,
source,
cb
)
} else {
return cb(new Error('unrecognized file'))
}
},
],
callback
)
}
)
},
deleteUpdate(user_id, project_id, path, source, callback) {
if (callback == null) {
callback = function () {}
}
return EditorController.deleteEntityWithPath(
project_id,
path,
source,
user_id,
function () {
return callback()
}
)
},
p: {
processDoc(project_id, user_id, fsPath, path, source, callback) {
return UpdateMerger.p.readFileIntoTextArray(
fsPath,
function (err, docLines) {
if (err != null) {
OError.tag(
err,
'error reading file into text array for process doc update',
{
project_id,
}
)
return callback(err)
}
logger.debug({ docLines }, 'processing doc update from tpds')
return EditorController.upsertDocWithPath(
project_id,
path,
docLines,
source,
user_id,
function (err) {
return callback(err)
}
)
}
)
},
processFile(project_id, fsPath, path, source, user_id, callback) {
return EditorController.upsertFileWithPath(
project_id,
path,
fsPath,
null,
source,
user_id,
function (err) {
return callback(err)
}
)
},
readFileIntoTextArray(path, callback) {
return fs.readFile(path, 'utf8', function (error, content) {
if (content == null) {
content = ''
}
if (error != null) {
OError.tag(error, 'error reading file into text array', {
path,
})
return callback(error)
}
const lines = content.split(/\r\n|\n|\r/)
return callback(error, lines)
})
},
},
}
}
module.exports.promises = promisifyAll(module.exports)
async function _findExistingFileType(projectId, path) {
const { docs, files } = await ProjectEntityHandler.promises.getAllEntities(
projectId
)
if (_.some(docs, d => d.path === path)) {
return 'doc'
}
if (_.some(files, f => f.path === path)) {
return 'file'
}
return null
}
async function _determineFileType(projectId, path, fsPath) {
// check if there is an existing file with the same path (we either need
// to overwrite it or delete it)
const existingFileType = await _findExistingFileType(projectId, path)
// determine whether the update should create a doc or binary file
const { binary, encoding } = await FileTypeManager.promises.getType(
path,
fsPath,
existingFileType
)
// If we receive a non-utf8 encoding, we won't be able to keep things in
// sync, so we'll treat non-utf8 files as binary
const isBinary = binary || encoding !== 'utf-8'
// Existing | Update | Action
// ---------|-----------|-------
// file | isBinary | existing-file
// file | !isBinary | existing-file
// doc | isBinary | new-file, delete-existing-doc
// doc | !isBinary | existing-doc
// null | isBinary | new-file
// null | !isBinary | new-doc
// if a binary file already exists, always keep it as a binary file
// even if the update looks like a text file
if (existingFileType === 'file') {
return { fileType: 'existing-file' }
}
// if there is an existing doc, keep it as a doc except when the
// incoming update is binary. In that case delete the doc and replace
// it with a new file.
if (existingFileType === 'doc') {
if (isBinary) {
return {
fileType: 'new-file',
deleteOriginalEntity: 'delete-existing-doc',
}
} else {
return { fileType: 'existing-doc' }
}
}
// if there no existing file, create a file or doc as needed
return { fileType: isBinary ? 'new-file' : 'new-doc' }
}
async function _mergeUpdate(userId, projectId, path, fsPath, source) {
const { fileType, deleteOriginalEntity } = await _determineFileType(
projectId,
path,
fsPath
)
if (deleteOriginalEntity) {
await deleteUpdate(userId, projectId, path, source)
}
if (['existing-file', 'new-file'].includes(fileType)) {
await _processFile(projectId, fsPath, path, source, userId)
} else if (['existing-doc', 'new-doc'].includes(fileType)) {
await _processDoc(projectId, userId, fsPath, path, source)
} else {
throw new Error('unrecognized file')
}
}
async function deleteUpdate(userId, projectId, path, source) {
try {
await EditorController.promises.deleteEntityWithPath(
projectId,
path,
source,
userId
)
} catch (err) {
logger.warn(
{ err, userId, projectId, path, source },
'failed to delete entity'
)
}
}
async function _processDoc(projectId, userId, fsPath, path, source) {
const docLines = await _readFileIntoTextArray(fsPath)
logger.debug({ docLines }, 'processing doc update from tpds')
await EditorController.promises.upsertDocWithPath(
projectId,
path,
docLines,
source,
userId
)
}
async function _processFile(projectId, fsPath, path, source, userId) {
await EditorController.promises.upsertFileWithPath(
projectId,
path,
fsPath,
null,
source,
userId
)
}
async function _readFileIntoTextArray(path) {
let content = await fsPromises.readFile(path, 'utf8')
if (content == null) {
content = ''
}
const lines = content.split(/\r\n|\n|\r/)
return lines
}
module.exports = {
mergeUpdate: callbackify(mergeUpdate),
_mergeUpdate: callbackify(_mergeUpdate),
deleteUpdate: callbackify(deleteUpdate),
promises: {
mergeUpdate,
_mergeUpdate, // called by GitBridgeHandler
deleteUpdate,
},
}

View file

@ -1,25 +1,15 @@
const SandboxedModule = require('sandboxed-module')
const sinon = require('sinon')
const modulePath = require('path').join(
__dirname,
'../../../../app/src/Features/ThirdPartyDataStore/UpdateMerger.js'
)
const { expect } = require('chai')
const BufferedStream = require('bufferedstream')
const MODULE_PATH =
'../../../../app/src/Features/ThirdPartyDataStore/UpdateMerger.js'
describe('UpdateMerger :', function () {
beforeEach(function () {
this.updateMerger = SandboxedModule.require(modulePath, {
requires: {
fs: (this.fs = { unlink: sinon.stub().callsArgWith(1) }),
'../Editor/EditorController': (this.EditorController = {}),
'../Uploads/FileTypeManager': (this.FileTypeManager = {}),
'../../infrastructure/FileWriter': (this.FileWriter = {}),
'../Project/ProjectEntityHandler': (this.ProjectEntityHandler = {}),
'@overleaf/settings': { path: { dumpPath: 'dump_here' } },
},
})
this.project_id = 'project_id_here'
this.user_id = 'mock-user-id'
this.projectId = 'project_id_here'
this.userId = 'mock-user-id'
this.docPath = this.newDocPath = '/folder/doc.tex'
this.filePath = this.newFilePath = '/folder/file.png'
@ -31,340 +21,311 @@ describe('UpdateMerger :', function () {
this.existingDocs = [{ path: '/main.tex' }, { path: '/folder/other.tex' }]
this.existingFiles = [{ path: '/figure.pdf' }, { path: '/folder/fig1.pdf' }]
this.ProjectEntityHandler.getAllEntities = sinon
.stub()
.callsArgWith(1, null, {
docs: this.existingDocs,
files: this.existingFiles,
})
this.fsPath = '/tmp/file/path'
this.fileContents = `\\documentclass{article}
\\usepackage[utf8]{inputenc}
\\title{42}
\\author{Jane Doe}
\\date{June 2011}`
this.docLines = this.fileContents.split('\n')
this.source = 'dropbox'
this.updateRequest = new BufferedStream()
this.FileWriter.writeStreamToDisk = sinon.stub().yields(null, this.fsPath)
this.callback = sinon.stub()
this.fsPromises = {
unlink: sinon.stub().resolves(),
readFile: sinon.stub().withArgs(this.fsPath).resolves(this.fileContents),
}
this.EditorController = {
promises: {
deleteEntityWithPath: sinon.stub().resolves(),
upsertDocWithPath: sinon.stub().resolves(),
upsertFileWithPath: sinon.stub().resolves(),
},
}
this.FileTypeManager = {
promises: {
getType: sinon.stub(),
},
}
this.FileWriter = {
promises: {
writeStreamToDisk: sinon.stub().resolves(this.fsPath),
},
}
this.ProjectEntityHandler = {
promises: {
getAllEntities: sinon.stub().resolves({
docs: this.existingDocs,
files: this.existingFiles,
}),
},
}
this.Settings = { path: { dumpPath: 'dump_here' } }
this.UpdateMerger = SandboxedModule.require(MODULE_PATH, {
requires: {
'fs/promises': this.fsPromises,
'../Editor/EditorController': this.EditorController,
'../Uploads/FileTypeManager': this.FileTypeManager,
'../../infrastructure/FileWriter': this.FileWriter,
'../Project/ProjectEntityHandler': this.ProjectEntityHandler,
'@overleaf/settings': this.Settings,
},
})
})
describe('mergeUpdate', function () {
describe('doc updates for a new doc', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon
.stub()
.yields(null, { binary: false, encoding: 'utf-8' })
this.updateMerger.p.processDoc = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({
binary: false,
encoding: 'utf-8',
})
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.docPath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should process update as doc', function () {
this.updateMerger.p.processDoc
.calledWith(
this.project_id,
this.user_id,
this.fsPath,
this.docPath,
this.source
)
.should.equal(true)
expect(
this.EditorController.promises.upsertDocWithPath
).to.have.been.calledWith(
this.projectId,
this.docPath,
this.docLines,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
describe('file updates for a new file ', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon
.stub()
.yields(null, { binary: true })
this.updateMerger.p.processFile = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({ binary: true })
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.filePath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should process update as file', function () {
this.updateMerger.p.processFile
.calledWith(
this.project_id,
this.fsPath,
this.filePath,
this.source,
this.user_id
)
.should.equal(true)
expect(
this.EditorController.promises.upsertFileWithPath
).to.have.been.calledWith(
this.projectId,
this.filePath,
this.fsPath,
null,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
describe('doc updates for an existing doc', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon
.stub()
.yields(null, { binary: false, encoding: 'utf-8' })
this.updateMerger.p.processDoc = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({
binary: false,
encoding: 'utf-8',
})
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.existingDocPath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should process update as doc', function () {
this.updateMerger.p.processDoc
.calledWith(
this.project_id,
this.user_id,
this.fsPath,
this.existingDocPath,
this.source
)
.should.equal(true)
expect(
this.EditorController.promises.upsertDocWithPath
).to.have.been.calledWith(
this.projectId,
this.existingDocPath,
this.docLines,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
describe('file updates for an existing file', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon
.stub()
.yields(null, { binary: true })
this.updateMerger.p.processFile = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({ binary: true })
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.existingFilePath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should process update as file', function () {
this.updateMerger.p.processFile
.calledWith(
this.project_id,
this.fsPath,
this.existingFilePath,
this.source,
this.user_id
)
.should.equal(true)
expect(
this.EditorController.promises.upsertFileWithPath
).to.have.been.calledWith(
this.projectId,
this.existingFilePath,
this.fsPath,
null,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
})
describe('file updates for an existing doc', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon.stub().yields(null, { binary: true })
this.updateMerger.deleteUpdate = sinon.stub().yields()
this.updateMerger.p.processFile = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({ binary: true })
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.existingDocPath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should delete the existing doc', function () {
this.updateMerger.deleteUpdate
.calledWith(
this.user_id,
this.project_id,
this.existingDocPath,
this.source
)
.should.equal(true)
expect(
this.EditorController.promises.deleteEntityWithPath
).to.have.been.calledWith(
this.projectId,
this.existingDocPath,
this.source,
this.userId
)
})
it('should process update as file', function () {
this.updateMerger.p.processFile
.calledWith(
this.project_id,
this.fsPath,
this.existingDocPath,
this.source,
this.user_id
)
.should.equal(true)
expect(
this.EditorController.promises.upsertFileWithPath
).to.have.been.calledWith(
this.projectId,
this.existingDocPath,
this.fsPath,
null,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
describe('doc updates for an existing file', function () {
beforeEach(function () {
this.FileTypeManager.getType = sinon.stub().yields(null, { binary: true })
this.updateMerger.deleteUpdate = sinon.stub().yields()
this.updateMerger.p.processFile = sinon.stub().yields()
this.updateMerger.mergeUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
this.FileTypeManager.promises.getType.resolves({ binary: true })
await this.UpdateMerger.promises.mergeUpdate(
this.userId,
this.projectId,
this.existingFilePath,
this.updateRequest,
this.source,
this.callback
this.source
)
})
it('should look at the file contents', function () {
this.FileTypeManager.getType.called.should.equal(true)
expect(this.FileTypeManager.promises.getType).to.have.been.called
})
it('should not delete the existing file', function () {
this.updateMerger.deleteUpdate.called.should.equal(false)
expect(this.EditorController.promises.deleteEntityWithPath).to.not.have
.been.called
})
it('should process update as file', function () {
this.updateMerger.p.processFile
.calledWith(
this.project_id,
this.fsPath,
this.existingFilePath,
this.source,
this.user_id
)
.should.equal(true)
expect(
this.EditorController.promises.upsertFileWithPath
).to.have.been.calledWith(
this.projectId,
this.existingFilePath,
this.fsPath,
null,
this.source,
this.userId
)
})
it('removes the temp file from disk', function () {
this.fs.unlink.calledWith(this.fsPath).should.equal(true)
expect(this.fsPromises.unlink).to.have.been.calledWith(this.fsPath)
})
})
describe('deleteUpdate', function () {
beforeEach(function () {
this.EditorController.deleteEntityWithPath = sinon.stub().yields()
this.updateMerger.deleteUpdate(
this.user_id,
this.project_id,
beforeEach(async function () {
await this.UpdateMerger.promises.deleteUpdate(
this.userId,
this.projectId,
this.docPath,
this.source,
this.callback
this.source
)
})
it('should delete the entity in the editor controller', function () {
this.EditorController.deleteEntityWithPath
.calledWith(this.project_id, this.docPath, this.source, this.user_id)
.should.equal(true)
})
})
describe('private methods', function () {
describe('processDoc', function () {
beforeEach(function () {
this.docLines =
'\\documentclass{article}\n\\usepackage[utf8]{inputenc}\n\n\\title{42}\n\\author{Jane Doe}\n\\date{June 2011}'
this.updateMerger.p.readFileIntoTextArray = sinon
.stub()
.yields(null, this.docLines)
this.EditorController.upsertDocWithPath = sinon.stub().yields()
this.updateMerger.p.processDoc(
this.project_id,
this.user_id,
this.fsPath,
this.docPath,
this.source,
this.callback
)
})
it('reads the temp file from disk', function () {
this.updateMerger.p.readFileIntoTextArray
.calledWith(this.fsPath)
.should.equal(true)
})
it('should upsert the doc in the editor controller', function () {
this.EditorController.upsertDocWithPath
.calledWith(
this.project_id,
this.docPath,
this.docLines,
this.source,
this.user_id
)
.should.equal(true)
})
})
describe('processFile', function () {
beforeEach(function () {
this.EditorController.upsertFileWithPath = sinon.stub().yields()
this.updateMerger.p.processFile(
this.project_id,
this.fsPath,
this.filePath,
this.source,
this.user_id,
this.callback
)
})
it('should upsert the file in the editor controller', function () {
this.EditorController.upsertFileWithPath
.calledWith(
this.project_id,
this.filePath,
this.fsPath,
null,
this.source,
this.user_id
)
.should.equal(true)
})
expect(
this.EditorController.promises.deleteEntityWithPath
).to.have.been.calledWith(
this.projectId,
this.docPath,
this.source,
this.userId
)
})
})
})