From aba5115cdba10e52bb2e576364cbf24c32331d2e Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 29 Aug 2013 18:25:58 +0200 Subject: [PATCH] Fix quality flaws --- .../issue/InternalRubyIssueService.java | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 41696ec744f..84914cf392f 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -65,7 +65,12 @@ public class InternalRubyIssueService implements ServerComponent { private static final String ID_PARAM = "id"; private static final String NAME_PARAM = "name"; private static final String DESCRIPTION_PARAM = "description"; - private static final String ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST = "action_plans.errors.action_plan_does_not_exist"; + private static final String PROJECT_PARAM = "project"; + private static final String USER_PARAM = "user"; + + private static final String ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST_MESSAGE = "action_plans.errors.action_plan_does_not_exist"; + private static final String ERRORS_CANT_BE_EMPTY_MESSAGE = "errors.cant_be_empty"; + private static final String ERRORS_IS_TOO_LONG_MESSAGE = "errors.is_too_long"; private final IssueService issueService; private final IssueCommentService commentService; @@ -249,7 +254,7 @@ public class InternalRubyIssueService implements ServerComponent { DefaultActionPlan existingActionPlan = (DefaultActionPlan) actionPlanService.findByKey(key, UserSession.get()); if (existingActionPlan == null) { Result result = Result.of(); - result.addError(Result.Message.ofL10n(ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST, key)); + result.addError(Result.Message.ofL10n(ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST_MESSAGE, key)); return result; } else { Result result = createActionPlanResult(parameters, existingActionPlan); @@ -299,7 +304,7 @@ public class InternalRubyIssueService implements ServerComponent { String name = parameters.get(NAME_PARAM); String description = parameters.get(DESCRIPTION_PARAM); String deadLineParam = parameters.get("deadLine"); - String projectParam = parameters.get("project"); + String projectParam = parameters.get(PROJECT_PARAM); checkMandatorySizeParameter(name, NAME_PARAM, 200, result); checkOptionalSizeParameter(description, DESCRIPTION_PARAM, 1000, result); @@ -310,8 +315,8 @@ public class InternalRubyIssueService implements ServerComponent { } Date deadLine = checkAndReturnDeadline(deadLineParam, result); - if (!Strings.isNullOrEmpty(projectParam) && !Strings.isNullOrEmpty(name) && (existingActionPlan == null || !name.equals(existingActionPlan.name())) - && actionPlanService.isNameAlreadyUsedForProject(name, projectParam)) { + // TODO move this check in the service, on creation and update + if (!Strings.isNullOrEmpty(projectParam) && !Strings.isNullOrEmpty(name) && isActionPlanNameAvailable(existingActionPlan, name, projectParam)) { result.addError(Result.Message.ofL10n("action_plans.same_name_in_same_project")); } @@ -333,9 +338,13 @@ public class InternalRubyIssueService implements ServerComponent { return result; } + private boolean isActionPlanNameAvailable(@Nullable DefaultActionPlan existingActionPlan, String name, String projectParam){ + return (existingActionPlan == null || !name.equals(existingActionPlan.name())) && actionPlanService.isNameAlreadyUsedForProject(name, projectParam); + } + private void checkProject(String projectParam, Result result) { if (Strings.isNullOrEmpty(projectParam)) { - result.addError(Result.Message.ofL10n("errors.cant_be_empty", "project")); + result.addError(Result.Message.ofL10n(ERRORS_CANT_BE_EMPTY_MESSAGE, PROJECT_PARAM)); } else { ResourceDto project = resourceDao.getResource(ResourceQuery.create().setKey(projectParam)); if (project == null) { @@ -363,7 +372,7 @@ public class InternalRubyIssueService implements ServerComponent { private Result createResultForExistingActionPlan(String actionPlanKey) { Result result = Result.of(); if (findActionPlan(actionPlanKey) == null) { - result.addError(Result.Message.ofL10n(ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST, actionPlanKey)); + result.addError(Result.Message.ofL10n(ACTION_PLANS_ERRORS_ACTION_PLAN_DOES_NOT_EXIST_MESSAGE, actionPlanKey)); } return result; } @@ -530,7 +539,7 @@ public class InternalRubyIssueService implements ServerComponent { String name = params.get(NAME_PARAM); String description = params.get(DESCRIPTION_PARAM); String data = params.get("data"); - String user = params.get("user"); + String user = params.get(USER_PARAM); Boolean sharedParam = RubyUtils.toBoolean(params.get("shared")); boolean shared = sharedParam != null ? sharedParam : false; @@ -538,7 +547,7 @@ public class InternalRubyIssueService implements ServerComponent { checkMandatoryParameter(id, ID_PARAM); } if (checkUser) { - checkMandatoryParameter(user, "user"); + checkMandatoryParameter(user, USER_PARAM); } checkMandatorySizeParameter(name, NAME_PARAM, 100); checkOptionalSizeParameter(description, DESCRIPTION_PARAM, 4000); @@ -576,39 +585,39 @@ public class InternalRubyIssueService implements ServerComponent { private void checkMandatoryParameter(String value, String paramName, Result result) { if (Strings.isNullOrEmpty(value)) { - result.addError(Result.Message.ofL10n("errors.cant_be_empty", paramName)); + result.addError(Result.Message.ofL10n(ERRORS_CANT_BE_EMPTY_MESSAGE, paramName)); } } private void checkMandatorySizeParameter(String value, String paramName, Integer size, Result result) { checkMandatoryParameter(value, paramName, result); if (!Strings.isNullOrEmpty(value) && value.length() > size) { - result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size)); + result.addError(Result.Message.ofL10n(ERRORS_IS_TOO_LONG_MESSAGE, paramName, size)); } } private void checkOptionalSizeParameter(String value, String paramName, Integer size, Result result) { if (!Strings.isNullOrEmpty(value) && value.length() > size) { - result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size)); + result.addError(Result.Message.ofL10n(ERRORS_IS_TOO_LONG_MESSAGE, paramName, size)); } } private void checkMandatoryParameter(String value, String paramName) { if (Strings.isNullOrEmpty(value)) { - throw BadRequestException.ofL10n("errors.cant_be_empty", paramName); + throw BadRequestException.ofL10n(ERRORS_CANT_BE_EMPTY_MESSAGE, paramName); } } private void checkMandatorySizeParameter(String value, String paramName, Integer size) { checkMandatoryParameter(value, paramName); if (!Strings.isNullOrEmpty(value) && value.length() > size) { - throw BadRequestException.ofL10n("errors.is_too_long", paramName, size); + throw BadRequestException.ofL10n(ERRORS_IS_TOO_LONG_MESSAGE, paramName, size); } } private void checkOptionalSizeParameter(String value, String paramName, Integer size) { if (!Strings.isNullOrEmpty(value) && value.length() > size) { - throw BadRequestException.ofL10n("errors.is_too_long", paramName, size); + throw BadRequestException.ofL10n(ERRORS_IS_TOO_LONG_MESSAGE, paramName, size); } } -- 2.39.5