Merge #2614 Encapsulate usages of WebClient

Cmdline/Action/Repo.cs

@@ -160,14 +160,12 @@ public int RunSubCommand(KSPManager manager, CommonOptions opts, SubCommandOptio
 
         private static RepositoryList FetchMasterRepositoryList(Uri master_uri = null)
         {
-            WebClient client = new WebClient();
-
             if (master_uri == null)
             {
                 master_uri = Repository.default_repo_master_list;
             }
 
-            string json = client.DownloadString(master_uri);
+            string json = Net.DownloadText(master_uri);
             return JsonConvert.DeserializeObject<RepositoryList>(json);
         }
 

Core/Net/Net.cs

@@ -170,7 +170,7 @@ public static void DownloadWithProgress(ICollection<DownloadTarget> downloadTarg
             }.DownloadAndWait(downloadTargets);
         }
 
-        public static string DownloadText(Uri url)
+        public static string DownloadText(Uri url, string authToken = "")
         {
             log.DebugFormat("About to download {0}", url.OriginalString);
 
@@ -179,13 +179,13 @@ public static string DownloadText(Uri url)
                 WebClient agent = MakeDefaultHttpClient();
 
                 // Check whether to use an auth token for this host
-                string token;
-                if (Win32Registry.TryGetAuthToken(url.Host, out token)
-                        && !string.IsNullOrEmpty(token))
+                if (!string.IsNullOrEmpty(authToken)
+                    || (Win32Registry.TryGetAuthToken(url.Host, out authToken)
+                        && !string.IsNullOrEmpty(authToken)))
                 {
                     log.InfoFormat("Using auth token for {0}", url.Host);
                     // Send our auth token to the GitHub API (or whoever else needs one)
-                    agent.Headers.Add("Authorization", $"token {token}");
+                    agent.Headers.Add("Authorization", $"token {authToken}");
                 }
 
                 return agent.DownloadString(url.OriginalString);

Core/Types/Repository.cs

@@ -53,7 +53,7 @@ public static RepositoryList DefaultRepositories()
             try
             {
                 return JsonConvert.DeserializeObject<RepositoryList>(
-                    new WebClient().DownloadString(Repository.default_repo_master_list)
+                    Net.DownloadText(Repository.default_repo_master_list)
                 );
             }
             catch

GUI/MainRepo.cs

@@ -15,14 +15,12 @@ public partial class Main
 
         public static RepositoryList FetchMasterRepositoryList(Uri master_uri = null)
         {
-            WebClient client = new WebClient();
-
             if (master_uri == null)
             {
                 master_uri = Repository.default_repo_master_list;
             }
 
-            string json = client.DownloadString(master_uri);
+            string json = Net.DownloadText(master_uri);
             return JsonConvert.DeserializeObject<RepositoryList>(json);
         }
 

Netkan/Services/CachingHttpService.cs

@@ -78,6 +78,10 @@ public string DownloadText(Uri url)
         {
             return Net.DownloadText(url);
         }
+        public string DownloadText(Uri url, string authToken)
+        {
+            return Net.DownloadText(url, authToken);
+        }
 
         public IEnumerable<Uri> RequestedURLs { get { return _requestedURLs; } }
 

Netkan/Services/IHttpService.cs

@@ -7,6 +7,7 @@ internal interface IHttpService
     {
         string DownloadPackage(Uri url, string identifier, DateTime? updated);
         string DownloadText(Uri url);
+        string DownloadText(Uri url, string authToken);
 
         IEnumerable<Uri> RequestedURLs { get; }
     }

Netkan/Sources/Github/GithubApi.cs

@@ -1,10 +1,11 @@
 using System;
 using System.Linq;
 using System.Net;
-using CKAN.Versioning;
 using log4net;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
+using CKAN.Versioning;
+using CKAN.NetKAN.Services;
 
 // We could use OctoKit for this, but since we're only pinging the
 // release API, I'm happy enough without yet another dependency.
@@ -16,21 +17,25 @@ internal sealed class GithubApi : IGithubApi
         private static readonly ILog Log = LogManager.GetLogger(typeof(GithubApi));
         private static readonly Uri ApiBase = new Uri("https://api.github.com/");
 
-        private readonly string _oauthToken;
+        private readonly IHttpService _http;
+        private readonly string       _oauthToken;
 
-        public GithubApi(string oauthToken = null)
+        public GithubApi(IHttpService http, string oauthToken = null)
         {
+            _http       = http;
             _oauthToken = oauthToken;
         }
 
         public GithubRepo GetRepo(GithubRef reference)
         {
-            return JsonConvert.DeserializeObject<GithubRepo>(Call(string.Format("repos/{0}", reference.Repository)));
+            return JsonConvert.DeserializeObject<GithubRepo>(
+                Call($"repos/{reference.Repository}")
+            );
         }
 
         public GithubRelease GetLatestRelease(GithubRef reference)
         {
-            var json = Call(string.Format("repos/{0}/releases", reference.Repository));
+            var json = Call($"repos/{reference.Repository}/releases");
             Log.Debug("Parsing JSON...");
             var releases = JArray.Parse(json);
 
@@ -86,20 +91,12 @@ public GithubRelease GetLatestRelease(GithubRef reference)
 
         private string Call(string path)
         {
-            var web = new WebClient();
-            web.Headers.Add("User-Agent", Net.UserAgentString);
-
-            if (_oauthToken != null)
-            {
-                web.Headers.Add("Authorization", string.Format("token {0}", _oauthToken));
-            }
-
             var url = new Uri(ApiBase, path);
             Log.DebugFormat("Calling {0}", url);
 
             try
             {
-                return web.DownloadString(url);
+                return _http.DownloadText(url, _oauthToken);
             }
             catch (WebException webEx)
             {

Netkan/Transformers/NetkanTransformer.cs

@@ -31,7 +31,7 @@ bool prerelease
                 new MetaNetkanTransformer(http),
                 new SpacedockTransformer(new SpacedockApi(http)),
                 new CurseTransformer(new CurseApi(http)),
-                new GithubTransformer(new GithubApi(githubToken), prerelease),
+                new GithubTransformer(new GithubApi(http, githubToken), prerelease),
                 new HttpTransformer(),
                 new JenkinsTransformer(new JenkinsApi(http)),
                 new AvcKrefTransformer(http),

Tests/NetKAN/Sources/Github/GithubApiTests.cs

@@ -1,5 +1,9 @@
-using CKAN.NetKAN.Sources.Github;
+using System;
+using System.IO;
 using NUnit.Framework;
+using CKAN;
+using CKAN.NetKAN.Services;
+using CKAN.NetKAN.Sources.Github;
 
 namespace Tests.NetKAN.Sources.Github
 {
@@ -9,14 +13,33 @@ public sealed class GithubApiTests
         // Ironically, despite the fact that these run on travis-ci, which is strongly integrated
         // to github, these sometimes cause test failures because github will throw random
         // 403s. (Hence we disable them in travis with --exclude=FlakyNetwork)
-
+
+        private string       _cachePath;
+        private NetFileCache _cache;
+
+        [OneTimeSetUp]
+        public void TestFixtureSetup()
+        {
+            _cachePath = Path.Combine(Path.GetTempPath(), "CKAN", Guid.NewGuid().ToString("N"));
+
+            Directory.CreateDirectory(_cachePath);
+
+            _cache = new NetFileCache(_cachePath);
+        }
+
+        [OneTimeTearDown]
+        public void TestFixtureTearDown()
+        {
+            Directory.Delete(_cachePath, recursive: true);
+        }
+
         [Test]
         [Category("FlakyNetwork")]
         [Category("Online")]
         public void GetsLatestReleaseCorrectly()
         {
             // Arrange
-            var sut = new GithubApi();
+            var sut = new GithubApi(new CachingHttpService(_cache));
 
             // Act
             var githubRelease = sut.GetLatestRelease(new GithubRef("#/ckan/github/KSP-CKAN/Test", false, false));