diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9dd4aefe96..85c055abf8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -44,6 +44,7 @@ All notable changes to this project will be documented in this file.
- [Build] Check version of PowerShell in build script (#2235 by: HebaruSan; reviewed: Olympic1)
- [Multiple] Add and change logging to make INFO readable (#2236 by: HebaruSan; reviewed: politas)
- [Multiple] Use shared installer code in GUI and fix reinstall problems (#2233 by: HebaruSan; reviewed: Olympic1, politas)
+- [Multiple] Don't clear available modules till after the new list is ready (#2238 by: HebaruSan; reviewed: politas)
### Internal
diff --git a/Cmdline/Action/Update.cs b/Cmdline/Action/Update.cs
index 022cfb6fc8..573d11c89e 100644
--- a/Cmdline/Action/Update.cs
+++ b/Cmdline/Action/Update.cs
@@ -136,7 +136,7 @@ private void UpdateRepository(CKAN.KSP ksp, string repository = null)
var updated = repository == null
? CKAN.Repo.UpdateAllRepositories(registry_manager, ksp, user)
- : CKAN.Repo.Update(registry_manager, ksp, user, true, repository);
+ : CKAN.Repo.Update(registry_manager, ksp, user, repository);
user.RaiseMessage("Updated information on {0} available modules", updated);
}
diff --git a/Core/Net/Repo.cs b/Core/Net/Repo.cs
index c9f5f4c34f..4e34745ef2 100644
--- a/Core/Net/Repo.cs
+++ b/Core/Net/Repo.cs
@@ -25,106 +25,47 @@ public static class Repo
// Forward to keep existing code compiling, will be removed soon.
public static readonly Uri default_ckan_repo = CKAN.Repository.default_ckan_repo_uri;
- internal static void ProcessRegistryMetadataFromJSON(string metadata, Registry registry, string filename)
- {
- log.DebugFormat("Converting metadata from JSON.");
-
- try
- {
- CkanModule module = CkanModule.FromJson(metadata);
- log.DebugFormat("Found {0} version {1}", module.identifier, module.version);
- registry.AddAvailable(module);
- }
- catch (Exception exception)
- {
- // Alas, we can get exceptions which *wrap* our exceptions,
- // because json.net seems to enjoy wrapping rather than propagating.
- // See KSP-CKAN/CKAN-meta#182 as to why we need to walk the whole
- // exception stack.
-
- bool handled = false;
-
- while (exception != null)
- {
- if (exception is UnsupportedKraken || exception is BadMetadataKraken)
- {
- // Either of these can be caused by data meant for future
- // clients, so they're not really warnings, they're just
- // informational.
-
- log.InfoFormat("Skipping {0} : {1}", filename, exception.Message);
-
- // I'd *love a way to "return" from the catch block.
- handled = true;
- break;
- }
-
- // Look further down the stack.
- exception = exception.InnerException;
- }
-
- // If we haven't handled our exception, then it really was exceptional.
- if (handled == false)
- {
- if (exception == null)
- {
- // Had exception, walked exception tree, reached leaf, got stuck.
- log.ErrorFormat("Error processing {0} (exception tree leaf)", filename);
- }
- else
- {
- // In case whatever's calling us is lazy in error reporting, we'll
- // report that we've got an issue here.
- log.ErrorFormat("Error processing {0} : {1}", filename, exception.Message);
- }
-
- throw;
- }
- }
- }
-
///
- /// Download and update the local CKAN meta-info.
- /// Optionally takes a URL to the zipfile repo to download.
- /// Returns the number of unique modules updated.
+ /// Download and update the local CKAN meta-info.
+ /// Optionally takes a URL to the zipfile repo to download.
+ /// Returns the number of unique modules updated.
///
public static int UpdateAllRepositories(RegistryManager registry_manager, KSP ksp, IUser user)
{
- // If we handle multiple repositories, we will call ClearRegistry() ourselves...
- registry_manager.registry.ClearAvailable();
- // TODO this should already give us a pre-sorted list
SortedDictionary sortedRepositories = registry_manager.registry.Repositories;
+ List allAvail = new List();
foreach (KeyValuePair repository in sortedRepositories)
{
log.InfoFormat("About to update {0}", repository.Value.name);
- UpdateRegistry(repository.Value.uri, registry_manager.registry, ksp, user, false);
+ List avail = UpdateRegistry(repository.Value.uri, ksp, user);
log.InfoFormat("Updated {0}", repository.Value.name);
+ // Merge all the lists
+ allAvail.AddRange(avail);
+ }
+ // Save allAvail to the registry if we found anything
+ if (allAvail.Count > 0)
+ {
+ registry_manager.registry.SetAllAvailable(allAvail);
+ // Save our changes.
+ registry_manager.Save(enforce_consistency: false);
}
-
- // Save our changes.
- registry_manager.Save(enforce_consistency: false);
ShowUserInconsistencies(registry_manager.registry, user);
- // Return how many we got!
- return registry_manager.registry.Available(ksp.VersionCriteria()).Count;
- }
-
- public static int Update(RegistryManager registry_manager, KSP ksp, IUser user, Boolean clear = true, string repo = null)
- {
- if (repo == null)
+ List metadataChanges = GetChangedInstalledModules(registry_manager.registry);
+ if (metadataChanges.Count > 0)
{
- return Update(registry_manager, ksp, user, clear, (Uri)null);
+ HandleModuleChanges(metadataChanges, user, ksp);
}
- return Update(registry_manager, ksp, user, clear, new Uri(repo));
+ // Return how many we got!
+ return registry_manager.registry.Available(ksp.VersionCriteria()).Count;
}
///
- /// Updates the supplied registry from the URL given.
- /// This does not *save* the registry. For that, you probably want Repo.Update
+ /// Retrieve available modules from the URL given.
///
- internal static void UpdateRegistry(Uri repo, Registry registry, KSP ksp, IUser user, Boolean clear = true)
+ private static List UpdateRegistry(Uri repo, KSP ksp, IUser user)
{
// Use this opportunity to also update the build mappings... kind of hacky
ServiceLocator.Container.Resolve().Refresh();
@@ -139,30 +80,40 @@ internal static void UpdateRegistry(Uri repo, Registry registry, KSP ksp, IUser
catch (System.Net.WebException)
{
user.RaiseMessage("Connection to {0} could not be established.", repo);
- return;
- }
-
- // Clear our list of known modules.
- if (clear)
- {
- registry.ClearAvailable();
+ return null;
}
// Check the filetype.
FileType type = FileIdentifier.IdentifyFile(repo_file);
+ List newAvailable = null;
switch (type)
{
case FileType.TarGz:
- UpdateRegistryFromTarGz(repo_file, registry);
+ newAvailable = UpdateRegistryFromTarGz(repo_file);
break;
case FileType.Zip:
- UpdateRegistryFromZip(repo_file, registry);
+ newAvailable = UpdateRegistryFromZip(repo_file);
break;
}
- List metadataChanges = new List();
+ // Remove our downloaded meta-data now we've processed it.
+ // Seems weird to do this as part of a transaction, but Net.Download uses them, so let's be consistent.
+ file_transaction.Delete(repo_file);
+ return newAvailable;
+ }
+
+ ///
+ /// Find installed modules that have different metadata in their equivalent available module
+ ///
+ /// Registry to scan
+ ///
+ /// List of CkanModules that are available and have changed metadata
+ ///
+ private static List GetChangedInstalledModules(Registry registry)
+ {
+ List metadataChanges = new List();
foreach (InstalledModule installedModule in registry.InstalledModules)
{
string identifier = installedModule.identifier;
@@ -170,7 +121,7 @@ internal static void UpdateRegistry(Uri repo, Registry registry, KSP ksp, IUser
Version installedVersion = registry.InstalledVersion(identifier);
if (!(registry.available_modules.ContainsKey(identifier)))
{
- log.InfoFormat("UpdateRegistry, module {0}, version {1} not in repository ({2})", identifier, installedVersion, repo);
+ log.InfoFormat("UpdateRegistry, module {0}, version {1} not in registry", identifier, installedVersion);
continue;
}
@@ -189,62 +140,63 @@ internal static void UpdateRegistry(Uri repo, Registry registry, KSP ksp, IUser
metadataChanges.Add(registry.available_modules[identifier].module_version[installedVersion]);
}
}
+ return metadataChanges;
+ }
- if (metadataChanges.Any())
+ ///
+ /// Resolve differences between installed and available metadata for given ModuleInstaller
+ ///
+ /// List of modules that changed
+ /// Object for user interaction callbacks
+ /// Game instance
+ private static void HandleModuleChanges(List metadataChanges, IUser user, KSP ksp)
+ {
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < metadataChanges.Count; i++)
{
- StringBuilder sb = new StringBuilder();
-
- for (int i = 0; i < metadataChanges.Count; i++)
- {
- CkanModule module = metadataChanges[i];
-
- sb.AppendLine(string.Format("- {0} {1}", module.identifier, module.version));
- }
+ CkanModule module = metadataChanges[i];
+ sb.AppendLine(string.Format("- {0} {1}", module.identifier, module.version));
+ }
- if (user.RaiseYesNoDialog(string.Format(@"The following mods have had their metadata changed since last update:
+ if (user.RaiseYesNoDialog(string.Format(@"The following mods have had their metadata changed since last update:
{0}
You should reinstall them in order to preserve consistency with the repository.
Do you wish to reinstall now?", sb)))
+ {
+ ModuleInstaller installer = ModuleInstaller.GetInstance(ksp, new NullUser());
+ // New upstream metadata may break the consistency of already installed modules
+ // e.g. if user installs modules A and B and then later up A is made to conflict with B
+ // This is perfectly normal and shouldn't produce an error, therefore we skip enforcing
+ // consistency. However, we will show the user any inconsistencies later on.
+
+ // Use the identifiers so we use the overload that actually resolves relationships
+ // Do each changed module one at a time so a failure of one doesn't cause all the others to fail
+ foreach (string changedIdentifier in metadataChanges.Select(i => i.identifier))
{
- ModuleInstaller installer = ModuleInstaller.GetInstance(ksp, new NullUser());
- // New upstream metadata may break the consistency of already installed modules
- // e.g. if user installs modules A and B and then later up A is made to conflict with B
- // This is perfectly normal and shouldn't produce an error, therefore we skip enforcing
- // consistency. However, we will show the user any inconsistencies later on.
-
- // Use the identifiers so we use the overload that actually resolves relationships
- // Do each changed module one at a time so a failure of one doesn't cause all the others to fail
- foreach (string changedIdentifier in metadataChanges.Select(i => i.identifier))
+ try
{
- try
- {
- installer.Upgrade(
- new[] { changedIdentifier },
- new NetAsyncModulesDownloader(new NullUser()),
- enforceConsistency: false
- );
- }
- // Thrown when a dependency couldn't be satisfied
- catch(ModuleNotFoundKraken)
- {
- log.WarnFormat("Skipping installation of {0} due to relationship error.", changedIdentifier);
- user.RaiseMessage("Skipping installation of {0} due to relationship error.", changedIdentifier);
- }
- // Thrown when a conflicts relationship is violated
- catch (InconsistentKraken)
- {
- log.WarnFormat("Skipping installation of {0} due to relationship error.", changedIdentifier);
- user.RaiseMessage("Skipping installation of {0} due to relationship error.", changedIdentifier);
- }
+ installer.Upgrade(
+ new[] { changedIdentifier },
+ new NetAsyncModulesDownloader(new NullUser()),
+ enforceConsistency: false
+ );
+ }
+ // Thrown when a dependency couldn't be satisfied
+ catch (ModuleNotFoundKraken)
+ {
+ log.WarnFormat("Skipping installation of {0} due to relationship error.", changedIdentifier);
+ user.RaiseMessage("Skipping installation of {0} due to relationship error.", changedIdentifier);
+ }
+ // Thrown when a conflicts relationship is violated
+ catch (InconsistentKraken)
+ {
+ log.WarnFormat("Skipping installation of {0} due to relationship error.", changedIdentifier);
+ user.RaiseMessage("Skipping installation of {0} due to relationship error.", changedIdentifier);
}
}
}
-
- // Remove our downloaded meta-data now we've processed it.
- // Seems weird to do this as part of a transaction, but Net.Download uses them, so let's be consistent.
- file_transaction.Delete(repo_file);
}
private static bool MetadataEquals(CkanModule metadata, CkanModule oldMetadata)
@@ -393,7 +345,28 @@ private static bool RelationshipsAreEquivalent(List a, L
return true;
}
- private static int Update(RegistryManager registry_manager, KSP ksp, IUser user, Boolean clear = true, Uri repo = null)
+ ///
+ /// Set a registry's available modules to the list from just one repo
+ ///
+ /// Manager of the regisry of interest
+ /// Game instance
+ /// Object for user interaction callbacks
+ /// Repository to check
+ ///
+ /// Number of modules found in repo
+ ///
+ public static int Update(RegistryManager registry_manager, KSP ksp, IUser user, string repo = null)
+ {
+ if (repo == null)
+ {
+ return Update(registry_manager, ksp, user, (Uri)null);
+ }
+
+ return Update(registry_manager, ksp, user, new Uri(repo));
+ }
+
+ // Same as above, just with a Uri instead of string for the repo
+ public static int Update(RegistryManager registry_manager, KSP ksp, IUser user, Uri repo = null)
{
// Use our default repo, unless we've been told otherwise.
if (repo == null)
@@ -401,10 +374,13 @@ private static int Update(RegistryManager registry_manager, KSP ksp, IUser user,
repo = default_ckan_repo;
}
- UpdateRegistry(repo, registry_manager.registry, ksp, user, clear);
-
- // Save our changes!
- registry_manager.Save(enforce_consistency: false);
+ List newAvail = UpdateRegistry(repo, ksp, user);
+ if (newAvail != null && newAvail.Count > 0)
+ {
+ registry_manager.registry.SetAllAvailable(newAvail);
+ // Save our changes!
+ registry_manager.Save(enforce_consistency: false);
+ }
ShowUserInconsistencies(registry_manager.registry, user);
@@ -413,14 +389,13 @@ private static int Update(RegistryManager registry_manager, KSP ksp, IUser user,
}
///
- /// Updates the supplied registry from the supplied zip file.
- /// This will *clear* the registry of available modules first.
- /// This does not *save* the registry. For that, you probably want Repo.Update
+ /// Returns available modules from the supplied tar.gz file.
///
- internal static void UpdateRegistryFromTarGz(string path, Registry registry)
+ private static List UpdateRegistryFromTarGz(string path)
{
log.DebugFormat("Starting registry update from tar.gz file: \"{0}\".", path);
+ List modules = new List();
// Open the gzip'ed file.
using (Stream inputStream = File.OpenRead(path))
{
@@ -474,22 +449,26 @@ internal static void UpdateRegistryFromTarGz(string path, Registry registry)
// Convert the buffer data to a string.
string metadata_json = Encoding.ASCII.GetString(buffer);
- ProcessRegistryMetadataFromJSON(metadata_json, registry, filename);
+ CkanModule module = ProcessRegistryMetadataFromJSON(metadata_json, filename);
+ if (module != null)
+ {
+ modules.Add(module);
+ }
}
}
}
}
+ return modules;
}
///
- /// Updates the supplied registry from the supplied zip file.
- /// This will *clear* the registry of available modules first.
- /// This does not *save* the registry. For that, you probably want Repo.Update
+ /// Returns available modules from the supplied zip file.
///
- internal static void UpdateRegistryFromZip(string path, Registry registry)
+ private static List UpdateRegistryFromZip(string path)
{
log.DebugFormat("Starting registry update from zip file: \"{0}\".", path);
+ List modules = new List();
using (var zipfile = new ZipFile(path))
{
// Walk the archive, looking for .ckan files.
@@ -516,13 +495,76 @@ internal static void UpdateRegistryFromZip(string path, Registry registry)
stream.Close();
}
- ProcessRegistryMetadataFromJSON(metadata_json, registry, filename);
+ CkanModule module = ProcessRegistryMetadataFromJSON(metadata_json, filename);
+ if (module != null)
+ {
+ modules.Add(module);
+ }
}
zipfile.Close();
}
+ return modules;
}
+ private static CkanModule ProcessRegistryMetadataFromJSON(string metadata, string filename)
+ {
+ log.DebugFormat("Converting metadata from JSON.");
+
+ try
+ {
+ CkanModule module = CkanModule.FromJson(metadata);
+ log.DebugFormat("Found {0} version {1}", module.identifier, module.version);
+ return module;
+ }
+ catch (Exception exception)
+ {
+ // Alas, we can get exceptions which *wrap* our exceptions,
+ // because json.net seems to enjoy wrapping rather than propagating.
+ // See KSP-CKAN/CKAN-meta#182 as to why we need to walk the whole
+ // exception stack.
+
+ bool handled = false;
+
+ while (exception != null)
+ {
+ if (exception is UnsupportedKraken || exception is BadMetadataKraken)
+ {
+ // Either of these can be caused by data meant for future
+ // clients, so they're not really warnings, they're just
+ // informational.
+
+ log.InfoFormat("Skipping {0} : {1}", filename, exception.Message);
+
+ // I'd *love a way to "return" from the catch block.
+ handled = true;
+ break;
+ }
+
+ // Look further down the stack.
+ exception = exception.InnerException;
+ }
+
+ // If we haven't handled our exception, then it really was exceptional.
+ if (handled == false)
+ {
+ if (exception == null)
+ {
+ // Had exception, walked exception tree, reached leaf, got stuck.
+ log.ErrorFormat("Error processing {0} (exception tree leaf)", filename);
+ }
+ else
+ {
+ // In case whatever's calling us is lazy in error reporting, we'll
+ // report that we've got an issue here.
+ log.ErrorFormat("Error processing {0} : {1}", filename, exception.Message);
+ }
+
+ throw;
+ }
+ return null;
+ }
+ }
private static void ShowUserInconsistencies(Registry registry, IUser user)
{
diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs
index fa623abd7d..a5da74740e 100644
--- a/Core/Registry/Registry.cs
+++ b/Core/Registry/Registry.cs
@@ -347,13 +347,16 @@ private void SealionTransaction()
#endregion
- ///
- /// Clears all available modules from the registry.
- ///
- public void ClearAvailable()
+ public void SetAllAvailable(IEnumerable newAvail)
{
SealionTransaction();
+ // Clear current modules
available_modules = new Dictionary();
+ // Add the new modules
+ foreach (CkanModule module in newAvail)
+ {
+ AddAvailable(module);
+ }
}
///
@@ -365,7 +368,7 @@ public void AddAvailable(CkanModule module)
var identifier = module.identifier;
// If we've never seen this module before, create an entry for it.
- if (! available_modules.ContainsKey(identifier))
+ if (!available_modules.ContainsKey(identifier))
{
log.DebugFormat("Adding new available module {0}", identifier);
available_modules[identifier] = new AvailableModule(identifier);
diff --git a/Tests/Core/Net/NetAsyncModulesDownloader.cs b/Tests/Core/Net/NetAsyncModulesDownloader.cs
index be9f327c08..876ac9580d 100644
--- a/Tests/Core/Net/NetAsyncModulesDownloader.cs
+++ b/Tests/Core/Net/NetAsyncModulesDownloader.cs
@@ -14,6 +14,7 @@ namespace Tests.Core.Net
public class NetAsyncModulesDownloader
{
+ private CKAN.RegistryManager manager;
private CKAN.Registry registry;
private DisposableKSP ksp;
private CKAN.IDownloader async;
@@ -29,13 +30,12 @@ public void Setup()
// Give us a registry to play with.
ksp = new DisposableKSP();
- registry = CKAN.RegistryManager.Instance(ksp.KSP).registry;
- registry.ClearAvailable();
+ manager = CKAN.RegistryManager.Instance(ksp.KSP);
+ registry = manager.registry;
registry.ClearDlls();
registry.Installed().Clear();
-
// Make sure we have a registry we can use.
- CKAN.Repo.UpdateRegistry(TestData.TestKANZip(), registry, ksp.KSP, new NullUser());
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.TestKANZip());
// Ready our downloader.
async = new CKAN.NetAsyncModulesDownloader(new NullUser());
@@ -128,4 +128,3 @@ public void RandSdownload()
}
}
-
diff --git a/Tests/Core/Net/Repo.cs b/Tests/Core/Net/Repo.cs
index bd583edcc2..e4f0b3be84 100644
--- a/Tests/Core/Net/Repo.cs
+++ b/Tests/Core/Net/Repo.cs
@@ -8,6 +8,7 @@ namespace Tests.Core.Net
[TestFixture]
public class Repo
{
+ private CKAN.RegistryManager manager;
private CKAN.Registry registry;
private DisposableKSP ksp;
@@ -15,8 +16,8 @@ public class Repo
public void Setup()
{
ksp = new DisposableKSP();
- registry = CKAN.RegistryManager.Instance(ksp.KSP).registry;
- registry.ClearAvailable();
+ manager = CKAN.RegistryManager.Instance(ksp.KSP);
+ registry = manager.registry;
registry.ClearDlls();
registry.Installed().Clear();
}
@@ -30,7 +31,7 @@ public void TearDown()
[Test]
public void UpdateRegistryTarGz()
{
- CKAN.Repo.UpdateRegistry(TestData.TestKANTarGz(), registry, ksp.KSP, new NullUser());
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.TestKANTarGz());
// Test we've got an expected module.
CkanModule far = registry.LatestAvailable("FerramAerospaceResearch", new KspVersionCriteria(KspVersion.Parse("0.25.0")));
@@ -41,10 +42,10 @@ public void UpdateRegistryTarGz()
[Test]
public void UpdateRegistryZip()
{
- CKAN.Repo.UpdateRegistry(TestData.TestKANZip(), registry, ksp.KSP, new NullUser());
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.TestKANZip());
// Test we've got an expected module.
- CkanModule far = registry.LatestAvailable("FerramAerospaceResearch", new KspVersionCriteria (KspVersion.Parse("0.25.0")));
+ CkanModule far = registry.LatestAvailable("FerramAerospaceResearch", new KspVersionCriteria(KspVersion.Parse("0.25.0")));
Assert.AreEqual("v0.14.3.2", far.version.ToString());
}
@@ -54,7 +55,7 @@ public void BadKanTarGz()
{
Assert.DoesNotThrow(delegate
{
- CKAN.Repo.UpdateRegistry(TestData.BadKANTarGz(), registry, ksp.KSP, new NullUser());
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.BadKANTarGz());
});
}
@@ -62,9 +63,9 @@ public void BadKanTarGz()
public void BadKanZip()
{
Assert.DoesNotThrow(delegate
- {
- CKAN.Repo.UpdateRegistry(TestData.BadKANZip(), registry, ksp.KSP, new NullUser());
- });
+ {
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.BadKANZip());
+ });
}
}
}
diff --git a/Tests/Core/Relationships/SanityChecker.cs b/Tests/Core/Relationships/SanityChecker.cs
index f005096a20..3ba1366dee 100644
--- a/Tests/Core/Relationships/SanityChecker.cs
+++ b/Tests/Core/Relationships/SanityChecker.cs
@@ -13,6 +13,7 @@ namespace Tests.Core.Relationships
[TestFixture]
public class SanityChecker
{
+ private CKAN.RegistryManager manager;
private CKAN.Registry registry;
private DisposableKSP ksp;
@@ -21,12 +22,11 @@ public void Setup()
{
ksp = new DisposableKSP();
- registry = CKAN.RegistryManager.Instance(ksp.KSP).registry;
- registry.ClearAvailable();
+ manager = CKAN.RegistryManager.Instance(ksp.KSP);
+ registry = manager.registry;
registry.ClearDlls();
registry.Installed().Clear();
-
- Repo.UpdateRegistry(TestData.TestKANZip(), registry, ksp.KSP, new NullUser());
+ CKAN.Repo.Update(manager, ksp.KSP, new NullUser(), TestData.TestKANZip());
}
[Test]
@@ -191,4 +191,3 @@ private static void TestDepends(List to_remove, List mods, L
}
}
}
-