CsvHelper icon indicating copy to clipboard operation
CsvHelper copied to clipboard

Converter not called in CsvHelper version 29 and 30

Open follesoe opened this issue 3 years ago • 3 comments

Describe the bug An undocumented breaking change, or bug, is introduced in CsvHelper v29.0.0. The converter registered in the ClassMap is no longer invoked, and the field is not mapped.

To Reproduce

Example single file Program.cs:

using System.Globalization;
using CsvHelper;
using CsvHelper.Configuration;

public class Row
{
    public int Value;		
}

public sealed class RowMapping : ClassMap<Row>
{
    public RowMapping()
    {			
        Map(t => t.Value)
            .Name("value")
            .Convert(r => NegateConverter(r.Row, "value"));			
    }

    private static int NegateConverter(IReaderRow row, string field)
    {
        Console.WriteLine("NegateConverter called");
        return Convert.ToInt32(row[field]) * -1;
    }
}	

public static class Program
{
    public static void Main()
    {
        string csvString = string.Empty;

        var rows = new List<Row> { new Row { Value = 10 }};

        // Serialize
        using (var writer = new StringWriter())
        using (var csv = new CsvWriter(writer, CultureInfo.InvariantCulture, true))
        {
            csv.Context.RegisterClassMap<RowMapping>();
            csv.WriteRecords(rows);
            csv.Flush();
            csvString = writer.ToString();
            Console.WriteLine(csvString);
        }

        // Deserialize
        using (var reader = new StringReader(csvString))
        using (var csv = new CsvReader(reader, CultureInfo.InvariantCulture, false))
        {
            csv.Context.RegisterClassMap<RowMapping>();
            csv.Read();
            var row = csv.GetRecord<Row>();            
        }
    }
}

A .csproj file using CsvHelper 28.0.1:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="CsvHelper" Version="28.0.1" />
  </ItemGroup>
  <ItemGroup>
    <Compile Include="..\Shared\Program.cs" Link="Program.cs" />  
  </ItemGroup>
</Project>

A .csproj file using CsvHelper 30.0.1:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="CsvHelper" Version="30.0.1" />
  </ItemGroup>
  <ItemGroup>
    <Compile Include="..\Shared\Program.cs" Link="Program.cs" />
  </ItemGroup>
</Project>

Expected behavior I expect the Convert method to be invoked when using CsvHelper 30.0.1.

Screenshots

Running the example code using CsvHelper v28:

follesoe@Jonas-MacBook-Pro-M1-16-inch Version28 % dotnet run
value
10

NegateConverter called

Running the example code using CsvHelper 31:

follesoe@Jonas-MacBook-Pro-M1-16-inch Version30 % dotnet run
value

Additional context As you can see from the output, the NegateConverter called method is invoked when compiling and running against CsvHelper 28, but no 30. I did check the change log to see if this is a breaking change that I should have been aware of, but as far as I can see, it is not. This bug bit us in production, as I did an update of the library, and didn't have unit tests covering this part of our code.

follesoe avatar Dec 19 '22 15:12 follesoe

Further testing seems to suggest the issue is related to different behavior when only assigning ConvertFromString<int> and not ConvertToString<Row>.

In version 28 it would simply pass the value through without conversion (or default conversion), on 30 it returns an empty string.

follesoe avatar Dec 19 '22 15:12 follesoe

The converter isn't being invoked because you're calling Read once, which is the header row.

There is another issue though. When you have the converter specified, it's not writing the value. The converter there is only for reading, so that shouldn't happen. I'll have to look into that.

JoshClose avatar Dec 20 '22 22:12 JoshClose

I'm having the same issue. Any updates?

dan296 avatar Jul 14 '23 15:07 dan296