From 1a57c8a8888a521b9a6fc7bda8625ed5c66fc344 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 1 Mar 2017 12:29:02 +0100 Subject: [PATCH] SONAR-8460 Remove creation BadRequestException with Errors --- .../exceptions/BadRequestException.java | 8 +--- .../QualityGateConditionsUpdater.java | 46 +++++++++---------- .../qualitygate/QualityGateUpdater.java | 19 ++++---- .../server/qualitygate/QualityGates.java | 19 ++++---- .../exceptions/BadRequestExceptionTest.java | 8 ---- .../sonar/server/ws/WebServiceEngineTest.java | 8 ++-- 6 files changed, 45 insertions(+), 63 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java b/server/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java index 6212ad56571..e6d4ae4b302 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java +++ b/server/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java @@ -40,18 +40,14 @@ public class BadRequestException extends ServerException { } public static BadRequestException create(List errorMessages) { - return create(new Errors().add(errorMessages.stream().map(Message::of).collect(Collectors.toList()))); + checkArgument(!errorMessages.isEmpty(), "At least one error message is required"); + return new BadRequestException(new Errors().add(errorMessages.stream().map(Message::of).collect(Collectors.toList()))); } public static BadRequestException create(String... errorMessages) { return create(asList(errorMessages)); } - public static BadRequestException create(Errors e) { - checkArgument(!e.messages().isEmpty(), "At least one error message is required"); - return new BadRequestException(e); - } - public Errors errors() { return errors; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java index c1e3f9753d0..57680c81cd4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java @@ -19,6 +19,7 @@ */ package org.sonar.server.qualitygate; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Objects; @@ -34,9 +35,6 @@ import org.sonar.db.metric.MetricDto; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDto; import org.sonar.server.computation.task.projectanalysis.qualitymodel.RatingGrid.Rating; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.Errors; -import org.sonar.server.exceptions.Message; import org.sonar.server.exceptions.NotFoundException; import static com.google.common.base.Strings.isNullOrEmpty; @@ -130,19 +128,17 @@ public class QualityGateConditionsUpdater { } private static void validateCondition(MetricDto metric, String operator, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period) { - Errors errors = new Errors(); + List errors = new ArrayList<>(); validateMetric(metric, errors); checkOperator(metric, operator, errors); checkThresholds(warningThreshold, errorThreshold, errors); checkPeriod(metric, period, errors); checkRatingMetric(metric, warningThreshold, errorThreshold, period, errors); - if (!errors.messages().isEmpty()) { - throw BadRequestException.create(errors); - } + checkRequest(errors.isEmpty(), errors); } - private static void validateMetric(MetricDto metric, Errors errors) { - check(errors, isAlertable(metric), format("Metric '%s' cannot be used to define a condition.", metric.getKey())); + private static void validateMetric(MetricDto metric, List errors) { + check(isAlertable(metric), errors, "Metric '%s' cannot be used to define a condition.", metric.getKey()); } private static boolean isAlertable(MetricDto metric) { @@ -153,20 +149,20 @@ public class QualityGateConditionsUpdater { return !metric.isDataType() && !CoreMetrics.ALERT_STATUS_KEY.equals(metric.getKey()); } - private static void checkOperator(MetricDto metric, String operator, Errors errors) { + private static void checkOperator(MetricDto metric, String operator, List errors) { ValueType valueType = valueOf(metric.getValueType()); - check(errors, isOperatorAllowed(operator, valueType), format("Operator %s is not allowed for metric type %s.", operator, metric.getValueType())); + check(isOperatorAllowed(operator, valueType), errors, "Operator %s is not allowed for metric type %s.", operator, metric.getValueType()); } - private static void checkThresholds(@Nullable String warningThreshold, @Nullable String errorThreshold, Errors errors) { - check(errors, warningThreshold != null || errorThreshold != null, "At least one threshold (warning, error) must be set."); + private static void checkThresholds(@Nullable String warningThreshold, @Nullable String errorThreshold, List errors) { + check(warningThreshold != null || errorThreshold != null, errors, "At least one threshold (warning, error) must be set."); } - private static void checkPeriod(MetricDto metric, @Nullable Integer period, Errors errors) { + private static void checkPeriod(MetricDto metric, @Nullable Integer period, List errors) { if (period == null) { - check(errors, !metric.getKey().startsWith("new_"), "A period must be selected for differential metrics."); + check(!metric.getKey().startsWith("new_"), errors, "A period must be selected for differential metrics."); } else { - check(errors, period == 1, "The only valid quality gate period is 1, the leak period."); + check(period == 1, errors, "The only valid quality gate period is 1, the leak period."); } } @@ -181,15 +177,15 @@ public class QualityGateConditionsUpdater { : format("Condition on metric '%s' over leak period already exists.", metric.getShortName())); } - private static void checkRatingMetric(MetricDto metric, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period, Errors errors) { + private static void checkRatingMetric(MetricDto metric, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period, List errors) { if (!metric.getValueType().equals(RATING.name())) { return; } if (!isCoreRatingMetric(metric.getKey())) { - errors.add(Message.of(format("The metric '%s' cannot be used", metric.getShortName()))); + errors.add(format("The metric '%s' cannot be used", metric.getShortName())); } if (period != null && !metric.getKey().startsWith("new_")) { - errors.add(Message.of(format("The metric '%s' cannot be used on the leak period", metric.getShortName()))); + errors.add(format("The metric '%s' cannot be used on the leak period", metric.getShortName())); } if (!isValidRating(warningThreshold)) { addInvalidRatingError(warningThreshold, errors); @@ -203,12 +199,12 @@ public class QualityGateConditionsUpdater { checkRatingGreaterThanOperator(errorThreshold, errors); } - private static void addInvalidRatingError(@Nullable String value, Errors errors) { - errors.add(Message.of(format("'%s' is not a valid rating", value))); + private static void addInvalidRatingError(@Nullable String value, List errors) { + errors.add(format("'%s' is not a valid rating", value)); } - private static void checkRatingGreaterThanOperator(@Nullable String value, Errors errors) { - check(errors, isNullOrEmpty(value) || !Objects.equals(toRating(value), E), format("There's no worse rating than E (%s)", value)); + private static void checkRatingGreaterThanOperator(@Nullable String value, List errors) { + check(isNullOrEmpty(value) || !Objects.equals(toRating(value), E), errors, "There's no worse rating than E (%s)", value); } private static Rating toRating(String value) { @@ -219,9 +215,9 @@ public class QualityGateConditionsUpdater { return isNullOrEmpty(value) || RATING_VALID_INT_VALUES.contains(value); } - private static boolean check(Errors errors, boolean expression, String message) { + private static boolean check(boolean expression, List errors, String message, String... args) { if (!expression) { - errors.add(Message.of(message)); + errors.add(format(message, args)); } return expression; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateUpdater.java index 7afedf22db0..43beb798dc3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateUpdater.java @@ -19,16 +19,17 @@ */ package org.sonar.server.qualitygate; +import java.util.ArrayList; +import java.util.List; import javax.annotation.Nullable; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.Errors; -import org.sonar.server.exceptions.Message; import org.sonar.server.util.Validation; import static com.google.common.base.Strings.isNullOrEmpty; +import static java.lang.String.format; +import static org.sonar.server.ws.WsUtils.checkRequest; public class QualityGateUpdater { @@ -46,22 +47,20 @@ public class QualityGateUpdater { } private void validateQualityGate(@Nullable Long qGateId, @Nullable String name) { - Errors errors = new Errors(); + List errors = new ArrayList<>(); if (isNullOrEmpty(name)) { - errors.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Name")); + errors.add(format(Validation.CANT_BE_EMPTY_MESSAGE, "Name")); } else { checkQualityGateDoesNotAlreadyExist(qGateId, name, errors); } - if (!errors.messages().isEmpty()) { - throw BadRequestException.create(errors); - } + checkRequest(errors.isEmpty(), errors); } - private void checkQualityGateDoesNotAlreadyExist(@Nullable Long qGateId, String name, Errors errors) { + private void checkQualityGateDoesNotAlreadyExist(@Nullable Long qGateId, String name, List errors) { QualityGateDto existingQgate = dbClient.qualityGateDao().selectByName(name); boolean isModifyingCurrentQgate = qGateId != null && existingQgate != null && existingQgate.getId().equals(qGateId); if (!isModifyingCurrentQgate && existingQgate != null) { - errors.add(Message.of(Validation.IS_ALREADY_USED_MESSAGE, "Name")); + errors.add(format(Validation.IS_ALREADY_USED_MESSAGE, "Name")); } } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java index 2c508e65da1..f6eb82bb0a7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java @@ -20,7 +20,9 @@ package org.sonar.server.qualitygate; import com.google.common.base.Strings; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; @@ -37,15 +39,14 @@ import org.sonar.db.qualitygate.QualityGateConditionDao; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDao; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.Errors; -import org.sonar.server.exceptions.Message; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.UserSession; import org.sonar.server.util.Validation; +import static java.lang.String.format; import static org.sonar.core.permission.GlobalPermissions.QUALITY_GATE_ADMIN; import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException; +import static org.sonar.server.ws.WsUtils.checkRequest; /** * Methods from this class should be moved to {@link QualityGateUpdater} and to new classes QualityGateFinder / QualityGateConditionsUpdater / etc. @@ -226,22 +227,20 @@ public class QualityGates { } private void validateQualityGate(@Nullable Long updatingQgateId, @Nullable String name) { - Errors errors = new Errors(); + List errors = new ArrayList<>(); if (Strings.isNullOrEmpty(name)) { - errors.add(Message.of(Validation.CANT_BE_EMPTY_MESSAGE, "Name")); + errors.add(format(Validation.CANT_BE_EMPTY_MESSAGE, "Name")); } else { checkQgateNotAlreadyExists(updatingQgateId, name, errors); } - if (!errors.messages().isEmpty()) { - throw BadRequestException.create(errors); - } + checkRequest(errors.isEmpty(), errors); } - private void checkQgateNotAlreadyExists(@Nullable Long updatingQgateId, String name, Errors errors) { + private void checkQgateNotAlreadyExists(@Nullable Long updatingQgateId, String name, List errors) { QualityGateDto existingQgate = dao.selectByName(name); boolean isModifyingCurrentQgate = updatingQgateId != null && existingQgate != null && existingQgate.getId().equals(updatingQgateId); if (!isModifyingCurrentQgate && existingQgate != null) { - errors.add(Message.of(Validation.IS_ALREADY_USED_MESSAGE, "Name")); + errors.add(format(Validation.IS_ALREADY_USED_MESSAGE, "Name")); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java b/server/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java index 920cb09d2a2..307e6c84815 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java @@ -52,14 +52,6 @@ public class BadRequestExceptionTest { assertThat(underTest.errors().messages().stream().map(Message::getMessage)).containsOnly("error1", "error2"); } - @Test - public void create_exception_from_errors() throws Exception { - Errors errors = new Errors().add(Message.of("error1"), Message.of("error2")); - BadRequestException underTest = BadRequestException.create(errors); - - assertThat(underTest.errors().messages().stream().map(Message::getMessage)).containsOnly("error1", "error2"); - } - @Test public void getMessage_return_first_error() throws Exception { BadRequestException underTest = BadRequestException.create(asList("error1", "error2")); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index 32c931f262b..5da0f2430c1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -20,6 +20,8 @@ package org.sonar.server.ws; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.MissingFormatArgumentException; import javax.servlet.http.HttpServletResponse; import org.apache.catalina.connector.ClientAbortException; @@ -35,8 +37,6 @@ import org.sonar.api.server.ws.internal.ValidatingRequest; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.Errors; -import org.sonar.server.exceptions.Message; import org.sonarqube.ws.MediaTypes; import static java.lang.String.format; @@ -350,9 +350,9 @@ public class WebServiceEngineTest { createNewDefaultAction(newController, "fail_with_multiple_messages") .createParam("count", "Number of error messages to generate") .setHandler((request, response) -> { - Errors errors = new Errors(); + List errors = new ArrayList<>(); for (int count = 0; count < Integer.valueOf(request.param("count")); count++) { - errors.add(Message.of("Bad request reason #" + count)); + errors.add("Bad request reason #" + count); } throw BadRequestException.create(errors); }); -- 2.39.5