Fix issue #755 - Regression in interfaces mapping introduced by #649

[lofcz]

Fixes #755, @andrerav could you please take a look and potentially release a new pre-release version? This regression could've affected many customers (it did us at least) - but the pattern is common.

@DocSvartz CC, I want to verify this doesn't break your code (tests are passing).

[andrerav]

@DocSvartz Please take a look and let me know if this looks okay to you. I can make a speedy release this afternoon :)

[DocSvartz]

@lofcz
Your solution is suitable as a temporary one.
But there are still very strange things happening there :)

Please Do not Close Issue.

[lofcz]

@DocSvartz could you elaborate? I find it strange that if we add any props to the interface:

[TestMethod]
public void MappingToInterface_VerifyReadonlyPropsInterfaceAnyPropRule()
{
    SampleInterfaceClsWithProp source = new SampleInterfaceClsWithProp
    {
        ActivityData = new SampleActivityDataWithProp
        {
            Data = new SampleActivityParsedData
            {
                Steps = new List<string> { "A", "B", "C" }
            }
        }
    };

    SampleInterfaceClsWithProp target = source.Adapt<SampleInterfaceClsWithProp>();
    target.ShouldNotBeNull();
    target.ShouldSatisfyAllConditions(
        () => target.ActivityData.ShouldBe(source.ActivityData)
    );
}

public interface IActivityDataWithProp
{
    int Test { get; set; }
}

public class SampleInterfaceClsWithProp
{
    public IActivityDataWithProp? ActivityData { get; set; }

    public SampleInterfaceClsWithProp()
    {

    }

    public SampleInterfaceClsWithProp(IActivityDataWithProp data)
    {
        SetActivityData(data);
    }

    public void SetActivityData(IActivityDataWithProp data)
    {
        ActivityData = data;
    }
}

public class SampleActivityDataWithProp : IActivityDataWithProp
{
    public int Test { get; set; } = 42;
    public SampleActivityParsedData Data { get; set; }
}

public class SampleActivityParsedData
{
    public List<string> Steps { get; set; } = new List<string>();
}

When mapping with default config we run into:

// BaseAdapter.cs:
//if mapping to interface, create dynamic type implementing it
else if (arg.DestinationType.GetTypeInfo().IsInterface)
{
...

And we synthesize a new type implementing just Test.

[DocSvartz]

As far as I understand, this is done in order to create an instance class that can be filled with values ​​from the source.
because it is impossible to create an interface instance

[lofcz]

That's right but wouldn't it be better to do something like https://github.com/lofcz/DeepCloner/blob/9abf692a58da2e76b4dac6a266f45fbd573dc621/DeepCloner.Core/Helpers/DeepClonerExprGenerator.cs instead? The current behavior is unintuitive and I can't think of a scenario where one would intentionally want to do this. Synthesizing a new type also means methods are lost.

[DocSvartz]

If you are talking about the Update scenario:

var t = sourse.Adapt(destination)

Yes, This should not happen.

And in the Create scenario:

var s = sourse.Adapt<IDestination>()

You can't know which implementation will be chosen anyway

[lofcz]

Yes, I mean "updating" specifically (cloning an existing object into another existing object).

[DocSvartz]

But "updating" happens exactly as you want.

[TestMethod]
public void MappingToInteraceWithIgnorePrivateSetProperty()
{
    TypeAdapterConfig<InterfaceSource723, InterfaceDestination723>
        .NewConfig()
        .TwoWays()
        .Ignore(dest => dest.Ignore);

    InterfaceSource723 dataSourse = new Data723() { Inter = "IterDataSourse", Ignore = "IgnoreDataSourse" };
    InterfaceDestination723 dataDestination = new Data723() { Inter = "IterDataDestination", Ignore = "IgnoreDataDestination" };

    var idestination = dataDestination.Adapt<InterfaceDestination723>();
    var isourse = dataDestination.Adapt<InterfaceSource723>();

    var result = dataSourse.Adapt(dataDestination); result this is type = Data723 is not   new created Type

    result.Ignore.ShouldBe("IgnoreDataDestination"); 
    result.Inter.ShouldBe("IterDataSourse");
}

I think, you want the "Creating" scenario to copy the Implementing "interface Source" Type to the Destination?

But it seems that this will only work if they match. As in your example.

[DocSvartz]

That's what I mean

IActivityData  data = new MyData() 

 var result =   data.Adapt<IActivityData>() where result is type MyData 

Hm, There is another option though.
When the SourceType also implements the Destination interface.
This should work too.

[DocSvartz]

@lofcz @andrerav .
If my proposal solves (from i write in review) the problem as well as the original one.
I think it's better to use it. Until we figure it out.

In any case, I agree with this pull request.

[DocSvartz]

@lofcz

I even managed to make this option work. ))
Did you want something like this?

  [TestMethod]
  public void MappingToInterface_VerifyReadonlyPropsInterfaceRule()
  {


      SampleInterfaceCls source = new SampleInterfaceCls
      {
          ActivityData = new SampleActivityData
          {
              Data = new SampleActivityParsedData
              {
                  Steps = new List<string> { "A", "B", "C" }
              },
              Temp = "Temp data"

          }

      };
      SampleInterfaceCls source2 = new SampleInterfaceCls
      {
          ActivityData = new SampleActivityData
          {
              Data = new SampleActivityParsedData
              {
                  Steps = new List<string> { "X", "Y", "Z" }
              },
              Temp = "Update Temp data"

          }

      };

      var target = source.Adapt<SampleInterfaceClsParent>();
      var target2 = source2.Adapt<SampleInterfaceCls>();
      var update = target.Adapt(target2);

      target.ShouldNotBeNull();
      target.ShouldSatisfyAllConditions(
          () => target.ActivityData.ShouldBe(source.ActivityData)
      );

      update.ActivityData.ShouldBe(target.ActivityData);

      var resultdata = ((IActivityData)update.ActivityData);

      resultdata.Temp.ShouldBe("Temp data");

      ((SampleActivityData)update.ActivityData).Data.Steps[0].ShouldBe("A");
      ((SampleActivityData)update.ActivityData).Data.Steps[1].ShouldBe("B");
      ((SampleActivityData)update.ActivityData).Data.Steps[2].ShouldBe("C");

  }

    public interface IActivityData : IActivityDataParent
    {
        public string Temp {  get; set; }
    }

    public interface IActivityDataParent
    {

    }

    public class SampleInterfaceCls
    {
        [Newtonsoft.Json.JsonIgnore]
        public IActivityDataParent? ActivityData { get; set; }

        public SampleInterfaceCls()
        {

        }

        public SampleInterfaceCls(IActivityDataParent data)
        {
            SetActivityData(data);
        }

        public void SetActivityData(IActivityDataParent data)
        {
            ActivityData = data;
        }
    }

    public class SampleInterfaceClsParent
    {
        [Newtonsoft.Json.JsonIgnore]
        public IActivityData? ActivityData { get; set; }

        public SampleInterfaceClsParent()
        {

        }

        public SampleInterfaceClsParent(IActivityData data)
        {
            SetActivityData(data);
        }

        public void SetActivityData(IActivityData data)
        {
            ActivityData = data;
        }
    }

[lofcz]

@lofcz

I even managed to make this option work. )) Did you want something like this?

  [TestMethod]
  public void MappingToInterface_VerifyReadonlyPropsInterfaceRule()
  {


      SampleInterfaceCls source = new SampleInterfaceCls
      {
          ActivityData = new SampleActivityData
          {
              Data = new SampleActivityParsedData
              {
                  Steps = new List<string> { "A", "B", "C" }
              },
              Temp = "Temp data"

          }

      };
      SampleInterfaceCls source2 = new SampleInterfaceCls
      {
          ActivityData = new SampleActivityData
          {
              Data = new SampleActivityParsedData
              {
                  Steps = new List<string> { "X", "Y", "Z" }
              },
              Temp = "Update Temp data"

          }

      };

      var target = source.Adapt<SampleInterfaceClsParent>();
      var target2 = source2.Adapt<SampleInterfaceCls>();
      var update = target.Adapt(target2);

      target.ShouldNotBeNull();
      target.ShouldSatisfyAllConditions(
          () => target.ActivityData.ShouldBe(source.ActivityData)
      );

      update.ActivityData.ShouldBe(target.ActivityData);

      var resultdata = ((IActivityData)update.ActivityData);

      resultdata.Temp.ShouldBe("Temp data");

      ((SampleActivityData)update.ActivityData).Data.Steps[0].ShouldBe("A");
      ((SampleActivityData)update.ActivityData).Data.Steps[1].ShouldBe("B");
      ((SampleActivityData)update.ActivityData).Data.Steps[2].ShouldBe("C");

  }

    public interface IActivityData : IActivityDataParent
    {
        public string Temp {  get; set; }
    }

    public interface IActivityDataParent
    {

    }

    public class SampleInterfaceCls
    {
        [Newtonsoft.Json.JsonIgnore]
        public IActivityDataParent? ActivityData { get; set; }

        public SampleInterfaceCls()
        {

        }

        public SampleInterfaceCls(IActivityDataParent data)
        {
            SetActivityData(data);
        }

        public void SetActivityData(IActivityDataParent data)
        {
            ActivityData = data;
        }
    }

    public class SampleInterfaceClsParent
    {
        [Newtonsoft.Json.JsonIgnore]
        public IActivityData? ActivityData { get; set; }

        public SampleInterfaceClsParent()
        {

        }

        public SampleInterfaceClsParent(IActivityData data)
        {
            SetActivityData(data);
        }

        public void SetActivityData(IActivityData data)
        {
            ActivityData = data;
        }
    }

Looks great to me.

[DocSvartz]

@lofcz Thanks!

@andrerav This can be applied and release
What I came up with may require additional discussion :)

[andrerav]

Thank you @lofcz and @DocSvartz!