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

Add IsRelational(), IsSqlServer(), IsInMemory(), etc. (or GetProviderName()) on DatabaseFacade #6983

Closed
punmechanic opened this issue Nov 10, 2016 · 13 comments · Fixed by #19521
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. punted-for-2.0 type-enhancement
Milestone

Comments

@punmechanic
Copy link

punmechanic commented Nov 10, 2016

Steps to reproduce

Given the following class:

class DummyContext : DbContext
{
     protected override void OnConfiguring(DbContextOptionsBuilder builder)
     {
            builder.UseInMemoryDatabase();
     }
}

When used in the following manner:

public class WittyClassName
{
    DbContext ctx;

    public WittyClassName(DbContext ctx)
    {
        this.ctx = ctx;
    }

    public async Task DoWorkAsync()
    {
        await ctx.Database.MigrateAsync();
    }
}

The issue

The above code will grant the following error on the line of MigrateAsync when executing DoWorkAsync:

System.InvalidOperationException : Unable to resolve service for type 'Microsoft.EntityFrameworkCore.Migrations.IMigrator'. This is often because no database provider has been configured for this DbContext. A provider can be configured by overriding the DbContext.OnConfiguring method or by using AddDbContext on the application service provider. If AddDbContext is used, then also ensure that your DbContext type accepts a DbContextOptions<TContext> object in its constructor and passes it to the base constructor for DbContext.
  Stack Trace:
       at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
       at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.MigrateAsync(DatabaseFacade databaseFacade, CancellationToken cancellationToken)

This only occurs with the in memory database from my experience and works just fine with the others I've used (which is just sqlite). I would have expected this to have just "worked"; if an in memory schema cannot be created, then make the migration be a no-op.

I can resolve this by providing my own service provider but this involves having to mock not only IMigrator but any other service that the database uses, like IServiceScopeFactory. This is a lot of work to have an integration test in an in memory database that utilises migrations. The other option would be to leave this section of the code untested.

Further technical details

EF Core version: 1.0.1, Microsoft.EntityFrameworkCore.InMemory is also 1.0.1.
Operating system: OSX Sierra 10.12.1
Visual Studio version: using VS Code 1.5.3

@punmechanic punmechanic changed the title MigrateAsync will fail at runtime due to IMigrator being missing on In Memory databsaes MigrateAsync will fail at runtime due to IMigrator being missing on In Memory databases Nov 10, 2016
@punmechanic
Copy link
Author

punmechanic commented Nov 10, 2016

Another alternative is to use Microsoft.EntityFrameworkCore.Sqlite with the connection string set to Data Source=:memory:.

As an aside, I'm not sure if I would class this as a bug or not, but it's definitely Unexpected Behaviour in my opinion. Feels a bit like a leaky abstraction, but this is mostly a consequence of my wonky architecture in my application.

@ErikEJ
Copy link
Contributor

ErikEJ commented Nov 10, 2016

InMemory is not a relational provider, and therefore does not support migrations

@rowanmiller
Copy link
Contributor

SQLite in in-memory mode is definitely the way to go if you want to do things like migrations. As @ErikEJ mentioned, InMemory is not intended to be a relational database. The Migrations feature is only contained in the relational libraries since it doesn't really make sense for all types of databases. So InMemory doesn't even depend on it. We're going to update the docs to point folks more heavily towards SQLite rather than InMemory.

Leaving open so that we can discuss making migrations not-throw (and just no-op) when the provider doesn't have migrations. This should probably be a "throw but allow a flag that stops it throwing" approach because there could be relational providers that just haven't implemented migrations yet.

@punmechanic
Copy link
Author

Cool, good to hear. Totally understand that InMemory is not relational, but the error doesn't really convey that very well. :(

@divega
Copy link
Contributor

divega commented Nov 11, 2016

EF Team Triage: we want to improve the exception message to explain that migrations is not supported by the provider and add sugar to ask if the database supports this functionality (e.g. IsRelational(), provider-specific IsSqlServer() and IsInMemory() or GetProviderName()) to make it easier to write conditional code in tests.

@divega divega added this to the 1.2.0 milestone Nov 11, 2016
@ajcvickers ajcvickers modified the milestones: Backlog, 2.0.0 Apr 17, 2017
@AndriySvyryd
Copy link
Member

Merge with #8566 ?

@onionhammer
Copy link

onionhammer commented Oct 11, 2018

InMemory is not a relational provider, and therefore does not support migrations

@ErikEJ
And therefore does not support seed data? this does not track for me. InMemory should be the easiest provider to support seed-data on

@divega divega changed the title MigrateAsync will fail at runtime due to IMigrator being missing on In Memory databases Add IsRelational(), IsSqlServer(), IsInMemory(), etc. (or GetProviderName()) to DatabaseFacade Apr 2, 2019
@divega divega changed the title Add IsRelational(), IsSqlServer(), IsInMemory(), etc. (or GetProviderName()) to DatabaseFacade Add IsRelational(), IsSqlServer(), IsInMemory(), etc. (or GetProviderName()) on DatabaseFacade Apr 2, 2019
@AndriySvyryd
Copy link
Member

Already fixed by #8545?

@ajcvickers
Copy link
Member

Ping @roji

@ajcvickers
Copy link
Member

Checked and all are implemented except IsRelational.

@WeihanLi
Copy link
Contributor

WeihanLi commented Jan 7, 2020

Checked and all are implemented except IsRelational.

any plan for IsRelational ?

@ajcvickers
Copy link
Member

@WeihanLi This issue is in the Backlog milestone with the consider-for-next-release tag. This means it's not currently planned for the 5.0 release, but it's something we may implement in 5.0 as a stretch goal--in this case because it's a small change.

This issue is also marked with good first issue which indicates it is a good candidate for a community pull request.

@WeihanLi
Copy link
Contributor

WeihanLi commented Jan 7, 2020

thanks for the info @ajcvickers

@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed propose-close labels Jan 9, 2020
@ajcvickers ajcvickers modified the milestones: Backlog, 5.0.0 Jan 9, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview1 Mar 13, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview1, 5.0.0 Nov 7, 2020
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. punted-for-2.0 type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants