Exit sputnik with error code if checks fail

This allows scripts/tools to take the return result into account.

Furthermore, Extensions of sputnik like the sputnik-maven-plugin can use
a return value of the Engine to communicate the result of a run and do
something like fail a build.

Implement this by adding a Score object (with the review label and the
score value as fields) and return that in Engine.run().

If a failing score value (< 0) is returned, then call System.exit with
the score as the error status.

src/main/java/pl/touk/sputnik/Main.java

@@ -14,6 +14,7 @@
 import pl.touk.sputnik.connector.ConnectorFacadeFactory;
 import pl.touk.sputnik.connector.ConnectorType;
 import pl.touk.sputnik.engine.Engine;
+import pl.touk.sputnik.engine.score.Score;
 
 public final class Main {
     private static final String SPUTNIK = "sputnik";
@@ -38,7 +39,10 @@ public static void main(String[] args) {
         configuration.updateWithCliOptions(commandLine);
 
         ConnectorFacade facade = getConnectorFacade(configuration);
-        new Engine(facade, facade, configuration).run();
+        Score runResult = new Engine(facade, facade, configuration).run();
+        if (runResult.getScore() < 0) {
+            System.exit(runResult.getScore());
+        }
     }
 
     private static ConnectorFacade getConnectorFacade(Configuration configuration) {

src/main/java/pl/touk/sputnik/connector/gerrit/ReviewInputBuilder.java

@@ -11,6 +11,8 @@
 import pl.touk.sputnik.review.Review;
 import pl.touk.sputnik.review.ReviewFile;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -27,7 +29,9 @@ public class ReviewInputBuilder {
     public ReviewInput toReviewInput(@NotNull Review review, @Nullable String tag) {
         ReviewInput reviewInput = new ReviewInput();
         reviewInput.message = Joiner.on(MESSAGE_SEPARATOR).join(review.getMessages());
-        reviewInput.labels = new HashMap<>(review.getScores());
+        reviewInput.labels = review.getScore() == null
+                ? Collections.emptyMap()
+                : Collections.singletonMap(review.getScore().getLabel(), (short) review.getScore().getScore());
         if (StringUtils.isNotBlank(tag)) {
             reviewInput.tag = tag;
         }

src/main/java/pl/touk/sputnik/engine/Engine.java

@@ -6,6 +6,9 @@
 import pl.touk.sputnik.connector.ReviewPublisher;
 import pl.touk.sputnik.engine.visitor.AfterReviewVisitor;
 import pl.touk.sputnik.engine.visitor.BeforeReviewVisitor;
+import pl.touk.sputnik.engine.score.ScoreStrategy;
+import pl.touk.sputnik.engine.score.ScoreStrategyFactory;
+import pl.touk.sputnik.engine.score.Score;
 import pl.touk.sputnik.review.Review;
 import pl.touk.sputnik.review.ReviewFile;
 import pl.touk.sputnik.review.ReviewFormatterFactory;
@@ -26,7 +29,7 @@ public Engine(ConnectorFacade facade, ReviewPublisher reviewPublisher, Configura
         this.config = configuration;
     }
 
-    public void run() {
+    public Score run() {
         List<ReviewFile> reviewFiles = facade.listFiles();
         Review review = new Review(reviewFiles, ReviewFormatterFactory.get(config));
 
@@ -44,6 +47,11 @@ public void run() {
             afterReviewVisitor.afterReview(review);
         }
 
+        ScoreStrategy scoreStrategy = new ScoreStrategyFactory().buildScoreStrategy(config);
+        Score score = scoreStrategy.score(review);
+        review.setScore(score);
         reviewPublisher.publish(review);
+
+        return score;
     }
 }

src/main/java/pl/touk/sputnik/engine/VisitorBuilder.java

@@ -1,6 +1,5 @@
 package pl.touk.sputnik.engine;
 
-import com.google.common.collect.ImmutableMap;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.math.NumberUtils;
@@ -14,24 +13,12 @@
 import pl.touk.sputnik.engine.visitor.LimitCommentVisitor;
 import pl.touk.sputnik.engine.visitor.RegexFilterFilesVisitor;
 import pl.touk.sputnik.engine.visitor.SummaryMessageVisitor;
-import pl.touk.sputnik.engine.visitor.score.NoScore;
-import pl.touk.sputnik.engine.visitor.score.ScoreAlwaysPass;
-import pl.touk.sputnik.engine.visitor.score.ScorePassIfEmpty;
-import pl.touk.sputnik.engine.visitor.score.ScorePassIfNoErrors;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
-
-import static org.apache.commons.lang3.Validate.notBlank;
 
 @Slf4j
 public class VisitorBuilder {
-    private static final String NOSCORE = "NOSCORE";
-    private static final String SCOREALWAYSPASS = "SCOREALWAYSPASS";
-    private static final String SCOREPASSIFEMPTY = "SCOREPASSIFEMPTY";
-    private static final String SCOREPASSIFNOERRORS = "SCOREPASSIFNOERRORS";
-
     @NotNull
     public List<BeforeReviewVisitor> buildBeforeReviewVisitors(Configuration configuration) {
         List<BeforeReviewVisitor> beforeReviewVisitors = new ArrayList<>();
@@ -65,40 +52,7 @@ public List<AfterReviewVisitor> buildAfterReviewVisitors(Configuration configura
             afterReviewVisitors.add(new LimitCommentVisitor(maxNumberOfComments));
         }
 
-        afterReviewVisitors.add(buildScoreAfterReviewVisitor(configuration));
-
         return afterReviewVisitors;
     }
 
-    @NotNull
-    private AfterReviewVisitor buildScoreAfterReviewVisitor(Configuration configuration) {
-        Map<String, Short> passingScore = ImmutableMap.<String, Short>of(
-                configuration.getProperty(GeneralOption.SCORE_PASSING_KEY),
-                Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE))
-        );
-        Map<String, Short> failingScore = ImmutableMap.<String, Short>of(
-                configuration.getProperty(GeneralOption.SCORE_FAILING_KEY),
-                Short.valueOf(configuration.getProperty(GeneralOption.SCORE_FAILING_VALUE))
-        );
-        String scoreStrategy = configuration.getProperty(GeneralOption.SCORE_STRATEGY);
-        notBlank(scoreStrategy);
-
-        switch(scoreStrategy.toUpperCase()) {
-            case NOSCORE:
-                return new NoScore();
-
-            case SCOREALWAYSPASS:
-                return new ScoreAlwaysPass(passingScore);
-
-            case SCOREPASSIFEMPTY:
-                return new ScorePassIfEmpty(passingScore, failingScore);
-
-            case SCOREPASSIFNOERRORS:
-                return new ScorePassIfNoErrors(passingScore, failingScore);
-
-            default:
-                log.warn("Score strategy {} not found, using default ScoreAlwaysPass", scoreStrategy);
-                return new ScoreAlwaysPass(passingScore);
-        }
-    }
 }

src/main/java/pl/touk/sputnik/engine/score/NoScore.java

@@ -0,0 +1,17 @@
+package pl.touk.sputnik.engine.score;
+
+import lombok.EqualsAndHashCode;
+import lombok.extern.slf4j.Slf4j;
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.review.Review;
+
+@Slf4j
+@EqualsAndHashCode
+public class NoScore implements ScoreStrategy {
+
+    @Override
+    public Score score(@NotNull Review review) {
+        log.info("No score for review");
+        return null;
+    }
+}

src/main/java/pl/touk/sputnik/engine/score/Score.java

@@ -0,0 +1,15 @@
+package pl.touk.sputnik.engine.score;
+
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+
+@AllArgsConstructor
+@Getter
+@EqualsAndHashCode
+@ToString
+public class Score {
+    private final String label;
+    private final int score;
+}

src/main/java/pl/touk/sputnik/engine/score/ScoreAlwaysPass.java

@@ -0,0 +1,22 @@
+package pl.touk.sputnik.engine.score;
+
+import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.review.Review;
+
+@Slf4j
+@Getter
+@EqualsAndHashCode
+@AllArgsConstructor
+public class ScoreAlwaysPass implements ScoreStrategy {
+    private final Score passingScore;
+
+    @Override
+    public Score score(@NotNull Review review) {
+        log.info("Adding static passing score {} to review", passingScore);
+        return passingScore;
+    }
+}

src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfEmpty.java → src/main/java/pl/touk/sputnik/engine/score/ScorePassIfEmpty.java

@@ -1,30 +1,28 @@
-package pl.touk.sputnik.engine.visitor.score;
+package pl.touk.sputnik.engine.score;
 
 import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.jetbrains.annotations.NotNull;
-import pl.touk.sputnik.engine.visitor.AfterReviewVisitor;
 import pl.touk.sputnik.review.Review;
 
-import java.util.Map;
-
 @Slf4j
 @Getter
+@EqualsAndHashCode
 @AllArgsConstructor
-public class ScorePassIfEmpty implements AfterReviewVisitor {
-    private final Map<String, Short> passingScore;
-    private final Map<String, Short> failingScore;
+public class ScorePassIfEmpty implements ScoreStrategy {
+    private final Score passingScore;
+    private final Score failingScore;
 
     @Override
-    public void afterReview(@NotNull Review review) {
+    public Score score(@NotNull Review review) {
         if (review.getTotalViolationCount() == 0) {
             log.info("Adding passing score {} for no violation(s) found", passingScore);
-            review.setScores(passingScore);
-            return;
+            return passingScore;
         }
 
         log.info("Adding failing score {} for {} violations found", failingScore, review.getTotalViolationCount());
-        review.setScores(failingScore);
+        return failingScore;
     }
 }

src/main/java/pl/touk/sputnik/engine/visitor/score/ScorePassIfNoErrors.java → src/main/java/pl/touk/sputnik/engine/score/ScorePassIfNoErrors.java

@@ -1,32 +1,30 @@
-package pl.touk.sputnik.engine.visitor.score;
+package pl.touk.sputnik.engine.score;
 
 import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.jetbrains.annotations.NotNull;
-import pl.touk.sputnik.engine.visitor.AfterReviewVisitor;
 import pl.touk.sputnik.review.Review;
 import pl.touk.sputnik.review.Severity;
 
-import java.util.Map;
-
 @Slf4j
 @Getter
+@EqualsAndHashCode
 @AllArgsConstructor
-public class ScorePassIfNoErrors implements AfterReviewVisitor {
-    private final Map<String, Short> passingScore;
-    private final Map<String, Short> failingScore;
+public class ScorePassIfNoErrors implements ScoreStrategy {
+    private final Score passingScore;
+    private final Score failingScore;
 
     @Override
-    public void afterReview(@NotNull Review review) {
+    public Score score(@NotNull Review review) {
         Integer errorCount = review.getViolationCount().get(Severity.ERROR);
         if (errorCount == null || errorCount == 0) {
             log.info("Adding passing score {} for no errors found", passingScore);
-            review.setScores(passingScore);
-            return;
+            return passingScore;
         }
 
         log.info("Adding failing score {} for {} errors found", failingScore, errorCount);
-        review.setScores(failingScore);
+        return failingScore;
     }
 }

src/main/java/pl/touk/sputnik/engine/score/ScoreStrategy.java

@@ -0,0 +1,8 @@
+package pl.touk.sputnik.engine.score;
+
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.review.Review;
+
+public interface ScoreStrategy {
+    Score score(@NotNull Review review);
+}

src/main/java/pl/touk/sputnik/engine/score/ScoreStrategyFactory.java

@@ -0,0 +1,48 @@
+package pl.touk.sputnik.engine.score;
+
+import lombok.extern.slf4j.Slf4j;
+import org.jetbrains.annotations.NotNull;
+import pl.touk.sputnik.configuration.Configuration;
+import pl.touk.sputnik.configuration.GeneralOption;
+
+import static org.apache.commons.lang3.Validate.notBlank;
+
+@Slf4j
+public class ScoreStrategyFactory {
+    private static final String NOSCORE = "NOSCORE";
+    private static final String SCOREALWAYSPASS = "SCOREALWAYSPASS";
+    private static final String SCOREPASSIFEMPTY = "SCOREPASSIFEMPTY";
+    private static final String SCOREPASSIFNOERRORS = "SCOREPASSIFNOERRORS";
+
+    @NotNull
+    public ScoreStrategy buildScoreStrategy(Configuration configuration) {
+        Score passingScore = new Score(
+                configuration.getProperty(GeneralOption.SCORE_PASSING_KEY),
+                Short.valueOf(configuration.getProperty(GeneralOption.SCORE_PASSING_VALUE))
+        );
+        Score failingScore = new Score(
+                configuration.getProperty(GeneralOption.SCORE_FAILING_KEY),
+                Short.valueOf(configuration.getProperty(GeneralOption.SCORE_FAILING_VALUE))
+        );
+        String scoreStrategy = configuration.getProperty(GeneralOption.SCORE_STRATEGY);
+        notBlank(scoreStrategy);
+
+        switch(scoreStrategy.toUpperCase()) {
+            case NOSCORE:
+                return new NoScore();
+
+            case SCOREALWAYSPASS:
+                return new ScoreAlwaysPass(passingScore);
+
+            case SCOREPASSIFEMPTY:
+                return new ScorePassIfEmpty(passingScore, failingScore);
+
+            case SCOREPASSIFNOERRORS:
+                return new ScorePassIfNoErrors(passingScore, failingScore);
+
+            default:
+                log.warn("Score strategy {} not found, using default ScoreAlwaysPass", scoreStrategy);
+                return new ScoreAlwaysPass(passingScore);
+        }
+    }
+}