aspnetboilerplate / aspnetboilerplate

ASP.NET Boilerplate - Web Application Framework
https://aspnetboilerplate.com
MIT License
11.78k stars 3.79k forks source link

Improvements for Dapper integration #4465

Open emisand opened 5 years ago

emisand commented 5 years ago

Dapper Repository improvements

Currently the IDapperRepository provided by ABP is intended to be used on Entities defined in the model and DbContext.

If I want to run a query with a result that is not an Entity, for example a DTO or a scalar value, then I would have to implement that query inside a repository for an existing entity, which may not be coherent at all.

Look at this example:

public class PersonDapperRepository : DapperEfRepositoryBase<InAccessDbContext, Person, Guid>
{
    public PersonDapperRepository(IActiveTransactionProvider activeTransactionProvider) : base(activeTransactionProvider)
    {
    }

    public Task<DateListDto> GetMenuCreationDateList()
    {
        return QueryAsync<DateListDto>("SELECT CreationTime AS Date FROM Menu");
    }
}

Even if my query is returning a DTO (DateListDto) object, I must create inside a generic repository for a specific Entity (Person).

I suggest you to create a IDapperRepository that does not take generic arguments for an entity and primary key, so we can make queries not related to an entity in particular. This repository should also be used with dependency injection so we can excecute Dapper queries without defining a custom dapper repository.

Also there are 2 parameters used in the Dapper Query() and QueryAsync() methods that should be exposed in order to allow the excecution of stored procedures and other query types. This parameters are int? commandTimeout = null, CommandType? commandType = null

DateTime materialization

Also another problem with the current Dapper integration is that DateTime values are not created the same way as it's done with Entity Framework. In Entity Framework, the DateTime materialization is configured at AbpEntityMaterializerSource

That class allows ABP to set the DateTimeKind property the DateTime values retrieved from the database based on the configured ClockProvider

For Dapper the following classes should be added:

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

    public override DateTime Parse(object value)
    {
        return Clock.Normalize((DateTime)value);
    }
}

public class NullableDateTimeHandler : SqlMapper.TypeHandler<DateTime?>
{
    public override void SetValue(IDbDataParameter parameter, DateTime? value)
    {
        parameter.Value = value;
    }

    public override DateTime? Parse(object value)
    {
        if (value == null)
        {
            return null;
        }

        DateTime? dateTime = (DateTime?)value;

        return Clock.Normalize(dateTime.Value);
    }
}

Then those type handlers should be enabled by adding the following lines to the Initialize method of the corresponding AbpModule or maybe somewhere in the DefaultDbContextResolver

SqlMapper.AddTypeHandler(new NullableDateTimeHandler());
SqlMapper.AddTypeHandler(new DateTimeHandler());

Conclusions

The suggestions I made are easy to implement and would allow us to make better repositories and make the DateTime materialization consistent between Entity Framework and Dapper.

Let me know if you have any questions about this, or if you want me to make a pull request with an implementation of the suggestions I made.

hikalkan commented 5 years ago

Thank you for your suggestions. Will check it in the next milestone.

maliming commented 5 years ago

hi @emisand

I implemented a Dapper base class that only requires DbContent. https://github.com/aspnetboilerplate/aspnetboilerplate/commit/2e4fcd1299ea20ba79d3fc47fa87d15eefcd1900#diff-c6bf62f378a817efe38370f9c0f8c9bcR12 It provides DbConnection and DbTransaction so you can use everything from dapper.

Regarding DateTimeHandler, dapper currently seems to have a problem to be solved. https://github.com/StackExchange/Dapper/issues/607

emisand commented 5 years ago

@maliming Thanks for the update. It's weird to see that error in Dapper's repo, the code example I provided works perfectly for me in my ASP.NET Zero solution. Maybe you could try it anyways and create some unit tests to check if it works for nullable datetimes.

emisand commented 5 years ago

@maliming

According to StackExchange/Dapper#471 The part of TypeHandler that is currently broken is SetValue(), so Parse() works as expected.

As you can see in the DateTimeHandler I implemented, it's not doing any custom code for the SetValue method, because we only want to translate DateTimes from UTC when they are read, and we don't really need to transform them when they are written to the database using Dapper as all DateTime values we write into the database should already have their DateTimeKind setted.

I think AbpEntityMaterializerSource works the same way, but please confirm if it's enough to have the DateTime parsing and not the SetValue for any supported use case.

maliming commented 5 years ago

Thank you for your supplement, I will confirm again today,

maliming commented 5 years ago

Another problem is that we can't implement the ShouldDisableDateTimeNormalization function.

amin168 commented 4 years ago

hi @ismcagdas Could you merge this enhancement as soon as possible?

ismcagdas commented 4 years ago

@amin168 I have added it to 5.1. Will work on it soon.

amin168 commented 4 years ago

@ismcagdas But my project's version is 4.x. Could you merge it into 4.x version? Thanks.

ismcagdas commented 4 years ago

Sure, we will apply same changes for 4.12 version.

amin168 commented 4 years ago

Thanks a lot

maliming commented 4 years ago

@amin168 You should know that Dapper enhancements have some features that are not yet implemented.