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

Version field doesn't work with inheritance #17

Open
gregmeess opened this issue Sep 10, 2020 · 2 comments
Open

Version field doesn't work with inheritance #17

gregmeess opened this issue Sep 10, 2020 · 2 comments

Comments

@gregmeess
Copy link

gregmeess commented Sep 10, 2020

I'm trying to support migrations for a collection of classes that inherit from each other.

If a class with migrations inherits from another class with migrations, currently they both try and use the same version number. Unless I am missing something, it doesn't seem possible to have migrations for both a parent and child class.

Thinking about solutions, it seems like adding custom version name would allow a workaround where each class can specify a different version number.

The code would look as follows:

[Migratable("", "VersionClassA")]
class ClassA
{
    public int VersionClassA = 6;
    public double PropertyA { get; set; }
}

[Migratable("", "VersionClassB")]
class ClassB : ClassA
{
    public int VersionClassB = 3;
    public int PropertyB { get; set; }
}

[Migratable("", "VersionClassC")]
class ClassC : ClassB
{
    public int VersionClassC = 7;
    public string PropertyC { get; set; }
}

Corresponding JSON would have all version numbers:

{
  "VersionClassA": 4,
  "VersionClassB": 1,
  "VersionClassC": 5,
  "PropertyA": 0.555,
  "PropertyB": 100,
  "PropertyC": "foo"
}
@gregmeess
Copy link
Author

Another potential workaround that doesn't require changes: override the version number in the derived class, but call the appropriate migration methods from the parent class in the derived class migration method.

Example:

    [Migratable("")]
    public class ParentClass
    {
        public virtual int Version { get; } = 2;

        protected static JObject Migrate_1(JObject data, JsonSerializer serializer)
        {
            // do migration
        }

        protected static JObject Migrate_2(JObject data, JsonSerializer serializer)
        {
            // do migration
        }
    }

    [Migratable("")]
    public class ChildClass : ParentClass
    {
        public virtual int Version { get; } = 3;

        protected static JObject Migrate_1(JObject data, JsonSerializer serializer)
        {
            data = ParentClass.Migrate_1(data, serializer);
            // do child class migration
        }

        protected static JObject Migrate_2(JObject data, JsonSerializer serializer)
        {
            // do child class migration that doesn't include up-revving parent class
        }

        protected static JObject Migrate_3(JObject data, JsonSerializer serializer)
        {
            data = ParentClass.Migrate_2(data, serializer);
            // do child class migration
        }
    }

If I'm thinking through this correctly:

  • Every time a parent class version increments, any derived classes also need to increment so that they can call the parent class migration.
  • Child classes can increment version without the parent class being affected, so version numbers won't necessarily be the same between parent and child class.
  • The onus is on the developer to manage version numbers in an inheritance hierarchy and make sure the appropriate migrations are getting called in derived classes.

@vangogih
Copy link

@gregmeess hey mate,

I rewrote this plugin and made it from 5 to 10 times faster:
Link to plugin: https://github.com/vangogih/FastMigrations.Json.Net

According to your recommendations I wrote test cases to be support inheritance without pain)
Link to test case

Also you can check ParentMock and ChildV10Mock to check the structure and idea how we should add inheritance support while working with plugin.

It has been a while since your comment but I hope you will find the time to check it ;)

Don't hesitate to create an issue or PR!
Looking forward to getting feedback from you!

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

No branches or pull requests

2 participants