Repodb: Enhancement: [Feature Request] Set default enum conversion

[SpaceOgre]

At the moment RepoDb is using NVARCHAR/TEXT as default conversion for enums and offers the possibility to change these either per enum type via the TypeMapper or per property via the FluentMapper. It also checks the database schema, but this is not possible in all scenarios.

I usually always want my enums to be ints in the database and would like to be able to set a default enum conversion for RepoDb so I don't have to map each enum manually.

I would suggest something like this, that can be called when doing the rest of the RepoDb setup:
c# RepoDb.TypeMapper.DefaultEnumConversion = EnumConversion.Int; RepoDb.TypeMapper.DefaultEnumConversion = EnumConversion.Nvarchar;

Using an enum and not bool since more options might be nice and I think it is clearer in this instance too.

[fredliex]

Perhaps two or more of the above situations will occur at the same time in the same system.
So I recommend using ConverterFactory like the following

RepoDb.TypeMapper.EnumConverterFactory = new DefaultEnumConverterFactory();

public class DefaultEnumConverterFactory : IEnumConverterFactory
{
    public object CreateConverter(Type enumType) => new StringEnumConverter<TEnum>();
}
public class StringEnumConverter<TEnum> : IEnumConverter<TEnum, string>
{
     public TEnum Get (string input, ClassProperty property) => ...;
     public string Set (TEnum input, ClassProperty property) => ...;
}

Then you can customize EnumConverterFactory and IntEnumConverter like follow

RepoDb.TypeMapper.EnumConverterFactory = new CustomEnumConverterFactory();

public class CustomEnumConverterFactory : IEnumConverterFactory
{
    public object CreateConverter(Type enumType) => new IntEnumConverter<TEnum>();
}

do you think it is similar to IPropertyHandler? :smiley:

[fredliex]

If switch to thinking with IPropertyHandler, it will roughly look like this

RepoDb.TypeMapper.PropertyHandlerFactory = new DefaultPropertyHandlerFactory();

public class DefaultPropertyHandlerFactory : IPropertyHandlerFactory
{
    public object CreateHandler(Type propType)
    {
        if (propType.IsEnum)
        {
            return new EnumStringPropertyHandler<TEnum>();
        }

        return null;
    }
}
public class EnumStringPropertyHandler<TEnum> : IPropertyHandler<string, TEnum?>
{
     public TEnum Get (string input, ClassProperty property) => ...;
     public string Set (TEnum input, ClassProperty property) => ...;
}

Then you can customize PropertyHandlerFactory and EnumIntPropertyHandler like follow

RepoDb.TypeMapper.PropertyHandlerFactory = new CustomPropertyHandlerFactory();

public class CustomPropertyHandlerFactory : DefaultPropertyHandlerFactory, IPropertyHandlerFactory
{
    object IPropertyHandlerFactory.CreateHandler(Type enumType)
    {
        if (..........)  // custome condition
        {
            return new EnumIntPropertyHandler<TEnum>();
        }

        return base.CreateHandler(enumType);
    }
}
public class EnumIntPropertyHandler<TEnum> : IPropertyHandler<int, TEnum?>
{
     public TEnum Get (int input, ClassProperty property) => ...;
     public int? Set (TEnum input, ClassProperty property) => ...;
}

Do you think it is similar to #646 ?
:grin:

[mikependon]

Gents, as this may requires a change to the compiler for the non-schema-based operations, it is important to take note that RepoDB is intelligent enough if it understands your schema, and to some operations (i.e.: Insert<T>, Merge<T>, etc), RepoDB understands your schema.

It is very unfortunate that RepoDB could not understand the target schema if you are to use the following operations (ExecuteNonQuery, ExecuteScalar and ExecuteReader), as the table and/or model is not defined somewhere.

In my case, as I tested and validated that, you do not need to do any action item at all on this area, up until you required it on the call on the non-schema-based operations mentioned above.

To elaborate further, the details below is a replication process for this scenario. There, I have a Person table that has 2 columns pertained for enum values CustomerType and CustomerStatus. I also have created a dedicated enumeration for each and simply call the schema-based and non-schema-based operations.

Table:

CREATE TABLE [dbo].[Person](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](32) NOT NULL,
    [Address] [nvarchar](max) NOT NULL,
    [CustomerType] [int] NULL,
    [CustomerStatus] [nvarchar](32) NULL,
    [CreatedDateUtc] [datetime2](7) NOT NULL,
 CONSTRAINT [PK_Person] PRIMARY KEY CLUSTERED 
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]
GO

Enumerations:

namespace EnumPropertyHandlerTest.Enumerations
{
    public enum CustomerStatus
    {
        Active,
        Inactive
    }
}

namespace EnumPropertyHandlerTest.Enumerations
{
    public enum CustomerType
    {
        Direct = 1,
        Indirect = 2,
        Partner = 3
    }
}

Models:

using EnumPropertyHandlerTest.Enumerations;
using System;

namespace EnumPropertyHandlerTest.Models
{
    public class Person
    {
        public long Id { get; set; }
        public string Name { get; set; }
        public string Address { get; set; }
        public CustomerType CustomerType { get; set; }
        public CustomerStatus CustomerStatus { get; set; }
        public DateTime CreatedDateUtc { get; set; }
    }
}

Main:

using EnumPropertyHandlerTest.Enumerations;
using EnumPropertyHandlerTest.Models;
using Microsoft.Data.SqlClient;
using RepoDb;
using System;

namespace EnumPropertyHandlerTest
{
    class Program
    {
        static void Main(string[] args)
        {
            Initialize();
            TestSchemaBased();
            TestNonSchemaBased();
        }

        static void Initialize()
        {
            SqlServerBootstrap.Initialize();
        }

        static void TestSchemaBased()
        {
            using (var connection = new SqlConnection("Server=.;Database=DemoRepoDB;Integrated Security=True;"))
            {
                connection.Truncate<Person>();

                var id = connection.Insert<Person, long>(new Person
                {
                    Name = "John Doe",
                    Address = "New York",
                    CustomerType = CustomerType.Direct,
                    CustomerStatus = CustomerStatus.Active,
                    CreatedDateUtc = DateTime.UtcNow
                });

                var people = connection.QueryAll<Person>();
            }
        }

        static void TestNonSchemaBased()
        {
            using (var connection = new SqlConnection("Server=.;Database=DemoRepoDB;Integrated Security=True;"))
            {
                connection.Truncate<Person>();

                var entity = new
                {
                    Name = "John Doe",
                    Address = "New York",
                    CustomerType = CustomerType.Direct,
                    CustomerStatus = CustomerStatus.Active,
                    CreatedDateUtc = DateTime.UtcNow
                };
                var id = connection.ExecuteScalar<long>("INSERT INTO Person (Name, Address, CustomerType, CustomerStatus) " +
                    "VALUES (@Name, @Address, @CustomerType, @CustomerStatus); SELECT CONVERT(BIGINT, SCOPE_IDENTITY());", entity);

                var people = connection.QueryAll<Person>();
            }
        }
    }
}

You will notice that the issue only happens when we called the ExecuteScalar non-schema based operation.

Microsoft.Data.SqlClient.SqlException: 'Conversion failed when converting the nvarchar value 'Direct' to data type int.'

Therefore, please help me understand if your use-case falls on that operations.

[mikependon]

And here, if we are to put the proper PropertyHandler to handle the case of the failing one, then it will be fixed and handled properly.

Note: It is a Type-Level mapping.

PropertyHandler:

using EnumPropertyHandlerTest.Enumerations;
using RepoDb;
using RepoDb.Interfaces;
using System;

namespace EnumPropertyHandlerTest.PropertyHandlers
{
    public class CustomerTypePropertyHandler : IPropertyHandler<int, CustomerType>
    {
        public CustomerType Get(int input, ClassProperty property)
        {
            var enumType = typeof(CustomerType);
            return (CustomerType)Enum.Parse(enumType, Enum.GetName(enumType, input));
        }

        public int Set(CustomerType input, ClassProperty property)
        {
            return Convert.ToInt32(input);
        }
    }
}

Initialize:

static void Initialize()
{
    SqlServerBootstrap.Initialize();

    FluentMapper
        .Type<CustomerType>()
        .DbType(System.Data.DbType.Int32, true);
}

Please let me know if this address your issues.

[mikependon]

Here is the actual project I simulated (.NET Core 3.1) - EnumPropertyHandlerTest.zip

[fredliex]

I know what you mean
In fact, most of my current situations do use model as the processing object, but it still cannot cover all the scenarios I encountered.
(I am currently trying to transfer a large system from a slightly modified dapper to RepoDb)

In addition. If these enums appear repeatedly in multiple models, these duplications will become frustrating.

I will try to see if I can do something, and if everything goes well, I will create an PR for your reference.

thank you very much.

[mikependon]

In addition. If these enums appear repeatedly in multiple models, these duplications will become frustrating.

It will not for as long you use the type level mapping like below, specially you are all using the model based implementations.

FluentMapper
        .Type<CustomerType>()
        .DbType(System.Data.DbType.Int32, true);

[kbilsted]

I don't think the original suggestion of setting int/nvarchar to handle enums is sufficiently.

The propertyhandler is a good uniform way to describe the mapping of each type in your system whether it be some nested structure or an enum.

When dealing with enums, recall that you can have flagged enums, that you may want to handle separately - I've been in that situation some years back where flags were joined to form new settings. The propertyhandler can encompass this requirements as well.

[mikependon]

@kbilsted - true, and thank you for your comments specifically on the bit-wised (flagged enums). However, we still need to have a default conversion of enum when it is being pushed into the DB (not pull/query), specially to those operations that is not entity-based (i.e: ExecuteScalar, ExecuteNonQuery and ExecuteReader), in which there is no way the library to understand which underlying table is being targeted.

Currently, RepoDB handles the enums very well if you call the entity-based operations (i.e: Insert<T>, Merge<T>, ExecuteQuery<T>, etc), but is converting it to TEXT/NVARCHAR data type in database if you are calling the non-entity based operations (mentioned above).

If we are to handle it defaultly to INT, it is as simple as commenting some TEXT/NVARCHAR conversion code from the compiler, but I am hesitant to do that due some scenarios other users already implemented.

To elaborate further, given with this class.

public enum CustomerType
{
    Direct,
    Indirect
};

public class Customer
{
    public long Id { get; set; }
    public string Name { get; set; }
    public CustomerType Type { get; set; }
}

The call below will succeed if the property CustomerType is TEXT/NVARCHAR in the DB.

var entity = new Customer { Name = "John Doe", Type = CustomerType.Direct };
connection.ExecuteScalar("INSERT INTO Customer (Name, CustomerType) VALUES (@Name, @Type)", entity);

But it will throw an exception if the CustomerType property is of type INT. Of course, unless otherwise you cast it like below or specifically add a mapping to that specific property to DbType.Int32k.

var entity = new Customer { Name = "John Doe", Type = (int)CustomerType.Direct };
connection.ExecuteScalar("INSERT INTO Customer (Name, CustomerType) VALUES (@Name, @Type)", entity);

Thoughts?

[mikependon]

@SpaceOgre - I really would like to capture you as a contributor to this, since you found the issue and made the recommendations. Would you be able to make the PR for the solutions here? It is very simple as it is already pre-tested by me on our DEV ENV.

As of writing this, I had introduce this line of code already. See below.

public static DbType EnumDefaultDatabaseType { get; set; } = DbType.String;

By default, it is DbType.String so it would not affect any existing functionalities and behaviors.

The actual fixes are very simple, you just need to set the following lines to utilize the newly introduced property from the Converter object.

• In the DbCommandExtension, simply change the following lines.
  
  
  
  • 268
  
  
  • 340
  
  
  • 469
  
  
  • 525
  
  
  • 594
  
  
  • 598
  
  
• In the compiler class PlainTypeToDbParameters, simply change the following lines.
  
  
  
  • 51
  
  

Looking forward for your PR, otherwise, please do let me know if you cannot do so I can push it myself or ask the other guys to contribute to it.

By the way, please do not challenge why it is placed as a static property and not an object-based approach like (object settings) as initially thought at #118. That is on top of our mind. For now, let us keep everything simple and easy to adopt.

[kbilsted]

this is not the better approach. How do you support all but a few enums are to be int and the rest string? E.g. if you are transitioning your dB..

My advice is to go for a code hook.

[SpaceOgre]

@kbilsted the TypeMapper and FluentMapper should still take priority, like it does today. This would just change what the default is.

[SpaceOgre]

@mikependon sure I can look into making a PR :)
Will hopefully have time tomorrow.

[mikependon]

@kbilsted - this is just an enhacement to an existing default DbType.String. Other than that, everything works as it is. I hope your concerns and/or whatever use-cases can be addressed by the customized Property Handlers, specifically the code hook?

EDIT: You can clearly see on the lines of code I shared above, the library is defaulting it to DbType.String, and even to the compiled one, which I guess must be good if we have something like this that could override that behavior.

Of course, thanks for the inputs and I hope I cleared the thoughts of everyone here.

[mikependon]

@SpaceOgre - what's up? as I am planning to make a new beta release, would be happy to include this one 😁

[SpaceOgre]

@mikependon Sorry, got swamped at work yesterday and did not manage to get to it. Will try and do it today :)

[SpaceOgre]

@mikependon I have done the code changes but I'm not sure if there are any existing test to update or if I should write new ones where should they be placed?

[mikependon]

TBH, I already had pre-tested that on our environment. The default is DbType.String and your changes should not affect that. However, I can also add more Integration Tests that targets the DbType.Int32 or the non-String database type.