Refactoring: Moved refName validation method to core for better re-use

Also:
- use IStatus result to avoid null and improve readability
- use stream API for efficiency
- use Message.format instead of NLS.bind

Change-Id: I9118b18b6c0fce5ce1e381d8088061181b384eba
Signed-off-by: Max Hohenegger <eclipse@hohenegger.eu>

9 files changed, 135 insertions, 77 deletions

diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
index c1de1cf76a..979dfbcd60 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
@@ -502,6 +502,24 @@ public class CoreText extends NLS {
 	public static String Utils_InvalidAdapterError;
 
 	/** */
+	public static String ValidationUtils_CanNotResolveRefMessage;
+
+	/** */
+	public static String ValidationUtils_InvalidRefNameMessage;
+
+	/** */
+	public static String ValidationUtils_InvalidRevision;
+
+	/** */
+	public static String ValidationUtils_RefAlreadyExistsMessage;
+
+	/** */
+	public static String ValidationUtils_RefNameConflictsWithExistingMessage;
+
+	/** */
+	public static String ValidationUtils_PleaseEnterNameMessage;
+
+	/** */
 	public static String ReportingTypedConfigGetter_invalidConfig;
 
 	/** */
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/Utils.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/Utils.java
index 249b703000..4dc203d124 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/Utils.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/Utils.java
@@ -10,11 +10,21 @@
  *******************************************************************************/
 package org.eclipse.egit.core.internal;
 
+import static java.util.stream.Collectors.joining;
+import static org.eclipse.core.runtime.Status.OK_STATUS;
+
+import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.Collection;
 
 import org.eclipse.core.runtime.IAdaptable;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.egit.core.Activator;
+import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.errors.RevisionSyntaxException;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.Repository;
 
 /**
  * Utility class
@@ -114,4 +124,70 @@ public class Utils {
 			return null;
 		}
 	}
+
+	/**
+	 * Validates a given ref name, including testing whether a ref with that
+	 * name already exists, or if the name conflicts with an already existing
+	 * ref.
+	 *
+	 * @param refNameInput
+	 *            Short ref name.
+	 * @param repo
+	 *            Repository {@code refNameInput} should be tested against.
+	 * @param refPrefix
+	 *            The ref namespace (refs/heads/, refs/tags, ...). Must end with
+	 *            a slash. If empty, {@code refNameInput} must be a full ref
+	 *            name.
+	 * @param errorOnEmptyName
+	 *            Whether or not to treat an empty {@code refNameInput} as
+	 *            erroneous.
+	 *
+	 * @return {@link org.eclipse.core.runtime.Status#OK_STATUS} in case of
+	 *         successful validation, or an error status with respective error
+	 *         message.
+	 */
+	@NonNull
+	public static IStatus validateNewRefName(String refNameInput,
+			@NonNull Repository repo, @NonNull String refPrefix,
+			final boolean errorOnEmptyName) {
+		if (refNameInput == null || refNameInput.isEmpty()) {
+			if (errorOnEmptyName) {
+				return Activator.error(
+						CoreText.ValidationUtils_PleaseEnterNameMessage, null);
+			} else {
+				// ignore this
+				return OK_STATUS;
+			}
+		}
+		String testFor = refPrefix + refNameInput;
+		if (!Repository.isValidRefName(testFor)) {
+			return Activator.error(MessageFormat.format(
+					CoreText.ValidationUtils_InvalidRefNameMessage, testFor),
+					null);
+		}
+		try {
+			if (repo.resolve(testFor) != null) {
+				return Activator.error(MessageFormat.format(
+						CoreText.ValidationUtils_RefAlreadyExistsMessage,
+						testFor), null);
+			}
+			RefDatabase refDatabase = repo.getRefDatabase();
+			Collection<String> conflictingNames = refDatabase
+					.getConflictingNames(testFor);
+			if (!conflictingNames.isEmpty()) {
+				String joined = conflictingNames.stream().sorted()
+						.collect(joining(", ")); //$NON-NLS-1$
+				return Activator.error(MessageFormat.format(
+						CoreText.ValidationUtils_RefNameConflictsWithExistingMessage,
+						joined), null);
+			}
+		} catch (IOException e) {
+			return Activator.error(e.getMessage(), e);
+		} catch (RevisionSyntaxException e) {
+			String m = MessageFormat
+					.format(CoreText.ValidationUtils_InvalidRevision, testFor);
+			return Activator.error(m, e);
+		}
+		return OK_STATUS;
+	}
 }
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
index f20cfc4c28..eef9d2f6fa 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
@@ -205,6 +205,13 @@ SubmoduleUpdateOperation_cloning=Updating submodules in {0}: cloning submodule {
 
 Utils_InvalidAdapterError={0}.getAdapter({1}.class) returned an object of type {2}, which is not of the requested type. Please ask the provider of {0} to fix this bug in their code.
 
+ValidationUtils_CanNotResolveRefMessage=Cannot resolve {0}
+ValidationUtils_InvalidRefNameMessage={0} is not a valid name for a ref
+ValidationUtils_InvalidRevision=Invalid revision {0}
+ValidationUtils_RefAlreadyExistsMessage=Ref {0} already exists
+ValidationUtils_RefNameConflictsWithExistingMessage=Name conflicts with existing refs: {0}
+ValidationUtils_PleaseEnterNameMessage=Please enter a ref name
+
 ReportingTypedConfigGetter_invalidConfig=Git config value ''{0}'' is invalid; using default value ''{1}''
 ReportingTypedConfigGetter_invalidConfigIgnored=Ignored invalid git config value ''{0}''
 ReportingTypedConfigGetter_invalidConfigWithLocation=Git config ''{0}'': value ''{1}'' is invalid; using default value ''{2}''
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
index a267d25c79..7fb8331010 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
@@ -25,6 +25,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.eclipse.core.commands.State;
 import org.eclipse.egit.core.RepositoryUtil;
+import org.eclipse.egit.core.internal.CoreText;
 import org.eclipse.egit.core.op.BranchOperation;
 import org.eclipse.egit.core.op.CloneOperation;
 import org.eclipse.egit.ui.Activator;
@@ -322,7 +323,7 @@ public class GitRepositoriesViewBranchHandlingTest extends
 		newBranchNameText.setText("invalid~name");
 
 		renameDialog.bot().text(" " + // the text is now in the error message, and the MessageAreaDialog seems to add a space
-				NLS.bind(UIText.ValidationUtils_InvalidRefNameMessage,
+				NLS.bind(CoreText.ValidationUtils_InvalidRefNameMessage,
 						"refs/heads/invalid~name"));
 		assertFalse(renameDialog.bot()
 				.button(UIText.RenameBranchDialog_RenameButtonLabel)
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
index fdc28cf748..2b752393d7 100755
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
@@ -191,9 +191,25 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener
 	public static void handleIssue(int severity, String message, Throwable throwable,
 			boolean show) {
 		IStatus status = toStatus(severity, message, throwable);
+		handleStatus(status, show);
+	}
+
+	/**
+	 * Handle a status. The status is logged. If <code>show</code> is
+	 * <code>true</code> the issue is shown to the user.
+	 *
+	 * @param status
+	 *            Status to be handled.
+	 *
+	 * @param show
+	 *            Whether or not to show the issue to the user.
+	 * @since 5.2
+	 */
+	public static void handleStatus(IStatus status, boolean show) {
 		int style = StatusManager.LOG;
-		if (show)
+		if (show) {
 			style |= StatusManager.SHOW;
+		}
 		StatusManager.getManager().handle(status, style);
 	}
 
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
index 92e1d791f4..a5682044b1 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
@@ -5162,24 +5162,6 @@ public class UIText extends NLS {
 	public static String SwitchToMenu_OtherMenuLabel;
 
 	/** */
-	public static String ValidationUtils_CanNotResolveRefMessage;
-
-	/** */
-	public static String ValidationUtils_InvalidRefNameMessage;
-
-	/** */
-	public static String ValidationUtils_InvalidRevision;
-
-	/** */
-	public static String ValidationUtils_RefAlreadyExistsMessage;
-
-	/** */
-	public static String ValidationUtils_RefNameConflictsWithExistingMessage;
-
-	/** */
-	public static String ValidationUtils_PleaseEnterNameMessage;
-
-	/** */
 	public static String GitMergeEditorInput_CalculatingDiffTaskName;
 
 	/** */
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/ValidationUtils.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/ValidationUtils.java
index 13246982ee..807dfaebd2 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/ValidationUtils.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/ValidationUtils.java
@@ -11,24 +11,18 @@
 
 package org.eclipse.egit.ui.internal;
 
-import java.io.IOException;
-import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
+import static org.eclipse.egit.core.internal.Utils.validateNewRefName;
 
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.egit.ui.Activator;
 import org.eclipse.jface.dialogs.IInputValidator;
-import org.eclipse.jgit.errors.RevisionSyntaxException;
-import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.util.StringUtils;
-import org.eclipse.osgi.util.NLS;
 
 /**
  * A collection of validators
  */
 public class ValidationUtils {
+
 	/**
 	 * Creates and returns input validator for refNames
 	 *
@@ -38,50 +32,20 @@ public class ValidationUtils {
 	 * @return input validator for refNames
 	 */
 	public static IInputValidator getRefNameInputValidator(
-			final Repository repo, final String refPrefix, final boolean errorOnEmptyName) {
+			final Repository repo, final String refPrefix,
+			final boolean errorOnEmptyName) {
 		return new IInputValidator() {
 			@Override
 			public String isValid(String newText) {
-				if (newText.length() == 0) {
-					if (errorOnEmptyName)
-						return UIText.ValidationUtils_PleaseEnterNameMessage;
-					else
-						// ignore this
-						return null;
+				IStatus validationStatus = validateNewRefName(newText, repo,
+						refPrefix, errorOnEmptyName);
+				if (validationStatus.isOK()) {
+					return null;
 				}
-				String testFor = refPrefix + newText;
-				if (!Repository.isValidRefName(testFor))
-					return NLS.bind(
-							UIText.ValidationUtils_InvalidRefNameMessage,
-							testFor);
-				try {
-					if (repo.resolve(testFor) != null)
-						return NLS.bind(
-								UIText.ValidationUtils_RefAlreadyExistsMessage,
-								testFor);
-					RefDatabase refDatabase = repo.getRefDatabase();
-					Collection<String> conflictingNames = refDatabase.getConflictingNames(testFor);
-					if (!conflictingNames.isEmpty()) {
-						ArrayList<String> names = new ArrayList<>(conflictingNames);
-						Collections.sort(names);
-						String joined = StringUtils.join(names, ", "); //$NON-NLS-1$
-						return NLS.bind(
-								UIText.ValidationUtils_RefNameConflictsWithExistingMessage,
-								joined);
-					}
-				} catch (IOException e) {
-					Activator.logError(NLS.bind(
-							UIText.ValidationUtils_CanNotResolveRefMessage,
-							testFor), e);
-					return e.getMessage();
-				} catch (RevisionSyntaxException e) {
-					String m = MessageFormat.format(
-							UIText.ValidationUtils_InvalidRevision,
-							testFor);
-					Activator.logError(m, e);
-					return m;
+				if (validationStatus.getException() != null) {
+					Activator.handleStatus(validationStatus, false);
 				}
-				return null;
+				return validationStatus.getMessage();
 			}
 		};
 	}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/merge/GitMergeEditorInput.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/merge/GitMergeEditorInput.java
index 260acf8ec8..da2d5afb4e 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/merge/GitMergeEditorInput.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/merge/GitMergeEditorInput.java
@@ -35,6 +35,7 @@ import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.egit.core.internal.CompareCoreUtils;
+import org.eclipse.egit.core.internal.CoreText;
 import org.eclipse.egit.core.internal.storage.GitFileRevision;
 import org.eclipse.egit.core.internal.storage.WorkingTreeFileRevision;
 import org.eclipse.egit.core.internal.util.ResourceUtil;
@@ -174,7 +175,7 @@ public class GitMergeEditorInput extends CompareEditorInput {
 				ObjectId mergeHead = repo.resolve(target);
 				if (mergeHead == null)
 					throw new IOException(NLS.bind(
-							UIText.ValidationUtils_CanNotResolveRefMessage,
+							CoreText.ValidationUtils_CanNotResolveRefMessage,
 							target));
 				rightCommit = rw.parseCommit(mergeHead);
 			} catch (IOException e) {
@@ -188,7 +189,7 @@ public class GitMergeEditorInput extends CompareEditorInput {
 				ObjectId head = repo.resolve(Constants.HEAD);
 				if (head == null)
 					throw new IOException(NLS.bind(
-							UIText.ValidationUtils_CanNotResolveRefMessage,
+							CoreText.ValidationUtils_CanNotResolveRefMessage,
 							Constants.HEAD));
 				headCommit = rw.parseCommit(head);
 			} catch (IOException e) {
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
index 773d3c8502..76260ff82f 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
@@ -1832,13 +1832,6 @@ SimpleConfigurePushDialog_SaveAndPushButton=Save and Push
 SkipRebaseCommand_CancelDialogMessage=The skip operation was canceled.
 SkipRebaseCommand_JobName=Skipping commit during Rebase
 
-ValidationUtils_CanNotResolveRefMessage=Cannot resolve {0}
-ValidationUtils_InvalidRefNameMessage={0} is not a valid name for a ref
-ValidationUtils_InvalidRevision=Invalid revision {0}
-ValidationUtils_RefAlreadyExistsMessage=Ref {0} already exists
-ValidationUtils_RefNameConflictsWithExistingMessage=Name conflicts with existing refs: {0}
-ValidationUtils_PleaseEnterNameMessage=Please enter a ref name
-
 GitMergeEditorInput_CalculatingDiffTaskName=Calculating Differences
 GitMergeEditorInput_CheckingResourcesTaskName=Checking resources
 GitMergeEditorInput_MergeEditorTitle=Repository ''{0}'': Merging ''{1}'' into ''{2}''