Skip to content

Commit

Permalink
Merge pull request #51 from kbarbounakis/cleanup-after-close-stream
Browse files Browse the repository at this point in the history
close buffered stream
  • Loading branch information
kbarbounakis authored Oct 4, 2021
2 parents 804e2ee + aa66051 commit 142702a
Show file tree
Hide file tree
Showing 10 changed files with 213 additions and 14 deletions.
69 changes: 69 additions & 0 deletions modules/@themost/express/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# IDE
.idea
.DS_Store
.vscode
# Logs
logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*

# Runtime data
pids
*.pid
*.seed
*.pid.lock

# Directory for instrumented libs generated by jscoverage/JSCover
lib-cov

# Coverage directory used by tools like istanbul
coverage

# nyc test coverage
.nyc_output

# Grunt intermediate storage (http://gruntjs.com/creating-plugins#storing-task-files)
.grunt

# Bower dependency directory (https://bower.io/)
bower_components

# node-waf configuration
.lock-wscript

# Compiled binary addons (https://nodejs.org/api/addons.html)
build/Release

# Dependency directories
node_modules/
test/**/node_modules
jspm_packages/

# TypeScript v1 declaration files
typings/

# Optional npm cache directory
.npm

# Optional eslint cache
.eslintcache

# Optional REPL history
.node_repl_history

# Output of 'npm pack'
*.tgz

# Yarn Integrity file
.yarn-integrity

# dotenv environment variables file
.env

# next.js build output
.next

# application
dist/
2 changes: 2 additions & 0 deletions modules/@themost/express/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ test
.vscode
.eslintignore
rollup.config.js
babel.config.js
jsconfig.json

43 changes: 43 additions & 0 deletions modules/@themost/express/babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
module.exports = function (api) {
api.cache(false);
return {
"sourceMaps": true,
"retainLines": true,
"presets": [
[
"@babel/preset-env",
{
"targets": {
"node": "6"
}
}
]
],
"ignore": [
/\/node_modules/
],
"plugins": [
[
"@babel/plugin-transform-async-to-generator"
],
[
"@babel/plugin-proposal-export-default-from"
],
[
"@babel/plugin-proposal-export-namespace-from"
],
[
"@babel/plugin-proposal-decorators",
{
"legacy": true
}
],
[
"@babel/plugin-proposal-class-properties",
{
"loose": true
}
]
]
};
};
5 changes: 5 additions & 0 deletions modules/@themost/express/jsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"experimentalDecorators": true
}
}
2 changes: 1 addition & 1 deletion modules/@themost/express/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion modules/@themost/express/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@themost/express",
"version": "1.5.16",
"version": "1.5.18",
"description": "MOST Data ORM Express Middleware",
"main": "dist/themost_express.cjs.js",
"module": "dist/themost_express.esm.js",
Expand Down
40 changes: 31 additions & 9 deletions modules/@themost/express/src/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import _ from "lodash";

import Q from "q";
import {ODataModelBuilder, EdmMapping, DataQueryable, EdmType} from "@themost/data";
import {LangUtils, HttpNotFoundError, HttpBadRequestError, HttpMethodNotAllowedError} from '@themost/common';
import {LangUtils, HttpNotFoundError, HttpBadRequestError, HttpMethodNotAllowedError, TraceUtils} from '@themost/common';
import { ResponseFormatter, StreamFormatter } from "./formatter";
import {multerInstance} from "./multer";
import fs from 'fs';
Expand Down Expand Up @@ -1163,8 +1163,8 @@ function postEntitySetAction(options) {
}
// add context as the first parameter
actionParameters.push(req.context);

let tryGetStream = tryGetActionStream(parameters);
const multerOptions = req.context.getApplication().getConfiguration().getSourceAt('settings/multer');
let tryGetStream = tryGetActionStream(parameters, multerOptions);
return tryGetStream(req, res, (err) => {
if (err) {
return next(err);
Expand All @@ -1187,6 +1187,17 @@ function postEntitySetAction(options) {
bufferedStream.contentEncoding = file.encoding;
bufferedStream.contentType = file.mimetype;
bufferedStream.contentFileName = file.originalname;
bufferedStream.on('close', () => {
TraceUtils.debug(`(postEntitySetAction), Closing read stream, ${file.path}`);
try {
if (fs.existsSync(file.path)) {
fs.unlinkSync(file.path);
}
} catch (error) {
TraceUtils.warn(`(postEntitySetAction) An error occurred while trying to cleanup user uploaded content ${file.path}`);
TraceUtils.warn(error);
}
});
}
}
if (bufferedStream == null && x.nullable === false) {
Expand Down Expand Up @@ -1415,9 +1426,10 @@ function getEntityFunction(options) {

/**
* @param actionParameters
* @param {*} options
* @returns {function(*, *, *): *}
*/
function tryGetActionStream(actionParameters) {
function tryGetActionStream(actionParameters, options) {
let result = function(req, res, next) {
return next();
};
Expand All @@ -1427,7 +1439,7 @@ function tryGetActionStream(actionParameters) {
});
if (files.length>0) {
// use multer()
result = multerInstance().fields(files.map((x) => {
result = multerInstance(options).fields(files.map((x) => {
return {
name: x.name
}
Expand Down Expand Up @@ -1512,7 +1524,8 @@ function postEntityAction(options) {
const parameters = _.filter(action.parameters, x => {
return x.name !== 'bindingParameter';
});
let tryGetStream = tryGetActionStream(parameters);
const multerOptions = req.context.getApplication().getConfiguration().getSourceAt('settings/multer');
let tryGetStream = tryGetActionStream(parameters, multerOptions);
return tryGetStream(req, res, (err) => {
if (err) {
return next(err);
Expand All @@ -1536,6 +1549,17 @@ function postEntityAction(options) {
bufferedStream.contentEncoding = file.encoding;
bufferedStream.contentType = file.mimetype;
bufferedStream.contentFileName = file.originalname;
bufferedStream.on('close', () => {
TraceUtils.debug(`(postEntityAction), Closing read stream, ${file.path}`);
try {
if (fs.existsSync(file.path)) {
fs.unlinkSync(file.path);
}
} catch (error) {
TraceUtils.warn(`(postEntityAction) An error occurred while trying to cleanup user uploaded content ${file.path}`);
TraceUtils.warn(error);
}
});
}
}
if (bufferedStream == null && x.nullable === false) {
Expand All @@ -1548,7 +1572,6 @@ function postEntityAction(options) {
} else {
actionParameters.push(req.body[x.name]);
}

}
});
}
Expand Down Expand Up @@ -1595,8 +1618,7 @@ function postEntityAction(options) {
}).catch(function(err) {
return next(err);
});
})

});
}
// entity type does not have an instance method with the given name, continue
return next();
Expand Down
7 changes: 5 additions & 2 deletions modules/@themost/express/src/multer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ import multer from "multer";
import path from 'path';
import os from 'os';
/**
* @params {multer.Options=} options
* Returns an instance of multer that is configured based on application configuration
* @returns {multer.Instance}
*/
function multerInstance() {
function multerInstance(options) {
// set user storage from configuration or use default content folder content/user
const userContentRoot = path.resolve(os.tmpdir(), 'userContent');
return multer({ dest: userContentRoot });
return multer(Object.assign({
dest: userContentRoot
}, options));
}

export {multerInstance}
50 changes: 50 additions & 0 deletions modules/@themost/express/src/service.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,56 @@ describe('serviceRouter', () => {

});

it('should post file that exceeds file limit', async () => {
const app1 = express();
// create a new instance of data application
const application = new ExpressDataApplication(path.resolve(__dirname, 'test/config'));

app1.use(express.json({
reviver: dateReviver
}));
// hold data application
app1.set('ExpressDataApplication', application);
// use data middleware (register req.context)
app1.use(application.middleware(app1));
// use test passport strategy
passport.use(passportStrategy);
// set service router
app1.use('/api/', passport.authenticate('bearer', { session: false }), serviceRouter);
// change user
spyOn(passportStrategy, 'getUser').and.returnValue({
name: 'alexis.rees@example.com'
});

application.getConfiguration().setSourceAt('settings/multer', {
limits: {
fileSize: 100
}
});

let response = await request(app1)
.post('/api/users/me/uploadAvatar')
.field('alternateName', 'testing')
.field('published', true)
.attach('file', path.resolve(__dirname, 'test/models/avatars/avatar1.png'))
expect(response.status).toBe(500);
expect(response.text.includes('File too large')).toBeTruthy();

application.getConfiguration().setSourceAt('settings/multer', {
limits: {
fileSize: 2000000
}
});

response = await request(app1)
.post('/api/users/me/uploadAvatar')
.field('alternateName', 'testing')
.field('published', true)
.attach('file', path.resolve(__dirname, 'test/models/avatars/avatar1.png'))
expect(response.status).toBe(200);

});

it('should post file for entity set action', async () => {
const app1 = express();
// create a new instance of data application
Expand Down
7 changes: 6 additions & 1 deletion modules/@themost/express/src/test/models/UserModel.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const {DataObject, EdmMapping, EdmType} = require('@themost/data');
import fs from 'fs';
import path from 'path';
import os from 'os';

function readStream(stream) {
// eslint-disable-next-line no-undef
Expand Down Expand Up @@ -56,8 +57,8 @@ class User extends DataObject {
@EdmMapping.param('file', EdmType.EdmStream, false)
@EdmMapping.action('uploadAvatar', 'Object')
async uploadAvatar(file, attributes) {
// eslint-disable-next-line no-unused-vars
const blob = await readStream(file);
console.log('Content-Type', file.contentType);
return Object.assign({}, attributes, {
dateCreated: new Date()
});
Expand All @@ -73,6 +74,7 @@ class User extends DataObject {
@EdmMapping.param('file', EdmType.EdmStream, false)
@EdmMapping.action('uploadTestAvatar', 'Object')
static async uploadTestAvatar(context, file, attributes) {
// eslint-disable-next-line no-unused-vars
const blob = await readStream(file);
return Object.assign({}, attributes, {
dateCreated: new Date()
Expand All @@ -95,6 +97,7 @@ class User extends DataObject {
}

@EdmMapping.func('staticEmptyContent', 'Object')
// eslint-disable-next-line no-unused-vars
static getStaticEmptyContent(context) {
return null;
}
Expand All @@ -111,12 +114,14 @@ class User extends DataObject {

@EdmMapping.param('message', EdmType.EdmString, false)
@EdmMapping.action('emptyContent', 'Object')
// eslint-disable-next-line no-unused-vars
postEmptyContent(message) {
return null;
}

@EdmMapping.param('message', EdmType.EdmString, false)
@EdmMapping.action('staticEmptyContent', 'Object')
// eslint-disable-next-line no-unused-vars
static staticPostEmptyContent(context, message) {
return null;
}
Expand Down

0 comments on commit 142702a

Please sign in to comment.