From 06c4c0f74f5abd2df18b0d156da71ee29dfae575 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Mon, 16 Mar 2020 11:35:01 +0000
Subject: [PATCH] Fix incorrect key when deleting projects

---
 services/filestore/app/js/Errors.js                   |  4 +++-
 services/filestore/app/js/FileController.js           | 11 +++++++----
 services/filestore/app/js/FileHandler.js              |  8 ++++----
 .../filestore/test/unit/js/FileControllerTests.js     |  2 +-
 services/filestore/test/unit/js/FileHandlerTests.js   |  7 +++----
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/services/filestore/app/js/Errors.js b/services/filestore/app/js/Errors.js
index 445b666e17..1beefb79c8 100644
--- a/services/filestore/app/js/Errors.js
+++ b/services/filestore/app/js/Errors.js
@@ -25,6 +25,7 @@ class ConversionsDisabledError extends BackwardCompatibleError {}
 class ConversionError extends BackwardCompatibleError {}
 class SettingsError extends BackwardCompatibleError {}
 class TimeoutError extends BackwardCompatibleError {}
+class InvalidParametersError extends BackwardCompatibleError {}
 
 class FailedCommandError extends OError {
   constructor(command, code, stdout, stderr) {
@@ -50,5 +51,6 @@ module.exports = {
   ConversionError,
   HealthCheckError,
   SettingsError,
-  TimeoutError
+  TimeoutError,
+  InvalidParametersError
 }
diff --git a/services/filestore/app/js/FileController.js b/services/filestore/app/js/FileController.js
index 9ddafb9f69..0e663f9421 100644
--- a/services/filestore/app/js/FileController.js
+++ b/services/filestore/app/js/FileController.js
@@ -161,13 +161,16 @@ function deleteFile(req, res, next) {
 
 function deleteProject(req, res, next) {
   metrics.inc('deleteProject')
-  const { project_id: projectId, bucket } = req
+  const { key, bucket } = req
 
-  req.requestLogger.setMessage('getting project size')
-  req.requestLogger.addFields({ projectId, bucket })
+  req.requestLogger.setMessage('deleting project')
+  req.requestLogger.addFields({ key, bucket })
 
-  FileHandler.deleteProject(bucket, projectId, function(err) {
+  FileHandler.deleteProject(bucket, key, function(err) {
     if (err) {
+      if (err instanceof Errors.InvalidParametersError) {
+        return res.sendStatus(400)
+      }
       next(err)
     } else {
       res.sendStatus(204)
diff --git a/services/filestore/app/js/FileHandler.js b/services/filestore/app/js/FileHandler.js
index 40bdc95e34..a6032350b1 100644
--- a/services/filestore/app/js/FileHandler.js
+++ b/services/filestore/app/js/FileHandler.js
@@ -5,7 +5,7 @@ const LocalFileWriter = require('./LocalFileWriter')
 const FileConverter = require('./FileConverter')
 const KeyBuilder = require('./KeyBuilder')
 const ImageOptimiser = require('./ImageOptimiser')
-const { ConversionError, WriteError } = require('./Errors')
+const { ConversionError, InvalidParametersError } = require('./Errors')
 
 module.exports = {
   insertFile: callbackify(insertFile),
@@ -27,7 +27,7 @@ module.exports = {
 async function insertFile(bucket, key, stream) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
   if (!convertedKey.match(/^[0-9a-f]{24}\/[0-9a-f]{24}/i)) {
-    throw new WriteError({
+    throw new InvalidParametersError({
       message: 'key does not match validation regex',
       info: { bucket, key, convertedKey }
     })
@@ -39,7 +39,7 @@ async function insertFile(bucket, key, stream) {
 async function deleteFile(bucket, key) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
   if (!convertedKey.match(/^[0-9a-f]{24}\/[0-9a-f]{24}/i)) {
-    throw new WriteError({
+    throw new InvalidParametersError({
       message: 'key does not match validation regex',
       info: { bucket, key, convertedKey }
     })
@@ -52,7 +52,7 @@ async function deleteFile(bucket, key) {
 
 async function deleteProject(bucket, key) {
   if (!key.match(/^[0-9a-f]{24}\//i)) {
-    throw new WriteError({
+    throw new InvalidParametersError({
       message: 'key does not match validation regex',
       info: { bucket, key }
     })
diff --git a/services/filestore/test/unit/js/FileControllerTests.js b/services/filestore/test/unit/js/FileControllerTests.js
index 4a99a875a9..16fbb3641c 100644
--- a/services/filestore/test/unit/js/FileControllerTests.js
+++ b/services/filestore/test/unit/js/FileControllerTests.js
@@ -263,7 +263,7 @@ describe('FileController', function() {
     it('should tell the file handler', function(done) {
       res.sendStatus = code => {
         code.should.equal(204)
-        expect(FileHandler.deleteProject).to.have.been.calledWith(bucket, projectId)
+        expect(FileHandler.deleteProject).to.have.been.calledWith(bucket, key)
         done()
       }
       FileController.deleteProject(req, res, next)
diff --git a/services/filestore/test/unit/js/FileHandlerTests.js b/services/filestore/test/unit/js/FileHandlerTests.js
index acd3b8fc86..7823c9454f 100644
--- a/services/filestore/test/unit/js/FileHandlerTests.js
+++ b/services/filestore/test/unit/js/FileHandlerTests.js
@@ -159,10 +159,9 @@ describe('FileHandler', function() {
     it('should tell the filestore manager to delete the folder', function(done) {
       FileHandler.deleteProject(bucket, projectKey, err => {
         expect(err).not.to.exist
-        expect(PersistorManager.promises.deleteDirectory).to.have.been.calledWith(
-          bucket,
-          projectKey
-        )
+        expect(
+          PersistorManager.promises.deleteDirectory
+        ).to.have.been.calledWith(bucket, projectKey)
         done()
       })
     })