Make compatibility with GitExtensions v4.1 & Gerrit server v3.7.0 (#74)

poupounetjoyeux · web-flow · commit 76fae48d8b75 · 2023-09-22T11:30:40.000-06:00
* Fix missing method implementation OsShellUtil.OpenUrlInDefaultBrowser exception

 - Due to the move of GitCommands

Fix the parsing of the remote command lines output to retrieve the gerrit change URI
 - Keep the legacy parsing using 'New Changes' line
 - Following gerrit v3.7.0 remote outputs modifications take care of '[NEW]' at the end of line
 - Use a global regex on the command line ouput to retrieve the change URI in bot cases
 - Add a unit test with both use cases with command outputs test resource files

Update the README to indicates supported version (master-branch needs to be updated at release time depending of the new version the plugin will take)

* Update README typo to respect conventions

* Update following review recommendations
diff --git a/README.md b/README.md
@@ -25,11 +25,12 @@ Use the following pattern to link revision data to Gerrit:
 
 ## Compatibility Version Matrix
 
-| Git Extensions     | Gerrit Plugin       |
-|--------------------|---------------------|
-| v <= 3.5.x         | v <= 1.3.2          |
-| 3.5.x < v <= 4.0.0 | 2.0.0 <= v <= 2.0.1 |
-| 4.0.1 <= v         | 2.0.5 <= v          |
+| Git Extensions      | Gerrit Plugin       |
+|---------------------|---------------------|
+| v <= 3.5.x          | v <= 1.3.2          |
+| 3.5.x < v <= 4.0.0  | 2.0.0 <= v <= 2.0.1 |
+| 4.0.1 <= v <= 4.0.2 | 2.0.5               |
+| 4.1 <= v            | master-branch       |
 
 ## GitExtensions Plugin Template infomration
 
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/GerritUtilTests.cs b/UnitTests/GitExtensions.GerritPlugin.Tests/GerritUtilTests.cs
@@ -0,0 +1,23 @@
+﻿using NUnit.Framework;
+using System.IO;
+
+namespace GitExtensions.GerritPlugin.Tests
+{
+    [TestFixture]
+    public class GerritUtilTests
+    {
+        [TestCase("NewChange_GerritLegacy", true)]
+        [TestCase("UpdatedChange_GerritLegacy", false)]
+        [TestCase("NewChange_GerritNew", true)]
+        [TestCase("UpdatedChange_GerritNew", false)]
+        [TestCase("UnparsableUri", false)]
+        public void TestPublishedGerritChangeUriParsing(string commandPromptFileName, bool hadNewChange)
+        {
+            var commandPrompt = File.ReadAllText(Path.Combine(Directory.GetCurrentDirectory(), "TestResources", $"{commandPromptFileName}.txt"));
+            Assert.That(GerritUtil.HadNewChange(commandPrompt, out var changeUri), Is.EqualTo(hadNewChange));
+
+            var expectedChangeUri = hadNewChange ? "https://my.domain.test/c/MyProjectPath/MyProject/+/2664" : null;
+            Assert.That(changeUri, Is.EqualTo(expectedChangeUri), "We ddidn't get the right change URI from remote command output");
+        }
+    }
+}
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/GitExtensions.GerritPlugin.Tests.csproj b/UnitTests/GitExtensions.GerritPlugin.Tests/GitExtensions.GerritPlugin.Tests.csproj
@@ -15,4 +15,10 @@
   <ItemGroup>
     <ProjectReference Include="..\..\src\GitExtensions.GerritPlugin\GitExtensions.GerritPlugin.csproj" />
   </ItemGroup>
+
+  <ItemGroup>
+	  <None Update="TestResources\**\*.*">
+		  <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+	  </None>
+  </ItemGroup>
 </Project>
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/NewChange_GerritLegacy.txt b/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/NewChange_GerritLegacy.txt
@@ -0,0 +1,13 @@
+﻿remote: Resolving deltas: 100% (122/122)
+remote: Processing changes: refs: 1, updated: 1, done
+remote:
+remote: SUCCESS
+remote:
+remote: New Changes:
+remote:   https://my.domain.test/c/MyProjectPath/MyProject/+/2664 Testing a publish with only updates
+remote:
+remote:
+remote:
+To https://my.domain.test/MyProjectPath/MyProject
+ * [new reference]   HEAD -> refs/for/main
+Done
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/NewChange_GerritNew.txt b/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/NewChange_GerritNew.txt
@@ -0,0 +1,12 @@
+remote: Resolving deltas: 100% (122/122)
+remote: Processing changes: refs: 1, updated: 1, done
+remote:
+remote: SUCCESS
+remote:
+remote:   https://my.domain.test/c/MyProjectPath/MyProject/+/2664 Testing a publish with new change [NEW]
+remote:
+remote:
+remote:
+To https://my.domain.test/MyProjectPath/MyProject
+ * [new reference]   HEAD -> refs/for/main
+Done
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UnparsableUri.txt b/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UnparsableUri.txt
@@ -0,0 +1,12 @@
+﻿remote: Resolving deltas: 100% (122/122)
+remote: Processing changes: refs: 1, updated: 1, done
+remote:
+remote: SUCCESS
+remote:
+remote:   https://my.domain.test/ Testing a publish with badly formated remote output [NEW]
+remote:
+remote:
+remote:
+To https://my.domain.test/MyProjectPath/MyProject
+ * [new reference]   HEAD -> refs/for/main
+Done
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UpdatedChange_GerritLegacy.txt b/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UpdatedChange_GerritLegacy.txt
@@ -0,0 +1,13 @@
+remote: Resolving deltas: 100% (122/122)
+remote: Processing changes: refs: 1, updated: 1, done
+remote:
+remote: SUCCESS
+remote:
+remote: Updated Changes:
+remote:   https://my.domain.test/c/MyProjectPath/MyProject/+/2664 Testing a publish with only updates
+remote:
+remote:
+remote:
+To https://my.domain.test/MyProjectPath/MyProject
+ * [new reference]   HEAD -> refs/for/main
+Done
diff --git a/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UpdatedChange_GerritNew.txt b/UnitTests/GitExtensions.GerritPlugin.Tests/TestResources/UpdatedChange_GerritNew.txt
@@ -0,0 +1,12 @@
+remote: Resolving deltas: 100% (122/122)
+remote: Processing changes: refs: 1, updated: 1, done
+remote:
+remote: SUCCESS
+remote:
+remote:   https://my.domain.test/c/MyProjectPath/MyProject/+/2664 Testing a publish with only updates
+remote:
+remote:
+remote:
+To https://my.domain.test/MyProjectPath/MyProject
+ * [new reference]   HEAD -> refs/for/main
+Done
diff --git a/src/GitExtensions.GerritPlugin/FormGerritChangeSubmitted.cs b/src/GitExtensions.GerritPlugin/FormGerritChangeSubmitted.cs
@@ -1,4 +1,5 @@
 ﻿using System.Windows.Forms;
+using GitCommands;
 using GitUI;
 
 namespace GitExtensions.GerritPlugin
diff --git a/src/GitExtensions.GerritPlugin/FormGerritPublish.cs b/src/GitExtensions.GerritPlugin/FormGerritPublish.cs
@@ -10,6 +10,7 @@
 using GitUIPluginInterfaces;
 using JetBrains.Annotations;
 using ResourceManager;
+using System.Text.RegularExpressions;
 
 namespace GitExtensions.GerritPlugin
 {
@@ -104,31 +105,9 @@ private bool PublishChange(IWin32Window owner)
 
             if (!pushCommand.ErrorOccurred)
             {
-                bool hadNewChanges = false;
-                string change = null;
-
-                foreach (string line in pushCommand.CommandOutput.Split('\n'))
-                {
-                    if (hadNewChanges)
-                    {
-                        const char esc = (char)27;
-                        change = line
-                            .RemovePrefix("remote:")
-                            .SubstringUntilLast(esc)
-                            .Trim()
-                            .SubstringUntil(' ');
-                        break;
-                    }
-
-                    if (line.Contains("New Changes"))
-                    {
-                        hadNewChanges = true;
-                    }
-                }
-
-                if (change != null)
+                if(GerritUtil.HadNewChange(pushCommand.CommandOutput, out var changeUri))
                 {
-                    FormGerritChangeSubmitted.ShowSubmitted(owner, change);
+                    FormGerritChangeSubmitted.ShowSubmitted(owner, changeUri);
                 }
             }
 
diff --git a/src/GitExtensions.GerritPlugin/FormPluginInformation.cs b/src/GitExtensions.GerritPlugin/FormPluginInformation.cs
@@ -1,4 +1,5 @@
 ﻿using System.Windows.Forms;
+using GitCommands;
 using GitUI;
 
 namespace GitExtensions.GerritPlugin
diff --git a/src/GitExtensions.GerritPlugin/GerritUtil.cs b/src/GitExtensions.GerritPlugin/GerritUtil.cs
@@ -2,6 +2,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using System.Windows.Forms;
 using GitCommands;
@@ -27,6 +28,32 @@ public static async Task<string> RunGerritCommandAsync(
             var fetchUrl = GetFetchUrl(module, remote);
 
             return await RunGerritCommandAsync(owner, module, command, fetchUrl, remote, stdIn).ConfigureAwait(false);
+        }
+
+        public static bool HadNewChange(string commandPrompt, out string changeUri)
+        {
+            if(string.IsNullOrEmpty(commandPrompt))
+            {
+                changeUri = null;
+                return false;
+            }
+
+            if (commandPrompt.Split('\n').All(line => !line.Contains("New Changes") && !line.EndsWith("[NEW]")))
+            {
+                changeUri = null;
+                return false;
+            }
+
+            var changeUriRegex = new Regex(@"https?://[^ ]+/c/[^ ]+/\+/[0-9]+", RegexOptions.Multiline);
+            var changeUriMatch = changeUriRegex.Match(commandPrompt);
+            if (!changeUriMatch.Success)
+            {
+                changeUri = null;
+                return false;
+            }
+
+            changeUri = changeUriMatch.Value;
+            return true;
         }
 
         public static Uri GetFetchUrl(IGitModule module, string remote)

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using System.Windows.Forms;`
	`2`	`+using GitCommands;`
`2`	`3`	`using GitUI;`
`3`	`4`
`4`	`5`	`namespace GitExtensions.GerritPlugin`