Skip to content

Commit

Permalink
CSHARP-4493: Fix incorrect flattening of $expr in $and operator (#1072)
Browse files Browse the repository at this point in the history
  • Loading branch information
sanych-sun authored and rstam committed May 24, 2023
1 parent ad5700b commit 5a973c2
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 136 deletions.
14 changes: 10 additions & 4 deletions src/MongoDB.Driver/FilterDefinitionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1711,7 +1711,9 @@ public override BsonDocument Render(IBsonSerializer<TDocument> documentSerialize

private static void AddClause(BsonDocument document, BsonElement clause)
{
if (clause.Name == "$and")
var fieldOrOperatorName = clause.Name;

if (fieldOrOperatorName == "$and")
{
// flatten out nested $and
foreach (var item in (BsonArray)clause.Value)
Expand All @@ -1726,10 +1728,12 @@ private static void AddClause(BsonDocument document, BsonElement clause)
{
((BsonArray)document[0]).Add(new BsonDocument(clause));
}
else if (document.Contains(clause.Name))
else if (document.Contains(fieldOrOperatorName))
{
var existingClause = document.GetElement(clause.Name);
if (existingClause.Value is BsonDocument existingClauseValue && clause.Value is BsonDocument clauseValue)
var existingClause = document.GetElement(fieldOrOperatorName);
if (IsFieldName(fieldOrOperatorName) &&
existingClause.Value is BsonDocument existingClauseValue &&
clause.Value is BsonDocument clauseValue)
{
var clauseOperator = clauseValue.ElementCount > 0 ? clauseValue.GetElement(0).Name : null;
if (clauseValue.Names.Any(op => existingClauseValue.Contains(op)) ||
Expand All @@ -1751,6 +1755,8 @@ private static void AddClause(BsonDocument document, BsonElement clause)
{
document.Add(clause);
}

static bool IsFieldName(string fieldOrOperatorName) => !fieldOrOperatorName.StartsWith("$");
}

private static void PromoteFilterToDollarForm(BsonDocument document, BsonElement clause)
Expand Down
187 changes: 55 additions & 132 deletions tests/MongoDB.Driver.Tests/FilterDefinitionBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,145 +46,68 @@ public void All_Typed()
}

[Fact]
public void And()
public void FilterDefinition_and_operator()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
subject.Eq("a", 1),
subject.Eq("b", 2));
var filter = subject.Eq("a", 1) & "{ a: 2 }";

Assert(filter, "{a: 1, b: 2}");
}

[Fact]
public void And_with_clashing_keys_should_get_promoted_to_dollar_form()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
subject.Eq("a", 1),
subject.Eq("a", 2));

Assert(filter, "{$and: [{a: 1}, {a: 2}]}");
}

[Fact]
public void And_with_clashing_keys_but_different_operators_should_get_merged()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
subject.Gt("a", 1),
subject.Lt("a", 10));

Assert(filter, "{a: {$gt: 1, $lt: 10}}");
Assert(filter, "{ $and : [ { a: 1 }, { a: 2 } ] }");
}

[Theory]
[InlineData("{ geoField : { $geoWithin : { $box : [ [ 1.0, 2.0 ], [ 3.0, 4.0 ] ] } } }", "{ geoField : { $near : [ 5.0, 6.0 ] } }")]
[InlineData("{ geoField : { $near : [ 5.0, 6.0 ] } }", "{ geoField : { $geoWithin : { $box : [ [ 1.0, 2.0 ], [ 3.0, 4.0 ] ] } } }")]
[InlineData("{ geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [ 1, 2 ] } } } }", "{ geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [ [ [ 1, 2 ], [ 3, 4 ], [ 5, 6 ], [ 7, 8 ] ] ] } } } }")]
[InlineData("{ geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [ [ [ 1, 2 ], [ 3, 4 ], [ 5, 6 ], [ 7, 8 ] ] ] } } } }", "{ geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [ 1, 2 ] } } } }")]
public void And_with_clashing_keys_and_different_operators_but_with_filters_that_support_only_dollar_form_should_get_promoted_to_dollar_form(string firstFilter, string secondFilter)
{
var subject = CreateSubject<BsonDocument>();

var combinedFilter = subject.And(firstFilter, secondFilter);

Assert(combinedFilter, $"{{ $and : [ {firstFilter}, {secondFilter} ] }}");
}

[Fact]
public void And_with_clashing_keys_and_different_operators_but_with_filters_that_support_only_dollar_form_and_empty_filter_should_ignore_empty_filter()
{
var subject = CreateSubject<BsonDocument>();

var combinedFilter = subject.And(
"{ geoField : { $near : [ 5.0, 6.0 ] } }",
"{ geoField : { } }");

Assert(combinedFilter, "{ geoField : { $near : [ 5.0, 6.0 ] } }");
}

[Fact]
public void And_with_an_empty_filter()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
"{}",
subject.Eq("a", 10));

Assert(filter, "{a: 10}");
}

[Fact]
public void And_with_a_nested_and_should_get_flattened()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
subject.And("{a: 1}", new BsonDocument("b", 2)),
subject.Eq("c", 3));

Assert(filter, "{a: 1, b: 2, c: 3}");
}

[Fact]
public void And_with_a_nested_and_and_clashing_keys()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.And(
subject.And(subject.Eq("a", 1), subject.Eq("a", 2)),
subject.Eq("c", 3));

Assert(filter, "{$and: [{a: 1}, {a: 2}, {c: 3}]}");
}

[Fact]
public void And_with_a_nested_and_and_clashing_operators_on_the_same_key()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.Lt("a", 1) & subject.Lt("a", 2);

Assert(filter, "{$and: [{a: {$lt: 1}}, {a: {$lt: 2}}]}");
}

[Fact]
public void And_with_a_nested_and_and_clashing_keys_using_ampersand()
{
var subject = CreateSubject<BsonDocument>();
var filter = subject.Eq("a", 1) & "{a: 2}" & new BsonDocument("c", 3);

Assert(filter, "{$and: [{a: 1}, {a: 2}, {c: 3}]}");
}

[Fact]
public void And_with_no_clauses()
{
var subject = CreateSubject<BsonDocument>();

var filter = subject.And();

Assert(filter, "{ $and : [] }");
}

[Fact]
public void And_with_one_empty_clause()
{
var subject = CreateSubject<BsonDocument>();
var empty = Builders<BsonDocument>.Filter.Empty;

var filter = subject.And(empty);
[InlineData("{ $and : [] }")]
[InlineData("{}", "{}")]
[InlineData("{}", "{}", "{}")]
[InlineData("{ a : 10 }", "{}", "{ a : 10 }")]
[InlineData("{ a : 10 }", "{ a : 10 }", "{}")]
[InlineData("{ a : 1, b : 2, c : 3 }", "{ a : 1 }", "{ b : 2 }", "{ c : 3 }")]
[InlineData("{ a : 1, b : 2, c : 3 }", "{ a : 1 }", "{ $and: [{ b : 2 }, { c : 3 }] }")]
[InlineData("{ a : { $gt : 1, $lt : 10 } }", "{ a : { $gt : 1 } }", "{ a : { $lt : 10 } }")]
[InlineData("{ $and : [{ a : { $lt : 1 } }, { a : { $lt : 2 } }] }", "{ a : { $lt : 1 } }", "{ a : { $lt : 2 } }")]
[InlineData("{ $and : [{ a : 1 }, { a : 2 }] }", "{ a : 1 }", "{ a : 2 }")]
[InlineData("{ $and : [{ a : 1 }, { a : 2 }, { c : 3 }] }", "{ a : 1 }", "{ a : 2 }", "{ c : 3 }")]
[InlineData("{ $and : [{ c : 3 }, { a : 1 }, { a : 2 }] }", "{ c : 3 }", "{ a : 1 }", "{ a : 2 }")]
[InlineData("{ $and : [{ a : 1 }, { a : 2 }, { c : 3 }] }", "{ $and : [{ a : 1 }, { a : 2 }] }", "{ c : 3}")]
[InlineData("{ $and : [{ a : 1 }, { a : 2 }, { c : 3 }] }", "{ $and : [{ a : 1 }, { $and : [{ a : 2 }] }] }", "{ c : 3 }")]
[InlineData("{ $and : [{ a : 1 }, { a : 2 }, { c : 3 }] }", "{ a : 1 }", "{ $and : [{ a : 2 }, { c : 3 }] }")]

[InlineData("{ geoField : { $near : [5.0, 6.0] } }", "{ geoField : { $near : [5.0, 6.0] } }", "{}")]
[InlineData(
"{ $and : [{ geoField : { $near : [40.0, 18.0] } }, { geoField : { $near : [42.0, 10.0] } }] }",
"{ geoField: { $near : [40.0, 18.0] } }",
"{ geoField : { $near : [42.0, 10.0] } }")]
[InlineData(
"{ $and : [{ geoField : { $geoWithin : { $box : [[1.0, 2.0], [3.0, 4.0]] } } }, { geoField : { $near : [5.0, 6.0] } }] }",
"{ geoField : { $geoWithin : { $box : [[1.0, 2.0], [3.0, 4.0]] } } }",
"{ geoField : { $near : [5.0, 6.0] } }")]
[InlineData(
"{ $and : [{ geoField : { $near : [5.0, 6.0] } }, { geoField : { $geoWithin : { $box : [[1.0, 2.0], [3.0, 4.0]] } } }] }",
"{ geoField : { $near : [5.0, 6.0] } }",
"{ geoField : { $geoWithin : { $box : [[1.0, 2.0], [3.0, 4.0]] } } }")]
[InlineData(
"{ $and : [{ geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [1, 2] } } } }, { geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [[[1, 2], [3, 4], [5, 6], [7, 8]]] } } } }] }",
"{ geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [1, 2] } } } }",
"{ geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [[[1, 2], [3, 4], [5, 6], [7, 8]]] } } } }")]
[InlineData(
"{ $and : [{ geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [[[1, 2], [3, 4], [5, 6], [7, 8]]] } } } }, { geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [1, 2] } } } }] }",
"{ geoField : { $geoIntersects : { $geometry : { type : 'Polygon', coordinates: [[[1, 2], [3, 4], [5, 6], [7, 8]]] } } } }",
"{ geoField : { $nearSphere : { $geometry : { type : 'Point', coordinates : [1, 2] } } } }")]

[InlineData("{ a : 1 , $expr : { $eq : ['$_id', 1] } }", "{ a : 1 }", "{ $expr : { $eq : ['$_id', 1] } }")]
[InlineData("{ $expr : { $eq : ['$_id', 1] }, a: 1 }", "{ $expr : { $eq : ['$_id', 1] } }", "{ a: 1 }")]
[InlineData("{ $expr : { $eq : ['$_id', 1] } }", "{ $expr : { $eq : ['$_id', 1] } }", "{}")]
[InlineData(
"{ $and : [{ $expr : { $eq : ['$_id', 1] } }, { $expr : { $ne : ['$a', ''] } }] }",
"{ $expr : { $eq : ['$_id', 1] } }",
"{ $expr : { $ne : ['$a', ''] } }")]
public void And(string expected, params string[] clauses)
{
var subject = CreateSubject<BsonDocument>();
var args = clauses.Select(c => new JsonFilterDefinition<BsonDocument>(c));

var filter = subject.And(args);

Assert(filter, "{ }");
}

[Fact]
public void And_with_two_empty_clauses()
{
var subject = CreateSubject<BsonDocument>();
var empty = Builders<BsonDocument>.Filter.Empty;

var filter = subject.And(empty, empty);

Assert(filter, "{ }");
Assert(filter, expected);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/* 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 System.Linq.Expressions;
using FluentAssertions;
using Xunit;

namespace MongoDB.Driver.Tests.Linq.Linq3ImplementationTests.Jira
{
public class CSharp4493Tests : Linq3IntegrationTest
{
[Fact]
public void Find_with_predicate_should_work()
{
var collection = CreateCollection();
string[] emails = { "email4@test.net" };
string[] addresses = { "495 pacific street plymouth, ma 02360" };
Expression<Func<Customer, bool>> filterByAddressAndEmails =
c => c.Emails.Any(ce => emails.Contains(ce.Email.ToLower())) &&
addresses.Contains(c.Address.FullAddress.ToLower());
FilterDefinition<Customer> emptyPredicate = Builders<Customer>.Filter.Empty;

var find = collection.Find(filterByAddressAndEmails & emptyPredicate);

var translatedFilter = TranslateFindFilter(collection, find);
translatedFilter.Should().Be("{$and: [{$expr: {$anyElementTrue: {$map: {input: '$Emails', as: 'ce', in: {$in: [{$toLower: '$$ce.Email'}, ['email4@test.net']]}}}}}, {$expr: {$in: [{$toLower: '$Address.FullAddress'}, ['495 pacific street plymouth, ma 02360']]}}]}");

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

private IMongoCollection<Customer> CreateCollection()
{
var collection = GetCollection<Customer>("C");

CreateCollection(
collection,
new Customer {
Id = 1,
Address = new AddressMetadata
{
FullAddress = "111 atlantic street plymouth, ma 02345"
},
Emails = new[]
{
new EmailMetadata
{
Email = "email4@test.net"
}
}
},
new Customer
{
Id = 2,
Address = new AddressMetadata
{
FullAddress = "495 pacific street plymouth, ma 02360"
},
Emails = new[]
{
new EmailMetadata
{
Email = "email4@test.net"
}
}
},
new Customer
{
Id = 3,
Address = new AddressMetadata
{
FullAddress = "495 pacific street plymouth, ma 02360"
},
Emails = new[]
{
new EmailMetadata
{
Email = "notEmail4@test.net"
}
}
});

return collection;
}

public class Customer
{
public int Id { get; set; }
public AddressMetadata Address { get; set; }
public IList<EmailMetadata> Emails { get; set; }
}

public sealed record AddressMetadata
{
public string FullAddress { get; set; }
}

public sealed record EmailMetadata
{
public string Email { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ protected BsonDocument TranslateFilter<TDocument>(IMongoCollection<TDocument> co
return filter.Render(documentSerializer, serializerRegistry, linqProvider);
}

protected BsonDocument TranslateFindFilter<TDocument, TProjection>(IMongoCollection<TDocument> collection, IFindFluent<TDocument, TProjection> find)
{
var filterDefinition = ((FindFluent<TDocument, TProjection>)find).Filter;
return filterDefinition.Render(collection.DocumentSerializer, BsonSerializer.SerializerRegistry, LinqProvider.V3);
}

protected BsonDocument TranslateFindProjection<TDocument, TProjection>(
IMongoCollection<TDocument> collection,
IFindFluent<TDocument, TProjection> find)
Expand Down

0 comments on commit 5a973c2

Please sign in to comment.