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

Option to not load the navigation property #45

Open
leopripos opened this issue Jan 22, 2021 · 8 comments
Open

Option to not load the navigation property #45

leopripos opened this issue Jan 22, 2021 · 8 comments
Assignees

Comments

@leopripos
Copy link

leopripos commented Jan 22, 2021

Do we already have this feature?

If no, I think this is necessery to make sure that Include statement is called or not in my repository or query.
The sample scenario:

public class Lesson {
  public int Id {get; set;}
}

public class Student {
  public int Id {get; set;}

  public List<Lesson> Lessons   { get; set; }
}

var student1 = new Student(){
  Id = 1,
  Lessons = new List<Lesson>(){
    new Lesson(){
      Id = 1,
    }
  }
}

var students = new List<Student>(){
  student1
};

var dataSetMock = students.AsQueryable().BuildMockDbSet();

var dbContextMock = new Mock<IDbContextInterface>();
dbContextMock.SetupGet(e => e.Students)
    .Returns(dataSetMock.Object);

var repository = new StudentsRepository(dbContextMock.Object);

var studentResult = repository.GetById("12312312");

studentResult.Lessons.Should().NotBeNull();

But this will causes studentResult == student1 returns false because we need to instansiate new object.

My suggestion for this, we can add new param in BuildMockDbSet

public static IQueryable<TEntity> BuildMock<TEntity>(this IQueryable<TEntity> data, bool loadNavigation= true) where TEntity : class {}

Thanks
:)

@romantitov
Copy link
Owner

Hi @leopripos, thanks you for the feedback. I would really like to help you.

What happening in repository.GetById ?
Why do you expect that repository.GetById("12312312") will create a new object?

@romantitov romantitov self-assigned this Jan 22, 2021
@romantitov
Copy link
Owner

Closing the issue, since there is not enough details to reproduce it

@leopripos
Copy link
Author

Sorry @romantitov for the slow response. My bad.

The issue is "How do I make sure (test) that the Lessons navigation property is included or not in my repository"
Here is the repository: Good version

public Student GetById(string id)
{
      return _dbContext.Students
                .Include(e => e.Lessons)
                .Where(e => e.Id == id)
                .FirstOrDefault();
}

In the test code above, studentResult.Lessons.Should().NotBeNull(), will always success even I write my repository without Include like this: (Bad version)

public Student GetById(string id)
{
      return _dbContext.Students
                .Where(e => e.Id == id)
                .FirstOrDefault();
}

That's because, it returns the student1 object which was used when creating DbSet mock. And yes, the Lessons property of object student1 has value, not null.

@romantitov romantitov reopened this Jan 29, 2021
@thomaslevesque
Copy link

@leopripos I think the problem is that MockQueryable creates an in-memory queryable (Linq to Objects), but Include is an Entity Framework feature, which translates to SQL joins. Since the queryable isn't an EF Core queryable, it doesn't know how to handle Include. Unfortunately I don't think it would be possible to support Include in MockQueryable...

If it were possible, it would be awesome... I just ran into an issue with a filtered include (EF Core 5), and I'm now realizing that MockQueryable can't do anything about it.

@romantitov
Copy link
Owner

romantitov commented Feb 4, 2021

@leopripos, thank you for more detailed explanation. I agree, that would be a very useful feature. I even did some research and I think technically it is possible (with some changes) to see whether an Include executed or not. But since Include is an extension it is not so easy to verify an execution of the extension with frameworks like Moq. Unfortunately the feature is not something what can be easily done and will take some time.

I usually work with the project in my free time and now I'm quite busy with my commercial projects for now.

I'll keep the issue opened and who knows, maybe one day me or someone else will add the feature to the project. But I cannot promise that it will be soon.

@thomaslevesque
Copy link

@romantitov assuming you're able to detect the call to Include, I think there's still a problem. For instance, if I take @leopripos' code as an example:

var student1 = new Student(){
  Id = 1,
  Lessons = new List<Lesson>(){
    new Lesson(){
      Id = 1,
    }
  }
}

var students = new List<Student>(){
  student1
};

var dataSetMock = students.AsQueryable().BuildMockDbSet();

dataSetMock will return student1 as-is. But if you take the use of Include into account, dataSetMock without Include(s => s.Lessons) should return student1 without the content of Lessons, so you would need to either (1) clear the Lessons list on the original student1 instance, or (2) clone student1 without the list.

Both approaches have issues:

  • (1) would modify the original object, which the user might not expect. Also, you would need to keep track of the original content of Lessons somehow is case the query is repeated with the appropriate Include
  • (2) would return a different instance than the original. I think this is reasonable, but it's a breaking change compared to the current behavior, and might break some tests (I know it would break some of mine). Also, there's the problem of how to clone the original instance, which might not always be possible

@romantitov
Copy link
Owner

@thomaslevesque I see it a bit different. I think that it's not a good idea to create a new unexpected instance, but I would like to make it possible to verify that expected Include was executed:

var student1 = new Student(){
  Id = 1,
  Lessons = new List<Lesson>(){
    new Lesson(){
      Id = 1,
    }
  }
}

var students = new List<Student>(){
  student1
};

var dataSetMock = students.AsQueryable().BuildMockDbSet();
.....


dataSetMock.VerifyInclude(x => x.Lessons );

@thomaslevesque
Copy link

Ah, I see

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

3 participants