Skip to content

Commit

Permalink
refactor: use route handler return value even when middleware do not …
Browse files Browse the repository at this point in the history
…return it
  • Loading branch information
thetutlage committed Jan 9, 2023
1 parent b266b97 commit fd4a810
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 58 deletions.
5 changes: 3 additions & 2 deletions src/router/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type { ContainerResolver } from '@adonisjs/fold'
import type { StoreRouteNode } from '../types/route.js'
import type { HttpContext } from '../http_context/main.js'
import { useReturnValue } from './factories/use_return_value.js'

/**
* Executor to execute the route middleware pipeline the route
Expand All @@ -20,10 +21,10 @@ export function execute(route: StoreRouteNode, resolver: ContainerResolver<any>,
.runner()
.finalHandler(async () => {
if (typeof route.handler === 'function') {
return route.handler(ctx)
return Promise.resolve(route.handler(ctx)).then(useReturnValue(ctx))
}

return route.handler.handle(resolver, ctx)
return route.handler.handle(resolver, ctx).then(useReturnValue(ctx))
})
.run(async (middleware, next) => {
if (typeof middleware === 'function') {
Expand Down
File renamed without changes.
2 changes: 0 additions & 2 deletions src/server/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { Router } from '../router/main.js'
import { HttpContext } from '../http_context/main.js'
import { finalHandler } from './factories/final_handler.js'
import { writeResponse } from './factories/write_response.js'
import { useReturnValue } from './factories/use_return_value.js'
import { asyncLocalStorage } from '../http_context/local_storage.js'
import { middlewareHandler } from './factories/middleware_handler.js'

Expand Down Expand Up @@ -143,7 +142,6 @@ export class Server {
.errorHandler((error) => this.#resolvedErrorHandler.handle(error, ctx))
.finalHandler(finalHandler(this.#router!, resolver, ctx))
.run(middlewareHandler(resolver, ctx))
.then(useReturnValue(ctx))
.catch((error) => {
ctx.logger.fatal({ err: error }, 'Exception raised by error handler')
return this.#defaultErrorHandler.handle(error, ctx)
Expand Down
2 changes: 1 addition & 1 deletion tests/router/execute.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ test.group('Route | execute', () => {

routeJSON.handler = {
reference: '#controllers/home',
handle(container, ctx) {
async handle(container, ctx) {
assert.strictEqual(container, resolver)
assert.strictEqual(ctx, context)
stack.push('controller')
Expand Down
118 changes: 65 additions & 53 deletions tests/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,71 @@ test.group('Server | Response handling', () => {
assert.equal(text, 'handled')
})

test('use route handler return value when handler is not async', async ({ assert }) => {
const app = new AppFactory().create()
const server = new ServerFactory().merge({ app }).create()
const httpServer = createServer(server.handle.bind(server))

await app.init()

server.use([])
server.getRouter().get('/', () => 'handled')
await server.boot()

const { text } = await supertest(httpServer).get('/').expect(200)
assert.equal(text, 'handled')
})

test('use route handler return value when middleware does not return it', async ({ assert }) => {
const app = new AppFactory().create()
const server = new ServerFactory().merge({ app }).create()
const httpServer = createServer(server.handle.bind(server))

await app.init()

server.use([])
server
.getRouter()
.get('/', async () => 'handled')
.middleware(async (_, next) => {
await next()
})

await server.boot()

const { text } = await supertest(httpServer).get('/').expect(200)
assert.equal(text, 'handled')
})

test('use route handler return value when server middleware does not return it', async ({
assert,
}) => {
const app = new AppFactory().create()
const server = new ServerFactory().merge({ app }).create()
const httpServer = createServer(server.handle.bind(server))

await app.init()

server.use([
async () => {
return {
default: class GlobalMiddleware {
async handle(_: HttpContext, next: NextFn) {
await next()
}
},
}
},
])

server.getRouter().get('/', async () => 'handled')

await server.boot()

const { text } = await supertest(httpServer).get('/').expect(200)
assert.equal(text, 'handled')
})

test('do not use return value when response.send is called', async ({ assert }) => {
const app = new AppFactory().create()
const server = new ServerFactory().merge({ app }).create()
Expand Down Expand Up @@ -415,59 +480,6 @@ test.group('Server | middleware', () => {
assert.equal(text, 'completed')
})

test('terminate request from server middleware by returning value', async ({ assert }) => {
const stack: string[] = []

const app = new AppFactory().create()
const server = new ServerFactory().merge({ app }).create()
const httpServer = createServer(server.handle.bind(server))
await app.init()

class LogMiddleware {
handle(__: HttpContext, _: NextFn) {
stack.push('fn1')
return 'completed'
}
}

class LogMiddleware2 {
handle(_: HttpContext, next: NextFn) {
stack.push('fn2')
return next()
}
}

server.use([
async () => {
return {
default: LogMiddleware,
}
},
async () => {
return {
default: LogMiddleware2,
}
},
])

server
.getRouter()!
.get('/', async () => {
stack.push('handler')
return 'done'
})
.middleware(async function routeMiddleware(_: HttpContext, next: NextFn) {
stack.push('route fn1')
await next()
})

await server.boot()

const { text } = await supertest(httpServer).get('/').expect(200)
assert.deepEqual(stack, ['fn1'])
assert.equal(text, 'completed')
})

test('terminate request from server by raising exception', async ({ assert }) => {
const stack: string[] = []

Expand Down

0 comments on commit fd4a810

Please sign in to comment.