Merge pull request #122 from github/shana/121-url-conversion-should-never-throw

ToRepositoryUrl should never throw, it's a conversion helper

Author: haacked

src/GitHub.Exports/Primitives/UriString.cs

@@ -108,6 +108,7 @@ bool ParseScpSyntax(string scpString)
                 Host = match.Groups["host"].Value.ToNullIfEmpty();
                 Owner = match.Groups["owner"].Value.ToNullIfEmpty();
                 RepositoryName = GetRepositoryName(match.Groups["repo"].Value);
+                IsScpUri = true;
                 return true;
             }
             return false;
@@ -123,15 +124,18 @@ bool ParseScpSyntax(string scpString)
 
         public bool IsFileUri { get; private set; }
 
+        public bool IsScpUri { get; private set; }
+
         public bool IsValidUri => url != null;
 
         /// <summary>
         /// Attempts a best-effort to convert the remote origin to a GitHub Repository URL.
         /// </summary>
-        /// <returns></returns>
+        /// <returns>A converted uri, or the existing one if we can't convert it (which might be null)</returns>
         public Uri ToRepositoryUrl()
         {
-            if (url != null && IsFileUri) return url;
+            // we only want to process urls that represent network resources
+            if (!IsScpUri && (!IsValidUri || IsFileUri)) return url;
 
             var scheme = url != null && IsHypertextTransferProtocol
                 ? url.Scheme

src/GitHub.Exports/Services/GitService.cs

@@ -167,8 +167,7 @@ public static UriString GetOriginUri(IRepository repo)
         {
             return repo
                 ?.Network
-                .Remotes
-                .FirstOrDefault(x => x.Name.Equals("origin", StringComparison.Ordinal))
+                .Remotes["origin"]
                 ?.Url;
         }
 

src/UnitTests/GitHub.Exports/GitServiceTests.cs

@@ -0,0 +1,37 @@
+using System;
+using GitHub.Services;
+using LibGit2Sharp;
+using NSubstitute;
+using Xunit;
+using System.Linq;
+using System.Collections;
+using System.Collections.Generic;
+
+public class GitServiceTests : TestBaseClass
+{
+    [Theory]
+    [InlineData("asdf", null)]
+    [InlineData("", null)]
+    [InlineData(null, null)]
+    [InlineData("file:///C:/dev/exp/foo", "file:///C:/dev/exp/foo")]
+    [InlineData("http://example.com/", "http://example.com/")]
+    [InlineData("http://[email protected]/foo/bar", "http://example.com/foo/bar")]
+    [InlineData("https://github.com/github/Windows", "https://github.com/github/Windows")]
+    [InlineData("https://github.com/github/Windows.git", "https://github.com/github/Windows")]
+    [InlineData("https://[email protected]/github/Windows.git", "https://github.com/github/Windows")]
+    [InlineData("http://example.com:4000/github/Windows", "http://example.com:4000/github/Windows")]
+    [InlineData("[email protected]:github/Windows.git", "https://192.168.1.2/github/Windows")]
+    [InlineData("[email protected]:org/repo.git", "https://example.com/org/repo")]
+    [InlineData("ssh://[email protected]:443/shana/cef", "https://github.com/shana/cef")]
+    [InlineData("ssh://[email protected]:23/haacked/encourage", "https://example.com:23/haacked/encourage")]
+    public void GetUriShouldNotThrow(string url, string expected)
+    {
+        var origin = Substitute.For<Remote>();
+        origin.Url.Returns(url);
+        var repository = Substitute.For<IRepository>();
+        repository.Network.Remotes["origin"].Returns(origin);
+
+        var gitservice = new GitService();
+        Assert.Equal(expected, gitservice.GetUri(repository)?.ToString());
+    }
+}

src/UnitTests/GitHub.Primitives/UriStringTests.cs

@@ -177,6 +177,27 @@ public void ConvertsToWebUrl(string uriString, string expected)
         {
             Assert.Equal(new Uri(expected), new UriString(uriString).ToRepositoryUrl());
         }
+
+        [Theory]
+        [InlineData("asdf", null)]
+        [InlineData("", null)]
+        [InlineData("file:///C:/dev/exp/foo", "file:///C:/dev/exp/foo")]
+        [InlineData("http://example.com/", "http://example.com/")]
+        [InlineData("http://[email protected]/foo/bar", "http://example.com/foo/bar")]
+        [InlineData("https://github.com/github/Windows", "https://github.com/github/Windows")]
+        [InlineData("https://github.com/github/Windows.git", "https://github.com/github/Windows")]
+        [InlineData("https://[email protected]/github/Windows.git", "https://github.com/github/Windows")]
+        [InlineData("http://example.com:4000/github/Windows", "http://example.com:4000/github/Windows")]
+        [InlineData("[email protected]:github/Windows.git", "https://192.168.1.2/github/Windows")]
+        [InlineData("[email protected]:org/repo.git", "https://example.com/org/repo")]
+        [InlineData("ssh://[email protected]:443/shana/cef", "https://github.com/shana/cef")]
+        [InlineData("ssh://[email protected]:23/haacked/encourage", "https://example.com:23/haacked/encourage")]
+        public void ShouldNeverThrow(string url, string expected)
+        {
+            Uri uri;
+            Uri.TryCreate(expected, UriKind.Absolute, out uri);
+            Assert.Equal(uri, new UriString(url).ToRepositoryUrl());
+        }
     }
 
     public class TheAdditionOperator

src/UnitTests/UnitTests.csproj

@@ -155,6 +155,7 @@
     <Compile Include="GitHub.App\ViewModels\RepositoryCreationViewModelTests.cs" />
     <Compile Include="GitHub.App\ViewModels\RepositoryPublishViewModelTests.cs" />
     <Compile Include="GitHub.Exports.Reactive\Caches\AccountCacheItemTests.cs" />
+    <Compile Include="GitHub.Exports\GitServiceTests.cs" />
     <Compile Include="GitHub.Exports\VSServicesTests.cs" />
     <Compile Include="GitHub.Extensions\UriExtensionTests.cs" />
     <Compile Include="GitHub.Primitives\UriStringTests.cs" />