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

Mapster modifies underlying collection which is being iterated over, when TypeAdapterConfig.GlobalSettings.Compile() is called. #68

Closed
centur opened this issue Jul 4, 2016 · 2 comments

Comments

@centur
Copy link
Contributor

centur commented Jul 4, 2016

Hi. Seems that there is an issue with Compile() call.

Mapster is modifying collection when it's being iterated.

The problem happens when there is top level mapping defined between objects, but nested property ( usually a collection of some type) is not defined explicitly).

For the code below:
If I have 3 mappings defined:

MainSrc -> MainDest
List<SrcItem> ->  List<DestItem>
SrcItem -> DestItem

In this case Compile works perfectly

If List<SrcItem> -> List<DestItem> is commented out - it throws vague InvalidOperationException: Collection was modified; enumeration operation may not execute.

This happens because TypeAdapterConfig.GetSettings(x) is not safe - it modifies collection this.RuleMap that is being iterated in TypeAdapterConfig.Compile() call.

// public class TypeAdapterConfig
//...

 private TypeAdapterSettings GetSettings(TypeTuple key)
    {
      TypeAdapterRule typeAdapterRule;
      if (!this.RuleMap.TryGetValue(key, out typeAdapterRule))
      {
        lock (this.RuleMap)
        {
          if (!this.RuleMap.TryGetValue(key, out typeAdapterRule))
          {
            typeAdapterRule = key.Source == typeof (void) ? TypeAdapterConfig.CreateDestinationTypeRule(key) : this.CreateTypeTupleRule(key);
            this.Rules.Add(typeAdapterRule); // <----------- modifies  collection on Read             this.RuleMap.Add(key, typeAdapterRule); // <----------- modifies collection on Read which is being iterated in Compile()

          }
        }
      }
      return typeAdapterRule.Settings;
    }

Here is bug repro example (works well in LinqPad with Mapster and AutoFixture nugets but can be easily converted to a console app):

void Main()
{

    WebDTOMapSetup.Verify();

    var fixt = new Fixture(); 
    var input = fixt.Create<MainSrc>();

    var output = input.Adapt<MainSrc,MainDest>();
    output.Dump();
}

public class WebDTOMapSetup
{
    static WebDTOMapSetup()
    {
        configme();
    }
    private static void configme()
    {

        TypeAdapterConfig<SrcItem, DestItem>.ForType();

        // BUG TRIGGER IS HERE --------> Comment this line to see mapster bug
        TypeAdapterConfig<List<SrcItem>, List<DestItem>>.ForType();

        //Validate globally
        TypeAdapterConfig<MainSrc, MainDest>.ForType()
        .Map(d=>d.DestItems, s=>s.SrcItems)
        ;

        TypeAdapterConfig.GlobalSettings.Compile();
    }

    public static void Verify()
    {
        TypeAdapterConfig.GlobalSettings.Compile();
    }
}

class MainSrc
{
    public int SrcId { get; set; }
    public List<SrcItem> SrcItems { get; set; }
}
class MainDest
{
    public int DestId { get; set; }
    public List<DestItem> DestItems { get; set; }
}
class SrcItem
{
    public int ItemId { get; set; }
    public string StringData { get; set; }
}
class DestItem
{
    public int ItemId { get; set; }
    public string StringData { get; set; }
}
@centur
Copy link
Contributor Author

centur commented Jul 4, 2016

I'm not sure how exactly it should be fixed, maybe by copying collection before iteration.
If it's by design - it will force everyone to define all possible combinations of collection mappings which kind of defeats the purpose of the mapping itself...

@chaowlert
Copy link
Collaborator

Thank you for reporting & preparing test case. I will fix and update soon.

kellyethridge pushed a commit to kellyethridge/Mapster that referenced this issue Oct 1, 2016
…s being iterated over, when TypeAdapterConfig.GlobalSettings.Compile() is called
kellyethridge pushed a commit to kellyethridge/Mapster that referenced this issue Nov 13, 2017
…s being iterated over, when TypeAdapterConfig.GlobalSettings.Compile() is called
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