From befe9ffd64cbe0fc9e298a5385ce59ccc8bdae9a Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Tue, 10 Jun 2025 13:15:11 +0530 Subject: [PATCH] fix: improve null safety in action execution analytics data collection (#40905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fix NPE in ActionExecutionSolutionCEImpl by replacing `Map.of()` with explicit HashMap entries. This change improves error resilience by adding null checks with default values for all map entries and makes the code more maintainable by separating each property assignment into individual operations. Fixes #40904 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 48335f468fb5104ff911f277d121817202ea7545 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Tue, 10 Jun 2025 07:43:44 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Refactor** - Improved handling of missing or null information in analytics event data to ensure more consistent and reliable reporting. No changes to user-facing features or workflows. --- .../ce/ActionExecutionSolutionCEImpl.java | 94 ++++++++----------- 1 file changed, 40 insertions(+), 54 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java index c6e5c8ffce..4a51d65d06 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java @@ -1127,27 +1127,21 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE ? ApplicationMode.PUBLISHED.toString() : ApplicationMode.EDIT.toString(); - final Map data = new HashMap<>(Map.of( - "username", - user.getUsername(), - "type", - pluginType, - "pluginName", - plugin.getName(), - "name", - actionDTO.getName(), - "datasource", - Map.of("name", datasourceStorage.getName()), - "workspaceId", - application.getWorkspaceId(), - "appId", - actionDTO.getApplicationId(), - FieldName.APP_MODE, - appMode, - "appName", - application.getName(), - "isExampleApp", - application.isAppIsExample())); + final Map data = new HashMap<>(); + data.put("username", user.getUsername()); + data.put("type", pluginType); + data.put("pluginName", plugin.getName()); + data.put("name", actionDTO.getName()); + + Map datasourceInfo = new HashMap<>(); + datasourceInfo.put("name", datasourceStorage.getName()); + data.put("datasource", datasourceInfo); + + data.put("workspaceId", application.getWorkspaceId()); + data.put("appId", actionDTO.getApplicationId()); + data.put(FieldName.APP_MODE, appMode); + data.put("appName", application.getName()); + data.put("isExampleApp", application.isAppIsExample()); String dsCreatedAt = ""; if (datasourceStorage.getCreatedAt() != null) { @@ -1160,37 +1154,28 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE List executionParams = paramsList.stream().map(param -> param.getValue()).collect(Collectors.toList()); - data.putAll(Map.of( - "request", - request, + data.put("request", request); + data.put( "isSuccessfulExecution", - ObjectUtils.defaultIfNull(actionExecutionResult.getIsExecutionSuccess(), false), - "statusCode", - ObjectUtils.defaultIfNull(actionExecutionResult.getStatusCode(), ""), - "timeElapsed", - timeElapsed, - "actionCreated", - DateUtils.ISO_FORMATTER.format(actionDTO.getCreatedAt()), - "actionId", - ObjectUtils.defaultIfNull(actionDTO.getId(), ""))); - data.putAll(Map.of( + ObjectUtils.defaultIfNull(actionExecutionResult.getIsExecutionSuccess(), false)); + data.put("statusCode", ObjectUtils.defaultIfNull(actionExecutionResult.getStatusCode(), "")); + data.put("timeElapsed", timeElapsed); + data.put("actionCreated", DateUtils.ISO_FORMATTER.format(actionDTO.getCreatedAt())); + data.put("actionId", ObjectUtils.defaultIfNull(actionDTO.getId(), "")); + data.put( FieldName.ACTION_EXECUTION_REQUEST_PARAMS_SIZE, - executeActionDto.getTotalReadableByteCount(), - FieldName.ACTION_EXECUTION_REQUEST_PARAMS_COUNT, - executionParams.size())); + executeActionDto.getTotalReadableByteCount()); + data.put(FieldName.ACTION_EXECUTION_REQUEST_PARAMS_COUNT, executionParams.size()); setContextSpecificProperties(data, actionDTO, pageName); ActionExecutionResult.PluginErrorDetails pluginErrorDetails = actionExecutionResult.getPluginErrorDetails(); - - data.putAll(Map.of("pluginErrorDetails", ObjectUtils.defaultIfNull(pluginErrorDetails, ""))); - + data.put("pluginErrorDetails", ObjectUtils.defaultIfNull(pluginErrorDetails, "")); if (pluginErrorDetails != null) { - data.putAll(Map.of( - "appsmithErrorCode", pluginErrorDetails.getAppsmithErrorCode(), - "appsmithErrorMessage", pluginErrorDetails.getAppsmithErrorMessage(), - "errorType", pluginErrorDetails.getErrorType())); + data.put("appsmithErrorCode", pluginErrorDetails.getAppsmithErrorCode()); + data.put("appsmithErrorMessage", pluginErrorDetails.getAppsmithErrorMessage()); + data.put("errorType", pluginErrorDetails.getErrorType()); } data.putAll(DatasourceAnalyticsUtils.getAnalyticsPropertiesWithStorageOnActionExecution( @@ -1219,15 +1204,15 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE actionExecutionResult.getRequest().getQuery(); } - final Map eventData = new HashMap<>(Map.of( - FieldName.ACTION, actionDTO, - FieldName.DATASOURCE, datasourceStorage, - FieldName.APP_MODE, appMode, - FieldName.ACTION_EXECUTION_RESULT, actionExecutionResult, - FieldName.ACTION_EXECUTION_TIME, timeElapsed, - FieldName.ACTION_EXECUTION_QUERY, executionRequestQuery, - FieldName.APPLICATION, application, - FieldName.PLUGIN, plugin)); + final Map eventData = new HashMap<>(); + eventData.put(FieldName.ACTION, actionDTO); + eventData.put(FieldName.DATASOURCE, datasourceStorage); + eventData.put(FieldName.APP_MODE, appMode); + eventData.put(FieldName.ACTION_EXECUTION_RESULT, actionExecutionResult); + eventData.put(FieldName.ACTION_EXECUTION_TIME, timeElapsed); + eventData.put(FieldName.ACTION_EXECUTION_QUERY, executionRequestQuery); + eventData.put(FieldName.APPLICATION, application); + eventData.put(FieldName.PLUGIN, plugin); if (executeActionDto.getTotalReadableByteCount() <= Constraint.MAX_ANALYTICS_SIZE_BYTES) { // Only send params info if total size is less than 5 MB @@ -1259,7 +1244,8 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE } protected void setContextSpecificProperties(Map data, ActionDTO actionDTO, String contextName) { - data.putAll(Map.of("pageId", ObjectUtils.defaultIfNull(actionDTO.getPageId(), ""), "pageName", contextName)); + data.put("pageId", ObjectUtils.defaultIfNull(actionDTO.getPageId(), "")); + data.put("pageName", ObjectUtils.defaultIfNull(contextName, "")); } protected Mono setAutoGeneratedHeaders(Plugin plugin, ActionDTO actionDTO, HttpHeaders httpHeaders) {