Use ResourceMapper in ldp#delete

Author: rubensworks

lib/handlers/delete.js

@@ -6,13 +6,14 @@ function handler (req, res, next) {
   debug('DELETE -- Request on' + req.originalUrl)
 
   const ldp = req.app.locals.ldp
-  ldp.delete(req.hostname, req.path, function (err) {
-    if (err) {
+  ldp.delete(req)
+    .then(() => {
+      debug('DELETE -- Ok.')
+      res.sendStatus(200)
+      next()
+    })
+    .catch(err => {
       debug('DELETE -- Failed to delete: ' + err)
-      return next(err)
-    }
-    debug('DELETE -- Ok.')
-    res.sendStatus(200)
-    return next()
-  })
+      next(err)
+    })
 }

lib/ldp.js

@@ -432,58 +432,58 @@ class LDP {
     )
   }
 
-  delete (host, resourcePath, callback) {
-    const ldp = this
-    const root = !ldp.multiuser ? ldp.root : ldp.root + host + '/'
-    const filename = utils.uriToFilename(resourcePath, root)
-    ldp.stat(filename).then(stats => {
-      if (stats.isDirectory()) {
-        return ldp.deleteContainer(filename, callback)
-      } else {
-        return ldp.deleteResource(filename, callback)
-      }
-    },
-      err => callback(error(404, "Can't find " + err))
-    )
+  async delete (url) {
+    const { path } = await this.resourceMapper.mapUrlToFile({ url })
+
+    // First check if the path points to a valid file
+    let stats
+    try {
+      stats = await this.stat(path)
+    } catch (err) {
+      throw error(404, "Can't find " + err)
+    }
+
+    // If so, delete the directory or file
+    if (stats.isDirectory()) {
+      return this.deleteContainer(path)
+    } else {
+      return this.deleteResource(path)
+    }
   }
 
-  deleteContainer (directory, callback) {
-    const self = this
+  async deleteContainer (directory) {
     if (directory[ directory.length - 1 ] !== '/') {
       directory += '/'
     }
 
-    let countValid = 0
-    fs.readdir(directory, function (err, list) {
-      if (err) return callback(error(404, 'The container does not exist'))
-
-      if (list.indexOf(self.suffixMeta) > -1) {
-        countValid++
-      }
-
-      if (list.indexOf(self.suffixAcl) > -1) {
-        countValid++
-      }
+    // Ensure the container exists
+    let list
+    try {
+      list = await promisify(fs.readdir)(directory)
+    } catch (err) {
+      throw error(404, 'The container does not exist')
+    }
 
-      if (list.length !== countValid) {
-        return callback(error(409, 'Container is not empty'))
-      }
+    // Ensure the container is empty (we ignore .meta and .acl)
+    if (list.some(file => file !== this.suffixMeta && file !== this.suffixAcl)) {
+      throw error(409, 'Container is not empty')
+    }
 
-      return rimraf(directory, function (err) {
-        if (err) return callback(error(err, 'Failed to delete the container'))
-        return callback(null)
-      })
-    })
+    // Delete the directory recursively
+    try {
+      await promisify(rimraf)(directory)
+    } catch (err) {
+      throw error(err, 'Failed to delete the container')
+    }
   }
 
-  deleteResource (filename, callback) {
-    return fs.unlink(filename, function (err, data) {
-      if (err) {
-        debug.container('DELETE -- unlink() error: ' + err)
-        return callback(error(err, 'Failed to delete resource'))
-      }
-      return callback(null, data)
-    })
+  async deleteResource (filename) {
+    try {
+      return promisify(fs.unlink)(filename)
+    } catch (err) {
+      debug.container('DELETE -- unlink() error: ' + err)
+      throw error(err, 'Failed to delete resource')
+    }
   }
 
   getAvailablePath (host, containerURI, { slug = uuid.v1(), extension }) {

test/integration/ldp-test.js

@@ -1,4 +1,6 @@
-var assert = require('chai').assert
+var chai = require('chai')
+var assert = chai.assert
+chai.use(require('chai-as-promised'))
 var $rdf = require('rdflib')
 var ns = require('solid-namespace')($rdf)
 var LDP = require('../../lib/ldp')
@@ -153,21 +155,67 @@ describe('LDP', function () {
   })
 
   describe('delete', function () {
-    it.skip('should delete a file in an existing dir', function (done) {
+    it('should error when deleting a non-existing file', () => {
+      return assert.isRejected(ldp.delete('/resources/testPut.txt'))
+    })
+
+    it.skip('should delete a file in an existing dir', async () => {
+      // First create a dummy file
       var stream = stringToStream('hello world')
-      ldp.put('/resources/testPut.txt', stream).then(() => {
-        fs.stat(ldp.root + '/resources/testPut.txt', function (err) {
-          if (err) {
-            return done(err)
-          }
-          ldp.delete('localhost', '/resources/testPut.txt', function (err) {
-            if (err) done(err)
-            fs.stat(ldp.root + '/resources/testPut.txt', function (err) {
-              return done(err ? null : new Error('file still exists'))
-            })
-          })
-        })
+      await ldp.put('/resources/testPut.txt', stream)
+      // Make sure it exists
+      fs.stat(ldp.root + '/resources/testPut.txt', function (err) {
+        if (err) {
+          throw err
+        }
+      })
+
+      // Now delete the dummy file
+      await ldp.delete('/resources/testPut.txt')
+      // Make sure it does not exist anymore
+      fs.stat(ldp.root + '/resources/testPut.txt', function (err, s) {
+        if (!err) {
+          throw new Error('file still exists')
+        }
+      })
+    })
+
+    it.skip('should fail to delete a non-empty folder', async () => {
+      // First create a dummy file
+      var stream = stringToStream('hello world')
+      await ldp.put('/resources/dummy/testPutBlocking.txt', stream)
+      // Make sure it exists
+      fs.stat(ldp.root + '/resources/dummy/testPutBlocking.txt', function (err) {
+        if (err) {
+          throw err
+        }
       })
+
+      // Now try to delete its folder
+      return assert.isRejected(ldp.delete('/resources/dummy/'))
+    })
+
+    it.skip('should fail to delete nested non-empty folders', async () => {
+      // First create a dummy file
+      var stream = stringToStream('hello world')
+      await ldp.put('/resources/dummy/dummy2/testPutBlocking.txt', stream)
+      // Make sure it exists
+      fs.stat(ldp.root + '/resources/dummy/dummy2/testPutBlocking.txt', function (err) {
+        if (err) {
+          throw err
+        }
+      })
+
+      // Now try to delete its parent folder
+      return assert.isRejected(ldp.delete('/resources/dummy/'))
+    })
+
+    after(async function () {
+      // Clean up after delete tests
+      await ldp.delete('/resources/dummy/testPutBlocking.txt')
+      await ldp.delete('/resources/dummy/dummy2/testPutBlocking.txt')
+      await ldp.delete('/resources/dummy/dummy2/')
+      await ldp.delete('/resources/dummy/')
     })
   })
   describe('listContainer', function () {