landmarkhw/Dapper.GraphQL

Support for ExecuteWithSqlIdentity using Func to select identifying property

benmccallum opened this issue · 3 comments

Current implementation of ExecuteWithSqlIdentity and the equivalent SqlLite implementation require the developer to specify the type of their entity's identifier (int, long, etc). Problem is this could change over time, e.g. changing User.Id from int to long as your user base grows. This would be a tricky change as the ExecuteWithSqlIdentity<TIdentityType> method isn't linked (in a refactoring sense) to the TEntity it identifies.

Can we use a method signature like:
TIdentityType ExecuteWithSqlIdentity<TEntity, TIdentityType>(this SqlInsertContext context, IDbConnection dbConnection, Func<TEntity, TIdentityType> identityTypeSelector);

Gave it a go and the issue is that when you're doing:

personId = SqlBuilder
    .Insert(person)
    .ExecuteWithSqlIdentity<int>(db);

the type returned by Insert is just a SqlInsertContext, with no reference to the TEntity it will be inserting. That means the dev has to pass TEntity, but then you need to pass TIdentityType too... defeating the purpose.

Although at least it requires those types match and would break on change of the property's type... so that's a win.

Give me a little longer (gotta run now) and I'll put in a PR for this for discussion. I'm mostly done.

PR created. I think it adds value. I'll definitely swap over to using those overloads for the type-safety across refactors. Take a look and let me know what you think. And if you like it whether we should deprecate the originals.