Skip to content

Option for avoiding mapping on the Parent #26

Closed

Description

@ravensorb

Love your implementation. I was curious any thoughts to adding support for doing the mapping on the child instance instead of the parent? This would provide better support for a more flexible IoC pattern -

  • where the implementer of the interface controls the mapping rather than the author of the interface.

Example: The interface is defined in one assembly by a framework author

public interface IAnimal
{
    string Breed {get;set;}
}

and then the concrete implementation would be in another file (or assembly)

[JsonSubType(typeof(IAnimal), PropertyName="Breed", PropertyValue="Dog")]
public class Dog : IAnimal
{
   public string Breed {get;set;} = "Dog";
   public bool HasChip {get;set;}
}

[JsonSubType(typeof(IAnimal), PropertyName="Breed", PropertyValue="Cat")]
public class Cat : IAnimal
{
   public string Breed {get;set;} = "Cat";
   public bool HasClaws {get;set;}
}

this could be implemented using the DependencyInjection or by using Reflection for all known types loaded (supporting the DI pattern would be more ideal I think)

Thoughts?

Activity

  1. manuc66 commented on Jan 27, 2018

    @manuc66
    Owner

    Hi, I agree that it might be better to do the mapping on the child instead of the parent.

    In branch feature/base-without-useless-property I have support for the case you're exposing (having no possibility to add attributes on the parent)

    JsonConvert.DefaultSettings = () => new JsonSerializerSettings();
    var settings = JsonConvert.DefaultSettings();
    settings.Converters.Add(JsonSubtypesConverterBuilder
      .Of(typeof(IAnimal), "Breed")
      .RegisterSubtype(typeof(Dog), "Dog")
      .RegisterSubtype(typeof(Cat), "Cat")
      .Build());
    
    IAnimal animal = new Dog();
    
    var json = JsonConvert.SerializeObject(animal, settings);
    JsonConvert.DeserializeObject<IExpression2>(json, settings);

    But it have situations where I did not managed yet to fix some edge cases, so it's not released yet.

  2. manuc66 commented on Jan 27, 2018

    @manuc66
    Owner

    If JsonSubTypes uses TypeDescriptor.GetAttributes instead of Type.GetCustomAttributes something like this could have been implemented easily:

    [JsonConverter(typeof(JsonSubtypes), "Breed")]
    public interface IAnimal
    {
        string Breed {get;set;}
    }

    The concrete type whitout attributes:

    public class Dog : IAnimal
    {
       public string Breed {get;set;} = "Dog";
       public bool HasChip {get;set;}
    }
    
    public class Cat : IAnimal
    {
       public string Breed {get;set;} = "Cat";
       public bool HasClaws {get;set;}
    }
    // the mapping setup
    TypeDescriptor.AddAttributes(typeof(IAnimal),
      new JsonSubtypes.KnownSubTypeAttribute(typeof(Cat), "cat"),
      new JsonSubtypes.KnownSubTypeAttribute(typeof(Dog), "dog"));
    
    var json = "{\"catLives\":6,\"@type\":\"cat\",\"age\":11}";
    
    var result = JsonConvert.DeserializeObject<Animal>(json);
    
    Assert.AreEqual(typeof(Cat), result.GetType());
    Assert.AreEqual(11, result.Age);
    Assert.AreEqual(6, (result as Cat)?.Lives);

    But TypeDescriptor.GetAttribute(...) is not available for at least netstandard1.3 and netstandard1.4

  3. ravensorb commented on Jan 27, 2018

    @ravensorb
    Author

    They both are great ideas. How close is the 1st option? If is good enough to test for the standard use cases (maybe just part it as experimental and release it?)

  4. changed the title [-]Option for attribute on child class instead of Parent?[/-] [+]Option for avoiding mapping on the Parent[/+] on Jan 28, 2018
  5. manuc66 commented on Jan 28, 2018

    @manuc66
    Owner

    The 1st option is now released in v1.3.0 with #28

  6. ravensorb commented on Mar 8, 2018

    @ravensorb
    Author

    I think I found an issue with #28 - if the interface and the concrete class is in different assemblies there is an issue in creating the concrete instance. I think the issue is when the interface and class are in different name space. It looks like their is an assumption in the code that the namespace for the interface is used to build the namespace for the class.

  7. manuc66 commented on Mar 8, 2018

    @manuc66
    Owner

    @ravensorb I tried to reproduce the issue you described but did not managed (see: https://github.com/manuc66/JsonSubTypes/tree/bugfix/try-to-reproduce-bug). Can you provide some sample code and open an new issue with it?

    Thanks

  8. ravensorb commented on Mar 8, 2018

    @ravensorb
    Author

    So I tracked it down -- it looks like the code assumes that everything is in a single assembly (interface and classes). For my scenario I have the interface in one assembly and the concrete classes spread out over several other assemblies.

    To repro create 3 assemblies

    • MainApp
    • Interface
    • Implementation

    This will show the issue. Now the hard part is, I am not sure what the solution might be as you are supporting a lot of the legacy frameworks via PCL (I think) and up until the latest version AppDomain is not usable which makes getting a list of all loaded assemblies a bit tricky from what I can remember.

  9. manuc66 commented on Mar 8, 2018

    @manuc66
    Owner

    When dynamic registration is used (which has been introduced with #28) the sub type should be is taken from the registration and not from the parent type assembly.

    With #28 the method GetSubTypeMapping is overrided and then should provides some values so it did not fall in the GetTypeByName (which build the subtype from the assemble of the parent, but not in this case).

  10. ravensorb commented on Mar 8, 2018

    @ravensorb
    Author

    issue with that is, it assumes that everything is loaded immediately and that is not always the case when you have things handled via DI

  11. added this to the 1.3.0 milestone on Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions