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

TypeHandler is not called for parameterised constructor #461

Closed
JoeStead opened this issue Feb 15, 2016 · 6 comments
Closed

TypeHandler is not called for parameterised constructor #461

JoeStead opened this issue Feb 15, 2016 · 6 comments

Comments

@JoeStead
Copy link

We've had to create a TypeHandler to handle DateTimeOffset because SQLite has no concept of date/time, so it is saved as a string, which is fine. When querying the data back out, the Parse method is only called if it is being serialised to a type with a parameterless constructor. If a constructor exists which initialises all the properties the type handler is not called, which results in a "InvalidOperationException" with the following

A parameterless default constructor or one matching signature (System.String Id, System.String SomeValue, System.String SomeDateValue) is required for DapperTypeHandler.ParameterisedTypeConstructor materialization

Note how it is looking for SomeDateValue to be a string still.

I'm not sure if I'm explaining this the best way, I haven't been using Dapper for very long, so this may not be the best way of solving this problem. Below is a small, reproducible example using an in memory SQLite database.

using System;
using System.Data;
using System.Data.SQLite;
using Dapper;

namespace DapperTypeHandler
{
    class Program
    {
        static void Main(string[] args)
        {
            SqlMapper.AddTypeHandler(new DateTimeOffsetHandler());

            var dbConnection = new SQLiteConnection("Data Source=:memory:");
            dbConnection.Open();
            dbConnection.Execute(@"CREATE TABLE SomeTable (
                                      Id                VARCHAR,
                                      SomeValue         VARCHAR,
                                      SomeDateValue     DATETIMEOFFSET
                                    )");

            dbConnection.Execute(
                "INSERT INTO SomeTable (Id, SomeValue, SomeDateValue) VALUES (@Id, @SomeValue, @SomeDateValue)",
                new
                {
                    Id = "id",
                    SomeValue = "what up?",
                    SomeDateValue = new DateTimeOffset(2016, 02, 15, 16, 0, 0, TimeSpan.Zero)
                });

            var parameterlessWorks = dbConnection.Query<ParameterlessTypeConstructor>("SELECT * FROM SomeTable");

            //throws about not being able to find constructor (It expects the DateTime field to be a string still)
            var parameterDoesNot = dbConnection.Query<ParameterisedTypeConstructor>("SELECT * FROM SomeTable"); 

        }
    }

    class DateTimeOffsetHandler : SqlMapper.TypeHandler<DateTimeOffset>
    {
        public override void SetValue(IDbDataParameter parameter, DateTimeOffset value)
        {
            parameter.Value = value.ToString();
        }

        public override DateTimeOffset Parse(object value)
        {
            return DateTimeOffset.Parse(value.ToString());
        }
    }

    class ParameterlessTypeConstructor
    {
        public string Id { get; set; }

        public string SomeValue { get; set; }
        public DateTimeOffset SomeDateValue { get; set; }
    }

    class ParameterisedTypeConstructor
    {
        public ParameterisedTypeConstructor(string id, string someValue, DateTimeOffset someDateValue)
        {
            Id = id;
            SomeValue = someValue;
            SomeDateValue = someDateValue;
        }

        public string Id { get; set; }

        public string SomeValue { get; set; }
        public DateTimeOffset SomeDateValue { get; set; }
    }
}
mgravell added a commit that referenced this issue Feb 15, 2016
…nning on SQL-Server - needs to be changed to SQLite to repro
@mgravell
Copy link
Member

Thanks; I've pulled it in as an initial test, but haven't repro'd yet, due to SQL-Server talking natively in datetimeoffset, and SQLite not working properly on DNX451 (my test rig targets DNX451). Will evolve this (probably using a completely artificial test type) and see if we can get it working. The fun question will probably be around constructor resolution - i.e. how does that happen reliably (by name? by type?). Those are questions that can be deferred a bit when using properties / fields, but for a constructor, for example, there could be 3 constructors - choosing between them when none is an exact match and all 3 would require (say) 1 conversion... could be fun. Might need the caller to implement a custom ITypeMap to compensate.

@JoeStead
Copy link
Author

I would have thought if there is an ambiguous constructor, it would just throw. Trying to infer the constructor precedence based on TypeHandlers/Maps/Whatever would easily lead to some very confusing scenarios, and given this issue seems to have gone undetected, it doesn't seem to be overly important?

@mgravell
Copy link
Member

Indeed, I'm mostly aiming for "make it work". I have a spike on my local
clone with an artificial type ("struct Blarg", IIRC), so that I can make it
react in a provider-independent way - in progress.
On 16 Feb 2016 3:57 p.m., "Joe Stead" [email protected] wrote:

I would have thought if there is an ambiguous constructor, it would just
throw. Trying to infer the constructor precedence based on
TypeHandlers/Maps/Whatever would easily lead to some very confusing
scenarios, and given this issue seems to have gone undetected, it doesn't
seem to be overly important?


Reply to this email directly or view it on GitHub
#461 (comment)
.

@mgravell
Copy link
Member

Should be fixed next build, but if you happen to be able to build locally and check it, I'd be eager to hear how it goes

@mgravell
Copy link
Member

This is in 1.50.0-beta8 (the "beta" here is just because it includes the DNX targets, which is technically RC1). https://www.nuget.org/packages/Dapper/1.50.0-beta8

@JoeStead
Copy link
Author

Awesome, haven't had chance to try it out yet, but will hopefully get round to it today. Thanks, really quick turnaround too 😄

basicdevelopment pushed a commit to basicdevelopment/Dapper that referenced this issue Jul 25, 2018
…e issue: DapperLib#718). It implements the method suggested by numerous other users but leaves the byte order (endian) to the .net layer.

It also fixes the guid equivalent issue raised in DapperLib#461.
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

Successfully merging a pull request may close this issue.

2 participants