From 65a48808a7bfbadf2a83f2f38ca3abde086c9f4b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sun, 27 Mar 2022 19:53:48 -0500 Subject: [PATCH] Use cfg parser for Netkan --- Netkan/CKAN-netkan.csproj | 4 + Netkan/Processors/Inflator.cs | 6 +- Netkan/Services/CachingConfigParser.cs | 85 +++++++++++++++++++ Netkan/Services/IConfigParser.cs | 12 +++ .../Transformers/LocalizationsTransformer.cs | 34 +++----- Netkan/Transformers/NetkanTransformer.cs | 3 +- Netkan/Validators/CkanValidator.cs | 12 +-- Netkan/Validators/ForClauseValidator.cs | 60 +++++++++++++ .../ModuleManagerDependsValidator.cs | 33 +++++-- Netkan/Validators/PluginsValidator.cs | 22 +++-- .../Games/KerbalSpaceProgram/ConfigParser.cs | 30 ++++--- Tests/NetKAN/Validators/CkanValidatorTests.cs | 12 ++- 12 files changed, 252 insertions(+), 61 deletions(-) create mode 100644 Netkan/Services/CachingConfigParser.cs create mode 100644 Netkan/Services/IConfigParser.cs create mode 100644 Netkan/Validators/ForClauseValidator.cs diff --git a/Netkan/CKAN-netkan.csproj b/Netkan/CKAN-netkan.csproj index ba8a389f46..e96b0aa495 100644 --- a/Netkan/CKAN-netkan.csproj +++ b/Netkan/CKAN-netkan.csproj @@ -46,6 +46,7 @@ + @@ -71,7 +72,9 @@ + + @@ -129,6 +132,7 @@ + diff --git a/Netkan/Processors/Inflator.cs b/Netkan/Processors/Inflator.cs index 22b82bf327..1bec2be1dc 100644 --- a/Netkan/Processors/Inflator.cs +++ b/Netkan/Processors/Inflator.cs @@ -26,9 +26,11 @@ public Inflator(string cacheDir, bool overwriteCache, string githubToken, bool p IModuleService moduleService = new ModuleService(); IFileService fileService = new FileService(cache); + IConfigParser configParser = new CachingConfigParser(moduleService); http = new CachingHttpService(cache, overwriteCache); - ckanValidator = new CkanValidator(http, moduleService); - transformer = new NetkanTransformer(http, fileService, moduleService, githubToken, prerelease, netkanValidator); + ckanValidator = new CkanValidator(http, moduleService, configParser); + transformer = new NetkanTransformer(http, fileService, moduleService, configParser, + githubToken, prerelease, netkanValidator); } internal IEnumerable Inflate(string filename, Metadata netkan, TransformOptions opts) diff --git a/Netkan/Services/CachingConfigParser.cs b/Netkan/Services/CachingConfigParser.cs new file mode 100644 index 0000000000..a9e1bb4cf5 --- /dev/null +++ b/Netkan/Services/CachingConfigParser.cs @@ -0,0 +1,85 @@ +using System; +using System.IO; +using System.Linq; +using System.Collections.Generic; + +using log4net; +using ICSharpCode.SharpZipLib.Zip; +using ParsecSharp; + +namespace CKAN.NetKAN.Services +{ + using NodeCache = Dictionary; + + /// + /// Since parsing cfg files can be expensive, cache results for 15 minutes + /// + internal sealed class CachingConfigParser : IConfigParser + { + public CachingConfigParser(IModuleService modSvc) + { + moduleService = modSvc; + } + + public Dictionary GetConfigNodes(CkanModule module, ZipFile zip, GameInstance inst) + => GetCachedNodes(module) ?? AddAndReturn( + module, + moduleService.GetConfigFiles(module, zip, inst).ToDictionary( + cfg => cfg, + cfg => KSPConfigParser.ConfigFile.ToArray().Parse( + new StreamReader(zip.GetInputStream(cfg.source)).ReadToEnd()) + .CaseOf(failure => + { + log.InfoFormat("{0}:{1}:{2}: {3}", + inst.ToRelativeGameDir(cfg.destination), + failure.State.Position.Line, + failure.State.Position.Column, + failure.Message); + return new KSPConfigNode[] { }; + }, + success => success.Value))); + + private Dictionary AddAndReturn(CkanModule module, + Dictionary nodes) + { + log.DebugFormat("Caching config nodes for {0}", module); + cache.Add(module, + new ConfigNodesCacheEntry() + { + Value = nodes, + Timestamp = DateTime.Now, + }); + return nodes; + } + + private Dictionary GetCachedNodes(CkanModule module) + { + if (cache.TryGetValue(module, out ConfigNodesCacheEntry entry)) + { + if (DateTime.Now - entry.Timestamp < stringCacheLifetime) + { + log.DebugFormat("Using cached nodes for {0}", module); + return entry.Value; + } + else + { + log.DebugFormat("Purging stale nodes for {0}", module); + cache.Remove(module); + } + } + return null; + } + + private readonly IModuleService moduleService; + private readonly NodeCache cache = new NodeCache(); + // Re-use parse results within 15 minutes + private static readonly TimeSpan stringCacheLifetime = new TimeSpan(0, 15, 0); + private static readonly ILog log = LogManager.GetLogger(typeof(CachingConfigParser)); + } + + public class ConfigNodesCacheEntry + { + public Dictionary Value; + public DateTime Timestamp; + } +} diff --git a/Netkan/Services/IConfigParser.cs b/Netkan/Services/IConfigParser.cs new file mode 100644 index 0000000000..d1eb175de7 --- /dev/null +++ b/Netkan/Services/IConfigParser.cs @@ -0,0 +1,12 @@ +using System; +using System.Collections.Generic; + +using ICSharpCode.SharpZipLib.Zip; + +namespace CKAN.NetKAN.Services +{ + internal interface IConfigParser + { + Dictionary GetConfigNodes(CkanModule module, ZipFile zip, GameInstance inst); + } +} diff --git a/Netkan/Transformers/LocalizationsTransformer.cs b/Netkan/Transformers/LocalizationsTransformer.cs index 731d897dee..fe654092c4 100644 --- a/Netkan/Transformers/LocalizationsTransformer.cs +++ b/Netkan/Transformers/LocalizationsTransformer.cs @@ -1,10 +1,11 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Text.RegularExpressions; + using ICSharpCode.SharpZipLib.Zip; using log4net; using Newtonsoft.Json.Linq; + using CKAN.Extensions; using CKAN.NetKAN.Extensions; using CKAN.NetKAN.Model; @@ -21,10 +22,11 @@ internal sealed class LocalizationsTransformer : ITransformer /// /// HTTP service /// Module service - public LocalizationsTransformer(IHttpService http, IModuleService moduleService) + public LocalizationsTransformer(IHttpService http, IModuleService moduleService, IConfigParser parser) { _http = http; _moduleService = moduleService; + _parser = parser; } /// @@ -56,16 +58,13 @@ public IEnumerable Transform(Metadata metadata, TransformOptions opts) log.Debug("Extracting locales"); // Extract the locale names from the ZIP's cfg files - var locales = _moduleService.GetConfigFiles(mod, zip, inst) - .Select(cfg => new StreamReader(zip.GetInputStream(cfg.source)).ReadToEnd()) - .SelectMany(contents => localizationRegex.Matches(contents).Cast() - .Select(m => m.Groups["contents"].Value)) - .SelectMany(contents => localeRegex.Matches(contents).Cast() - .Where(m => m.Groups["contents"].Value.Contains("=")) - .Select(m => m.Groups["locale"].Value)) - .Distinct() - .OrderBy(l => l) - .Memoize(); + var locales = _parser.GetConfigNodes(mod, zip, inst) + .SelectMany(kvp => kvp.Value) + .Where(node => node.Name == localizationsNodeName) + .SelectMany(node => node.Children.Select(child => child.Name)) + .Distinct() + .OrderBy(l => l) + .Memoize(); log.Debug("Locales extracted"); if (locales.Any()) @@ -82,20 +81,13 @@ public IEnumerable Transform(Metadata metadata, TransformOptions opts) } } + private const string localizationsNodeName = "Localization"; private const string localizationsProperty = "localizations"; private readonly IHttpService _http; private readonly IModuleService _moduleService; + private readonly IConfigParser _parser; private static readonly ILog log = LogManager.GetLogger(typeof(LocalizationsTransformer)); - - private static readonly Regex localizationRegex = new Regex( - @"^\s*Localization\b\s*{(?[^{}]+({[^{}]*}[^{}]*)+)}", - RegexOptions.Compiled | RegexOptions.Multiline | RegexOptions.Singleline - ); - private static readonly Regex localeRegex = new Regex( - @"^\s*(?[-a-zA-Z]+).*?{(?.*?)}", - RegexOptions.Compiled | RegexOptions.Multiline | RegexOptions.Singleline - ); } } diff --git a/Netkan/Transformers/NetkanTransformer.cs b/Netkan/Transformers/NetkanTransformer.cs index fd8b5d9563..2e28ed297c 100644 --- a/Netkan/Transformers/NetkanTransformer.cs +++ b/Netkan/Transformers/NetkanTransformer.cs @@ -24,6 +24,7 @@ public NetkanTransformer( IHttpService http, IFileService fileService, IModuleService moduleService, + IConfigParser configParser, string githubToken, bool prerelease, IValidator validator @@ -43,7 +44,7 @@ IValidator validator new AvcKrefTransformer(http, ghApi), new InternalCkanTransformer(http, moduleService), new AvcTransformer(http, moduleService, ghApi), - new LocalizationsTransformer(http, moduleService), + new LocalizationsTransformer(http, moduleService, configParser), new VersionEditTransformer(), new ForcedVTransformer(), new EpochTransformer(), diff --git a/Netkan/Validators/CkanValidator.cs b/Netkan/Validators/CkanValidator.cs index 53018e8f2e..854d90e7d7 100644 --- a/Netkan/Validators/CkanValidator.cs +++ b/Netkan/Validators/CkanValidator.cs @@ -8,10 +8,8 @@ internal sealed class CkanValidator : IValidator { private readonly List _validators; - public CkanValidator(IHttpService downloader, IModuleService moduleService) + public CkanValidator(IHttpService downloader, IModuleService moduleService, IConfigParser configParser) { - this.downloader = downloader; - this.moduleService = moduleService; _validators = new List { new IsCkanModuleValidator(), @@ -21,8 +19,9 @@ public CkanValidator(IHttpService downloader, IModuleService moduleService) new ObeysCKANSchemaValidator(), new KindValidator(), new HarmonyValidator(downloader, moduleService), - new ModuleManagerDependsValidator(downloader, moduleService), - new PluginsValidator(downloader, moduleService), + new ModuleManagerDependsValidator(downloader, moduleService, configParser), + new PluginsValidator(downloader, moduleService, configParser), + new ForClauseValidator(downloader, moduleService, configParser), new CraftsInShipsValidator(downloader, moduleService), }; } @@ -40,8 +39,5 @@ public void ValidateCkan(Metadata metadata, Metadata netkan) Validate(metadata); new MatchingIdentifiersValidator(netkan.Identifier).Validate(metadata); } - - private IHttpService downloader; - private IModuleService moduleService; } } diff --git a/Netkan/Validators/ForClauseValidator.cs b/Netkan/Validators/ForClauseValidator.cs new file mode 100644 index 0000000000..6168698742 --- /dev/null +++ b/Netkan/Validators/ForClauseValidator.cs @@ -0,0 +1,60 @@ +using System.Linq; + +using Newtonsoft.Json.Linq; +using ICSharpCode.SharpZipLib.Zip; +using log4net; + +using CKAN.NetKAN.Services; +using CKAN.NetKAN.Model; +using CKAN.Games; + +namespace CKAN.NetKAN.Validators +{ + internal sealed class ForClauseValidator : IValidator + { + public ForClauseValidator(IHttpService http, IModuleService moduleService, IConfigParser parser) + { + _http = http; + _moduleService = moduleService; + _parser = parser; + } + + public void Validate(Metadata metadata) + { + Log.Info("Validating that :FOR[] clauses specify the right mod"); + + JObject json = metadata.Json(); + CkanModule mod = CkanModule.FromJson(json.ToString()); + if (!mod.IsDLC) + { + var package = _http.DownloadModule(metadata); + if (!string.IsNullOrEmpty(package)) + { + ZipFile zip = new ZipFile(package); + GameInstance inst = new GameInstance(new KerbalSpaceProgram(), "/", "dummy", new NullUser()); + + // Check for :FOR[identifier] in .cfg files + var mismatchedIdentifiers = KerbalSpaceProgram + .IdentifiersFromConfigNodes( + _parser.GetConfigNodes(mod, zip, inst) + .SelectMany(kvp => kvp.Value)) + .Where(ident => ident != mod.identifier + && Identifier.ValidIdentifierPattern.IsMatch(ident)) + .OrderBy(s => s) + .ToArray(); + if (mismatchedIdentifiers.Any()) + { + Log.WarnFormat("Found :FOR[] clauses with the wrong identifiers: {0}", + string.Join(", ", mismatchedIdentifiers)); + } + } + } + } + + private readonly IHttpService _http; + private readonly IModuleService _moduleService; + private readonly IConfigParser _parser; + + private static readonly ILog Log = LogManager.GetLogger(typeof(ForClauseValidator)); + } +} diff --git a/Netkan/Validators/ModuleManagerDependsValidator.cs b/Netkan/Validators/ModuleManagerDependsValidator.cs index 58d1e9cbda..984ccc47e7 100644 --- a/Netkan/Validators/ModuleManagerDependsValidator.cs +++ b/Netkan/Validators/ModuleManagerDependsValidator.cs @@ -1,9 +1,11 @@ using System.IO; using System.Linq; using System.Text.RegularExpressions; + using Newtonsoft.Json.Linq; using ICSharpCode.SharpZipLib.Zip; using log4net; + using CKAN.NetKAN.Services; using CKAN.NetKAN.Model; using CKAN.Extensions; @@ -13,10 +15,11 @@ namespace CKAN.NetKAN.Validators { internal sealed class ModuleManagerDependsValidator : IValidator { - public ModuleManagerDependsValidator(IHttpService http, IModuleService moduleService) + public ModuleManagerDependsValidator(IHttpService http, IModuleService moduleService, IConfigParser parser) { _http = http; _moduleService = moduleService; + _parser = parser; } public void Validate(Metadata metadata) @@ -32,10 +35,10 @@ public void Validate(Metadata metadata) { ZipFile zip = new ZipFile(package); GameInstance inst = new GameInstance(new KerbalSpaceProgram(), "/", "dummy", new NullUser()); - var mmConfigs = _moduleService.GetConfigFiles(mod, zip, inst) - .Where(cfg => moduleManagerRegex.IsMatch( - new StreamReader(zip.GetInputStream(cfg.source)).ReadToEnd())) - .Memoize(); + var mmConfigs = _parser.GetConfigNodes(mod, zip, inst) + .Where(kvp => kvp.Value.Any(node => HasAnyModuleManager(node))) + .Select(kvp => kvp.Key) + .ToArray(); bool dependsOnMM = mod?.depends?.Any(r => r.ContainsAny(identifiers)) ?? false; @@ -56,13 +59,25 @@ public void Validate(Metadata metadata) private string[] identifiers = new string[] { "ModuleManager" }; - private static readonly Regex moduleManagerRegex = new Regex( - @"^\s*[@+$\-!%]|^\s*[a-zA-Z0-9_]+:", - RegexOptions.Compiled | RegexOptions.Multiline | RegexOptions.Singleline - ); + private static bool HasAnyModuleManager(KSPConfigNode node) + => node.Operator != MMOperator.Insert + || node.Filters != null + || node.Needs != null + || node.Has != null + || node.Index != null + || node.Properties.Any(prop => HasAnyModuleManager(prop)) + || node.Children.Any( child => HasAnyModuleManager(child)); + + private static bool HasAnyModuleManager(KSPConfigProperty prop) + => prop.Operator != MMOperator.Insert + || prop.Needs != null + || prop.Index != null + || prop.ArrayIndex != null + || prop.AssignmentOperator != null; private readonly IHttpService _http; private readonly IModuleService _moduleService; + private readonly IConfigParser _parser; private static readonly ILog Log = LogManager.GetLogger(typeof(ModuleManagerDependsValidator)); } diff --git a/Netkan/Validators/PluginsValidator.cs b/Netkan/Validators/PluginsValidator.cs index c4e25f3dc1..46f7e1c5db 100644 --- a/Netkan/Validators/PluginsValidator.cs +++ b/Netkan/Validators/PluginsValidator.cs @@ -1,9 +1,10 @@ using System.IO; using System.Linq; -using System.Text.RegularExpressions; + using Newtonsoft.Json.Linq; using ICSharpCode.SharpZipLib.Zip; using log4net; + using CKAN.Extensions; using CKAN.NetKAN.Services; using CKAN.NetKAN.Model; @@ -13,10 +14,11 @@ namespace CKAN.NetKAN.Validators { internal sealed class PluginsValidator : IValidator { - public PluginsValidator(IHttpService http, IModuleService moduleService) + public PluginsValidator(IHttpService http, IModuleService moduleService, IConfigParser parser) { _http = http; _moduleService = moduleService; + _parser = parser; } public void Validate(Metadata metadata) @@ -42,15 +44,22 @@ public void Validate(Metadata metadata) .OrderBy(f => f) .ToList(); var dllIdentifiers = dllPaths - .Select(p => inst.DllPathToIdentifier(p)) + .SelectMany(p => inst.game.IdentifiersFromFileName(inst, p)) .Where(ident => !string.IsNullOrEmpty(ident) && !identifiersToIgnore.Contains(ident)) .ToHashSet(); if (dllIdentifiers.Any() && !dllIdentifiers.Contains(metadata.Identifier)) { - Log.WarnFormat( - "No plugin matching the identifier, manual installations won't be detected: {0}", - string.Join(", ", dllPaths)); + // Check for :FOR[identifier] in .cfg files + var cfgIdentifiers = KerbalSpaceProgram.IdentifiersFromConfigNodes( + _parser.GetConfigNodes(mod, zip, inst) + .SelectMany(kvp => kvp.Value)); + if (!cfgIdentifiers.Contains(metadata.Identifier)) + { + Log.WarnFormat( + "No plugin or :FOR[] clause matching the identifier, manual installations won't be detected: {0}", + string.Join(", ", dllPaths)); + } } bool boundedCompatibility = json.ContainsKey("ksp_version") || json.ContainsKey("ksp_version_max"); @@ -76,6 +85,7 @@ public void Validate(Metadata metadata) private readonly IHttpService _http; private readonly IModuleService _moduleService; + private readonly IConfigParser _parser; private static readonly ILog Log = LogManager.GetLogger(typeof(PluginsValidator)); } diff --git a/Tests/Core/Games/KerbalSpaceProgram/ConfigParser.cs b/Tests/Core/Games/KerbalSpaceProgram/ConfigParser.cs index 3e97fdc237..c5fddf75a6 100644 --- a/Tests/Core/Games/KerbalSpaceProgram/ConfigParser.cs +++ b/Tests/Core/Games/KerbalSpaceProgram/ConfigParser.cs @@ -363,16 +363,16 @@ public void ConfigNodeParse_MultipleNames_Works() ConfigNode.Parse( @"@PART[KA_Engine_125_02|KA_Engine_250_02|KA_Engine_625_02]:NEEDS[UmbraSpaceIndustries/KarbonitePlus] { - @MODULE[ModuleEngines*] - { - @atmosphereCurve - { - !key,* = nope - key = 0 10000 -17578.79 -17578.79 - key = 1 1500 -1210.658 -1210.658 - key = 4 0.001 0 0 - } - } + @MODULE[ModuleEngines*] + { + @atmosphereCurve + { + !key,* = nope + key = 0 10000 -17578.79 -17578.79 + key = 1 1500 -1210.658 -1210.658 + key = 4 0.001 0 0 + } + } }" ).WillSucceed(v => { @@ -503,6 +503,16 @@ public void ConfigFileParse_NestedNode_Works() // Comment at the end of a node } + + @PART[*]:HAS[#engineType[EXAMPLE]]:FOR[RealismOverhaulEngines]:NEEDS[DONOTRUNME] + { + !MODULE[ModuleEngineConfigs],*{} // Comment between a one line node and another node + + //If the original engine doesn't have a gimbal, you must set up a module gimbal for it first + @MODULE[ModuleGimbal] + { + } + } // Another top level comment diff --git a/Tests/NetKAN/Validators/CkanValidatorTests.cs b/Tests/NetKAN/Validators/CkanValidatorTests.cs index 5a27a64565..7678f10538 100644 --- a/Tests/NetKAN/Validators/CkanValidatorTests.cs +++ b/Tests/NetKAN/Validators/CkanValidatorTests.cs @@ -33,11 +33,12 @@ public void DoesNotThrowOnValidCkan() // Arrange var mHttp = new Mock(); var mModuleService = new Mock(); + var mConfigParser = new Mock(); mModuleService.Setup(i => i.HasInstallableFiles(It.IsAny(), It.IsAny())) .Returns(true); - var sut = new CkanValidator(mHttp.Object, mModuleService.Object); + var sut = new CkanValidator(mHttp.Object, mModuleService.Object, mConfigParser.Object); var json = (JObject)ValidCkan.DeepClone(); // Act @@ -58,11 +59,12 @@ public void DoesThrowWhenMissingProperty(string propertyName) // Arrange var mHttp = new Mock(); var mModuleService = new Mock(); + var mConfigParser = new Mock(); mModuleService.Setup(i => i.HasInstallableFiles(It.IsAny(), It.IsAny())) .Returns(true); - var sut = new CkanValidator(mHttp.Object, mModuleService.Object); + var sut = new CkanValidator(mHttp.Object, mModuleService.Object, mConfigParser.Object); var json = (JObject)ValidCkan.DeepClone(); json.Remove(propertyName); @@ -81,11 +83,12 @@ public void DoesThrowWhenIdentifiersDoNotMatch() // Arrange var mHttp = new Mock(); var mModuleService = new Mock(); + var mConfigParser = new Mock(); mModuleService.Setup(i => i.HasInstallableFiles(It.IsAny(), It.IsAny())) .Returns(true); - var sut = new CkanValidator(mHttp.Object, mModuleService.Object); + var sut = new CkanValidator(mHttp.Object, mModuleService.Object, mConfigParser.Object); var json = new JObject(); json["spec_version"] = 1; json["identifier"] = "AmazingMod"; @@ -105,6 +108,7 @@ public void DoesThrowWhenNoInstallableFiles() // Arrange var mHttp = new Mock(); var mModuleService = new Mock(); + var mConfigParser = new Mock(); mModuleService.Setup(i => i.HasInstallableFiles(It.IsAny(), It.IsAny())) .Returns(false); @@ -113,7 +117,7 @@ public void DoesThrowWhenNoInstallableFiles() netkan["spec_version"] = 1; netkan["identifier"] = "AwesomeMod"; - var sut = new CkanValidator(mHttp.Object, mModuleService.Object); + var sut = new CkanValidator(mHttp.Object, mModuleService.Object, mConfigParser.Object); var json = (JObject)ValidCkan.DeepClone(); // Act