Skip to content

Commit 901060f

Browse files
thetutlagethetutlage
committed
refactor: make router and instance properties private
1 parent 81c1cb9 commit 901060f

File tree

3 files changed

+92
-37
lines changed

3 files changed

+92
-37
lines changed

‎benchmarks/adonisjs.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const server = new Server(app, encryption, {
3131

3232
server.use([], [], {})
3333

34-
server.router!.get('/', async (ctx) => {
34+
server.getRouter()!.get('/', async (ctx) => {
3535
return ctx.response.send({ hello: 'world' })
3636
})
3737
await server.boot()

‎src/server/main.ts‎

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import { RuntimeException } from '@poppinss/utils'
1313
import type Encryption from '@adonisjs/encryption'
1414
import type { ContainerResolver } from '@adonisjs/fold'
1515
import type { Application } from '@adonisjs/application'
16-
import type { IncomingMessage, ServerResponse } from 'node:http'
16+
import type { Server as HttpsServer } from 'node:https'
17+
import type { ServerResponse, IncomingMessage, Server as HttpServer } from 'node:http'
1718

1819
import type { LazyImport } from '../types/base.js'
1920
import type { RequestConfig } from '../types/request.js'
@@ -74,10 +75,14 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
7475
#serverMiddlewareStack?: Middleware<ParsedGlobalMiddleware>
7576

7677
/**
77-
* The router instance. Router exists after middleware have been
78-
* registered with the server.
78+
* Reference to the router used by the server
7979
*/
80-
router?: Router
80+
#router?: Router
81+
82+
/**
83+
* Reference to the underlying HTTP server in use
84+
*/
85+
#underlyingHttpServer?: HttpServer | HttpsServer
8186

8287
/**
8388
* Know if async local storage is enabled or not.
@@ -125,7 +130,7 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
125130
* Creates an instance of the router
126131
*/
127132
#createRouter(middlewareStore: MiddlewareStore<NamedMiddleware>) {
128-
this.router = new Router(this.#app, this.#encryption, middlewareStore)
133+
this.#router = new Router(this.#app, this.#encryption, middlewareStore)
129134
}
130135

131136
/**
@@ -143,7 +148,7 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
143148
*/
144149
#createContext(req: IncomingMessage, res: ServerResponse) {
145150
const request = new Request(req, res, this.#encryption, this.#config)
146-
const response = new Response(req, res, this.#encryption, this.#config, this.router!)
151+
const response = new Response(req, res, this.#encryption, this.#config, this.#router!)
147152
return new HttpContext(request, response, this.#app.logger.child({}))
148153
}
149154

@@ -180,9 +185,9 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
180185
debug('booting HTTP server')
181186

182187
/**
183-
* Ensure middleware are registered
188+
* Ensure router exists.
184189
*/
185-
if (!this.router) {
190+
if (!this.#router) {
186191
throw new RuntimeException(
187192
'Cannot boot HTTP server. Register middleware using "server.use" first'
188193
)
@@ -191,7 +196,7 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
191196
/**
192197
* Commit routes
193198
*/
194-
this.router.commit()
199+
this.#router.commit()
195200

196201
/**
197202
* Register custom error handler
@@ -211,13 +216,51 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
211216
*/
212217
#handleRequest(ctx: HttpContext, resolver: ContainerResolver) {
213218
return this.#serverMiddlewareStack!.runner()
214-
.finalHandler(finalHandler(this.router!, resolver, ctx))
219+
.finalHandler(finalHandler(this.#router!, resolver, ctx))
215220
.run(middlewareHandler(resolver, ctx))
216221
.then(useReturnValue(ctx))
217222
.catch((error) => this.#resolvedErrorHandler.handle(error, ctx))
218223
.finally(writeResponse(ctx))
219224
}
220225

226+
/**
227+
* Set the HTTP server instance used to listen for requests.
228+
*/
229+
setServer(server: HttpServer | HttpsServer) {
230+
this.#underlyingHttpServer = server
231+
}
232+
233+
/**
234+
* Close the underlying HTTP server
235+
*/
236+
close() {
237+
return new Promise<void>((resolve, reject) => {
238+
this.#underlyingHttpServer?.close((error) => {
239+
if (error) {
240+
return reject(error)
241+
}
242+
243+
resolve()
244+
})
245+
})
246+
}
247+
248+
/**
249+
* Returns reference to the underlying HTTP server
250+
* in use
251+
*/
252+
getServer() {
253+
return this.#underlyingHttpServer
254+
}
255+
256+
/**
257+
* Returns reference to the router instance used
258+
* by the server.
259+
*/
260+
getRouter() {
261+
return this.#router
262+
}
263+
221264
/**
222265
* Handle request
223266
*/

‎tests/server.spec.ts‎

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ test.group('Server | Response handling', () => {
2525
await app.init()
2626

2727
server.use([], [], {})
28-
server.router!.get('/', async ({ response }) => response.send('handled'))
28+
server.getRouter()!.get('/', async ({ response }) => response.send('handled'))
2929
await server.boot()
3030

3131
const { text } = await supertest(httpServer).get('/').expect(200)
@@ -40,7 +40,7 @@ test.group('Server | Response handling', () => {
4040
await app.init()
4141

4242
server.use([], [], {})
43-
server.router!.get('/', async () => 'handled')
43+
server.getRouter()!.get('/', async () => 'handled')
4444
await server.boot()
4545

4646
const { text } = await supertest(httpServer).get('/').expect(200)
@@ -55,7 +55,7 @@ test.group('Server | Response handling', () => {
5555
await app.init()
5656

5757
server.use([], [], {})
58-
server.router!.get('/', async ({ response }) => {
58+
server.getRouter()!.get('/', async ({ response }) => {
5959
response.send('handled')
6060
return 'done'
6161
})
@@ -76,12 +76,13 @@ test.group('Server | Response handling', () => {
7676

7777
server.use([], [], {})
7878
server
79-
.router!.get('/guides/:doc', async ({ params }) => {
79+
.getRouter()!
80+
.get('/guides/:doc', async ({ params }) => {
8081
assert.deepEqual(params, { doc: 'introduction' })
8182
})
8283
.as('guides')
8384

84-
server.router!.on('/docs/:doc').redirect('guides')
85+
server.getRouter()!.on('/docs/:doc').redirect('guides')
8586
await server.boot()
8687

8788
const { redirects } = await supertest(httpServer).get('/docs/introduction').redirects(1)
@@ -103,12 +104,13 @@ test.group('Server | Response handling', () => {
103104

104105
server.use([], [], {})
105106
server
106-
.router!.get('/guides/:doc', async ({ params }) => {
107+
.getRouter()!
108+
.get('/guides/:doc', async ({ params }) => {
107109
assert.deepEqual(params, { doc: 'introduction' })
108110
})
109111
.as('guides')
110112

111-
server.router!.on('/docs/:doc').redirectToPath('/guides/introduction')
113+
server.getRouter()!.on('/docs/:doc').redirectToPath('/guides/introduction')
112114
await server.boot()
113115

114116
const { redirects } = await supertest(httpServer).get('/docs/introduction').redirects(1)
@@ -128,7 +130,8 @@ test.group('Server | Response handling', () => {
128130
server.use([], [], {})
129131

130132
server
131-
.router!.get('/', async ({ response }) => response.send('handled'))
133+
.getRouter()!
134+
.get('/', async ({ response }) => response.send('handled'))
132135
.domain(':tenant.adonisjs.com')
133136

134137
await server.boot()
@@ -151,7 +154,8 @@ test.group('Server | Response handling', () => {
151154
server.use([], [], {})
152155

153156
server
154-
.router!.get('/', async ({ response }) => response.send('handled'))
157+
.getRouter()!
158+
.get('/', async ({ response }) => response.send('handled'))
155159
.domain(':tenant.adonisjs.com')
156160

157161
await server.boot()
@@ -202,7 +206,7 @@ test.group('Server | middleware', () => {
202206
{}
203207
)
204208

205-
server.router!.get('/', async () => {
209+
server.getRouter()!.get('/', async () => {
206210
stack.push('handler')
207211
return 'done'
208212
})
@@ -253,7 +257,8 @@ test.group('Server | middleware', () => {
253257
)
254258

255259
server
256-
.router!.get('/', async () => {
260+
.getRouter()!
261+
.get('/', async () => {
257262
stack.push('handler')
258263
return 'done'
259264
})
@@ -308,7 +313,8 @@ test.group('Server | middleware', () => {
308313
)
309314

310315
server
311-
.router!.get('/', async () => {
316+
.getRouter()!
317+
.get('/', async () => {
312318
stack.push('handler')
313319
return 'done'
314320
})
@@ -364,7 +370,8 @@ test.group('Server | middleware', () => {
364370
)
365371

366372
server
367-
.router!.get('/', async () => {
373+
.getRouter()!
374+
.get('/', async () => {
368375
stack.push('handler')
369376
return 'done'
370377
})
@@ -420,7 +427,8 @@ test.group('Server | middleware', () => {
420427
)
421428

422429
server
423-
.router!.get('/', async () => {
430+
.getRouter()!
431+
.get('/', async () => {
424432
stack.push('handler')
425433
return 'done'
426434
})
@@ -518,7 +526,7 @@ test.group('Server | error handler', () => {
518526
}
519527
})
520528

521-
server.router!.get('/', () => {})
529+
server.getRouter()!.get('/', () => {})
522530
await server.boot()
523531

524532
const { text } = await supertest(httpServer).get('/').expect(200)
@@ -560,7 +568,10 @@ test.group('Server | error handler', () => {
560568
}
561569
})
562570

563-
server.router!.get('/', () => {}).middleware('auth')
571+
server
572+
.getRouter()!
573+
.get('/', () => {})
574+
.middleware('auth')
564575
await server.boot()
565576

566577
const { text } = await supertest(httpServer).get('/').expect(200)
@@ -588,7 +599,7 @@ test.group('Server | error handler', () => {
588599
}
589600
})
590601

591-
server.router!.get('/', () => {
602+
server.getRouter()!.get('/', () => {
592603
throw new Error('Something went wrong')
593604
})
594605
await server.boot()
@@ -604,7 +615,7 @@ test.group('Server | error handler', () => {
604615
await app.init()
605616

606617
server.use([], [], {})
607-
server.router!.get('/', async () => {
618+
server.getRouter()!.get('/', async () => {
608619
return {
609620
toJSON() {
610621
throw new Error('blowup')
@@ -642,7 +653,7 @@ test.group('Server | force content negotiation', () => {
642653
server.use([], [], {})
643654
await server.boot()
644655

645-
server.router!.get('/', async ({ request, response }) => {
656+
server.getRouter()!.get('/', async ({ request, response }) => {
646657
response.send(request.header('accept'))
647658
})
648659
await server.boot()
@@ -660,7 +671,8 @@ test.group('Server | force content negotiation', () => {
660671
server.use([], [], {})
661672

662673
server
663-
.router!.get('/users/:id', async ({ request }) => {
674+
.getRouter()!
675+
.get('/users/:id', async ({ request }) => {
664676
return {
665677
hasValidSignature: request.hasValidSignature(),
666678
}
@@ -672,7 +684,7 @@ test.group('Server | force content negotiation', () => {
672684
/**
673685
* Make a signed url
674686
*/
675-
const url = server.router!.makeSignedUrl('showUser', [1], {
687+
const url = server.getRouter()!.makeSignedUrl('showUser', [1], {
676688
qs: { site: 1, db: 'pg', dbUser: 1 },
677689
})
678690

@@ -696,7 +708,7 @@ test.group('Server | force content negotiation', () => {
696708

697709
server.use([], [], {})
698710

699-
server.router!.get('/', async (ctx) => {
711+
server.getRouter()!.get('/', async (ctx) => {
700712
return {
701713
enabled: HttpContext.usingAsyncLocalStorage,
702714
get: HttpContext.get() === ctx,
@@ -737,7 +749,7 @@ test.group('Server | force content negotiation', () => {
737749

738750
server.use([], [], {})
739751

740-
server.router!.get('/', async (ctx) => {
752+
server.getRouter()!.get('/', async (ctx) => {
741753
return HttpContext.runOutsideContext(() => {
742754
return {
743755
enabled: HttpContext.usingAsyncLocalStorage,
@@ -771,14 +783,14 @@ test.group('Server | force content negotiation', () => {
771783

772784
server.use([], [], {})
773785

774-
server.router!.get('/', async () => {
786+
server.getRouter()!.get('/', async () => {
775787
return {
776788
enabled: HttpContext.usingAsyncLocalStorage,
777789
get: HttpContext.get() === null,
778790
}
779791
})
780792

781-
server.router!.get('/fail', async () => {
793+
server.getRouter()!.get('/fail', async () => {
782794
return HttpContext.getOrFail()
783795
})
784796

@@ -820,7 +832,7 @@ test.group('Server | force content negotiation', () => {
820832

821833
server.use([], [], {})
822834

823-
server.router!.get('/', async (ctx) => {
835+
server.getRouter()!.get('/', async (ctx) => {
824836
return HttpContext.runOutsideContext(() => {
825837
return {
826838
enabled: HttpContext.usingAsyncLocalStorage,

0 commit comments

Comments
 (0)