feat: export ctx.cleanupRequestFiles to improve cleanup more easy (#22)

.gitignore

@@ -4,3 +4,4 @@ node_modules/
 coverage/
 test/fixtures/**/run
 .idea/
+.nyc_output/

README.md

@@ -123,7 +123,8 @@ The usage very similar to [bodyParser](https://eggjs.org/en/basics/controller.ht
 - `ctx.request.body`: Get all the multipart fields and values, except `file`.
 - `ctx.request.files`: Contains all `file` from the multipart request, it's an Array object.
 
-**WARNING: you should remove the temporary upload file after you use it**
+**WARNING: you should remove the temporary upload files after you use it**,
+the `async ctx.cleanupRequestFiles()` method will be very helpful.
 
 ### Enable `file` mode on config
 
@@ -177,8 +178,8 @@ module.exports = class extends Controller {
       // process file or upload to cloud storage
       result = await ctx.oss.put(name, file.filepath);
     } finally {
-      // need to remove the tmp file
-      await fs.unlink(file.filepath);
+      // need to remove the tmp files
+      await ctx.cleanupRequestFiles();
     }
 
     ctx.body = {
@@ -224,8 +225,8 @@ module.exports = class extends Controller {
         // process file or upload to cloud storage
         result = await ctx.oss.put('egg-multipart-test/' + file.filename, file.filepath);
       } finally {
-        // need to remove the tmp file
-        await fs.unlink(file.filepath);
+        // need to remove the tmp files
+        await ctx.cleanupRequestFiles();
       }
       console.log(result);
     }

app/extend/context.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const rimraf = require('mz-modules/rimraf');
 const parse = require('co-busboy');
 const Readable = require('stream').Readable;
 
@@ -12,6 +13,28 @@ class EmptyStream extends Readable {
 const HAS_CONSUMED = Symbol('Context#multipartHasConsumed');
 
 module.exports = {
+  /**
+   * clean up request tmp files helper
+   * @method Context#cleanupRequestFiles
+   * @param {Array<String>} [files] - file paths need to clenup, default is `ctx.request.files`.
+   */
+  async cleanupRequestFiles(files) {
+    if (!files || !files.length) {
+      files = this.request.files;
+    }
+    if (Array.isArray(files)) {
+      for (const file of files) {
+        try {
+          await rimraf(file.filepath);
+        } catch (err) {
+          // warning log
+          this.coreLogger.warn('[egg-multipart-cleanupRequestFiles-error] file: %j, error: %s',
+            file, err);
+        }
+      }
+    }
+  },
+
   /**
    * create multipart.parts instance, to get separated files.
    * @method Context#multipart

app/middleware/multipart.js

@@ -9,20 +9,7 @@ const sendToWormhole = require('stream-wormhole');
 const moment = require('moment');
 
 module.exports = options => {
-  async function cleanup(requestFiles) {
-    for (const file of requestFiles) {
-      try {
-        await fs.unlink(file.filepath);
-      } catch (_) {
-        // do nothing
-      }
-    }
-  }
-
-  async function limit(requestFiles, code, message) {
-    // cleanup requestFiles
-    await cleanup(requestFiles);
-
+  async function limit(code, message) {
     // throw 413 error
     const err = new Error(message);
     err.code = code;
@@ -44,7 +31,7 @@ module.exports = options => {
       try {
         part = await parts();
       } catch (err) {
-        await cleanup(requestFiles);
+        await ctx.cleanupRequestFiles(requestFiles);
         throw err;
       }
 
@@ -55,10 +42,12 @@ module.exports = options => {
         const fieldnameTruncated = part[2];
         const valueTruncated = part[3];
         if (valueTruncated) {
-          return await limit(requestFiles, 'Request_fieldSize_limit', 'Reach fieldSize limit');
+          await ctx.cleanupRequestFiles(requestFiles);
+          return await limit('Request_fieldSize_limit', 'Reach fieldSize limit');
         }
         if (fieldnameTruncated) {
-          return await limit(requestFiles, 'Request_fieldNameSize_limit', 'Reach fieldNameSize limit');
+          await ctx.cleanupRequestFiles(requestFiles);
+          return await limit('Request_fieldNameSize_limit', 'Reach fieldNameSize limit');
         }
 
         // arrays are busboy fields
@@ -97,7 +86,8 @@ module.exports = options => {
 
       // https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L221
       if (part.truncated) {
-        return await limit(requestFiles, 'Request_fileSize_limit', 'Reach fileSize limit');
+        await ctx.cleanupRequestFiles(requestFiles);
+        return await limit('Request_fileSize_limit', 'Reach fileSize limit');
       }
     } while (part != null);
 

test/file-mode.test.js

@@ -289,6 +289,25 @@ describe('test/file-mode.test.js', () => {
     assert(res.data.toString().includes('TypeError: the multipart request can\'t be consumed twice'));
   });
 
+  it('should use cleanupRequestFiles after request end', async () => {
+    const form = formstream();
+    form.field('foo', 'fengmk2').field('love', 'egg');
+    form.file('file2', __filename);
+    // other form fields
+    form.field('work', 'with Node.js');
+
+    const headers = form.headers();
+    const res = await urllib.request(host + '/upload?cleanup=true', {
+      method: 'POST',
+      headers,
+      stream: form,
+    });
+
+    assert(res.status === 200);
+    const data = JSON.parse(res.data);
+    assert(data.files.length === 1);
+  });
+
   describe('schedule/clean_tmpdir', () => {
     it('should remove nothing', async () => {
       app.mockLog();

test/fixtures/apps/file-mode/app/controller/upload.js

@@ -6,6 +6,10 @@ module.exports = async ctx => {
     files: ctx.request.files,
   };
 
+  if (ctx.query.cleanup === 'true') {
+    await ctx.cleanupRequestFiles();
+  }
+
   if (ctx.query.call_multipart_twice) {
     ctx.multipart();
   }