Skip to content

Commit 531fa6e

Browse files
authored
CSHARP-4659: Improve BsonClassMapSerializer to support stuctures (#1098)
1 parent f958b93 commit 531fa6e

File tree

5 files changed

+63
-97
lines changed

5 files changed

+63
-97
lines changed

src/MongoDB.Bson/Serialization/BsonMemberMap.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,12 @@ private Action<object, object> GetFieldSetter()
577577
throw new BsonSerializationException(message);
578578
}
579579

580+
if (fieldInfo.DeclaringType.IsValueType)
581+
{
582+
// Fallback to reflection for value type to avoid boxing-unboxing problems
583+
return (instance, value) => fieldInfo.SetValue(instance, value);
584+
}
585+
580586
var objParameter = Expression.Parameter(typeof(object), "obj");
581587
var valueParameter = Expression.Parameter(typeof(object), "value");
582588
var field = Expression.Field(Expression.Convert(objParameter, fieldInfo.DeclaringType), fieldInfo);
@@ -630,6 +636,12 @@ private Action<object, object> GetPropertySetter()
630636
throw new BsonSerializationException(message);
631637
}
632638

639+
if (propertyInfo.DeclaringType.IsValueType)
640+
{
641+
// Fallback to reflection for value type to avoid boxing-unboxing problems
642+
return (instance, value) => propertyInfo.SetValue(instance, value);
643+
}
644+
633645
// lambdaExpression = (obj, value) => ((TClass) obj).SetMethod((TMember) value)
634646
var objParameter = Expression.Parameter(typeof(object), "obj");
635647
var valueParameter = Expression.Parameter(typeof(object), "value");

src/MongoDB.Bson/Serialization/Serializers/BsonClassMapSerializer.cs

Lines changed: 32 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ public class BsonClassMapSerializer<TClass> : SerializerBase<TClass>, IBsonIdPro
3030
{
3131
// private fields
3232
private readonly BsonClassMap _classMap;
33-
private readonly bool _isReferenceType;
34-
private readonly bool _isValueType;
3533

3634
// constructors
3735
/// <summary>
@@ -42,21 +40,19 @@ public BsonClassMapSerializer(BsonClassMap classMap)
4240
{
4341
if (classMap == null)
4442
{
45-
throw new ArgumentNullException("classMap");
43+
throw new ArgumentNullException(nameof(classMap));
4644
}
4745
if (classMap.ClassType != typeof(TClass))
4846
{
4947
var message = string.Format("Must be a BsonClassMap for the type {0}.", typeof(TClass));
50-
throw new ArgumentException(message, "classMap");
48+
throw new ArgumentException(message, nameof(classMap));
5149
}
5250
if (!classMap.IsFrozen)
5351
{
5452
throw new ArgumentException("Class map is not frozen.", nameof(classMap));
5553
}
5654

5755
_classMap = classMap;
58-
_isValueType = _classMap.ClassType.IsValueType;
59-
_isReferenceType = !_isValueType;
6056
}
6157

6258
// public properties
@@ -87,22 +83,17 @@ public override TClass Deserialize(BsonDeserializationContext context, BsonDeser
8783
bsonReader.ReadNull();
8884
return default(TClass);
8985
}
90-
else
91-
{
92-
var discriminatorConvention = _classMap.GetDiscriminatorConvention();
9386

94-
var actualType = discriminatorConvention.GetActualType(bsonReader, args.NominalType);
95-
if (actualType == typeof(TClass))
96-
{
97-
return DeserializeClass(context);
98-
}
99-
else
100-
{
101-
var serializer = BsonSerializer.LookupSerializer(actualType);
102-
return (TClass)serializer.Deserialize(context);
103-
}
87+
var discriminatorConvention = _classMap.GetDiscriminatorConvention();
10488

89+
var actualType = discriminatorConvention.GetActualType(bsonReader, args.NominalType);
90+
if (actualType == typeof(TClass))
91+
{
92+
return DeserializeClass(context);
10593
}
94+
95+
var serializer = BsonSerializer.LookupSerializer(actualType);
96+
return (TClass)serializer.Deserialize(context);
10697
}
10798

10899
/// <summary>
@@ -124,7 +115,9 @@ public TClass DeserializeClass(BsonDeserializationContext context)
124115
}
125116

126117
Dictionary<string, object> values = null;
127-
var document = default(TClass);
118+
// Important for struct support:
119+
// should use object variable here, so created value should be boxed right away!
120+
object document = null;
128121

129122
ISupportInitialize supportsInitialization = null;
130123
if (_classMap.HasCreatorMaps)
@@ -135,13 +128,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
135128
else
136129
{
137130
// for mutable classes we deserialize the values directly into the result object
138-
if (_isValueType)
139-
{
140-
var message = string.Format("Value class {0} cannot be deserialized without a constructor.", _classMap.ClassType.FullName);
141-
throw new BsonSerializationException(message);
142-
}
143-
144-
document = (TClass)_classMap.CreateInstance();
131+
document = _classMap.CreateInstance();
145132

146133
if (document == null)
147134
{
@@ -173,7 +160,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
173160
var memberMap = allMemberMaps[memberMapIndex];
174161
if (memberMapIndex != extraElementsMemberMapIndex)
175162
{
176-
if (_isReferenceType && document != null)
163+
if (document != null)
177164
{
178165
if (memberMap.IsReadOnly)
179166
{
@@ -193,7 +180,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
193180
}
194181
else
195182
{
196-
if (_isReferenceType && document != null)
183+
if (document != null)
197184
{
198185
DeserializeExtraElementMember(context, document, elementName, memberMap);
199186
}
@@ -215,7 +202,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
215202
if (extraElementsMemberMapIndex >= 0)
216203
{
217204
var extraElementsMemberMap = _classMap.ExtraElementsMemberMap;
218-
if (_isReferenceType && document != null)
205+
if (document != null)
219206
{
220207
DeserializeExtraElementMember(context, document, elementName, extraElementsMemberMap);
221208
}
@@ -267,7 +254,7 @@ public TClass DeserializeClass(BsonDeserializationContext context)
267254
throw new FormatException(message);
268255
}
269256

270-
if (_isReferenceType && document != null)
257+
if (document != null)
271258
{
272259
memberMap.ApplyDefaultValue(document);
273260
}
@@ -289,18 +276,16 @@ public TClass DeserializeClass(BsonDeserializationContext context)
289276
}
290277
}
291278

292-
if (_isReferenceType && document != null)
279+
if (document != null)
293280
{
294281
if (supportsInitialization != null)
295282
{
296283
supportsInitialization.EndInit();
297284
}
298-
return document;
299-
}
300-
else
301-
{
302-
return CreateInstanceUsingCreator(values);
285+
return (TClass)document;
303286
}
287+
288+
return CreateInstanceUsingCreator(values);
304289
}
305290

306291
/// <summary>
@@ -372,20 +357,18 @@ public override void Serialize(BsonSerializationContext context, BsonSerializati
372357
if (value == null)
373358
{
374359
bsonWriter.WriteNull();
360+
return;
375361
}
376-
else
362+
363+
var actualType = value.GetType();
364+
if (actualType == typeof(TClass))
377365
{
378-
var actualType = value.GetType();
379-
if (actualType == typeof(TClass))
380-
{
381-
SerializeClass(context, args, value);
382-
}
383-
else
384-
{
385-
var serializer = BsonSerializer.LookupSerializer(actualType);
386-
serializer.Serialize(context, args, value);
387-
}
366+
SerializeClass(context, args, value);
367+
return;
388368
}
369+
370+
var serializer = BsonSerializer.LookupSerializer(actualType);
371+
serializer.Serialize(context, args, value);
389372
}
390373

391374
/// <summary>
@@ -433,14 +416,7 @@ private BsonCreatorMap ChooseBestCreator(Dictionary<string, object> values)
433416
private TClass CreateInstanceUsingCreator(Dictionary<string, object> values)
434417
{
435418
var creatorMap = ChooseBestCreator(values);
436-
var document = creatorMap.CreateInstance(values); // removes values consumed
437-
438-
if (values.Count > 0 && _isValueType)
439-
{
440-
var message = string.Format("Value class {0} cannot be deserialized unless all values can be passed to a constructor.", _classMap.ClassType.FullName);
441-
throw new BsonSerializationException(message);
442-
}
443-
419+
object document = creatorMap.CreateInstance(values); // removes values consumed
444420
var supportsInitialization = document as ISupportInitialize;
445421
if (supportsInitialization != null)
446422
{
@@ -473,8 +449,6 @@ private void DeserializeExtraElementMember(
473449
string elementName,
474450
BsonMemberMap extraElementsMemberMap)
475451
{
476-
var bsonReader = context.Reader;
477-
478452
if (extraElementsMemberMap.MemberType == typeof(BsonDocument))
479453
{
480454
var extraElements = (BsonDocument)extraElementsMemberMap.Getter(obj);
@@ -514,8 +488,6 @@ private void DeserializeExtraElementValue(
514488
string elementName,
515489
BsonMemberMap extraElementsMemberMap)
516490
{
517-
var bsonReader = context.Reader;
518-
519491
if (extraElementsMemberMap.MemberType == typeof(BsonDocument))
520492
{
521493
BsonDocument extraElements;
@@ -561,8 +533,6 @@ private void DeserializeExtraElementValue(
561533

562534
private object DeserializeMemberValue(BsonDeserializationContext context, BsonMemberMap memberMap)
563535
{
564-
var bsonReader = context.Reader;
565-
566536
try
567537
{
568538
return memberMap.GetSerializer().Deserialize(context);

tests/MongoDB.Driver.Legacy.Tests/Jira/CSharp218Tests.cs

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@
1313
* limitations under the License.
1414
*/
1515

16-
using System;
17-
using System.IO;
1816
using MongoDB.Bson;
19-
using MongoDB.Driver;
2017
using Xunit;
2118

2219
namespace MongoDB.Driver.Tests.Jira.CSharp218
@@ -49,32 +46,26 @@ public CSharp218Tests()
4946
}
5047

5148
[Fact]
52-
public void TestDeserializeClassWithStructPropertyWithoutConstructorFails()
49+
public void TestDeserializeClassWithStructPropertyWithoutConstructorSucceeds()
5350
{
5451
_collection.RemoveAll();
5552
var c = new C { Id = ObjectId.GenerateNewId(), P = new P { X = 1, Y = 2 } };
5653
_collection.Insert(c);
57-
try
58-
{
59-
_collection.FindOneAs<C>();
60-
Assert.True(false, "Expected an exception to be thrown.");
61-
}
62-
catch (Exception ex)
63-
{
64-
var expectedMessage = "An error occurred while deserializing the P field of class MongoDB.Driver.Tests.Jira.CSharp218.CSharp218Tests+C: Value class MongoDB.Driver.Tests.Jira.CSharp218.CSharp218Tests+P cannot be deserialized without a constructor.";
65-
Assert.IsType<FormatException>(ex);
66-
Assert.IsType<BsonSerializationException>(ex.InnerException);
67-
Assert.Equal(expectedMessage, ex.Message);
68-
}
54+
55+
var result = _collection.FindOneAs<C>();
56+
Assert.Equal(1, result.P.X);
57+
Assert.Equal(2, result.P.Y);
6958
}
7059

7160
[Fact]
72-
public void TestDeserializeStructFails()
61+
public void TestDeserializeStructSucceeds()
7362
{
7463
_collection.RemoveAll();
7564
var s = new S { Id = ObjectId.GenerateNewId(), P = new P { X = 1, Y = 2 } };
7665
_collection.Insert(s);
77-
Assert.Throws<BsonSerializationException>(() => _collection.FindOneAs<S>());
66+
var result = _collection.FindOneAs<S>();
67+
Assert.Equal(1, result.P.X);
68+
Assert.Equal(2, result.P.Y);
7869
}
7970

8071
[Fact]

tests/MongoDB.Driver.Tests/Linq/Linq3ImplementationTests/Jira/CSharp4391Tests.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,27 @@ public void Deserializing_struct_with_constructor_should_work()
6565
}
6666

6767
[Fact]
68-
public void Deserializing_struct_with_partial_constructor_should_fail()
68+
public void Deserializing_struct_with_partial_constructor_should_work()
6969
{
7070
var json = "{ _id : 1, X : 2, Y : 3 }";
7171

72-
var expection = Record.Exception(() => { _ = BsonSerializer.Deserialize<T>(json); });
72+
var result = BsonSerializer.Deserialize<T>(json);
7373

74-
expection.Should().BeOfType<BsonSerializationException>();
75-
expection.Message.Should().Contain("cannot be deserialized unless all values can be passed to a constructor");
74+
result.Id.Should().Be(1);
75+
result.X.Should().Be(2);
76+
result.Y.Should().Be(3);
7677
}
7778

7879
[Fact]
79-
public void Deserializing_struct_with_no_constructor_should_fail()
80+
public void Deserializing_struct_with_no_constructor_should_work()
8081
{
8182
var json = "{ _id : 1, X : 2, Y : 3 }";
8283

83-
var expection = Record.Exception(() => { _ = BsonSerializer.Deserialize<U>(json); });
84+
var result = BsonSerializer.Deserialize<U>(json);
8485

85-
expection.Should().BeOfType<BsonSerializationException>();
86-
expection.Message.Should().Contain("cannot be deserialized without a constructor");
86+
result.Id.Should().Be(1);
87+
result.X.Should().Be(2);
88+
result.Y.Should().Be(3);
8789
}
8890

8991
[Fact]

tests/MongoDB.Driver.Tests/Linq/Linq3ImplementationTests/Translators/ExpressionToAggregationExpressionTranslators/MemberInitExpressionToAggregationExpressionTranslatorTests.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,15 +189,6 @@ public struct SpawnDataStructParameterless
189189
public int Identifier;
190190
public DateTime SpawnDate;
191191
public string SpawnText;
192-
193-
// this constructor is required to be able to deserialize instances of this struct
194-
[BsonConstructor]
195-
public SpawnDataStructParameterless(int identifier, DateTime spawnDate, string spawnText)
196-
{
197-
Identifier = identifier;
198-
SpawnDate = spawnDate;
199-
SpawnText = spawnText;
200-
}
201192
}
202193

203194
public class SpawnDataClass

0 commit comments

Comments
 (0)