SONAR-8460 Remove Errors

server/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java

@@ -21,7 +21,6 @@ package org.sonar.server.exceptions;
 
 import com.google.common.base.MoreObjects;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
@@ -32,23 +31,24 @@ import static java.util.Arrays.asList;
  */
 public class BadRequestException extends ServerException {
 
-  private final transient Errors errors;
+  private final transient List<String> errors;
 
-  private BadRequestException(Errors e) {
-    super(HTTP_BAD_REQUEST, e.messages().get(0).getMessage());
-    this.errors = e;
+  private BadRequestException(List<String> errors) {
+    super(HTTP_BAD_REQUEST, errors.get(0));
+    this.errors = errors;
   }
 
   public static BadRequestException create(List<String> errorMessages) {
     checkArgument(!errorMessages.isEmpty(), "At least one error message is required");
-    return new BadRequestException(new Errors().add(errorMessages.stream().map(Message::of).collect(Collectors.toList())));
+    checkArgument(errorMessages.stream().noneMatch(message -> message == null || message.isEmpty()), "Message cannot be empty");
+    return new BadRequestException(errorMessages);
   }
 
   public static BadRequestException create(String... errorMessages) {
     return create(asList(errorMessages));
   }
 
-  public Errors errors() {
+  public List<String> errors() {
     return errors;
   }
 

server/sonar-server/src/main/java/org/sonar/server/exceptions/Errors.java

@@ -1,63 +0,0 @@
-/*
- * SonarQube
- * Copyright (C) 2009-2017 SonarSource SA
- * mailto:info AT sonarsource DOT com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-package org.sonar.server.exceptions;
-
-import com.google.common.base.MoreObjects;
-import com.google.common.collect.Lists;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-
-public class Errors {
-
-  private final List<Message> messages = Lists.newArrayList();
-
-  public Errors() {
-  }
-
-  public Errors add(Errors errors) {
-    this.messages.addAll(errors.messages());
-    return this;
-  }
-
-  public Errors add(Collection<Message> messages) {
-    for (Message message : messages) {
-      add(message);
-    }
-    return this;
-  }
-
-  public Errors add(Message message, Message... others) {
-    messages.add(message);
-    Collections.addAll(messages, others);
-    return this;
-  }
-
-  public List<Message> messages() {
-    return messages;
-  }
-
-  @Override
-  public String toString() {
-    return MoreObjects.toStringHelper(this)
-      .add("messages", messages)
-      .toString();
-  }
-}

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java

@@ -20,19 +20,17 @@
 package org.sonar.server.qualityprofile;
 
 import com.google.common.collect.Lists;
-import org.sonar.db.qualityprofile.QualityProfileDto;
-import org.sonar.server.exceptions.Errors;
-
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
-
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+import org.sonar.db.qualityprofile.QualityProfileDto;
 
 public class BulkChangeResult {
 
   private final QualityProfileDto profile;
-  private final Errors errors = new Errors();
+  private final List<String> errors = new ArrayList<>();
   private int succeeded = 0;
   private int failed = 0;
   private final List<ActiveRuleChange> changes = Lists.newArrayList();
@@ -50,7 +48,7 @@ public class BulkChangeResult {
     return profile;
   }
 
-  public Errors getErrors() {
+  public List<String> getErrors() {
     return errors;
   }
 

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java

@@ -141,7 +141,7 @@ public class QProfileReset {
         result.addChanges(changes);
       } catch (BadRequestException e) {
         result.incrementFailed();
-        result.getErrors().add(e.errors());
+        result.getErrors().addAll(e.errors());
       }
     }
 

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java

@@ -422,7 +422,7 @@ public class RuleActivator {
         } catch (BadRequestException e) {
           // other exceptions stop the bulk activation
           result.incrementFailed();
-          result.getErrors().add(e.errors());
+          result.getErrors().addAll(e.errors());
         }
       }
       dbSession.commit();
@@ -450,7 +450,7 @@ public class RuleActivator {
         } catch (BadRequestException e) {
           // other exceptions stop the bulk activation
           result.incrementFailed();
-          result.getErrors().add(e.errors());
+          result.getErrors().addAll(e.errors());
         }
       }
       dbSession.commit();

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java

@@ -26,8 +26,6 @@ import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.text.JsonWriter;
-import org.sonar.core.util.stream.Collectors;
-import org.sonar.server.exceptions.Message;
 import org.sonar.server.qualityprofile.BulkChangeResult;
 import org.sonar.server.qualityprofile.QProfileService;
 import org.sonar.server.rule.ws.RuleQueryFactory;
@@ -110,7 +108,7 @@ public class BulkRuleActivationActions {
     JsonWriter json = response.newJsonWriter().beginObject();
     json.prop("succeeded", result.countSucceeded());
     json.prop("failed", result.countFailed());
-    writeErrors(json, result.getErrors().messages().stream().map(Message::getMessage).collect(Collectors.toList()));
+    writeErrors(json, result.getErrors());
     json.endObject().close();
   }
 

server/sonar-server/src/main/java/org/sonar/server/rule/RuleCreator.java

@@ -32,14 +32,12 @@ import org.sonar.api.rule.Severity;
 import org.sonar.api.server.ServerSide;
 import org.sonar.api.server.rule.RuleParamType;
 import org.sonar.api.utils.System2;
-import org.sonar.core.util.stream.Collectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleDto.Format;
 import org.sonar.db.rule.RuleParamDto;
 import org.sonar.server.exceptions.BadRequestException;
-import org.sonar.server.exceptions.Message;
 import org.sonar.server.rule.index.RuleIndexer;
 import org.sonar.server.util.TypeValidations;
 
@@ -117,7 +115,7 @@ public class RuleCreator {
       try {
         validateParam(ruleParam, newRule.parameter(ruleParam.getName()));
       } catch (BadRequestException validationError) {
-        errors.addAll(validationError.errors().messages().stream().map(Message::getMessage).collect(Collectors.toList()));
+        errors.addAll(validationError.errors());
       }
     }
     checkRequest(errors.isEmpty(), errors);

server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java

@@ -36,15 +36,13 @@ import org.sonar.api.server.ws.internal.ValidatingRequest;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.api.utils.text.JsonWriter;
-import org.sonar.core.util.stream.Collectors;
 import org.sonar.server.exceptions.BadRequestException;
-import org.sonar.server.exceptions.Errors;
-import org.sonar.server.exceptions.Message;
 import org.sonar.server.exceptions.ServerException;
 import org.sonarqube.ws.MediaTypes;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.util.Collections.singletonList;
 import static org.apache.commons.lang.StringUtils.substring;
 import static org.apache.commons.lang.StringUtils.substringAfterLast;
 import static org.apache.commons.lang.StringUtils.substringBeforeLast;
@@ -104,11 +102,11 @@ public class WebServiceEngine implements LocalConnector, Startable {
       verifyRequest(action, request);
       action.handler().handle(request, response);
     } catch (IllegalArgumentException e) {
-      sendErrors(response, 400, new Errors().add(Message.of(e.getMessage())));
+      sendErrors(response, 400, singletonList(e.getMessage()));
     } catch (BadRequestException e) {
       sendErrors(response, 400, e.errors());
     } catch (ServerException e) {
-      sendErrors(response, e.httpCode(), new Errors().add(Message.of(e.getMessage())));
+      sendErrors(response, e.httpCode(), singletonList(e.getMessage()));
     } catch (Exception e) {
       Response.Stream stream = response.stream();
       if (stream instanceof ServletResponse.ServletStream && ((ServletResponse.ServletStream) stream).response().isCommitted()) {
@@ -119,7 +117,7 @@ public class WebServiceEngine implements LocalConnector, Startable {
       LOGGER.error("Fail to process request " + request, e);
       // Sending exception message into response is a vulnerability. Error must be
       // displayed only in logs.
-      sendErrors(response, 500, new Errors().add(Message.of("An error has occurred. Please contact your administrator")));
+      sendErrors(response, 500, singletonList("An error has occurred. Please contact your administrator"));
     }
   }
 
@@ -131,7 +129,7 @@ public class WebServiceEngine implements LocalConnector, Startable {
     return controller == null ? null : controller.action(actionKey);
   }
 
-  private static void sendErrors(Response response, int status, Errors errors) {
+  private static void sendErrors(Response response, int status, List<String> errors) {
     Response.Stream stream = response.stream();
     if (stream instanceof ServletResponse.ServletStream) {
       ((ServletResponse.ServletStream) stream).reset();
@@ -141,7 +139,7 @@ public class WebServiceEngine implements LocalConnector, Startable {
 
     try (JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8))) {
       json.beginObject();
-      writeErrors(json, errors.messages().stream().map(Message::getMessage).collect(Collectors.toList()));
+      writeErrors(json, errors);
       json.endObject();
     } catch (Exception e) {
       // Do not hide the potential exception raised in the try block.

server/sonar-server/src/test/java/org/sonar/server/email/ws/SendActionTest.java

@@ -27,7 +27,6 @@ import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
-import org.sonar.server.exceptions.Message;
 import org.sonar.server.notification.email.EmailNotificationChannel;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
@@ -111,7 +110,7 @@ public class SendActionTest {
       executeRequest("john@doo.com", "Test Message from SonarQube", "This is a test message from SonarQube at http://localhost:9000");
       fail();
     } catch (BadRequestException e) {
-      assertThat(e.errors().messages()).extracting(Message::getMessage).containsExactly(
+      assertThat(e.errors()).containsExactly(
         "root cause", "parent cause", "child cause", "last message");
     }
   }

server/sonar-server/src/test/java/org/sonar/server/exceptions/BadRequestExceptionTest.java

@@ -42,14 +42,14 @@ public class BadRequestExceptionTest {
   public void create_exception_from_list() throws Exception {
     BadRequestException underTest = BadRequestException.create(asList("error1", "error2"));
 
-    assertThat(underTest.errors().messages().stream().map(Message::getMessage)).containsOnly("error1", "error2");
+    assertThat(underTest.errors()).containsOnly("error1", "error2");
   }
 
   @Test
   public void create_exception_from_var_args() throws Exception {
     BadRequestException underTest = BadRequestException.create("error1", "error2");
 
-    assertThat(underTest.errors().messages().stream().map(Message::getMessage)).containsOnly("error1", "error2");
+    assertThat(underTest.errors()).containsOnly("error1", "error2");
   }
 
   @Test

server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java

@@ -43,7 +43,6 @@ import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleParamDto;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.exceptions.BadRequestException;
-import org.sonar.server.exceptions.Message;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndex;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
 import org.sonar.server.rule.index.RuleIndex;
@@ -495,8 +494,7 @@ public class RuleActivatorMediumTest {
       activate(activation, XOO_P1_KEY);
       fail();
     } catch (BadRequestException e) {
-      Message msg = e.errors().messages().get(0);
-      assertThat(msg.getMessage()).isEqualTo("Value 'foo' must be an integer.");
+      assertThat(e.getMessage()).isEqualTo("Value 'foo' must be an integer.");
       verifyZeroActiveRules(XOO_P1_KEY);
     }
   }

server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java

@@ -357,7 +357,7 @@ public class UserUpdaterTest {
         .build());
       fail();
     } catch (BadRequestException e) {
-      assertThat(e.errors().messages()).hasSize(3);
+      assertThat(e.errors()).hasSize(3);
     }
   }
 

server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java

@@ -22,7 +22,6 @@ 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;
 import org.apache.commons.io.IOUtils;
@@ -39,7 +38,6 @@ import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonarqube.ws.MediaTypes;
 
-import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -285,13 +283,15 @@ public class WebServiceEngineTest {
   }
 
   @Test
-  public void render_real_exception_when_failing_to_write_json_errors() {
-    ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/fail_to_write_errors");
+  public void does_not_fail_to_render_error_message_having_percent() {
+    ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/error_message_having_percent");
     DumbResponse response = new DumbResponse();
 
-    expectedException.expect(MissingFormatArgumentException.class);
-    expectedException.expectMessage("Format specifier '%s'");
     underTest.execute(request, response);
+
+    assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"this should not fail %s\"}]}");
+    assertThat(response.stream().status()).isEqualTo(400);
+    assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON);
   }
 
   @Test
@@ -356,10 +356,9 @@ public class WebServiceEngineTest {
           }
           throw BadRequestException.create(errors);
         });
-      createNewDefaultAction(newController, "fail_to_write_errors")
+      createNewDefaultAction(newController, "error_message_having_percent")
         .setHandler((request, response) -> {
-          // Try to simulate an error when generating JSON errors
-          format("this will fail as no args are given %s");
+          throw new IllegalArgumentException("this should not fail %s");
         });
       createNewDefaultAction(newController, "alive")
         .setHandler((request, response) -> response.noContent());