Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using It.IsAny<object>() with a lambda from dynamic fails when binding properties #298

Closed
tomasaschan opened this issue Nov 7, 2016 · 5 comments

Comments

@tomasaschan
Copy link

I have the following call to a service I want to mock:

_database.GetEntity<Foo>(new { Id = id }); // _database is of type IDatabase

I tried doing it this way:

var dbMock = new Mock<IDatabase>(MockBehavior.Strict);
var listOfStuff = GetListOfStuff(); // an in-memory ICollection<Foo> with test data
dbMock.Setup(db => db.GetEntity<Foo>(It.IsAny<object>()))
    .Returns((dynamic parameters) => list.Single(f => f.Id == parameters.Id));

That compiles OK, and when I run the test with a breakpoint in the lambda passed to Single, hovering `parameters shows this:

mouse hovering parameters

so I know that it catches the correct object. However, when I execute that line - or, for that matter, if I add parameters.Id to watch expressions - I get

'parameters.Id' threw an exception of type 'Microsoft.CSharp.RuntimeBinder.RuntimeBinderException'
'object' does not contain a definition for 'Id'

Is this something inherent in how dynamic works, or could I change my usage of Moq to fix this?

@dmccaffery
Copy link

dmccaffery commented Nov 18, 2016

new { Id = id }

This is not dynamic; its an anonymous type. The compiler creates a new type for you during compilation. It (essentially) gets translated to the following:

**internal** class AnonymousType_Id_String
{
    private readonly string id;

    public AnonymousType_Id_String(string id)
    {
        this.id = id;
    }

    public string Id => this.id;
}

_database.GetEntity<Foo>(new AnonymousType_String(id));

The reason that the code compiles is that there is very little static checking around dynamic types... if the signature of your method is: IDatabase.GetEntity(dynamic parameters), then the compiler cannot perform any static checking on parameters.

The static type of the anonymous type is inaccessible because it is internal to the DLL in which it is defined, which is a different DLL than your test method containing your Moq is; so the runtime binder cannot access the property of Id.

I would suggest a few things:

This is not really the design intent behind dynamic, whereby a static type is morphed into a dynamic binding... dynamic should be a true dynamic type, such as ExpandoObject. I highly recommend using another generic to define the query parameters as an expression rather than rely on dynamic binding:

IDatabase.GetEntity<T>(Expression<Func<T, bool>> query);

Using that signature would look like:

_database.GetEntity<Foo>(foo => foo.Id == id);

@tomasaschan
Copy link
Author

As you've probably guessed, there's a very simple ORM underneath here. Currently, the object I'm passing in as new { Id = id } is reflected over, and supports a very simple set of scenarios: if the value is an IEnumerable<T> for some T, we can generate an IN statement, otherwise we generate a = statement. It's impossible to pass something other than that, and the code that generates the SQL statements and parameters is (relatively) simple.

If we start taking an Expression<Func<T, bool>> instead, there are suddenly no compile-time checks that hinder me from doing e.g. foo => foo.Id < id, for which we have no support for generating the SQL statements. Thus, there would be no way to let the type system declare what operations are supported, and only runtime errors would show when an unsupported operation is used. (Or, we could implement a full linq provider. We don't want to do that.)

Is there no other way to test this type of expression?

@dmccaffery
Copy link

dmccaffery commented Nov 18, 2016

An anonymous type is not an expression. Its a static type that the compiler is naming for you.

Short answer: with anonymous types wrapped as dynamics, there really isn't a sound way to test this. You would have to use reflection to access the properties of the actual type (ignoring dynamic typing). It is mostly a 'hack' around the compiler that you are even able to do this, as you're basically telling the compiler to ignore everything with the use of the dynamic keyword.

I don't quite understand what your test is trying to achieve, as it seems like you're reimplementing what GetEntity(dynamic obj); usually does, at least in part.

If you really, really want to use a dynamic API surface, change your test to do the .Single outside of the Linq to Moqs expression:

var list = GetListOfStuff();
var item = list.Single(f => f.Id == id);
...
    .Returns(object parameters => item);

That being said, the dynamic keyword disables all compile time checks; that's the primary reason why it exists. With your current implementation, I could do:

_database.GetEntity<Foo>(new { Bar = "GitHubRocks" });

This would compile perfectly fine. Absolutely any object would be allowed. A strongly-typed expression has compile time checks, especially type checking. I did assume you had an ORM, and I also assumed that the ORM, whatever it is, had an IQueryable implementation and an expression visitor, which is why I made the recommendation.

If you are supporting an extremely finite number of possibilities, use polymorphism:

IDatabase.GetEntity<T, TKey>(TKey id)
 where TKey : IEquatable<TKey>;

IDatabase.GetEntity<T, TKey>(IEnumerable<TKey> ids)
 where TKey : IEquatable<TKey>;

If you want to sacrifice performance for the sake of a cleaner API:

IDatabase.GetEntity<T>(object id);
IDatabase.GetEntity<T>(IEnumerable<object> ids);

Just pray that someone doesn't send in an IO stream; because you're throwing away any ability for the compiler to reason about what id is supposed to be.

@tomasaschan
Copy link
Author

Hm. Thanks for explaining the reason dynamic doesn't help me here again - I think I didn't really grok it the first time, but I do now.

The ORM underneath is a small "nano-ORM" we've built ourselves to support our needs but no more, so reflection over the properties is literally what we do. There's no IQueryable implementation and no expression visitor; we just generate raw SQL and a SqlParameter[] based on reflecting over the input object.

The reason I wanted to mock it this way (and not, e.g., by returning a single pre-determined item) is that the Setup will be re-used for multiple calls in the same test, and I want each invocation to return the correct object from the list. I'll probably build some type of custom, generic fake object to solve this need instead, which does the same logic against an in-memory collection also with reflection; alternatively, we'll pick up a "real" ORM which we can mock satisfyingly.

Thanks for taking the time to explain why my approach won't work, and to give some options - I really appreciate it!

@dmccaffery
Copy link

you could, potentially, do something like this:

var dbMock = new Mock<IDatabase>(MockBehavior.Strict);
var listOfStuff = GetListOfStuff(); // an in-memory ICollection<Foo> with test data
dbMock.Setup(db => db.GetEntity<Foo>(It.IsAny<object>()))
    .Returns(parameters => list.Single(f => f.Id == parameters.GetType().GetTypeInfo().GetDeclaredProperty("Id").GetValue(parameter)));

as a very hacky, but quick workaround. If you're not on CoreCLR or portable, you won't need the GetTypeInfo() extension method in between.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants