cezarypiatek / MappingGenerator

:arrows_counterclockwise: "AutoMapper" like, Roslyn based, code fix provider that allows to generate mapping code in design time.
https://marketplace.visualstudio.com/items?itemName=54748ff9-45fc-43c2-8ec5-cf7912bc3b84.mappinggenerator
MIT License
1.03k stars 119 forks source link

Improve interaction with `[InitRequired]` #167

Closed jhf closed 3 years ago

jhf commented 3 years ago

Feature description Make [InitRequired] error messages work with generated DTO mapping, by making the mapper use static conversion methods in the target class.

Reasoning behind I really enjoy how [InitRequired] provides clear error messages if a property is forgotten. However, when a mapping constructor is used, then there is no error message if some properties are not set, hence the [InitRequired] has no effect. If I instead make a static method (extension or not) and generate a mapping, then the [InitRequired] is respected. However, for nested data types, the mapper does only use constructor mappings, and not static methods. The end result is that I have to change to use static methods by hand. If instead the mapper would recognise static methods in the target class, and use those for mapping, I could combine the mapper with [InitRequired].

Real word example of the usage

This is how first tried to use the mapping with constructor, I've commented on the pain points.

namespace ExampleProjectInitRequiredAndConstructorMapper
{
    using SmartAnalyzers.CSharpExtensions.Annotations;
    using System.Collections.Generic;

    namespace Db
    {
        [InitRequired]
        public record Place
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
        }
        [InitRequired]
        public record Category
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Slug { get; set; }
            public long PlaceId { get; set; }
        }
    }
    namespace Search
    {
        [TwinType(typeof(Db.Place), IgnoredMembers = new[] { nameof(Db.Place.Id) })]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }

/////////////////////////////////// Ignores missing Categories :-(
            public Place(Db.Place from)
            {
                Guid = from.Guid;
                Name = from.Name;
            }
        }
        [TwinType(typeof(Db.Category),
        IgnoredMembers = new[] {
                nameof(Db.Category.Id),
                nameof(Db.Category.PlaceId),
                // Replaced by locale aware and searchable NameX
                nameof(Db.Category.Slug),
        })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string NameEnglish { get; set; }
            public string NameNorwegian { get; set; }

///////////////////////////////// Ignores missing NameEnglish and NameNorwegian :-(
            public Category(Db.Category from)
            {
                Guid = from.Guid;
            }
        }
    }

    namespace MobileApi
    {
        [TwinType(typeof(Search.Place))]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }

///////////////////////////////// Ignores missing Categories :-(
            public Place(Search.Place from)
            {
                Guid = from.Guid;
                Name = from.Name;
///////////////////////////////// Detects and uses constructor mapping :-)
///////////////////////////////// But that mapping doesn't warn about missing properties :-(
                Categories = from.Categories.ConvertAll(fromCategory => new Category(fromCategory));
            }
        }
        [TwinType(typeof(Search.Category),
            IgnoredMembers = new[] {
                // Replaced by Name with client locale
                nameof(Search.Category.NameEnglish),
                nameof(Search.Category.NameNorwegian),
            })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public Place Place { get; set; }

///////////////////////////////// Ignores missing Name and Place :-(
            public Category(Search.Category from)
            {
                Guid = from.Guid;
            }
        }

    }
}

Then I tried to use static mapper functions instead, but those are not detected for generated mappings.

namespace ExampleProjectInitRequiredAndStaticMapper
{
    using SmartAnalyzers.CSharpExtensions.Annotations;
    using System.Collections.Generic;

    namespace Db
    {
        [InitRequired]
        public record Place
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
        }
        [InitRequired]
        public record Category
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Slug { get; set; }
            public long PlaceId { get; set; }
        }
    }
    namespace Search
    {
        [TwinType(typeof(Db.Place), IgnoredMembers = new[] { nameof(Db.Place.Id) })]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }

///////////////////////////////// Awarns about missing Categories :-)
            public static Place FromDb(Db.Place from)
            {
                return new Place
                {
                    Guid = from.Guid,
                    Name = from.Name
                };
            }
        }
        [TwinType(typeof(Db.Category),
        IgnoredMembers = new[] {
                nameof(Db.Category.Id),
                nameof(Db.Category.PlaceId),
                // Replaced by locale aware and searchable NameX
                nameof(Db.Category.Slug),
        })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string NameEnglish { get; set; }
            public string NameNorwegian { get; set; }

///////////////////////////////// Warns about missing NameEnglish and NameNorwegian :-)
            public static Category FromDb(Db.Category from)
            {
                return new Category
                {
                    Guid = from.Guid
                };
            }
        }
    }

    namespace MobileApi
    {
        [TwinType(typeof(Search.Place))]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }

            public static Place FromSearch(Search.Place from)
            {
                return new Place
                {
                    Guid = from.Guid,
                    Name = from.Name,
///////////////////////////////// Does not detect and use FromSearch(Search.Category) :-(
                    Categories = from.Categories.ConvertAll(fromCategory => new Category
                    {
                        Guid = fromCategory.Guid
                    })
                };
            }
        }
        [TwinType(typeof(Search.Category),
            IgnoredMembers = new[] {
                // Replaced by Name with client locale
                nameof(Search.Category.NameEnglish),
                nameof(Search.Category.NameNorwegian),
            })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public Place Place { get; set; }

            public static Category FromSearch(Search.Category from)
///////////////////////////////// Warns about missing Name :-)
            {
                return new Category
                {
                    Guid = from.Guid
                };
            }
        }

    }
}
cezarypiatek commented 3 years ago

Hi @jhf

I need more time to understand what is the real problem that you are facing with. But there is one important thing: [InitOnly] and [InitRequired] were invented to replace constructors with initialization blocks and you should not mix those two approaches. Use [InitRequired] or constructor, not both. Another thing, adding constructors that map objects from a different application layer/area introduce unnecessary coupling - this is another argument to avoid that. From my own experience - keeping mappings in a separate component is the best approach.

jhf commented 3 years ago

Hi @jhf

I need more time to understand what is the real problem that you are facing with. But there is one important thing: [InitOnly] and [InitRequired] were invented to replace constructors with initialization blocks and you should not mix those two approaches.

Yes, indeed, but I don't see how the MappingGenerator can detect non constructor code.

Use [InitRequired] or constructor, not both.

Yes, I tried to use [InitRequired] without constructor, but I couldn't get MappingGenerator to detect that mapping function and use it.

Another thing, adding constructors that map objects from a different application layer/area introduce unnecessary coupling - this is another argument to avoid that.

Yes, and that is why I wish I could have a a static method - on the target class - or a separate static method, mapping between the classes, in some converter/mapping place, and have the MappingGenerator use that function, by default, if present.

From my own experience - keeping mappings in a separate component is the best approach.

I very much agree, do you have an example of how the MappingGenerator can be used with functions elsewhere, to reduce coupling?

cezarypiatek commented 3 years ago

If you put the sub-mapping method in the same class as the one in which you want to use it then it should be used in generated code when you choose "Generate mapping code using member functions"

https://user-images.githubusercontent.com/7759991/110687755-29922380-81e1-11eb-8f70-f422923d70ba.mp4

Does it solve your problem?

jhf commented 3 years ago

Thank you! I interpreted "member functions" as member functions on the target class or source class. But it is member functions in the class you are currently using as a mapper 🤦

So, my problem is solved with the code below, when using the "Generate mapping code using member functions" function.

namespace ExampleProjectInitRequiredAndSeparateMappingLogic
{
    using SmartAnalyzers.CSharpExtensions.Annotations;
    using System.Collections.Generic;

    namespace Db
    {
        [InitRequired]
        public record Place
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
        }
        [InitRequired]
        public record Category
        {
            public long Id { get; set; }
            public System.Guid Guid { get; set; }
            public string Slug { get; set; }
            public long PlaceId { get; set; }
        }
    }
    namespace Search
    {
        [TwinType(typeof(Db.Place), IgnoredMembers = new[] { nameof(Db.Place.Id) })]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }
        }
        [TwinType(typeof(Db.Category),
        IgnoredMembers = new[] {
                nameof(Db.Category.Id),
                nameof(Db.Category.PlaceId),
                // Replaced by locale aware and searchable NameX
                nameof(Db.Category.Slug),
        })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string NameEnglish { get; set; }
            public string NameNorwegian { get; set; }
        }
    }

    namespace MobileApi
    {
        [TwinType(typeof(Search.Place))]
        [InitRequired]
        public record Place
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public List<Category> Categories { get; set; }
        }
        [TwinType(typeof(Search.Category),
            IgnoredMembers = new[] {
                // Replaced by Name with client locale
                nameof(Search.Category.NameEnglish),
                nameof(Search.Category.NameNorwegian),
            })]
        [InitRequired]
        public record Category
        {
            public System.Guid Guid { get; set; }
            public string Name { get; set; }
            public Place Place { get; set; }
        }

    }

    namespace Converters
    {
        public class DbToSearch
        {
            public System.Func<string, string> SomeInjectedResource { get; set; }

            ///////////////////////////////// Warns about missing Categories :-)
            public Search.Place Map(Db.Place from)
            {
                return new Search.Place
                {
                    Guid = from.Guid,
                    Name = from.Name
                };
            }

            ///////////////////////////////// Warns about missing NameEnglish and NameNorwegian :-)
            public Search.Category Map(Db.Category from)
            {
                return new Search.Category
                {
                    Guid = from.Guid
                };
            }
        }
        public class SearchToMobileApi
        {
            ///////////////////////////////// Does detect and use FromSearch(Search.Category) :-)
            public MobileApi.Place FromSearch(Search.Place from)
            {
                return new MobileApi.Place
                {
                    Guid = from.Guid,
                    Name = from.Name,
                    Categories = from.Categories.ConvertAll(fromCategory => FromSearch(fromCategory))
                };
            }

                ///////////////////////////////// Warns about missing Name :-)
            public MobileApi.Category FromSearch(Search.Category from)
            {
                return new MobileApi.Category
                {
                    Guid = from.Guid
                };
            }
        }

    }
}
cezarypiatek commented 3 years ago

The name of the option might be misleading, this was the best that came to my mind. If you have a better idea of how this should be named then I'm all ears.

jhf commented 3 years ago

I think the example on the front page could be clearer, and demonstrate that usage.

I also think a skeleton, like I wrote in this ticket, could be of interest, to see how it can be tied together.

The only mention of member methods is the "Updating member method" that is on the DTO class itself, so a little example would probably clear this up.

Thank you for making these practical tools :-)