From 02904441406f72825dad995b0518e78f0e902018 Mon Sep 17 00:00:00 2001 From: Markus Ewald Date: Wed, 8 Feb 2012 19:26:05 +0000 Subject: [PATCH] Fixed a bug in the cloners: if a class has properties with only a setter or only a getter, the property-based clones now ignore that property; added a unit test that verifies types without a default constructor can be cloned as well git-svn-id: file:///srv/devel/repo-conversion/nusu@248 d2e56fa2-650e-0410-a79f-9358c0239efd --- Source/Cloning/CloneFactoryTest.cs | 41 +++++ .../ExpressionTreeCloner.PropertyBased.cs | 154 +++++++++--------- Source/Cloning/ExpressionTreeCloner.Test.cs | 13 ++ Source/Cloning/ReflectionCloner.Test.cs | 13 ++ Source/Cloning/ReflectionCloner.cs | 58 ++++--- Source/Cloning/SerializationCloner.Test.cs | 13 ++ Source/Cloning/SerializationCloner.cs | 16 +- 7 files changed, 200 insertions(+), 108 deletions(-) diff --git a/Source/Cloning/CloneFactoryTest.cs b/Source/Cloning/CloneFactoryTest.cs index 1dd5d9c..7c99fbf 100644 --- a/Source/Cloning/CloneFactoryTest.cs +++ b/Source/Cloning/CloneFactoryTest.cs @@ -78,6 +78,14 @@ namespace Nuclex.Support.Cloning { public TestReferenceType[,][] ReferenceTypeArrayField; /// An array property of reference types public TestReferenceType[,][] ReferenceTypeArrayProperty { get; set; } + /// A reference type field that's always null + public TestReferenceType AlwaysNullField; + /// A reference type property that's always null + public TestReferenceType AlwaysNullProperty { get; set; } + /// A property that only has a getter + public TestReferenceType GetOnlyProperty { get { return null; } } + /// A property that only has a s + public TestReferenceType SetOnlyProperty { set { } } } @@ -104,11 +112,44 @@ namespace Nuclex.Support.Cloning { public TestReferenceType[,][] ReferenceTypeArrayField; /// An array property of reference types public TestReferenceType[,][] ReferenceTypeArrayProperty { get; set; } + /// A reference type field that's always null + public TestReferenceType AlwaysNullField; + /// A reference type property that's always null + public TestReferenceType AlwaysNullProperty { get; set; } + /// A property that only has a getter + public TestReferenceType GetOnlyProperty { get { return null; } } + /// A property that only has a s + public TestReferenceType SetOnlyProperty { set { } } } #endregion // struct HierarchicalReferenceType + #region class ClassWithoutDefaultConstructor + + /// A class that does not have a default constructor + public class ClassWithoutDefaultConstructor { + + /// + /// Initializes a new instance of the class without default constructor + /// + /// Dummy value that will be saved by the instance + public ClassWithoutDefaultConstructor(int dummy) { + this.dummy = dummy; + } + + /// Dummy value that has been saved by the instance + public int Dummy { + get { return this.dummy; } + } + + /// Dummy value that has been saved by the instance + private int dummy; + + } + + #endregion // class ClassWithoutDefaultConstructor + /// /// Verifies that a cloned object exhibits the expected state for the type of /// clone that has been performed diff --git a/Source/Cloning/ExpressionTreeCloner.PropertyBased.cs b/Source/Cloning/ExpressionTreeCloner.PropertyBased.cs index 52cac35..550bbfe 100644 --- a/Source/Cloning/ExpressionTreeCloner.PropertyBased.cs +++ b/Source/Cloning/ExpressionTreeCloner.PropertyBased.cs @@ -213,44 +213,46 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - Type propertyType = propertyInfo.PropertyType; + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + Type propertyType = propertyInfo.PropertyType; - if(propertyType.IsPrimitive || (propertyType == typeof(string))) { - transferExpressions.Add( - Expression.Assign( - Expression.Property(clone, propertyInfo), - Expression.Property(original, propertyInfo) - ) - ); - } else if(propertyType.IsValueType) { - ParameterExpression originalProperty = Expression.Variable(propertyType); - variables.Add(originalProperty); - transferExpressions.Add( - Expression.Assign( - originalProperty, Expression.Property(original, propertyInfo) - ) - ); + if(propertyType.IsPrimitive || (propertyType == typeof(string))) { + transferExpressions.Add( + Expression.Assign( + Expression.Property(clone, propertyInfo), + Expression.Property(original, propertyInfo) + ) + ); + } else if(propertyType.IsValueType) { + ParameterExpression originalProperty = Expression.Variable(propertyType); + variables.Add(originalProperty); + transferExpressions.Add( + Expression.Assign( + originalProperty, Expression.Property(original, propertyInfo) + ) + ); - ParameterExpression clonedProperty = Expression.Variable(propertyType); - variables.Add(clonedProperty); - transferExpressions.Add( - Expression.Assign(clonedProperty, Expression.New(propertyType)) - ); + ParameterExpression clonedProperty = Expression.Variable(propertyType); + variables.Add(clonedProperty); + transferExpressions.Add( + Expression.Assign(clonedProperty, Expression.New(propertyType)) + ); - generateShallowPropertyBasedComplexCloneExpressions(propertyType, originalProperty, clonedProperty, transferExpressions, variables); + generateShallowPropertyBasedComplexCloneExpressions(propertyType, originalProperty, clonedProperty, transferExpressions, variables); - transferExpressions.Add( - Expression.Assign( - Expression.Property(clone, propertyInfo), clonedProperty - ) - ); - } else { - transferExpressions.Add( - Expression.Assign( - Expression.Property(clone, propertyInfo), - Expression.Property(original, propertyInfo) - ) - ); + transferExpressions.Add( + Expression.Assign( + Expression.Property(clone, propertyInfo), clonedProperty + ) + ); + } else { + transferExpressions.Add( + Expression.Assign( + Expression.Property(clone, propertyInfo), + Expression.Property(original, propertyInfo) + ) + ); + } } } } @@ -503,52 +505,54 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - Type propertyType = propertyInfo.PropertyType; + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + Type propertyType = propertyInfo.PropertyType; - if(propertyType.IsPrimitive || (propertyType == typeof(string))) { - // Primitive types and strings can be transferred by simple assignment - transferExpressions.Add( - Expression.Assign( - Expression.Property(clone, propertyInfo), - Expression.Property(original, propertyInfo) - ) - ); - } else if(propertyType.IsValueType) { - ParameterExpression originalProperty = Expression.Variable(propertyType); - variables.Add(originalProperty); - ParameterExpression clonedProperty = Expression.Variable(propertyType); - variables.Add(clonedProperty); + if(propertyType.IsPrimitive || (propertyType == typeof(string))) { + // Primitive types and strings can be transferred by simple assignment + transferExpressions.Add( + Expression.Assign( + Expression.Property(clone, propertyInfo), + Expression.Property(original, propertyInfo) + ) + ); + } else if(propertyType.IsValueType) { + ParameterExpression originalProperty = Expression.Variable(propertyType); + variables.Add(originalProperty); + ParameterExpression clonedProperty = Expression.Variable(propertyType); + variables.Add(clonedProperty); - transferExpressions.Add( - Expression.Assign( - originalProperty, Expression.Property(original, propertyInfo) - ) - ); - transferExpressions.Add( - Expression.Assign(clonedProperty, Expression.New(propertyType)) - ); + transferExpressions.Add( + Expression.Assign( + originalProperty, Expression.Property(original, propertyInfo) + ) + ); + transferExpressions.Add( + Expression.Assign(clonedProperty, Expression.New(propertyType)) + ); - // A nested value type is part of the parent and will have its properties directly - // assigned without boxing, new instance creation or anything like that. - generatePropertyBasedComplexTypeTransferExpressions( - propertyType, - originalProperty, - clonedProperty, - variables, - transferExpressions - ); + // A nested value type is part of the parent and will have its properties directly + // assigned without boxing, new instance creation or anything like that. + generatePropertyBasedComplexTypeTransferExpressions( + propertyType, + originalProperty, + clonedProperty, + variables, + transferExpressions + ); - transferExpressions.Add( - Expression.Assign( - Expression.Property(clone, propertyInfo), - clonedProperty - ) - ); + transferExpressions.Add( + Expression.Assign( + Expression.Property(clone, propertyInfo), + clonedProperty + ) + ); - } else { - generatePropertyBasedReferenceTypeTransferExpressions( - original, clone, transferExpressions, variables, propertyInfo, propertyType - ); + } else { + generatePropertyBasedReferenceTypeTransferExpressions( + original, clone, transferExpressions, variables, propertyInfo, propertyType + ); + } } } } diff --git a/Source/Cloning/ExpressionTreeCloner.Test.cs b/Source/Cloning/ExpressionTreeCloner.Test.cs index 5b520b3..42c3a6e 100644 --- a/Source/Cloning/ExpressionTreeCloner.Test.cs +++ b/Source/Cloning/ExpressionTreeCloner.Test.cs @@ -45,6 +45,19 @@ namespace Nuclex.Support.Cloning { Assert.IsNull(this.cloneFactory.ShallowPropertyClone(null)); } + /// + /// Verifies that clones of objects whose class doesn't possess a default constructor + /// can be made + /// + [Test] + public void ClassWithoutDefaultConstructorCanBeCloned() { + var original = new ClassWithoutDefaultConstructor(1234); + ClassWithoutDefaultConstructor clone = this.cloneFactory.DeepFieldClone(original); + + Assert.AreNotSame(original, clone); + Assert.AreEqual(original.Dummy, clone.Dummy); + } + /// Verifies that clones of primitive types can be created [Test] public void PrimitiveTypesCanBeCloned() { diff --git a/Source/Cloning/ReflectionCloner.Test.cs b/Source/Cloning/ReflectionCloner.Test.cs index 6603ea4..925e255 100644 --- a/Source/Cloning/ReflectionCloner.Test.cs +++ b/Source/Cloning/ReflectionCloner.Test.cs @@ -45,6 +45,19 @@ namespace Nuclex.Support.Cloning { Assert.IsNull(this.cloneFactory.ShallowPropertyClone(null)); } + /// + /// Verifies that clones of objects whose class doesn't possess a default constructor + /// can be made + /// + [Test] + public void ClassWithoutDefaultConstructorCanBeCloned() { + var original = new ClassWithoutDefaultConstructor(1234); + ClassWithoutDefaultConstructor clone = this.cloneFactory.DeepFieldClone(original); + + Assert.AreNotSame(original, clone); + Assert.AreEqual(original.Dummy, clone.Dummy); + } + /// Verifies that clones of primitive types can be created [Test] public void PrimitiveTypesCanBeCloned() { diff --git a/Source/Cloning/ReflectionCloner.cs b/Source/Cloning/ReflectionCloner.cs index 494a2a5..2f778cd 100644 --- a/Source/Cloning/ReflectionCloner.cs +++ b/Source/Cloning/ReflectionCloner.cs @@ -196,19 +196,21 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - Type propertyType = propertyInfo.PropertyType; - object originalValue = propertyInfo.GetValue(original, null); - if(originalValue != null) { - if(propertyType.IsPrimitive || (propertyType == typeof(string))) { - // Primitive types can be assigned directly - propertyInfo.SetValue(clone, originalValue, null); - } else if(propertyType.IsValueType) { - // Value types are seen as part of the original type and are thus recursed into - propertyInfo.SetValue(clone, shallowCloneComplexPropertyBased(originalValue), null); - } else if(propertyType.IsArray) { // Arrays are assigned directly in a shallow clone - propertyInfo.SetValue(clone, originalValue, null); - } else { // Complex types are directly assigned without creating a copy - propertyInfo.SetValue(clone, originalValue, null); + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + Type propertyType = propertyInfo.PropertyType; + object originalValue = propertyInfo.GetValue(original, null); + if(originalValue != null) { + if(propertyType.IsPrimitive || (propertyType == typeof(string))) { + // Primitive types can be assigned directly + propertyInfo.SetValue(clone, originalValue, null); + } else if(propertyType.IsValueType) { + // Value types are seen as part of the original type and are thus recursed into + propertyInfo.SetValue(clone, shallowCloneComplexPropertyBased(originalValue), null); + } else if(propertyType.IsArray) { // Arrays are assigned directly in a shallow clone + propertyInfo.SetValue(clone, originalValue, null); + } else { // Complex types are directly assigned without creating a copy + propertyInfo.SetValue(clone, originalValue, null); + } } } } @@ -366,20 +368,22 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - Type propertyType = propertyInfo.PropertyType; - object originalValue = propertyInfo.GetValue(original, null); - if(originalValue != null) { - if(propertyType.IsPrimitive || (propertyType == typeof(string))) { - // Primitive types can be assigned directly - propertyInfo.SetValue(clone, originalValue, null); - } else if(propertyType.IsArray) { // Arrays need to be cloned element-by-element - propertyInfo.SetValue( - clone, - deepCloneArrayPropertyBased((Array)originalValue, propertyType.GetElementType()), - null - ); - } else { // Complex types need to be cloned member-by-member - propertyInfo.SetValue(clone, deepCloneSinglePropertyBased(originalValue), null); + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + Type propertyType = propertyInfo.PropertyType; + object originalValue = propertyInfo.GetValue(original, null); + if(originalValue != null) { + if(propertyType.IsPrimitive || (propertyType == typeof(string))) { + // Primitive types can be assigned directly + propertyInfo.SetValue(clone, originalValue, null); + } else if(propertyType.IsArray) { // Arrays need to be cloned element-by-element + propertyInfo.SetValue( + clone, + deepCloneArrayPropertyBased((Array)originalValue, propertyType.GetElementType()), + null + ); + } else { // Complex types need to be cloned member-by-member + propertyInfo.SetValue(clone, deepCloneSinglePropertyBased(originalValue), null); + } } } } diff --git a/Source/Cloning/SerializationCloner.Test.cs b/Source/Cloning/SerializationCloner.Test.cs index 58edcd5..93b7fbc 100644 --- a/Source/Cloning/SerializationCloner.Test.cs +++ b/Source/Cloning/SerializationCloner.Test.cs @@ -43,6 +43,19 @@ namespace Nuclex.Support.Cloning { Assert.IsNull(this.cloneFactory.DeepPropertyClone(null)); } + /// + /// Verifies that clones of objects whose class doesn't possess a default constructor + /// can be made + /// + [Test] + public void ClassWithoutDefaultConstructorCanBeCloned() { + var original = new ClassWithoutDefaultConstructor(1234); + ClassWithoutDefaultConstructor clone = this.cloneFactory.DeepFieldClone(original); + + Assert.AreNotSame(original, clone); + Assert.AreEqual(original.Dummy, clone.Dummy); + } + /// Verifies that clones of primitive types can be created [Test] public void PrimitiveTypesCanBeCloned() { diff --git a/Source/Cloning/SerializationCloner.cs b/Source/Cloning/SerializationCloner.cs index 71cb1fd..5c0c2d6 100644 --- a/Source/Cloning/SerializationCloner.cs +++ b/Source/Cloning/SerializationCloner.cs @@ -184,7 +184,9 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - info.AddValue(propertyInfo.Name, propertyInfo.GetValue(objectToSerialize, null)); + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + info.AddValue(propertyInfo.Name, propertyInfo.GetValue(objectToSerialize, null)); + } } } @@ -210,11 +212,13 @@ namespace Nuclex.Support.Cloning { ); for(int index = 0; index < propertyInfos.Length; ++index) { PropertyInfo propertyInfo = propertyInfos[index]; - propertyInfo.SetValue( - deserializedObject, - info.GetValue(propertyInfo.Name, propertyInfo.PropertyType), - null - ); + if(propertyInfo.CanRead && propertyInfo.CanWrite) { + propertyInfo.SetValue( + deserializedObject, + info.GetValue(propertyInfo.Name, propertyInfo.PropertyType), + null + ); + } } return deserializedObject;