JoshClose / CsvHelper

Library to help reading and writing CSV files
http://joshclose.github.io/CsvHelper/
Other
4.76k stars 1.07k forks source link

Bug introduced in 2.16.3 for child class headers #738

Closed atruskie closed 7 years ago

atruskie commented 7 years ago

Using this code:

        public class BaseClass
        {
            public string A { get; set; }
        }

        public class ChildClass : BaseClass
        {
            public string B { get; set; }
        }

        [TestMethod]
        public void TestAllHeadersAreOutput()
        {
            var instance = new ChildClass() { A = "Hello", B = "World" };
            var expected = @"A,B
Hello,World";

            BaseClass[] instances = { instance };

            using (var writer = new StringWriter())
            {
                var serializer = new CsvHelper.CsvWriter(writer);

                serializer.WriteRecords(instances);

                Assert.AreEqual(expected, writer.ToString());
            }
        }

Fails in v2.16.3. The output from writer.ToString() is:

Framework CsvHelper Version Output
net461 3.0.0-chi06 A
World,Hello
net461 2.16.3.0 A
World,Hello
net461 2.16.2.0 A
Hello

Personally, I'd expect the output to be:

A,B
Hello,World

But it's clear the output from 2.16.3 and up is wrong.

jamesbascle commented 7 years ago

I would assert that in this particular case a bug was introduced in that you have a handle on it as a base class, thus it should not automap any child class properties, and neither print their values or print their headers, and just automap base class properties.

The reason I think that is the correct behavior is because you could have any number of subclasses with various properties defined on them in that base class array. It would obviously be incorrect to have to iterate all the way through the ienumerable to find the subclasses and give them a particular column index for each of their properties before the file can be written.

Thus, I assert that the bug is that world is written at all.

On Aug 24, 2017 9:15 PM, "Anthony Truskinger" notifications@github.com wrote:

Using this code:

    public class BaseClass
    {
        public string A { get; set; }
    }

    public class ChildClass : BaseClass
    {
        public string B { get; set; }
    }

    [TestMethod]
    public void TestAllHeadersAreOutput()
    {
        var instance = new ChildClass() { A = "Hello", B = "World" };
        var expected = @"A,B

Hello,World";

        BaseClass[] instances = { instance };

        using (var writer = new StringWriter())
        {
            var serializer = new CsvHelper.CsvWriter(writer);

            serializer.WriteRecords(instances);

            Assert.AreEqual(expected, writer.ToString());
        }
    }

Fails in v2.16.3. The output from writer.ToString() is: Framework CsvHelper Version Output net461 3.0.0-chi06 A World,Hello net461 2.16.3.0 A World,Hello net461 2.16.2.0 A Hello

Personally, I'd expect the output to be:

A,B Hello,World

But it's clear the output from 2.16.3 and up is wrong.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JoshClose/CsvHelper/issues/738, or mute the thread https://github.com/notifications/unsubscribe-auth/AD_ohcQs7kZJhHY65n398IkVZBoS6Gi3ks5sbiA9gaJpZM4PCHWA .

JoshClose commented 7 years ago

I think this is fixed in the latest source. This unit test passes.

[TestClass]
public class BaseClassTests
{
    [TestMethod]
    public void EnsureChildNotWrittenWhenListIsParent()
    {
        var record = new Child
        {
            ChildProp = "child",
            ParentProp = "parent"
        };
        Parent[] records = { record };

        using( var stream = new MemoryStream() )
        using( var writer = new StreamWriter( stream ) )
        using( var reader = new StreamReader( stream ) )
        using( var csv = new CsvWriter( writer ) )
        {
            csv.WriteRecords( records );
            writer.Flush();
            stream.Position = 0;

            var expected = new StringBuilder();
            expected.AppendLine( "ParentProp" );
            expected.AppendLine( "parent" );

            Assert.AreEqual( expected.ToString(), reader.ReadToEnd() );
        }
    }

    private class Parent
    {
        public string ParentProp { get; set; }
    }

    private class Child : Parent
    {
        public string ChildProp { get; set; }
    }
}
atruskie commented 7 years ago

@jamesbascle yeah that's a fair stance. The behaviour in 2.16.2 matches what you say and is a reasonable default behaviour.

@JoshClose - thanks for checking. I tried checking the latest source but couldn't get the solution to build.

JoshClose commented 7 years ago

3.0.0-RC02 is on nuget and you can try that