Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend send method #90

Merged
merged 13 commits into from
May 27, 2020
102 changes: 74 additions & 28 deletions libs/response-extensions.js
Original file line number Diff line number Diff line change
@@ -1,41 +1,87 @@
'use strict'

const { forEachObject } = require('./utils')

const CONTENT_TYPE_HEADER = 'content-type'
const TYPE_JSON = 'application/json; charset=utf-8'
const TYPE_PLAIN = 'text/plain; charset=utf-8'
const TYPE_OCTET = 'application/octet-stream'

const NOOP = () => {}

const stringify = (obj) => {
// ToDo: fast json stringify ?
return JSON.stringify(obj)
}

const preEnd = (res, contentType, statusCode) => {
res.setHeader(CONTENT_TYPE_HEADER, contentType || TYPE_PLAIN)
res.statusCode = statusCode
}

const parseErr = (error) => {
const errorCode = error.status || error.code || error.statusCode
const statusCode = typeof errorCode === 'number' ? parseInt(errorCode) : 500

return [
statusCode,
stringify({
code: statusCode,
message: error.message,
data: error.data
})
]
}

/**
* The friendly 'res.send' method
* No comments needed ;)
*/
module.exports.send = (options, req, res) => (data = 200, code = 200, headers = null, cb = () => {}) => {
if (headers !== null) {
Object.keys(headers).forEach((key) => {
res.setHeader(key.toLowerCase(), headers[key])
})
}
module.exports.send = (options, req, res) => {
return (data = 200, code = 200, headers = null, cb = NOOP) => {
let contentType

if (typeof data === 'number') {
code = parseInt(data, 10)
data = res.body
} else if (data instanceof Error) {
const errorCode = data.status || data.code || data.statusCode
code = typeof errorCode === 'number' ? parseInt(errorCode) : 500
data = {
code,
message: data.message,
data: data.data
}
res.setHeader(CONTENT_TYPE_HEADER, 'application/json')
}
if (data instanceof Error) {
const [statusCode, errStr] = parseErr(data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendError does the whole process, it does not return an array so that a destructuring is performed later, this is not good if we want to improve performance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my updates, intermediate functions does not end the request, just update the data object. (with exception of the stream support).
I will update this code to actually remove the usage of destructing.

contentType = TYPE_JSON
code = statusCode
data = errStr
} else {
if (headers && typeof headers === 'object') {
forEachObject(headers, (value, key) => {
res.setHeader(key.toLowerCase(), value)
})
}

if (typeof data === 'object' && data instanceof Buffer === false) {
if (!res.hasHeader(CONTENT_TYPE_HEADER)) {
res.setHeader(CONTENT_TYPE_HEADER, 'application/json')
}
data = JSON.stringify(data)
}
// NOTE: only retrieve content-type after setting custom headers
contentType = res.getHeader(CONTENT_TYPE_HEADER)

if (typeof data === 'number') {
code = parseInt(data, 10)
data = res.body
}

res.statusCode = code
if (data) {
if (data instanceof Buffer) {
contentType = contentType || TYPE_OCTET
} else if (!!data && typeof data.pipe === 'function') {
contentType = contentType || TYPE_OCTET

// finally end request
res.end(data, cb)
// NOTE: we exceptionally handle the response termination for streams
preEnd(res, contentType, code)

data.pipe(res)
data.on('end', cb)

return
} else if (typeof data === 'object') {
contentType = contentType || TYPE_JSON
data = stringify(data)
}
}
}

preEnd(res, contentType, code)
res.end(data, cb)
}
}
8 changes: 8 additions & 0 deletions libs/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports.forEachObject = (obj, cb) => {
const keys = Object.keys(obj)
const length = keys.length

for (let i = 0; i < length; i++) {
cb(obj[keys[i]], keys[i])
}
}
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
"devDependencies": {
"@hapi/hapi": "^19.1.1",
"chai": "^4.2.0",
"connect-query": "^1.0.0",
"express": "^4.17.1",
"express-jwt": "^5.3.3",
"fastify": "^2.14.1",
Expand Down
30 changes: 0 additions & 30 deletions specs/buffer.test.js

This file was deleted.

96 changes: 96 additions & 0 deletions specs/send.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
'use strict'

/* global describe, it */
const request = require('supertest')
const { createReadStream, readFileSync } = require('fs')
const path = require('path')

describe('All Responses', () => {
let server
const service = require('../index')()

service.get('/string', (req, res) => {
res.send('Hello World!')
})

service.get('/buffer', (req, res) => {
res.send(Buffer.from('Hello World!'))
})

service.get('/buffer-string', (req, res) => {
res.setHeader('content-type', 'text/plan; charset=utf-8')
res.send(Buffer.from('Hello World!'))
})

service.get('/json', (req, res) => {
res.send({ id: 'restana' })
})

service.get('/stream', (req, res) => {
res.setHeader('content-type', 'text/html; charset=utf-8')
res.send(createReadStream(path.resolve(__dirname, '../demos/static/src/index.html'), { encoding: 'utf8' }))
})

service.get('/error', (req, res) => {
const err = new Error('Test')
err.code = 501
res.send(err)
})

it('should start service', async () => {
server = await service.start(~~process.env.PORT)
})

it('should GET 200 and string content on /string', async () => {
await request(server)
.get('/string')
.expect(200)
.expect('Hello World!')
})

it('should GET 200 and buffer content on /buffer', async () => {
await request(server)
.get('/buffer')
.expect(200)
.expect('content-type', 'application/octet-stream')
.expect(Buffer.from('Hello World!'))
})

it('should GET 200 and string content on /buffer-string', async () => {
await request(server)
.get('/buffer-string')
.expect(200)
.expect('Hello World!')
})

it('should GET 200 and json content on /json', async () => {
await request(server)
.get('/json')
.expect(200)
.expect('content-type', 'application/json; charset=utf-8')
.expect({ id: 'restana' })
})

it('should GET 200 and buffer content on /stream', async () => {
await request(server)
.get('/stream')
.expect(200)
.expect('content-type', 'text/html; charset=utf-8')
.expect(readFileSync(path.resolve(__dirname, '../demos/static/src/index.html'), 'utf8'))
})

it('should GET 501 and json content on /error', async () => {
await request(server)
.get('/error')
.expect(501)
.expect('content-type', 'application/json; charset=utf-8')
.expect({
code: 501,
message: 'Test'
})
})

it('should successfully terminate the service', async () => {
await service.close()
})
})