From 502298aab96099a8445532c59eb37867e8576887 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sun, 3 Apr 2016 18:47:43 -0700 Subject: [PATCH 1/2] Cleanup HttpUri.PathCombine --- .../Http/HttpUriFixture.cs | 29 +++++++++++++++++++ .../NzbDrone.Common.Test.csproj | 1 + src/NzbDrone.Common/Http/HttpUri.cs | 18 ++---------- 3 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 src/NzbDrone.Common.Test/Http/HttpUriFixture.cs diff --git a/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs b/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs new file mode 100644 index 000000000..09cda2be7 --- /dev/null +++ b/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs @@ -0,0 +1,29 @@ +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Common.Http; +using NzbDrone.Test.Common; + +namespace NzbDrone.Common.Test.Http +{ + public class HttpUriFixture : TestBase + { + private HttpUri GivenHttpUri(string basePath) + { + return new HttpUri("http", "localhost", 8989, basePath, null, null); + } + + [TestCase("", "", "")] + [TestCase("base", "", "/base")] + [TestCase("/base", "", "/base")] + [TestCase("", "relative", "/relative")] + [TestCase("", "/relative", "/relative")] + [TestCase("base", "relative", "/base/relative")] + [TestCase("base", "/relative", "/base/relative")] + [TestCase("/base", "relative", "/base/relative")] + [TestCase("/base", "/relative", "/base/relative")] + public void should_combine_base_path_and_relative_path(string basePath, string relativePath, string expected) + { + GivenHttpUri(basePath).CombinePath(relativePath).Path.Should().Be(expected); + } + } +} diff --git a/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj b/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj index 338a9e926..84892c6b1 100644 --- a/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj +++ b/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj @@ -83,6 +83,7 @@ + diff --git a/src/NzbDrone.Common/Http/HttpUri.cs b/src/NzbDrone.Common/Http/HttpUri.cs index c8c83de66..05d12e7d8 100644 --- a/src/NzbDrone.Common/Http/HttpUri.cs +++ b/src/NzbDrone.Common/Http/HttpUri.cs @@ -144,23 +144,12 @@ namespace NzbDrone.Common.Http return basePath; } - var newPath = "/" + relativePath.TrimStart('/'); - - if (basePath != null && !relativePath.StartsWith("/")) + if (basePath.IsNullOrWhiteSpace()) { - var baseSlashIndex = basePath.LastIndexOf('/'); - - if (baseSlashIndex == basePath.Length - 1) - { - newPath = basePath.TrimEnd('/') + newPath; - } - else if (baseSlashIndex != 0) - { - newPath = basePath.Substring(0, baseSlashIndex) + newPath; - } + return "/" + relativePath.TrimStart('/'); } - return newPath; + return basePath.TrimEnd("/") + "/" + relativePath.TrimStart('/'); } public HttpUri SetQuery(string query) @@ -199,7 +188,6 @@ namespace NzbDrone.Common.Http return SetQuery(builder.ToString()); } - public override int GetHashCode() { return _uri.GetHashCode(); From 3a162be265af6a983115e30aa191019d8af327d3 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Mon, 4 Apr 2016 21:28:59 +0200 Subject: [PATCH 2/2] CombinePath now simple, uri resolve done via operator and CombineRelativePath. --- .../Http/HttpUriFixture.cs | 71 ++++++++++++++++--- src/NzbDrone.Common/Http/HttpUri.cs | 31 ++++++-- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs b/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs index 09cda2be7..099ab990f 100644 --- a/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs +++ b/src/NzbDrone.Common.Test/Http/HttpUriFixture.cs @@ -7,23 +7,78 @@ namespace NzbDrone.Common.Test.Http { public class HttpUriFixture : TestBase { - private HttpUri GivenHttpUri(string basePath) + [TestCase("", "", "")] + [TestCase("/", "", "/")] + [TestCase("base", "", "base")] + [TestCase("/base", "", "/base")] + [TestCase("/base/", "", "/base/")] + [TestCase("", "relative", "relative")] + [TestCase("", "/relative", "/relative")] + [TestCase("/", "relative", "/relative")] + [TestCase("/", "/relative", "/relative")] + [TestCase("base", "relative", "relative")] + [TestCase("base", "/relative", "/relative")] + [TestCase("/base", "relative", "/relative")] + [TestCase("/base", "/relative", "/relative")] + [TestCase("/base/", "relative", "/base/relative")] + [TestCase("/base/", "/relative", "/relative")] + [TestCase("base/sub", "relative", "base/relative")] + [TestCase("base/sub", "/relative", "/relative")] + [TestCase("/base/sub", "relative", "/base/relative")] + [TestCase("/base/sub", "/relative", "/relative")] + [TestCase("/base/sub/", "relative", "/base/sub/relative")] + [TestCase("/base/sub/", "/relative", "/relative")] + [TestCase("abc://host.com:8080/root/file.xml", "relative/path", "abc://host.com:8080/root/relative/path")] + [TestCase("abc://host.com:8080/root/file.xml", "/relative/path", "abc://host.com:8080/relative/path")] + [TestCase("abc://host.com:8080/root/file.xml?query=1#fragment", "relative/path", "abc://host.com:8080/root/relative/path")] + [TestCase("abc://host.com:8080/root/file.xml?query=1#fragment", "/relative/path", "abc://host.com:8080/relative/path")] + [TestCase("abc://host.com:8080/root/api", "relative/path", "abc://host.com:8080/root/relative/path")] + [TestCase("abc://host.com:8080/root/api", "/relative/path", "abc://host.com:8080/relative/path")] + [TestCase("abc://host.com:8080/root/api/", "relative/path", "abc://host.com:8080/root/api/relative/path")] + [TestCase("abc://host.com:8080/root/api/", "/relative/path", "abc://host.com:8080/relative/path")] + [TestCase("abc://host.com:8080/root/api/", "//otherhost.com/path", "abc://otherhost.com/path")] + public void should_combine_uri(string basePath, string relativePath, string expected) { - return new HttpUri("http", "localhost", 8989, basePath, null, null); + var newUri = new HttpUri(basePath) + new HttpUri(relativePath); + newUri.FullUri.Should().Be(expected); } [TestCase("", "", "")] - [TestCase("base", "", "/base")] + [TestCase("/", "", "/")] + [TestCase("base", "", "base")] [TestCase("/base", "", "/base")] - [TestCase("", "relative", "/relative")] + [TestCase("/base/", "", "/base/")] + [TestCase("", "relative", "relative")] [TestCase("", "/relative", "/relative")] - [TestCase("base", "relative", "/base/relative")] - [TestCase("base", "/relative", "/base/relative")] + [TestCase("/", "relative", "/relative")] + [TestCase("/", "/relative", "/relative")] + [TestCase("base", "relative", "base/relative")] + [TestCase("base", "/relative", "base/relative")] [TestCase("/base", "relative", "/base/relative")] [TestCase("/base", "/relative", "/base/relative")] - public void should_combine_base_path_and_relative_path(string basePath, string relativePath, string expected) + [TestCase("/base/", "relative", "/base/relative")] + [TestCase("/base/", "/relative", "/base/relative")] + [TestCase("base/sub", "relative", "base/sub/relative")] + [TestCase("base/sub", "/relative", "base/sub/relative")] + [TestCase("/base/sub", "relative", "/base/sub/relative")] + [TestCase("/base/sub", "/relative", "/base/sub/relative")] + [TestCase("/base/sub/", "relative", "/base/sub/relative")] + [TestCase("/base/sub/", "/relative", "/base/sub/relative")] + [TestCase("/base/sub/", "relative/", "/base/sub/relative/")] + [TestCase("/base/sub/", "/relative/", "/base/sub/relative/")] + [TestCase("abc://host.com:8080/root/file.xml", "relative/path", "abc://host.com:8080/root/file.xml/relative/path")] + [TestCase("abc://host.com:8080/root/file.xml", "/relative/path", "abc://host.com:8080/root/file.xml/relative/path")] + [TestCase("abc://host.com:8080/root/file.xml?query=1#fragment", "relative/path", "abc://host.com:8080/root/file.xml/relative/path?query=1#fragment")] + [TestCase("abc://host.com:8080/root/file.xml?query=1#fragment", "/relative/path", "abc://host.com:8080/root/file.xml/relative/path?query=1#fragment")] + [TestCase("abc://host.com:8080/root/api", "relative/path", "abc://host.com:8080/root/api/relative/path")] + [TestCase("abc://host.com:8080/root/api", "/relative/path", "abc://host.com:8080/root/api/relative/path")] + [TestCase("abc://host.com:8080/root/api/", "relative/path", "abc://host.com:8080/root/api/relative/path")] + [TestCase("abc://host.com:8080/root/api/", "/relative/path", "abc://host.com:8080/root/api/relative/path")] + public void should_combine_relative_path(string basePath, string relativePath, string expected) { - GivenHttpUri(basePath).CombinePath(relativePath).Path.Should().Be(expected); + var newUri = new HttpUri(basePath).CombinePath(relativePath); + + newUri.FullUri.Should().Be(expected); } } } diff --git a/src/NzbDrone.Common/Http/HttpUri.cs b/src/NzbDrone.Common/Http/HttpUri.cs index 05d12e7d8..a52a06137 100644 --- a/src/NzbDrone.Common/Http/HttpUri.cs +++ b/src/NzbDrone.Common/Http/HttpUri.cs @@ -131,7 +131,7 @@ namespace NzbDrone.Common.Http return _queryParams; } } - + public HttpUri CombinePath(string path) { return new HttpUri(Scheme, Host, Port, CombinePath(Path, path), Query, Fragment); @@ -146,10 +146,32 @@ namespace NzbDrone.Common.Http if (basePath.IsNullOrWhiteSpace()) { - return "/" + relativePath.TrimStart('/'); + return relativePath; } - return basePath.TrimEnd("/") + "/" + relativePath.TrimStart('/'); + return basePath.TrimEnd('/') + "/" + relativePath.TrimStart('/'); + } + + private static string CombineRelativePath(string basePath, string relativePath) + { + if (relativePath.IsNullOrWhiteSpace()) + { + return basePath; + } + + if (relativePath.StartsWith("/")) + { + return relativePath; + } + + var baseSlashIndex = basePath.LastIndexOf('/'); + + if (baseSlashIndex >= 0) + { + return basePath.Substring(0, baseSlashIndex) + "/" + relativePath; + } + + return relativePath; } public HttpUri SetQuery(string query) @@ -188,6 +210,7 @@ namespace NzbDrone.Common.Http return SetQuery(builder.ToString()); } + public override int GetHashCode() { return _uri.GetHashCode(); @@ -240,7 +263,7 @@ namespace NzbDrone.Common.Http if (relativeUrl.Path.IsNotNullOrWhiteSpace()) { - return new HttpUri(baseUrl.Scheme, baseUrl.Host, baseUrl.Port, HttpUri.CombinePath(baseUrl.Path, relativeUrl.Path), relativeUrl.Query, relativeUrl.Fragment); + return new HttpUri(baseUrl.Scheme, baseUrl.Host, baseUrl.Port, CombineRelativePath(baseUrl.Path, relativeUrl.Path), relativeUrl.Query, relativeUrl.Fragment); } return new HttpUri(baseUrl.Scheme, baseUrl.Host, baseUrl.Port, baseUrl.Path, relativeUrl.Query, relativeUrl.Fragment);