refactor(realtime): updateNote

Signed-off-by: BoHong Li <a60814billy@gmail.com>

lib/realtime.js

@@ -111,65 +111,96 @@ function disconnectSocketOnNote (note) {
 }
 
 function updateNote (note, callback) {
-  models.Note.findOne({
-    where: {
-      id: note.id
-    }
-  }).then(function (_note) {
-    if (!_note) return callback(null, null)
-    // update user note history
-    var tempUsers = Object.assign({}, note.tempUsers)
-    note.tempUsers = {}
-    Object.keys(tempUsers).forEach(function (key) {
-      updateHistory(key, note, tempUsers[key])
-    })
-    if (note.lastchangeuser) {
-      if (_note.lastchangeuserId !== note.lastchangeuser) {
-        models.User.findOne({
-          where: {
-            id: note.lastchangeuser
-          }
-        }).then(function (user) {
-          if (!user) return callback(null, null)
-          note.lastchangeuserprofile = models.User.getProfile(user)
-          return finishUpdateNote(note, _note, callback)
-        }).catch(function (err) {
-          logger.error(err)
-          return callback(err, null)
-        })
-      } else {
-        return finishUpdateNote(note, _note, callback)
-      }
-    } else {
-      note.lastchangeuserprofile = null
-      return finishUpdateNote(note, _note, callback)
-    }
-  }).catch(function (err) {
+  _updateNoteAsync(note).then(_note => {
+    callback(null, _note)
+  }).catch((err) => {
     logger.error(err)
     return callback(err, null)
   })
 }
 
-function finishUpdateNote (note, _note, callback) {
-  if (!note || !note.server) return callback(null, null)
-  var body = note.server.document
-  var title = note.title = models.Note.parseNoteTitle(body)
-  var values = {
+function findNoteByIdAsync (id) {
+  return models.Note.findOne({
+    where: {
+      id: id
+    }
+  })
+}
+
+function updateHistoryForEveryUserCollaborateNote (note) {
+  // update history to every user in this note
+  const tempUsers = Object.assign({}, note.tempUsers)
+  note.tempUsers = {}
+  // update history should async function, but in there return values is not matter
+  Object.keys(tempUsers).forEach(function (key) {
+    exports.updateHistory(key, note, tempUsers[key])
+  })
+}
+
+async function getUserProfileByIdAsync (id) {
+  const user = await models.User.findOne({
+    where: {
+      id: id
+    }
+  })
+  if (!user) return null
+  return models.User.getProfile(user)
+}
+
+class UserNotFoundException extends Error {
+  constructor () {
+    super('user not found')
+    this.name = this.constructor.name
+    Error.captureStackTrace(this, this.constructor)
+  }
+}
+
+async function getLastChangeUserProfileAsync (currentLastChangeUserId, lastChangeUserIdInDatabase, lastChangeUserProfileInDatabase) {
+  if (!currentLastChangeUserId) return null
+  if (currentLastChangeUserId === lastChangeUserIdInDatabase) return lastChangeUserProfileInDatabase
+  const profile = await getUserProfileByIdAsync(currentLastChangeUserId)
+  if (!profile) {
+    throw new UserNotFoundException()
+  }
+  return profile
+}
+
+function buildNoteUpdateData (note) {
+  const body = note.server.document
+  const title = note.title = models.Note.parseNoteTitle(body)
+  return {
     title: title,
     content: body,
     authorship: note.authorship,
     lastchangeuserId: note.lastchangeuser,
     lastchangeAt: Date.now()
   }
-  _note.update(values).then(function (_note) {
-    saveRevisionJob.setSaverSleep(false)
-    return callback(null, _note)
-  }).catch(function (err) {
-    logger.error(err)
-    return callback(err, null)
-  })
 }
 
+async function _updateNoteAsync (note) {
+  let noteModel = await findNoteByIdAsync(note.id)
+  if (!noteModel) return null
+
+  updateHistoryForEveryUserCollaborateNote(note)
+
+  try {
+    note.lastchangeuserprofile = await getLastChangeUserProfileAsync(
+      note.lastchangeuser,
+      noteModel.lastchangeuserId,
+      noteModel.lastchangeuserprofile
+    )
+  } catch (err) {
+    if (err instanceof UserNotFoundException) {
+      return null
+    }
+    throw err
+  }
+
+  if (!note || !note.server) return null
+  noteModel = await noteModel.update(buildNoteUpdateData(note))
+  saveRevisionJob.setSaverSleep(false)
+  return noteModel
+}
 
 function getStatus (callback) {
   models.Note.count().then(function (notecount) {
@@ -776,7 +807,6 @@ exports = module.exports = realtime
 exports.extractNoteIdFromSocket = extractNoteIdFromSocket
 exports.parseNoteIdFromSocket = parseNoteIdFromSocket
 exports.updateNote = updateNote
-exports.finishUpdateNote = finishUpdateNote
 exports.failConnection = failConnection
 exports.isDuplicatedInSocketQueue = isDuplicatedInSocketQueue
 exports.updateUserData = updateUserData
@@ -794,6 +824,7 @@ exports.disconnectSocketOnNote = disconnectSocketOnNote
 exports.queueForDisconnect = queueForDisconnect
 exports.terminate = terminate
 exports.getUserPool = getUserPool
+exports.updateHistory = updateHistory
 exports.disconnectProcessQueue = disconnectProcessQueue
 exports.notes = notes
 exports.users = users

test/realtime/realtime.test.js

@@ -37,76 +37,6 @@ function removeModuleFromRequireCache (modulePath) {
 }
 
 describe('realtime', function () {
-  describe('updateNote', function () {
-    let realtime, fakeNote
-    beforeEach(() => {
-      mock('../../lib/logger', {
-        error: () => {
-        }
-      })
-      mock('../../lib/history', {})
-      mock('../../lib/models', {
-        Note: {
-          findOne: async function () {
-            return fakeNote
-          }
-        }
-      })
-      mock('../../lib/config', {})
-    })
-
-    afterEach(() => {
-      mock.stopAll()
-    })
-
-    it('should return null when note not found', function (done) {
-      fakeNote = null
-      realtime = require('../../lib/realtime')
-
-      sinon.stub(realtime, 'finishUpdateNote').callsFake(function (a, b, callback) {
-        callback(null, b)
-      })
-
-      const fakeCallback = sinon.fake()
-      realtime.updateNote({ id: '123' }, fakeCallback)
-      setTimeout(() => {
-        assert.ok(fakeCallback.called)
-        assert.deepStrictEqual(fakeCallback.getCall(0).args, [null, null])
-        sinon.restore()
-        done()
-      }, 50)
-    })
-  })
-
-  describe('finishUpdateNote', function () {
-    let realtime
-    beforeEach(() => {
-      mock('../../lib/logger', {})
-      mock('../../lib/history', {})
-      mock('../../lib/models', {
-        Note: {
-          parseNoteTitle: (data) => (data)
-        }
-      })
-      mock('../../lib/config', {})
-      realtime = require('../../lib/realtime')
-    })
-
-    afterEach(() => {
-      removeModuleFromRequireCache('../../lib/realtime')
-      mock.stopAll()
-    })
-
-    it('return null when note is null', () => {
-      const fakeCallback = sinon.fake()
-
-      realtime.finishUpdateNote(null, {}, fakeCallback)
-
-      assert.ok(fakeCallback.calledOnce)
-      assert.deepStrictEqual(fakeCallback.lastCall.args, [null, null])
-    })
-  })
-
   describe('connection', function () {
     let realtime
     beforeEach(() => {

test/realtime/updateNote.test.js

@@ -0,0 +1,286 @@
+/* eslint-env node, mocha */
+'use strict'
+
+const assert = require('assert')
+const mock = require('mock-require')
+const sinon = require('sinon')
+
+const {removeLibModuleCache} = require('./utils')
+const {createFakeLogger} = require('../testDoubles/loggerFake')
+const realtimeJobStub = require('../testDoubles/realtimeJobStub')
+
+describe('realtime#updateNote', function () {
+  let modelsStub
+  let realtime
+  const now = 1546300800000
+  let clock
+
+  beforeEach(() => {
+    removeLibModuleCache()
+    clock = sinon.useFakeTimers({
+      now,
+      toFake: ['Date']
+    })
+    modelsStub = {
+      Note: {
+        findOne: sinon.stub()
+      },
+      User: {
+        findOne: sinon.stub()
+      }
+    }
+
+    mock('../../lib/config', {})
+    mock('../../lib/logger', createFakeLogger())
+    mock('../../lib/models', modelsStub)
+    mock('../../lib/realtimeUpdateDirtyNoteJob', realtimeJobStub)
+    mock('../../lib/realtimeCleanDanglingUserJob', realtimeJobStub)
+    // mock('../../lib/realtimeSaveRevisionJob', realtimeJobStub)
+  })
+
+  afterEach(() => {
+    mock.stopAll()
+    clock.restore()
+    sinon.restore()
+    removeLibModuleCache()
+  })
+
+  it('should save history to each edited user', function (done) {
+    modelsStub.Note.findOne.returns(Promise.resolve({}))
+    realtime = require('../../lib/realtime')
+    const updateHistoryStub = sinon.stub(realtime, 'updateHistory')
+
+    const callback = sinon.stub()
+    const note = {
+      tempUsers: {
+        'user1': Date.now()
+      }
+    }
+    realtime.updateNote(note, callback)
+    clock.restore()
+    setTimeout(() => {
+      assert(updateHistoryStub.calledOnce)
+      assert(updateHistoryStub.lastCall.calledWith('user1', note, now))
+      done()
+    }, 50)
+  })
+
+  it('should set lastchangeprofile when lastchangeuser is set', function (done) {
+    const callback = sinon.stub()
+
+    const note = {
+      lastchangeuser: 'user1'
+    }
+
+    modelsStub.Note.findOne.returns(Promise.resolve({}))
+
+    modelsStub.User.findOne.withArgs({
+      where: {
+        id: 'user1'
+      }
+    }).returns(Promise.resolve({
+      id: 'user1',
+      profile: '{ "displayName": "User 01" }'
+    }))
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+
+    realtime = require('../../lib/realtime')
+
+    realtime.updateNote(note, callback)
+    clock.restore()
+    setTimeout(() => {
+      assert(note.lastchangeuserprofile.name === 'User 01')
+      done()
+    }, 50)
+  })
+
+  it('should save note with new data', function (done) {
+    const callback = sinon.stub()
+    const note = {
+      lastchangeuser: 'user1',
+      server: {
+        document: '# title\n\n## test2'
+      },
+      authorship: []
+    }
+
+    modelsStub.Note.parseNoteTitle = sinon.stub().returns('title')
+    const updateNoteStub = sinon.stub().returns(Promise.resolve({}))
+    modelsStub.Note.findOne.returns(Promise.resolve({
+      update: updateNoteStub
+    }))
+
+    modelsStub.User.findOne.withArgs({
+      where: {
+        id: 'user1'
+      }
+    }).returns(Promise.resolve({
+      id: 'user1',
+      profile: '{ "displayName": "User 01" }'
+    }))
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+    clock.tick(1000)
+
+    realtime = require('../../lib/realtime')
+    realtime.updateNote(note, callback)
+    setTimeout(() => {
+      assert(note.lastchangeuserprofile.name === 'User 01')
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === null)
+      assert(updateNoteStub.calledOnce)
+      assert(updateNoteStub.lastCall.args[0].lastchangeAt === now + 1000)
+      assert(updateNoteStub.lastCall.args[0].title === 'title')
+      assert(updateNoteStub.lastCall.args[0].content === '# title\n\n## test2')
+      done()
+    }, 50)
+  })
+
+  it('should save note when lsatChangeUser is guest', function (done) {
+    const callback = sinon.stub()
+    const note = {
+      server: {
+        document: '# title\n\n## test2'
+      },
+      authorship: []
+    }
+
+    modelsStub.Note.parseNoteTitle = sinon.stub().returns('title')
+    const updateNoteStub = sinon.stub().returns(Promise.resolve({}))
+    modelsStub.Note.findOne.returns(Promise.resolve({
+      update: updateNoteStub
+    }))
+
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+    clock.tick(1000)
+
+    realtime = require('../../lib/realtime')
+    realtime.updateNote(note, callback)
+    setTimeout(() => {
+      assert(modelsStub.User.findOne.callCount === 0)
+      assert(note.lastchangeuserprofile === null)
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === null)
+      assert(updateNoteStub.calledOnce)
+      assert(updateNoteStub.lastCall.args[0].lastchangeAt === now + 1000)
+      assert(updateNoteStub.lastCall.args[0].title === 'title')
+      assert(updateNoteStub.lastCall.args[0].content === '# title\n\n## test2')
+      done()
+    }, 50)
+  })
+
+  it('should save note when lastChangeUser as same as database', function (done) {
+    const callback = sinon.stub()
+    const note = {
+      lastchangeuser: 'user1',
+      server: {
+        document: '# title\n\n## test2'
+      },
+      authorship: []
+    }
+
+    modelsStub.Note.parseNoteTitle = sinon.stub().returns('title')
+    const updateNoteStub = sinon.stub().returns(Promise.resolve({}))
+    modelsStub.Note.findOne.returns(Promise.resolve({
+      update: updateNoteStub,
+      lastchangeuserId: 'user1'
+    }))
+
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+    clock.tick(1000)
+
+    realtime = require('../../lib/realtime')
+    realtime.updateNote(note, callback)
+    setTimeout(() => {
+      assert(modelsStub.User.findOne.callCount === 0)
+      assert(modelsStub.User.getProfile.callCount === 0)
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === null)
+      assert(updateNoteStub.calledOnce)
+      assert(updateNoteStub.lastCall.args[0].lastchangeAt === now + 1000)
+      assert(updateNoteStub.lastCall.args[0].lastchangeuserId === 'user1')
+      assert(updateNoteStub.lastCall.args[0].title === 'title')
+      assert(updateNoteStub.lastCall.args[0].content === '# title\n\n## test2')
+      done()
+    }, 50)
+  })
+
+  it('should not save note when lastChangeUser not found in database', function (done) {
+    const callback = sinon.stub()
+    const note = {
+      lastchangeuser: 'user1',
+      server: {
+        document: '# title\n\n## test2'
+      },
+      authorship: []
+    }
+
+    modelsStub.Note.parseNoteTitle = sinon.stub().returns('title')
+    const updateNoteStub = sinon.stub().returns(Promise.resolve({}))
+    modelsStub.Note.findOne.returns(Promise.resolve({
+      update: updateNoteStub
+    }))
+    modelsStub.User.findOne.returns(Promise.resolve(null))
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+    clock.tick(1000)
+
+    realtime = require('../../lib/realtime')
+    realtime.updateNote(note, callback)
+    setTimeout(() => {
+      assert(modelsStub.User.findOne.called)
+      assert(modelsStub.User.getProfile.callCount === 0)
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === null)
+      assert(callback.lastCall.args[1] === null)
+      assert(updateNoteStub.callCount === 0)
+      done()
+    }, 50)
+  })
+
+  it('should not save note when note.server not exists', function (done) {
+    const callback = sinon.stub()
+    const note = {
+      lastchangeuser: 'user1',
+      authorship: []
+    }
+
+    modelsStub.Note.parseNoteTitle = sinon.stub().returns('title')
+    const updateNoteStub = sinon.stub().returns(Promise.resolve({}))
+    modelsStub.Note.findOne.returns(Promise.resolve({
+      update: updateNoteStub
+    }))
+
+    modelsStub.User.findOne.withArgs({
+      where: {
+        id: 'user1'
+      }
+    }).returns(Promise.resolve({
+      id: 'user1',
+      profile: '{ "displayName": "User 01" }'
+    }))
+    modelsStub.User.getProfile = sinon.stub().returns({
+      name: 'User 01'
+    })
+    clock.tick(1000)
+
+    realtime = require('../../lib/realtime')
+    realtime.updateNote(note, callback)
+    setTimeout(() => {
+      assert(note.lastchangeuserprofile.name === 'User 01')
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === null)
+      assert(callback.lastCall.args[1] === null)
+      assert(updateNoteStub.callCount === 0)
+      done()
+    }, 50)
+  })
+})

test/testDoubles/loggerFake.js

@@ -0,0 +1,15 @@
+'use strict'
+
+const sinon = require('sinon')
+
+function createFakeLogger() {
+  return {
+    error: sinon.stub(),
+    warn: sinon.stub(),
+    info: sinon.stub(),
+    debug: sinon.stub(),
+    log: sinon.stub()
+  }
+}
+
+exports.createFakeLogger = createFakeLogger