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

Enums: error when dbValue doesn't exist in Enum-Values #1099

Closed
Garios opened this issue Oct 13, 2022 · 16 comments
Closed

Enums: error when dbValue doesn't exist in Enum-Values #1099

Garios opened this issue Oct 13, 2022 · 16 comments
Assignees
Labels
bug Something isn't working fixed The bug, issue, incident has been fixed.

Comments

@Garios
Copy link

Garios commented Oct 13, 2022

Bug Description

I think #956 has the same problem.

If I have values in table, which doesn't exist in the given Enum, then I get the following error. I don't use string-values. I use byte, short, int, long.

Exception Message:

System.ArgumentNullException: 'Value cannot be null. Arg_ParamName_Name'

Model:

    public enum TestEnum : int
    {
        None = 0,
        Type1 = 1
    }
    public class TestTable
    {
        public long ID { get; set; }
        public TestEnum TestType { get; set; }
    }

Compiler creates:

new TestTable() 
{
    ID = reader.GetInt64(0),
    TestEnum = Convert(Parse(TestEnum, GetName(TestEnum, Convert(reader.GetInt32(1), Object)), True), TestEnum)
}

Assume reader.GetInt32(1) returns "4". But "4" doesn't exist in TestEnum. Then GetName returns "null". Parse throws the error...

Expected Behaviour
return default(TestEnum)

Possible Solution

long dbValue = 4; //note, in this case the db-type differs from the corresponding enum-type int
var baseType = typeof(TestEnum).GetEnumUnderlyingType();

object convertedDbValue = dbValue;
if (baseType != dbValue.GetType()) //if dbValue-Type differs from underlying enum-type
	convertedDbValue = Convert.ChangeType(dbValue, baseType); //ex. long to int

object result;
if (Enum.IsDefined(typeof(TestEnum), convertedDbValue))
	result = Enum.ToObject(typeof(TestEnum), dbValue);
else
	result = default(TestEnum);
return result;

Solution translated to Compiler
Replace the following Method in Compiler.cs:

(I never used Expression before!)

internal static Expression ConvertExpressionToEnumExpressionForNonString(Expression expression, Type toEnumType)
{
	Type baseType = toEnumType.GetEnumUnderlyingType();
	if (baseType != expression.Type)
	{
		var convertMethod = StaticType.Convert.GetMethod("ChangeType", new[] { StaticType.Object, StaticType.Type });
		var convertParameters = new Expression[]
		{
			Expression.Convert(expression, StaticType.Object),
			Expression.Constant(baseType)
		};
		expression = Expression.Call(convertMethod, convertParameters);
	}
	else
		expression = Expression.Convert(expression, StaticType.Object);

	var isDefinedMethod = StaticType.Enum.GetMethod("IsDefined", new[] { StaticType.Type, StaticType.Object });
	var isDefinedParameters = new Expression[]
	{
			Expression.Constant(toEnumType),
			expression
	};            
	var isDefinedExpression = Expression.Call(isDefinedMethod, isDefinedParameters);

	var toObjectMethod = StaticType.Enum.GetMethod("ToObject", new[] { StaticType.Type, StaticType.Object });
	var toObjectParameters = new Expression[]
	{
			Expression.Constant(toEnumType),
			expression
	};
	var toObjectExpression = Expression.Call(toObjectMethod, toObjectParameters);

	var defaultExpression = Expression.Convert(Expression.Default(toEnumType), StaticType.Object);

	return Expression.Condition(isDefinedExpression, toObjectExpression, defaultExpression);
}

Library Version:
Current master.

@Garios Garios added the bug Something isn't working label Oct 13, 2022
@mikependon mikependon pinned this issue Oct 13, 2022
@Garios
Copy link
Author

Garios commented Oct 15, 2022

I have thought about it again. The following code should be the best. It corresponds to the C# language specification. More about this topic can be found here IsBad(Enum.IsDefined). There is also an interesting Enum-Discussion.

var result = (TestEnum)reader.GetInt32(1);

Solution translated to Compiler

internal static Expression ConvertExpressionToEnumExpressionForNonString(Expression expression, Type toEnumType)
{
        return Expression.Convert(expression, toEnumType);
}

An Enum-Option could be also conceivable

  • this solution: accept all (c# design)
  • the upper solution: use default, when not defined
  • the current solution: throw error, when not defined

@mikependon
Copy link
Owner

The following error occurs when reading a value of Enum from the DB which is not present to the target CLR Enum Type.

Int Type: System.ArgumentNullException: 'Value cannot be null. (Parameter 'value')'
String Type: System.ArgumentException: 'Requested value 'Other' was not found.'

@mikependon
Copy link
Owner

Relating: #814

@mikependon
Copy link
Owner

I would like to know how to best handle this thing. Should I silently handle and pass it? Or, throw an exception like what's happening today?

Or, should I introduce a new setting in the GlobalConfiguration.Options?

The fix is a bit simple, like simply casting it as per the 814. (see below)

// Enum
public enum Gender
{
    Male = 1,
    Female = 2
}

// Simply cast (only for Int type)
var male = (Gender)1;
var female = (Gender)2;
var other = (Gender)3;

But I guess, it would have a scenario impact to the user.

@Garios
Copy link
Author

Garios commented Oct 24, 2022

Using GlobalConfiguration.Options.EnumHandling

public enum EnumHandling
{
    ThrowError = 0, //Default
    UseDefault = 1,
    Cast = 2
}

internal static Expression ConvertExpressionToEnumExpressionForNonString(Expression expression, Type toEnumType)
{
    EnumHandling handler = EnumHandling.ThrowError; //GlobalConfiguration.Options.EnumHandling

    if (handler == EnumHandling.Cast)
        return Expression.Convert(expression, toEnumType);



    Type baseType = toEnumType.GetEnumUnderlyingType();
    if (baseType != expression.Type)
    {
        var convertMethod = StaticType.Convert.GetMethod("ChangeType", new[] { StaticType.Object, StaticType.Type });
        var convertParameters = new Expression[]
        {
            Expression.Convert(expression, StaticType.Object),
            Expression.Constant(baseType)
        };
        expression = Expression.Call(convertMethod, convertParameters);
    }
    else
        expression = Expression.Convert(expression, StaticType.Object);

    var isDefinedMethod = StaticType.Enum.GetMethod("IsDefined", new[] { StaticType.Type, StaticType.Object });
    var isDefinedParameters = new Expression[]
    {
            Expression.Constant(toEnumType),
            expression
    };
    var isDefinedExpression = Expression.Call(isDefinedMethod, isDefinedParameters);


    Expression notDefinedExpression;
    if (handler == EnumHandling.UseDefault)
        notDefinedExpression = Expression.Convert(Expression.Default(toEnumType), StaticType.Object);
    else
    {
        var stringConcat = StaticType.String.GetMethod("Concat", new[] { typeof(string), typeof(string) });
        var errorMessage = Expression.Call(stringConcat, Expression.Constant(toEnumType.Name + " doesn't define enum-value "), Expression.Call(expression, StaticType.Object.GetMethod("ToString")));

        var argumentExceptionConstructor = typeof(ArgumentException).GetConstructor(new[] { StaticType.String });
        var throwException = Expression.Throw(Expression.New(argumentExceptionConstructor, errorMessage), StaticType.Object);

        notDefinedExpression = throwException;
    }


    var toObjectMethod = StaticType.Enum.GetMethod("ToObject", new[] { StaticType.Type, StaticType.Object });
    var toObjectParameters = new Expression[]
    {
            Expression.Constant(toEnumType),
            expression
    };
    var toObjectExpression = Expression.Call(toObjectMethod, toObjectParameters);

    return Expression.Condition(isDefinedExpression, toObjectExpression, notDefinedExpression);
}

@mikependon
Copy link
Owner

Thanks for the inputs, we will consider this ;)

mikependon added a commit that referenced this issue Oct 24, 2022
#1099 Fixes to the Automatic Enum conversion for Non-Defined values.
@mikependon mikependon added the fixed The bug, issue, incident has been fixed. label Oct 24, 2022
@mikependon
Copy link
Owner

The fixes will be available on the next release > RepoDB v1.13.0-alpha2.

@mikependon mikependon unpinned this issue Oct 24, 2022
@mikependon
Copy link
Owner

Will you be able to test the latest alpha? (Alpha 3)

@Garios
Copy link
Author

Garios commented Oct 25, 2022

With RepoDB v1.13.0-alpha3 from nuget, I get the same error like before.

But, I currently use a "fork", for fixing the errors, on my side. If I copy the code from current Compiler.cs, it works on my side. It seems, that your changes aren't included in alpha3.

@mikependon
Copy link
Owner

Have you tried setting the ConversionType to Automatic? I forgot to mention, the Casting and Enum Default conversion will only happen if you have set it to 'Automatic'.

@Garios
Copy link
Author

Garios commented Oct 25, 2022

yes, I did.

@mikependon
Copy link
Owner

Below are the code changes made, the Integration Tests are not yet written though.

Enum as String:

if (GlobalConfiguration.Options.ConversionType == ConversionType.Automatic)

Enum as Int:
return Expression.Convert(expression, toEnumType);

We tested many times and it is working?

@mikependon
Copy link
Owner

Oh. Looks like then, we might have built the package from the master before the actual fixes/code has been merged then. Anyway, we can issue a new alpha, or maybe promote to a beta so we can start work on our documentations. We will notify you ASAP once the new package has been pushed.

@mikependon mikependon pinned this issue Oct 25, 2022
@mikependon
Copy link
Owner

Please use the version RepoDb v1.13.0-beta1.

@Garios
Copy link
Author

Garios commented Oct 26, 2022

Now it works, Thanks!!

@mikependon
Copy link
Owner

Thanks for confirming. We can now start with the updates on the documentation and change log blogging :)

@mikependon mikependon unpinned this issue Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed The bug, issue, incident has been fixed.
Projects
None yet
Development

No branches or pull requests

2 participants