Merge pull request #190 from octokit/mind-your-data-structures

Bugfix - mind your data structures

Octokit.Tests/Helpers/UriExtensionsTests.cs

@@ -36,20 +36,6 @@ namespace Octokit.Tests.Helpers
                 Assert.Equal(new Uri("issues?foo=fooval&bar=barval", UriKind.Relative), uriWithParameters);
             }
 
-            [Fact(Skip="I don't believe this test is valid")]
-            public void OverwritesExistingParameters()
-            {
-                var uri = new Uri("https://example.com?crap=crapola");
-
-                var uriWithParameters = uri.ApplyParameters(new Dictionary<string, string>
-                {
-                    {"foo", "fooval"},
-                    {"bar", "barval"}
-                });
-
-                Assert.Equal(new Uri("https://example.com?foo=fooval&bar=barval"), uriWithParameters);
-            }
-
             [Fact]
             public void DoesNotChangeUrlWhenParametersEmpty()
             {
@@ -77,6 +63,18 @@ namespace Octokit.Tests.Helpers
                 Assert.True(actual.Query.Contains("page=2"));
             }
 
+            [Fact]
+            public void DoesNotChangePassedInDictionary()
+            {
+                var uri = new Uri("https://api.github.com/repositories/1/milestones?state=closed&sort=due_date&direction=asc&page=2");
+
+                var parameters = new Dictionary<string, string> { { "state", "open" }, { "sort", "other" } };
+
+                uri.ApplyParameters(parameters);
+
+                Assert.Equal(2, parameters.Count);
+            }
+
             [Fact]
             public void EnsuresArgumentNotNull()
             {

Octokit/Helpers/UriExtensions.cs

@@ -12,6 +12,10 @@ namespace Octokit
 
             if (parameters == null || !parameters.Any()) return uri;
 
+            // to prevent values being persisted across requests
+            // use a temporary dictionary which combines new and existing parameters
+            IDictionary<string,string> p = new Dictionary<string, string>(parameters);
+
             string queryString;
             if (uri.IsAbsoluteUri)
             {
@@ -34,13 +38,13 @@ namespace Octokit
 
             foreach (var existing in existingParameters)
             {
-                if (!parameters.ContainsKey(existing.Key))
+                if (!p.ContainsKey(existing.Key))
                 {
-                    parameters.Add(existing);
+                    p.Add(existing);
                 }
             }
 
-            string query = String.Join("&", parameters.Select(kvp => kvp.Key + "=" + kvp.Value));
+            string query = String.Join("&", p.Select(kvp => kvp.Key + "=" + kvp.Value));
             if (uri.IsAbsoluteUri)
             {
                 var uriBuilder = new UriBuilder(uri)