Added FormattableLogMessage to optimize log message format. (#55)

novalisdenahi · web-flow · commit 162687c7fd84 · 2024-07-24T11:13:08.000+02:00
* Added FormattableLogMessage to optimize log message format.

* Updates based on code review.

* Prepare 9.4.0 release

* Refactor based on review

* Fix issues based on quality check.

* Update based on review

* Set formatLogMessage methods to protected visibility.
diff --git a/gradle.properties b/gradle.properties
@@ -1 +1 @@
-version=9.3.0
+version=9.4.0
diff --git a/src/main/java/com/configcat/ConfigCatClient.java b/src/main/java/com/configcat/ConfigCatClient.java
@@ -480,17 +480,17 @@ private boolean checkSettingsAvailable(SettingResult settingResult, String empty
 
     private <T> Result<Setting> checkSettingAvailable(SettingResult settingResult, String key, T defaultValue) {
         if (settingResult.isEmpty()) {
-            String errorMessage = ConfigCatLogMessages.getConfigJsonIsNotPresentedWithDefaultValue(key, "defaultValue", defaultValue);
-            this.logger.error(1000, errorMessage);
-            return Result.error(errorMessage, null);
+            Object formattableLogMessage = ConfigCatLogMessages.getConfigJsonIsNotPresentedWithDefaultValue(key, "defaultValue", defaultValue);
+            this.logger.error(1000, formattableLogMessage);
+            return Result.error(formattableLogMessage, null);
         }
 
         Map<String, Setting> settings = settingResult.settings();
         Setting setting = settings.get(key);
         if (setting == null) {
-            String errorMessage = ConfigCatLogMessages.getSettingEvaluationFailedDueToMissingKey(key, "defaultValue", defaultValue, settings.keySet());
-            this.logger.error(1001, errorMessage);
-            return Result.error(errorMessage, null);
+            FormattableLogMessage formattableLogMessage = ConfigCatLogMessages.getSettingEvaluationFailedDueToMissingKey(key, "defaultValue", defaultValue, settings.keySet());
+            this.logger.error(1001, formattableLogMessage);
+            return Result.error(formattableLogMessage, null);
         }
 
         return Result.success(setting);
@@ -506,9 +506,9 @@ private <T> T getValueFromSettingsMap(Class<T> classOfT, SettingResult settingRe
 
             return this.evaluate(classOfT, checkSettingResult.value(), key, getEvaluateUser(user), settingResult.fetchTime(), settingResult.settings()).getValue();
         } catch (Exception e) {
-            String errorMessage = ConfigCatLogMessages.getSettingEvaluationFailedForOtherReason(key, "defaultValue", defaultValue);
-            this.logger.error(2001, errorMessage, e);
-            this.configCatHooks.invokeOnFlagEvaluated(EvaluationDetails.fromError(key, defaultValue, errorMessage + " " + e.getMessage(), user));
+            FormattableLogMessage formattableLogMessage = ConfigCatLogMessages.getSettingEvaluationFailedForOtherReason(key, "defaultValue", defaultValue);
+            this.logger.error(2001, formattableLogMessage, e);
+            this.configCatHooks.invokeOnFlagEvaluated(EvaluationDetails.fromError(key, defaultValue, formattableLogMessage + " " + e.getMessage(), user));
             return defaultValue;
         }
     }
diff --git a/src/main/java/com/configcat/ConfigCatHooks.java b/src/main/java/com/configcat/ConfigCatHooks.java
@@ -116,11 +116,14 @@ void invokeOnClientReady(ClientCacheState clientCacheState) {
         }
     }
 
-    void invokeOnError(String error) {
+    void invokeOnError(Object error) {
         lock.readLock().lock();
         try {
-            for (Consumer<String> func : this.onError) {
-                func.accept(error);
+            if(!this.onError.isEmpty()) {
+                String errorMessage = error.toString();
+                for (Consumer<String> func : this.onError) {
+                    func.accept(errorMessage);
+                }
             }
         } finally {
             lock.readLock().unlock();
diff --git a/src/main/java/com/configcat/ConfigCatLogMessages.java b/src/main/java/com/configcat/ConfigCatLogMessages.java
diff --git a/src/main/java/com/configcat/ConfigCatLogger.java b/src/main/java/com/configcat/ConfigCatLogger.java
@@ -23,39 +23,39 @@ public ConfigCatLogger(Logger logger) {
         this(logger, LogLevel.WARNING);
     }
 
-    public void warn(int eventId, String message) {
+    public void warn(int eventId, Object message) {
         if (filter(eventId,  LogLevel.WARNING, message, null)) {
             this.logger.warn("[{}] {}", eventId, message);
         }
     }
 
-    public void error(int eventId, String message, Exception exception) {
+    public void error(int eventId, Object message, Exception exception) {
         if (this.configCatHooks != null) this.configCatHooks.invokeOnError(message);
         if (filter(eventId,  LogLevel.ERROR, message, exception)) {
             this.logger.error("[{}] {}", eventId, message, exception);
         }
     }
 
-    public void error(int eventId, String message) {
+    public void error(int eventId, Object message) {
         if (this.configCatHooks != null) this.configCatHooks.invokeOnError(message);
         if (filter(eventId,  LogLevel.ERROR, message, null)) {
             this.logger.error("[{}] {}", eventId, message);
         }
     }
 
-    public void info(int eventId, String message) {
+    public void info(int eventId, Object message) {
         if (filter(eventId,  LogLevel.INFO, message, null)) {
             this.logger.info("[{}] {}", eventId, message);
         }
     }
 
-    public void debug(String message) {
+    public void debug(Object message) {
         if (filter(0,  LogLevel.DEBUG, message, null)) {
             this.logger.debug("[{}] {}", 0, message);
         }
     }
 
-    private boolean filter(int eventId, LogLevel logLevel, String message, Exception exception) {
+    private boolean filter(int eventId, LogLevel logLevel, Object message, Exception exception) {
         return this.logLevel.ordinal() <= logLevel.ordinal() && (this.filterFunction == null || this.filterFunction.apply(logLevel, eventId, message, exception));
     }
 }
diff --git a/src/main/java/com/configcat/ConfigFetcher.java b/src/main/java/com/configcat/ConfigFetcher.java
@@ -98,7 +98,7 @@ private CompletableFuture<FetchResponse> getResponseAsync(final String eTag) {
             @Override
             public void onFailure(@NotNull Call call, @NotNull IOException e) {
                 int logEventId = 1103;
-                String message = ConfigCatLogMessages.FETCH_FAILED_DUE_TO_UNEXPECTED_ERROR;
+                Object message = ConfigCatLogMessages.FETCH_FAILED_DUE_TO_UNEXPECTED_ERROR;
                 if (!isClosed.get()) {
                     if (e instanceof SocketTimeoutException) {
                         logEventId = 1102;
@@ -130,14 +130,14 @@ public void onResponse(@NotNull Call call, @NotNull Response response) {
                         logger.error(1100, message);
                         future.complete(FetchResponse.failed(message, true));
                     } else {
-                        String message = ConfigCatLogMessages.getFetchFailedDueToUnexpectedHttpResponse(response.code(), response.message());
-                        logger.error(1101, message);
-                        future.complete(FetchResponse.failed(message, false));
+                        FormattableLogMessage formattableLogMessage = ConfigCatLogMessages.getFetchFailedDueToUnexpectedHttpResponse(response.code(), response.message());
+                        logger.error(1101, formattableLogMessage);
+                        future.complete(FetchResponse.failed(formattableLogMessage, false));
                     }
                 } catch (SocketTimeoutException e) {
-                    String message = ConfigCatLogMessages.getFetchFailedDueToRequestTimeout(httpClient.connectTimeoutMillis(), httpClient.readTimeoutMillis(), httpClient.writeTimeoutMillis());
-                    logger.error(1102, message, e);
-                    future.complete(FetchResponse.failed(message, false));
+                    FormattableLogMessage formattableLogMessage = ConfigCatLogMessages.getFetchFailedDueToRequestTimeout(httpClient.connectTimeoutMillis(), httpClient.readTimeoutMillis(), httpClient.writeTimeoutMillis());
+                    logger.error(1102, formattableLogMessage, e);
+                    future.complete(FetchResponse.failed(formattableLogMessage, false));
                 } catch (Exception e) {
                     String message = ConfigCatLogMessages.FETCH_FAILED_DUE_TO_UNEXPECTED_ERROR;
                     logger.error(1103, message, e);
diff --git a/src/main/java/com/configcat/ConfigService.java b/src/main/java/com/configcat/ConfigService.java
@@ -59,9 +59,9 @@ public ConfigService(String sdkKey,
                     lock.lock();
                     try {
                         this.configCatHooks.invokeOnClientReady(determineCacheState(cachedEntry.get()));
-                        String message = ConfigCatLogMessages.getAutoPollMaxInitWaitTimeReached(autoPollingMode.getMaxInitWaitTimeSeconds());
-                        this.logger.warn(4200, message);
-                        completeRunningTask(Result.error(message, cachedEntry.get()));
+                        FormattableLogMessage formattableLogMessage = ConfigCatLogMessages.getAutoPollMaxInitWaitTimeReached(autoPollingMode.getMaxInitWaitTimeSeconds());
+                        this.logger.warn(4200, formattableLogMessage);
+                        completeRunningTask(Result.error(formattableLogMessage, cachedEntry.get()));
                     } finally {
                         lock.unlock();
                     }
diff --git a/src/main/java/com/configcat/Constants.java b/src/main/java/com/configcat/Constants.java
@@ -7,7 +7,7 @@ private Constants() { /* prevent from instantiation*/ }
     static final long DISTANT_PAST = 0;
     static final String CONFIG_JSON_NAME = "config_v6.json";
     static final String SERIALIZATION_FORMAT_VERSION = "v2";
-    static final String VERSION = "9.3.0";
+    static final String VERSION = "9.4.0";
 
     static final String SDK_KEY_PROXY_PREFIX = "configcat-proxy/";
     static final String SDK_KEY_PREFIX = "configcat-sdk-1";
diff --git a/src/main/java/com/configcat/EvaluationDetails.java b/src/main/java/com/configcat/EvaluationDetails.java
@@ -9,7 +9,7 @@ public class EvaluationDetails<T> {
     private final String variationId;
     private final User user;
     private final boolean isDefaultValue;
-    private final String error;
+    private final Object error;
     private final long fetchTimeUnixMilliseconds;
     private final TargetingRule matchedTargetingRule;
     private final PercentageOption matchedPercentageOption;
@@ -19,7 +19,7 @@ public EvaluationDetails(T value,
                              String variationId,
                              User user,
                              boolean isDefaultValue,
-                             String error,
+                             Object error,
                              long fetchTimeUnixMilliseconds,
                              TargetingRule matchedTargetingRule,
                              PercentageOption matchedPercentageOption) {
@@ -34,7 +34,7 @@ public EvaluationDetails(T value,
         this.matchedPercentageOption = matchedPercentageOption;
     }
 
-    static <T> EvaluationDetails<T> fromError(String key, T defaultValue, String error, User user) {
+    static <T> EvaluationDetails<T> fromError(String key, T defaultValue, Object error, User user) {
         return new EvaluationDetails<>(defaultValue, key, "", user, true, error, Constants.DISTANT_PAST, null, null);
     }
 
@@ -81,7 +81,10 @@ public boolean isDefaultValue() {
      * In case of an error, this field contains the error message.
      */
     public String getError() {
-        return error;
+        if(error !=  null) {
+            return error.toString();
+        }
+        return null;
     }
 
     /**
diff --git a/src/main/java/com/configcat/FetchResponse.java b/src/main/java/com/configcat/FetchResponse.java
@@ -9,7 +9,7 @@ public enum Status {
 
     private final Status status;
     private final Entry entry;
-    private final String error;
+    private final Object error;
     private final boolean fetchTimeUpdatable;
 
     public boolean isFetched() {
@@ -32,11 +32,11 @@ public Entry entry() {
         return this.entry;
     }
 
-    public String error() {
-        return this.error;
+    public Object error() {
+       return error;
     }
 
-    FetchResponse(Status status, Entry entry, String error, boolean fetchTimeUpdatable) {
+    FetchResponse(Status status, Entry entry, Object error, boolean fetchTimeUpdatable) {
         this.status = status;
         this.entry = entry;
         this.error = error;
@@ -51,7 +51,7 @@ public static FetchResponse notModified() {
         return new FetchResponse(Status.NOT_MODIFIED, Entry.EMPTY, null, true);
     }
 
-    public static FetchResponse failed(String error, boolean fetchTimeUpdatable) {
+    public static FetchResponse failed(Object error, boolean fetchTimeUpdatable) {
         return new FetchResponse(Status.FAILED, Entry.EMPTY, error, fetchTimeUpdatable);
     }
 }
diff --git a/src/main/java/com/configcat/FormattableLogMessage.java b/src/main/java/com/configcat/FormattableLogMessage.java
@@ -0,0 +1,25 @@
+package com.configcat;
+
+class FormattableLogMessage {
+
+    private String cachedMessage;
+    protected final String message;
+    protected final Object[] args;
+
+    FormattableLogMessage(String message, Object... args) {
+        this.message = message;
+        this.args = args;
+    }
+
+    protected String formatLogMessage(){
+        return String.format(message, args);
+    }
+
+    @Override
+    public String toString() {
+        if(cachedMessage == null) {
+            cachedMessage = formatLogMessage();
+        }
+        return cachedMessage;
+    }
+}
diff --git a/src/main/java/com/configcat/FormattableLogMessageWithKeySet.java b/src/main/java/com/configcat/FormattableLogMessageWithKeySet.java
@@ -0,0 +1,21 @@
+package com.configcat;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+class FormattableLogMessageWithKeySet extends FormattableLogMessage {
+
+    FormattableLogMessageWithKeySet(String message, Object... args) {
+        super(message,args);
+    }
+
+    @Override
+    protected String formatLogMessage() {
+        Object keySetObject = args[args.length - 1];
+        if(keySetObject instanceof Set) {
+            Set<String> keySet = (Set<String>) keySetObject;
+            args[args.length - 1] = keySet.stream().map(keyTo -> "'" + keyTo + "'").collect(Collectors.joining(", "));
+        }
+        return String.format(message, args);
+    }
+}
diff --git a/src/main/java/com/configcat/FormattableLogMessageWithUserCondition.java b/src/main/java/com/configcat/FormattableLogMessageWithUserCondition.java
@@ -0,0 +1,18 @@
+package com.configcat;
+
+class FormattableLogMessageWithUserCondition extends FormattableLogMessage {
+
+    FormattableLogMessageWithUserCondition(String message, Object... args) {
+        super(message,args);
+    }
+
+    @Override
+    protected String formatLogMessage() {
+        Object userConditionObject = args[0];
+        if(userConditionObject instanceof UserCondition) {
+            UserCondition userCondition = (UserCondition) userConditionObject;
+            args[0] = EvaluateLogger.formatUserCondition(userCondition);
+        }
+        return String.format(message, args);
+    }
+}
diff --git a/src/main/java/com/configcat/LogFilterFunction.java b/src/main/java/com/configcat/LogFilterFunction.java
@@ -11,9 +11,9 @@ public interface LogFilterFunction {
      *
      * @param logLevel  Event severity level.
      * @param eventId  Event identifier.
-     * @param message Message.
+     * @param message Message object. The formatted message String can be obtained by calling {@code toString}. (It is guaranteed that the message string is built only once even if {@code toString} called multiple times.)
      * @param exception The exception object related to the message (if any).
      * @return True to log the event, false will leave out the log.
      */
-     boolean apply(LogLevel logLevel, int eventId, String message, Throwable exception);
+     boolean apply(LogLevel logLevel, int eventId, Object message, Throwable exception);
 }
diff --git a/src/main/java/com/configcat/RefreshResult.java b/src/main/java/com/configcat/RefreshResult.java
@@ -5,9 +5,9 @@
  */
 public class RefreshResult {
     private final boolean success;
-    private final String error;
+    private final Object error;
 
-    RefreshResult(boolean success, String error) {
+    RefreshResult(boolean success, Object error) {
         this.success = success;
         this.error = error;
     }
@@ -17,6 +17,9 @@ public boolean isSuccess() {
     }
 
     public String error() {
-        return error;
+        if(error !=  null) {
+            return error.toString();
+        }
+        return null;
     }
 }
diff --git a/src/main/java/com/configcat/Result.java b/src/main/java/com/configcat/Result.java
@@ -2,9 +2,9 @@
 
 final class Result<T> {
     private final T value;
-    private final String error;
+    private final Object error;
 
-    private Result(T value, String error) {
+    private Result(T value, Object error) {
         this.value = value;
         this.error = error;
     }
@@ -13,11 +13,11 @@ T value() {
         return this.value;
     }
 
-    String error() {
+    Object error() {
         return this.error;
     }
 
-    static <T> Result<T> error(String error, T value) {
+    static <T> Result<T> error(Object error, T value) {
         return new Result<>(value, error);
     }
 
diff --git a/src/test/java/com/configcat/LoggerTests.java b/src/test/java/com/configcat/LoggerTests.java
@@ -92,7 +92,7 @@ public void noLog() {
     public void excludeLogEvents() {
         Logger mockLogger = mock(Logger.class);
 
-        LogFilterFunction filterLogFunction = ( LogLevel logLevel, int eventId, String message, Throwable exception) -> eventId != 1001 && eventId != 3001 && eventId != 5001;
+        LogFilterFunction filterLogFunction = ( LogLevel logLevel, int eventId, Object message, Throwable exception) -> eventId != 1001 && eventId != 3001 && eventId != 5001 && !message.toString().contains("error");
 
         ConfigCatLogger logger = new ConfigCatLogger(mockLogger, LogLevel.INFO, null, filterLogFunction);
 
@@ -107,7 +107,7 @@ public void excludeLogEvents() {
         verify(mockLogger, never()).debug(anyString(), eq(0), eq("debug"));
         verify(mockLogger, times(1)).info(anyString(), eq(5000), eq("info"));
         verify(mockLogger, times(1)).warn(anyString(), eq(3000), eq("warn"));
-        verify(mockLogger, times(1)).error(anyString(), eq(1000), eq("error"), any(Exception.class));
+        verify(mockLogger, never()).error(anyString(), eq(1000), eq("error"), any(Exception.class));
         verify(mockLogger, never()).info(anyString(), eq(5001), eq("info"));
         verify(mockLogger, never()).warn(anyString(), eq(3001), eq("warn"));
         verify(mockLogger, never()).error(anyString(), eq(1001), eq("error"), any(Exception.class));

Original file line number	Diff line number	Diff line change
`@@ -116,11 +116,14 @@ void invokeOnClientReady(ClientCacheState clientCacheState) {`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`		`- void invokeOnError(String error) {`
	`119`	`+ void invokeOnError(Object error) {`
`120`	`120`	`lock.readLock().lock();`
`121`	`121`	`try {`
`122`		`- for (Consumer<String> func : this.onError) {`
`123`		`- func.accept(error);`
	`122`	`+ if(!this.onError.isEmpty()) {`
	`123`	`+ String errorMessage = error.toString();`
	`124`	`+ for (Consumer<String> func : this.onError) {`
	`125`	`+ func.accept(errorMessage);`
	`126`	`+ }`
`124`	`127`	`}`
`125`	`128`	`} finally {`
`126`	`129`	`lock.readLock().unlock();`
Original file line number	Diff line number	Diff line change
`@@ -23,39 +23,39 @@ public ConfigCatLogger(Logger logger) {`
`23`	`23`	`this(logger, LogLevel.WARNING);`
`24`	`24`	`}`
`25`	`25`
`26`		`- public void warn(int eventId, String message) {`
	`26`	`+ public void warn(int eventId, Object message) {`
`27`	`27`	`if (filter(eventId, LogLevel.WARNING, message, null)) {`
`28`	`28`	`this.logger.warn("[{}] {}", eventId, message);`
`29`	`29`	`}`
`30`	`30`	`}`
`31`	`31`
`32`		`- public void error(int eventId, String message, Exception exception) {`
	`32`	`+ public void error(int eventId, Object message, Exception exception) {`
`33`	`33`	`if (this.configCatHooks != null) this.configCatHooks.invokeOnError(message);`
`34`	`34`	`if (filter(eventId, LogLevel.ERROR, message, exception)) {`
`35`	`35`	`this.logger.error("[{}] {}", eventId, message, exception);`
`36`	`36`	`}`
`37`	`37`	`}`
`38`	`38`
`39`		`- public void error(int eventId, String message) {`
	`39`	`+ public void error(int eventId, Object message) {`
`40`	`40`	`if (this.configCatHooks != null) this.configCatHooks.invokeOnError(message);`
`41`	`41`	`if (filter(eventId, LogLevel.ERROR, message, null)) {`
`42`	`42`	`this.logger.error("[{}] {}", eventId, message);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`
`46`		`- public void info(int eventId, String message) {`
	`46`	`+ public void info(int eventId, Object message) {`
`47`	`47`	`if (filter(eventId, LogLevel.INFO, message, null)) {`
`48`	`48`	`this.logger.info("[{}] {}", eventId, message);`
`49`	`49`	`}`
`50`	`50`	`}`
`51`	`51`
`52`		`- public void debug(String message) {`
	`52`	`+ public void debug(Object message) {`
`53`	`53`	`if (filter(0, LogLevel.DEBUG, message, null)) {`
`54`	`54`	`this.logger.debug("[{}] {}", 0, message);`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`
`58`		`- private boolean filter(int eventId, LogLevel logLevel, String message, Exception exception) {`
	`58`	`+ private boolean filter(int eventId, LogLevel logLevel, Object message, Exception exception) {`
`59`	`59`	`return this.logLevel.ordinal() <= logLevel.ordinal() && (this.filterFunction == null \|\| this.filterFunction.apply(logLevel, eventId, message, exception));`
`60`	`60`	`}`
`61`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ public enum Status {`
`9`	`9`
`10`	`10`	`private final Status status;`
`11`	`11`	`private final Entry entry;`
`12`		`- private final String error;`
	`12`	`+ private final Object error;`
`13`	`13`	`private final boolean fetchTimeUpdatable;`
`14`	`14`
`15`	`15`	`public boolean isFetched() {`
`@@ -32,11 +32,11 @@ public Entry entry() {`
`32`	`32`	`return this.entry;`
`33`	`33`	`}`
`34`	`34`
`35`		`- public String error() {`
`36`		`- return this.error;`
	`35`	`+ public Object error() {`
	`36`	`+ return error;`
`37`	`37`	`}`
`38`	`38`
`39`		`- FetchResponse(Status status, Entry entry, String error, boolean fetchTimeUpdatable) {`
	`39`	`+ FetchResponse(Status status, Entry entry, Object error, boolean fetchTimeUpdatable) {`
`40`	`40`	`this.status = status;`
`41`	`41`	`this.entry = entry;`
`42`	`42`	`this.error = error;`
`@@ -51,7 +51,7 @@ public static FetchResponse notModified() {`
`51`	`51`	`return new FetchResponse(Status.NOT_MODIFIED, Entry.EMPTY, null, true);`
`52`	`52`	`}`
`53`	`53`
`54`		`- public static FetchResponse failed(String error, boolean fetchTimeUpdatable) {`
	`54`	`+ public static FetchResponse failed(Object error, boolean fetchTimeUpdatable) {`
`55`	`55`	`return new FetchResponse(Status.FAILED, Entry.EMPTY, error, fetchTimeUpdatable);`
`56`	`56`	`}`
`57`	`57`	`}`