refactor: move config normalization to defineConfig method

Author: thetutlage

src/define_config.ts

@@ -8,16 +8,14 @@
  */
 
 import proxyAddr from 'proxy-addr'
-import type { RequestConfig } from './types/request.js'
-import type { ResponseConfig } from './types/response.js'
+import string from '@poppinss/utils/string'
+import type { ServerConfig } from './types/server.js'
 
 /**
  * Define configuration for the HTTP server
  */
-export function defineConfig(
-  config: Partial<RequestConfig & ResponseConfig>
-): RequestConfig & ResponseConfig {
-  return {
+export function defineConfig(config: Partial<ServerConfig>): ServerConfig {
+  const normalizedConfig = {
     allowMethodSpoofing: false,
     trustProxy: proxyAddr.compile('loopback'),
     subdomainOffset: 2,
@@ -34,4 +32,10 @@ export function defineConfig(
     },
     ...config,
   }
+
+  if (normalizedConfig.cookie.maxAge) {
+    normalizedConfig.cookie.maxAge = string.seconds.parse(normalizedConfig.cookie.maxAge)
+  }
+
+  return normalizedConfig
 }

src/request.ts

@@ -37,6 +37,17 @@ import type { HttpContext } from './http_context/main.js'
  * using `request.request` property.
  */
 export class Request extends Macroable {
+  /**
+   * Encryption module to verify signed URLs and unsign/decrypt
+   * cookies
+   */
+  #encryption: Encryption
+
+  /**
+   * Request config
+   */
+  #config: RequestConfig
+
   /**
    * Request body set using `setBody` method
    */
@@ -90,11 +101,13 @@ export class Request extends Macroable {
   constructor(
     public request: IncomingMessage,
     public response: ServerResponse,
-    private encryption: Encryption,
-    private config: RequestConfig
+    encryption: Encryption,
+    config: RequestConfig
   ) {
     super()
 
+    this.#config = config
+    this.#encryption = encryption
     this.parsedUrl = parse(this.request.url!, false)
     this.#parseQueryString()
   }
@@ -114,7 +127,7 @@ export class Request extends Macroable {
    */
   #initiateCookieParser() {
     if (!this.#cookieParser) {
-      this.#cookieParser = new CookieParser(this.header('cookie')!, this.encryption)
+      this.#cookieParser = new CookieParser(this.header('cookie')!, this.#encryption)
     }
   }
 
@@ -133,7 +146,7 @@ export class Request extends Macroable {
    */
   id(): string | undefined {
     let requestId = this.header('x-request-id')
-    if (!requestId && this.config.generateRequestId) {
+    if (!requestId && this.#config.generateRequestId) {
       requestId = cuid()
       this.request.headers['x-request-id'] = requestId
     }
@@ -320,7 +333,7 @@ export class Request extends Macroable {
    * ```
    */
   method(): string {
-    if (this.config.allowMethodSpoofing && this.intended() === 'POST') {
+    if (this.#config.allowMethodSpoofing && this.intended() === 'POST') {
       return this.input('_method', this.intended()).toUpperCase()
     }
     return this.intended()
@@ -382,12 +395,12 @@ export class Request extends Macroable {
    * The value of trustProxy is passed directly to [proxy-addr](https://www.npmjs.com/package/proxy-addr)
    */
   ip(): string {
-    const ipFn = this.config.getIp
+    const ipFn = this.#config.getIp
     if (typeof ipFn === 'function') {
       return ipFn(this)
     }
 
-    return proxyaddr(this.request, this.config.trustProxy)
+    return proxyaddr(this.request, this.#config.trustProxy)
   }
 
   /**
@@ -409,7 +422,7 @@ export class Request extends Macroable {
    * The value of trustProxy is passed directly to [proxy-addr](https://www.npmjs.com/package/proxy-addr)
    */
   ips(): string[] {
-    return proxyaddr.all(this.request, this.config.trustProxy)
+    return proxyaddr.all(this.request, this.#config.trustProxy)
   }
 
   /**
@@ -437,7 +450,7 @@ export class Request extends Macroable {
       return 'https'
     }
 
-    if (!trustProxy(this.request.socket.remoteAddress!, this.config.trustProxy)) {
+    if (!trustProxy(this.request.socket.remoteAddress!, this.#config.trustProxy)) {
       return this.parsedUrl.protocol || 'http'
     }
 
@@ -478,7 +491,7 @@ export class Request extends Macroable {
      * Use X-Fowarded-Host when we trust the proxy header and it
      * exists
      */
-    if (trustProxy(this.request.socket.remoteAddress!, this.config.trustProxy)) {
+    if (trustProxy(this.request.socket.remoteAddress!, this.#config.trustProxy)) {
       host = this.header('X-Forwarded-Host') || host
     }
 
@@ -540,7 +553,7 @@ export class Request extends Macroable {
       return []
     }
 
-    const offset = this.config.subdomainOffset
+    const offset = this.#config.subdomainOffset
     const subdomains = hostname.split('.').reverse().slice(offset)
 
     /*
@@ -907,7 +920,7 @@ export class Request extends Macroable {
     /*
      * Return false when signature fails
      */
-    const signedUrl = this.encryption.verifier.unsign(signature, purpose)
+    const signedUrl = this.#encryption.verifier.unsign(signature, purpose)
     if (!signedUrl) {
       return false
     }

src/response.ts

@@ -64,7 +64,7 @@ export class Response extends Macroable {
   #router: Router
 
   /**
-   * Response defaults
+   * Response config
    */
   #config: ResponseConfig
 

src/server/main.ts

@@ -7,7 +7,6 @@
  * file that was distributed with this source code.
  */
 
-import string from '@poppinss/utils/string'
 import Middleware from '@poppinss/middleware'
 import { RuntimeException } from '@poppinss/utils'
 import type Encryption from '@adonisjs/encryption'
@@ -17,10 +16,8 @@ import type { Server as HttpsServer } from 'node:https'
 import type { ServerResponse, IncomingMessage, Server as HttpServer } from 'node:http'
 
 import type { LazyImport } from '../types/base.js'
-import type { RequestConfig } from '../types/request.js'
-import type { ResponseConfig } from '../types/response.js'
-import type { ErrorHandlerAsAClass, ServerErrorHandler } from '../types/server.js'
 import type { MiddlewareAsClass, ParsedGlobalMiddleware } from '../types/middleware.js'
+import type { ErrorHandlerAsAClass, ServerConfig, ServerErrorHandler } from '../types/server.js'
 
 import debug from '../debug.js'
 import { Request } from '../request.js'
@@ -67,7 +64,7 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
   /**
    * Server config
    */
-  #config: RequestConfig & ResponseConfig
+  #config: ServerConfig
 
   /**
    * Server middleware stack runs on every incoming HTTP request
@@ -91,29 +88,15 @@ export class Server<NamedMiddleware extends Record<string, LazyImport<Middleware
     return asyncLocalStorage.isEnabled
   }
 
-  constructor(app: Application, encryption: Encryption, config: RequestConfig & ResponseConfig) {
+  constructor(app: Application, encryption: Encryption, config: ServerConfig) {
     this.#app = app
     this.#encryption = encryption
     this.#config = config
-    this.#preProcessCookieMaxAge()
     this.#createAsyncLocalStore()
 
     debug('server config: %O', this.#config)
   }
 
-  /**
-   * Pre-processing cookie maxAge property to avoid re-parsing on
-   * every request
-   */
-  #preProcessCookieMaxAge() {
-    /*
-     * Pre process config to convert max age string to seconds.
-     */
-    if (this.#config.cookie.maxAge && typeof this.#config.cookie.maxAge === 'string') {
-      this.#config.cookie.maxAge = string.seconds.parse(this.#config.cookie.maxAge)
-    }
-  }
-
   /**
    * Create async local storage store when enabled
    */

src/types/request.ts

@@ -47,12 +47,4 @@ export type RequestConfig = {
    * the `proxy-addr` package to compute this value.
    */
   trustProxy: (address: string, distance: number) => boolean
-
-  /**
-   * Whether or not to create an async local storage store for
-   * the HTTP context.
-   *
-   * Defaults to false
-   */
-  useAsyncLocalStorage?: boolean
 }

src/types/server.ts

@@ -8,6 +8,8 @@
  */
 
 import type { Constructor } from './base.js'
+import type { RequestConfig } from './request.js'
+import type { ResponseConfig } from './response.js'
 import type { HttpContext } from '../http_context/main.js'
 
 /**
@@ -21,3 +23,17 @@ export type ServerErrorHandler = {
  * Error handler represented as a class
  */
 export type ErrorHandlerAsAClass = Constructor<ServerErrorHandler>
+
+/**
+ * Config accepted by the HTTP server
+ */
+export type ServerConfig = RequestConfig &
+  ResponseConfig & {
+    /**
+     * Whether or not to create an async local storage store for
+     * the HTTP context.
+     *
+     * Defaults to false
+     */
+    useAsyncLocalStorage?: boolean
+  }

tests/define_config.spec.ts

@@ -0,0 +1,49 @@
+/*
+ * @adonisjs/http-server
+ *
+ * (c) AdonisJS
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+import { test } from '@japa/runner'
+import { defineConfig } from '../index.js'
+
+test.group('Define config', () => {
+  test('define server config', ({ assert }) => {
+    assert.containsSubset(defineConfig({}), {
+      allowMethodSpoofing: false,
+      etag: false,
+      cookie: {
+        httpOnly: true,
+        path: '/',
+        maxAge: 7200,
+        sameSite: false,
+        secure: false,
+      },
+      generateRequestId: false,
+      jsonpCallbackName: 'callback',
+      subdomainOffset: 2,
+      useAsyncLocalStorage: false,
+    })
+  })
+
+  test('give preference to defined config', ({ assert }) => {
+    assert.containsSubset(defineConfig({ allowMethodSpoofing: true }), {
+      allowMethodSpoofing: true,
+      etag: false,
+      cookie: {
+        httpOnly: true,
+        path: '/',
+        maxAge: 7200,
+        sameSite: false,
+        secure: false,
+      },
+      generateRequestId: false,
+      jsonpCallbackName: 'callback',
+      subdomainOffset: 2,
+      useAsyncLocalStorage: false,
+    })
+  })
+})