Skip to content

Commit

Permalink
Merge pull request #611 from pjf/610_relationship_resolver
Browse files Browse the repository at this point in the history
Bugfix: Always use current KSP version when resolving relationships
  • Loading branch information
RichardLake committed Dec 17, 2014
2 parents 9884dc1 + e3ce310 commit bf2582f
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 47 deletions.
2 changes: 1 addition & 1 deletion CKAN/CKAN/ModuleInstaller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CkanModule> modsToInstall = resolver.ModList();
List<CkanModule> downloads = new List<CkanModule> ();

Expand Down
7 changes: 4 additions & 3 deletions CKAN/CKAN/Registry/Registry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ public List<CkanModule> Incompatible(KSPVersion ksp_version)

if (available == null)
{
incompatible.Add(LatestAvailable(candidate));
incompatible.Add(LatestAvailable(candidate, null));
}
}

Expand All @@ -422,7 +422,7 @@ public List<CkanModule> 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);

Expand All @@ -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.
/// </summary>
public List<CkanModule> LatestAvailableWithProvides(string module, KSPVersion ksp_version = null)
public List<CkanModule> LatestAvailableWithProvides(string module, KSPVersion ksp_version)
{
log.DebugFormat("Finding latest available with provides for {0}", module);

Expand Down
8 changes: 5 additions & 3 deletions CKAN/CKAN/Relationships/RelationshipResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ public class RelationshipResolver
private static readonly ILog log = LogManager.GetLogger(typeof(RelationshipResolver));
private readonly Dictionary<string, CkanModule> modlist = new Dictionary<string, CkanModule>();
private Registry registry;
private KSPVersion kspversion;

/// <summary>
/// Creates a new resolver that will find a way to install all the modules specified.
/// </summary>
public RelationshipResolver(ICollection<string> modules, RelationshipResolverOptions options, Registry registry)
public RelationshipResolver(ICollection<string> 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
Expand All @@ -56,7 +58,7 @@ public RelationshipResolver(ICollection<string> 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);
Expand Down Expand Up @@ -166,7 +168,7 @@ private void ResolveStanza(IEnumerable<RelationshipDescriptor> stanza, Relations
continue;
}

List<CkanModule> candidates = registry.LatestAvailableWithProvides(dep_name);
List<CkanModule> candidates = registry.LatestAvailableWithProvides(dep_name, kspversion);

if (candidates.Count == 0)
{
Expand Down
2 changes: 1 addition & 1 deletion CKAN/GUI/GUIMod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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() : "-";
Expand Down
2 changes: 1 addition & 1 deletion CKAN/GUI/MainInstall.cs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ private void UpdateRecommendedDialog(Dictionary<string, List<string>> mods, bool

try
{
module = RegistryManager.Instance(manager.CurrentInstance).registry.LatestAvailable(pair.Key);
module = RegistryManager.Instance(manager.CurrentInstance).registry.LatestAvailable(pair.Key, CurrentInstance.Version());
}
catch
{
Expand Down
4 changes: 2 additions & 2 deletions CKAN/GUI/MainModList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public List<KeyValuePair<CkanModule, GUIModChangeType>> ComputeChangeSetFromModL
RelationshipResolver resolver;
try
{
resolver = new RelationshipResolver(modulesToInstall.ToList(), options, registry);
resolver = new RelationshipResolver(modulesToInstall.ToList(), options, registry, current_instance.Version());
}
catch (Exception)
{
Expand All @@ -202,7 +202,7 @@ public List<KeyValuePair<CkanModule, GUIModChangeType>> 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<CkanModule, GUIModChangeType>(mod, GUIModChangeType.Remove)));
}
Expand Down
12 changes: 6 additions & 6 deletions CKAN/Tests/CKAN/Registry/Registry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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<ModuleNotFoundKraken>(delegate
{
registry.LatestAvailable(identifier);
registry.LatestAvailable(identifier,null);
});
}

Expand Down
31 changes: 19 additions & 12 deletions CKAN/Tests/CKAN/Relationships/RelationshipResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public void Constructor_WithoutModules_AlwaysReturns()
options = RelationshipResolver.DefaultOpts();
Assert.DoesNotThrow(() => new RelationshipResolver(new List<string>(),
options,
registry));
registry,
null));
}

[Test]
Expand All @@ -49,7 +50,8 @@ public void Constructor_WithConflictingModules_Throws()
Assert.Throws<InconsistentKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));
}

[Test]
Expand Down Expand Up @@ -77,7 +79,8 @@ public void Constructor_WithMultipleModulesProviding_Throws()
Assert.Throws<TooManyModsProvideKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));

}

Expand All @@ -91,7 +94,8 @@ public void Constructor_WithMissingModules_Throws()
Assert.Throws<ModuleNotFoundKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));

}

Expand All @@ -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());
}

Expand All @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -177,7 +181,8 @@ public void Constructor_WithConflictingModulesInDependancies_ThrowUnderDefaultSe
Assert.Throws<InconsistentKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));
}
[Test]
public void Constructor_WithSuggests_HasSugestedInModlist()
Expand All @@ -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);
}

Expand All @@ -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<CkanModule>
{
Expand All @@ -238,7 +243,8 @@ public void Constructor_WithMissingDependants_Throws()
Assert.Throws<ModuleNotFoundKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));

}

Expand All @@ -255,7 +261,8 @@ public void Constructor_WithRegisteryThatHasRequiredModuleRemoved_Throws()
Assert.Throws<ModuleNotFoundKraken>(() => new RelationshipResolver(
list,
options,
registry));
registry,
null));

}

Expand Down
36 changes: 18 additions & 18 deletions CKAN/Tests/CKAN/Relationships/SanityChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void Void()
public void DogeCoin()
{
// Test with a module that depends and conflicts with nothing.
var mods = new List<CkanModule> {registry.LatestAvailable("DogeCoinFlag")};
var mods = new List<CkanModule> {registry.LatestAvailable("DogeCoinFlag",null)};

Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "DogeCoinFlag");
}
Expand All @@ -49,13 +49,13 @@ public void CustomBiomes()
{
var mods = new List<CkanModule>();

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");
}

Expand All @@ -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<CKAN.Module> { registry.LatestAvailable("SRL") };
var mods = new List<CKAN.Module> { registry.LatestAvailable("SRL",null) };
var dlls = new List<string> { "QuickRevert" };

Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "SRL can be installed by itself");
Expand All @@ -92,9 +92,9 @@ public void ModulesToProvides()
{
var mods = new List<CKAN.Module>
{
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);
Expand All @@ -112,13 +112,13 @@ public void FindUnmetDependencies()
var dlls = new List<string>();
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");
Expand All @@ -132,14 +132,14 @@ public void ReverseDepends()
{
var mods = new List<CKAN.Module>
{
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<string>();
Expand Down

0 comments on commit bf2582f

Please sign in to comment.