Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move downloads outside of gui transaction #2073

Merged
merged 6 commits into from
Aug 26, 2017
Merged

Move downloads outside of gui transaction #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
02f55b1
License cleanup
archer884 Aug 23, 2017
bfab451
Remove unused code
archer884 Aug 23, 2017
92e8b66
Cleanup usings
archer884 Aug 23, 2017
f212052
Whitespace cleanup
archer884 Aug 23, 2017
1a24f8d
Add ModuleResolution class
archer884 Aug 23, 2017
565269c
Move download outside GUI transaction
archer884 Aug 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Core/CKAN-core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
<Compile Include="Types\CKANVersion.cs" />
<Compile Include="Registry\IRegistryQuerier.cs" />
<Compile Include="Types\ExportFileType.cs" />
<Compile Include="Types\ModuleResolution.cs" />
<Compile Include="User.cs" />
<Compile Include="Types\Version.cs" />
<Compile Include="Types\Kraken.cs" />
Expand Down
105 changes: 74 additions & 31 deletions Core/ModuleInstaller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using ICSharpCode.SharpZipLib.Core;
using ICSharpCode.SharpZipLib.Zip;
using log4net;
using CKAN.Types;

namespace CKAN
{
Expand All @@ -28,7 +29,7 @@ public class ModuleInstaller
private static SortedList<string, ModuleInstaller> instances = new SortedList<string, ModuleInstaller>();

private static readonly ILog log = LogManager.GetLogger(typeof(ModuleInstaller));
private static readonly TxFileManager file_transaction = new TxFileManager ();
private static readonly TxFileManager file_transaction = new TxFileManager();

private RegistryManager registry_manager;
private KSP ksp;
Expand Down Expand Up @@ -144,8 +145,6 @@ public static string CachedOrDownload(string identifier, Version version, Uri ur
return full_path;
}



public void InstallList(
List<string> modules,
RelationshipResolverOptions options,
Expand Down Expand Up @@ -175,7 +174,7 @@ public void InstallList(
{
var resolver = new RelationshipResolver(modules, options, registry_manager.registry, ksp.VersionCriteria());
var modsToInstall = resolver.ModList().ToList();
List<CkanModule> downloads = new List<CkanModule> ();
List<CkanModule> downloads = new List<CkanModule>();

// TODO: All this user-stuff should be happening in another method!
// We should just be installing mods as a transaction.
Expand Down Expand Up @@ -251,6 +250,46 @@ public void InstallList(
User.RaiseProgress("Done!\r\n", 100);
}

public ModuleResolution ResolveModules(IEnumerable<string> modules, RelationshipResolverOptions options)
{
var resolver = new RelationshipResolver(modules, options, registry_manager.registry, ksp.VersionCriteria());
return new ModuleResolution(resolver.ModList(), m => ksp.Cache.IsCachedZip(m.download));
}

public void EnsureCache(List<CkanModule> modules, IDownloader downloader = null)
{
if (!modules.Any())
{
return;
}

downloader = downloader ?? new NetAsyncModulesDownloader(User);
downloader.DownloadModules(ksp.Cache, modules);
}

public void InstallList(ModuleResolution modules, RelationshipResolverOptions options)
{
// We're about to install all our mods; so begin our transaction.
using (TransactionScope transaction = CkanTransaction.CreateTransactionScope())
{
var enumeratedMods = modules.Select((m, i) => new { Idx = i, Module = m });
foreach (var item in enumeratedMods)
{
var percentComplete = (item.Idx * 100) / modules.Count;
User.RaiseProgress(string.Format("Installing mod \"{0}\"", item.Module), percentComplete);
Install(item.Module);
}

User.RaiseProgress("Updating registry", 70);

registry_manager.Save(!options.without_enforce_consistency);

User.RaiseProgress("Commiting filesystem changes", 80);

transaction.Complete();
}
}

/// <summary>
/// Returns the module contents if and only if we have it
/// available in our cache. Returns null, otherwise.
Expand Down Expand Up @@ -300,7 +339,7 @@ private void Install(CkanModule module, string filename = null)
}

// Find our in the cache if we don't already have it.
filename = filename ?? Cache.GetCachedZip(module.download,true);
filename = filename ?? Cache.GetCachedZip(module.download, true);

// If we *still* don't have a file, then kraken bitterly.
if (filename == null)
Expand Down Expand Up @@ -415,7 +454,7 @@ internal static List<InstallableFile> FindInstallableFiles(ModuleInstallDescript
{
string installDir;
bool makeDirs;
var files = new List<InstallableFile> ();
var files = new List<InstallableFile>();

// Normalize the path before doing everything else
// TODO: This really should happen in the ModuleInstallDescriptor itself.
Expand Down Expand Up @@ -469,25 +508,28 @@ internal static List<InstallableFile> FindInstallableFiles(ModuleInstallDescript
throw new BadInstallLocationKraken("Unknown install_to " + stanza.install_to);
}
}
else switch (stanza.install_to)
else
{
case "Tutorial":
installDir = ksp == null ? null : ksp.Tutorial();
makeDirs = true;
break;

case "Scenarios":
installDir = ksp == null ? null : ksp.Scenarios();
makeDirs = true;
break;

case "GameRoot":
installDir = ksp == null ? null : ksp.GameDir();
makeDirs = false;
break;

default:
throw new BadInstallLocationKraken("Unknown install_to " + stanza.install_to);
switch (stanza.install_to)
{
case "Tutorial":
installDir = ksp == null ? null : ksp.Tutorial();
makeDirs = true;
break;

case "Scenarios":
installDir = ksp == null ? null : ksp.Scenarios();
makeDirs = true;
break;

case "GameRoot":
installDir = ksp == null ? null : ksp.GameDir();
makeDirs = false;
break;

default:
throw new BadInstallLocationKraken("Unknown install_to " + stanza.install_to);
}
}

// O(N^2) solution, as we're walking the zipfile for each stanza.
Expand All @@ -496,7 +538,8 @@ internal static List<InstallableFile> FindInstallableFiles(ModuleInstallDescript
foreach (ZipEntry entry in zipfile)
{
// Skips things not prescribed by our install stanza.
if (! stanza.IsWanted(entry.Name)) {
if (!stanza.IsWanted(entry.Name))
{
continue;
}

Expand Down Expand Up @@ -612,7 +655,7 @@ internal static string TransformOutputName(string file, string outputName, strin
/// </summary>
public static List<InstallableFile> FindInstallableFiles(CkanModule module, ZipFile zipfile, KSP ksp)
{
var files = new List<InstallableFile> ();
var files = new List<InstallableFile>();

try
{
Expand Down Expand Up @@ -672,7 +715,7 @@ internal static void CopyZipEntry(ZipFile zipfile, ZipEntry entry, string fullPa
// Skip if we're not making directories for this install.
if (!makeDirs)
{
log.DebugFormat ("Skipping {0}, we don't make directories for this path", fullPath);
log.DebugFormat("Skipping {0}, we don't make directories for this path", fullPath);
return;
}

Expand All @@ -693,7 +736,7 @@ internal static void CopyZipEntry(ZipFile zipfile, ZipEntry entry, string fullPa
}

// We don't allow for the overwriting of files. See #208.
if (File.Exists (fullPath))
if (File.Exists(fullPath))
{
throw new FileExistsKraken(fullPath, string.Format("Trying to write {0} but it already exists.", fullPath));
}
Expand Down Expand Up @@ -778,7 +821,7 @@ public void UninstallList(IEnumerable<string> mods)

public void UninstallList(string mod)
{
var list = new List<string> {mod};
var list = new List<string> { mod };
UninstallList(list);
}

Expand Down Expand Up @@ -824,7 +867,7 @@ private void Uninstall(string modName)
// This is the fastest way to do this test.
if ((attr & FileAttributes.Directory) == FileAttributes.Directory)
{
if ( !(directoriesToDelete.Contains(path)))
if (!directoriesToDelete.Contains(path))
{
directoriesToDelete.Add(path);
}
Expand All @@ -836,7 +879,7 @@ private void Uninstall(string modName)
// Since we check for directory contents when deleting, this should purge empty
// dirs, making less ModuleManager headaches for people.
var directoryName = Path.GetDirectoryName(path);
if ( !(directoriesToDelete.Contains(directoryName)))
if (!(directoriesToDelete.Contains(directoryName)))
{
directoriesToDelete.Add(directoryName);
}
Expand Down
61 changes: 5 additions & 56 deletions Core/Types/CkanModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
using System.Linq;
using System.Runtime.Serialization;
using System.Text.RegularExpressions;
using log4net;
using Newtonsoft.Json;
using System.Transactions;
using Autofac;
using CKAN.Versioning;
using log4net;
using Newtonsoft.Json;

namespace CKAN
{
Expand Down Expand Up @@ -288,11 +287,6 @@ public CkanModule(string json, IGameComparator comparator)
{
_comparator = comparator;

if (!validate_json_against_schema(json))
{
throw new BadMetadataKraken(null, "Validation against spec failed");
}

try
{
// Use the json string to populate our object
Expand Down Expand Up @@ -362,54 +356,9 @@ private void DeSerialisationFixes(StreamingContext like_i_could_care)
throw new InvalidModuleAttributesException("ksp_version mixed with ksp_version_(min|max)", this);
}

if (license == null)
{
license = new List<License> { new License("unknown") };
}

if (@abstract == null)
{
@abstract = "";
}

if (name == null)
{
name = "";
}
}

private static bool validate_json_against_schema(string json)
{

log.Debug("In-client JSON schema validation unimplemented.");
return true;
// due to Newtonsoft Json not supporting v4 of the standard, we can't actually do this :(

// if (metadata_schema == null)
// {
// string schema = "";
//
// try
// {
// schema = File.ReadAllText(metadata_schema_path);
// }
// catch (Exception)
// {
// if (!metadata_schema_missing_warning_fired)
// {
// User.Error("Couldn't open metadata schema at \"{0}\", will not validate metadata files",
// metadata_schema_path);
// metadata_schema_missing_warning_fired = true;
// }
//
// return true;
// }
//
// metadata_schema = JsonSchema.Parse(schema);
// }
//
// JObject obj = JObject.Parse(json);
// return obj.IsValid(metadata_schema);
license = license ?? new List<License> { License.UnknownLicense };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, this sets licence to "unknown" if it doesn't match anything in the licence list? We would generally prefer a failure (this code is mainly intended for netkan)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@politas The old code would create a new instance of an unknown license in that case, on line 367. The new code does the same thing but uses the static License.UnknownLicense instead of a new instance. I probably shouldn't have been doing arbitrary little cleanup-y things like that in a branch about timeouts, but the effect (creating the unknown license) is not changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. The little code cleanups are making it hard to review, given my poor understanding of the transaction code!

@abstract = @abstract ?? string.Empty;
name = name ?? string.Empty;
}

/// <summary>
Expand Down
3 changes: 3 additions & 0 deletions Core/Types/License.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ namespace CKAN
[JsonConverter(typeof(JsonSimpleStringConverter))]
public class License
{
static License _unknownLicense;
public static License UnknownLicense => _unknownLicense ?? (_unknownLicense = new License("unknown"));

// TODO: It would be lovely for our build system to write these for us.
private static readonly HashSet<string> valid_licenses = new HashSet<string> {
"public-domain",
Expand Down
45 changes: 45 additions & 0 deletions Core/Types/ModuleResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace CKAN.Types
{
public class ModuleResolution : IEnumerable<CkanModule>
{
public List<CkanModule> CachedModules { get; set; }
public List<CkanModule> UncachedModules { get; set; }

public IEnumerable<CkanModule> All => CachedModules.Concat(UncachedModules);

public int Count => CachedModules.Count + UncachedModules.Count;

public ModuleResolution(IEnumerable<CkanModule> modules, Func<CkanModule, bool> isCached)
{
CachedModules = new List<CkanModule>();
UncachedModules = new List<CkanModule>();

foreach (var module in modules)
{
if (isCached(module))
{
CachedModules.Add(module);
}
else
{
UncachedModules.Add(module);
}
}
}

public IEnumerator<CkanModule> GetEnumerator()
{
return All.GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}
}
Loading