From d86584addefaf46be866776fb0a2441ef5f0a255 Mon Sep 17 00:00:00 2001 From: ImalshaD Date: Thu, 4 Jun 2026 00:03:12 +0530 Subject: [PATCH] Address review comments. --- .../core/ServerConfigManagementService.java | 39 ++++++------------- .../v1/function/CompatibilitySettingUtil.java | 26 +++++++++---- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java b/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java index dc0b6540d2..086013ee35 100644 --- a/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java +++ b/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/core/ServerConfigManagementService.java @@ -2516,8 +2516,8 @@ public CompatibilitySettings getCompatibilitySettings() { CompatibilitySetting setting = compatibilitySettingsService.getCompatibilitySettings(tenantDomain); return CompatibilitySettingUtil.toCompatibilitySettings(setting); } catch (CompatibilitySettingException e) { - throw handleCompatibilitySettingsError(e, Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_RETRIEVE, - null); + throw CompatibilitySettingUtil.handleCompatibilitySettingsException(e, + Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_RETRIEVE, null); } } @@ -2536,8 +2536,8 @@ public CompatibilitySettings patchCompatibilitySettings(CompatibilitySettings co compatibilitySettingsService.updateCompatibilitySettings(tenantDomain, setting); return CompatibilitySettingUtil.toCompatibilitySettings(updatedSetting); } catch (CompatibilitySettingException e) { - throw handleCompatibilitySettingsError(e, Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_UPDATE, - null); + throw CompatibilitySettingUtil.handleCompatibilitySettingsException(e, + Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_UPDATE, null); } } @@ -2550,11 +2550,10 @@ public CompatibilitySettings patchCompatibilitySettings(CompatibilitySettings co public CompatibilitySettingsGroup getCompatibilitySettingsByGroup(String settingGroup) { if (!CompatibilitySettingUtil.isValidSettingGroupName(settingGroup)) { - throw handleCompatibilitySettingsError( - new IllegalArgumentException("Invalid setting group name: " + settingGroup), + throw CompatibilitySettingUtil.handleCompatibilitySettingsException( + Response.Status.BAD_REQUEST, Constants.ErrorMessage.ERROR_CODE_INVALID_INPUT, - settingGroup - ); + settingGroup); } try { @@ -2563,30 +2562,16 @@ public CompatibilitySettingsGroup getCompatibilitySettingsByGroup(String setting compatibilitySettingsService.getCompatibilitySettingsByGroup(tenantDomain, settingGroup); if (!CompatibilitySettingUtil.hasSettingGroup(setting, settingGroup)) { - throw handleCompatibilitySettingsError( - new IllegalArgumentException("Setting group not found: " + settingGroup), + throw CompatibilitySettingUtil.handleCompatibilitySettingsException( + Response.Status.NOT_FOUND, Constants.ErrorMessage.ERROR_CODE_SETTING_GROUP_NOT_FOUND, - settingGroup - ); + settingGroup); } return CompatibilitySettingUtil.toCompatibilitySettingsGroup(setting, settingGroup); } catch (CompatibilitySettingException e) { - throw handleCompatibilitySettingsError(e, Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_RETRIEVE, - settingGroup); + throw CompatibilitySettingUtil.handleCompatibilitySettingsException(e, + Constants.ErrorMessage.ERROR_CODE_COMPATIBILITY_SETTINGS_RETRIEVE, settingGroup); } } - - /** - * Handle compatibility settings errors. - * - * @param e Exception. - * @param errorEnum Error enum. - * @param data Additional data. - * @return APIError. - */ - private APIError handleCompatibilitySettingsError(Exception e, Constants.ErrorMessage errorEnum, String data) { - - return CompatibilitySettingUtil.handleCompatibilitySettingsException(e, errorEnum, data); - } } diff --git a/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/CompatibilitySettingUtil.java b/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/CompatibilitySettingUtil.java index 79836daf0b..f5127d5d42 100644 --- a/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/CompatibilitySettingUtil.java +++ b/components/org.wso2.carbon.identity.api.server.configs/org.wso2.carbon.identity.api.server.configs.v1/src/main/java/org/wso2/carbon/identity/api/server/configs/v1/function/CompatibilitySettingUtil.java @@ -199,6 +199,24 @@ public static APIError handleCompatibilitySettingsException(Exception e, Constan return new APIError(status, errorResponse); } + /** + * Build an APIError for compatibility settings flows where the HTTP status is known up-front + * (e.g., NOT_FOUND for a missing setting group or BAD_REQUEST for invalid input) and there is no + * underlying exception to extract details from. + * + * @param status HTTP response status. + * @param errorEnum Error Message enum. + * @param data Extra data. + * @return APIError. + */ + public static APIError handleCompatibilitySettingsException(Response.Status status, + Constants.ErrorMessage errorEnum, + String data) { + + ErrorResponse errorResponse = getErrorBuilder(errorEnum, data).build(LOG, errorEnum.description()); + return new APIError(status, errorResponse); + } + /** * Return error builder with given error message enum and data. * @@ -223,12 +241,6 @@ private static ErrorResponse.Builder getErrorBuilder(Constants.ErrorMessage erro */ private static String includeData(Constants.ErrorMessage error, String data) { - String message; - if (StringUtils.isNotBlank(data)) { - message = String.format(error.description(), data); - } else { - message = error.description(); - } - return message; + return StringUtils.isNotBlank(data) ? String.format(error.description(), data) : error.description(); } }