Lazy Logging + SLF4J best practices OpenRewrite recipe

CRoberto1926 · Nov 15, 2024 · 4f2b436 · 4f2b436
1 parent 8e2663d
commit 4f2b436
Show file tree

Hide file tree

Showing 47 changed files with 750 additions and 818 deletions.
diff --git a/common/kadai-common-data/src/main/java/io/kadai/sampledata/SampleDataGenerator.java b/common/kadai-common-data/src/main/java/io/kadai/sampledata/SampleDataGenerator.java
@@ -103,28 +103,32 @@ private List<String> parseScripts(Stream<String> scripts) {
 
   private void runScripts(Consumer<ScriptRunner> consumer) {
     try (Connection connection = dataSource.getConnection()) {
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug(
-            "Generating sample data for database of type '{}' with url '{}' and schema '{}'.",
-            DB.getDB(connection).dbProductName,
-            connection.getMetaData().getURL(),
-            schema);
-      }
+      LOGGER
+          .atDebug()
+          .setMessage(
+              "Generating sample data for database of type '{}' with url '{}' and schema '{}'.")
+          .addArgument(() -> DB.getDB(connection).dbProductName)
+          .addArgument(
+              () -> {
+                try {
+                  return connection.getMetaData().getURL();
+                } catch (SQLException e) {
+                  throw new RuntimeSqlException("Failed to get URL for database.", e);
+                }
+              })
+          .addArgument(() -> schema)
+          .log();
 
       StringWriter outWriter = new StringWriter();
       StringWriter errorWriter = new StringWriter();
 
       ScriptRunner runner = getScriptRunner(connection, outWriter, errorWriter);
       consumer.accept(runner);
 
-      if (LOGGER.isTraceEnabled()) {
-        LOGGER.trace(outWriter.toString());
-      }
-      if (LOGGER.isErrorEnabled()) {
-        String trimmedErrorString = errorWriter.toString().trim();
-        if (!trimmedErrorString.isEmpty()) {
-          LOGGER.error(trimmedErrorString);
-        }
+      LOGGER.atTrace().setMessage(outWriter::toString).log();
+
+      if (!errorWriter.getBuffer().isEmpty()) {
+        LOGGER.atError().setMessage(() -> errorWriter.toString().trim()).log();
       }
     } catch (SQLException e) {
       throw new RuntimeSqlException("Failed to execute script.", e);

diff --git a/...on/kadai-common-logging/src/main/java/io/kadai/common/internal/logging/LoggingAspect.java b/...on/kadai-common-logging/src/main/java/io/kadai/common/internal/logging/LoggingAspect.java
@@ -44,22 +44,6 @@ public static boolean isLoggingAspectEnabled() {
     return LazyHolder.LOGGING_ASPECT_ENABLED;
   }
 
-  private static String mapParametersNameValue(String[] parameterNames, Object[] values) {
-    Map<String, Object> parametersNameToValue = new HashMap<>();
-
-    if (parameterNames.length > 0) {
-      for (int i = 0; i < parameterNames.length; i++) {
-        parametersNameToValue.put(parameterNames[i], values[i]);
-      }
-    }
-
-    StringBuilder stringBuilder = new StringBuilder();
-    for (Entry<String, Object> parameter : parametersNameToValue.entrySet()) {
-      stringBuilder.append(parameter.getKey()).append(" = ").append(parameter.getValue());
-    }
-    return stringBuilder.toString();
-  }
-
   @Pointcut(
       "!@annotation(io.kadai.common.internal.logging.NoLogging)"
           + " && !within(@io.kadai.common.internal.logging.NoLogging *)"
@@ -80,14 +64,12 @@ public void beforeMethodExecuted(JoinPoint joinPoint) {
       Logger currentLogger =
           CLASS_TO_LOGGER.computeIfAbsent(declaringTypeName, LoggerFactory::getLogger);
 
-      if (currentLogger.isTraceEnabled()) {
-        String methodName = methodSignature.getName();
-        Object[] values = joinPoint.getArgs();
-        String[] parameterNames = methodSignature.getParameterNames();
-        String parametersValues = mapParametersNameValue(parameterNames, values);
-
-        currentLogger.trace("entry to {}({})", methodName, parametersValues);
-      }
+      currentLogger
+          .atTrace()
+          .setMessage("entry to {}({})")
+          .addArgument(methodSignature::getName)
+          .addArgument(() -> mapParametersNameValue(joinPoint))
+          .log();
     }
   }
 
@@ -115,6 +97,25 @@ public void afterMethodExecuted(JoinPoint joinPoint, Object returnedObject) {
     }
   }
 
+  private static String mapParametersNameValue(JoinPoint joinPoint) {
+    Object[] values = joinPoint.getArgs();
+    MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature();
+    String[] parameterNames = methodSignature.getParameterNames();
+    Map<String, Object> parametersNameToValue = new HashMap<>();
+
+    if (parameterNames.length > 0) {
+      for (int i = 0; i < parameterNames.length; i++) {
+        parametersNameToValue.put(parameterNames[i], values[i]);
+      }
+    }
+
+    StringBuilder stringBuilder = new StringBuilder();
+    for (Entry<String, Object> parameter : parametersNameToValue.entrySet()) {
+      stringBuilder.append(parameter.getKey()).append(" = ").append(parameter.getValue());
+    }
+    return stringBuilder.toString();
+  }
+
   // This Initialization-on-demand holder idiom is necessary so that the retrieval of the system
   // property will be executed during the execution of the first JointPoint.
   // This allows us to set the system property during test execution BEFORE retrieving the system

diff --git a/...mmon-security/src/main/java/io/kadai/common/internal/security/CurrentUserContextImpl.java b/...mmon-security/src/main/java/io/kadai/common/internal/security/CurrentUserContextImpl.java
@@ -48,14 +48,10 @@ public CurrentUserContextImpl(boolean shouldUseLowerCaseForAccessIds) {
     this.shouldUseLowerCaseForAccessIds = shouldUseLowerCaseForAccessIds;
     try {
       Class.forName(WSSUBJECT_CLASSNAME);
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("WSSubject detected. Assuming that Kadai runs on IBM WebSphere.");
-      }
+      LOGGER.debug("WSSubject detected. Assuming that Kadai runs on IBM WebSphere.");
       runningOnWebSphere = true;
     } catch (ClassNotFoundException e) {
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("No WSSubject detected. Using JAAS subject further on.");
-      }
+      LOGGER.debug("No WSSubject detected. Using JAAS subject further on.");
       runningOnWebSphere = false;
     }
   }
@@ -103,14 +99,10 @@ private String getUserIdFromWsSubject() {
       Method getCallerSubjectMethod =
           wsSubjectClass.getMethod(GET_CALLER_SUBJECT_METHOD, (Class<?>[]) null);
       Subject callerSubject = (Subject) getCallerSubjectMethod.invoke(null, (Object[]) null);
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("Subject of caller: {}", callerSubject);
-      }
+      LOGGER.debug("Subject of caller: {}", callerSubject);
       if (callerSubject != null) {
         Set<Object> publicCredentials = callerSubject.getPublicCredentials();
-        if (LOGGER.isDebugEnabled()) {
-          LOGGER.debug("Public credentials of caller: {}", publicCredentials);
-        }
+        LOGGER.debug("Public credentials of caller: {}", publicCredentials);
         return publicCredentials.stream()
             .map(
                 // we could use CheckedFunction#wrap here, but this either requires a dependency

diff --git a/...on/kadai-common/src/main/java/io/kadai/common/internal/configuration/DbSchemaCreator.java b/...on/kadai-common/src/main/java/io/kadai/common/internal/configuration/DbSchemaCreator.java
@@ -61,13 +61,20 @@ public DbSchemaCreator(DataSource dataSource, String schema) {
    */
   public boolean run() throws SQLException {
     try (Connection connection = dataSource.getConnection()) {
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug(
-            "Using database of type {} with url '{}'",
-            DB.getDB(connection).dbProductName,
-            connection.getMetaData().getURL());
-      }
       DB db = DB.getDB(connection);
+      LOGGER
+          .atDebug()
+          .setMessage("Using database of type {} with url '{}'")
+          .addArgument(db.dbProductName)
+          .addArgument(
+              () -> {
+                try {
+                  return connection.getMetaData().getURL();
+                } catch (SQLException e) {
+                  throw new RuntimeException(e);
+                }
+              })
+          .log();
 
       ScriptRunner runner = getScriptRunnerInstance(connection);
 
@@ -80,11 +87,9 @@ public boolean run() throws SQLException {
         return true;
       }
     }
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug(outWriter.toString());
-    }
-    if (!errorWriter.toString().trim().isEmpty()) {
-      LOGGER.error(errorWriter.toString());
+    LOGGER.atDebug().setMessage(outWriter::toString).log();
+    if (!errorWriter.getBuffer().isEmpty()) {
+      LOGGER.atError().setMessage(errorWriter::toString).log();
     }
     return false;
   }
@@ -93,9 +98,18 @@ public boolean isValidSchemaVersion(String expectedMinVersion) {
     try (Connection connection = dataSource.getConnection()) {
       connection.setSchema(this.schemaName);
       SqlRunner runner = new SqlRunner(connection);
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("{}", connection.getMetaData());
-      }
+      LOGGER
+          .atDebug()
+          .setMessage("{}")
+          .addArgument(
+              () -> {
+                try {
+                  return connection.getMetaData();
+                } catch (SQLException e) {
+                  throw new RuntimeException(e);
+                }
+              })
+          .log();
 
       String query =
           "select VERSION from KADAI_SCHEMA_VERSION where "
@@ -113,9 +127,7 @@ public boolean isValidSchemaVersion(String expectedMinVersion) {
             expectedMinVersion);
         return false;
       } else {
-        if (LOGGER.isDebugEnabled()) {
-          LOGGER.debug("Schema version is valid.");
-        }
+        LOGGER.debug("Schema version is valid.");
         return true;
       }
 
@@ -154,17 +166,13 @@ private boolean isSchemaPreexisting(Connection connection, DB db) {
         BufferedReader reader = new BufferedReader(inputReader)) {
       runner.runScript(getSqlSchemaNameParsed(reader));
     } catch (RuntimeSqlException | IOException e) {
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("Schema does not exist.");
-        if (!errorWriter.toString().trim().isEmpty()) {
-          LOGGER.debug(errorWriter.toString());
-        }
+      LOGGER.debug("Schema does not exist.");
+      if (!errorWriter.getBuffer().isEmpty()) {
+        LOGGER.atDebug().setMessage(errorWriter::toString).log();
       }
       return false;
     }
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("Schema does exist.");
-    }
+    LOGGER.debug("Schema does exist.");
     return true;
   }
 

diff --git a/common/kadai-common/src/main/java/io/kadai/common/internal/persistence/MapTypeHandler.java b/common/kadai-common/src/main/java/io/kadai/common/internal/persistence/MapTypeHandler.java
@@ -39,10 +39,8 @@ public class MapTypeHandler extends BaseTypeHandler<Map<String, Object>> {
   public void setNonNullParameter(
       PreparedStatement ps, int i, Map<String, Object> parameter, JdbcType jdbcType)
       throws SQLException {
-    if (parameter != null && parameter.size() > 0) {
-      if (LOGGER.isDebugEnabled()) {
-        LOGGER.debug("Input-Map before serializing: {}", parameter);
-      }
+    if (parameter != null && !parameter.isEmpty()) {
+      LOGGER.debug("Input-Map before serializing: {}", parameter);
       // Convert Map to JSON string
       JSONObject jsonObj = new JSONObject(parameter);
       ps.setString(i, jsonObj.toString());

diff --git a/...loghistory-provider/src/main/java/io/kadai/loghistory/impl/LogfileHistoryServiceImpl.java b/...loghistory-provider/src/main/java/io/kadai/loghistory/impl/LogfileHistoryServiceImpl.java
@@ -50,46 +50,59 @@ public void initialize(KadaiEngine kadaiEngine) {
       historyLogger = LOGGER;
     }
 
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug(
-          "LogfileHistoryServiceProvider initialized with name: {} ", historyLogger.getName());
-    }
+    LOGGER
+        .atDebug()
+        .setMessage("LogfileHistoryServiceProvider initialized with name: {} ")
+        .addArgument(() -> historyLogger.getName())
+        .log();
   }
 
   @Override
   public void create(TaskHistoryEvent event) {
-
-    try {
-      if (historyLogger.isInfoEnabled()) {
-        historyLogger.info(objectMapper.writeValueAsString(event));
-      }
-    } catch (JsonProcessingException e) {
-      throw new SystemException("Caught exception while serializing history event to JSON ", e);
-    }
+    historyLogger
+        .atInfo()
+        .setMessage(
+            () -> {
+              try {
+                return objectMapper.writeValueAsString(event);
+              } catch (JsonProcessingException e) {
+                throw new SystemException(
+                    "Caught exception while serializing history event to JSON ", e);
+              }
+            })
+        .log();
   }
 
   @Override
   public void create(WorkbasketHistoryEvent event) {
-
-    try {
-      if (historyLogger.isInfoEnabled()) {
-        historyLogger.info(objectMapper.writeValueAsString(event));
-      }
-    } catch (JsonProcessingException e) {
-      throw new SystemException("Caught exception while serializing history event to JSON ", e);
-    }
+    historyLogger
+        .atInfo()
+        .setMessage(
+            () -> {
+              try {
+                return objectMapper.writeValueAsString(event);
+              } catch (JsonProcessingException e) {
+                throw new SystemException(
+                    "Caught exception while serializing history event to JSON ", e);
+              }
+            })
+        .log();
   }
 
   @Override
   public void create(ClassificationHistoryEvent event) {
-
-    try {
-      if (historyLogger.isInfoEnabled()) {
-        historyLogger.info(objectMapper.writeValueAsString(event));
-      }
-    } catch (JsonProcessingException e) {
-      throw new SystemException("Caught exception while serializing history event to JSON ", e);
-    }
+    historyLogger
+        .atInfo()
+        .setMessage(
+            () -> {
+              try {
+                return objectMapper.writeValueAsString(event);
+              } catch (JsonProcessingException e) {
+                throw new SystemException(
+                    "Caught exception while serializing history event to JSON ", e);
+              }
+            })
+        .log();
   }
 
   @Override

diff --git a/...plehistory-provider/src/main/java/io/kadai/simplehistory/impl/jobs/HistoryCleanupJob.java b/...plehistory-provider/src/main/java/io/kadai/simplehistory/impl/jobs/HistoryCleanupJob.java
@@ -198,9 +198,7 @@ private int deleteEvents(List<String> taskIdsToDeleteHistoryEventsFor)
 
     simpleHistoryService.deleteHistoryEventsByTaskIds(taskIdsToDeleteHistoryEventsFor);
 
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("{} events deleted.", deletedTasksCount);
-    }
+    LOGGER.debug("{} events deleted.", deletedTasksCount);
 
     return deletedTasksCount;
   }

diff --git a/lib/kadai-cdi/src/test/java/io/kadai/KadaiCdiTestRestController.java b/lib/kadai-cdi/src/test/java/io/kadai/KadaiCdiTestRestController.java
@@ -80,7 +80,7 @@ public Response startTask() throws Exception {
 
     Task result = kadaiEjb.getTaskService().createTask(task);
 
-    LOGGER.info(result.getId() + ":" + result.getOwner());
+    LOGGER.info("{}:{}", result.getId(), result.getOwner());
     return Response.status(200).entity(result.getId()).build();
   }