Skip to content

Commit f8d38a8

Browse files
DmitryLukyanovDmitryLukyanov
committed
CSHARP-2889: BsonClassMap.LookupClassMap supports private constructors inconsistently.
1 parent defc9bd commit f8d38a8

File tree

2 files changed

+270
-12
lines changed

2 files changed

+270
-12
lines changed

‎src/MongoDB.Bson/Serialization/Conventions/ImmutableTypeClassMapConvention.cs

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,28 @@ public class ImmutableTypeClassMapConvention : ConventionBase, IClassMapConventi
2929
public void Apply(BsonClassMap classMap)
3030
{
3131
var typeInfo = classMap.ClassType.GetTypeInfo();
32-
if (typeInfo.IsAbstract)
33-
{
34-
return;
35-
}
3632

3733
if (typeInfo.GetConstructor(Type.EmptyTypes) != null)
3834
{
3935
return;
4036
}
4137

42-
var properties = typeInfo.GetProperties(BindingFlags.Instance | BindingFlags.Public );
43-
if (properties.Any(p => p.CanWrite))
38+
var propertyBindingFlags = BindingFlags.Public | BindingFlags.Instance;
39+
var properties = typeInfo.GetProperties(propertyBindingFlags);
40+
if (properties.Any(CanWrite))
4441
{
4542
return; // a type that has any writable properties is not immutable
4643
}
4744

48-
var anyConstructorsWereMapped = false;
49-
foreach (var ctor in typeInfo.GetConstructors())
45+
var anyConstructorsWereFound = false;
46+
var constructorBindingFlags = BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance;
47+
foreach (var ctor in typeInfo.GetConstructors(constructorBindingFlags))
5048
{
49+
if (ctor.IsPrivate)
50+
{
51+
continue; // do not consider private constructors
52+
}
53+
5154
var parameters = ctor.GetParameters();
5255
if (parameters.Length != properties.Length)
5356
{
@@ -66,16 +69,26 @@ public void Apply(BsonClassMap classMap)
6669
continue;
6770
}
6871

69-
classMap.MapConstructor(ctor);
72+
if (ctor.IsPublic && !typeInfo.IsAbstract)
73+
{
74+
// we need to save constructorInfo only for public constructors in non abstract classes
75+
classMap.MapConstructor(ctor);
76+
}
7077

71-
anyConstructorsWereMapped = true;
78+
anyConstructorsWereFound = true;
7279
}
7380

74-
if (anyConstructorsWereMapped)
81+
if (anyConstructorsWereFound)
7582
{
76-
// if any constructors were mapped by this convention then map all the properties also
83+
// if any constructors were found by this convention
84+
// then map all the properties from the ClassType inheritance level also
7785
foreach (var property in properties)
7886
{
87+
if (property.DeclaringType != classMap.ClassType)
88+
{
89+
continue;
90+
}
91+
7992
var memberMap = classMap.MapMember(property);
8093
if (classMap.IsAnonymous)
8194
{
@@ -85,5 +98,12 @@ public void Apply(BsonClassMap classMap)
8598
}
8699
}
87100
}
101+
102+
// private methods
103+
private bool CanWrite(PropertyInfo propertyInfo)
104+
{
105+
// CanWrite gets true even if a property has only a private setter
106+
return propertyInfo.CanWrite && (propertyInfo.SetMethod?.IsPublic ?? false);
107+
}
88108
}
89109
}

‎tests/MongoDB.Bson.Tests/Jira/CSharp1559Tests.cs

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
/* Copyright 2020-present MongoDB Inc.
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS,
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
using System;
17+
using System.Reflection;
18+
using FluentAssertions;
19+
using MongoDB.Bson.Serialization;
20+
using MongoDB.Bson.Serialization.Attributes;
21+
using MongoDB.Bson.TestHelpers;
22+
using Xunit;
23+
24+
namespace MongoDB.Bson.Tests.Jira
25+
{
26+
public class CSharp1559Tests
27+
{
28+
[Theory]
29+
[InlineData(typeof(DerivedWithoutSetter_BaseWithoutSetter))]
30+
[InlineData(typeof(DerivedWithoutSetter_BaseWithPrivateSetterAndWithProtectedConstructor))]
31+
[InlineData(typeof(DerivedWithoutSetterAndWithBsonElement_BaseWithoutSetterAndWithProtectedConstructor))]
32+
[InlineData(typeof(DerivedWithoutSetterAndWithBsonElementAndWithPrivateConstructor_BaseWithoutSetter))]
33+
[InlineData(typeof(DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetterAndWithProtectedConstructor))]
34+
[InlineData(typeof(DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetter))]
35+
[InlineData(typeof(DerivedWithPrivateSetterAndWithBsonElement_BaseWithoutSetter))]
36+
public void Serialization_should_work_as_expected(Type testCaseType)
37+
{
38+
var testCase = Activator.CreateInstance(testCaseType, 1, 2);
39+
40+
var json = testCase.ToJson();
41+
var result = BsonSerializer.Deserialize(json, testCaseType);
42+
43+
var x = GetPropertyValue(result, "X");
44+
var y = GetPropertyValue(result, "Y");
45+
x.Should().Be(1);
46+
y.Should().Be(2);
47+
}
48+
49+
[Fact]
50+
public void Serialization_with_mismatching_type_between_constructor_argument_and_base_property()
51+
{
52+
var testCase = new DerivedWithMismatchedXTypeComparingWithBase(1, 2);
53+
54+
var exception = Record.Exception(() => testCase.ToJson());
55+
var e = exception.Should().BeOfType<BsonSerializationException>().Subject;
56+
57+
e.Message.Should().Be("Creator map for class MongoDB.Bson.Tests.Jira.CSharp1559Tests+DerivedWithMismatchedXTypeComparingWithBase has 2 arguments, but none are configured.");
58+
}
59+
60+
[Fact]
61+
public void Serialization_with_more_than_one_type_in_inheritance_hierarchy()
62+
{
63+
var testCase = new DerivedWithoutSetter_IntermediateBaseAndWithProtectedConstructor(1, 2, 3);
64+
65+
var json = testCase.ToJson();
66+
var result = BsonSerializer.Deserialize<DerivedWithoutSetter_IntermediateBaseAndWithProtectedConstructor>(json);
67+
68+
result.X.Should().Be(1);
69+
result.Y.Should().Be(2);
70+
result.Z.Should().Be(3);
71+
}
72+
73+
// private methods
74+
private int GetPropertyValue(object value, string propertyName)
75+
{
76+
return (int)Reflector.GetPropertyValue(value, propertyName, BindingFlags.Public | BindingFlags.Instance);
77+
}
78+
79+
// nested types
80+
public class BaseWithoutSetter
81+
{
82+
public BaseWithoutSetter(int? x)
83+
{
84+
X = x;
85+
}
86+
87+
public int? X { get; }
88+
}
89+
90+
public class DerivedWithMismatchedXTypeComparingWithBase : BaseWithoutSetter
91+
{
92+
public DerivedWithMismatchedXTypeComparingWithBase(int x, int y) : base(x)
93+
{
94+
Y = y;
95+
}
96+
97+
public int Y { get; }
98+
}
99+
100+
public class DerivedWithoutSetter_BaseWithoutSetter : BaseWithoutSetter
101+
{
102+
private readonly int _y;
103+
public DerivedWithoutSetter_BaseWithoutSetter(int? x, int y) : base(x)
104+
{
105+
_y = y;
106+
}
107+
108+
public int Y => _y;
109+
}
110+
111+
public class DerivedWithoutSetterAndWithBsonElementAndWithPrivateConstructor_BaseWithoutSetter : BaseWithoutSetter
112+
{
113+
private DerivedWithoutSetterAndWithBsonElementAndWithPrivateConstructor_BaseWithoutSetter() : base(null)
114+
{
115+
}
116+
117+
public DerivedWithoutSetterAndWithBsonElementAndWithPrivateConstructor_BaseWithoutSetter(int? x, int y) : base(x)
118+
{
119+
Y = y;
120+
}
121+
122+
[BsonElement("y")]
123+
public int Y { get; }
124+
}
125+
126+
public class DerivedWithPrivateSetterAndWithBsonElement_BaseWithoutSetter : BaseWithoutSetter
127+
{
128+
public DerivedWithPrivateSetterAndWithBsonElement_BaseWithoutSetter(int? x, int y) : base(x)
129+
{
130+
Y = y;
131+
}
132+
133+
[BsonElement("y")]
134+
public int Y { get; private set; }
135+
}
136+
137+
public abstract class AbstractBaseWithoutSetter
138+
{
139+
public AbstractBaseWithoutSetter(int? x)
140+
{
141+
X = x;
142+
}
143+
144+
public int? X { get; }
145+
}
146+
147+
public class DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetter : AbstractBaseWithoutSetter
148+
{
149+
public DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetter(int? x, int y) : base(x)
150+
{
151+
Y = y;
152+
}
153+
154+
public int Y { get; }
155+
}
156+
157+
public abstract class AbstractBaseWithoutSetterAndWithProtectedConstructor
158+
{
159+
protected AbstractBaseWithoutSetterAndWithProtectedConstructor(int? x)
160+
{
161+
X = x;
162+
}
163+
164+
public int? X { get; }
165+
}
166+
167+
public class DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetterAndWithProtectedConstructor : AbstractBaseWithoutSetterAndWithProtectedConstructor
168+
{
169+
public DerivedWithoutSetterAndWithoutBsonElement_AbstractBaseWithoutSetterAndWithProtectedConstructor(int? x, int y) : base(x)
170+
{
171+
Y = y;
172+
}
173+
174+
public int Y { get; }
175+
}
176+
177+
public class BaseWithoutSetterAndWithProtectedConstructor
178+
{
179+
protected BaseWithoutSetterAndWithProtectedConstructor(int? x)
180+
{
181+
X = x;
182+
}
183+
184+
public int? X { get; }
185+
}
186+
187+
public class DerivedWithoutSetterAndWithBsonElement_BaseWithoutSetterAndWithProtectedConstructor : BaseWithoutSetterAndWithProtectedConstructor
188+
{
189+
public DerivedWithoutSetterAndWithBsonElement_BaseWithoutSetterAndWithProtectedConstructor(int? x, int y) : base(x)
190+
{
191+
Y = y;
192+
}
193+
194+
[BsonElement("y")]
195+
public int Y { get; }
196+
}
197+
198+
public class BaseWithPrivateSetterAndWithProtectedConstructor
199+
{
200+
protected BaseWithPrivateSetterAndWithProtectedConstructor(int? x)
201+
{
202+
X = x;
203+
}
204+
205+
public int? X { get; private set; }
206+
}
207+
208+
public class DerivedWithoutSetter_BaseWithPrivateSetterAndWithProtectedConstructor : BaseWithPrivateSetterAndWithProtectedConstructor
209+
{
210+
public DerivedWithoutSetter_BaseWithPrivateSetterAndWithProtectedConstructor(int? x, int y) : base(x)
211+
{
212+
Y = y;
213+
}
214+
215+
public int Y { get; }
216+
}
217+
218+
public abstract class IntermediateBaseWithoutSetterAndWithProtectedConstructor : BaseWithPrivateSetterAndWithProtectedConstructor
219+
{
220+
protected IntermediateBaseWithoutSetterAndWithProtectedConstructor(int? z, int? x) : base(x)
221+
{
222+
Z = z;
223+
}
224+
225+
public int? Z { get; }
226+
}
227+
228+
public class DerivedWithoutSetter_IntermediateBaseAndWithProtectedConstructor : IntermediateBaseWithoutSetterAndWithProtectedConstructor
229+
{
230+
public DerivedWithoutSetter_IntermediateBaseAndWithProtectedConstructor(int? x, int y, int? z) : base(z, x)
231+
{
232+
Y = y;
233+
}
234+
235+
public int Y { get; }
236+
}
237+
}
238+
}

0 commit comments

Comments
 (0)