Skip to content

Commit 7bfdee5

Browse files
committed
Fix condition to using constructor based deserialization is too tight. Part of issue #178.
* If the default constructor found, generator searches parameterful constructors instead of throwing exception immediately. This rescues many cases including * All properties have private setters. This is better to use constructor deserialization over reflection based setters. * Properties/fields looks appendable collection but they are immutable like FSharpSet. * This commit also fixes MessagePackDeserializationConstructutorAttribute will not be used when the type has default constructor.
1 parent 8942c1c commit 7bfdee5

1 file changed

Lines changed: 40 additions & 12 deletions

File tree

src/MsgPack/Serialization/SerializationTarget.cs

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,33 @@ public static SerializationTarget Prepare( SerializationContext context, Type ta
9595

9696
if ( memberCandidates.Length == 0 )
9797
{
98-
var constructor = FindDeserializationConstructor( targetType );
99-
return new SerializationTarget( ComplementMembers( getters, context, targetType ), constructor );
98+
var deserializationConstructor = FindDeserializationConstructor( targetType );
99+
return new SerializationTarget( ComplementMembers( getters, context, targetType ), deserializationConstructor );
100100
}
101101

102-
var defaultConstructor = targetType.GetConstructor( ReflectionAbstractions.EmptyTypes );
103-
if ( defaultConstructor == null && !targetType.GetIsValueType() )
102+
// Try to get default constructor.
103+
var constructor = targetType.GetConstructor( ReflectionAbstractions.EmptyTypes );
104+
if ( constructor == null && !targetType.GetIsValueType() )
104105
{
105-
throw SerializationExceptions.NewTargetDoesNotHavePublicDefaultConstructor( targetType );
106+
// Try to get deserialization constructor.
107+
var deserializationConstructor = FindDeserializationConstructor( targetType );
108+
if ( deserializationConstructor == null )
109+
{
110+
throw SerializationExceptions.NewTargetDoesNotHavePublicDefaultConstructor( targetType );
111+
}
112+
113+
constructor = deserializationConstructor;
114+
}
115+
else
116+
{
117+
// Let's prefer annotated constructor here.
118+
var markedConstructors = FindExplicitDeserializationConstructors( targetType.GetConstructors() );
119+
if ( markedConstructors.Count == 1 )
120+
{
121+
// For backward compatibility, no exceptions are thrown here even if mulitiple deserialization constructor attributes in the type
122+
// just use default constructor for it.
123+
constructor = markedConstructors[ 0 ];
124+
}
106125
}
107126

108127
// Because members' order is equal to declared order is NOT guaranteed, so explicit ordering is required.
@@ -118,7 +137,7 @@ public static SerializationTarget Prepare( SerializationContext context, Type ta
118137
members = ComplementMembers( memberCandidates, context, targetType );
119138
}
120139

121-
return new SerializationTarget( members, defaultConstructor );
140+
return new SerializationTarget( members, constructor );
122141
}
123142

124143
private static MemberInfo[] GetDistinctMembers( Type type )
@@ -129,10 +148,10 @@ private static MemberInfo[] GetDistinctMembers( Type type )
129148
{
130149
var members =
131150
#if !NETSTANDARD1_1 && !NETSTANDARD1_3
132-
type.FindMembers(
133-
MemberTypes.Field | MemberTypes.Property,
151+
type.FindMembers(
152+
MemberTypes.Field | MemberTypes.Property,
134153
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly,
135-
null,
154+
null,
136155
null
137156
);
138157
#else
@@ -153,7 +172,7 @@ private static MemberInfo[] GetDistinctMembers( Type type )
153172
return distinctMembers.ToArray();
154173
}
155174

156-
private static IEnumerable<SerializingMember> GetTargetMembers(Type type)
175+
private static IEnumerable<SerializingMember> GetTargetMembers( Type type )
157176
{
158177
Contract.Assert( type != null, "type != null" );
159178

@@ -282,8 +301,8 @@ private static ConstructorInfo FindDeserializationConstructor( Type targetType )
282301
}
283302

284303
// The marked construtor is always preferred.
285-
var markedConstructors = constructors.Where( ctor => ctor.IsDefined( typeof( MessagePackDeserializationConstructorAttribute ) ) ).ToArray();
286-
switch ( markedConstructors.Length )
304+
var markedConstructors = FindExplicitDeserializationConstructors( constructors );
305+
switch ( markedConstructors.Count )
287306
{
288307
case 0:
289308
{
@@ -336,6 +355,11 @@ private static ConstructorInfo FindDeserializationConstructor( Type targetType )
336355
}
337356
}
338357

358+
private static IList<ConstructorInfo> FindExplicitDeserializationConstructors( IEnumerable<ConstructorInfo> construtors )
359+
{
360+
return construtors.Where( ctor => ctor.IsDefined( typeof( MessagePackDeserializationConstructorAttribute ) ) ).ToArray();
361+
}
362+
339363
private static SerializationException NewTypeCannotBeSerializedException( Type targetType )
340364
{
341365
return new SerializationException(
@@ -384,6 +408,10 @@ private static bool CheckTargetEligibility( MemberInfo member )
384408
}
385409
else
386410
{
411+
#if DEBUG
412+
Contract.Assert( false, $"Unknown type member {member}" );
413+
#endif // DEBUG
414+
// ReSharper disable once HeuristicUnreachableCode
387415
return true;
388416
}
389417

0 commit comments

Comments
 (0)