From 81f25c0cd57643126de39b1ca04ebbdae3352bbd Mon Sep 17 00:00:00 2001 From: deiu Date: Mon, 25 Jan 2016 15:39:41 -0500 Subject: [PATCH 1/7] rewriting delete for containers --- lib/ldp.js | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/ldp.js b/lib/ldp.js index d85b2ce92..ba17f51c1 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -15,6 +15,7 @@ var error = require('./http-error') var stringToStream = require('./utils').stringToStream var extend = require('extend') var doWhilst = require('async').doWhilst +var rimraf = require('rimraf') var turtleExtension = '.ttl' function LDP (argv) { @@ -424,24 +425,44 @@ LDP.prototype.delete = function (host, resourcePath, callback) { } if (stats.isDirectory()) { - return ldp.deleteContainerMetadata(filename, callback) + return ldp.deleteContainer(filename, callback) } else { return ldp.deleteResource(filename, callback) } }) } -LDP.prototype.deleteContainerMetadata = function (directory, callback) { +LDP.prototype.deleteContainer = function (directory, callback) { + var self = this if (directory[directory.length - 1] !== '/') { directory += '/' } - return fs.unlink(directory + this.suffixMeta, function (err, data) { - if (err) { - debug.container('DELETE -- unlink() error: ' + err) - return callback(error(err, 'Failed to delete container')) + // TODO + // readdir and if + // check if acl exist, check if meta exist + var countValid = 0 + fs.readdir(directory, function (err, list) { + console.log(list) + if (err) return callback(error(404, 'The container does not exist')) + + if (list.indexOf(self.suffixMeta) > -1) { + countValid++ } - return callback(null, data) + + if (list.indexOf(self.suffixAcl) > -1) { + countValid++ + } + + console.log(countValid, list, list.length) + if (list.length !== countValid) { + return callback(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) + }) }) } From e11c6a3cbc1d0b0384f87adb76fe920a6cbed24b Mon Sep 17 00:00:00 2001 From: deiu Date: Mon, 25 Jan 2016 16:15:31 -0500 Subject: [PATCH 2/7] Removed console.log() prints --- lib/ldp.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/ldp.js b/lib/ldp.js index ba17f51c1..12da8f8b4 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -443,7 +443,6 @@ LDP.prototype.deleteContainer = function (directory, callback) { // check if acl exist, check if meta exist var countValid = 0 fs.readdir(directory, function (err, list) { - console.log(list) if (err) return callback(error(404, 'The container does not exist')) if (list.indexOf(self.suffixMeta) > -1) { @@ -454,7 +453,6 @@ LDP.prototype.deleteContainer = function (directory, callback) { countValid++ } - console.log(countValid, list, list.length) if (list.length !== countValid) { return callback(error(409, 'Container is not empty')) } From 3769aee0f509bfac9535fb4e3dc9abed3b4bacb1 Mon Sep 17 00:00:00 2001 From: deiu Date: Mon, 25 Jan 2016 16:18:07 -0500 Subject: [PATCH 3/7] Added more tests for DELETE --- test/http.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/http.js b/test/http.js index b6b4a104a..00c6de717 100644 --- a/test/http.js +++ b/test/http.js @@ -288,6 +288,35 @@ describe('HTTP APIs', function () { server.delete('/put-resource-1.ttl') .expect(200, done) }) + it('should fail to delete non-empty containers', function (done) { + server.put('/fooo/bar.ttl') + .set('content-type', 'text/turtle') + .expect(function () { + server.delete('/fooo/') + .expect(function () { + fs.unlinkSync(__dirname + '/resources/fooo/bar.ttl') + fs.rmdirSync(__dirname + '/resources/fooo/') + }) + .expect(409) + }) + .expect(201, done) + }) + it('should delete a new and empty container', function (done) { + server.post('/') + .set('content-type', 'text/turtle') + .set('slug', 'fooo') + .set('link', '; rel="type"') + .set('content-type', 'text/turtle') + .expect(function () { + server.delete('/fooo/') + .expect(function () { + server.get('/fooo/') + .expect(404) + }) + .expect(200) + }) + .expect(201, done) + }) }) describe('POST API', function () { From 465897de16df7a5aff903c0ed085d2024b431e23 Mon Sep 17 00:00:00 2001 From: deiu Date: Mon, 25 Jan 2016 16:29:09 -0500 Subject: [PATCH 4/7] Removed todo comments --- lib/ldp.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/ldp.js b/lib/ldp.js index 12da8f8b4..471041484 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -438,9 +438,6 @@ LDP.prototype.deleteContainer = function (directory, callback) { directory += '/' } - // TODO - // readdir and if - // check if acl exist, check if meta exist var countValid = 0 fs.readdir(directory, function (err, list) { if (err) return callback(error(404, 'The container does not exist')) From 798b1cfa70fb92ebe1cecbaefc727cbe97fbc1e3 Mon Sep 17 00:00:00 2001 From: deiu Date: Mon, 25 Jan 2016 16:30:25 -0500 Subject: [PATCH 5/7] Rename container to /foo/ --- test/http.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/http.js b/test/http.js index 00c6de717..77f30d78a 100644 --- a/test/http.js +++ b/test/http.js @@ -292,10 +292,10 @@ describe('HTTP APIs', function () { server.put('/fooo/bar.ttl') .set('content-type', 'text/turtle') .expect(function () { - server.delete('/fooo/') + server.delete('/foo/') .expect(function () { - fs.unlinkSync(__dirname + '/resources/fooo/bar.ttl') - fs.rmdirSync(__dirname + '/resources/fooo/') + fs.unlinkSync(__dirname + '/resources/foo/bar.ttl') + fs.rmdirSync(__dirname + '/resources/foo/') }) .expect(409) }) @@ -304,13 +304,13 @@ describe('HTTP APIs', function () { it('should delete a new and empty container', function (done) { server.post('/') .set('content-type', 'text/turtle') - .set('slug', 'fooo') + .set('slug', 'foo') .set('link', '; rel="type"') .set('content-type', 'text/turtle') .expect(function () { - server.delete('/fooo/') + server.delete('/foo/') .expect(function () { - server.get('/fooo/') + server.get('/foo/') .expect(404) }) .expect(200) From b29eca10a391bb7e776abd417a9b80b8048dead5 Mon Sep 17 00:00:00 2001 From: Dmitri Zagidulin Date: Tue, 26 Jan 2016 10:06:22 -0500 Subject: [PATCH 6/7] Clean up Delete API tests For PR #215 - Include RSVP promise lib - Extract test container and resource creation to helper methods - Pass failing tests --- package.json | 1 + test/http.js | 122 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 83 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index 669a5d6da..baed51a96 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "chai": "^3.0.0", "mocha": "^2.2.5", "nock": "^2.10.0", + "rsvp": "^3.1.0", "standard": "^5.4.1", "supertest": "^1.0.1" }, diff --git a/test/http.js b/test/http.js index 77f30d78a..8a183ead9 100644 --- a/test/http.js +++ b/test/http.js @@ -3,6 +3,51 @@ var fs = require('fs') var li = require('li') var ldnode = require('../index') var rm = require('./test-utils').rm +var RSVP = require('rsvp') + +var suffixAcl = '.acl' +var suffixMeta = '.meta' +var ldpServer = ldnode({ + root: __dirname + '/resources' +}) +var server = supertest(ldpServer) + +/** + * Creates a new test basic container via an LDP POST + * (located in `test/resources/{containerName}`) + * @method createTestContainer + * @param containerName {String} Container name used as slug, no leading `/` + * @return {RSVP.Promise} Promise obj, for use with Mocha's `before()` etc + */ +function createTestContainer (containerName) { + return new RSVP.Promise(function (resolve, reject) { + server.post('/') + .set('content-type', 'text/turtle') + .set('slug', containerName) + .set('link', '; rel="type"') + .set('content-type', 'text/turtle') + .end(function (error, res) { + error ? reject(error) : resolve(res) + }) + }) +} + +/** + * Creates a new turtle test resource via an LDP PUT + * (located in `test/resources/{containerName}`) + * @method createTestResource + * @param resourceName {String} Resource name (should have a leading `/`) + * @return {RSVP.Promise} Promise obj, for use with Mocha's `before()` etc + */ +function createTestResource (resourceName) { + return new RSVP.Promise(function (resolve, reject) { + server.put(resourceName) + .set('content-type', 'text/turtle') + .end(function (error, res) { + error ? reject(error) : resolve(res) + }) + }) +} describe('HTTP APIs', function () { var emptyResponse = function (res) { @@ -35,15 +80,6 @@ describe('HTTP APIs', function () { return handler } - var ldpServer = ldnode({ - root: __dirname + '/resources' - }) - - var suffixAcl = '.acl' - var suffixMeta = '.meta' - - var server = supertest(ldpServer) - describe('GET Root container', function () { it('should have Access-Control-Allow-Origin as the req.Origin', function (done) { server.get('/') @@ -265,57 +301,63 @@ describe('HTTP APIs', function () { .set('content-type', 'text/turtle') .expect(hasHeader('describedBy', 'baz.ttl' + suffixMeta)) .expect(hasHeader('acl', 'baz.ttl' + suffixAcl)) - .expect(function () { - fs.unlinkSync(__dirname + '/resources/foo/bar/baz.ttl') - fs.rmdirSync(__dirname + '/resources/foo/bar/') - fs.rmdirSync(__dirname + '/resources/foo/') - }) .expect(201, done) }) - it('should return 409 code when trying to put to a container', function (done) { - server.put('/') - .expect(409, done) + it('should return 409 code when trying to put to a container', + function (done) { + server.put('/') + .expect(409, done) + } + ) + // Cleanup + after(function () { + rm('/foo/') }) }) describe('DELETE API', function () { + before(function () { + // Ensure all these are finished before running tests + return RSVP.all([ + rm('/false-file-48484848'), + createTestContainer('delete-test-empty-container'), + createTestResource('/put-resource-1.ttl'), + createTestContainer('delete-test-non-empty'), + createTestResource('/delete-test-non-empty/test.ttl') + ]) + }) + it('should return 404 status when deleting a file that does not exists', function (done) { server.delete('/false-file-48484848') .expect(404, done) }) + it('should delete previously PUT file', function (done) { server.delete('/put-resource-1.ttl') .expect(200, done) }) + it('should fail to delete non-empty containers', function (done) { - server.put('/fooo/bar.ttl') - .set('content-type', 'text/turtle') - .expect(function () { - server.delete('/foo/') - .expect(function () { - fs.unlinkSync(__dirname + '/resources/foo/bar.ttl') - fs.rmdirSync(__dirname + '/resources/foo/') - }) - .expect(409) - }) - .expect(201, done) + server.delete('/delete-test-non-empty/') + .expect(409, done) }) + it('should delete a new and empty container', function (done) { - server.post('/') - .set('content-type', 'text/turtle') - .set('slug', 'foo') - .set('link', '; rel="type"') - .set('content-type', 'text/turtle') + server.delete('/delete-test-empty-container/') .expect(function () { - server.delete('/foo/') - .expect(function () { - server.get('/foo/') - .expect(404) - }) - .expect(200) + // Ensure container was deleted + server.get('/delete-test-empty-container/') + .expect(404) }) - .expect(201, done) + .end(done) + }) + + after(function () { + // Clean up after DELETE API tests + rm('/put-resource-1.ttl') + rm('/delete-test-non-empty/') + rm('/delete-test-empty-container/') }) }) From 711b03a43e9e3787379e3291fbf8e5ec2fa41b9b Mon Sep 17 00:00:00 2001 From: Dmitri Zagidulin Date: Tue, 26 Jan 2016 10:11:39 -0500 Subject: [PATCH 7/7] Fix param name --- test/http.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/http.js b/test/http.js index 8a183ead9..7fd363ad6 100644 --- a/test/http.js +++ b/test/http.js @@ -34,7 +34,7 @@ function createTestContainer (containerName) { /** * Creates a new turtle test resource via an LDP PUT - * (located in `test/resources/{containerName}`) + * (located in `test/resources/{resourceName}`) * @method createTestResource * @param resourceName {String} Resource name (should have a leading `/`) * @return {RSVP.Promise} Promise obj, for use with Mocha's `before()` etc