mikependon / RepoDB

A hybrid ORM library for .NET.
Apache License 2.0
1.7k stars 124 forks source link

Bug: DeleteAll ignores parameter tableName #645

Closed bobduncan closed 3 years ago

bobduncan commented 3 years ago

Context I have multiple tables using the same table structure. Thus I have only 1 c# class and try to use the tableName param to specify the table that I want to modify. The following code gives me an error whilst I expected it to work. I am using Nuget 1.12.4 - net48

            using (var connection = CreateConnection())
            {
                connection.DeleteAll("MySpecificTable", new List<MyGenericTableClass> {new MyGenericTableClass(1)});
            }

Result This results in an error: "There are no database fields found for table 'MyGenericTableClass'. Make sure that the target table 'MyGenericTableClass' is present in the database and/or atleast a single field is available."

Expected No error and deletes uses the primary key of the MyGenericTableClass to delete an item from table 'MySpecificTable' where primary key is 1.

Guessed solution

Implementation is using ClassProperty keyOrIdentityKey = DbConnectionExtension.GetAndGuardPrimaryKeyOrIdentityKey<TEntity>(connection, transaction); which uses ClassMappedNameCache.Get<TEntity>() to retrieve 'tableName' instead of the parameter passed into the DeleteAll method, whilst an overload with explicit tableName is also available.

    internal static ClassProperty GetAndGuardPrimaryKeyOrIdentityKey<TEntity>(
      IDbConnection connection,
      string tableName,
      IDbTransaction transaction)
      where TEntity : class
    {
      return DbConnectionExtension.GetAndGuardPrimaryKeyOrIdentityKey(connection, tableName, transaction, typeof (TEntity));
    }
mikependon commented 3 years ago

Hi @bobduncan , thank you for reporting this issue. We have tested your issue using the latest release version of RepoDB v1.12.5-beta3 and RepoDb.SqlServer v1.1.1 and it is happening there.

We also spent time investigating the issue and you are correct on pointing us to the correct fix. Many thanks for that! 👍

We would be very glad if you could contribute this fix towards this repository so your findings and recommendation will be recorded? Can you make a PR? We will then issue you the new beta (beta4) including this fix.

The fix must be applied here (sync) and here (async) by simply passing the tableName argument when calling the GetAndGuardPrimaryKeyOrIdentityKeyAsync method.

Please do not hesitate to revert if you cannot make the PR, we can always push the fix for you.

bobduncan commented 3 years ago

Hey @mikependon

Sorry for the delay. Here is the PR you requested including unit tests to verify that it worked. https://github.com/mikependon/RepoDB/pull/649

mikependon commented 3 years ago

The fix is now available at RepoDb v1.12.5-beta4.

mikependon commented 3 years ago

Closing this ticket now. Thanks