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

Question - mapping to value-types #165

Closed
bslbckr opened this issue Oct 28, 2019 · 7 comments
Closed

Question - mapping to value-types #165

bslbckr opened this issue Oct 28, 2019 · 7 comments
Assignees
Labels
enhancement in-preview A feature or bug fix exists in a preview release, and a full release will follow

Comments

@bslbckr
Copy link

bslbckr commented Oct 28, 2019

We recently observed some strange behavior when it comes to mapping to members of type DateTime. We have to deal a lot with protobuf-messages where time-values are represented by the Google.Protobuf.WellKnownTypes.Timestamp (cf https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/timestamp) class.

It is straight forware to map from DateTime to Timestamp using the CreateInstancesUsing(...)-Method. However, for mapping in the opposite direction we fail to specify the mapping.

What is the preferred way to create new instances of DateTime if we don't want to map them to and from strings?

Our mappings:

Mapper.WhenMapping.From<Timestamp>().To<DateTime>()
    .CreateInstancesUsing(c => c.Source.ToDateTime())
    .And.IgnoreTargetMembersWhere(_ => true);
Mapper.WhenMapping.From<DateTime>().To<Timestamp>()
    .CreateInstancesUsing(c => Timestamp.FromDateTime(c.Source.ToUniversalTime()))
    .And.IgnoreTargetMembersWhere(_ => true);

What yields the following execution plans:

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// Map Program.ProtoA -> Program.A
// Rule Set: CreateNew
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

paToAData =>
{
    try
    {
        var a = new Program.A();
        // No data source for Date

        return a;
    }
    catch (Exception ex)
    {
        throw MappingException.For(
            "CreateNew",
            "Program.ProtoA",
            "Program.A",
            ex);
    }
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// Map Program.A -> Program.ProtoA
// Rule Set: CreateNew
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

aToPaData =>
{
    try
    {
        var protoA = aToPaData.Target = new Program.ProtoA();
        protoA.Date =
        {
            IObjectMappingData<DateTime, Timestamp> dtToTData;
            try
            {
                dtToTData = MappingDataFactory.ForChild(
                    aToPaData.Source.Date,
                    protoA.Date,
                    aToPaData.EnumerableIndex,
                    "Date",
                    0,
                    aToPaData);

                var timestamp = dtToTData.Target ?? (dtToTData.Target = Timestamp.FromDateTime(dtToTData.Source.ToUniversalTime()));
                // Seconds is ignored by filter:
// true
                // Nanos is ignored by filter:
// true

                return timestamp;
            }
            catch (Exception ex)
            {
                throw MappingException.For(
                    "CreateNew",
                    "Program.A.Date",
                    "Program.ProtoA.Date",
                    ex);
            }
        };

        return protoA;
    }
    catch (Exception ex)
    {
        throw MappingException.For(
            "CreateNew",
            "Program.A",
            "Program.ProtoA",
            ex);
    }
}

with Protoa and A being:

        public class ProtoA
	{
		public Timestamp Date{get; set;}
	}

	public class A
	{
		public DateTime Date {get; set; }
	}

The complete example could be found in this fiddle: https://dotnetfiddle.net/Nm8Bz1

@SteveWilkes
Copy link
Member

Hi!

Thanks for raising the issues you've found - rest assured I'm working on them :)

For this one, I never implemented using CreateInstancesUsing for 'simple' types - it works for classes and user-defined structs, it errors if you try to use it for primitives, but things like DateTime (and Guid, and string) fell through the gaps. I'm implementing it, and will have it in a preview in due course.

All the best,

Steve

@bslbckr
Copy link
Author

bslbckr commented Nov 1, 2019

That's great news (btw. i'd already seen your commit 😉)

If you say that CreateInstancesUsing isn't intended for this use case what about limiting it to reference types and introduce a new api-method that allows for explicit value-conversion.
E.g.

WhenMapping.From<Timestamp>().To<DateTime>().UseConversion(ctxt => ctxt.Source.ToDateTime());

@SteveWilkes
Copy link
Member

SteveWilkes commented Nov 1, 2019

It used to be limited to reference types, but it also supports user structs, so the constraint had to go. It's no problem - intuitively it should work for DateTimes, so I'm sorting that out.

I'm considering having the mapper auto-detect public [TargetType] To[TargetType]() methods the same way it detects static factory methods, so this case shouldn't need configuration in the end :)

SteveWilkes added a commit that referenced this issue Nov 2, 2019
* Failing unit test / Simplifying configured factory creation

* Tidying

* Support for configured object factories for simple (not primitive) types, re: #165

* Splitting out incorrect object factory configuration tests

* Removing unnecessary ignore from test

* Implementing simple type factories using ObjectFactories

* Adding lightweight IMemberMapperData implementation

* Creating element mapper data when using a configured simple type object factory for an enumerable element

* Support for simple type factory use in simple type enumerable mapping

* Support for conditional simple-type factories / Splitting simple-type factory tests into dedicated test class

* Support for conditional simple value factories with fallback to default conversion

* Support for simple type factory Funcs

* Support for nullable simple type factories

* Optimising simple type factory expression creation

* Test coverage for nullable simple type to simple type factory / Support for TimeSpan mapping (?!)

* Handling nested access checks in nullable-to-simple type factory use

* TimeSpan mapping test coverage

* Fixing test for .NET 3.5

* Adding package icon setting
@SteveWilkes
Copy link
Member

This is implemented in the latest v1.6 branch - release to follow! Cheers! :)

@bslbckr
Copy link
Author

bslbckr commented Nov 4, 2019

Great, we will give it a try once the preview version is available on nuget.

@SteveWilkes
Copy link
Member

This is included in v1.6-preview5, which is now available on NuGet. Thanks again!

@SteveWilkes SteveWilkes added the in-preview A feature or bug fix exists in a preview release, and a full release will follow label Nov 30, 2019
@SteveWilkes
Copy link
Member

The fix for this is included in v1.6, which is now available on NuGet.

Thanks again for the feedback!

SteveWilkes added a commit that referenced this issue Feb 16, 2020
* Updating to v1.6

* Fixing numeric to non-int-derived enum mapping

* v1.6-preview1 NuGet package

* Mapping non-mappable-element enumerables to empty collections

* Support for type-pairing using interfaces, re: #163

* v1.6-preview2

* Adding project icon

* Bugs/issue163 (#164)

* Adding .NET Core 3 test project

* Removing assembly scanning for interfaces / Handling interface -> implementation type pairing / Tidying

* Removing issue-specific test + updating .NET Core 3 package versions

* Updating icon, adding v1.6-preview3 package

* Tidying

* Tidying

* Tidying

* Splitting Root- and MemberDataSourceSetFactories

* Adding EmptyDataSourceSet / Filtering out unusable fallback data sources in DataSourceSet factory method instead of MemberPopulator

* Making EmptyDataSourceSet a singleton / Adding NullMemberPopulator

* Removing IEnumerable from IDataSourceSet

* Extending interface mapping test coverage

* Fixing .NET 3.5 source filters

* Features/simple type create instances using (#169)

* Failing unit test / Simplifying configured factory creation

* Tidying

* Support for configured object factories for simple (not primitive) types, re: #165

* Splitting out incorrect object factory configuration tests

* Removing unnecessary ignore from test

* Implementing simple type factories using ObjectFactories

* Adding lightweight IMemberMapperData implementation

* Creating element mapper data when using a configured simple type object factory for an enumerable element

* Support for simple type factory use in simple type enumerable mapping

* Support for conditional simple-type factories / Splitting simple-type factory tests into dedicated test class

* Support for conditional simple value factories with fallback to default conversion

* Support for simple type factory Funcs

* Support for nullable simple type factories

* Optimising simple type factory expression creation

* Test coverage for nullable simple type to simple type factory / Support for TimeSpan mapping (?!)

* Handling nested access checks in nullable-to-simple type factory use

* TimeSpan mapping test coverage

* Fixing test for .NET 3.5

* Adding package icon setting

* Bugs/issue166 (#170)

* Only populating MapperDatas in maptime-created ObjectMappingDatas if necessary + available, re: #166

* Renames for clarity

* Tidying

* Updating release notes

* Fixing translation of mapping plans with assignment of a local enum variable, re: #168 (#171)

Using GetVariableNameInCamelCase() for multi-invocation local variables

* Only creting a mapping LambdaExpression when necessary

* Organising mapping data source factory classes

* Tidying

* Updating to v1.6-preview4

* Lazy-loading ObjectMapperData ChidMapperData and DataSourcesByTargetMember

* Replacing Dictionary<,> with simple array-based alternative

* General tidying

* Ensuring root mapping plans include the mapper func parameter

* Removing capture creation in QualifiedMember pathfactories

* Using less derived parameter types

* Handling runtime-typed, simple-to-complex data sources configured using Map(s => s, t=> t), re: #174 (#177)

* Features/element index (#178)

* Renaming EnumerableIndex to ElementIndex

* Adding ElementKey through, adding failing test

* Support for ElementKey!

* Support for ElementKey with element-value-typed source Dictionaries

* Extra test coverage

* Updating to v1.6-preview5

* Type-Specific naming rules in the static API (#181)

* Fixing API / Adding ConfiguredNamingPattern / Adding type-specific naming tests

* Tidying

* Updating naming settings to be non-ruleset-specific

* Moving MapperContext into BasicMapperData

* Tidying

* Renaming BasicMapperData

* Setting QualifiedMemberContext on members

* Fixing tests

* Updating documentation

* Updating release notes

* Bugs/183 abstract member validation (#185)

* Support for applying custom data sources to base types only
* Improved detection of unmappable target types in mapping validation
* Tidying

* Extra test coverage

* Explicit support for DateTimeOffset mapping, re: #183

* Counting System.Drawing as a Base Class Library, re: #180

* Removing root source dynamic mapping tests from .NET Standard 1.0, re: #183

* Features/derived type mapping improvements (#186)

* Support for using MapTo() without specifying a derived source type, re: #172
* Test coverage for nested interface type pairing, re: #172

* Bugs/176 complex type data source method (#187)

* Adding failing tests re: using a custom method as a data source for a complex type

* Splitting NestedAccessCheck finding and Multi-invocation finding / Moving multi-invocation handling to MemberPopulator

* Making ExpressionInfoFinder static

* Moving multi-invocation handling back into DataSourceBase

* Applying multi-invocation replacements to DataSourceBase populations

* Handling assignment of chained multi-invocation variables / Optimising Expression replacement

* Handling null return values from custom object factories / Optimising Member Binding generation

* Registering static method complex type data sources as factoey methods

* Optimising for single multi-invocation

* Tidying

* Processing multi-invocations in DataSource finalisation

* Revert "Processing multi-invocations in DataSource finalisation"

This reverts commit 5312747.

* Avoiding multi-invocation processing of alternate population branches

* Processing multi-invocations in DataSource finalisation

* Skipping multi-invocation checks for composite data source value expressions
Optimising empty child mapper data collection access

* Fixing .NET 3.5 invocation comparison
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement in-preview A feature or bug fix exists in a preview release, and a full release will follow
Projects
None yet
Development

No branches or pull requests

2 participants