Skip to content

Commit

Permalink
Router: Better and less buggy combineMiddleware()
Browse files Browse the repository at this point in the history 
Signed-off-by: Fadion Dashi <jonidashi@gmail.com>
  • Loading branch information
@fadion
fadion committed Mar 22, 2018
1 parent 621ba4d commit f50499c
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 37 deletions.
27 changes: 18 additions & 9 deletions packages/router/lib/route.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { is } = require('@sapphirejs/commons')
const { is } = require('@sapphirejs/common')
const settings = require('./settings')
const errors = require('./errors/messages')
const InvalidRouteArguments = require('./errors/invalid-route-arguments')
Expand Down Expand Up @@ -200,7 +200,7 @@ class Route {
this._routes.push({
type: settings.type.http,
path: path,
middleware: middleware,
middleware: this._combineMiddleware(middleware),
handler: callback,
meta: {
method: method
Expand Down Expand Up @@ -289,13 +289,22 @@ class Route {
* @returns {null|array}
*/
_combineMiddleware(first, second) {
if (!first && !second) return null
if (Array.isArray(first) && Array.isArray(second)) return [...first, ...second]
if (Array.isArray(first)) return [...first, second]
if (Array.isArray(second)) return [first, ...second]
if (!first) return [second]
if (!second) return [first]
return [first, second]
return this._flattenArray([first, second])
.filter(item => item)
}

/**
* Flattens a multidimensional array to
* a single dimesion.
*
* @private
* @param {array} array
* @returns {array}
*/
_flattenArray(array) {
return array.reduce((acc, item) => {
return acc.concat(Array.isArray(item) ? this._flattenArray(item) : item)
}, [])
}

/**
Expand Down
72 changes: 44 additions & 28 deletions packages/router/test/route.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { UserController, CommentController, TaskController } = require('./helpers
test('every http method', () => {
const route = new Route()
const cb = () => {}

route.get('/path', cb)
route.post('/path', cb)
route.put('/path', cb)
Expand All @@ -19,7 +19,7 @@ test('every http method', () => {
expect(routes[0]).toMatchObject({
type: settings.type.http,
path: '/path',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.get
Expand All @@ -29,7 +29,7 @@ test('every http method', () => {
expect(routes[1]).toMatchObject({
type: settings.type.http,
path: '/path',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.post
Expand All @@ -39,7 +39,7 @@ test('every http method', () => {
expect(routes[2]).toMatchObject({
type: settings.type.http,
path: '/path',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.put
Expand All @@ -49,7 +49,7 @@ test('every http method', () => {
expect(routes[3]).toMatchObject({
type: settings.type.http,
path: '/path',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.patch
Expand All @@ -59,7 +59,7 @@ test('every http method', () => {
expect(routes[4]).toMatchObject({
type: settings.type.http,
path: '/path',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.delete
Expand All @@ -77,7 +77,7 @@ test('http route with only a callback', () => {

expect(routes.length).toBe(1)
expect(routes[0].handler).toEqual(cb)
expect(routes[0].middleware).toBeNull()
expect(routes[0].middleware).toEqual([])
})

test('http route with middleware and callback', () => {
Expand All @@ -88,8 +88,8 @@ test('http route with middleware and callback', () => {
route.get('/path', mw, cb)

const routes = route.export()
expect(routes[0].middleware).toEqual(mw)

expect(routes[0].middleware).toEqual([mw])
expect(routes[0].handler).toEqual(cb)
})

Expand Down Expand Up @@ -152,7 +152,7 @@ test('http group', () => {
expect(routes[0]).toMatchObject({
type: settings.type.http,
path: '/path/hi',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.get
Expand All @@ -162,15 +162,15 @@ test('http group', () => {
expect(routes[1]).toMatchObject({
type: settings.type.http,
path: '/path/hey',
middleware: null,
middleware: [],
handler: cb,
meta: {
method: settings.method.post
}
})
})

test('http group combines middleware', () => {
test('http group combines both middleware', () => {
const route = new Route()
const cb = () => {}
const mw = () => {}
Expand All @@ -187,7 +187,7 @@ test('http group combines middleware', () => {
expect(routes[1].middleware).toEqual([mwGroup, mw, mw])
})

test('http group combines array middleware', () => {
test('http group combines both array middleware', () => {
const route = new Route()
const cb = () => {}
const mw = () => {}
Expand All @@ -204,6 +204,22 @@ test('http group combines array middleware', () => {
expect(routes[1].middleware).toEqual([mwGroup, mwGroup, mw, mw])
})

test('http group combines only routes middleware', () => {
const route = new Route()
const cb = () => {}
const mw = () => {}

route.group('/path', (route) => {
route.get('/hi', mw, cb)
route.get('/hi', [mw, mw], cb)
})

const routes = route.export()

expect(routes[0].middleware).toEqual([mw])
expect(routes[1].middleware).toEqual([mw, mw])
})

test('empty http group does not build routes', () => {
const route = new Route()

Expand Down Expand Up @@ -256,7 +272,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users',
middleware: null,
middleware: [],
handler: controller.index,
meta: {
method: settings.method.get
Expand All @@ -265,7 +281,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users/:id/edit',
middleware: null,
middleware: [],
handler: controller.edit,
meta: {
method: settings.method.get
Expand All @@ -274,7 +290,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users/new',
middleware: null,
middleware: [],
handler: controller.store,
meta: {
method: settings.method.get
Expand All @@ -283,7 +299,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users/:id',
middleware: null,
middleware: [],
handler: controller.show,
meta: {
method: settings.method.get
Expand All @@ -292,7 +308,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users',
middleware: null,
middleware: [],
handler: controller.create,
meta: {
method: settings.method.post
Expand All @@ -301,7 +317,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users/:id',
middleware: null,
middleware: [],
handler: controller.update,
meta: {
method: settings.method.put
Expand All @@ -310,7 +326,7 @@ test('http resource', () => {
{
type: settings.type.http,
path: '/users/:id',
middleware: null,
middleware: [],
handler: controller.destroy,
meta: {
method: settings.method.delete
Expand All @@ -328,7 +344,7 @@ test('http resource with middleware', () => {

const routes = route.export()

expect(routes[0].middleware).toEqual(mw)
expect(routes[0].middleware).toEqual([mw])
})

test('http resource with array of middleware', () => {
Expand All @@ -353,7 +369,7 @@ test('http resource with "only" option', () => {
{
type: settings.type.http,
path: '/users',
middleware: null,
middleware: [],
handler: controller.index,
meta: {
method: settings.method.get
Expand All @@ -362,7 +378,7 @@ test('http resource with "only" option', () => {
{
type: settings.type.http,
path: '/users',
middleware: null,
middleware: [],
handler: controller.create,
meta: {
method: settings.method.post
Expand All @@ -381,7 +397,7 @@ test('http resource with "except" option', () => {
{
type: settings.type.http,
path: '/users/:id',
middleware: null,
middleware: [],
handler: controller.update,
meta: {
method: settings.method.put
Expand All @@ -390,7 +406,7 @@ test('http resource with "except" option', () => {
{
type: settings.type.http,
path: '/users/:id',
middleware: null,
middleware: [],
handler: controller.destroy,
meta: {
method: settings.method.delete
Expand Down Expand Up @@ -420,7 +436,7 @@ test('nested http resource', () => {
{
type: settings.type.http,
path: '/users',
middleware: null,
middleware: [],
handler: userController.index,
meta: {
method: settings.method.get
Expand All @@ -429,7 +445,7 @@ test('nested http resource', () => {
{
type: settings.type.http,
path: '/users/comments/:id',
middleware: null,
middleware: [],
handler: commentController.update,
meta: {
method: settings.method.put
Expand Down Expand Up @@ -469,4 +485,4 @@ test('throws when http resource middleware is not an array of functions', () =>
expect(() => {
route.resource('/users', ['hi', 10], new UserController())
}).toThrow(InvalidRouteArguments)
})
})

0 comments on commit f50499c

Please sign in to comment.