Skip to content

Commit cbd4add

Browse files
yfakariyayfakariya
committed
Fix wrong code generation for constructor based deserialization. #233
This commit fixes wrong code generation due to strange name calculation bug.
1 parent ef95185 commit cbd4add

13 files changed

Lines changed: 326 additions & 14 deletions

‎CHANGES.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,3 +640,8 @@ Release 0.9.0 beta2 2017/2/12
640640
* Fix null items of complex type in List<T> or Dictionary<TKey, TValue> will not be deserialized as null. Issue #211. (from 0.8.1)
641641
* Fix types which implement IPackable and IUnpackable but do not have any members cannot be serialized. Issue #202
642642
* Fix Windows Native build error. Issue #206.
643+
644+
Release 0.9.0 RC1 T.B.D.
645+
646+
BUG FIXES
647+
* Fix constructor deserialization fails if the constructor parameters order is not lexical. Issue #233

‎src/MsgPack/Serialization/AbstractSerializers/SerializerBuilder`2.Object.cs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -845,12 +845,6 @@ private TConstruct EmitObjectUnpackFromCore( TContext context, SerializationTarg
845845
#endif // FEATURE_TAP
846846
;
847847

848-
int constructorParameterIndex = 0;
849-
var fieldNames =
850-
targetInfo.IsConstructorDeserialization
851-
? targetInfo.DeserializationConstructor.GetParameters().Select( p => p.Name ).ToArray()
852-
: targetInfo.Members.Where( m => m.MemberName != null ).Select( m => m.MemberName ).ToArray();
853-
854848
for ( int i = 0; i < targetInfo.Members.Count; i++ )
855849
{
856850
var count = i;
@@ -881,6 +875,8 @@ private TConstruct EmitObjectUnpackFromCore( TContext context, SerializationTarg
881875
}
882876
else
883877
{
878+
var name = targetInfo.Members[ count ].MemberName;
879+
Contract.Assert( !String.IsNullOrEmpty( name ), targetInfo.Members[ count ] + "@" + i + " does not have member name.");
884880
var unpackedItem =
885881
context.DefineUnpackedItemParameterInSetValueMethods( targetInfo.Members[ count ].Member.GetMemberValueType() );
886882
Func<TConstruct> storeValueStatementEmitter;
@@ -892,7 +888,7 @@ private TConstruct EmitObjectUnpackFromCore( TContext context, SerializationTarg
892888
context,
893889
context.UnpackingContextInSetValueMethods,
894890
Metadata._DynamicUnpackingContext.Set,
895-
this.MakeStringLiteral( context, fieldNames[ count ] ),
891+
this.MakeStringLiteral( context, name ),
896892
targetInfo.Members[ count ].Member.GetMemberValueType().GetIsValueType()
897893
? this.EmitBoxExpression(
898894
context,
@@ -903,11 +899,9 @@ targetInfo.Members[ count ].Member.GetMemberValueType().GetIsValueType()
903899
}
904900
else if ( targetInfo.IsConstructorDeserialization || this.TargetType.GetIsValueType() )
905901
{
906-
var name = fieldNames[ constructorParameterIndex ];
907902
storeValueStatementEmitter =
908903
() =>
909904
this.EmitSetField( context, context.UnpackingContextInSetValueMethods, unpackingContext.Type, name, unpackedItem );
910-
constructorParameterIndex++;
911905
}
912906
else
913907
{
@@ -1013,7 +1007,7 @@ private UnpackingContextInfo EmitObjectUnpackingContextInitialization( TContext
10131007
{
10141008
var constructorParameters = targetInfo.DeserializationConstructor.GetParameters();
10151009
var contextFields =
1016-
constructorParameters.Select( p => new KeyValuePair<string, TypeDefinition>( p.Name, p.ParameterType ) ).ToArray();
1010+
constructorParameters.Select( ( p, i ) => new KeyValuePair<string, TypeDefinition>( targetInfo.GetCorrespondingMemberName( i ) ?? ( "__OrphanParameter" + i.ToString( CultureInfo.InvariantCulture ) ), p.ParameterType ) ).ToArray();
10171011
var constructorArguments = new List<TConstruct>( constructorParameters.Length );
10181012
var mappableConstructorArguments = new HashSet<string>();
10191013
var initializationStatements =

‎test/MsgPack.UnitTest.CodeDom/Serialization/ArrayCodeDomBasedAutoMessagePackSerializerTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3855,6 +3855,31 @@ public void TestOptionalConstructorString_Success()
38553855
}
38563856
}
38573857

3858+
// Issue233
3859+
[Test]
3860+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
3861+
{
3862+
var endpoints =
3863+
new EndpointList(
3864+
"Test String One",
3865+
new Dictionary<string, string[]>
3866+
{
3867+
{ "ConfigService", new [] { "ur1", "ur2" } },
3868+
{ "TestService", new [] { "ur1", "ur2" } }
3869+
},
3870+
"Test String Two"
3871+
);
3872+
3873+
var context = new SerializationContext();
3874+
var ser = context.GetSerializer<EndpointList>();
3875+
var bytes = ser.PackSingleObject( endpoints );
3876+
var endpointsDeser = ser.UnpackSingleObject( bytes );
3877+
3878+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
3879+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
3880+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
3881+
}
3882+
38583883
[Test]
38593884
public void TestCollection_Success()
38603885
{

‎test/MsgPack.UnitTest.CodeDom/Serialization/MapCodeDomBasedAutoMessagePackSerializerTest.cs‎

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3932,6 +3932,31 @@ public void TestOptionalConstructorString_Success()
39323932
}
39333933
}
39343934

3935+
// Issue233
3936+
[Test]
3937+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
3938+
{
3939+
var endpoints =
3940+
new EndpointList(
3941+
"Test String One",
3942+
new Dictionary<string, string[]>
3943+
{
3944+
{ "ConfigService", new [] { "ur1", "ur2" } },
3945+
{ "TestService", new [] { "ur1", "ur2" } }
3946+
},
3947+
"Test String Two"
3948+
);
3949+
3950+
var context = new SerializationContext();
3951+
var ser = context.GetSerializer<EndpointList>();
3952+
var bytes = ser.PackSingleObject( endpoints );
3953+
var endpointsDeser = ser.UnpackSingleObject( bytes );
3954+
3955+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
3956+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
3957+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
3958+
}
3959+
39353960
[Test]
39363961
public void TestCollection_Success()
39373962
{
@@ -21081,25 +21106,73 @@ public void TestUriFieldArrayNull()
2108121106
}
2108221107

2108321108
[Test]
21084-
public void TestVersionField()
21109+
public void TestVersionConstructorMajorMinor()
21110+
{
21111+
this.TestCoreWithAutoVerify( new Version( 1, 2 ), GetSerializationContext() );
21112+
}
21113+
21114+
[Test]
21115+
public void TestVersionConstructorMajorMinorArray()
21116+
{
21117+
this.TestCoreWithAutoVerify( Enumerable.Repeat( new Version( 1, 2 ), 2 ).ToArray(), GetSerializationContext() );
21118+
}
21119+
21120+
[Test]
21121+
public void TestVersionConstructorMajorMinorNull()
21122+
{
21123+
this.TestCoreWithAutoVerify( default( Version ), GetSerializationContext() );
21124+
}
21125+
21126+
[Test]
21127+
public void TestVersionConstructorMajorMinorArrayNull()
21128+
{
21129+
this.TestCoreWithAutoVerify( default( Version[] ), GetSerializationContext() );
21130+
}
21131+
21132+
[Test]
21133+
public void TestVersionConstructorMajorMinorBuild()
21134+
{
21135+
this.TestCoreWithAutoVerify( new Version( 1, 2, 3 ), GetSerializationContext() );
21136+
}
21137+
21138+
[Test]
21139+
public void TestVersionConstructorMajorMinorBuildArray()
21140+
{
21141+
this.TestCoreWithAutoVerify( Enumerable.Repeat( new Version( 1, 2, 3 ), 2 ).ToArray(), GetSerializationContext() );
21142+
}
21143+
21144+
[Test]
21145+
public void TestVersionConstructorMajorMinorBuildNull()
21146+
{
21147+
this.TestCoreWithAutoVerify( default( Version ), GetSerializationContext() );
21148+
}
21149+
21150+
[Test]
21151+
public void TestVersionConstructorMajorMinorBuildArrayNull()
21152+
{
21153+
this.TestCoreWithAutoVerify( default( Version[] ), GetSerializationContext() );
21154+
}
21155+
21156+
[Test]
21157+
public void TestFullVersionConstructor()
2108521158
{
2108621159
this.TestCoreWithAutoVerify( new Version( 1, 2, 3, 4 ), GetSerializationContext() );
2108721160
}
2108821161

2108921162
[Test]
21090-
public void TestVersionFieldArray()
21163+
public void TestFullVersionConstructorArray()
2109121164
{
2109221165
this.TestCoreWithAutoVerify( Enumerable.Repeat( new Version( 1, 2, 3, 4 ), 2 ).ToArray(), GetSerializationContext() );
2109321166
}
2109421167

2109521168
[Test]
21096-
public void TestVersionFieldNull()
21169+
public void TestFullVersionConstructorNull()
2109721170
{
2109821171
this.TestCoreWithAutoVerify( default( Version ), GetSerializationContext() );
2109921172
}
2110021173

2110121174
[Test]
21102-
public void TestVersionFieldArrayNull()
21175+
public void TestFullVersionConstructorArrayNull()
2110321176
{
2110421177
this.TestCoreWithAutoVerify( default( Version[] ), GetSerializationContext() );
2110521178
}

‎test/MsgPack.UnitTest/Serialization/ArrayFieldBasedAutoMessagePackSerializerTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3855,6 +3855,31 @@ public void TestOptionalConstructorString_Success()
38553855
}
38563856
}
38573857

3858+
// Issue233
3859+
[Test]
3860+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
3861+
{
3862+
var endpoints =
3863+
new EndpointList(
3864+
"Test String One",
3865+
new Dictionary<string, string[]>
3866+
{
3867+
{ "ConfigService", new [] { "ur1", "ur2" } },
3868+
{ "TestService", new [] { "ur1", "ur2" } }
3869+
},
3870+
"Test String Two"
3871+
);
3872+
3873+
var context = new SerializationContext();
3874+
var ser = context.GetSerializer<EndpointList>();
3875+
var bytes = ser.PackSingleObject( endpoints );
3876+
var endpointsDeser = ser.UnpackSingleObject( bytes );
3877+
3878+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
3879+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
3880+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
3881+
}
3882+
38583883
[Test]
38593884
public void TestCollection_Success()
38603885
{

‎test/MsgPack.UnitTest/Serialization/ArrayGenerationBasedAutoMessagePackSerializerTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,31 @@ public void TestNonZeroBoundMultidimensionalArray()
908908
#endif // !SILVERLIGHT
909909

910910

911+
// Issue233
912+
[Test]
913+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
914+
{
915+
var endpoints =
916+
new EndpointList(
917+
"Test String One",
918+
new Dictionary<string, string[]>
919+
{
920+
{ "ConfigService", new [] { "ur1", "ur2" } },
921+
{ "TestService", new [] { "ur1", "ur2" } }
922+
},
923+
"Test String Two"
924+
);
925+
926+
var context = new SerializationContext();
927+
var ser = context.GetSerializer<EndpointList>();
928+
var bytes = ser.PackSingleObject( endpoints );
929+
var endpointsDeser = ser.UnpackSingleObject( bytes );
930+
931+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
932+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
933+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
934+
}
935+
911936
[Test]
912937
public void TestCollection_Success()
913938
{

‎test/MsgPack.UnitTest/Serialization/ArrayReflectionBasedAutoMessagePackSerializerTest.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3855,6 +3855,31 @@ public void TestOptionalConstructorString_Success()
38553855
}
38563856
}
38573857

3858+
// Issue233
3859+
[Test]
3860+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
3861+
{
3862+
var endpoints =
3863+
new EndpointList(
3864+
"Test String One",
3865+
new Dictionary<string, string[]>
3866+
{
3867+
{ "ConfigService", new [] { "ur1", "ur2" } },
3868+
{ "TestService", new [] { "ur1", "ur2" } }
3869+
},
3870+
"Test String Two"
3871+
);
3872+
3873+
var context = new SerializationContext();
3874+
var ser = context.GetSerializer<EndpointList>();
3875+
var bytes = ser.PackSingleObject( endpoints );
3876+
var endpointsDeser = ser.UnpackSingleObject( bytes );
3877+
3878+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
3879+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
3880+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
3881+
}
3882+
38583883
[Test]
38593884
public void TestCollection_Success()
38603885
{

‎test/MsgPack.UnitTest/Serialization/AutoMessagePackSerializerTest.Types.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14713,6 +14713,26 @@ public string GetValue()
1471314713
}
1471414714

1471514715
#endregion -- Empty interfaces --
14716+
14717+
#region -- Issue 233 (constructor based deserialization when the parameters are not in lexicol order) --
14718+
14719+
public class EndpointList
14720+
{
14721+
public string StringOne { get; }
14722+
14723+
public Dictionary<string, string[]> Endpoints { get; }
14724+
14725+
public string StringTwo { get; }
14726+
14727+
public EndpointList( string stringOne, Dictionary<string, string[]> endpoints, string stringTwo )
14728+
{
14729+
StringOne = stringOne;
14730+
Endpoints = endpoints;
14731+
StringTwo = stringTwo;
14732+
}
14733+
}
14734+
14735+
#endregion -- Issue 233 (constructor based deserialization when the parameters are not in lexicol order) --
1471614736
}
1471714737

1471814738
// Issue #108

‎test/MsgPack.UnitTest/Serialization/AutoMessagePackSerializerTest.Types.tt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2449,6 +2449,26 @@ if ( isAsync )
24492449
#>
24502450

24512451
#endregion -- Empty interfaces --
2452+
2453+
#region -- Issue 233 (constructor based deserialization when the parameters are not in lexicol order) --
2454+
2455+
public class EndpointList
2456+
{
2457+
public string StringOne { get; }
2458+
2459+
public Dictionary<string, string[]> Endpoints { get; }
2460+
2461+
public string StringTwo { get; }
2462+
2463+
public EndpointList( string stringOne, Dictionary<string, string[]> endpoints, string stringTwo )
2464+
{
2465+
StringOne = stringOne;
2466+
Endpoints = endpoints;
2467+
StringTwo = stringTwo;
2468+
}
2469+
}
2470+
2471+
#endregion -- Issue 233 (constructor based deserialization when the parameters are not in lexicol order) --
24522472
}
24532473

24542474
// Issue #108

‎test/MsgPack.UnitTest/Serialization/AutoMessagePackSerializerTest.ttinclude‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,6 +2069,31 @@ namespace MsgPack.Serialization
20692069
}
20702070
#>
20712071

2072+
// Issue233
2073+
[Test]
2074+
public void TestConstructorDeserializationWithParametersNotInLexicalOrder()
2075+
{
2076+
var endpoints =
2077+
new EndpointList(
2078+
"Test String One",
2079+
new Dictionary<string, string[]>
2080+
{
2081+
{ "ConfigService", new [] { "ur1", "ur2" } },
2082+
{ "TestService", new [] { "ur1", "ur2" } }
2083+
},
2084+
"Test String Two"
2085+
);
2086+
2087+
var context = new SerializationContext();
2088+
var ser = context.GetSerializer<EndpointList>();
2089+
var bytes = ser.PackSingleObject( endpoints );
2090+
var endpointsDeser = ser.UnpackSingleObject( bytes );
2091+
2092+
Assert.That( endpointsDeser.StringOne, Is.EqualTo( endpoints.StringOne ) );
2093+
Assert.That( endpointsDeser.StringTwo, Is.EqualTo( endpoints.StringTwo ) );
2094+
Assert.That( endpointsDeser.Endpoints, Is.EqualTo( endpoints.Endpoints ) );
2095+
}
2096+
20722097
[Test]
20732098
public void TestCollection_Success()
20742099
{

0 commit comments

Comments
 (0)