Skip to content

Commit

Permalink
feat: getFileStream() can accept non file request (#17)
Browse files Browse the repository at this point in the history
```js
const stream = await ctx.getFileStream({ required: false });
if (stream.filename) {
  console.log('uploaded ' + stream.filename);
} else {
  console.log('file not exists');
}
```

closes eggjs/egg#2178
  • Loading branch information
fengmk2 committed Aug 7, 2018
1 parent 3a6656c commit 5ece18a
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ sudo: false
language: node_js
node_js:
- '8'
- '9'
- '10'
install:
- npm i npminstall && npminstall
script:
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2017 Alibaba Group Holding Limited and other contributors.
Copyright (c) 2017-present Alibaba Group Holding Limited and other contributors.

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ const Controller = require('egg').Controller;
module.exports = Class UploadController extends Controller {
async upload() {
const ctx = this.ctx;
// file not exists will response 400 error
const stream = await ctx.getFileStream();
const name = 'egg-multipart-test/' + path.basename(stream.filename);
let result;
Expand All @@ -150,6 +151,33 @@ module.exports = Class UploadController extends Controller {
fields: stream.fields,
};
}

async uploadNotRequiredFile() {
const ctx = this.ctx;
// file not required
const stream = await ctx.getFileStream({ requireFile: false });
let result;
if (stream.filename) {
const name = 'egg-multipart-test/' + path.basename(stream.filename);
try {
// process file or upload to cloud storage
result = await ctx.oss.put(name, stream);
} catch (err) {
// must consume the stream, otherwise browser will be stuck.
await sendToWormhole(stream);
throw err;
}
} else {
// must consume the empty stream
await sendToWormhole(stream);
}

ctx.body = {
url: result && result.url,
// process form fields by `stream.fields`
fields: stream.fields,
};
}
};
```

Expand Down
27 changes: 22 additions & 5 deletions app/extend/context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
'use strict';

const parse = require('co-busboy');
const Readable = require('stream').Readable;

class EmptyStream extends Readable {
_read() {
this.push(null);
}
}

module.exports = {
/**
Expand Down Expand Up @@ -28,15 +35,25 @@ module.exports = {
* console.log(stream.fields);
* ```
* @method Context#getFileStream
* @param {Object} options
* - {Boolean} options.requireFile - required file submit, default is true
* @return {ReadStream} stream
* @since 1.0.0
*/
async getFileStream() {
async getFileStream(options) {
options = options || {};
const parts = this.multipart({ autoFields: true });
const stream = await parts();
// stream not exists, treat as an exception
if (!stream || !stream.filename) {
this.throw(400, 'Can\'t found upload file');
let stream = await parts();

if (options.requireFile !== false) {
// stream not exists, treat as an exception
if (!stream || !stream.filename) {
this.throw(400, 'Can\'t found upload file');
}
}

if (!stream) {
stream = new EmptyStream();
}
stream.fields = parts.field;
stream.once('limit', () => {
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
environment:
matrix:
- nodejs_version: '8'
- nodejs_version: '9'
- nodejs_version: '10'

install:
- ps: Install-Product node $env:nodejs_version
Expand Down
16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,28 +40,28 @@
"app.js"
],
"ci": {
"version": "8, 9",
"version": "8, 10",
"license": true
},
"dependencies": {
"co-busboy": "^1.4.0",
"humanize-bytes": "^1.0.1"
},
"devDependencies": {
"autod": "^2.10.1",
"egg": "next",
"egg-bin": "^4.3.5",
"autod": "^3.0.1",
"egg": "^2.9.1",
"egg-bin": "^4.8.1",
"egg-ci": "^1.8.0",
"egg-mock": "^3.13.1",
"eslint": "^4.10.0",
"eslint-config-egg": "^5.1.1",
"egg-mock": "^3.18.0",
"eslint": "^5.2.0",
"eslint-config-egg": "^7.0.0",
"formstream": "^1.1.0",
"is-type-of": "^1.0.0",
"mkdirp": "^0.5.1",
"mz": "^2.7.0",
"stream-wormhole": "^1.0.3",
"supertest": "^3.0.0",
"urllib": "^2.25.1",
"urllib": "^2.29.1",
"webstorm-disable-index": "^1.2.0"
}
}
21 changes: 21 additions & 0 deletions test/fixtures/apps/upload-one-file/app/controller/async.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,26 @@ module.exports = app => {
fields: stream.fields,
};
}

async allowEmpty() {
const ctx = this.ctx;
const stream = await ctx.getFileStream({ requireFile: false });
if (stream.filename) {
const name = 'egg-multipart-test/' + process.version + '-' + Date.now() + '-' + path.basename(stream.filename);
const result = await ctx.oss.put(name, stream);
ctx.body = {
name: result.name,
url: result.url,
status: result.res.status,
fields: stream.fields,
};
return;
}

stream.resume();
ctx.body = {
fields: stream.fields,
};
}
};
};
2 changes: 2 additions & 0 deletions test/fixtures/apps/upload-one-file/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,6 @@ module.exports = app => {
});

app.post('/upload/async', 'async.async');

app.post('/upload/allowEmpty', 'async.allowEmpty');
};
39 changes: 28 additions & 11 deletions test/multipart.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,29 @@ describe('test/multipart.test.js', () => {
assert(res.data.toString().includes('Can\'t found upload file'));
});

it('should no file upload and only fields', function* () {
const form = formstream();
form.field('hi', 'ok');
form.field('hi2', 'ok2');

const headers = form.headers();
const url = host + '/upload/allowEmpty';
const res = yield urllib.request(url, {
method: 'POST',
headers,
stream: form,
dataType: 'json',
});

assert(res.status === 200);
assert.deepEqual(res.data, {
fields: {
hi: 'ok',
hi2: 'ok2',
},
});
});

it('should 400 when no file speicified', function* () {
const form = formstream();
form.buffer('file', new Buffer(''), '', 'application/octet-stream');
Expand Down Expand Up @@ -480,9 +503,7 @@ describe('test/multipart.test.js', () => {
assert(res.status === 413);
assert(data.message === 'Request file too large');

const coreLogPath = path.join(app.baseDir, 'logs/oss/egg-web.log');
const content = yield fs.readFile(coreLogPath, 'utf8');
assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'coreLogger');
});

it('should ignore error when stream not handle error event', function* () {
Expand All @@ -503,10 +524,8 @@ describe('test/multipart.test.js', () => {
assert(res.status === 200);
assert(data.url);

const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log');
const content = yield fs.readFile(coreLogPath, 'utf8');
assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
assert(content.includes("filename: 'not-handle-error-event.js'"));
app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'errorLogger');
app.expectLog(/filename: ['"]not-handle-error-event.js['"]/, 'errorLogger');
});

it('should ignore stream next errors after limit event fire', function* () {
Expand All @@ -527,10 +546,8 @@ describe('test/multipart.test.js', () => {
assert(res.status === 200);
assert(data.url);

const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log');
const content = yield fs.readFile(coreLogPath, 'utf8');
assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
assert(content.includes("filename: 'not-handle-error-event-and-mock-stream-error.js'"));
app.expectLog('nodejs.MultipartFileTooLargeError: Request file too large', 'errorLogger');
app.expectLog(/filename: ['"]not-handle-error-event-and-mock-stream-error.js['"]/, 'errorLogger');
});
});
});

0 comments on commit 5ece18a

Please sign in to comment.