(split from main repo) Gerrit: fix server version parsing (#7559)

dfev77 · RussKie · commit 503e9da27d7a · 2019-12-30T23:53:43.000+11:00
* Gerrit: add configuration for gerrit version

Connecting to gerrit and retrieving configuration requires too many
configurations to work properly due to possible configuration
combinations (api port, http/https, etc.).
Instead just allow the user to configure the desired Gerrit version.

Extract code that is dependent on Gerrit version into separate
classes (list of publish types and logic for composing command) in
order to be able to test it.

* Gerrit: Don't close window is unsuccessful

So that user can change values and retry
diff --git a/FormGerritBase.cs b/FormGerritBase.cs
@@ -1,15 +1,12 @@
 ﻿using System;
 using GitUI;
 using GitUIPluginInterfaces;
-using RestSharp;
 
 namespace Gerrit
 {
     public class FormGerritBase : GitExtensionsForm
     {
         protected GerritSettings Settings { get; private set; }
-        protected Version Version { get; private set; }
-        protected readonly RestClient client = new RestClient();
         protected readonly IGitUICommands UICommands;
         protected IGitModule Module => UICommands.GitModule;
 
@@ -39,34 +36,7 @@ protected override void OnLoad(EventArgs e)
                 return;
             }
 
-            SetRestClientUrl();
-            Version = GetGerritVersion();
-
             base.OnLoad(e);
         }
-
-        private void SetRestClientUrl()
-        {
-            string host = Settings.Host;
-
-            if (!host.StartsWith("http://") || !host.StartsWith("https://"))
-            {
-                host = "https://" + host;
-            }
-
-            if (!host.EndsWith("/"))
-            {
-                host += "/";
-            }
-
-            client.BaseUrl = new Uri(host);
-        }
-
-        private Version GetGerritVersion()
-        {
-            RestRequest request = new RestRequest("/config/server/version");
-            IRestResponse response = client.Execute(request);
-            return Version.Parse(response.Content.Replace(")]}'", "").Replace("\n", "").Replace("\"", "").Split('-')[0]);
-        }
     }
 }
diff --git a/FormGerritPublish.cs b/FormGerritPublish.cs
@@ -1,7 +1,9 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Windows.Forms;
+using Gerrit.Server;
 using GitCommands;
 using GitExtUtils;
 using GitExtUtils.GitUI.Theming;
@@ -14,42 +16,34 @@ namespace Gerrit
 {
     public partial class FormGerritPublish : FormGerritBase
     {
-        private string _currentBranchRemote;
-
         #region Translation
         private readonly TranslationString _publishGerritChangeCaption = new TranslationString("Publish Gerrit Change");
 
         private readonly TranslationString _publishCaption = new TranslationString("Publish change");
 
         private readonly TranslationString _selectRemote = new TranslationString("Please select a remote repository");
         private readonly TranslationString _selectBranch = new TranslationString("Please enter a branch");
-
-        private readonly TranslationString _publishTypeReview = new TranslationString("For Review");
-        private readonly TranslationString _publishTypeWip = new TranslationString("Work-in-Progress");
-        private readonly TranslationString _publishTypePrivate = new TranslationString("Private");
         #endregion
 
-        public FormGerritPublish(IGitUICommands uiCommand)
+        private string _currentBranchRemote;
+        private GerritCapabilities _capabilities;
+
+        public FormGerritPublish(IGitUICommands uiCommand, GerritCapabilities capabilities)
             : base(uiCommand)
         {
+            _capabilities = capabilities;
             InitializeComponent();
             Publish.Image = Images.Push.AdaptLightness();
             InitializeComplete();
-            PublishType.Items.AddRange(new object[]
-            {
-                new KeyValuePair<string, string>(_publishTypeReview.Text, ""),
-                new KeyValuePair<string, string>(_publishTypeWip.Text, "wip")
-            });
-            PublishType.SelectedIndex = 0;
         }
 
         protected override void OnLoad(EventArgs e)
         {
             base.OnLoad(e);
-            if (Version >= Version.Parse("2.15"))
-            {
-                PublishType.Items.Add(new KeyValuePair<string, string>(_publishTypePrivate.Text, "private"));
-            }
+
+            _capabilities.PublishTypes.ForEach(
+                item => PublishType.Items.Add(item));
+            PublishType.SelectedIndex = 0;
         }
 
         private void PublishClick(object sender, EventArgs e)
@@ -88,59 +82,17 @@ private bool PublishChange(IWin32Window owner)
 
             GerritUtil.StartAgent(owner, Module, _NO_TRANSLATE_Remotes.Text);
 
-            List<string> additionalOptions = new List<string>();
-
-            string reviewers = _NO_TRANSLATE_Reviewers.Text.Trim();
-            if (!string.IsNullOrEmpty(reviewers))
-            {
-                additionalOptions.AddRange(reviewers.Split(new[] { ' ', ',', ';', '|' })
-                                                    .Where(r => !string.IsNullOrEmpty(r))
-                                                    .Select(r => "r=" + r));
-            }
-
-            string cc = _NO_TRANSLATE_Cc.Text.Trim();
-            if (!string.IsNullOrEmpty(cc))
-            {
-                additionalOptions.AddRange(cc.Split(new[] { ' ', ',', ';', '|' })
-                                             .Where(r => !string.IsNullOrEmpty(r))
-                                             .Select(r => "cc=" + r));
-            }
-
-            string topic = _NO_TRANSLATE_Topic.Text.Trim();
-            if (!string.IsNullOrEmpty(topic))
-            {
-                additionalOptions.Add("topic=" + topic);
-            }
-
-            string hashtag = _NO_TRANSLATE_Hashtag.Text.Trim();
-            if (!string.IsNullOrEmpty(hashtag))
-            {
-                additionalOptions.Add("hashtag=" + hashtag);
-            }
-
-            additionalOptions = additionalOptions.Where(r => !string.IsNullOrEmpty(r)).ToList();
-
-            string publishType = ((KeyValuePair<string, string>)PublishType.SelectedItem).Value;
-            string targetRef = "for";
-            if (Version >= Version.Parse("2.15"))
-            {
-                additionalOptions.Add(publishType);
-            }
-            else if (publishType == "wip")
-            {
-                targetRef = "drafts";
-            }
-
-            string targetBranch = $"refs/{targetRef}/{branch}";
-            if (additionalOptions.Count > 0)
-            {
-                targetBranch += "%" + string.Join(",", additionalOptions);
-            }
+            var builder = _capabilities.NewBuilder()
+                .WithReviewers(_NO_TRANSLATE_Reviewers.Text)
+                .WithCC(_NO_TRANSLATE_Cc.Text)
+                .WithTopic(_NO_TRANSLATE_Topic.Text)
+                .WithHashTag(_NO_TRANSLATE_Hashtag.Text)
+                .WithPublishType(((KeyValuePair<string, string>)PublishType.SelectedItem).Value);
 
             var pushCommand = UICommands.CreateRemoteCommand();
             pushCommand.CommandText = PushCmd(
                 _NO_TRANSLATE_Remotes.Text,
-                targetBranch);
+                builder.Build(branch));
 
             pushCommand.Remote = _NO_TRANSLATE_Remotes.Text;
             pushCommand.Title = _publishCaption.Text;
@@ -176,7 +128,7 @@ private bool PublishChange(IWin32Window owner)
                 }
             }
 
-            return true;
+            return !pushCommand.ErrorOccurred;
         }
 
         [CanBeNull]
diff --git a/Gerrit.csproj b/Gerrit.csproj
@@ -96,6 +96,10 @@
       <DesignTime>True</DesignTime>
       <DependentUpon>Resources.resx</DependentUpon>
     </Compile>
+    <Compile Include="Server\CommandBuilder.cs" />
+    <Compile Include="Server\CommandBuilderWithDraftSupport.cs" />
+    <Compile Include="Server\CommandBuilderWithPrivateSupport.cs" />
+    <Compile Include="Server\GerritCapabilities.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\GitCommands\GitCommands.csproj">
@@ -157,5 +161,6 @@
     <Content Include="Resources\GerritInstallHook.png" />
     <None Include="Resources\IconGerrit.png" />
   </ItemGroup>
+  <ItemGroup />
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
 </Project>
diff --git a/GerritPlugin.cs b/GerritPlugin.cs
@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using System.Windows.Forms;
 using Gerrit.Properties;
+using Gerrit.Server;
 using GitUI;
 using GitUIPluginInterfaces;
 using JetBrains.Annotations;
@@ -28,6 +29,13 @@ public class GerritPlugin : GitPluginBase, IGitPluginForRepository
         private readonly TranslationString _installCommitMsgHookDownloadFileFailed = new TranslationString("Could not download the commit-msg file. Please install the commit-msg hook manually.");
         #endregion
 
+        private const string DefaultGerritVersion = "2.15 or newer";
+
+        private readonly ChoiceSetting _predefinedGerritVersion = new ChoiceSetting(
+            "Treat Gerrit as having version",
+            new[] { DefaultGerritVersion, "Older then 2.15" },
+            DefaultGerritVersion);
+
         private static readonly Dictionary<string, bool> _validatedHooks = new Dictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
         private static readonly object _syncRoot = new object();
 
@@ -42,7 +50,7 @@ public class GerritPlugin : GitPluginBase, IGitPluginForRepository
         private ToolStripButton _installCommitMsgMenuItem;
 
         // public only because of FormTranslate
-        public GerritPlugin() : base(false)
+        public GerritPlugin() : base(true)
         {
             SetNameAndDescription("Gerrit Code Review");
             Translate();
@@ -248,7 +256,11 @@ private void Initialize(Form form)
 
         private void publishMenuItem_Click(object sender, EventArgs e)
         {
-            using (var form = new FormGerritPublish(_gitUiCommands))
+            var capabilities = _predefinedGerritVersion.ValueOrDefault(Settings) == DefaultGerritVersion
+                ? GerritCapabilities.Version2_15
+                : GerritCapabilities.OldestVersion;
+
+            using (var form = new FormGerritPublish(_gitUiCommands, capabilities))
             {
                 form.ShowDialog(_mainForm);
             }
@@ -421,5 +433,10 @@ public override bool Execute(GitUIEventArgs args)
 
             return false;
         }
+
+        public override IEnumerable<ISetting> GetSettings()
+        {
+            yield return _predefinedGerritVersion;
+        }
     }
 }
diff --git a/Properties/AssemblyInfo.cs b/Properties/AssemblyInfo.cs
@@ -1,3 +1,5 @@
 ﻿using System.Reflection;
+using System.Runtime.CompilerServices;
 
 [assembly: AssemblyDescription("GitExtensions plugin for integration with Gerrit")]
+[assembly: InternalsVisibleTo("GerritTests")]
diff --git a/Server/CommandBuilder.cs b/Server/CommandBuilder.cs
@@ -0,0 +1,71 @@
+﻿using System;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace Gerrit.Server
+{
+    public abstract class CommandBuilder
+    {
+        protected string TargetRef = "for";
+        protected List<string> CommandArguments { get; } = new List<string>();
+
+        private static IEnumerable<string> SplitAndComposeArguments(string argumentName, string values)
+        {
+            values = values?.Trim();
+            if (string.IsNullOrEmpty(values))
+            {
+                return Enumerable.Empty<string>();
+            }
+
+            return ComposeArguments(argumentName, values.Split(new[] { ' ', ',', ';', '|' }, StringSplitOptions.RemoveEmptyEntries));
+        }
+
+        private static IEnumerable<string> ComposeArguments(string argumentName, string[] values)
+        {
+            return values.Where(text => !string.IsNullOrEmpty(text))
+                .Select(text => $"{argumentName}={text}");
+        }
+
+        private static IEnumerable<string> ComposeArgument(string argumentName, string value)
+        {
+            return ComposeArguments(argumentName, new[] { value?.Trim() });
+        }
+
+        public CommandBuilder WithReviewers(string text)
+        {
+            CommandArguments.AddRange(SplitAndComposeArguments("r", text));
+            return this;
+        }
+
+        public CommandBuilder WithCC(string text)
+        {
+            CommandArguments.AddRange(SplitAndComposeArguments("cc", text));
+            return this;
+        }
+
+        public CommandBuilder WithTopic(string topic)
+        {
+            CommandArguments.AddRange(ComposeArgument(@"topic", topic));
+            return this;
+        }
+
+        public CommandBuilder WithHashTag(string hashTag)
+        {
+            CommandArguments.AddRange(ComposeArgument(@"hashtag", hashTag));
+            return this;
+        }
+
+        public abstract CommandBuilder WithPublishType(string publishType);
+
+        public virtual string Build(string branch)
+        {
+            string targetBranch = $"refs/{TargetRef}/{branch}";
+            if (CommandArguments.Count > 0)
+            {
+                targetBranch += "%" + string.Join(",", CommandArguments);
+            }
+
+            return targetBranch;
+        }
+    }
+}
diff --git a/Server/CommandBuilderWithDraftSupport.cs b/Server/CommandBuilderWithDraftSupport.cs
@@ -0,0 +1,17 @@
+﻿namespace Gerrit.Server
+{
+    public class CommandBuilderWithDraftSupport : CommandBuilder
+    {
+        public const string DraftsPublishType = @"drafts";
+
+        public override CommandBuilder WithPublishType(string publishType)
+        {
+            if (publishType == DraftsPublishType)
+            {
+                TargetRef = DraftsPublishType;
+            }
+
+            return this;
+        }
+    }
+}
diff --git a/Server/CommandBuilderWithPrivateSupport.cs b/Server/CommandBuilderWithPrivateSupport.cs
@@ -0,0 +1,24 @@
+﻿namespace Gerrit.Server
+{
+    public class CommandBuilderWithPrivateSupport : CommandBuilder
+    {
+        private string _publishType;
+
+        public override CommandBuilder WithPublishType(string publishType)
+        {
+            _publishType = publishType?.Trim();
+
+            return this;
+        }
+
+        public override string Build(string branch)
+        {
+            if (!string.IsNullOrEmpty(_publishType))
+            {
+                CommandArguments.Add(_publishType);
+            }
+
+            return base.Build(branch);
+        }
+    }
+}
diff --git a/Server/GerritCapabilities.cs b/Server/GerritCapabilities.cs
