refactor: Remove global debug logger mock and implement no-op logger for improved logging control

Author: lewisjcs

src/keys/get-management-token.spec.ts

@@ -14,11 +14,8 @@ import { Logger } from '../utils'
 import { sign } from 'jsonwebtoken'
 import { LRUCache } from 'lru-cache'
 
-// Mock the debug logger to spy on it
-const mockDebugInstance = {
-  extend: sinon.stub().returns(sinon.spy()),
-}
-sinon.stub(require('debug'), 'default').returns(mockDebugInstance)
+// No longer need to mock debug globally
+// sinon.stub(require('debug'), 'default').returns(mockDebugInstance)
 
 const PRIVATE_KEY = fs.readFileSync(path.join(__dirname, '..', '..', 'keys', 'key.pem'), 'utf-8')
 const APP_ID = 'app_id'
@@ -36,24 +33,38 @@ const defaultCache: LRUCache<string, string> = new LRUCache({ max: 10 })
 
 describe('getManagementToken', () => {
   let loggerSpy: sinon.SinonSpy
+  let createLoggerStub: sinon.SinonStub
 
   beforeEach(() => {
     // Reset the spy before each test
-    loggerSpy = mockDebugInstance.extend() as sinon.SinonSpy
-    loggerSpy.resetHistory()
+    loggerSpy = sinon.spy()
+    // Reset stubs if they exist
+    if (createLoggerStub) {
+      createLoggerStub.restore()
+    }
     // Reset cache as well
     defaultCache.clear()
   })
 
+  afterEach(() => {
+    // Ensure stubs are restored after each test
+    if (createLoggerStub) {
+      createLoggerStub.restore()
+    }
+  })
+
   it('fetches a token and logs by default', async () => {
     const mockToken = 'token'
-    // Use the real logger creator, but we spy on the debug instance it creates
-    const { createLogger } = await import('../utils/logger')
-    const logger = createLogger({ filename: __filename })
+    // Stub createLogger to return our spy
+    const loggerUtils = await import('../utils/logger')
+    const mockLogger = { log: loggerSpy } as unknown as Logger // Create a mock logger object
+    createLoggerStub = sinon.stub(loggerUtils, 'createLogger').returns(mockLogger)
+
     const post = sinon.stub()
     post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) })
     const httpClient = { post } as unknown as HttpClient
-    const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache)
+    // createGetManagementToken will now receive the mockLogger via the stub
+    const getManagementTokenFn = createGetManagementToken(mockLogger, httpClient, defaultCache)
 
     const result = await getManagementTokenFn(PRIVATE_KEY, DEFAULT_OPTIONS) // disableLogging is default false
 
@@ -70,12 +81,16 @@ describe('getManagementToken', () => {
 
   it('does not log when disableLogging is true', async () => {
     const mockToken = 'token-no-log'
-    const { createLogger } = await import('../utils/logger')
-    const logger = createLogger({ filename: __filename })
+    // Stub createLogger like before
+    const loggerUtils = await import('../utils/logger')
+    const mockLogger = { log: loggerSpy } as unknown as Logger
+    createLoggerStub = sinon.stub(loggerUtils, 'createLogger').returns(mockLogger)
+
     const post = sinon.stub()
     post.resolves({ statusCode: 201, body: JSON.stringify({ token: mockToken }) })
     const httpClient = { post } as unknown as HttpClient
-    const getManagementTokenFn = createGetManagementToken(logger, httpClient, defaultCache)
+    // createGetManagementToken receives the stubbed logger
+    const getManagementTokenFn = createGetManagementToken(mockLogger, httpClient, defaultCache)
 
     const result = await getManagementTokenFn(PRIVATE_KEY, {
       ...DEFAULT_OPTIONS,
@@ -90,6 +105,8 @@ describe('getManagementToken', () => {
       ),
     )
     // Assert that the logger spy was NOT called
+    // Because disableLogging: true, the internal logic uses noOpLogger,
+    // so the log method of the passed-in mockLogger (our spy) is never invoked.
     assert(!loggerSpy.called, 'Logger should not have been called when disableLogging is true')
   })
 

src/keys/get-management-token.ts

@@ -23,22 +23,26 @@ export interface GetManagementTokenOptions {
 
 let defaultCache: LRUCache<string, string> | undefined
 
+// Create a logger with a specific namespace that is unlikely to be enabled,
+// effectively making it a no-op logger while conforming to the Logger type.
+const noOpLogger: Logger = createLogger({ filename: __filename + ':noop' })
+
 /**
  * Synchronously sign the given privateKey into a JSON Web Token string
  * @internal
  */
 const generateOneTimeToken = (
   privateKey: string,
   { appId, keyId }: { appId: string; keyId?: string },
-  { log, disableLogging }: { log: Logger; disableLogging: boolean },
+  { log }: { log: Logger },
 ): string => {
-  if (!disableLogging) log('Signing a JWT token with private key')
+  log('Signing a JWT token with private key')
   try {
     const baseSignOptions: SignOptions = { algorithm: 'RS256', issuer: appId, expiresIn: '10m' }
     const signOptions: SignOptions = keyId ? { ...baseSignOptions, keyid: keyId } : baseSignOptions
 
     const token = sign({}, privateKey, signOptions)
-    if (!disableLogging) log('Successfully signed token')
+    log('Successfully signed token')
     return token
   } catch (e) {
     log('Unable to sign token')
@@ -53,11 +57,11 @@ const getTokenFromOneTimeToken = async (
     spaceId,
     environmentId,
   }: { appInstallationId: string; spaceId: string; environmentId: string },
-  { log, http, disableLogging }: { log: Logger; http: HttpClient; disableLogging: boolean },
+  { log, http }: { log: Logger; http: HttpClient },
 ): Promise<string> => {
   const validateStatusCode = createValidateStatusCode([201])
 
-  if (!disableLogging) log(`Requesting CMA Token with given App Token`)
+  log(`Requesting CMA Token with given App Token`)
 
   const response = await http.post(
     `spaces/${spaceId}/environments/${environmentId}/app_installations/${appInstallationId}/access_tokens`,
@@ -71,10 +75,9 @@ const getTokenFromOneTimeToken = async (
     },
   )
 
-  if (!disableLogging)
-    log(
-      `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`,
-    )
+  log(
+    `Successfully retrieved CMA Token for app ${appInstallationId} in space ${spaceId} and environment ${environmentId}`,
+  )
 
   return JSON.parse(response.body).token
 }
@@ -95,6 +98,7 @@ export const createGetManagementToken = (
 
     const reuseToken = opts.reuseToken ?? true
     const disableLogging = opts.disableLogging ?? false
+    const effectiveLog = disableLogging ? noOpLogger : log
 
     const cacheKey =
       opts.appInstallationId + opts.spaceId + opts.environmentId + privateKey.slice(32, 132)
@@ -108,9 +112,9 @@ export const createGetManagementToken = (
     const appToken = generateOneTimeToken(
       privateKey,
       { appId: opts.appInstallationId, keyId: opts.keyId },
-      { log, disableLogging },
+      { log: effectiveLog },
     )
-    const ott = await getTokenFromOneTimeToken(appToken, opts, { log, http, disableLogging })
+    const ott = await getTokenFromOneTimeToken(appToken, opts, { log: effectiveLog, http })
     if (reuseToken) {
       const decoded = decode(ott)
       if (decoded && typeof decoded === 'object' && decoded.exp) {
@@ -153,9 +157,11 @@ export const getManagementToken = (privateKey: string, opts: GetManagementTokenO
     defaultCache = new LRUCache({ max: 10 })
   }
   const httpClientOpts = typeof opts.host !== 'undefined' ? { prefixUrl: opts.host } : {}
+  const disableLogging = opts.disableLogging ?? false
+  const effectiveLog = disableLogging ? noOpLogger : createLogger({ filename: __filename })
 
   return createGetManagementToken(
-    createLogger({ filename: __filename }),
+    effectiveLog,
     createHttpClient(httpClientOpts),
     defaultCache!,
   )(privateKey, opts)