MergeUtil#newMerger: Throw a specific exception for invalid strategy

Instead of checking for a valid strategy with checkArgument, which
will raise IllegalArgumentException, throw a specific exception. Add
the new InvalidMergeStrategyException for this purpose.

Derive InvalidMergeStrategyException from RuntimeException which
results in the same behavior as previously. Deriving from a checked
exception type would require further refactoring; this can be done
in a follow-up commit if necessary.

Change-Id: I604d374fab553bf471332cd5b52e0224e464397c

java/com/google/gerrit/exceptions/InvalidMergeStrategyException.java

@@ -0,0 +1,23 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.exceptions;
+
+public class InvalidMergeStrategyException extends RuntimeException {
+  private static final long serialVersionUID = 1L;
+
+  public InvalidMergeStrategyException(String strategy) {
+    super("invalid merge strategy: " + strategy);
+  }
+}

java/com/google/gerrit/server/git/MergeUtil.java

@@ -40,6 +40,7 @@ import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.LabelId;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.extensions.registration.DynamicSet;
@@ -265,7 +266,8 @@ public class MergeUtil {
       boolean ignoreIdenticalTree,
       boolean allowConflicts)
       throws MissingObjectException, IncorrectObjectTypeException, IOException,
-          MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException {
+          MergeIdenticalTreeException, MergeConflictException, MethodNotAllowedException,
+          InvalidMergeStrategyException {
 
     ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
     m.setBase(originalCommit.getParent(parentIndex));
@@ -431,7 +433,8 @@ public class MergeUtil {
       PersonIdent committerIndent,
       String commitMsg,
       RevWalk rw)
-      throws IOException, MergeIdenticalTreeException, MergeConflictException {
+      throws IOException, MergeIdenticalTreeException, MergeConflictException,
+          InvalidMergeStrategyException {
 
     if (!MergeStrategy.THEIRS.getName().equals(mergeStrategy)
         && rw.isMergedInto(originalCommit, mergeTip)) {
@@ -745,7 +748,7 @@ public class MergeUtil {
       BranchNameKey destBranch,
       CodeReviewCommit mergeTip,
       CodeReviewCommit n)
-      throws IntegrationException {
+      throws IntegrationException, InvalidMergeStrategyException {
     ThreeWayMerger m = newThreeWayMerger(inserter, repoConfig);
     try {
       if (m.merge(mergeTip, n)) {
@@ -866,7 +869,8 @@ public class MergeUtil {
         .collect(joining(",", "Merge changes ", merged.size() > 5 ? ", ..." : ""));
   }
 
-  public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig) {
+  public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig)
+      throws InvalidMergeStrategyException {
     return newThreeWayMerger(inserter, repoConfig, mergeStrategyName());
   }
 
@@ -890,7 +894,8 @@ public class MergeUtil {
   }
 
   public static ThreeWayMerger newThreeWayMerger(
-      ObjectInserter inserter, Config repoConfig, String strategyName) {
+      ObjectInserter inserter, Config repoConfig, String strategyName)
+      throws InvalidMergeStrategyException {
     Merger m = newMerger(inserter, repoConfig, strategyName);
     checkArgument(
         m instanceof ThreeWayMerger,
@@ -899,9 +904,12 @@ public class MergeUtil {
     return (ThreeWayMerger) m;
   }
 
-  public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName) {
+  public static Merger newMerger(ObjectInserter inserter, Config repoConfig, String strategyName)
+      throws InvalidMergeStrategyException {
     MergeStrategy strategy = MergeStrategy.get(strategyName);
-    checkArgument(strategy != null, "invalid merge strategy: %s", strategyName);
+    if (strategy == null) {
+      throw new InvalidMergeStrategyException(strategyName);
+    }
     return strategy.newMerger(
         new ObjectInserter.Filter() {
           @Override

java/com/google/gerrit/server/restapi/change/CreateChange.java

@@ -27,6 +27,7 @@ import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.SubmitType;
@@ -339,8 +340,8 @@ public class CreateChange
         bu.execute();
       }
       return ins.getChange();
-    } catch (IllegalArgumentException e) {
-      throw new BadRequestException(e.getMessage(), e);
+    } catch (InvalidMergeStrategyException e) {
+      throw new BadRequestException(e.getMessage());
     }
   }
 

java/com/google/gerrit/server/restapi/project/CheckMergeability.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.restapi.project;
 
+import com.google.gerrit.exceptions.InvalidMergeStrategyException;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.common.MergeableInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -120,7 +121,7 @@ public class CheckMergeability implements RestReadView<BranchResource> {
           result.conflicts = ((ResolveMerger) m).getUnmergedPaths();
         }
       }
-    } catch (IllegalArgumentException e) {
+    } catch (InvalidMergeStrategyException e) {
       throw new BadRequestException(e.getMessage());
     }
     return Response.ok(result);