feat(logger): move log level away from the log

LedgerHQ · Feb 14, 2024 · 43a4745 · 43a4745
1 parent 454ff1d
commit 43a4745
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 68 deletions.
diff --git a/packages/core/src/api/Logger.test.ts b/packages/core/src/api/Logger.test.ts
@@ -50,35 +50,31 @@ describe("ConsoleLogger", () => {
     });
 
     it("should log Info level", () => {
-      logObject.setLevel(LogLevel.Info);
-      logger.log(logObject);
+      logger.log(LogLevel.Info, logObject);
       expect(info).toHaveBeenCalledWith("[LOGGER]", "test");
     });
 
     it("should log Warn level", () => {
-      logObject.setLevel(LogLevel.Warning);
-      logger.log(logObject);
+      logger.log(LogLevel.Warning, logObject);
       expect(warn).toHaveBeenCalledWith("[LOGGER]", "test");
     });
 
     it("should log Debug level", () => {
-      logObject.setLevel(LogLevel.Debug);
-      logger.log(logObject);
+      logger.log(LogLevel.Debug, logObject);
       expect(debug).toHaveBeenCalledWith("[LOGGER]", "test");
     });
 
     it("should default to Log level if none present", () => {
       // @ts-expect-error disable for tests
-      logObject.setLevel(undefined);
-      logger.log(logObject);
+      logger.log(null, logObject);
       expect(log).toHaveBeenCalledWith("[LOGGER]", "test");
     });
 
     describe("error", () => {
       it("should log Error level", () => {
         const err = new Error("test");
         logObject = LogBuilder.buildFromError(err);
-        logger.log(logObject);
+        logger.log(LogLevel.Error, logObject);
         expect(warn).toHaveBeenCalledWith("[LOGGER]", "test");
         expect(error).toHaveBeenCalledWith(err);
       });
@@ -87,7 +83,7 @@ describe("ConsoleLogger", () => {
         const originalError = new Error("test error");
         const err = new CustomError(originalError);
         logObject = LogBuilder.buildFromError(err);
-        logger.log(logObject);
+        logger.log(LogLevel.Error, logObject);
         expect(logObject.context.tag).toBe("CustomError");
         expect(warn).toHaveBeenCalledWith("[LOGGER]", "test error");
         expect(error).toHaveBeenCalledWith(originalError);
@@ -96,16 +92,15 @@ describe("ConsoleLogger", () => {
       it("should log Error level with custom error and no original error", () => {
         const err = new CustomError();
         logObject = LogBuilder.buildFromError(err);
-        logger.log(logObject);
+        logger.log(LogLevel.Error, logObject);
         expect(logObject.context.tag).toBe("CustomError");
         expect(warn).toHaveBeenCalledWith("[LOGGER]", "CustomError");
         expect(error).toHaveBeenCalledWith(err);
       });
 
       it("should log Warn level if no error type in context", () => {
         logObject = LogBuilder.build({ type: "test" }, {}, "test");
-        logObject.setLevel(LogLevel.Error);
-        logger.log(logObject);
+        logger.log(LogLevel.Error, logObject);
         expect(warn).toHaveBeenCalledWith(
           "[LOGGER]",
           "[type !== 'error']",

diff --git a/packages/core/src/api/Logger.ts b/packages/core/src/api/Logger.ts
@@ -11,8 +11,8 @@ export type {
 } from "@internal/logger/service/Log";
 
 export class ConsoleLogger implements LoggerSubscriber {
-  log(log: Log): void {
-    switch (log.level) {
+  log(level: LogLevel, log: Log): void {
+    switch (level) {
       case LogLevel.Info:
         console.info("[LOGGER]", ...log.messages);
         break;

diff --git a/packages/core/src/internal/logger/service/DefaultLoggerService.test.ts b/packages/core/src/internal/logger/service/DefaultLoggerService.test.ts
@@ -1,9 +1,8 @@
 import { DefaultLoggerService } from "./DefaultLoggerService";
 import { LogLevel } from "./Log";
 import { LogBuilder } from "./LogBuilder";
-import { LoggerService } from "./LoggerService";
 
-let service: LoggerService;
+let service: DefaultLoggerService;
 
 const subscriber = {
   log: jest.fn(),
@@ -16,12 +15,11 @@ describe("LoggerService", () => {
     const log = LogBuilder.build({ type: "test" }, { key: "value" }, "message");
 
     service.info(log);
-
-    expect(subscriber.log).toHaveBeenCalledWith(log);
+    expect(subscriber.log).toHaveBeenCalledWith(LogLevel.Info, log);
   });
 
   describe("info", () => {
-    it("should have the correct LogLevel", () => {
+    it("should call _log with the correct LogLevel", () => {
       subscriber.log.mockClear();
       service = new DefaultLoggerService([subscriber]);
       const log = LogBuilder.build(
@@ -30,9 +28,11 @@ describe("LoggerService", () => {
         "message",
       );
 
-      service.info(log);
+      const spy = jest.spyOn(service, "_log").mockImplementation(jest.fn());
 
-      expect(log.level).toBe(LogLevel.Info);
+      service.info(log);
+      expect(spy).toHaveBeenCalledWith(LogLevel.Info, log);
+      spy.mockRestore();
     });
   });
 
@@ -46,9 +46,11 @@ describe("LoggerService", () => {
         "message",
       );
 
-      service.debug(log);
+      const spy = jest.spyOn(service, "_log").mockImplementation(jest.fn());
 
-      expect(log.level).toBe(LogLevel.Debug);
+      service.debug(log);
+      expect(spy).toHaveBeenCalledWith(LogLevel.Debug, log);
+      spy.mockRestore();
     });
   });
 
@@ -62,9 +64,11 @@ describe("LoggerService", () => {
         "message",
       );
 
-      service.warn(log);
+      const spy = jest.spyOn(service, "_log").mockImplementation(jest.fn());
 
-      expect(log.level).toBe(LogLevel.Warning);
+      service.warn(log);
+      expect(spy).toHaveBeenCalledWith(LogLevel.Warning, log);
+      spy.mockRestore();
     });
   });
 
@@ -78,9 +82,11 @@ describe("LoggerService", () => {
         { key: "value" },
       );
 
-      service.error(log);
+      const spy = jest.spyOn(service, "_log").mockImplementation(jest.fn());
 
-      expect(log.level).toBe(LogLevel.Error);
+      service.error(log);
+      expect(spy).toHaveBeenCalledWith(LogLevel.Error, log);
+      spy.mockRestore();
     });
   });
 });
diff --git a/packages/core/src/internal/logger/service/DefaultLoggerService.ts b/packages/core/src/internal/logger/service/DefaultLoggerService.ts
@@ -10,29 +10,25 @@ export class DefaultLoggerService implements LoggerService {
     this.subscribers = subscribers;
   }
 
-  _log(log: Log): void {
+  _log(level: LogLevel, log: Log): void {
     this.subscribers.forEach((subscriber) => {
-      subscriber.log(log);
+      subscriber.log(level, log);
     });
   }
 
   info(log: Log): void {
-    log.setLevel(LogLevel.Info);
-    this._log(log);
+    this._log(LogLevel.Info, log);
   }
 
   warn(log: Log): void {
-    log.setLevel(LogLevel.Warning);
-    this._log(log);
+    this._log(LogLevel.Warning, log);
   }
 
   debug(log: Log): void {
-    log.setLevel(LogLevel.Debug);
-    this._log(log);
+    this._log(LogLevel.Debug, log);
   }
 
   error(log: Log): void {
-    log.setLevel(LogLevel.Error);
-    this._log(log);
+    this._log(LogLevel.Error, log);
   }
 }
diff --git a/packages/core/src/internal/logger/service/Log.test.ts b/packages/core/src/internal/logger/service/Log.test.ts
@@ -1,23 +1,11 @@
-import { Log, LogLevel } from "./Log";
+import { Log } from "./Log";
 
 describe("Log", () => {
   it("should create a Log instance", () => {
     const log = new Log({ messages: ["test"], data: {}, context: {} });
     expect(log).toBeInstanceOf(Log);
   });
 
-  it("should set the level", () => {
-    const log = new Log({
-      messages: ["test"],
-      data: {},
-      context: {},
-      level: LogLevel.Info,
-    });
-    expect(log.level).toBe(LogLevel.Info);
-    log.setLevel(LogLevel.Debug);
-    expect(log.level).toBe(LogLevel.Debug);
-  });
-
   it("should add a message", () => {
     const log = new Log({ messages: ["test"], data: {}, context: {} });
     log.addMessage("test2");

diff --git a/packages/core/src/internal/logger/service/Log.ts b/packages/core/src/internal/logger/service/Log.ts
@@ -1,9 +1,9 @@
 export enum LogLevel {
-  Fatal,
-  Error,
-  Warning,
-  Info,
-  Debug,
+  Fatal = "FATAL",
+  Error = "ERROR",
+  Warning = "WARNING",
+  Info = "INFO",
+  Debug = "DEBUG",
 }
 
 export type LogContext = Partial<{
@@ -20,31 +20,23 @@ export type LogContructorArgs = {
   messages: LogMessages;
   data: LogData;
   context: LogContext;
-  level?: LogLevel;
 };
 
 export interface LoggerSubscriber {
-  log(log: Log): void;
+  log(level: LogLevel, log: Log): void;
 }
 
 export class Log {
-  level: LogLevel = LogLevel.Info;
   messages: string[];
   data: Record<string, unknown> = {}; // use Maybe type for null/undefined ?
   context: LogContext = {}; // use Maybe type for null/undefined ?
 
-  constructor({ messages, data, context, level }: LogContructorArgs) {
-    this.level = level ?? this.level;
+  constructor({ messages, data, context }: LogContructorArgs) {
     this.messages = messages;
     this.data = data;
     this.context = context;
   }
 
-  setLevel(level: LogLevel) {
-    this.level = level;
-    return this;
-  }
-
   addMessage(message: string) {
     this.messages.push(message);
     return this;

diff --git a/packages/core/src/internal/logger/service/LogBuilder.ts b/packages/core/src/internal/logger/service/LogBuilder.ts
@@ -1,6 +1,6 @@
 import { SdkError } from "@root/src/api/Error";
 
-import { Log, LogContext, LogData, LogLevel, LogMessages } from "./Log";
+import { Log, LogContext, LogData, LogMessages } from "./Log";
 
 export class LogBuilder {
   static build(
@@ -19,7 +19,6 @@ export class LogBuilder {
     const isSdkError = "_tag" in error;
 
     return new Log({
-      level: LogLevel.Error,
       context: {
         ...context,
         type: "error",