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

Noda Time plugin compatibility with AspNet Core Identity & plugin not being registered in migration #526

Closed
ghost opened this issue Jul 18, 2018 · 22 comments

Comments

@ghost
Copy link

ghost commented Jul 18, 2018

I'm currently using versions 2.1 of your wonderful driver for EF Core and also using the NodaTime plugin in order to user enhance date/time classes. Basically I have two main issues.

The first one is it's currently not possible for me to seed data in the Main of the application as it seems the plugin is not being registered. I've got this error popping. I've able to add migration and update the database though.

'The property 'Organization.CreatedAt' could not be mapped, because it is of type 'Instant' which is not a supported primitive type or a valid entity type. Either explicitly map this property, or ignore it using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.'

The second one is Iv'e inherited from IdentityUser in order to add CreatedAt and UpdatedAt properties of tipe Instant but I'm unable to add a migration. It throws me this error:

The current CSharpHelper cannot scaffold literals of type 'NodaTime.Instant'. Configure your services to use one that can.

Although when using a DateTime, everything is working properly.

I'm using version Npgsql.EntityFrameworkCore.PostgreSQL 2.1.1.1 and Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime 2.1.1

@austindrenski
Copy link
Contributor

@etiennemtl Are you explicitly registering the plugin when you configure services for DI?

Could you post some code showing how you're configuring EF Core?

@roji
Copy link
Member

roji commented Jul 18, 2018

Both issues likely have the same source. I'm assuming that you're seeding and attempting to create migrations from the command line tool (dotnet ef database update, dotnet ef migrations add XXX). If so, you probably need to read these docs, and make sure that your design-time DbContext correctly sets up the NodaTime plugin.

Let me know if this helps you.

@roji
Copy link
Member

roji commented Jul 18, 2018

Ah, just saw @austindrenski's response. Yes, if the above doc link doesn't help, try posting your context code etc.

@ghost
Copy link
Author

ghost commented Jul 18, 2018

Hey my DbContext lives in a seperate classlib project that is than added to my aspnetcore web project. The ApplicationDbContext is defined as follow:

public class ApplicationDbContext : IdentityDbContext<ApplicationUser>
{
        private readonly IClock clock;

        public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options, IClock clock)
            : base(options)
        {
            this.clock = clock;
        }

        protected override void OnModelCreating(ModelBuilder builder)
        {
            base.OnModelCreating(builder);

            // Use the new identity columns added in Postgresql 10.0
            builder.ForNpgsqlUseIdentityColumns();

            // IMPORTANT
            // Apply Postgresql naming conventions
            foreach (var entity in builder.Model.GetEntityTypes())
            {
                // Replace table names
                entity.Relational().TableName = entity.Relational().TableName.Pluralize().ToSnakeCase();

                // Replace column names            
                foreach (var property in entity.GetProperties())
                {
                    property.Relational().ColumnName = property.Name.ToSnakeCase();
                }

                foreach (var key in entity.GetKeys())
                {
                    key.Relational().Name = key.Relational().Name.ToSnakeCase();
                }

                foreach (var key in entity.GetForeignKeys())
                {
                    key.Relational().Name = key.Relational().Name.ToSnakeCase();
                }

                foreach (var index in entity.GetIndexes())
                {
                    index.Relational().Name = index.Relational().Name.ToSnakeCase();
                }
            }
        }
}

Besides it lives my DesignTimeFactory defined as follow:

public class ApplicationDbContextDesignTimeFactory : IDesignTimeDbContextFactory<ApplicationDbContext>
{   
    public ApplicationDbContext CreateDbContext(string[] args)
    {
        var builder = new DbContextOptionsBuilder<ApplicationDbContext>();
        var connectionString = Environment.GetEnvironmentVariable("DATABASE_URL");
        builder.UseNpgsql(connectionString, sql => sql.UseNodaTime());
        
        return new ApplicationDbContext(builder.Options, SystemClock.Instance);
    }
}

In my web project, in Startup the configure method is defined as follow:

public IServiceProvider ConfigureServices(IServiceCollection services)
{
    var connectionString = Environment.GetEnvironmentVariable("DATABASE_URL");
    services.AddDbContext<ApplicationDbContext>(options => options.UseNpgsql(connectionString, sql => sql.UseNodaTime()));

    services.AddIdentity<ApplicationUser, IdentityRole>()
        .AddEntityFrameworkStores<ApplicationDbContext>()
        .AddDefaultTokenProviders();
}

Here is an example command in order to add a migration

dotnet ef migrations add InitialIdentityModel --context applicationdbcontext -p ../Infrastructure/Infrastructure.csproj -s Web.csproj -o Identity/Migrations

And to update the database

dotnet ef database update -c applicationdbcontext -p ../Infrastructure/Infrastructure.csproj -s Web.csproj

@ghost
Copy link
Author

ghost commented Jul 19, 2018

@austindrenski For your information I'm also using StructureMap in order to register my other services.

When debugging the context, I see that the _options two extensions the first being an IServiceProvider referencing StructureMap. The other one being a Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal.NpgsqlOptionsExtensions containing a plugins being an instance of Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime.NodaTimePlugin. I really don't see why I'm getting this error...

@austindrenski
Copy link
Contributor

@etiennemtl Thanks for posting your context code. But looking at it, I'm still not sure what's happening.

Could you put together a minimal reproduction that recreates the exception, preferably in the form of a repo that we can clone?

@ghost
Copy link
Author

ghost commented Aug 2, 2018

@austindrenski I'm sorry for the late response, here is the repo https://github.com/etiennemtl/identity-npsql-nodatime. I've figured that the issue is when trying to configure multiple DbContext. It seems like even though each one is properly configured. When querying the Database, the configuration of each contexts seems to merge or something. I didn't find a way to isolate each of the context. There is two new contexts, they both target IdentityServer4. Maybe the bug is on there side. I'm quite lost at this point.

@roji
Copy link
Member

roji commented Nov 15, 2018

Sorry this issue hasn't received any attention recently.

As far as seeding NodaTime values, this is a known issue that was blocked on the EF Core side. The good news is that EF Core 2.2 contains the necessary infrastructure, and "code literal generation" has already been merged to Npgsql for many types. Unfortunately, this hasn't yet been done for NodaTime, which presents a few more complexities - but I hope we can finish this for the upcoming 2.2 release. This is tracked by #667.

In addition, EF Core implements a new plugin model in 2.2 (somewhat based on the one Npgsql introduced in 2.1), and the Npgsql provider has been changed to align with it (#658) - there's a good chance this would solve some of the issues you've been seeing.

As this issue is old and the problems it discusses are currently being worked on, I'm going to go ahead and close this - but please feel free to post back here, especially after 2.2 comes out.

@roji roji closed this as completed Nov 15, 2018
@Slaviusz
Copy link

Slaviusz commented May 2, 2019

Hi @roji,

it has happened to me today when I was refactoring an Entity to move some of its Instant properties to another; db-related 1:1 entity. After I made the changes and set-up new Entity with all properties and relations in fluent syntax I wanted to add new migration. I was presented with:
System.NotSupportedException: The type mapping for 'Instant' has not implemented code literal generation.
I'm on EFCore 2.2.4, Npgsql 2.2.0. Is this related?
Repro Github repo: https://github.com/Slaviusz/EFCoreSplittingEntityProblem

@roji
Copy link
Member

roji commented May 3, 2019

@Slaviusz code literal generation for NodaTime Instant is currently not implemented, and is tracked by #854

@Slaviusz
Copy link

Slaviusz commented May 3, 2019

@roji due to my lack of deeper understanding of EF Core, Npgsql and NodaTime I was wondering what is going on. Why do we need literal generation for as basic operation as creating new Entity or changing an existing Entity?
The refactoring operation I attempt to do is in fact no different than what I did many times before - Remove some properties from one entity and create another entity.
Is there a workaround for me? Maybe splitting this to two migrations?
It really blocks my progress on a project. Can you please explain to me what is going on because I was not able to extract any valuable information from the Exception / stack trace.

Thank You in advance.

@roji
Copy link
Member

roji commented May 3, 2019

@Slaviusz unless I'm mistaken, literal generation should only be required for data seeding, which is not exactly a critical feature. If you're seeing this message without doing any data seeding, please let me know.

@Slaviusz
Copy link

Slaviusz commented May 3, 2019

@roji I do no data seeding whatsoever. Please have a look at the simplistic repo I have created on GitHub. It has 3 commits, first one creates one complex entity and the third one does split it into two by moving Instant properties away. By trying to do "dotnet ef migrations add Split" on last commit you get the error message. This is even without any data or seeding applied.

@roji
Copy link
Member

roji commented May 3, 2019

I'll try looking at your repo and investigating deeper in the next couple of days. In the meantime, one sure workaround is to remove the existing migration and start from scratch.

@Slaviusz
Copy link

Slaviusz commented May 7, 2019

Hi @roji ,
I wanted to thank you for having a look. In the meantime I did experiment with the code and just removal of those Instant properties triggers the same error message. This is even with no dotnet ef database update applied.

What I figured out works is, if I split it to 2 migrations:

  1. Cast the Instant type to string (varchar), create first migration successfully
  2. Remove those properties creating second migration successfully

@roji
Copy link
Member

roji commented May 7, 2019

@Slaviusz sorry for not having the time to look at the moment...

In any case, the intention is to complete #854 for 3.0, which would make this go away. As long as you have a workaround at the moment, then we should be fine.

@invertedrider
Copy link

Workaround (remove previous migration) is working, however it would be quite annoying if I was already in prod; my previous migration is Initial which means that I must run dotnet ef database update 0 which drops the tables, and thus my data... Looking forward to resolution of #854!

@invertedrider
Copy link

Hi @roji ,
I wanted to thank you for having a look. In the meantime I did experiment with the code and just removal of those Instant properties triggers the same error message. This is even with no dotnet ef database update applied.

What I figured out works is, if I split it to 2 migrations:

  1. Cast the Instant type to string (varchar), create first migration successfully
  2. Remove those properties creating second migration successfully

@Slaviusz I don't quite understand your steps. When you say Cast, do you mean, change the property type for string, or truly adding a new property (string) that "casts" the value of the Instant property? If so how should it be named, and how does that solve the problem?
Also step 2 is unclear, probably because I don't understand step 1

Thanks!

@invertedrider
Copy link

Hi @roji ,
I wanted to thank you for having a look. In the meantime I did experiment with the code and just removal of those Instant properties triggers the same error message. This is even with no dotnet ef database update applied.

What I figured out works is, if I split it to 2 migrations:

  1. Cast the Instant type to string (varchar), create first migration successfully
  2. Remove those properties creating second migration successfully

On my side the problem arises when I add properties, not when I remove them; testing your workaround.

  1. Add a new property of type string (e.g.: public string Test{get;set;})
  2. Add a migration
  3. Change the property type to Instant
  4. Add another migration

So far, this is working. However, when updating the database, I have the following error:

Npgsql.PostgresException (0x80004005): 42804: column "date_updated" cannot be cast automatically to type timestamp without time zone
at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 490
at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1218
at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1042
at Npgsql.NpgsqlCommand.ExecuteNonQuery() in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1025
at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary2 parameterValues) at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary2 parameterValues)
at Microsoft.EntityFrameworkCore.Migrations.MigrationCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary2 parameterValues) at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQuery(IEnumerable1 migrationCommands, IRelationalConnection connection)
at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String contextType)
at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabase.<>c__DisplayClass0_1.<.ctor>b__0()
at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
42804: column "date_updated" cannot be cast automatically to type timestamp without time zone

@invertedrider
Copy link

@roji, do you have an ETA on version 3.0?

@Slaviusz
Copy link

Slaviusz commented Jul 3, 2019

Hey @invertedrider,
my workaround works only for removing as the Instant property has implicit conversion to String (TIMESTAMP to VARCHA on the DB side), however parsing a String to Instant is not so obvious due to different DateTime formats available.
What I meant by casting is really hard changing the type from Instant to String:
public Instant FaultyProperty { get; set; } becomes public string FaultyProperty { get; set; }

Then PostgreSQL does someting along the lines of:
ALTER TABLE x ADD COLUMN FaultyProperty2 (varchar);
UPDATE TABLE x SET FaultyProperty2 = FaultyProperty::varchar; <-- implicit casting here
ALTER TABLE x DROP COLUMN FaultyProperty;
ALTER TABLE x RENAME COLUMN FaultyProperty2 TO FaultyProperty;

Then of course the same column FaultyPropery is of type varchar and in the POCO the property FaultyProperty is of type string and dropping it in the next migration works.

Hope that helps.

Regards.

@invertedrider
Copy link

@Slaviusz thanks for the quick response

I've found in a related issue (#769, posted by @roji) that the problems lies in the fact that when the property is not nullable then EF Core asks for a litteral for the default value. Don't know why they would ask for it when removing a column; don't know either why it actually works on Initial migration; but your workaround is probably working because it has no problem going from Instant to string.

Anyway, in issue #769, @roji mentions that all you have to do is declare your property as nullable (e.g. Instant?) which removes the need for the litteral (no need for a default value). That's for adding properties however; I don't know if removing those properties will work directly or will still need your workaround

Take care

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

4 participants