SONAR-8460 Remove creation BadRequestException with Errors

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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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<String> 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
@@ -53,14 +53,6 @@ public class BadRequestExceptionTest {
   }
 
   @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<String> 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);
         });