From 5b79ed4c18df729e460886b433f328635c2f6e66 Mon Sep 17 00:00:00 2001 From: Paul Fenwick Date: Wed, 17 Dec 2014 13:28:21 +1100 Subject: [PATCH 1/2] Relationship resolver now uses KSP version in resolution. `LatestAvailableWithProvides` now requires a mandatory KSP version, so we don't accidentally do this again. --- CKAN/CKAN/ModuleInstaller.cs | 2 +- CKAN/CKAN/Registry/Registry.cs | 3 +- .../Relationships/RelationshipResolver.cs | 8 +++-- CKAN/GUI/MainModList.cs | 2 +- .../Relationships/RelationshipResolver.cs | 31 ++++++++++++------- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/CKAN/CKAN/ModuleInstaller.cs b/CKAN/CKAN/ModuleInstaller.cs index 4430114b00..a5fe07d9cf 100644 --- a/CKAN/CKAN/ModuleInstaller.cs +++ b/CKAN/CKAN/ModuleInstaller.cs @@ -144,7 +144,7 @@ public void InstallList( NetAsyncDownloader downloader = null ) { - var resolver = new RelationshipResolver(modules, options, registry_manager.registry); + var resolver = new RelationshipResolver(modules, options, registry_manager.registry, ksp.Version()); List modsToInstall = resolver.ModList(); List downloads = new List (); diff --git a/CKAN/CKAN/Registry/Registry.cs b/CKAN/CKAN/Registry/Registry.cs index 2071e4be2d..4c8f84e966 100644 --- a/CKAN/CKAN/Registry/Registry.cs +++ b/CKAN/CKAN/Registry/Registry.cs @@ -443,8 +443,9 @@ public CkanModule LatestAvailable(string module, KSPVersion ksp_version = null) /// satisifes the specified version. Takes into account module 'provides', /// which may result in a list of alternatives being provided. /// Returns an empty list if nothing is available for our system, which includes if no such module exists. + /// If no KSP version is provided, the latest module for *any* KSP version is given. /// - public List LatestAvailableWithProvides(string module, KSPVersion ksp_version = null) + public List LatestAvailableWithProvides(string module, KSPVersion ksp_version) { log.DebugFormat("Finding latest available with provides for {0}", module); diff --git a/CKAN/CKAN/Relationships/RelationshipResolver.cs b/CKAN/CKAN/Relationships/RelationshipResolver.cs index e47a8afc87..e2909fcadb 100644 --- a/CKAN/CKAN/Relationships/RelationshipResolver.cs +++ b/CKAN/CKAN/Relationships/RelationshipResolver.cs @@ -35,14 +35,16 @@ public class RelationshipResolver private static readonly ILog log = LogManager.GetLogger(typeof(RelationshipResolver)); private readonly Dictionary modlist = new Dictionary(); private Registry registry; + private KSPVersion kspversion; /// /// Creates a new resolver that will find a way to install all the modules specified. /// - public RelationshipResolver(ICollection modules, RelationshipResolverOptions options, Registry registry) + public RelationshipResolver(ICollection modules, RelationshipResolverOptions options, Registry registry, KSPVersion kspversion) { this.registry = registry; + this.kspversion = kspversion; // Start by figuring out what versions we're installing, and then // adding them to the list. This *must* be pre-populated with all @@ -56,7 +58,7 @@ public RelationshipResolver(ICollection modules, RelationshipResolverOpt foreach (string module in modules) { - CkanModule mod = registry.LatestAvailable(module); + CkanModule mod = registry.LatestAvailable(module, kspversion); if (mod == null) { throw new ModuleNotFoundKraken(module); @@ -166,7 +168,7 @@ private void ResolveStanza(IEnumerable stanza, Relations continue; } - List candidates = registry.LatestAvailableWithProvides(dep_name); + List candidates = registry.LatestAvailableWithProvides(dep_name, kspversion); if (candidates.Count == 0) { diff --git a/CKAN/GUI/MainModList.cs b/CKAN/GUI/MainModList.cs index 924a618a40..4580c0d66f 100644 --- a/CKAN/GUI/MainModList.cs +++ b/CKAN/GUI/MainModList.cs @@ -184,7 +184,7 @@ public List> ComputeChangeSetFromModL RelationshipResolver resolver; try { - resolver = new RelationshipResolver(modulesToInstall.ToList(), options, registry); + resolver = new RelationshipResolver(modulesToInstall.ToList(), options, registry, current_instance.Version()); } catch (Exception) { diff --git a/CKAN/Tests/CKAN/Relationships/RelationshipResolver.cs b/CKAN/Tests/CKAN/Relationships/RelationshipResolver.cs index 56085d6f11..221b06ae92 100644 --- a/CKAN/Tests/CKAN/Relationships/RelationshipResolver.cs +++ b/CKAN/Tests/CKAN/Relationships/RelationshipResolver.cs @@ -29,7 +29,8 @@ public void Constructor_WithoutModules_AlwaysReturns() options = RelationshipResolver.DefaultOpts(); Assert.DoesNotThrow(() => new RelationshipResolver(new List(), options, - registry)); + registry, + null)); } [Test] @@ -49,7 +50,8 @@ public void Constructor_WithConflictingModules_Throws() Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } [Test] @@ -77,7 +79,8 @@ public void Constructor_WithMultipleModulesProviding_Throws() Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } @@ -91,7 +94,8 @@ public void Constructor_WithMissingModules_Throws() Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } @@ -109,7 +113,7 @@ public void ModList_WithInstalledModules_DoesNotContainThem() AddToRegistry(mod_a); registry.Installed().Add(mod_a.identifier, mod_a.version); - var relationship_resolver = new RelationshipResolver(list, options, registry); + var relationship_resolver = new RelationshipResolver(list, options, registry, null); CollectionAssert.IsEmpty(relationship_resolver.ModList()); } @@ -128,7 +132,7 @@ public void ModList_WithInstalledModulesSugested_DoesNotContainThem() AddToRegistry(sugester, sugested); registry.Installed().Add(sugested.identifier, sugested.version); - var relationship_resolver = new RelationshipResolver(list, options, registry); + var relationship_resolver = new RelationshipResolver(list, options, registry, null); CollectionAssert.Contains(relationship_resolver.ModList(), sugested); } @@ -151,7 +155,7 @@ public void ModList_WithSugestedModulesThatWouldConflict_DoesNotContainThem() list.Add(mod.identifier); AddToRegistry(sugester, sugested, mod); - var relationship_resolver = new RelationshipResolver(list, options, registry); + var relationship_resolver = new RelationshipResolver(list, options, registry, null); CollectionAssert.DoesNotContain(relationship_resolver.ModList(), sugested); } @@ -177,7 +181,8 @@ public void Constructor_WithConflictingModulesInDependancies_ThrowUnderDefaultSe Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } [Test] public void Constructor_WithSuggests_HasSugestedInModlist() @@ -193,7 +198,7 @@ public void Constructor_WithSuggests_HasSugestedInModlist() list.Add(sugester.identifier); AddToRegistry(sugester, sugested); - var relationship_resolver = new RelationshipResolver(list, options, registry); + var relationship_resolver = new RelationshipResolver(list, options, registry, null); CollectionAssert.Contains(relationship_resolver.ModList(), sugested); } @@ -212,7 +217,7 @@ public void Constructor_ProvidesSatisfyDependencies() }); list.Add(depender.identifier); AddToRegistry(mod_b, depender); - var relationship_resolver = new RelationshipResolver(list, options, registry); + var relationship_resolver = new RelationshipResolver(list, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List { @@ -238,7 +243,8 @@ public void Constructor_WithMissingDependants_Throws() Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } @@ -255,7 +261,8 @@ public void Constructor_WithRegisteryThatHasRequiredModuleRemoved_Throws() Assert.Throws(() => new RelationshipResolver( list, options, - registry)); + registry, + null)); } From e3ce3100484270f0c5abefb3333631e14811c448 Mon Sep 17 00:00:00 2001 From: Paul Fenwick Date: Wed, 17 Dec 2014 13:43:01 +1100 Subject: [PATCH 2/2] CKAN.dll: LatestAvailable now requires a version number This means we can never go looking up mods which may be the wrong version for our install by accident. It may also fix some edge cases like #602 where we may show the wrong versions being available. Most of the changes in this commit are to the test cases, which now require the mandatory argument. :) Tested that this closes #610 with our NEAR test example. --- CKAN/CKAN/Registry/Registry.cs | 4 +-- CKAN/GUI/GUIMod.cs | 2 +- CKAN/GUI/MainInstall.cs | 2 +- CKAN/GUI/MainModList.cs | 2 +- CKAN/Tests/CKAN/Registry/Registry.cs | 12 +++---- .../Tests/CKAN/Relationships/SanityChecker.cs | 36 +++++++++---------- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/CKAN/CKAN/Registry/Registry.cs b/CKAN/CKAN/Registry/Registry.cs index 4c8f84e966..9dcf8b50e7 100644 --- a/CKAN/CKAN/Registry/Registry.cs +++ b/CKAN/CKAN/Registry/Registry.cs @@ -404,7 +404,7 @@ public List Incompatible(KSPVersion ksp_version) if (available == null) { - incompatible.Add(LatestAvailable(candidate)); + incompatible.Add(LatestAvailable(candidate, null)); } } @@ -422,7 +422,7 @@ public List Incompatible(KSPVersion ksp_version) // TODO: Consider making this internal, because practically everything should // be calling LatestAvailableWithProvides() - public CkanModule LatestAvailable(string module, KSPVersion ksp_version = null) + public CkanModule LatestAvailable(string module, KSPVersion ksp_version) { log.DebugFormat("Finding latest available for {0}", module); diff --git a/CKAN/GUI/GUIMod.cs b/CKAN/GUI/GUIMod.cs index 7bf38fd73b..09a914e124 100644 --- a/CKAN/GUI/GUIMod.cs +++ b/CKAN/GUI/GUIMod.cs @@ -34,7 +34,7 @@ public GUIMod(CkanModule mod, Registry registry, KSPVersion current_ksp_version) Authors = mod.author == null ? "N/A" : String.Join(",", mod.author); var installedVersion = registry.InstalledVersion(mod.identifier); - var latestVersion = registry.LatestAvailable(mod.identifier); + var latestVersion = registry.LatestAvailable(mod.identifier, current_ksp_version); var kspVersion = mod.ksp_version; InstalledVersion = installedVersion != null ? installedVersion.ToString() : "-"; diff --git a/CKAN/GUI/MainInstall.cs b/CKAN/GUI/MainInstall.cs index f84869fb91..1551764f21 100644 --- a/CKAN/GUI/MainInstall.cs +++ b/CKAN/GUI/MainInstall.cs @@ -500,7 +500,7 @@ private void UpdateRecommendedDialog(Dictionary> mods, bool try { - module = RegistryManager.Instance(manager.CurrentInstance).registry.LatestAvailable(pair.Key); + module = RegistryManager.Instance(manager.CurrentInstance).registry.LatestAvailable(pair.Key, CurrentInstance.Version()); } catch { diff --git a/CKAN/GUI/MainModList.cs b/CKAN/GUI/MainModList.cs index 4580c0d66f..71d161ea67 100644 --- a/CKAN/GUI/MainModList.cs +++ b/CKAN/GUI/MainModList.cs @@ -202,7 +202,7 @@ public List> ComputeChangeSetFromModL foreach (var reverseDependencies in modulesToRemove.Select(mod => installer.FindReverseDependencies(mod))) { //TODO This would be a good place to have a event that alters the row's graphics to show it will be removed - var modules = reverseDependencies.Select(rDep => registry.LatestAvailable(rDep)); + var modules = reverseDependencies.Select(rDep => registry.LatestAvailable(rDep, current_instance.Version())); changeset.UnionWith( modules.Select(mod => new KeyValuePair(mod, GUIModChangeType.Remove))); } diff --git a/CKAN/Tests/CKAN/Registry/Registry.cs b/CKAN/Tests/CKAN/Registry/Registry.cs index 363c866e34..093f4bcda1 100644 --- a/CKAN/Tests/CKAN/Registry/Registry.cs +++ b/CKAN/Tests/CKAN/Registry/Registry.cs @@ -100,10 +100,10 @@ public void TxEmbeddedCommit() { reg = CKAN.Registry.Empty(); reg.AddAvailable(module); - Assert.AreEqual(identifier, reg.LatestAvailable(identifier).identifier); + Assert.AreEqual(identifier, reg.LatestAvailable(identifier, null).identifier); scope.Complete(); } - Assert.AreEqual(identifier, reg.LatestAvailable(identifier).identifier); + Assert.AreEqual(identifier, reg.LatestAvailable(identifier, null).identifier); } [Test] @@ -114,11 +114,11 @@ public void TxCommit() using (var scope = new TransactionScope()) { registry.AddAvailable(module); - Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier).identifier); + Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier, null).identifier); scope.Complete(); } - Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier).identifier); + Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier, null).identifier); } [Test] @@ -129,14 +129,14 @@ public void TxRollback() using (var scope = new TransactionScope()) { registry.AddAvailable(module); - Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier).identifier); + Assert.AreEqual(module.identifier, registry.LatestAvailable(identifier,null).identifier); scope.Dispose(); // Rollback, our module should no longer be available. } Assert.Throws(delegate { - registry.LatestAvailable(identifier); + registry.LatestAvailable(identifier,null); }); } diff --git a/CKAN/Tests/CKAN/Relationships/SanityChecker.cs b/CKAN/Tests/CKAN/Relationships/SanityChecker.cs index 6c736f6570..f102cf0ea9 100644 --- a/CKAN/Tests/CKAN/Relationships/SanityChecker.cs +++ b/CKAN/Tests/CKAN/Relationships/SanityChecker.cs @@ -39,7 +39,7 @@ public void Void() public void DogeCoin() { // Test with a module that depends and conflicts with nothing. - var mods = new List {registry.LatestAvailable("DogeCoinFlag")}; + var mods = new List {registry.LatestAvailable("DogeCoinFlag",null)}; Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "DogeCoinFlag"); } @@ -49,13 +49,13 @@ public void CustomBiomes() { var mods = new List(); - mods.Add(registry.LatestAvailable("CustomBiomes")); + mods.Add(registry.LatestAvailable("CustomBiomes",null)); Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes without data"); - mods.Add(registry.LatestAvailable("CustomBiomesKerbal")); + mods.Add(registry.LatestAvailable("CustomBiomesKerbal",null)); Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes with stock data"); - mods.Add(registry.LatestAvailable("CustomBiomesRSS")); + mods.Add(registry.LatestAvailable("CustomBiomesRSS",null)); Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes with conflicting data"); } @@ -70,17 +70,17 @@ public void CustomBiomesWithDlls() // This would actually be a terrible thing for users to have, but it tests the // relationship we want. - mods.Add(registry.LatestAvailable("CustomBiomesKerbal")); + mods.Add(registry.LatestAvailable("CustomBiomesKerbal",null)); Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls), "CustomBiomes DLL, with config added"); - mods.Add(registry.LatestAvailable("CustomBiomesRSS")); + mods.Add(registry.LatestAvailable("CustomBiomesRSS",null)); Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls), "CustomBiomes with conflicting data"); } [Test] public void ConflictWithDll() { - var mods = new List { registry.LatestAvailable("SRL") }; + var mods = new List { registry.LatestAvailable("SRL",null) }; var dlls = new List { "QuickRevert" }; Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "SRL can be installed by itself"); @@ -92,9 +92,9 @@ public void ModulesToProvides() { var mods = new List { - registry.LatestAvailable("CustomBiomes"), - registry.LatestAvailable("CustomBiomesKerbal"), - registry.LatestAvailable("DogeCoinFlag") + registry.LatestAvailable("CustomBiomes",null), + registry.LatestAvailable("CustomBiomesKerbal",null), + registry.LatestAvailable("DogeCoinFlag",null) }; var provides = CKAN.SanityChecker.ModulesToProvides(mods); @@ -112,13 +112,13 @@ public void FindUnmetDependencies() var dlls = new List(); Assert.IsEmpty(CKAN.SanityChecker.FindUnmetDependencies(mods, dlls), "Empty list"); - mods.Add(registry.LatestAvailable("DogeCoinFlag")); + mods.Add(registry.LatestAvailable("DogeCoinFlag",null)); Assert.IsEmpty(CKAN.SanityChecker.FindUnmetDependencies(mods, dlls), "DogeCoinFlag"); - mods.Add(registry.LatestAvailable("CustomBiomes")); + mods.Add(registry.LatestAvailable("CustomBiomes",null)); Assert.Contains("CustomBiomesData", CKAN.SanityChecker.FindUnmetDependencies(mods, dlls).Keys, "Missing CustomBiomesData"); - mods.Add(registry.LatestAvailable("CustomBiomesKerbal")); + mods.Add(registry.LatestAvailable("CustomBiomesKerbal",null)); Assert.IsEmpty(CKAN.SanityChecker.FindUnmetDependencies(mods, dlls), "CBD+CBK"); mods.RemoveAll(x => x.identifier == "CustomBiomes"); @@ -132,14 +132,14 @@ public void ReverseDepends() { var mods = new List { - registry.LatestAvailable("CustomBiomes"), - registry.LatestAvailable("CustomBiomesKerbal"), - registry.LatestAvailable("DogeCoinFlag") + registry.LatestAvailable("CustomBiomes",null), + registry.LatestAvailable("CustomBiomesKerbal",null), + registry.LatestAvailable("DogeCoinFlag",null) }; // Make sure some of our expectations regarding dependencies are correct. - Assert.Contains("CustomBiomes", registry.LatestAvailable("CustomBiomesKerbal").depends.Select(x => x.name).ToList()); - Assert.Contains("CustomBiomesData", registry.LatestAvailable("CustomBiomes").depends.Select(x => x.name).ToList()); + Assert.Contains("CustomBiomes", registry.LatestAvailable("CustomBiomesKerbal",null).depends.Select(x => x.name).ToList()); + Assert.Contains("CustomBiomesData", registry.LatestAvailable("CustomBiomes",null).depends.Select(x => x.name).ToList()); // Removing DCF should only remove itself. var to_remove = new List();