diff --git a/Octokit.Tests/Helpers/UriExtensionsTests.cs b/Octokit.Tests/Helpers/UriExtensionsTests.cs index fab35b67..0095b84d 100644 --- a/Octokit.Tests/Helpers/UriExtensionsTests.cs +++ b/Octokit.Tests/Helpers/UriExtensionsTests.cs @@ -22,6 +22,20 @@ namespace Octokit.Tests.Helpers Assert.Equal(new Uri("https://example.com?foo=foo%20val&bar=barval"), uriWithParameters); } + [Fact] + public void AppendsParametersAsQueryStringWithRelativeUri() + { + var uri = new Uri("issues", UriKind.Relative); + + var uriWithParameters = uri.ApplyParameters(new Dictionary + { + {"foo", "fooval"}, + {"bar", "barval"} + }); + + Assert.Equal(new Uri("issues?foo=fooval&bar=barval", UriKind.Relative), uriWithParameters); + } + [Fact] public void ThrowsExceptionWhenNullValueProvided() { @@ -36,17 +50,16 @@ namespace Octokit.Tests.Helpers } [Fact] - public void AppendsParametersAsQueryStringToRelativeUri() + public void ThrowsExceptionWhenNullValueProvidedWithRelativeUri() { - var uri = new Uri("issues", UriKind.Relative); + var uri = new Uri("api/example", UriKind.Relative); - var uriWithParameters = uri.ApplyParameters(new Dictionary + var parameters = new Dictionary { - {"foo", "fooval"}, - {"bar", "barval"} - }); + {"foo", null } + }; - Assert.Equal(new Uri("issues?foo=fooval&bar=barval", UriKind.Relative), uriWithParameters); + Assert.Throws(() => uri.ApplyParameters(parameters)); } [Fact] @@ -61,19 +74,44 @@ namespace Octokit.Tests.Helpers Assert.Equal(uri, uriWithNullParameters); } + [Fact] + public void DoesNotChangeUrlWhenParametersEmptyWithRelativeUri() + { + var uri = new Uri("api/example", UriKind.Relative); + + var uriWithEmptyParameters = uri.ApplyParameters(new Dictionary()); + var uriWithNullParameters = uri.ApplyParameters(null); + + Assert.Equal(uri, uriWithEmptyParameters); + Assert.Equal(uri, uriWithNullParameters); + } + [Fact] public void CombinesExistingParametersWithNewParameters() { var uri = new Uri("https://api.github.com/repositories/1/milestones?state=closed&sort=due_date&direction=asc&page=2"); - var parameters = new Dictionary { { "state", "open" }, { "sort", "other" } }; - + var parameters = new Dictionary { { "state", "open" }, { "sort", "other" }, { "per_page", "5" } }; + var actual = uri.ApplyParameters(parameters); - Assert.True(actual.Query.Contains("state=open")); - Assert.True(actual.Query.Contains("sort=other")); - Assert.True(actual.Query.Contains("direction=asc")); - Assert.True(actual.Query.Contains("page=2")); + Assert.Equal( + new Uri("https://api.github.com/repositories/1/milestones?state=open&sort=other&per_page=5&direction=asc&page=2"), + actual); + } + + [Fact] + public void CombinesExistingParametersWithNewParametersToRelativeUri() + { + var uri = new Uri("repositories/1/milestones?state=closed&sort=due_date&direction=asc&page=2", UriKind.Relative); + + var parameters = new Dictionary { { "state", "open" }, { "sort", "other" }, { "per_page", "5" } }; + + var actual = uri.ApplyParameters(parameters); + + Assert.Equal( + new Uri("repositories/1/milestones?state=open&sort=other&per_page=5&direction=asc&page=2", UriKind.Relative), + actual); } [Fact] @@ -88,6 +126,18 @@ namespace Octokit.Tests.Helpers Assert.Equal(2, parameters.Count); } + [Fact] + public void DoesNotChangePassedInDictionaryForRelativeUri() + { + var uri = new Uri("/repositories/1/milestones?state=closed&sort=due_date&direction=asc&page=2", UriKind.Relative); + + var parameters = new Dictionary { { "state", "open" }, { "sort", "other" } }; + + uri.ApplyParameters(parameters); + + Assert.Equal(2, parameters.Count); + } + [Fact] public void EnsuresArgumentNotNull() { diff --git a/Octokit/Helpers/UriExtensions.cs b/Octokit/Helpers/UriExtensions.cs index 2ee1b873..7317b9b9 100644 --- a/Octokit/Helpers/UriExtensions.cs +++ b/Octokit/Helpers/UriExtensions.cs @@ -25,6 +25,12 @@ namespace Octokit // use a temporary dictionary which combines new and existing parameters IDictionary p = new Dictionary(parameters); + var hasQueryString = uri.OriginalString.IndexOf("?", StringComparison.Ordinal); + + string uriWithoutQuery = hasQueryString == -1 + ? uri.ToString() + : uri.OriginalString.Substring(0, hasQueryString); + string queryString; if (uri.IsAbsoluteUri) { @@ -32,7 +38,6 @@ namespace Octokit } else { - var hasQueryString = uri.OriginalString.IndexOf("?", StringComparison.Ordinal); queryString = hasQueryString == -1 ? "" : uri.OriginalString.Substring(hasQueryString); @@ -65,7 +70,7 @@ namespace Octokit return uriBuilder.Uri; } - return new Uri(uri + "?" + query, UriKind.Relative); + return new Uri(uriWithoutQuery + "?" + query, UriKind.Relative); } } }