From 49a1fb42f5b7f802d920207b6b48176670b7f6d4 Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Mon, 18 Jul 2022 15:24:01 -0400 Subject: [PATCH] Add custom YamlDotNet emitter that makes all fields with newlines use literal style. (#281) --- src/WingetCreateCore/Common/Serialization.cs | 153 ++++++++++-------- .../UnitTests/CharacterValidationTests.cs | 36 ++++- 2 files changed, 125 insertions(+), 64 deletions(-) diff --git a/src/WingetCreateCore/Common/Serialization.cs b/src/WingetCreateCore/Common/Serialization.cs index ff78c258..0f0dbdf6 100644 --- a/src/WingetCreateCore/Common/Serialization.cs +++ b/src/WingetCreateCore/Common/Serialization.cs @@ -4,10 +4,10 @@ namespace Microsoft.WingetCreateCore { using System; - using System.Collections.Generic; + using System.Collections.Generic; using System.IO; using System.Linq; - using System.Reflection; + using System.Reflection; using System.Runtime.Serialization; using System.Text; using Microsoft.WingetCreateCore.Models; @@ -20,9 +20,10 @@ namespace Microsoft.WingetCreateCore using YamlDotNet.Core; using YamlDotNet.Core.Events; using YamlDotNet.Serialization; - using YamlDotNet.Serialization.NamingConventions; - using YamlDotNet.Serialization.TypeInspectors; - + using YamlDotNet.Serialization.EventEmitters; + using YamlDotNet.Serialization.NamingConventions; + using YamlDotNet.Serialization.TypeInspectors; + /// /// Provides functionality for the serialization of JSON objects to yaml. /// @@ -39,13 +40,14 @@ public static class Serialization /// ISerializer object. public static ISerializer CreateSerializer() { - var serializer = new SerializerBuilder() - .WithNamingConvention(PascalCaseNamingConvention.Instance) + var serializer = new SerializerBuilder() + .WithNamingConvention(PascalCaseNamingConvention.Instance) .WithTypeConverter(new YamlStringEnumConverter()) .WithEmissionPhaseObjectGraphVisitor(args => new YamlSkipPropertyVisitor(args.InnerVisitor)) + .WithEventEmitter(nextEmitter => new MultilineScalarFlowStyleEmitter(nextEmitter)) .ConfigureDefaultValuesHandling(DefaultValuesHandling.OmitNull); return serializer.Build(); - } + } /// /// Helper to build a YAML deserializer. @@ -55,9 +57,9 @@ public static IDeserializer CreateDeserializer() { var deserializer = new DeserializerBuilder() .WithNamingConvention(PascalCaseNamingConvention.Instance) - .WithTypeConverter(new YamlStringEnumConverter()) - .WithTypeInspector(inspector => new AliasTypeInspector(inspector)) - .IgnoreUnmatchedProperties(); + .WithTypeConverter(new YamlStringEnumConverter()) + .WithTypeInspector(inspector => new AliasTypeInspector(inspector)) + .IgnoreUnmatchedProperties(); return deserializer.Build(); } @@ -199,68 +201,68 @@ private static string RemoveBom(string value) { string bomMarkUtf8 = Encoding.UTF8.GetString(Encoding.UTF8.GetPreamble()); return value.StartsWith(bomMarkUtf8, StringComparison.OrdinalIgnoreCase) ? value.Remove(0, bomMarkUtf8.Length) : value; - } - - /// - /// Custom TypeInspector to priorize properties that have a defined YamlMemberAttribute for custom override. - /// - private class AliasTypeInspector : TypeInspectorSkeleton - { - private readonly ITypeInspector innerTypeDescriptor; - - public AliasTypeInspector(ITypeInspector innerTypeDescriptor) - { - this.innerTypeDescriptor = innerTypeDescriptor; - } - - /// - /// Because certain properties were generated incorrectly, we needed to create custom fields for those properties. - /// Therefore to resolve naming conflicts during deserialization, we prioritize fields that have the YamlMemberAttribute defined - /// as that attribute indicates an override. - /// - public override IEnumerable GetProperties(Type type, object container) - { - var propertyDescriptors = this.innerTypeDescriptor.GetProperties(type, container); - var aliasDefinedProps = type.GetProperties().ToList() - .Where(p => - { - var yamlMemberAttribute = p.GetCustomAttribute(); - return yamlMemberAttribute != null && !string.IsNullOrEmpty(yamlMemberAttribute.Alias); - }) - .ToList(); - - if (aliasDefinedProps.Any()) - { - var overriddenProps = propertyDescriptors - .Where(prop => aliasDefinedProps.Any(aliasProp => - prop.Name == aliasProp.GetCustomAttribute().Alias && // Use Alias name (ex. ReleaseDate) instead of property name (ex. ReleaseDateString). - prop.Type != aliasProp.PropertyType)) - .ToList(); - - // Remove overridden properties from the returned list of deserializable properties. - return propertyDescriptors - .Where(prop => !overriddenProps.Any(overridenProp => - prop.Name == overridenProp.Name && - prop.Type == overridenProp.Type)) - .ToList(); - } - else - { - return propertyDescriptors; - } - } + } + + /// + /// Custom TypeInspector to priorize properties that have a defined YamlMemberAttribute for custom override. + /// + private class AliasTypeInspector : TypeInspectorSkeleton + { + private readonly ITypeInspector innerTypeDescriptor; + + public AliasTypeInspector(ITypeInspector innerTypeDescriptor) + { + this.innerTypeDescriptor = innerTypeDescriptor; + } + + /// + /// Because certain properties were generated incorrectly, we needed to create custom fields for those properties. + /// Therefore to resolve naming conflicts during deserialization, we prioritize fields that have the YamlMemberAttribute defined + /// as that attribute indicates an override. + /// + public override IEnumerable GetProperties(Type type, object container) + { + var propertyDescriptors = this.innerTypeDescriptor.GetProperties(type, container); + var aliasDefinedProps = type.GetProperties().ToList() + .Where(p => + { + var yamlMemberAttribute = p.GetCustomAttribute(); + return yamlMemberAttribute != null && !string.IsNullOrEmpty(yamlMemberAttribute.Alias); + }) + .ToList(); + + if (aliasDefinedProps.Any()) + { + var overriddenProps = propertyDescriptors + .Where(prop => aliasDefinedProps.Any(aliasProp => + prop.Name == aliasProp.GetCustomAttribute().Alias && // Use Alias name (ex. ReleaseDate) instead of property name (ex. ReleaseDateString). + prop.Type != aliasProp.PropertyType)) + .ToList(); + + // Remove overridden properties from the returned list of deserializable properties. + return propertyDescriptors + .Where(prop => !overriddenProps.Any(overridenProp => + prop.Name == overridenProp.Name && + prop.Type == overridenProp.Type)) + .ToList(); + } + else + { + return propertyDescriptors; + } + } } private class YamlStringEnumConverter : IYamlTypeConverter { public bool Accepts(Type type) - { + { Type u = Nullable.GetUnderlyingType(type); return type.IsEnum || ((u != null) && u.IsEnum); } public object ReadYaml(IParser parser, Type type) - { + { Type u = Nullable.GetUnderlyingType(type); if (u != null) { @@ -304,5 +306,30 @@ public override bool EnterMapping(IPropertyDescriptor key, IObjectDescriptor val return base.EnterMapping(key, value, context); } } + + /// + /// A custom emitter for YamlDotNet which ensures all multiline fields use a . + /// + private class MultilineScalarFlowStyleEmitter : ChainedEventEmitter + { + public MultilineScalarFlowStyleEmitter(IEventEmitter nextEmitter) : base(nextEmitter) { } + + public override void Emit(ScalarEventInfo eventInfo, IEmitter emitter) + { + if (typeof(string).IsAssignableFrom(eventInfo.Source.Type)) + { + var outString = eventInfo.Source.Value as string; + if (!string.IsNullOrEmpty(outString)) + { + bool isMultiLine = new[] { '\r', '\n', '\x85', '\x2028', '\x2029' }.Any(outString.Contains); + if (isMultiLine) + { + eventInfo = new ScalarEventInfo(eventInfo.Source) {Style = ScalarStyle.Literal}; + } + } + } + this.nextEmitter.Emit(eventInfo, emitter); + } + } } } diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/CharacterValidationTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/CharacterValidationTests.cs index 374789a2..cc123ac2 100644 --- a/src/WingetCreateTests/WingetCreateTests/UnitTests/CharacterValidationTests.cs +++ b/src/WingetCreateTests/WingetCreateTests/UnitTests/CharacterValidationTests.cs @@ -3,11 +3,13 @@ namespace Microsoft.WingetCreateUnitTests { + using System; using System.IO; + using System.Linq; using Microsoft.WingetCreateCore; using Microsoft.WingetCreateCore.Models.Singleton; using NUnit.Framework; - + /// /// Unit tests for verifying unicode text and directionality. /// @@ -48,5 +50,37 @@ public void VerifyTextSupport() File.Delete(testManifestFilePath); } } + + /// + /// Verifies that we aren't adding more newlines than expected. + /// + [Test] + public void VerfiyNewLineSupport() + { + string[] stringsWithNewLines = + { + "This\n has\n some newlines.", + "So\r\n does this.", + "And this does\x85.", + "As does this\x2028.", + "Me too!\x2029:)", + }; + + string testManifestFilePath = Path.Combine(Path.GetTempPath(), "TestManifest.yaml"); + + foreach (var i in stringsWithNewLines) + { + SingletonManifest written = new SingletonManifest { Description = i }; + File.WriteAllText(testManifestFilePath, written.ToYaml()); + SingletonManifest read = Serialization.DeserializeFromPath(testManifestFilePath); + + // we know when written that \r\n and \x85 characters are replaced with \n. + var writtenFixed = string.Join('\n', written.Description.Split(new string[] { "\n", "\r\n", "\x85" }, StringSplitOptions.None)); + + Assert.AreEqual(writtenFixed, read.Description, $"String {read.Description} had the wrong number of newlines :(."); + File.Delete(testManifestFilePath); + } + } + } }