Skip to content

Commit

Permalink
Fix duplicate key exception in CompatibilitySorter
Browse files Browse the repository at this point in the history
  • Loading branch information
HebaruSan committed Sep 16, 2023
1 parent c38c7a2 commit f0d3980
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 36 deletions.
50 changes: 27 additions & 23 deletions Core/Registry/CompatibilitySorter.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System.Collections.Generic;
using System.Linq;

using log4net;

using CKAN.Extensions;
using CKAN.Versioning;

Expand Down Expand Up @@ -31,7 +33,12 @@ public CompatibilitySorter(GameVersionCriteria crit
this.installed = installed;
this.dlls = dlls;
this.dlc = dlc;
PartitionModules(available, CompatibleProviders(crit, providers));
var merged = available.SelectMany(dict => dict)
.GroupBy(kvp => kvp.Key,
kvp => kvp.Value)
.ToDictionary(grp => grp.Key,
grp => AvailableModule.Merge(grp.ToList()));
PartitionModules(merged, CompatibleProviders(crit, providers));
}

/// <summary>
Expand Down Expand Up @@ -138,33 +145,30 @@ private Dictionary<string, HashSet<AvailableModule>> CompatibleProviders(
/// </summary>
/// <param name="available">All mods available from registry</param>
/// <param name="providers">Mapping from identifiers to mods providing those identifiers</param>
private void PartitionModules(IEnumerable<Dictionary<string, AvailableModule>> dicts,
private void PartitionModules(Dictionary<string, AvailableModule> available,
Dictionary<string, HashSet<AvailableModule>> providers)
{
log.Debug("Partitioning modules by compatibility");
foreach (var available in dicts)
// First get the ones that are trivially [in]compatible.
foreach (var kvp in available)
{
// First get the ones that are trivially [in]compatible.
foreach (var kvp in available)
if (kvp.Value.AllAvailable().All(m => !m.IsCompatible(CompatibleVersions)))
{
if (kvp.Value.AllAvailable().All(m => !m.IsCompatible(CompatibleVersions)))
{
// No versions compatible == incompatible
log.DebugFormat("Trivially incompatible: {0}", kvp.Key);
Incompatible.Add(kvp.Key, kvp.Value);
}
else if (kvp.Value.AllAvailable().All(m => m.depends == null))
{
// No dependencies == compatible
log.DebugFormat("Trivially compatible: {0}", kvp.Key);
Compatible.Add(kvp.Key, kvp.Value);
}
else
{
// Need to investigate this one more later
log.DebugFormat("Trivially indeterminate: {0}", kvp.Key);
Indeterminate.Add(kvp.Key, kvp.Value);
}
// No versions compatible == incompatible
log.DebugFormat("Trivially incompatible: {0}", kvp.Key);
Incompatible.Add(kvp.Key, kvp.Value);
}
else if (kvp.Value.AllAvailable().All(m => m.depends == null))
{
// No dependencies == compatible
log.DebugFormat("Trivially compatible: {0}", kvp.Key);
Compatible.Add(kvp.Key, kvp.Value);
}
else
{
// Need to investigate this one more later
log.DebugFormat("Trivially indeterminate: {0}", kvp.Key);
Indeterminate.Add(kvp.Key, kvp.Value);
}
}
log.Debug("Trivial mods done, moving on to indeterminates");
Expand Down
6 changes: 5 additions & 1 deletion Core/Registry/Registry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,11 @@ public CompatibilitySorter SetCompatibleVersion(GameVersionCriteria versCrit)
BuildProvidesIndex();
}
sorter = new CompatibilitySorter(
versCrit, repoDataMgr?.GetAllAvailDicts(repositories.Values),
versCrit,
repoDataMgr?.GetAllAvailDicts(
repositories.Values.OrderBy(r => r.priority)
// Break ties alphanumerically
.ThenBy(r => r.name)),
providers,
installed_modules, InstalledDlls.ToHashSet(), InstalledDlc);
}
Expand Down
15 changes: 6 additions & 9 deletions Core/Repositories/AvailableModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ internal void DeserialisationFixes(StreamingContext context)
Debug.Assert(module_version.Values.All(m => identifier.Equals(m.identifier)));
}

public static AvailableModule Merge(IList<AvailableModule> availMods)
=> availMods.Count == 1 ? availMods.First()
: new AvailableModule(availMods.First().identifier,
availMods.Reverse().SelectMany(am => am.AllAvailable()));

// The map of versions -> modules, that's what we're about!
// First element is the oldest version, last is the newest.
[JsonProperty]
Expand All @@ -71,7 +76,7 @@ private void OnError(StreamingContext context, ErrorContext errorContext)
/// <summary>
/// Record the given module version as being available.
/// </summary>
public void Add(CkanModule module)
private void Add(CkanModule module)
{
if (!module.identifier.Equals(identifier))
throw new ArgumentException(
Expand All @@ -81,14 +86,6 @@ public void Add(CkanModule module)
module_version[module.version] = module;
}

/// <summary>
/// Remove the given version from our list of available.
/// </summary>
public void Remove(ModuleVersion version)
{
module_version.Remove(version);
}

/// <summary>
/// Return the most recent release of a module with a optional ksp version to target and a RelationshipDescriptor to satisfy.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions Core/Repositories/RepositoryDataManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ private RepositoryData LoadRepoData(Repository repo, IProgress<long> progress)

private IEnumerable<RepositoryData> GetRepoDatas(IEnumerable<Repository> repos)
=> repos?.OrderBy(repo => repo.priority)
.ThenBy(repo => repo.name)
.Select(repo => GetRepoData(repo))
.Where(data => data != null)
?? Enumerable.Empty<RepositoryData>();
Expand Down
103 changes: 103 additions & 0 deletions Tests/Core/Registry/CompatibilitySorter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
using System.Collections.Generic;
using System.Linq;

using NUnit.Framework;

using CKAN;
using CKAN.Versioning;
using CKAN.Extensions;

using Tests.Data;

namespace Tests.Core.Registry
{
[TestFixture]
public class CompatibilitySorterTests
{
[Test,
TestCase(new string[]
{
@"{
""spec_version"": ""v1.4"",
""identifier"": ""kOS-EVA"",
""name"": ""kOS-EVA"",
""abstract"": ""Addon for kOS that allows controlling a kerbal while on EVA"",
""version"": ""0.2.0.0"",
""ksp_version_min"": ""1.8.0"",
""ksp_version_max"": ""1.12.99"",
""license"": ""GPL-3.0"",
""download"": ""https://github.com/"",
""depends"": [
{ ""name"": ""Harmony2"" }
]
}",
@"{
""spec_version"": ""v1.4"",
""identifier"": ""Harmony2"",
""name"": ""Harmony 2"",
""abstract"": ""A library for patching, replacing and decorating .NET and Mono methods during runtime"",
""version"": ""2.2.1.0"",
""ksp_version_min"": ""1.8.0"",
""ksp_version_max"": ""1.12.99"",
""license"": ""MIT"",
""download"": ""https://spacedockinfo/""
}",
},
new string[]
{
@"{
""spec_version"": ""v1.4"",
""identifier"": ""kOS-EVA"",
""name"": ""kOS-EVA"",
""abstract"": ""Addon for kOS that allows controlling a kerbal while on EVA"",
""version"": ""0.2.0.0"",
""ksp_version_min"": ""1.8.0"",
""ksp_version_max"": ""1.12.99"",
""license"": ""GPL-3.0"",
""download"": ""https://github.com/""
}",
},
"kOS-EVA"),
]
public void Constructor_OverlappingModules_HigherPriorityOverrides(string[] modules1,
string[] modules2,
string identifier)
{
// Arrange
var user = new NullUser();
using (var repo1 = new TemporaryRepository(0, modules1))
using (var repo2 = new TemporaryRepository(1, modules2))
using (var repoData = new TemporaryRepositoryData(user, repo1.repo,
repo2.repo))
{
var versCrit = new GameVersionCriteria(GameVersion.Parse("1.12.5"));
var repos = new Repository[] { repo1.repo, repo2.repo };
var providers = repoData.Manager
.GetAllAvailableModules(repos)
.GroupBy(am => am.AllAvailable().First().identifier)
.ToDictionary(grp => grp.Key,
grp => grp.ToHashSet());
var installed = new Dictionary<string, InstalledModule>();
var dlls = new HashSet<string>();
var dlcs = new Dictionary<string, ModuleVersion>();
var highPrio = repoData.Manager
.GetAvailableModules(Enumerable.Repeat<Repository>(repo1.repo, 1),
identifier)
.First()
.Latest();

// Act
var sorter = new CompatibilitySorter(
versCrit,
repoData.Manager.GetAllAvailDicts(repos),
providers, installed, dlls, dlcs);

// Assert
Assert.AreEqual(0, sorter.LatestIncompatible.Count);
Assert.AreEqual(2, sorter.LatestCompatible.Count);
Assert.AreEqual(CkanModule.ToJson(highPrio),
CkanModule.ToJson(sorter.LatestCompatible.First(m => m.identifier == identifier)));
}
}
}
}
11 changes: 8 additions & 3 deletions Tests/Data/TemporaryRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ namespace Tests.Data
/// </summary>
public class TemporaryRepository : IDisposable
{
public TemporaryRepository(params string[] fileContents)
public TemporaryRepository(int priority, params string[] fileContents)
{
path = Path.GetTempFileName();
repo = new Repository("temp", path, priority);

using (var outputStream = File.OpenWrite(path))
using (var gzipStream = new GZipOutputStream(outputStream))
Expand All @@ -39,8 +40,12 @@ public TemporaryRepository(params string[] fileContents)
}
}

public Uri uri => new Uri(path);
public Repository repo => new Repository("temp", uri);
public TemporaryRepository(params string[] fileContents)
: this(0, fileContents)
{ }

public Uri uri => new Uri(path);
public readonly Repository repo;

public void Dispose()
{
Expand Down

0 comments on commit f0d3980

Please sign in to comment.