Problems rebuilding MySQL Migration for IdentityServer.EntityFrameworkCore

[Y2zz]

[Y2zz]

Volo.Abp.IdentityServer has multiple fields that are out of length set to the primary key, causing the problem to be discovered

e.g:

1. ClientPostLogoutRedirectUriConsts.PostLogoutRedirectUriMaxLength
2. ClientPropertyConsts.ValueMaxLength
3. ClientRedirectUriConsts.RedirectUriMaxLength
4. SecretConsts.ValueMaxLength

[Y2zz]

Some problems have arisen due to the latest source code.
I can not solve this problem

[hikalkan]

@maliming we may need to change lenghts, or re-define the PK. Can you check and write all the problems and possible solutions please? Thanks.

[Y2zz]

The most appropriate solution is to move them out of the primary key.

[Y2zz]

I have a temporary solution, first turn the value down to solve the problem that cannot be migrated at the moment.

#1921

[maliming]

Maybe we should talk to the identity server because its EF doesn't seem to work with mysql.

Related IdentityServer/IdentityServer4#2145 (comment)

[maliming]

There are currently two problems in mysql:
The first one is varchar(length). Its maximum length is related to the character set. When a character occupies one or more possible bytes, the maximum length may be 65535 or 21844.

The second is the maximum length of the key column is 3072 bytes.

In short, these are compatibility issues between different databases, just as the length of column names in oracle cannot exceed 30.

We can customize the maximum length of index columns and varchar to be compatible with common database requirements.
But we can't determine the requirements of all the databases. That is to say we can't find a suitable length to meet the needs of the application and database.

Maybe we should keep in sync with identity server4. If there is compatibility problem with database length, etc, developers can modify it themselves. For example, when I use oracle, I need to rename some fields. (30 or 128 depends on the database version. )

@hikalkan What do you think?

[Y2zz]

This is my current solution

add IdentityServerModelCreatingExtensions.cs

    public static class IdentityServerModelCreatingExtensions
    {
        public static void ConfigureIdentityServerForGaea (this ModelBuilder builder)
        {
            // Solve the problem of MySQL migration
            // https://github.com/abpframework/abp/issues/1920
            
            builder.Entity<ApiSecret>(b =>
            {
                // After trying, you can also set it to 400
                b.Property(x => x.Value).HasMaxLength(300);
            });
            
            builder.Entity<ClientPostLogoutRedirectUri>(b =>
            {
                b.Property(x => x.PostLogoutRedirectUri).HasMaxLength(300); // or 400 ？
            });
            
            builder.Entity<ClientRedirectUri>(b =>
            {
                b.Property(x => x.RedirectUri).HasMaxLength(300); // or 400 ？
            });
            
            builder.Entity<ClientSecret>(b =>
            {
                b.Property(x => x.Value).HasMaxLength(300); // or 400 ？
            });
        }
    }

change MigrationsDbContext.cs

builder.ConfigureGaea();
builder.ConfigureIdentityServerForGaea(); // add

[hikalkan]

@Y2zz I suppose this is the most proper solution to this problem, you arrange columns based on the DBMS you're using.

Based on your solution, I will create a built-in way of it.

[hikalkan]

I've implemented it via 647b863

Usage:

builder.ConfigureIdentityServer(options =>
{
    options.DatabaseProvider = EfCoreDatabaseProvider.MySql;
});

Do this on your Migration DbContext.

[Y2zz]

I think this way deviates from my original intention.

[maliming]

@hikalkan If so, it means that we need to make special configurations for possible database providers in the module. Although some providers are not required to be configured by default.

I think we shouldn't do it, we should give it to the developer to handle it. Depends on the database used by the developer.

If a module may not be compatible with a database, the module should explain it and provide a solution.

[hikalkan]

@hikalkan If so, it means that we need to make special configurations for possible database providers in the module. Although some providers are not required to be configured by default.

Just making for exceptional cases. In the implementation, it only checks MySql for a few places. In the future, we may add some exceptions for other databases if needed.

I think we shouldn't do it, we should give it to the developer to handle it. Depends on the database used by the developer.

In general, I agree on this idea. However, if we don't implement it out of the box, then we will provide documentation for it and every developer will repeat it in their code or create another issue on GitHub to ask it to us.

I don't intent to solve all provider issues which is not possible. But, for example if one developer has problems with X provider, creates an issue and shows the solution then we can add it to the module so everyone can use easily.

Module configures its own database mapping, so being compatible to different databases is a plus.

[jack-gaojz-zz]

I've implemented it via 647b863

Usage:

builder.ConfigureIdentityServer(options =>
{
    options.DatabaseProvider = EfCoreDatabaseProvider.MySql;
});

Do this on your Migration DbContext.

I used the code change as below:

//builder.ConfigureIdentityServer();
builder.ConfigureIdentityServer(options => {`
      options.DatabaseProvider = EfCoreDatabaseProvider.Oracle;
});

The issue #2704 still reproduce it. :(

[maliming]

@jack-gaojz
Because the default is only compatible with mysql, you can refer to this commit(647b863) for compatibility with oracle, if you like, welcome to contribute code.

[jack-gaojz-zz]

@jack-gaojz
Because the default is only compatible with mysql, you can refer to this commit(647b863) for compatibility with oracle, if you like, welcome to contribute code.

Yes, that's my honor. I will do it. Thanks. :)

[LeiYangGH]

builder.ConfigureIdentityServerForGaea();

Which namespace is ApiSecret in? I cannot find this class and VS suggests ISecret, which lacks Value property.

[abdullahsalem]

Thanks @hikalkan and @maliming for the given temporary solutions. However, I think this issue should be open to make it noticeable and to prevent the upcoming duplicated issues, until some best-practices would be written in the documentation as it was mentioned:

In general, I agree on this idea. However, if we don't implement it out of the box, then we will provide documentation for it and every developer will repeat it in their code or create another issue on GitHub to ask it to us.

[ebicoglu]

if you have a an existing project with Migrations and you want to convert it to Oracle then you need to delete the Migrations folder to make it work. Because the existing migrations were created based on your previous database provider. I wrote this because some people fall into this trap.