[GitHub] [ignite-3] ashapkin commented on a diff in pull request #1482: IGNITE-18435 .NET: Add support for Enum mapping

[GitBox <gi...@apache.org>]

ashapkin commented on code in PR #1482:
URL: https://github.com/apache/ignite-3/pull/1482#discussion_r1058371496


##########
modules/platforms/dotnet/Apache.Ignite/Internal/Table/Serialization/ObjectSerializerHandler.cs:
##########
@@ -500,11 +500,13 @@ private static void EmitFieldRead(FieldInfo? fieldInfo, ILGenerator il, Column c
         private static void ValidateFieldType(FieldInfo fieldInfo, Column column)
         {
             var columnType = column.Type.ToType();
+
             var fieldType = Nullable.GetUnderlyingType(fieldInfo.FieldType) ?? fieldInfo.FieldType;
+            fieldType = fieldType.IsEnum ? Enum.GetUnderlyingType(fieldType) : fieldType;
 
             if (fieldType != columnType)
             {
-                var message = $"Can't map field '{fieldInfo.DeclaringType?.Name}.{fieldInfo.Name}' of type '{fieldType}' " +
+                var message = $"Can't map field '{fieldInfo.DeclaringType?.Name}.{fieldInfo.Name}' of type '{fieldInfo.FieldType}' " +

Review Comment:
   Just an idea - provide a list of supported types or a link to a future documentation link? 



##########
modules/platforms/dotnet/Apache.Ignite.Tests/Table/RecordViewPocoTests.cs:
##########
@@ -721,6 +721,34 @@ public async Task TestAllColumnsPocoNullable()
             Assert.AreEqual(poco.DateTime, res.DateTime);
         }
 
+        [Test]
+        public async Task TestEnumColumns()
+        {
+            // Normal values.
+            await Test(new PocoEnums.PocoIntEnum(1, PocoEnums.IntEnum.Foo));
+            await Test(new PocoEnums.PocoByteEnum(1, PocoEnums.ByteEnum.Foo));
+            await Test(new PocoEnums.PocoShortEnum(1, PocoEnums.ShortEnum.Foo));
+            await Test(new PocoEnums.PocoLongEnum(1, PocoEnums.LongEnum.Foo));
+
+            // Values that are not represented in the enum (it is just a number underneath).

Review Comment:
   We can also test the default value, like
   await Test(new PocoEnums.PocoIntEnum(1, default(PocoEnums.IntEnum)));



##########
modules/platforms/dotnet/Apache.Ignite/Internal/Table/Serialization/BinaryTupleMethods.cs:
##########
@@ -176,34 +176,36 @@ internal static class BinaryTupleMethods
         /// <summary>
         /// Gets the write method.
         /// </summary>
-        /// <param name="valueType">Type of the value to write.</param>
+        /// <param name="type">Type of the value to write.</param>
         /// <returns>Write method for the specified value type.</returns>
-        public static MethodInfo GetWriteMethod(Type valueType) =>
-            WriteMethods.TryGetValue(valueType, out var method) ? method : throw GetUnsupportedTypeException(valueType);
+        public static MethodInfo GetWriteMethod(Type type) =>
+            WriteMethods.TryGetValue(Unwrap(type), out var method) ? method : throw GetUnsupportedTypeException(type);
 
         /// <summary>
         /// Gets the write method.
         /// </summary>
-        /// <param name="valueType">Type of the value to write.</param>
+        /// <param name="type">Type of the value to write.</param>
         /// <returns>Write method for the specified value type.</returns>
-        public static MethodInfo? GetWriteMethodOrNull(Type valueType) => WriteMethods.GetValueOrDefault(valueType);
+        public static MethodInfo? GetWriteMethodOrNull(Type type) => WriteMethods.GetValueOrDefault(Unwrap(type));
 
         /// <summary>
         /// Gets the read method.
         /// </summary>
-        /// <param name="valueType">Type of the value to read.</param>
+        /// <param name="type">Type of the value to read.</param>
         /// <returns>Read method for the specified value type.</returns>
-        public static MethodInfo GetReadMethod(Type valueType) =>
-            ReadMethods.TryGetValue(valueType, out var method) ? method : throw GetUnsupportedTypeException(valueType);
+        public static MethodInfo GetReadMethod(Type type) =>
+            ReadMethods.TryGetValue(Unwrap(type), out var method) ? method : throw GetUnsupportedTypeException(type);
 
         /// <summary>
         /// Gets the read method.
         /// </summary>
-        /// <param name="valueType">Type of the value to read.</param>
+        /// <param name="type">Type of the value to read.</param>
         /// <returns>Read method for the specified value type.</returns>
-        public static MethodInfo? GetReadMethodOrNull(Type valueType) => ReadMethods.GetValueOrDefault(valueType);
+        public static MethodInfo? GetReadMethodOrNull(Type type) => ReadMethods.GetValueOrDefault(Unwrap(type));
 
         private static IgniteClientException GetUnsupportedTypeException(Type valueType) =>
             new(ErrorGroups.Client.Configuration, "Unsupported type: " + valueType);
+
+        private static Type Unwrap(Type type) => type.IsEnum ? Enum.GetUnderlyingType(type) : type;

Review Comment:
   Make it internal and use in [ObjectSerializerHandler.cs]?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org