GroBuf icon indicating copy to clipboard operation
GroBuf copied to clipboard

Bug while serialize class with overridable get-only property using AllPropertiesExtractor

Open borovskyav opened this issue 7 years ago • 5 comments

I have child class with override get only array of enum's property. While i serialize this, grobuf throws exception like "Hash code collision: members 'BaseClass.States' and 'ChildClass.States' have the same hash code = 15189835799450633671". Proof: https://github.com/borovskyav/GroBuf/commit/eff23cae4997d22ced62ca428e441046fd94a4d2

borovskyav avatar Oct 10 '17 15:10 borovskyav

It also does not work with public fields and writable fields. Add tests on it https://github.com/borovskyav/GroBuf/commit/c62f2dfa63c9dbe6e75f6f94c54b195925029c0d

borovskyav avatar Oct 11 '17 07:10 borovskyav

Expiriencing same problems on:

public abstract class Base
{
    protected virtual int Y { get; }
}

public class Derived : Base
{
    protected override int Y => _y;
    private static int _y = 1;
}

However, inlining _y solves problem:

public abstract class Base
{
    protected virtual int Y { get; }
}

public class Derived : Base
{
    protected override int Y => 1;
}

mr146 avatar May 01 '18 10:05 mr146

@mr146 The following tests are passing on the latest v1.3.2 release:

    [TestFixture]
    public class TestSerializeOnlyOverridableFieldsX
    {
        [Test]
        public void AllPropertiesExtractor()
        {
            var serializer = new Serializer(new AllPropertiesExtractor(), null, GroBufOptions.MergeOnRead);
            var expected = new Derived();
            Assert.DoesNotThrow(() => serializer.Serialize(expected));
        }

        [Test]
        public void PropertiesExtractor()
        {
            var serializer = new Serializer(new PropertiesExtractor(), null, GroBufOptions.MergeOnRead);
            var expected = new Derived();
            Assert.DoesNotThrow(() => serializer.Serialize(expected));
        }

        public abstract class Base
        {
            protected virtual int Y { get; }
        }

        public class Derived : Base
        {
            protected override int Y => _y;
            private static int _y = 1;
        }
    }

Please provide a failing test case if the problem persists.

AndrewKostousov avatar Jun 02 '18 13:06 AndrewKostousov

Nonetheless the test case VirtualProperty is failing while test case AbstractProperty is passing:

    public class TestVirtualProps
    {
        [Test]
        public void AbstractProperty()
        {
            var bytes = serializer.Serialize(new DerivedWithAbstractProp(42));
            Assert.That(serializer.Deserialize<DerivedWithAbstractProp>(bytes).Prop, Is.EqualTo(42));
        }

        [Test]
        public void VirtualProperty()
        {
            var bytes = serializer.Serialize(new DerivedWithVirtualProp(42));
            Assert.That(serializer.Deserialize<DerivedWithVirtualProp>(bytes).Prop, Is.EqualTo(42));
        }

        private readonly Serializer serializer = new Serializer(new AllPropertiesExtractor(), null, GroBufOptions.MergeOnRead);

        private abstract class BaseWithVirtualProp
        {
            public virtual int Prop { get; }
        }

        private class DerivedWithVirtualProp : BaseWithVirtualProp
        {
            public DerivedWithVirtualProp(int prop)
            {
                Prop = prop;
            }

            public override int Prop { get; }
        }

        private abstract class BaseWithAbstractProp
        {
            public abstract int Prop { get; }
        }

        private class DerivedWithAbstractProp : BaseWithAbstractProp
        {
            public DerivedWithAbstractProp(int prop)
            {
                Prop = prop;
            }

            public override int Prop { get; }
        }
    }

The problem is in AllPropertiesExtractor which collects "writable" properties from the class and all its ancestors in a rather dumb way. It completely ignores relations between members of types in the class hierarchy considering every type in the hierarchy separately. Hence the collision between properties DerivedWithVirtualProp.Prop and BaseWithVirtualProp.Prop.

AndrewKostousov avatar Jun 02 '18 18:06 AndrewKostousov

I believe this is still a challenge. I added a new issue, but I believe it is related to this one.

agatlin avatar Mar 10 '19 15:03 agatlin