Merge pull request #467 from octokit/encoding-fails-some-tests

Uri.EscapeDataString impacts search tests

Octokit.Tests/Clients/OauthClientTests.cs

@@ -42,8 +42,7 @@ public class OauthClientTests
 
             var result = client.GetGitHubLoginUrl(request);
 
-            const string expected = "https://github.com/login/oauth/authorize?client_id=secret&redirect_uri=https://example.com/foo?foo=bar&scope=foo,bar&state=canARY";
-            Assert.Equal(expected, result.ToString());
+            Assert.Equal("/login/oauth/authorize", result.AbsolutePath);
             Assert.Equal("?client_id=secret&redirect_uri=https%3A%2F%2Fexample.com%2Ffoo%3Ffoo%3Dbar&scope=foo%2Cbar&state=canARY", result.Query);
         }
     }

Octokit.Tests/Helpers/UriExtensionsTests.cs

@@ -22,6 +22,19 @@ namespace Octokit.Tests.Helpers
                 Assert.Equal(new Uri("https://example.com?foo=foo%20val&bar=barval"), uriWithParameters);
             }
 
+            [Fact]
+            public void ThrowsExceptionWhenNullValueProvided()
+            {
+                var uri = new Uri("https://example.com");
+
+                var parameters = new Dictionary<string, string>
+                {
+                    {"foo", null },
+                };
+
+                Assert.Throws<ArgumentNullException>(() => uri.ApplyParameters(parameters));
+            }
+
             [Fact]
             public void AppendsParametersAsQueryStringToRelativeUri()
             {

Octokit.Tests/Models/SearchCodeRequestTests.cs

@@ -1,4 +1,5 @@
-using Octokit;
+using System;
+using Octokit;
 using Octokit.Tests.Helpers;
 using Xunit;
 
@@ -16,5 +17,13 @@ public class SearchCodeRequestTests
             // If I can cast this to a writeable collection, then that defeats the purpose of a read only.
             AssertEx.IsReadOnlyCollection<string>(result);
         }
+
+        [Fact]
+        public void SortNotSpecifiedByDefault()
+        {
+            var request = new SearchCodeRequest("test");
+            Assert.True(String.IsNullOrWhiteSpace(request.Sort));
+            Assert.False(request.Parameters.ContainsKey("sort"));
+        }
     }
 }

Octokit.Tests/Models/SearchIssuesRequestTests.cs

@@ -1,4 +1,5 @@
-using Octokit;
+using System;
+using Octokit;
 using Octokit.Tests.Helpers;
 using Xunit;
 
@@ -16,5 +17,13 @@ internal class SearchIssuesRequestTests
             // If I can cast this to a writeable collection, then that defeats the purpose of a read only.
             AssertEx.IsReadOnlyCollection<string>(result);
         }
+
+        [Fact]
+        public void SortNotSpecifiedByDefault()
+        {
+            var request = new SearchIssuesRequest("test");
+            Assert.True(String.IsNullOrWhiteSpace(request.Sort));
+            Assert.False(request.Parameters.ContainsKey("sort"));
+        }
     }
 }

Octokit.Tests/Models/SearchUsersRequestTests.cs

@@ -1,4 +1,5 @@
-using Octokit;
+using System;
+using Octokit;
 using Octokit.Tests.Helpers;
 using Xunit;
 
@@ -16,5 +17,13 @@ internal class SearchUsersRequestTests
             // If I can cast this to a writeable collection, then that defeats the purpose of a read only.
             AssertEx.IsReadOnlyCollection<string>(result);
         }
+
+        [Fact]
+        public void SortNotSpecifiedByDefault()
+        {
+            var request = new SearchUsersRequest("shiftkey");
+            Assert.True(String.IsNullOrWhiteSpace(request.Sort));
+            Assert.False(request.Parameters.ContainsKey("sort"));
+        }
     }
 }

Octokit/Helpers/UriExtensions.cs

@@ -44,7 +44,13 @@ namespace Octokit
                 }
             }
 
-            string query = String.Join("&", p.Select(kvp => kvp.Key + "=" + Uri.EscapeDataString(kvp.Value)));
+            Func<string, string, string> mapValueFunc = (key, value) =>
+            {
+                if (key == "q") return value;
+                return Uri.EscapeDataString(value);
+            };
+
+            string query = String.Join("&", p.Select(kvp => kvp.Key + "=" + mapValueFunc(kvp.Key, kvp.Value)));
             if (uri.IsAbsoluteUri)
             {
                 var uriBuilder = new UriBuilder(uri)

Octokit/Models/Request/BaseSearchRequest.cs

@@ -83,7 +83,10 @@ namespace Octokit
                 var d = new Dictionary<string, string>();
                 d.Add("page", Page.ToString(CultureInfo.CurrentCulture));
                 d.Add("per_page", PerPage.ToString(CultureInfo.CurrentCulture));
-                d.Add("sort", Sort);
+                if (!String.IsNullOrWhiteSpace(Sort))
+                {
+                    d.Add("sort", Sort);
+                }
                 d.Add("order", SortOrder);
                 d.Add("q", TermAndQualifiers);
                 return d;