Skip to content

Commit 16ba84a

Browse files
rstamrstam
committed
CSHARP-2644: Code review changes.
1 parent dca125f commit 16ba84a

File tree

3 files changed

+102
-66
lines changed

3 files changed

+102
-66
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ Please see our [guidelines](CONTRIBUTING.md) for contributing to the driver.
108108
* Daniel Goldman daniel@stackwave.com
109109
* Simon Green simon@captaincodeman.com
110110
* James Hadwen james.hadwen@sociustec.com
111+
* Nuri Halperin https://github.com/nurih
111112
* Jacob Jewell jacobjewell@eflexsystems.com
112113
* Danny Kendrick https://github.com/dkendrick
113114
* Ruslan Khasanbaev https://github.com/flaksirus

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

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,56 @@
1-
using MongoDB.Bson.Serialization.IdGenerators;
1+
/* Copyright 2019-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 MongoDB.Bson.Serialization.IdGenerators;
217
using MongoDB.Bson.Serialization.Serializers;
318

419
namespace MongoDB.Bson.Serialization.Conventions
520
{
621
/// <summary>
7-
/// A convention that sets representation of a string id class member to ObjectId in BSON with a StringObjectIdGenerator.
22+
/// A convention that sets the representation of a string id class member to ObjectId in BSON with a StringObjectIdGenerator.
23+
/// This convention is only responsible for setting the serializer and idGenerator. It is assumed that this convention runs after
24+
/// other conventions that identify which member is the _id and that the _id has already been added to the class map.
825
/// </summary>
926
public class StringIdStoredAsObjectIdConvention : ConventionBase, IMemberMapConvention
1027
{
11-
/// <summary>
12-
/// Applies a post processing modification to the class map.
13-
/// </summary>
14-
/// <param name="memberMap">The BsonMemberMap map.</param>
15-
/// <remarks>This method sets both the serializer and the IdGenerator on the id member field.</remarks>
28+
/// <inheritdoc/>
1629
public void Apply(BsonMemberMap memberMap)
1730
{
18-
var idMemberMap = memberMap.ClassMap?.IdMemberMap;
19-
20-
if (idMemberMap == null)
31+
if (memberMap != memberMap.ClassMap.IdMemberMap)
2132
{
2233
return;
2334
}
2435

25-
if (idMemberMap.MemberType != typeof(string))
36+
if (memberMap.MemberType != typeof(string))
2637
{
2738
return;
2839
}
2940

30-
if (idMemberMap.IdGenerator != null)
41+
var defaultStringSerializer = BsonSerializer.LookupSerializer(typeof(string));
42+
if (memberMap.GetSerializer() != defaultStringSerializer)
3143
{
3244
return;
3345
}
3446

35-
var idSerializer = idMemberMap.GetSerializer();
36-
37-
if (idSerializer is StringSerializer stringSerializer && stringSerializer.Representation == BsonType.String)
47+
if (memberMap.IdGenerator != null)
3848
{
39-
idMemberMap.SetSerializer(new StringSerializer(representation: BsonType.ObjectId));
40-
idMemberMap.SetIdGenerator(StringObjectIdGenerator.Instance);
49+
return;
4150
}
51+
52+
memberMap.SetSerializer(new StringSerializer(representation: BsonType.ObjectId));
53+
memberMap.SetIdGenerator(StringObjectIdGenerator.Instance);
4254
}
4355
}
4456
}

‎tests/MongoDB.Bson.Tests/Serialization/Conventions/StringIdStoredAsObjectIdConventionTests.cs‎

Lines changed: 72 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,95 +1,118 @@
1-
using Xunit;
2-
using MongoDB.Bson.Serialization.Serializers;
1+
/* Copyright 2019-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 FluentAssertions;
317
using MongoDB.Bson.Serialization;
4-
using MongoDB.Bson.Serialization.IdGenerators;
518
using MongoDB.Bson.Serialization.Conventions;
19+
using MongoDB.Bson.Serialization.IdGenerators;
20+
using MongoDB.Bson.Serialization.Serializers;
21+
using Moq;
22+
using Xunit;
623

724
namespace MongoDB.Bson.Tests.Serialization.Conventions
825
{
926
public class StringIdStoredAsObjectIdConventionTests
1027
{
11-
BsonMemberMap SampleMap<T>() => new BsonClassMap<T>(cm => cm.AutoMap()).GetMemberMap("Id");
28+
private readonly IBsonSerializer _defaultInt32Serializer = BsonSerializer.LookupSerializer(typeof(int));
29+
private readonly IBsonSerializer _defaultStringSerializer = BsonSerializer.LookupSerializer(typeof(string));
1230

1331
[Fact]
14-
public void Apply_StringId_SetsSerializer()
32+
public void Apply_should_ignore_any_member_that_is_not_the_id()
1533
{
16-
var target = new StringIdStoredAsObjectIdConvention();
17-
var subject = SampleMap<TestClassWithStringId>();
34+
var subject = CreateSubject();
35+
var memberMap = GetMemberMap<TestClassWithStringId>("X");
1836

19-
target.Apply(subject);
37+
subject.Apply(memberMap);
2038

21-
Assert.IsType<StringSerializer>(subject.GetSerializer());
39+
memberMap.GetSerializer().Should().BeSameAs(_defaultStringSerializer);
40+
memberMap.IdGenerator.Should().BeNull();
2241
}
2342

2443
[Fact]
25-
public void Apply_StringId_SetsIdGenerator()
44+
public void Apply_should_ignore_any_id_that_is_not_of_type_string()
2645
{
27-
var target = new StringIdStoredAsObjectIdConvention();
28-
var subject = SampleMap<TestClassWithStringId>();
46+
var subject = CreateSubject();
47+
var memberMap = GetIdMemberMap<TestClassWithIntId>();
2948

30-
target.Apply(subject);
49+
subject.Apply(memberMap);
3150

32-
Assert.IsType<StringObjectIdGenerator>(subject.IdGenerator);
51+
memberMap.GetSerializer().Should().BeSameAs(_defaultInt32Serializer);
52+
memberMap.IdGenerator.Should().BeNull();
3353
}
3454

3555
[Fact]
36-
public void Apply_ExistingIdGenerator_DoesNotApply()
56+
public void Apply_should_ignore_any_id_that_already_has_a_serializer_configured()
3757
{
38-
var target = new StringIdStoredAsObjectIdConvention();
39-
var subject = SampleMap<TestClassWithStringId>();
40-
subject.SetIdGenerator(CombGuidGenerator.Instance);
58+
var subject = CreateSubject();
59+
var memberMap = GetIdMemberMap<TestClassWithStringId>();
60+
var serializer = new StringSerializer();
61+
memberMap.SetSerializer(serializer);
4162

42-
target.Apply(subject);
63+
subject.Apply(memberMap);
4364

44-
Assert.IsType<CombGuidGenerator>(subject.IdGenerator);
65+
memberMap.GetSerializer().Should().BeSameAs(serializer);
66+
memberMap.IdGenerator.Should().BeNull();
4567
}
4668

4769
[Fact]
48-
public void Apply_NotStringSerializer_DoesNotApply()
70+
public void Apply_should_ignore_any_id_that_already_has_an_idGenerator_configured()
4971
{
50-
var target = new StringIdStoredAsObjectIdConvention();
51-
var subject = SampleMap<TestClassWithStringId>();
52-
subject.SetSerializer(new FakeStringSerializer());
72+
var subject = CreateSubject();
73+
var memberMap = GetIdMemberMap<TestClassWithStringId>();
74+
var idGenerator = Mock.Of<IIdGenerator>();
75+
memberMap.SetIdGenerator(idGenerator);
5376

54-
target.Apply(subject);
77+
subject.Apply(memberMap);
5578

56-
Assert.IsType<FakeStringSerializer>(subject.GetSerializer());
79+
memberMap.GetSerializer().Should().BeSameAs(_defaultStringSerializer);
80+
memberMap.IdGenerator.Should().BeSameAs(idGenerator);
5781
}
5882

59-
6083
[Fact]
61-
public void Apply_IntId_LeavesSerializer()
84+
public void Apply_should_configure_id_serializer_and_idGenerator()
6285
{
63-
var target = new StringIdStoredAsObjectIdConvention();
64-
var subject = SampleMap<TestClassWithIntId>();
86+
var subject = CreateSubject();
87+
var memberMap = GetIdMemberMap<TestClassWithStringId>();
6588

66-
target.Apply(subject);
89+
subject.Apply(memberMap);
6790

68-
Assert.IsNotType<StringSerializer>(subject.GetSerializer());
91+
var stringSerializer = memberMap.GetSerializer().Should().BeOfType<StringSerializer>().Subject;
92+
stringSerializer.Representation.Should().Be(BsonType.ObjectId);
93+
memberMap.IdGenerator.Should().BeOfType<StringObjectIdGenerator>();
6994
}
7095

71-
[Fact]
72-
public void Apply_IntId_NoIdGenerator()
73-
{
74-
var target = new StringIdStoredAsObjectIdConvention();
75-
var subject = SampleMap<TestClassWithIntId>();
96+
// private methods
97+
private StringIdStoredAsObjectIdConvention CreateSubject()
98+
=> new StringIdStoredAsObjectIdConvention();
7699

77-
target.Apply(subject);
100+
private BsonMemberMap GetIdMemberMap<T>()
101+
=> GetMemberMap<T>("Id");
78102

79-
Assert.Null(subject.IdGenerator);
80-
}
103+
private BsonMemberMap GetMemberMap<T>(string memberName)
104+
=> new BsonClassMap<T>(cm => cm.AutoMap()).GetMemberMap(memberName);
81105

106+
// nested types
107+
private class TestClassWithIntId
108+
{
109+
public int Id { get; set; }
110+
}
82111

83-
public class TestClassWithStringId { public string Id; }
84-
85-
public class TestClassWithIntId { public int Id; }
86-
87-
class FakeStringSerializer : SealedClassSerializerBase<string>
112+
private class TestClassWithStringId
88113
{
89-
public BsonType Representation => BsonType.String;
114+
public string Id { get; set; }
115+
public string X { get; set; }
90116
}
91117
}
92118
}
93-
94-
95-

0 commit comments

Comments
 (0)