umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.49k stars 2.69k forks source link

Migration plan fails when using self defined Enum Types #14444

Open Musealali opened 1 year ago

Musealali commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

11.4.1

Bug summary

I am getting issues when running a migration plan to create a custom table and I get this error because the migration plan has a hard time resolving my enum.

image

And looking at the NPoco (which Umbraco uses) release notes (documentation is really bad on this), you can either use integer or string values for enums, see: https://github.com/schotime/NPoco/wiki/Release-Notes#2109

So adding either [ColumnType(typeof(string))] or [ColumnType(typeof(int))] to the Status property should work but it doesn't within Umbraco..

Its related to this codebase:

https://github.com/umbraco/Umbraco-CMS/blob/fccbed125cebab8604d5832f32c8ee9ea1e123[…]o.Infrastructure/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs

Maybe its because it only knows about sql types?

And therefor it fails on a self defined type?

might be the case that NPoco does support enums, but our codebase does not..

That might also be the reason why we have all the ModelDto classes and are mapping them to the actual models

Specifics

No response

Steps to reproduce

To reproduce this issue please follow:

https://docs.umbraco.com/umbraco-cms/extending/database

And create a migration plan for your project.

One thing to remember is in your Model/Schema/Table - remember to add a self defined Enum

image

Expected result / actual result

No response

kjac commented 1 year ago

Hi @Musealali,

Thank you for reaching out, and not least for all the root cause analysis work 💪

You're quite right... the syntax provider does not handle enums. It would be nice to have, but there is a bit of a caveat there... namely how does one define if the enum should be stored as a string or a numeric value?

If anyone can come up with a great solution for that, by all means please have a go at fixing this. I'm putting this up for grabs 😄

It is however recommended to maintain separate schema and DTO classes to avoid having either leak into the other - i.e. avoid having computed properties on the schema classes, and schema attribute mappings on the DTO classes. The docs article also mentions this (albeit in a side remark).

In the case of an enum, here's what one could do:

Schema definition and migration plan

public class EnumMigrationTestComponent : IComponent
{
    private readonly ICoreScopeProvider _coreScopeProvider;
    private readonly IMigrationPlanExecutor _migrationPlanExecutor;
    private readonly IKeyValueService _keyValueService;
    private readonly IRuntimeState _runtimeState;

    public EnumMigrationTestComponent(
        ICoreScopeProvider coreScopeProvider,
        IMigrationPlanExecutor migrationPlanExecutor,
        IKeyValueService keyValueService,
        IRuntimeState runtimeState)
    {
        _coreScopeProvider = coreScopeProvider;
        _migrationPlanExecutor = migrationPlanExecutor;
        _keyValueService = keyValueService;
        _runtimeState = runtimeState;
    }

    public void Initialize()
    {
        if (_runtimeState.Level < RuntimeLevel.Run)
        {
            return;
        }

        var migrationPlan = new MigrationPlan("EnumMigrationTest");
        migrationPlan.From(string.Empty)
            .To<AddEnumMigrationTestTable>("ED77065F-339C-4E67-9099-E34D016796AF");
        var upgrader = new Upgrader(migrationPlan);
        upgrader.Execute(_migrationPlanExecutor, _coreScopeProvider, _keyValueService);
    }

    public void Terminate()
    {
    }
}

public class AddEnumMigrationTestTable : MigrationBase
{
    public AddEnumMigrationTestTable(IMigrationContext context)
        : base(context)
    {
    }

    protected override void Migrate()
    {
        Logger.LogDebug("Running migration {MigrationStep}", nameof(AddEnumMigrationTestTable));

        if (TableExists("EnumMigrationTest") == false)
        {
            Create.Table<EnumMigrationTestSchema>().Do();
        }
        else
        {
            Logger.LogDebug("The database table {DbTable} already exists, skipping", "EnumMigrationTest");
        }
    }

    [TableName("EnumMigrationTest")]
    [PrimaryKey("Id", AutoIncrement = true)]
    [ExplicitColumns]
    public class EnumMigrationTestSchema
    {
        [PrimaryKeyColumn(AutoIncrement = true, IdentitySeed = 1)]
        [Column("Id")]
        public int Id { get; set; }

        [Column("Name")]
        public required string Name { get; set; }

        [Column("TestEnum")]
        public required string TestEnum { get; set; }
    }
}

DTO and data consumption

public class EnumMigrationTestController : UmbracoApiController
{
    private readonly IScopeProvider _scopeProvider;

    public EnumMigrationTestController(IScopeProvider scopeProvider)
        => _scopeProvider = scopeProvider;

    // GET /umbraco/api/enummigrationtest/getall
    [HttpGet]
    public IEnumerable<EnumMigrationTest> GetAll()
    {
        using IScope scope = _scopeProvider.CreateScope();
        List<EnumMigrationTest>? queryResults =
            scope.Database.Fetch<EnumMigrationTest>("SELECT * FROM EnumMigrationTest");
        scope.Complete();
        return queryResults;
    }
}

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

    public required string Name { get; set; }

    public required TestEnum TestEnum { get; set; }
}

public enum TestEnum
{
    One,
    Two,
    Three
}