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

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

Open
borovskyav opened this issue Oct 10, 2017 · 5 comments

Comments

@borovskyav
Copy link

borovskyav commented Oct 10, 2017

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: borovskyav@eff23ca

@borovskyav
Copy link
Author

borovskyav commented Oct 11, 2017

It also does not work with public fields and writable fields. Add tests on it borovskyav@c62f2df

@mr146
Copy link

mr146 commented May 1, 2018

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;
}

@AndrewKostousov
Copy link
Contributor

@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
Copy link
Contributor

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 added a commit that referenced this issue Jun 2, 2018
@agatlin
Copy link

agatlin commented Mar 10, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants