[CSHARP-447] 1.4.1 "QueryBuilder.In" fails when BsonArray argument is cast to IEnumerable<BsonValue> Created: 18/Apr/12  Updated: 02/Apr/15  Resolved: 21/Apr/12

Status: Closed
Project: C# Driver
Component/s: None
Affects Version/s: 1.4.1
Fix Version/s: 1.4.2

Type: Bug Priority: Blocker - P1
Reporter: Roman Kuzmin Assignee: Robert Stam
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

\Driver\Builders\QueryBuilder.cs(1015):
public QueryConditionList In(IEnumerable<BsonValue> values)

It calls `if (values.Contains(null)) ...` with a null argument. At the same time in

\Bson\ObjectModel\BsonArray.cs(648):
public bool Contains(BsonValue value)

.. there is:
if (value == null)
{
throw new ArgumentNullException("value");
}

Thus, `In` seems to be always throwing an exception in 1.4.1



 Comments   
Comment by Robert Stam [ 21/Apr/12 ]

The fix includes the following changes:

1. BsonArray.Contains now returns false instead of throwing ArgumentNullException when called with null argument
2. Query.All, Query.In and Query.NotIn now handle nulls in the list of values according to functional construction semantics and therefore just skip them

Comment by Robert Stam [ 21/Apr/12 ]

Note that this same issue applies to Query.All and Query.NotIn as well as Query.In.

Comment by Robert Stam [ 21/Apr/12 ]

Changed the summary description of the issue to clarify that it doesn't always fail, it only fails when a BsonArray argument is cast to IEnumerable<BsonValue>.

Comment by Robert Stam [ 18/Apr/12 ]

Yes it would only be a temporary workaround. This issue will be resolved in the next release (and sooner than that of course in the master branch).

Comment by Roman Kuzmin [ 18/Apr/12 ]

> Can you work around this issue by not casting?
As a workaround - yes, indeed. But I would not like to use this always. In fact I do not do any explicit casting in my code. It is done implicitly in a chain of functions at some point.

Comment by Roman Kuzmin [ 18/Apr/12 ]

I am glad that you can repro. So far it's just my tests failed, not something in use (yet). Hopefully it goes this way until the fix. Thank you for taking a look at this.

When you get any changes done I will rerun my Mdbc tests. There are not so many tests there but they work

Comment by Robert Stam [ 18/Apr/12 ]

It's the casting of a BsonArray that triggers this issue. See:

var array = new BsonArray { 1 };
var query1 = Query.In("x", array); // works
var query2 = Query.In("x", (IEnumerable<BsonValue>)array); // throws exception

BsonArray.Contains probably shouldn't check whether the value argument is null. It should just return false (it will always return false because a BsonArray can't possibly contain a null, the Add methods prevent it).

Can you work around this issue by not casting?

Comment by Craig Wilson [ 18/Apr/12 ]

Ok. I can reproduce. The actual callstack is below:

  • QueryConditionList:In(IEnumerable<BsonValue> values)
  • Enumerable.Contains(IEnumerable<BsonValue> source, BsonValue value)
  • BsonArray.Contains(BsonValue value) <--- this one gets called (and throws) because BsonArray implements ICollection<BsonArray>.

I have also confirmed this is a breaking change in 1.4.1, although I can't find the commit that introduced this piece. I'm sorry you hit this issue. Let us figure out next steps and we'll get back.

Comment by Roman Kuzmin [ 18/Apr/12 ]

As for the test script he it is. Grab the Mdbc package (zip) from github, put it into posh module folder, replace your dlls there with newer and run this script:

Import-Module Mdbc
query Foo -In Bar

It fails:
New-MdbcQuery : Value cannot be null.
Parameter name: value
At C:\ROM\DEV\Mdbc\Test.ps1:3 char:1
+ query Foo -In Bar

Comment by Roman Kuzmin [ 18/Apr/12 ]

Yes, it worked before. My test cases in the Mdbc module fail with the new version.

Comment by Roman Kuzmin [ 18/Apr/12 ]

Yes, I created a snippet. This code (valid, I guess?) always fail:

var list1 = new QueryConditionList("name");
var arr1 = new BsonArray();
arr1.Add(1);
System.Collections.Generic.IEnumerable<BsonValue> enum1 = arr1;
list1.In(enum1);

Comment by Craig Wilson [ 18/Apr/12 ]

No, they don't contain a test case for BsonArray because code you referenced never goes into the BsonArray code. The values.Contains code you are referencing on line 1015 does not refer to a BsonArray.Contains call, but rather the Enumerable.Contains method call.

The values.Contains call in this code was changed in commit: https://github.com/mongodb/mongo-csharp-driver/commit/8ef88b3acd50ba641bfddcb542a1438147c5f241#diff-1. This commit happened for the 1.4 release, meaning it did not change for 1.4.1.

So, my question is this: was this working before and is failing now? If it used to be working and now isn't, then definitely provide the code you mentioned above (the Mdbc posh module) and I'll try and reproduce to figure it out.

Comment by Robert Stam [ 18/Apr/12 ]

Can you provide the code snippet that calls Query.In and the stack trace? Thanks.

Comment by Roman Kuzmin [ 18/Apr/12 ]

BTW, the test cases above do not contain a case with BsonArray (or I just do not see?)

Comment by Roman Kuzmin [ 18/Apr/12 ]

Craig, I got this information not from code analysis but from the debugger at the moment of failure. So it is the right overload.

The code is called from a PowerShell script via Mdbc module (available at github). Is it possible for you to use it? Then I will provide the details.

Comment by Craig Wilson [ 18/Apr/12 ]

While this does look suspicious, it is in fact valid. The overload you are referring to (IEnumerable<BsonValue> values) is not a BsonArray. Hence, the Contains call you are referring to is no the BsonArray.Contains, but rather the Enumerable.Contains extension method. I wrote the following assertions in a test and they all pass correctly.

Assert.Throws<ArgumentOutOfRangeException>(() => Query.In("j", 2, null, 3));
Assert.Throws<ArgumentOutOfRangeException>(() => Query.In("j", new List<BsonValue>

{2, null, 3 }

));

Assert.Throws<ArgumentOutOfRangeException>(() => Query.In("j", BsonValue.Create(2), BsonValue.Create(null), BsonValue.Create(3)));
Assert.Throws<ArgumentOutOfRangeException>(() => Query.In("j", new List<BsonValue>

{BsonValue.Create(2), BsonValue.Create(null), BsonValue.Create(3) }

));

Would you mind posting the code that used to work that doesn't anymore so I can take a more specific look at the issue?

Generated at Wed Feb 07 21:36:51 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.