Replace String concatenation with Log4j ParameterizedMessage for read…

…ability (#943) * Replace strings in GetWorkflowTransportAction.java Signed-off-by: Patrik Cmil <[email protected]> * Replace strings in 5 files Signed-off-by: Patrik Cmil <[email protected]> * Replace strings in 10 files Signed-off-by: Patrik Cmil <[email protected]> * Replace strings in 40 files Signed-off-by: Patrik Cmil <[email protected]> * Changed back strings, which were not be parametrized Signed-off-by: Patrik Cmil <[email protected]> * Update CHANGELOG.md Signed-off-by: Patrik Cmil <[email protected]> * Add EOF line break Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Patrik Cmil <[email protected]> Signed-off-by: Daniel Widdis <[email protected]> Co-authored-by: Daniel Widdis <[email protected]>
opensearch-project · Nov 11, 2024 · 35b171e · 35b171e
1 parent 219aec5
commit 35b171e
Show file tree

Hide file tree

Showing 26 changed files with 297 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,3 +24,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 ### Documentation
 ### Maintenance
 ### Refactoring
+- Replace String concatenation with Log4j ParameterizedMessage for readability ([#943](https://github.com/opensearch-project/flow-framework/pull/943))
diff --git a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContentObject;
@@ -171,7 +172,10 @@ public static WorkflowNode parse(XContentParser parser) throws IOException {
                                         String configurationsString = ParseUtils.parseArbitraryStringToObjectMapToString(configurationsMap);
                                         userInputs.put(inputFieldName, configurationsString);
                                     } catch (Exception ex) {
-                                        String errorMessage = "Failed to parse" + inputFieldName + "map";
+                                        String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                            "Failed to parse {} map",
+                                            inputFieldName
+                                        ).getFormattedMessage();
                                         logger.error(errorMessage, ex);
                                         throw new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
                                     }

diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.get.GetRequest;
 import org.opensearch.action.search.SearchRequest;
@@ -201,7 +202,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
             try {
                 validateWorkflows(templateWithUser);
             } catch (Exception e) {
-                String errorMessage = "Workflow validation failed for template " + templateWithUser.name();
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                    "Workflow validation failed for template {}",
+                    templateWithUser.name()
+                ).getFormattedMessage();
                 logger.error(errorMessage, e);
                 listener.onFailure(
                     e instanceof FlowFrameworkException ? e : new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e))
@@ -218,7 +222,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                 flowFrameworkSettings.getMaxWorkflows(),
                 ActionListener.wrap(max -> {
                     if (FALSE.equals(max)) {
-                        String errorMessage = "Maximum workflows limit reached: " + flowFrameworkSettings.getMaxWorkflows();
+                        String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                            "Maximum workflows limit reached: {}",
+                            flowFrameworkSettings.getMaxWorkflows()
+                        ).getFormattedMessage();
                         logger.error(errorMessage);
                         FlowFrameworkException ffe = new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
                         listener.onFailure(ffe);
@@ -307,7 +314,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                         }));
                     }
                 }, exception -> {
-                    String errorMessage = "Failed to update use case template " + request.getWorkflowId();
+                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                        "Failed to update use case template {}",
+                        request.getWorkflowId()
+                    ).getFormattedMessage();
                     logger.error(errorMessage, exception);
                     if (exception instanceof FlowFrameworkException) {
                         listener.onFailure(exception);
@@ -350,7 +360,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                                 ActionListener.wrap(reprovisionResponse -> {
                                     listener.onResponse(new WorkflowResponse(reprovisionResponse.getWorkflowId()));
                                 }, exception -> {
-                                    String errorMessage = "Reprovisioning failed for workflow " + workflowId;
+                                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                        "Reprovisioning failed for workflow {}",
+                                        workflowId
+                                    ).getFormattedMessage();
                                     logger.error(errorMessage, exception);
                                     if (exception instanceof FlowFrameworkException) {
                                         listener.onFailure(exception);
@@ -382,9 +395,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                                                 );
                                                 listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
                                             }, exception -> {
-                                                String errorMessage = "Failed to update workflow "
-                                                    + request.getWorkflowId()
-                                                    + " in template index";
+                                                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                                    "Failed to update workflow {} in template index",
+                                                    request.getWorkflowId()
+                                                ).getFormattedMessage();
                                                 logger.error(errorMessage, exception);
                                                 if (exception instanceof FlowFrameworkException) {
                                                     listener.onFailure(exception);
@@ -399,7 +413,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                                         listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
                                     }
                                 }, exception -> {
-                                    String errorMessage = "Failed to update use case template " + request.getWorkflowId();
+                                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                        "Failed to update use case template {}",
+                                        request.getWorkflowId()
+                                    ).getFormattedMessage();
                                     logger.error(errorMessage, exception);
                                     if (exception instanceof FlowFrameworkException) {
                                         listener.onFailure(exception);
@@ -411,12 +428,18 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
                             );
                         }
                     } else {
-                        String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+                        String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                            "Failed to retrieve template ({}) from global context.",
+                            workflowId
+                        ).getFormattedMessage();
                         logger.error(errorMessage);
                         listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
                     }
                 }, exception -> {
-                    String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                        "Failed to retrieve template ({}) from global context.",
+                        workflowId
+                    ).getFormattedMessage();
                     logger.error(errorMessage, exception);
                     listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
                 }));

diff --git a/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.support.ActionFilters;
@@ -166,7 +167,10 @@ private void executeDeprovisionRequest(
                     )
                 );
         }, exception -> {
-            String errorMessage = "Failed to get workflow state for workflow " + workflowId;
+            String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                "Failed to get workflow state for workflow {}",
+                workflowId
+            ).getFormattedMessage();
             logger.error(errorMessage, exception);
             listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
         }));

diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.get.GetRequest;
 import org.opensearch.action.support.ActionFilters;
@@ -96,7 +97,8 @@ protected void doExecute(Task task, GetWorkflowStateRequest request, ActionListe
             );
 
         } catch (Exception e) {
-            String errorMessage = "Failed to get workflow: " + workflowId;
+            String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to get workflow: {}", workflowId)
+                .getFormattedMessage();
             logger.error(errorMessage, e);
             listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
         }
@@ -123,7 +125,8 @@ private void executeGetWorkflowStateRequest(
                     WorkflowState workflowState = WorkflowState.parse(parser);
                     listener.onResponse(new GetWorkflowStateResponse(workflowState, request.getAll()));
                 } catch (Exception e) {
-                    String errorMessage = "Failed to parse workflowState: " + r.getId();
+                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to parse workflowState: {}", r.getId())
+                        .getFormattedMessage();
                     logger.error(errorMessage, e);
                     listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST));
                 }
@@ -134,7 +137,8 @@ private void executeGetWorkflowStateRequest(
             if (e instanceof IndexNotFoundException) {
                 listener.onFailure(new FlowFrameworkException("Fail to find workflow status of " + workflowId, RestStatus.NOT_FOUND));
             } else {
-                String errorMessage = "Failed to get workflow status of: " + workflowId;
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to get workflow status of: {}", workflowId)
+                    .getFormattedMessage();
                 logger.error(errorMessage, e);
                 listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
             }

diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.get.GetRequest;
 import org.opensearch.action.support.ActionFilters;
@@ -104,7 +105,10 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<GetW
                     xContentRegistry
                 );
             } catch (Exception e) {
-                String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                    "Failed to retrieve template ({}) from global context.",
+                    workflowId
+                ).getFormattedMessage();
                 logger.error(errorMessage, e);
                 listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
             }
@@ -134,7 +138,10 @@ private void executeGetRequest(
             context.restore();
 
             if (!response.isExists()) {
-                String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                    "Failed to retrieve template ({}) from global context.",
+                    workflowId
+                ).getFormattedMessage();
                 logger.error(errorMessage);
                 listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
             } else {
@@ -144,7 +151,10 @@ private void executeGetRequest(
                 listener.onResponse(new GetWorkflowResponse(template));
             }
         }, exception -> {
-            String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+            String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                "Failed to retrieve template ({}) from global context.",
+                workflowId
+            ).getFormattedMessage();
             logger.error(errorMessage, exception);
             listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
         }));

diff --git a/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
@@ -10,6 +10,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.get.GetRequest;
 import org.opensearch.action.support.ActionFilters;
@@ -138,7 +139,10 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work
                 xContentRegistry
             );
         } catch (Exception e) {
-            String errorMessage = "Failed to retrieve template from global context for workflow " + workflowId;
+            String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                "Failed to retrieve template from global context for workflow {}",
+                workflowId
+            ).getFormattedMessage();
             logger.error(errorMessage, e);
             listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
         }
@@ -169,7 +173,10 @@ private void executeProvisionRequest(
             context.restore();
 
             if (!response.isExists()) {
-                String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                    "Failed to retrieve template ({}) from global context.",
+                    workflowId
+                ).getFormattedMessage();
                 logger.error(errorMessage);
                 listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
                 return;
@@ -212,7 +219,10 @@ private void executeProvisionRequest(
                                 ActionListener.wrap(templateResponse -> {
                                     listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
                                 }, exception -> {
-                                    String errorMessage = "Failed to update use case template " + request.getWorkflowId();
+                                    String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                        "Failed to update use case template {}",
+                                        request.getWorkflowId()
+                                    ).getFormattedMessage();
                                     logger.error(errorMessage, exception);
                                     if (exception instanceof FlowFrameworkException) {
                                         listener.onFailure(exception);
@@ -224,7 +234,10 @@ private void executeProvisionRequest(
                                 true
                             );
                         }, exception -> {
-                            String errorMessage = "Failed to update workflow state: " + workflowId;
+                            String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                                "Failed to update workflow state: {}",
+                                workflowId
+                            ).getFormattedMessage();
                             logger.error(errorMessage, exception);
                             listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
                         })
@@ -244,7 +257,10 @@ private void executeProvisionRequest(
                 logger.error("Workflow validation failed for workflow {}", workflowId);
                 listener.onFailure(exception);
             } else {
-                String errorMessage = "Failed to retrieve template from global context for workflow " + workflowId;
+                String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
+                    "Failed to retrieve template from global context for workflow {}",
+                    workflowId
+                ).getFormattedMessage();
                 logger.error(errorMessage, exception);
                 listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
             }