PR feedback 2

src/remote-handlers/abstract.js

@@ -1,4 +1,5 @@
 import eventToPromise from 'event-to-promise'
+import getStream from 'get-stream'
 
 export default class RemoteHandlerAbstract {
   constructor (remote) {
@@ -10,7 +11,7 @@ export default class RemoteHandlerAbstract {
   }
 
   async sync () {
-    return await this._sync()
+    return this._sync()
   }
 
   async _sync () {
@@ -18,7 +19,7 @@ export default class RemoteHandlerAbstract {
   }
 
   async forget () {
-    return await this._forget()
+    return this._forget()
   }
 
   async _forget () {
@@ -26,30 +27,27 @@ export default class RemoteHandlerAbstract {
   }
 
   async outputFile (file, data, options) {
-    return await this._outputFile(file, data, options)
+    return this._outputFile(file, data, options)
   }
 
   async _outputFile (file, data, options) {
-    const stream = this.createOutputStream(file)
+    const stream = await this.createOutputStream(file)
     const promise = eventToPromise(stream, 'finish')
     stream.end(data)
     return promise
   }
 
   async readFile (file, options) {
-    return await this._readFile(file, options)
+    return this._readFile(file, options)
   }
 
   async _readFile (file, options) {
-    const stream = this.createReadStream(file, options)
+    const stream = await this.createReadStream(file, options)
-    let data = ''
+    return getStream(stream)
-    stream.on('data', d => data += d)
-    await eventToPromise(stream, 'end')
-    return data
   }
 
   async rename (oldPath, newPath) {
-    return await this._rename(oldPath, newPath)
+    return this._rename(oldPath, newPath)
   }
 
   async _rename (oldPath, newPath) {
@@ -57,17 +55,21 @@ export default class RemoteHandlerAbstract {
   }
 
   async list (dir = '.') {
-    return await this._list(dir)
+    return this._list(dir)
   }
 
-  async _list (dir = '.') {
+  async _list (dir) {
     throw new Error('Not implemented')
   }
 
   async createReadStream (file, options) {
-    const length = await this.getSize(file)
     const stream = await this._createReadStream(file)
-    stream.length = length
+    if (!('length' in stream) || stream.length === null) {
+      try {
+        const length = await this.getSize(file)
+        stream.length = length
+      } catch (_) {}
+    }
     return stream
   }
 
@@ -76,7 +78,7 @@ export default class RemoteHandlerAbstract {
   }
 
   async createOutputStream (file, options) {
-    return await this._createOutputStream(file, options)
+    return this._createOutputStream(file, options)
   }
 
   async _createOutputStream (file, options) {
@@ -84,7 +86,7 @@ export default class RemoteHandlerAbstract {
   }
 
   async unlink (file) {
-    return await this._unlink(file)
+    return this._unlink(file)
   }
 
   async _unlink (file) {
@@ -92,10 +94,10 @@ export default class RemoteHandlerAbstract {
   }
 
   async getSize (file) {
-    return await this._getSize(file)
+    return this._getSize(file)
   }
 
   async _getSize (file) {
-    throw new Error('Not implement')
+    throw new Error('Not implemented')
   }
 }

src/remote-handlers/local.js

@@ -1,15 +1,18 @@
 import fs from 'fs-promise'
 import RemoteHandlerAbstract from './abstract'
 import startsWith from 'lodash.startswith'
-import {noop} from '../utils'
+import { noop } from '../utils'
-import {dirname, resolve} from 'path'
+import { dirname, resolve } from 'path'
 
 export default class LocalHandler extends RemoteHandlerAbstract {
+  get type () {
+    return 'local'
+  }
+
   _getInfo (remote) {
     if (!startsWith(remote.url, 'file://')) {
       throw new Error('Incorrect remote type')
     }
-    this.type = 'local'
     remote.path = remote.url.split('://')[1]
     remote.path = `/${remote.path}` // FIXME the heading slash has been forgotten on client side
     return remote
@@ -51,29 +54,29 @@ export default class LocalHandler extends RemoteHandlerAbstract {
   }
 
   async _readFile (file, options) {
-    return await fs.readFile(this._getFilePath(file), options)
+    return fs.readFile(this._getFilePath(file), options)
   }
 
   async _rename (oldPath, newPath) {
-    return await fs.rename(this._getFilePath(oldPath), this._getFilePath(newPath))
+    return fs.rename(this._getFilePath(oldPath), this._getFilePath(newPath))
   }
 
   async _list (dir = '.') {
-    return await fs.readdir(this._getFilePath(dir))
+    return fs.readdir(this._getFilePath(dir))
   }
 
   async _createReadStream (file, options) {
-    return await fs.createReadStream(this._getFilePath(file), options)
+    return fs.createReadStream(this._getFilePath(file), options)
   }
 
   async _createOutputStream (file, options) {
     const path = this._getFilePath(file)
     await fs.ensureDir(dirname(path))
-    return await fs.createWriteStream(path, options)
+    return fs.createWriteStream(path, options)
   }
 
   async _unlink (file) {
-    return await fs.unlink(this._getFilePath(file))
+    return fs.unlink(this._getFilePath(file))
   }
 
   async _getSize (file) {

src/remote-handlers/nfs.js

@@ -1,17 +1,18 @@
 import fs from 'fs-promise'
 import LocalHandler from './local'
 import startsWith from 'lodash.startswith'
-import {exec} from 'child_process'
+import execa from 'execa'
-import {forEach, promisify} from '../utils'
+import { forEach } from '../utils'
-
-const execAsync = promisify(exec)
 
 export default class NfsHandler extends LocalHandler {
+  get type () {
+    return 'nfs'
+  }
+
   _getInfo (remote) {
     if (!startsWith(remote.url, 'nfs://')) {
       throw new Error('Incorrect remote type')
     }
-    this.type = 'nfs'
     const url = remote.url.split('://')[1]
     const [host, share] = url.split(':')
     remote.path = '/tmp/xo-server/mounts/' + remote.id
@@ -23,7 +24,7 @@ export default class NfsHandler extends LocalHandler {
   async _loadRealMounts () {
     let stdout
     try {
-      [stdout] = await execAsync('findmnt -P -t nfs,nfs4 --output SOURCE,TARGET --noheadings')
+      [stdout] = await execa('findmnt', ['-P', '-t', 'nfs,nfs4', '--output', 'SOURCE,TARGET', '--noheadings'])
     } catch (exc) {
       // When no mounts are found, the call pretends to fail...
     }
@@ -50,7 +51,7 @@ export default class NfsHandler extends LocalHandler {
 
   async _mount (remote) {
     await fs.ensureDir(remote.path)
-    return await execAsync(`mount -t nfs ${remote.host}:${remote.share} ${remote.path}`)
+    return execa('mount', ['-t', 'nfs', `${remote.host}:${remote.share}`, remote.path])
   }
 
   async _sync () {
@@ -82,6 +83,6 @@ export default class NfsHandler extends LocalHandler {
   }
 
   async _umount (remote) {
-    await execAsync(`umount ${remote.path}`)
+    await execa('umount', [remote.path])
   }
 }

src/remote-handlers/smb.js

@@ -1,7 +1,7 @@
 import RemoteHandlerAbstract from './abstract'
 import Smb2 from '@marsaud/smb2-promise'
 import startsWith from 'lodash.startswith'
-import {noop} from '../utils'
+import { noop } from '../utils'
 
 export default class SmbHandler extends RemoteHandlerAbstract {
   constructor (remote) {
@@ -9,11 +9,14 @@ export default class SmbHandler extends RemoteHandlerAbstract {
     this._forget = noop
   }
 
+  get type () {
+    return 'smb'
+  }
+
   _getInfo (remote) {
     if (!startsWith(remote.url, 'smb://')) {
       throw new Error('Incorrect remote type')
     }
-    this.type = 'smb'
     const url = remote.url.split('://')[1]
     const [auth, smb] = url.split('@')
     const [username, password] = auth.split(':')
@@ -78,7 +81,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
       if (dir) {
         await client.ensureDir(dir)
       }
-      return await client.writeFile(path, data, options)
+      return client.writeFile(path, data, options)
     } finally {
       client.close()
     }
@@ -87,7 +90,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
   async _readFile (file, options) {
     const client = this._getClient(this._remote)
     try {
-      return await client.readFile(this._getFilePath(file), options)
+      return client.readFile(this._getFilePath(file), options)
     } finally {
       client.close()
     }
@@ -96,7 +99,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
   async _rename (oldPath, newPath) {
     const client = this._getClient(this._remote)
     try {
-      return await client.rename(this._getFilePath(oldPath), this._getFilePath(newPath))
+      return client.rename(this._getFilePath(oldPath), this._getFilePath(newPath))
     } finally {
       client.close()
     }
@@ -105,7 +108,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
   async _list (dir = '.') {
     const client = this._getClient(this._remote)
     try {
-      return await client.readdir(this._getFilePath(dir))
+      return client.readdir(this._getFilePath(dir))
     } finally {
       client.close()
     }
@@ -139,7 +142,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
   async _unlink (file) {
     const client = this._getClient(this._remote)
     try {
-      return await client.unlink(this._getFilePath(file))
+      return client.unlink(this._getFilePath(file))
     } finally {
       client.close()
     }
@@ -148,7 +151,7 @@ export default class SmbHandler extends RemoteHandlerAbstract {
   async _getSize (file) {
     const client = await this._getClient(this._remote)
     try {
-      return await client.getSize(this._getFilePath(file))
+      return client.getSize(this._getFilePath(file))
     } finally {
       client.close()
     }