Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/MongoDB.Bson/Serialization/BsonClassMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,11 @@ internal IDiscriminatorConvention GetDiscriminatorConvention()

void EnsureNoMemberMapConflicts(string elementName)
{
if (AppContext.TryGetSwitch("DisableCSharp4040Validation", out bool disableCSharp4040Validation) && disableCSharp4040Validation)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this switch is not set this change has no effect so should be very safe.

Copy link
Member

@sanych-sun sanych-sun Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use some more meaningful name? DisableDiscriminatorFieldUniqueValidation for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for thinking of that. I meant to ask if anyone wanted to suggest a different name.

What do others think?

In some ways the name is not very important because we plan to remove this anyway.

I hesitate to give it too good a name that sounds like we want people to use!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also suggest adding a namespace and a 'Switch' prefix:
Switch.MongoDB.Driver.DisableDiscriminatorFieldUniqueValidation

Looks like MS has the convention of prefixing the switches with 'Switch' string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used Switch.MongoDB.Driver.DisableDiscriminatorFieldConflictCheck.

The discriminator by definition is unique because there is only one. What we are checking is that it does not conflict with any members of the POCO.

{
return;
}

var conflictingMemberMap = _allMemberMaps.FirstOrDefault(memberMap => memberMap.ElementName == elementName);

if (conflictingMemberMap != null)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
/* Copyright 2010-present MongoDB Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

using System;
using System.Collections.Generic;
using System.Linq;
using MongoDB.Driver.TestHelpers;
using FluentAssertions;
using MongoDB.Bson.Serialization;
using MongoDB.Bson.Serialization.Conventions;
using MongoDB.Bson.Serialization.Serializers;
using MongoDB.Driver.Linq;
using Xunit;

namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira;

public class CSharp4040SwitchTests : LinqIntegrationTest<CSharp4040SwitchTests.ClassFixture>, IDisposable
{
static CSharp4040SwitchTests()
{
AppContext.SetSwitch("DisableCSharp4040Validation", true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajcvickers thank you for suggesting this approach. I was unaware of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second that, nice trick.


var discriminatorConvention = new HierarchicalDiscriminatorConvention("TypeNames");

BsonClassMap.RegisterClassMap<C>(cm =>
{
cm.AutoMap();
cm.SetIsRootClass(true);
cm.SetDiscriminatorIsRequired(true);
cm.MapMember(x => x.TypeNames).SetShouldSerializeMethod(_ => false);
cm.SetDiscriminatorConvention(discriminatorConvention);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only slightly different than what the user was doing in their sample code, but the end result is the same.

We can't modify the global serialization configuration the way they do, but we can simulate closely on this one class what they are doing globally.

});
}

public void Dispose()
{
AppContext.SetSwitch("DisableCSharp4040Validation", false);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is safe if any tests run in parallel.

We might choose not to push these tests to main, just the change to BsonClassMap.cs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question,
@sanych-sun any creative ideas?

Also if we are turning off the switch in Dispose, should we set turn it on in the constructor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not run tests in parallel, at least for now. We should make MongoClient not rely on statics before we can really run them in parallel. I suppose we either remove this workaround earlier, or will find a better way to configure this somehow via MongoClientSettings.

Copy link
Member

@sanych-sun sanych-sun Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now: setting this in constructor and removing in Dispose is good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if we are turning off the switch in Dispose, should we set turn it on in the constructor?

That might be too late.

What we really want to do is turn it off after ALL the tests in this file have run, but I don't know a way to do that. Turning it off after EACH test has run is the closest I can come to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not run tests in parallel, at least for now

So that means we could push this test file to main if we want to. We might remove the switch long before we are able to run tests in parallel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or will find a better way to configure this somehow via MongoClientSettings

To do it via MongoClientSettings would require adding to the public API. Since this is for one customer only a global switch should be adequate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we really want to do is turn it off after ALL the tests in this file have run, but I don't know a way to do that. Turning it off after EACH test has run is the closest I can come to that.

If it's turned off after each test, but set once, does that mean that just the first test case will see the switch turned on?
I think per-class disposing can be achieved by overriding dispose in ClassFixture.

Copy link
Contributor Author

@rstam rstam Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit odd... we are turning the switch on once in the static constructor and turning it off 6 times in the Dispose method (because there are 6 tests).

The reason it works OK is that by the time the first test has finished running the class map has been frozen and the switch is not consulted again.

I wouldn't worry too much about any of this since we will before long remove this switch and these tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are turning the switch on once in the static constructor and turning it off 6 times in the Dispose method (because there are 6 tests).

We can either set it in constructor and remove in Dispose of the test class - because test class is being created for each test, or we can use Fixture for that. Fixture is being created once for the whole test collection (test class in this case) and will be disposed only once - when all tests are done.

}

public CSharp4040SwitchTests(ClassFixture fixture)
: base(fixture)
{
}

[Fact]
public void Documents_should_serializer_as_expected()
{
var collection = Fixture.Collection;

var seralizedDocuments = collection.AsQueryable().As(BsonDocumentSerializer.Instance).ToList();

seralizedDocuments.Count.Should().Be(2);
seralizedDocuments[0].Should().Be("{ _id : 1, TypeNames : ['C', 'D'] }");
seralizedDocuments[1].Should().Be("{ _id : 2, TypeNames : ['C', 'D', 'E'] }");
}

[Fact]
public void OfType_C_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.OfType<C>();

var stages = Translate(collection, queryable);
AssertStages(stages, []);

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void OfType_D_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.OfType<D>();

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { TypeNames : 'D' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void OfType_E_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.OfType<E>();

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { TypeNames : 'E' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(2);
}

[Fact]
public void Where_TypeNames_Contains_C_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.Where(x => x.TypeNames.Contains("C"));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { TypeNames : 'C' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Where_TypeNames_Contains_D_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.Where(x => x.TypeNames.Contains("D"));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { TypeNames : 'D' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Where_TypeNames_Contains_E_should_work()
{
var collection = Fixture.Collection;

var queryable = collection.AsQueryable()
.Where(x => x.TypeNames.Contains("E"));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { TypeNames : 'E' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(2);
}

public abstract class C
{
public int Id { get; set; }
public virtual IReadOnlyList<string> TypeNames => ["C"];
}

public class D : C
{
public override IReadOnlyList<string> TypeNames => ["C", "D"];
}

public class E : D
{
public override IReadOnlyList<string> TypeNames => ["C", "D", "E"];
}

public sealed class ClassFixture : MongoCollectionFixture<C>
{
protected override IEnumerable<C> InitialData =>
[
new D { Id = 1 },
new E { Id = 2 }
];
}
}
Loading