Merge pull request #1266 from cozy-labs/move-merged-file-before-sync

core/merge: Treat move before upload as addition

core/merge.js

@@ -1,7 +1,8 @@
 /* @flow */
 
 const autoBind = require('auto-bind')
-const { clone } = require('lodash')
+const _ = require('lodash')
+const { clone } = _
 const { basename, dirname, extname, join } = require('path')
 
 const IdConflict = require('./IdConflict')
@@ -14,7 +15,8 @@ const {
   markSide,
   sameBinary,
   sameFile,
-  sameFolder
+  sameFolder,
+  upToDate
 } = require('./metadata')
 const move = require('./move')
 const { otherSide } = require('./side')
@@ -262,10 +264,13 @@ class Merge {
   }
 
   // Rename or move a file
-  async moveFileAsync (side /*: SideName */, doc /*: Metadata */, was /*: Metadata */) {
+  async moveFileAsync (side /*: SideName */, doc /*: Metadata */, was /*: Metadata */) /*: Promise<*> */ {
     log.debug({path: doc.path, oldpath: was.path}, 'moveFileAsync')
     const {path} = doc
-    if (was.sides && was.sides[side]) {
+    if (was.sides && !was.sides[otherSide(side)]) {
+      await this.pouch.remove(upToDate(was))
+      return this.addFileAsync(side, doc)
+    } else if (was.sides && was.sides[side]) {
       const file /*: ?Metadata */ = await this.pouch.byIdMaybeAsync(doc._id)
       const idConflict /*: ?IdConflictInfo */ = IdConflict.detect(side, doc, file)
       if (idConflict) {

core/metadata.js

@@ -103,7 +103,8 @@ module.exports = {
   sameBinary,
   markSide,
   buildDir,
-  buildFile
+  buildFile,
+  upToDate
 }
 
 function isFile (doc /*: Metadata */) /*: bool */ {
@@ -179,6 +180,8 @@ function invariants (doc /*: Metadata */) {
   let err
   if (!doc.sides) {
     err = new Error(`${doc._id} has no sides`)
+  } else if (doc._deleted && isUpToDate('local', doc) && isUpToDate('remote', doc)) {
+    err = null
   } else if (doc.sides.remote && !doc.remote) {
     err = new Error(`${doc._id} has 'sides.remote' but no remote`)
   } else if (doc.docType === 'file' && doc.md5sum == null) {
@@ -262,6 +265,13 @@ function markAsUpToDate (doc /*: Metadata */) {
   return rev
 }
 
+function upToDate (doc /*: Metadata */) /*: Metadata */ {
+  const clone = _.clone(doc)
+  const rev = Math.max(clone.sides.local, clone.sides.remote)
+
+  return _.assign(clone, { errors: undefined, sides: { local: rev, remote: rev } })
+}
+
 // Ensure new timestamp is never older than the previous one
 function assignMaxDate (doc /*: Metadata */, was /*: ?Metadata */) {
   if (was == null) return

core/pouch.js

@@ -5,7 +5,8 @@ const Promise = require('bluebird')
 const PouchDB = require('pouchdb')
 const async = require('async')
 const fs = require('fs-extra')
-const { isEqual } = require('lodash')
+const _ = require('lodash')
+const { isEqual } = _
 const path = require('path')
 
 const logger = require('./logger')
@@ -115,6 +116,10 @@ class Pouch {
     return this.db.put(doc).asCallback(callback)
   }
 
+  async remove (doc /*: Metadata */) /*: Promise<*> */ {
+    return this.put(_.defaults({ _deleted: true }, doc))
+  }
+
   // WARNING: bulkDocs is not a transaction, some updates can be applied while
   // others do not.
   // Make sure lock is acquired before using it to avoid conflict.

core/sync.js

@@ -194,6 +194,7 @@ class Sync {
         })
         .on('error', err => {
           if (this.changes) {
+            // FIXME: pas de cancel ici ??
             this.changes = null
             reject(err)
           }

test/integration/move.js

@@ -114,6 +114,46 @@ describe('Move', () => {
     })
   })
 
+  describe('unsynced file', () => {
+    beforeEach(async () => {
+      await helpers.local.syncDir.ensureDir('src')
+      await helpers.local.syncDir.ensureDir('dst')
+      await helpers.local.scan()
+      await helpers.syncAll()
+
+      await helpers.local.syncDir.outputFile('src/file', 'whatever file content')
+      await helpers.local.scan()
+    })
+
+    it('local', async () => {
+      should(await helpers.local.tree()).deepEqual([
+        'dst/',
+        'src/',
+        'src/file'
+      ])
+      await helpers.local.syncDir.move('src/file', 'dst/file')
+      // Sync will fail since file was already moved.
+      await helpers.syncAll()
+      // This will prepend made up unlink event to scan add one, ending up as
+      // the expected move.
+      await helpers.local.scan()
+      await helpers.syncAll()
+
+      should(await helpers.trees('metadata', 'remote')).deepEqual({
+        remote: [
+          'dst/',
+          'dst/file',
+          'src/'
+        ],
+        metadata: [
+          'dst/',
+          'dst/file',
+          'src/'
+        ]
+      })
+    })
+  })
+
   describe('directory', () => {
     let dir, dst, emptySubdir, file, parent, src, subdir
 

test/unit/merge.js

@@ -432,6 +432,38 @@ describe('Merge', function () {
       }
     })
 
+    it('identifies a local move without existing remote side as an addition', async function () {
+      let doc = {
+        _id: 'FOO/NEW',
+        path: 'FOO/NEW',
+        md5sum: 'ba1368789cce95b574dec70dfd476e61cbf00517',
+        docType: 'file',
+        updated_at: new Date(),
+        tags: ['courge', 'quux']
+      }
+      let was = {
+        _id: 'FOO/OLD',
+        path: 'FOO/OLD',
+        md5sum: 'ba1368789cce95b574dec70dfd476e61cbf00517',
+        docType: 'file',
+        updated_at: new Date(),
+        tags: ['courge', 'quux'],
+        sides: {
+          local: 1
+        }
+      }
+      const inserted = await this.pouch.db.put(clone(was))
+      was._rev = inserted.rev
+      await this.merge.moveFileAsync('local', clone(doc), clone(was))
+      const res = await this.pouch.db.get(doc._id)
+      doc.updated_at = doc.updated_at.toISOString()
+      res.should.have.properties(doc)
+      res.sides.local.should.equal(2)
+      should(res.moveFrom).be.undefined()
+      should(res.moveTo).be.undefined()
+      await should(this.pouch.db.get(was._id)).be.rejectedWith({status: 404})
+    })
+
     onPlatforms('win32', 'darwin', () => {
       it('resolves an identity conflict with an existing file', async function () {
         const identical = await builders.file().path('QUX').create()

test/unit/metadata.js

@@ -21,7 +21,8 @@ const {
   sameFolder,
   buildDir,
   buildFile,
-  invariants
+  invariants,
+  upToDate
 } = require('../../core/metadata')
 
 const { platform } = process
@@ -579,20 +580,56 @@ describe('metadata', function () {
       file = await builders.whatever().upToDate().remoteId('badbeef').build()
     })
 
-    it('pouch.put throws when trying to put bad doc (no sides)', async () => {
+    it('throws when trying to put bad doc (no sides)', async () => {
       should(() => invariants(Object.assign(file, {sides: null}))
         ).throw(/sides/)
     })
 
-    it('pouch.put throws when trying to put bad doc (no remote)', async () => {
+    it('throws when trying to put bad doc (no remote)', async () => {
       should(() => invariants(Object.assign(file, {remote: null}))
         ).throw(/sides\.remote/)
     })
 
-    it('pouch.put throws when trying to put bad doc (no md5sum)', async () => {
+    it('throws when trying to put bad doc (no md5sum)', async () => {
       file = await builders.file().upToDate().remoteId('badbeef').build()
       should(() => invariants(Object.assign(file, {md5sum: null}))
         ).throw(/checksum/)
     })
+
+    it('does not throw when trying to put bad doc when deleted and up-to-date', async () => {
+      should(() => invariants(Object.assign(file, {
+        _deleted: true, sides: { local: 0, remote: 0 }, remote: null, md5sum: null
+      }))).not.throw()
+    })
+  })
+
+  describe('upToDate', () => {
+    let doc
+    beforeEach(async () => {
+      const builders = new MetadataBuilders(this.pouch)
+      doc = await builders.whatever().notUpToDate().remoteId('badbeef').build()
+    })
+
+    it('returns a clone of the doc', () => {
+      const clone = upToDate(doc)
+
+      should(clone._id).eql(doc._id)
+      should(clone.path).eql(doc.path)
+
+      doc.path = '/new/doc/path'
+      should(clone.path).not.eql(doc.path)
+    })
+
+    it('returns a doc with both sides equal', () => {
+      const clone = upToDate(doc)
+
+      should(clone.sides.local).eql(clone.sides.remote)
+    })
+
+    it('removes errors', () => {
+      doc.errors = 1
+
+      should(upToDate(doc).errors).be.undefined()
+    })
   })
 })

test/unit/pouch.js

@@ -51,6 +51,82 @@ describe('Pouch', function () {
   })
 
   describe('ODM', function () {
+    describe('put', () => {
+      let doc, old
+
+      beforeEach(async function () {
+        const builders = new MetadataBuilders(this.pouch)
+
+        old = await builders.file().path('doc').create()
+        doc = _.clone(old)
+      })
+
+      it('does not update doc without sides', async function () {
+        _.unset(doc, 'sides')
+
+        await (() => { this.pouch.put(doc) }).should.throw()
+        should((await this.pouch.db.get(doc._id))._rev).equal(old._rev)
+      })
+
+      context('when doc is not deleted', () => {
+        beforeEach(function () {
+          doc._deleted = false
+        })
+
+        it('does not update doc with a remote side and no remote', async function () {
+          _.assign(doc, { remote: undefined, sides: { remote: 1 } })
+
+          await (() => { this.pouch.put(doc) }).should.throw()
+          should((await this.pouch.db.get(doc._id))._rev).equal(old._rev)
+        })
+      })
+
+      context('when doc is not up to date', () => {
+        beforeEach(function () {
+          doc.sides.local = 1
+          doc.sides.remote = 2
+        })
+
+        it('does not update doc with a remote side and no remote', async function () {
+          _.assign(doc, { remote: undefined })
+
+          await (() => { this.pouch.put(doc) }).should.throw()
+          should((await this.pouch.db.get(doc._id))._rev).equal(old._rev)
+        })
+      })
+
+      context('when doc is deleted and up to date', () => {
+        beforeEach(function () {
+          _.assign(doc, { _deleted: true, sides: { local: 1, remote: 1 } })
+        })
+
+        it('updates doc without remote', async function () {
+          _.assign(doc, { remote: undefined })
+
+          await (() => { this.pouch.put(doc) }).should.not.throw()
+          await should(this.pouch.db.get(doc._id)).be.rejectedWith({status: 404})
+          await should(this.pouch.db.get(old._id)).be.rejectedWith({status: 404})
+        })
+      })
+    })
+
+    describe('remove', () => {
+      let doc, old
+
+      beforeEach(async function () {
+        const builders = new MetadataBuilders(this.pouch)
+
+        old = await builders.file().path('doc').create()
+        doc = _.clone(old)
+      })
+
+      it('updates the _deleted attribute of the doc', async function () {
+        await (() => { this.pouch.remove(doc) }).should.not.throw()
+        await should(this.pouch.db.get(doc._id)).be.rejectedWith({status: 404})
+        await should(this.pouch.db.get(old._id)).be.rejectedWith({status: 404})
+      })
+    })
+
     describe('bulkDocs', () => {
       let doc1, doc2, old1, old2