Fixed a bug where we could end up with lists of nulls.

This also means I can now install firespitter. :)
KSP-CKAN · Oct 31, 2014 · a81fc52 · a81fc52
1 parent ee505c4
commit a81fc52
Show file tree

Hide file tree

Showing 7 changed files with 138 additions and 3 deletions.
diff --git a/CKAN/CKAN/Types/JsonSingleOrArrayConverter.cs b/CKAN/CKAN/Types/JsonSingleOrArrayConverter.cs
@@ -24,6 +24,13 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
             {
                 return token.ToObject<List<T>>();
             }
+
+            // If the object is null, we'll return null. Otherwise end up with a list of null.
+            if (token.ToObject<T>() == null)
+            {
+                return null;
+            }
+
             return new List<T> { token.ToObject<T>() };
         }
 

diff --git a/CKAN/Tests/CKAN/ModuleInstallDescriptor.cs b/CKAN/Tests/CKAN/ModuleInstallDescriptor.cs
@@ -0,0 +1,45 @@
+using NUnit.Framework;
+using Tests;
+using System.Collections.Generic;
+
+namespace CKANTests
+{
+    [TestFixture()]
+    public class ModuleInstallDescriptor
+    {
+        [Test()]
+        // NOTE: I've *never* got these to fail. The problem I'm trying to reproduce
+        // seems to involve saving to the registry and back. It's now fixed in
+        // JsonSingleOrArrayConverter.cs, but these tests remain, because tests are good.
+        public void Null_Filters()
+        {
+            // We had a bug whereby we could end up with a filter list of a single null.
+            // Make sure that doesn't happen ever again.
+
+            // We want a module that doesn't specify filters.
+            CKAN.CkanModule mod = TestData.kOS_014_module();
+
+            test_filter(mod.install[0].filter, "kOS/filter");
+            test_filter(mod.install[0].filter_regexp, "kOS/filter_regexp");
+
+            // And Firespitter seems to trigger it.
+
+            CKAN.CkanModule firespitter = TestData.FireSpitterModule();
+
+            foreach (var stanza in firespitter.install)
+            {
+                test_filter(stanza.filter, "Firespitter/filter");
+                test_filter(stanza.filter_regexp, "Firespitter/filter_regexp");
+            }
+        }
+
+        private void test_filter(List<string> filter, string message)
+        {
+            if (filter != null)
+            {
+                Assert.IsFalse(filter.Contains(null), message);
+            }
+        }
+    }
+}
+
diff --git a/CKAN/Tests/CKAN/ModuleInstaller.cs b/CKAN/Tests/CKAN/ModuleInstaller.cs
@@ -18,12 +18,12 @@ public void GenerateDefaultInstall()
             string filename = Tests.TestData.DogeCoinFlagZip();
             using (var zipfile = new ZipFile(filename))
             {
-                ModuleInstallDescriptor stanza = CKAN.ModuleInstaller.GenerateDefaultInstall("DogeCoinFlag", zipfile);
+                CKAN.ModuleInstallDescriptor stanza = CKAN.ModuleInstaller.GenerateDefaultInstall("DogeCoinFlag", zipfile);
 
                 TestDogeCoinStanza(stanza);
 
                 // Same again, but screwing up the case (we see this *all the time*)
-                ModuleInstallDescriptor stanza2 = CKAN.ModuleInstaller.GenerateDefaultInstall("DogecoinFlag", zipfile);
+                CKAN.ModuleInstallDescriptor stanza2 = CKAN.ModuleInstaller.GenerateDefaultInstall("DogecoinFlag", zipfile);
 
                 TestDogeCoinStanza(stanza2);
 
@@ -173,7 +173,7 @@ private static string CopyDogeFromZip()
             return tmpfile;
         }
 
-        private void TestDogeCoinStanza(ModuleInstallDescriptor stanza)
+        private void TestDogeCoinStanza(CKAN.ModuleInstallDescriptor stanza)
         {
             Assert.AreEqual("GameData", stanza.install_to);
             Assert.AreEqual("DogeCoinFlag-1.01/GameData/DogeCoinFlag",stanza.file);

diff --git a/CKAN/Tests/TestData.cs b/CKAN/Tests/TestData.cs
@@ -177,6 +177,10 @@ public static string KS_CustomAsteroids_string()
             return File.ReadAllText(Path.Combine(DataDir(),"KS/CustomAsteroids.json"));
         }
 
+        public static CKAN.CkanModule FireSpitterModule()
+        {
+            return CKAN.CkanModule.FromFile(Path.Combine(DataDir(), "Firespitter-6.3.5.ckan"));
+        }
     }
 }
 
diff --git a/CKAN/Tests/Tests.csproj b/CKAN/Tests/Tests.csproj
@@ -55,6 +55,7 @@
     <Compile Include="CKAN\KSPManager.cs" />
     <Compile Include="NetKAN\KSMod.cs" />
     <Compile Include="NetKAN\GitHubTests.cs" />
+    <Compile Include="CKAN\ModuleInstallDescriptor.cs" />
   </ItemGroup>
   <Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
   <ItemGroup>

diff --git a/NetKAN/Firespitter.ckan b/NetKAN/Firespitter.ckan
@@ -0,0 +1,31 @@
+{
+    "spec_version"    : 1,
+    "identifier"      : "Firespitter",
+    "$kref"           : "#/ckan/kerbalstuff/63",
+    "license"         : "restricted",
+    "comment"         : "Our version really depends on FirespitterCore",
+    "ksp_version_min" : "0.24.2",
+    "resources" : {
+        "github" : {
+            "url" : "https://github.com/snjo/Firespitter"
+        }
+    },
+    "depends" : [ {
+        "name"        : "FirespitterCore",
+        "min_version" : "7.0.5398.27328"
+    } ],
+    "x_supports" : [ { "name" : "PartCatalog" } ],
+    "install"  : [
+        {
+            "file"       : "Firespitter",
+            "install_to" : "GameData",
+            "filter"     : "Firespitter.dll",
+            "comment"    : "The DLL is installed from FirespitterCore"
+        },
+        {
+            "file"       : "PartCatalog",
+            "install_to" : "GameData",
+            "comment"    : "Firespitter graphics for PartCatalog"
+        }
+    ]
+}
diff --git a/t/data/Firespitter-6.3.5.ckan b/t/data/Firespitter-6.3.5.ckan
@@ -0,0 +1,47 @@
+{
+  "spec_version": 1,
+  "identifier": "Firespitter",
+  "license": "restricted",
+  "comment": "Our version really depends on FirespitterCore",
+  "ksp_version_min": "0.24.2",
+  "resources": {
+    "github": {
+      "url": "https://github.com/snjo/Firespitter"
+    },
+    "kerbalstuff": {
+      "url": "https://kerbalstuff.com/mod/63/Firespitter"
+    },
+    "homepage": "http://forum.kerbalspaceprogram.com/showthread.php/24551-Firespitter"
+  },
+  "depends": [
+    {
+      "name": "FirespitterCore",
+      "min_version": "7.0.5398.27328"
+    }
+  ],
+  "x_supports": [
+    {
+      "name": "PartCatalog"
+    }
+  ],
+  "install": [
+    {
+      "file": "Firespitter",
+      "install_to": "GameData",
+      "filter": "Firespitter.dll",
+      "comment": "The DLL is installed from FirespitterCore"
+    },
+    {
+      "file": "PartCatalog",
+      "install_to": "GameData",
+      "comment": "Firespitter graphics for PartCatalog"
+    }
+  ],
+  "name": "Firespitter",
+  "abstract": "Propeller plane and helicopter parts",
+  "author": "Snjo",
+  "version": "6.3.5",
+  "download": "https://kerbalstuff.com/mod/63/Firespitter/download/6.3.5",
+  "x_generated_by": "netkan",
+  "download_size": 37457313
+}