From 15ddb3275126e1546979bf5f37318bb8ac7f1151 Mon Sep 17 00:00:00 2001 From: Matho Camara Date: Thu, 14 Sep 2023 13:57:03 +0200 Subject: [PATCH 1/3] Added a check to catch self reference property Added extra check to avoid abstract class getting initialized --- src/Moryx/Serialization/DefaultSerialization.cs | 3 +++ src/Moryx/Serialization/EntryConvert/EntryConvert.cs | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/src/Moryx/Serialization/DefaultSerialization.cs b/src/Moryx/Serialization/DefaultSerialization.cs index eaaffe791..c5508166f 100644 --- a/src/Moryx/Serialization/DefaultSerialization.cs +++ b/src/Moryx/Serialization/DefaultSerialization.cs @@ -58,6 +58,9 @@ public virtual EntryPrototype[] Prototypes(Type memberType, ICustomAttributeProv } else { + // check if this member is abstract + if (memberType.IsAbstract) return prototypes.ToArray(); + var prototype = Activator.CreateInstance(memberType); if (memberType.IsClass) ValueProviderExecutor.Execute(prototype, new ValueProviderExecutorSettings().AddDefaultValueProvider()); diff --git a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs index c544a033c..7092676ce 100644 --- a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs +++ b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs @@ -271,6 +271,13 @@ public static Entry EncodeObject(object instance, ICustomSerialization customSer var filtered = customSerialization.GetProperties(instance.GetType()); foreach (var property in filtered) { + + var propertyType = property.GetValue(instance)?.GetType(); + //check if this property is a self reference (Descriptor) + // then skip it to avoid endless loop + var flag = propertyType == instanceType; + if (flag) continue; + var convertedProperty = EncodeProperty(property, customSerialization); object value; From e2dcc8a83367408ebe93f8166e756791a28d441d Mon Sep 17 00:00:00 2001 From: Matho Camara Date: Fri, 15 Sep 2023 08:59:56 +0200 Subject: [PATCH 2/3] Extended unit test to handle new case --- .../DocumentManager.cs | 17 ++++++++++ .../EntryConvert/EntryConvert.cs | 16 +++++---- .../EntrySerializeSerialization.cs | 9 ++++- .../Serialization/EntrySerializeDummies.cs | 26 ++++++++++++++ .../Serialization/SerializationTests.cs | 34 +++++++++++++++++++ 5 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 src/Moryx.Resources.Samples/DocumentManager.cs diff --git a/src/Moryx.Resources.Samples/DocumentManager.cs b/src/Moryx.Resources.Samples/DocumentManager.cs new file mode 100644 index 000000000..b9e838fd2 --- /dev/null +++ b/src/Moryx.Resources.Samples/DocumentManager.cs @@ -0,0 +1,17 @@ +using Moryx.AbstractionLayer.Resources; +using Moryx.Serialization; +using System.Collections.Generic; +using System.ComponentModel; +using System.Runtime.Serialization; + +namespace Moryx.Resources.Demo +{ + [ResourceRegistration] + [EntrySerialize] + public class DocumentManager : PublicResource + { + + [EntrySerialize] + public string FolderPath { get; set; } + } +} diff --git a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs index 7092676ce..d01790c8f 100644 --- a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs +++ b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs @@ -271,12 +271,16 @@ public static Entry EncodeObject(object instance, ICustomSerialization customSer var filtered = customSerialization.GetProperties(instance.GetType()); foreach (var property in filtered) { - - var propertyType = property.GetValue(instance)?.GetType(); - //check if this property is a self reference (Descriptor) - // then skip it to avoid endless loop - var flag = propertyType == instanceType; - if (flag) continue; + //exclude property that can lead to self reference + var propertyType = property.PropertyType; + if (propertyType == instanceType) + continue; + if(propertyType == typeof(Object)) + { + var propertyValue = property.GetValue(instance); + if (propertyValue != null && propertyValue?.GetType() == instanceType) + continue; + } var convertedProperty = EncodeProperty(property, customSerialization); diff --git a/src/Moryx/Serialization/EntrySerializeSerialization.cs b/src/Moryx/Serialization/EntrySerializeSerialization.cs index e89508d44..4c4f01b3a 100644 --- a/src/Moryx/Serialization/EntrySerializeSerialization.cs +++ b/src/Moryx/Serialization/EntrySerializeSerialization.cs @@ -66,7 +66,14 @@ public override IEnumerable GetProperties(Type sourceType) { var properties = sourceType.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance) .Where(BasePropertyFilter).Where(ExplicitPropertyFilter).ToArray(); - + var attributeOnClass = sourceType.GetCustomAttribute(false); + var attributeOnClassOrBaseClass = sourceType.GetCustomAttribute(true); + // to-do: remove this in .net8 and use restriction on the EntrySerializeAttribute + //EntrySerialize property on both class and base class + if (attributeOnClass != null && attributeOnClassOrBaseClass != null) + { + properties = properties.Where(property => property.GetCustomAttribute() != null).ToArray(); + } return EntrySerializeAttributeFilter(sourceType, properties); } diff --git a/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs b/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs index ae3bc9aef..40625274b 100644 --- a/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs +++ b/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs @@ -75,6 +75,23 @@ public class EntrySerialize_NeverClassNoMember public string NullMethod2() => "1234"; } + [EntrySerialize] + public class EntrySerialize_AlwaysClassAlwaysMember + { + [EntrySerialize] + public string AlwaysProperty { get; set; } = "123456"; + [EntrySerialize] + public int Property1 { get; set; } + [EntrySerialize] + public int Property2 { get; set; } + + public object Property => this; + + [EntrySerialize] + public EntrySerialize_AlwaysClassAlwaysMember AnotherProperty { get; set; } + internal IExplicitInterface ExplicitInterface { get; } + } + // ReSharper disable once InconsistentNaming [EntrySerialize(EntrySerializeMode.Never)] public class EntrySerialize_NeverClassAlwaysMember @@ -103,6 +120,15 @@ public class EntrySerialize_Inherited : EntrySerialize_InheritedBase public bool NullProperty3 { get; set; } = false; } + [EntrySerialize] + public class AlwaysClass_Inherited : EntrySerialize_InheritedBase + { + [EntrySerialize] + public string NullProperty2 { get; set; } = "789456"; + + public bool NullProperty3 { get; set; } = false; + } + public class EntrySerialize_Methods : EntrySerialize_InheritedBase { [EntrySerialize] diff --git a/src/Tests/Moryx.Tests/Serialization/SerializationTests.cs b/src/Tests/Moryx.Tests/Serialization/SerializationTests.cs index 7c03fa181..7c34efb00 100644 --- a/src/Tests/Moryx.Tests/Serialization/SerializationTests.cs +++ b/src/Tests/Moryx.Tests/Serialization/SerializationTests.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.IO; using System.Linq; +using System.Reflection; using System.Text; using Moryx.Configuration; using Moryx.Serialization; @@ -394,6 +395,39 @@ public void SurviveGetterException() Assert.NotNull(encoded.SubEntries[0].Value.Current); } + + [Test(Description = "Class with EntrySerializationAttribute should not Override the attribute on the Base Class Resource|PublicResource")] + public void ClassWithSerializationAttribute_AndBaseClass() + { + // Arrange + var serialization = new EntrySerializeSerialization { FormatProvider = new CultureInfo("en-US") }; + var dummy = new AlwaysClass_Inherited(); + + // Act + var encoded = EntryConvert.EncodeObject(dummy, serialization); + + // Assert + var alwaysProperties = 1; + Assert.That(encoded.SubEntries.Count, Is.EqualTo(alwaysProperties)); + } + + [Test(Description = "Class with EntrySerializationAttribute should not Override the attribute on the Base Class Resource|PublicResource")] + public void ClassWithSerializationAttribute() + { + // Arrange + var serialization = new EntrySerializeSerialization { FormatProvider = new CultureInfo("en-US") }; + var dummy = new EntrySerialize_AlwaysClassAlwaysMember(); + + // Act + var encoded = EntryConvert.EncodeObject(dummy, serialization); + + // Assert + var alwaysProperties = 3; + Assert.That(encoded.SubEntries.Count, Is.EqualTo(alwaysProperties)); + } + + + [TestCase(CollectionType.Array, 3, 2)] [TestCase(CollectionType.Array, 0, 4)] [TestCase(CollectionType.List, 5, 3)] From 1aa6a409821e8aef01266863272ddcad3c5e7c28 Mon Sep 17 00:00:00 2001 From: Thomas Fuchs Date: Mon, 9 Oct 2023 17:58:20 +0200 Subject: [PATCH 3/3] Correctly handle inheritance of EntrySerializeAttributes --- .../DocumentManager.cs | 17 -------- .../EntryConvert/EntryConvert.cs | 6 --- .../EntrySerializeSerialization.cs | 39 ++++++++++++------- .../Serialization/EntrySerializeDummies.cs | 5 +-- 4 files changed, 25 insertions(+), 42 deletions(-) delete mode 100644 src/Moryx.Resources.Samples/DocumentManager.cs diff --git a/src/Moryx.Resources.Samples/DocumentManager.cs b/src/Moryx.Resources.Samples/DocumentManager.cs deleted file mode 100644 index b9e838fd2..000000000 --- a/src/Moryx.Resources.Samples/DocumentManager.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Moryx.AbstractionLayer.Resources; -using Moryx.Serialization; -using System.Collections.Generic; -using System.ComponentModel; -using System.Runtime.Serialization; - -namespace Moryx.Resources.Demo -{ - [ResourceRegistration] - [EntrySerialize] - public class DocumentManager : PublicResource - { - - [EntrySerialize] - public string FolderPath { get; set; } - } -} diff --git a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs index d01790c8f..b62c19f28 100644 --- a/src/Moryx/Serialization/EntryConvert/EntryConvert.cs +++ b/src/Moryx/Serialization/EntryConvert/EntryConvert.cs @@ -275,12 +275,6 @@ public static Entry EncodeObject(object instance, ICustomSerialization customSer var propertyType = property.PropertyType; if (propertyType == instanceType) continue; - if(propertyType == typeof(Object)) - { - var propertyValue = property.GetValue(instance); - if (propertyValue != null && propertyValue?.GetType() == instanceType) - continue; - } var convertedProperty = EncodeProperty(property, customSerialization); diff --git a/src/Moryx/Serialization/EntrySerializeSerialization.cs b/src/Moryx/Serialization/EntrySerializeSerialization.cs index 4c4f01b3a..f2e347204 100644 --- a/src/Moryx/Serialization/EntrySerializeSerialization.cs +++ b/src/Moryx/Serialization/EntrySerializeSerialization.cs @@ -42,9 +42,9 @@ public EntrySerializeSerialization(Type filterBaseType) public override IEnumerable GetConstructors(Type sourceType) { var constructors = from ctor in base.GetConstructors(sourceType) - let mode = EvaluateSerializeMode(ctor) - where mode.HasValue && mode.Value == EntrySerializeMode.Always - select ctor; + let mode = EvaluateSerializeMode(ctor) + where mode.HasValue && mode.Value == EntrySerializeMode.Always + select ctor; return constructors; } @@ -54,9 +54,9 @@ public override IEnumerable GetMethods(Type sourceType) { var methods = sourceType.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance).Where(m => !m.IsSpecialName); methods = from method in methods - let mode = EvaluateSerializeMode(method) - where mode.HasValue && mode.Value == EntrySerializeMode.Always - select method; + let mode = EvaluateSerializeMode(method) + where mode.HasValue && mode.Value == EntrySerializeMode.Always + select method; return methods; } @@ -66,14 +66,6 @@ public override IEnumerable GetProperties(Type sourceType) { var properties = sourceType.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance) .Where(BasePropertyFilter).Where(ExplicitPropertyFilter).ToArray(); - var attributeOnClass = sourceType.GetCustomAttribute(false); - var attributeOnClassOrBaseClass = sourceType.GetCustomAttribute(true); - // to-do: remove this in .net8 and use restriction on the EntrySerializeAttribute - //EntrySerialize property on both class and base class - if (attributeOnClass != null && attributeOnClassOrBaseClass != null) - { - properties = properties.Where(property => property.GetCustomAttribute() != null).ToArray(); - } return EntrySerializeAttributeFilter(sourceType, properties); } @@ -154,6 +146,23 @@ private static IEnumerable EntrySerializeAttributeFilter(Type sour return properties; } + /// + /// Iterate the inheritance tree and find lowest declaration of the attribute + /// + private static EntrySerializeMode? EvaluateSerializeMode(Type attributeProvider) + { + // If more than 1 is declared, determine the lowest definition as it takes precedence + // For each declaration check assignability to determine lower type + var currentType = attributeProvider; + EntrySerializeAttribute lowestDeclaration = null; + while (currentType != typeof(object)) + { + lowestDeclaration = currentType.GetCustomAttribute(false) ?? lowestDeclaration; + currentType = currentType.BaseType; + } + return lowestDeclaration?.Mode; + } + /// /// Checks if the is existent and activated /// @@ -168,7 +177,7 @@ private static PropertyMode EvaluateSerializeMode(PropertyInfo property) return new PropertyMode { Property = property, - Mode = EvaluateSerializeMode((ICustomAttributeProvider)property) + Mode = property.GetCustomAttribute(true)?.Mode }; } diff --git a/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs b/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs index 40625274b..2a69691bb 100644 --- a/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs +++ b/src/Tests/Moryx.Tests/Serialization/EntrySerializeDummies.cs @@ -82,13 +82,10 @@ public class EntrySerialize_AlwaysClassAlwaysMember public string AlwaysProperty { get; set; } = "123456"; [EntrySerialize] public int Property1 { get; set; } - [EntrySerialize] - public int Property2 { get; set; } - - public object Property => this; [EntrySerialize] public EntrySerialize_AlwaysClassAlwaysMember AnotherProperty { get; set; } + internal IExplicitInterface ExplicitInterface { get; } }