perf: cache process.env value to increase performance (#1238)

adonisjs · Jun 10, 2020 · 291399a · 291399a
1 parent fac6bca
commit 291399a
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 51 deletions.
diff --git a/providers/AppProvider.ts b/providers/AppProvider.ts
@@ -16,6 +16,7 @@ import { HealthCheck } from '../src/HealthCheck'
 import envChecker from '../src/HealthCheck/Checkers/Env'
 import appKeyChecker from '../src/HealthCheck/Checkers/AppKey'
 import { HttpExceptionHandler } from '../src/HttpExceptionHandler'
+import { EnvContract } from '@ioc:Adonis/Core/Env'
 
 /**
  * The application provider that sticks all core components
@@ -106,10 +107,11 @@ export default class AppProvider {
    * Registers base health checkers
    */
   protected registerHealthCheckers () {
-    this.container.with(['Adonis/Core/HealthCheck'], (healthCheck: HealthCheck) => {
-      envChecker(healthCheck)
-      appKeyChecker(healthCheck)
-    })
+    this.container.with(['Adonis/Core/Env', 'Adonis/Core/HealthCheck'],
+      (env: EnvContract, healthCheck: HealthCheck) => {
+        envChecker(healthCheck, env.get('NODE_ENV', undefined) as string | undefined)
+        appKeyChecker(healthCheck, env.get('APP_KEY', undefined) as string | undefined)
+      })
   }
 
   /**

diff --git a/src/HealthCheck/Checkers/AppKey.ts b/src/HealthCheck/Checkers/AppKey.ts
@@ -32,9 +32,9 @@ const DISPLAY_NAME = 'App Key Check'
  * Check for the APP_KEY to ensure it is present and has
  * desired length.
  */
-export default function addAppKeyChecker (healthCheck: HealthCheckContract) {
+export default function addAppKeyChecker (healthCheck: HealthCheckContract, appKey?: string) {
   healthCheck.addChecker('appKey', async () => {
-    if (!process.env.APP_KEY) {
+    if (!appKey) {
       return {
         displayName: DISPLAY_NAME,
         health: {
@@ -44,7 +44,7 @@ export default function addAppKeyChecker (healthCheck: HealthCheckContract) {
       }
     }
 
-    if (process.env.APP_KEY && process.env.APP_KEY.length < 32) {
+    if (appKey && appKey.length < 32) {
       return {
         displayName: DISPLAY_NAME,
         health: {

diff --git a/src/HealthCheck/Checkers/Env.ts b/src/HealthCheck/Checkers/Env.ts
@@ -12,7 +12,7 @@ import { HealthCheckContract } from '@ioc:Adonis/Core/HealthCheck'
 /**
  * Message for missing app key
  */
-const MISSING_APP_KEY_MESSAGE = [
+const MISSING_NODE_ENV_MESSAGE = [
   'Missing NODE_ENV environment variable.',
   'It can make some parts of the application misbehave',
 ].join(' ')
@@ -23,9 +23,9 @@ const DISPLAY_NAME = 'Node Env Check'
  * Register the `env` checker to ensure that `NODE_ENV` environment
  * variable is defined.
  */
-export default function addEnvChecker (healthCheck: HealthCheckContract) {
+export default function addEnvChecker (healthCheck: HealthCheckContract, env?: string) {
   healthCheck.addChecker('env', async () => {
-    return process.env.NODE_ENV ? {
+    return env ? {
       displayName: DISPLAY_NAME,
       health: {
         healthy: true,
@@ -34,7 +34,7 @@ export default function addEnvChecker (healthCheck: HealthCheckContract) {
       displayName: DISPLAY_NAME,
       health: {
         healthy: false,
-        message: MISSING_APP_KEY_MESSAGE,
+        message: MISSING_NODE_ENV_MESSAGE,
       },
     }
   })

diff --git a/src/HttpExceptionHandler/index.ts b/src/HttpExceptionHandler/index.ts
@@ -9,6 +9,7 @@
 
 import { LoggerContract } from '@ioc:Adonis/Core/Logger'
 import { HttpContextContract } from '@ioc:Adonis/Core/HttpContext'
+import { NODE_ENV } from '../utils'
 
 /**
  * Http exception handler serves as the base exception handler
@@ -56,7 +57,7 @@ export abstract class HttpExceptionHandler {
    */
   protected disableStatusPagesInDevelopment: boolean = true
 
-  constructor (protected logger: LoggerContract) {
+  constructor (protected logger: LoggerContract, protected environement: string | undefined = NODE_ENV) {
   }
 
   /**
@@ -97,7 +98,7 @@ export abstract class HttpExceptionHandler {
    * which the app is runing
    */
   protected async makeJSONResponse (error: any, ctx: HttpContextContract) {
-    if (process.env.NODE_ENV === 'development') {
+    if (this.environement === 'development') {
       ctx.response.status(error.status).send({
         message: error.message,
         stack: error.stack,
@@ -118,7 +119,7 @@ export abstract class HttpExceptionHandler {
       errors: [
         {
           title: error.message,
-          ...(process.env.NODE_ENV === 'development' ? { detail: error.stack } : {}),
+          ...(this.environement === 'development' ? { detail: error.stack } : {}),
           code: error.code,
           status: error.status,
         },
@@ -132,7 +133,7 @@ export abstract class HttpExceptionHandler {
    */
   protected async makeHtmlResponse (error: any, ctx: HttpContextContract) {
     if (
-      process.env.NODE_ENV === 'development' &&
+      this.environement === 'development' &&
       (!this.expandedStatusPages[error.status] || this.disableStatusPagesInDevelopment)
     ) {
       const Youch = require('youch')

diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -9,6 +9,9 @@
 
 import { esmRequire, resolveFrom } from '@poppinss/utils'
 
+export const APP_KEY = process.env.APP_KEY
+export const NODE_ENV = process.env.NODE_ENV
+
 /**
  * Helper to know if error belongs to a missing module
  * error

diff --git a/test/app-key-checker.spec.ts b/test/app-key-checker.spec.ts
@@ -33,11 +33,9 @@ test.group('Env Health Checker', () => {
   })
 
   test('fail when APP_KEY is not secure', async (assert) => {
-    process.env.APP_KEY = '3910200'
-
     const application = new Application(__dirname, new Ioc(), {}, {})
     const healthCheck = new HealthCheck(application)
-    appKeyHealthChecker(healthCheck)
+    appKeyHealthChecker(healthCheck, '3910200')
 
     const report = await healthCheck.getReport()
     assert.deepEqual(report.report, {
@@ -50,16 +48,12 @@ test.group('Env Health Checker', () => {
         },
       },
     })
-
-    delete process.env.APP_KEY
   })
 
   test('work fine when APP_KEY is secure', async (assert) => {
-    process.env.APP_KEY = 'asecureandlongrandomsecret'
-
     const application = new Application(__dirname, new Ioc(), {}, {})
     const healthCheck = new HealthCheck(application)
-    appKeyHealthChecker(healthCheck)
+    appKeyHealthChecker(healthCheck, 'asecureandlongrandomsecret')
 
     const report = await healthCheck.getReport()
     assert.deepEqual(report.report, {
@@ -72,7 +66,5 @@ test.group('Env Health Checker', () => {
         },
       },
     })
-
-    delete process.env.APP_KEY
   })
 })
diff --git a/test/env-health-checker.spec.ts b/test/env-health-checker.spec.ts
@@ -16,9 +16,6 @@ import envHealthChecker from '../src/HealthCheck/Checkers/Env'
 
 test.group('Env Health Checker', () => {
   test('fail when NODE_ENV is not defined', async (assert) => {
-    const oldEnvValue = process.env.NODE_ENV
-    delete process.env.NODE_ENV
-
     const application = new Application(__dirname, new Ioc(), {}, {})
     const healthCheck = new HealthCheck(application)
     envHealthChecker(healthCheck)
@@ -33,15 +30,12 @@ test.group('Env Health Checker', () => {
         },
       },
     })
-
-    process.env.NODE_ENV = oldEnvValue
   })
 
   test('work fine when NODE_ENV is defined', async (assert) => {
-    process.env.NODE_ENV = 'development'
     const application = new Application(__dirname, new Ioc(), {}, {})
     const healthCheck = new HealthCheck(application)
-    envHealthChecker(healthCheck)
+    envHealthChecker(healthCheck, 'development')
 
     const report = await healthCheck.getReport()
     assert.deepEqual(report.report, {
@@ -52,7 +46,5 @@ test.group('Env Health Checker', () => {
         },
       },
     })
-
-    delete process.env.NODE_ENV
   })
 })
diff --git a/test/exception-handler.spec.ts b/test/exception-handler.spec.ts
@@ -248,7 +248,6 @@ test.group('HttpExceptionHandler', () => {
   })
 
   test('return stack trace when NODE_ENV=development', async (assert) => {
-    process.env.NODE_ENV = 'development'
     class AppHandler extends HttpExceptionHandler {
       protected context () {
         return { username: 'virk' }
@@ -259,7 +258,7 @@ test.group('HttpExceptionHandler', () => {
     }
 
     const logger = new FakeLogger(loggerConfig)
-    const handler = new AppHandler(logger)
+    const handler = new AppHandler(logger, 'development')
 
     const ctx = HttpContext.create(
       '/',
@@ -273,12 +272,9 @@ test.group('HttpExceptionHandler', () => {
 
     await handler.handle(new InvalidAuth('bad request'), ctx)
     assert.exists(ctx.response.lazyBody!.args[0].stack)
-
-    delete process.env.NODE_ENV
   })
 
   test('print youch html in development', async (assert) => {
-    process.env.NODE_ENV = 'development'
     class AppHandler extends HttpExceptionHandler {
       protected context () {
         return { username: 'virk' }
@@ -289,7 +285,7 @@ test.group('HttpExceptionHandler', () => {
     }
 
     const logger = new FakeLogger(loggerConfig)
-    const handler = new AppHandler(logger)
+    const handler = new AppHandler(logger, 'development')
 
     const ctx = HttpContext.create(
       '/',
@@ -303,8 +299,6 @@ test.group('HttpExceptionHandler', () => {
 
     await handler.handle(new InvalidAuth('bad request'), ctx)
     assert.isTrue(/youch/.test(ctx.response.lazyBody!.args[0]))
-
-    delete process.env.NODE_ENV
   })
 
   test('call handle on actual exception when method exists', async (assert) => {
@@ -451,8 +445,6 @@ test.group('HttpExceptionHandler', () => {
   })
 
   test('do not render status page when disabled for development mode', async (assert) => {
-    process.env.NODE_ENV = 'development'
-
     class AppHandler extends HttpExceptionHandler {
       protected statusPages = {
         404: '404.edge',
@@ -472,7 +464,7 @@ test.group('HttpExceptionHandler', () => {
     }
 
     const logger = new FakeLogger(loggerConfig)
-    const handler = new AppHandler(logger)
+    const handler = new AppHandler(logger, 'development')
 
     const ctx = HttpContext.create(
       '/',
@@ -493,15 +485,11 @@ test.group('HttpExceptionHandler', () => {
 
     assert.isTrue(/youch/.test(ctx.response.lazyBody!.args[0]))
     assert.equal(ctx.response.response.statusCode, 404)
-
-    delete process.env.NODE_ENV
   })
 
   test('always render status page when in production mode', async (assert) => {
     assert.plan(3)
 
-    process.env.NODE_ENV = 'production'
-
     class AppHandler extends HttpExceptionHandler {
       protected statusPages = {
         404: '404.edge',
@@ -521,7 +509,7 @@ test.group('HttpExceptionHandler', () => {
     }
 
     const logger = new FakeLogger(loggerConfig)
-    const handler = new AppHandler(logger)
+    const handler = new AppHandler(logger, 'production')
 
     const ctx = HttpContext.create(
       '/',
@@ -542,7 +530,6 @@ test.group('HttpExceptionHandler', () => {
     await handler.handle(new InvalidAuth('bad request'), ctx)
 
     assert.equal(ctx.response.response.statusCode, 404)
-    delete process.env.NODE_ENV
   })
 
   test('compute status pages from expression', async (assert) => {