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

Virtualize RepositoryBase methods #66

Closed
ngruson opened this issue Dec 6, 2020 · 5 comments
Closed

Virtualize RepositoryBase methods #66

ngruson opened this issue Dec 6, 2020 · 5 comments
Milestone

Comments

@ngruson
Copy link
Contributor

ngruson commented Dec 6, 2020

What do you feel about making the methods in RepositoryBase.cs virtual?
I ran into a NullReferenceException when unit testing my concrete EF6 implementation of the repository with a mocked DbContext.

public virtual async Task UpdateAsync(T entity)
{
  dbContext.Entry(entity).State = EntityState.Modified; // Entry call to mocked DbContext returns null

  await SaveChangesAsync();
}

I'm not able to mock the Entry method of DbContext because the constructor of DbEntityEntry is not public.

If UpdateAsync was virtual in RepositoryBase, I could override it in my concrete repository:

public override async Task UpdateAsync(T entity)
{
  ((MyDbContext)DbContext).SetModified(entity);
  await SaveChangesAsync();
}

SetModified in my concrete repository:

public virtual void SetModified(object entity)
{
  Entry(entity).State = EntityState.Modified;
}

I would need a protected property to get to the DbContext.
In this setup, I'm able to unit test my concrete repository without issues.

@fiseni
Copy link
Collaborator

fiseni commented Dec 6, 2020

Hi @ngruson,

It's not about the method implementations only, but also the namespaces used within the "RepositoryBase".

The EntityFramework6 project should fully implement IRepositoryBase interface, and not inherit from the existing one in EntityFrameworkCore project. These two "addon/plugin" projects shouldn't reference each other. As for making the methods virtual, sure, why not.

Also, we should have a new project for tests, something like EntityFramework6.IntegrationTests. That way we can clearly test upon EF6, without referencing the EFCore at all.

This is my stand initially, @ardalis may have a different opinion.

PS. We have to refactor the test infrastructure here. It's in our backlog for some time. We should have the following:

  • UnitTests project for the base package.
  • IntegrationsTests project for each plugin package.

[EDIT]
Now that I read it again, I think I misunderstood you, sorry :). In the next release, we may refactor the methods as virtual.

@ngruson
Copy link
Contributor Author

ngruson commented Dec 6, 2020

Hi @fiseni

Just to clarify further:
The EF6 base repository is not referencing the EF Core base repository and is implementing IRepositoryBase.
In my solution I have a repository which is inheriting from RepositoryBase (it's EF6 but it might as well be EF Core).
I have unit tests in my solution to test this repository. So this is not about the tests in Ardalis.Specification.

Shall I make a PR to virtualize these methods in both the EF Core and EF6 RepositoryBase classes?

@fiseni
Copy link
Collaborator

fiseni commented Dec 6, 2020

Lets' first wait to merge the existing PR, and fix the EF6 project properly. Once we sort that out, then you can create an additional PR for these changes. Sounds OK?

For now, please use your own fully implemented Repository (you may just copy the implementations and update them as you wish). Anyhow, the RepositoryBase is given just as an optional base class and just as a reference. We try to avoid leading or enforcing users to use it.

@fiseni
Copy link
Collaborator

fiseni commented Mar 2, 2021

The methods of base repositories now are marked as virtual in v5.

@fiseni fiseni added this to the 5.0 milestone Mar 2, 2021
@fiseni
Copy link
Collaborator

fiseni commented Mar 5, 2021

Implemented in version 5.0.3

@fiseni fiseni closed this as completed Mar 5, 2021
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