core/merge: Treat move before upload as addition

  If a new file is moved after we've detected the addition but before we
  could upload it, we should not try to apply the following move as the
  source file does not exist on the filesystem anymore but instead treat
  it as an addition of the destition file and remove the source file in
  Pouch.

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

@@ -125,7 +125,7 @@ describe('Move', () => {
       await helpers.local.scan()
     })
 
-    it.only('local', async () => {
+    it('local', async () => {
       should(await helpers.local.tree()).deepEqual([
         'dst/',
         'src/',
@@ -139,12 +139,18 @@ describe('Move', () => {
       await helpers.local.scan()
       await helpers.syncAll()
 
-      should(await helpers.remote.tree()).deepEqual([
-        '.cozy_trash/',
-        'dst/',
-        'dst/file',
-        'src/'
-      ])
+      should(await helpers.trees('metadata', 'remote')).deepEqual({
+        remote: [
+          'dst/',
+          'dst/file',
+          'src/'
+        ],
+        metadata: [
+          'dst/',
+          'dst/file',
+          'src/'
+        ]
+      })
     })
   })
 

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