From 1820aea9fda37fe43b64f906fecd4d2e5a0c5b5c Mon Sep 17 00:00:00 2001 From: Eideren Date: Tue, 9 Jan 2024 08:56:17 +0100 Subject: [PATCH 1/5] [Serialization] More useful throw when missing parameterless ctor --- .../Serialization/DefaultObjectFactory.cs | 8 +++++--- .../core/Stride.Core.Yaml/Serialization/IObjectFactory.cs | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs b/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs index 51fe1112c0..fc6a4e7b10 100644 --- a/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs +++ b/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs @@ -99,13 +99,14 @@ public static Type GetDefaultImplementation(Type type) return type; } + /// public object Create(Type type) { type = GetDefaultImplementation(type); - // We can't instantiate primitive or arrays + // We can't instantiate primitives or arrays if (PrimitiveDescriptor.IsPrimitive(type) || type.IsArray) - return null; + throw new InstanceCreationException($"Failed to create instance of type '{type}', wrong factory."); if (type.GetConstructor(EmptyTypes) != null || type.IsValueType) { @@ -119,11 +120,12 @@ public object Create(Type type) } } - return null; + throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor."); } public class InstanceCreationException : Exception { + public InstanceCreationException(string message) : base(message) { } public InstanceCreationException(string message, Exception innerException) : base(message, innerException) { } } } diff --git a/sources/core/Stride.Core.Yaml/Serialization/IObjectFactory.cs b/sources/core/Stride.Core.Yaml/Serialization/IObjectFactory.cs index f22da8bb9f..30f5ec0ac3 100644 --- a/sources/core/Stride.Core.Yaml/Serialization/IObjectFactory.cs +++ b/sources/core/Stride.Core.Yaml/Serialization/IObjectFactory.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2015 SharpYaml - Alexandre Mutel +// Copyright (c) 2015 SharpYaml - Alexandre Mutel // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -56,8 +56,8 @@ namespace Stride.Core.Yaml.Serialization public interface IObjectFactory { /// - /// Creates an instance of the specified type. Returns null if instance cannot be created. + /// Creates an instance of the specified type. Throws with an appropriate exception if the type cannot be created. /// object Create(Type type); } -} \ No newline at end of file +} From 44f862867526749dd6b9516af5cda06a6c8ddedc Mon Sep 17 00:00:00 2001 From: Eideren Date: Sat, 20 Jan 2024 16:22:45 +0100 Subject: [PATCH 2/5] Implement test --- .../Serialization/SerializationTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs index 4b7dd38d9e..c33b774ca4 100644 --- a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs +++ b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs @@ -816,6 +816,24 @@ public void DeserializeWithRepeatedSubObjects() Assert.Equal(22, family.Mother.Age); } + [Fact] + public void ThrowWithoutEmptyCtor() + { + try + { + SerializeThenDeserialize(new ClassWithNonEmptyCtor(default)); + } + catch (Exception ex) + { + Assert.True(ex.InnerException is DefaultObjectFactory.InstanceCreationException); + } + } + + class ClassWithNonEmptyCtor + { + public ClassWithNonEmptyCtor(bool parameter) { } + } + [Fact] public void DeserializeEmptyDocument() From 5eeaededb5b87237a459c1fa8e90204e73ef9683 Mon Sep 17 00:00:00 2001 From: Eideren Date: Sat, 20 Jan 2024 18:06:01 +0100 Subject: [PATCH 3/5] Ensure test fails when no exceptions were thrown --- .../Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs index c33b774ca4..173be9ceb3 100644 --- a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs +++ b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs @@ -822,6 +822,7 @@ public void ThrowWithoutEmptyCtor() try { SerializeThenDeserialize(new ClassWithNonEmptyCtor(default)); + Assert.Fail("An exception should have been thrown given that the class cannot be instanced because of the missing empty constructor "); } catch (Exception ex) { From 7b52840d951ac96141eb7d095fe08e35470b6341 Mon Sep 17 00:00:00 2001 From: Eideren Date: Sun, 21 Jan 2024 18:32:18 +0100 Subject: [PATCH 4/5] Swap to IsType --- .../Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs index 173be9ceb3..e8e2abc283 100644 --- a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs +++ b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs @@ -826,7 +826,7 @@ public void ThrowWithoutEmptyCtor() } catch (Exception ex) { - Assert.True(ex.InnerException is DefaultObjectFactory.InstanceCreationException); + Assert.IsType(ex.InnerException); } } From 7b17e7e3037222a4982ab0d87cc4e8337d617dc7 Mon Sep 17 00:00:00 2001 From: Eideren Date: Sun, 21 Jan 2024 19:23:00 +0100 Subject: [PATCH 5/5] Improve messages --- .../Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs | 2 +- .../core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs index e8e2abc283..83d3741494 100644 --- a/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs +++ b/sources/core/Stride.Core.Yaml.Tests/Serialization/SerializationTests.cs @@ -822,7 +822,7 @@ public void ThrowWithoutEmptyCtor() try { SerializeThenDeserialize(new ClassWithNonEmptyCtor(default)); - Assert.Fail("An exception should have been thrown given that the class cannot be instanced because of the missing empty constructor "); + Assert.Fail("An exception should have been thrown by this method before hitting this line, the class provided does not have an empty constructor"); } catch (Exception ex) { diff --git a/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs b/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs index fc6a4e7b10..fba1b3a04b 100644 --- a/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs +++ b/sources/core/Stride.Core.Yaml/Serialization/DefaultObjectFactory.cs @@ -120,7 +120,7 @@ public object Create(Type type) } } - throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor."); + throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not have a parameterless constructor."); } public class InstanceCreationException : Exception