From 61d6ec12daee843f758e5f828a713a72a767a94b Mon Sep 17 00:00:00 2001 From: Tyler Date: Tue, 18 Oct 2022 20:03:28 -0500 Subject: add detailed manifest validation errors at build time --- src/SMAPI.Toolkit/Serialization/Models/Manifest.cs | 96 ++++++++++++++++++++++ 1 file changed, 96 insertions(+) (limited to 'src/SMAPI.Toolkit') diff --git a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs index 8a449f0a..4f84a60d 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs @@ -1,9 +1,12 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Linq; using System.Text; using Newtonsoft.Json; using StardewModdingAPI.Toolkit.Serialization.Converters; +using StardewModdingAPI.Toolkit.Utilities; namespace StardewModdingAPI.Toolkit.Serialization.Models { @@ -103,6 +106,99 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models this.UpdateKeys = updateKeys ?? Array.Empty(); } + /// Try to validate a manifest's fields. Fails if any invalid field is found. + /// The error message to display to the user. + /// Returns whether the manifest was validated successfully. + public bool TryValidate(out string error) + { + // validate DLL / content pack fields + bool hasDll = !string.IsNullOrWhiteSpace(this.EntryDll); + bool isContentPack = this.ContentPackFor != null; + + // validate field presence + if (!hasDll && !isContentPack) + { + error = $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one."; + return false; + } + if (hasDll && isContentPack) + { + error = $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive."; + return false; + } + + // validate DLL filename format + if (hasDll && this.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any()) + { + error = $"manifest has invalid filename '{this.EntryDll}' for the EntryDLL field."; + return false; + } + + // validate content pack + else if (isContentPack) + { + // invalid content pack ID + if (string.IsNullOrWhiteSpace(this.ContentPackFor!.UniqueID)) + { + error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; + return false; + } + } + + // validate required fields + { + List missingFields = new List(3); + + if (string.IsNullOrWhiteSpace(this.Name)) + missingFields.Add(nameof(IManifest.Name)); + if (this.Version == null || this.Version.ToString() == "0.0.0") + missingFields.Add(nameof(IManifest.Version)); + if (string.IsNullOrWhiteSpace(this.UniqueID)) + missingFields.Add(nameof(IManifest.UniqueID)); + + if (missingFields.Any()) + { + error = $"manifest is missing required fields ({string.Join(", ", missingFields)})."; + return false; + } + } + + // validate ID format + if (!PathUtilities.IsSlug(this.UniqueID)) + { + error = "manifest specifies an invalid ID (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; + return false; + } + + // validate dependencies + foreach (IManifestDependency? dependency in this.Dependencies) + { + // null dependency + if (dependency == null) + { + error = $"manifest has a null entry under {nameof(IManifest.Dependencies)}."; + return false; + } + + // missing ID + if (string.IsNullOrWhiteSpace(dependency.UniqueID)) + { + error = $"manifest has a {nameof(IManifest.Dependencies)} entry with no {nameof(IManifestDependency.UniqueID)} field."; + return false; + } + + // invalid ID + if (!PathUtilities.IsSlug(dependency.UniqueID)) + { + error = $"manifest has a {nameof(IManifest.Dependencies)} entry with an invalid {nameof(IManifestDependency.UniqueID)} field (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; + return false; + } + } + + error = ""; + return true; + } + /// Override the update keys loaded from the mod info. /// The new update keys to set. internal void OverrideUpdateKeys(params string[] updateKeys) -- cgit From 55eec58eafb9ba07f3e8b0a1c8394cb114de17a0 Mon Sep 17 00:00:00 2001 From: Tyler Date: Wed, 19 Oct 2022 10:21:19 -0500 Subject: simplify ContentPackFor validation check --- src/SMAPI.Toolkit/Serialization/Models/Manifest.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'src/SMAPI.Toolkit') diff --git a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs index 4f84a60d..1607cf3e 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs @@ -134,15 +134,11 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models return false; } - // validate content pack - else if (isContentPack) + // validate content pack ID + else if (isContentPack && string.IsNullOrWhiteSpace(this.ContentPackFor!.UniqueID)) { - // invalid content pack ID - if (string.IsNullOrWhiteSpace(this.ContentPackFor!.UniqueID)) - { - error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; - return false; - } + error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; + return false; } // validate required fields -- cgit From 346fddda670704c1458e42104ee7405fc1de7ccc Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 10 Nov 2022 23:27:38 -0500 Subject: move validation logic out of Manifest model This avoids tightly coupling higher logic to the implementation class, since we can validate the interface. --- src/SMAPI.Toolkit/Framework/ManifestValidator.cs | 101 +++++++++++++++++++++ src/SMAPI.Toolkit/Serialization/Models/Manifest.cs | 92 ------------------- 2 files changed, 101 insertions(+), 92 deletions(-) create mode 100644 src/SMAPI.Toolkit/Framework/ManifestValidator.cs (limited to 'src/SMAPI.Toolkit') diff --git a/src/SMAPI.Toolkit/Framework/ManifestValidator.cs b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs new file mode 100644 index 00000000..62cfd8e9 --- /dev/null +++ b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs @@ -0,0 +1,101 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using StardewModdingAPI.Toolkit.Utilities; + +namespace StardewModdingAPI.Toolkit.Framework +{ + /// Validates manifest fields. + public static class ManifestValidator + { + /// Try to validate a manifest's fields. Fails if any invalid field is found. + /// The manifest to validate. + /// The error message to display to the user. + /// Returns whether the manifest was validated successfully. + public static bool TryValidate(IManifest manifest, out string error) + { + // validate DLL / content pack fields + bool hasDll = !string.IsNullOrWhiteSpace(manifest.EntryDll); + bool isContentPack = manifest.ContentPackFor != null; + + // validate field presence + if (!hasDll && !isContentPack) + { + error = $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one."; + return false; + } + if (hasDll && isContentPack) + { + error = $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive."; + return false; + } + + // validate DLL filename format + if (hasDll && manifest.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any()) + { + error = $"manifest has invalid filename '{manifest.EntryDll}' for the EntryDLL field."; + return false; + } + + // validate content pack ID + else if (isContentPack && string.IsNullOrWhiteSpace(manifest.ContentPackFor!.UniqueID)) + { + error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; + return false; + } + + // validate required fields + { + List missingFields = new List(3); + + if (string.IsNullOrWhiteSpace(manifest.Name)) + missingFields.Add(nameof(IManifest.Name)); + if (manifest.Version == null || manifest.Version.ToString() == "0.0.0") + missingFields.Add(nameof(IManifest.Version)); + if (string.IsNullOrWhiteSpace(manifest.UniqueID)) + missingFields.Add(nameof(IManifest.UniqueID)); + + if (missingFields.Any()) + { + error = $"manifest is missing required fields ({string.Join(", ", missingFields)})."; + return false; + } + } + + // validate ID format + if (!PathUtilities.IsSlug(manifest.UniqueID)) + { + error = "manifest specifies an invalid ID (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; + return false; + } + + // validate dependencies + foreach (IManifestDependency? dependency in manifest.Dependencies) + { + // null dependency + if (dependency == null) + { + error = $"manifest has a null entry under {nameof(IManifest.Dependencies)}."; + return false; + } + + // missing ID + if (string.IsNullOrWhiteSpace(dependency.UniqueID)) + { + error = $"manifest has a {nameof(IManifest.Dependencies)} entry with no {nameof(IManifestDependency.UniqueID)} field."; + return false; + } + + // invalid ID + if (!PathUtilities.IsSlug(dependency.UniqueID)) + { + error = $"manifest has a {nameof(IManifest.Dependencies)} entry with an invalid {nameof(IManifestDependency.UniqueID)} field (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; + return false; + } + } + + error = ""; + return true; + } + } +} diff --git a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs index 1607cf3e..8a449f0a 100644 --- a/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs +++ b/src/SMAPI.Toolkit/Serialization/Models/Manifest.cs @@ -1,12 +1,9 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; using System.Text; using Newtonsoft.Json; using StardewModdingAPI.Toolkit.Serialization.Converters; -using StardewModdingAPI.Toolkit.Utilities; namespace StardewModdingAPI.Toolkit.Serialization.Models { @@ -106,95 +103,6 @@ namespace StardewModdingAPI.Toolkit.Serialization.Models this.UpdateKeys = updateKeys ?? Array.Empty(); } - /// Try to validate a manifest's fields. Fails if any invalid field is found. - /// The error message to display to the user. - /// Returns whether the manifest was validated successfully. - public bool TryValidate(out string error) - { - // validate DLL / content pack fields - bool hasDll = !string.IsNullOrWhiteSpace(this.EntryDll); - bool isContentPack = this.ContentPackFor != null; - - // validate field presence - if (!hasDll && !isContentPack) - { - error = $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one."; - return false; - } - if (hasDll && isContentPack) - { - error = $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive."; - return false; - } - - // validate DLL filename format - if (hasDll && this.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any()) - { - error = $"manifest has invalid filename '{this.EntryDll}' for the EntryDLL field."; - return false; - } - - // validate content pack ID - else if (isContentPack && string.IsNullOrWhiteSpace(this.ContentPackFor!.UniqueID)) - { - error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; - return false; - } - - // validate required fields - { - List missingFields = new List(3); - - if (string.IsNullOrWhiteSpace(this.Name)) - missingFields.Add(nameof(IManifest.Name)); - if (this.Version == null || this.Version.ToString() == "0.0.0") - missingFields.Add(nameof(IManifest.Version)); - if (string.IsNullOrWhiteSpace(this.UniqueID)) - missingFields.Add(nameof(IManifest.UniqueID)); - - if (missingFields.Any()) - { - error = $"manifest is missing required fields ({string.Join(", ", missingFields)})."; - return false; - } - } - - // validate ID format - if (!PathUtilities.IsSlug(this.UniqueID)) - { - error = "manifest specifies an invalid ID (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; - return false; - } - - // validate dependencies - foreach (IManifestDependency? dependency in this.Dependencies) - { - // null dependency - if (dependency == null) - { - error = $"manifest has a null entry under {nameof(IManifest.Dependencies)}."; - return false; - } - - // missing ID - if (string.IsNullOrWhiteSpace(dependency.UniqueID)) - { - error = $"manifest has a {nameof(IManifest.Dependencies)} entry with no {nameof(IManifestDependency.UniqueID)} field."; - return false; - } - - // invalid ID - if (!PathUtilities.IsSlug(dependency.UniqueID)) - { - error = $"manifest has a {nameof(IManifest.Dependencies)} entry with an invalid {nameof(IManifestDependency.UniqueID)} field (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; - return false; - } - } - - error = ""; - return true; - } - /// Override the update keys loaded from the mod info. /// The new update keys to set. internal void OverrideUpdateKeys(params string[] updateKeys) -- cgit From 867afdd96ff8896dc81fdab204cf045713d32d91 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Thu, 10 Nov 2022 23:27:38 -0500 Subject: tweak new code --- src/SMAPI.Toolkit/Framework/ManifestValidator.cs | 57 +++++++++++++----------- 1 file changed, 31 insertions(+), 26 deletions(-) (limited to 'src/SMAPI.Toolkit') diff --git a/src/SMAPI.Toolkit/Framework/ManifestValidator.cs b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs index 62cfd8e9..461dc325 100644 --- a/src/SMAPI.Toolkit/Framework/ManifestValidator.cs +++ b/src/SMAPI.Toolkit/Framework/ManifestValidator.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using StardewModdingAPI.Toolkit.Utilities; @@ -8,40 +9,47 @@ namespace StardewModdingAPI.Toolkit.Framework /// Validates manifest fields. public static class ManifestValidator { - /// Try to validate a manifest's fields. Fails if any invalid field is found. + /// Validate a manifest's fields. /// The manifest to validate. - /// The error message to display to the user. - /// Returns whether the manifest was validated successfully. - public static bool TryValidate(IManifest manifest, out string error) + /// The error message indicating why validation failed, if applicable. + /// Returns whether all manifest fields validated successfully. + [SuppressMessage("ReSharper", "ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract", Justification = "This is the method that ensures those annotations are respected.")] + public static bool TryValidateFields(IManifest manifest, out string error) { - // validate DLL / content pack fields + // + // Note: SMAPI assumes that it can grammatically append the returned sentence in the + // form "failed loading because its ". Any errors returned should be valid + // in that format, unless the SMAPI call is adjusted accordingly. + // + bool hasDll = !string.IsNullOrWhiteSpace(manifest.EntryDll); bool isContentPack = manifest.ContentPackFor != null; - // validate field presence - if (!hasDll && !isContentPack) - { - error = $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one."; - return false; - } - if (hasDll && isContentPack) + // validate use of EntryDll vs ContentPackFor fields + if (hasDll == isContentPack) { - error = $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive."; + error = hasDll + ? $"manifest sets both {nameof(IManifest.EntryDll)} and {nameof(IManifest.ContentPackFor)}, which are mutually exclusive." + : $"manifest has no {nameof(IManifest.EntryDll)} or {nameof(IManifest.ContentPackFor)} field; must specify one."; return false; } - // validate DLL filename format - if (hasDll && manifest.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any()) + // validate EntryDll/ContentPackFor format + if (hasDll) { - error = $"manifest has invalid filename '{manifest.EntryDll}' for the EntryDLL field."; - return false; + if (manifest.EntryDll!.Intersect(Path.GetInvalidFileNameChars()).Any()) + { + error = $"manifest has invalid filename '{manifest.EntryDll}' for the {nameof(IManifest.EntryDll)} field."; + return false; + } } - - // validate content pack ID - else if (isContentPack && string.IsNullOrWhiteSpace(manifest.ContentPackFor!.UniqueID)) + else { - error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; - return false; + if (string.IsNullOrWhiteSpace(manifest.ContentPackFor!.UniqueID)) + { + error = $"manifest declares {nameof(IManifest.ContentPackFor)} without its required {nameof(IManifestContentPackFor.UniqueID)} field."; + return false; + } } // validate required fields @@ -69,24 +77,21 @@ namespace StardewModdingAPI.Toolkit.Framework return false; } - // validate dependencies + // validate dependency format foreach (IManifestDependency? dependency in manifest.Dependencies) { - // null dependency if (dependency == null) { error = $"manifest has a null entry under {nameof(IManifest.Dependencies)}."; return false; } - // missing ID if (string.IsNullOrWhiteSpace(dependency.UniqueID)) { error = $"manifest has a {nameof(IManifest.Dependencies)} entry with no {nameof(IManifestDependency.UniqueID)} field."; return false; } - // invalid ID if (!PathUtilities.IsSlug(dependency.UniqueID)) { error = $"manifest has a {nameof(IManifest.Dependencies)} entry with an invalid {nameof(IManifestDependency.UniqueID)} field (IDs must only contain letters, numbers, underscores, periods, or hyphens)."; -- cgit