MoonStorm / FastCrud

fast .NET ORM for strongly typed people
MIT License
506 stars 128 forks source link

EnitityMapping overide to exclude columns not working as expected #187

Closed scott-mac closed 5 months ago

scott-mac commented 1 year ago

I followed this guide: https://github.com/MoonStorm/FastCrud/wiki/Multiple-entity-mappings for excluding columns from my update. Since I have several cases where I want to omit the same columns I created an overloaded method in my data access layer like this:

public virtual bool Update(T entity, IDbTransaction transaction = null, string[] excludeColumns = null) {            
            using (var conn = GetConnection()) {
                //note, I also have some multi-database requirements so I override for that first.
                //so in this case DefaultMapping is used to set the dialect for all default calls, like this
                // OrmConfiguration.GetDefaultEntityMapping<T>().Clone().SetDialect(dialect);
                var updateMapping = DefaultMapping.Clone();
                if (excludeColumns != null) {
                    updateMapping.UpdatePropertiesExcluding(prop => prop.IsExcludedFromUpdates = true, excludeColumns);
                }

                return conn.Update<T>(entity, statement => statement.WithTimeout(GetTimeout())
                    .AttachToTransaction(transaction)
                    .WithEntityMappingOverride(updateMapping));
            }
        }

I would expect from how the methods are named that UpdatePropertiesExcluding would leave all existing property mappings intact with the exception of whatever you pass in, but its the opposite behavior, it keeps the mappings that you pass into the UpdatePropertiesExcluding method and excludes everything else. I tried changing that line to this:

updateMapping.UpdatePropertiesExcluding(prop => prop.IsExcludedFromUpdates = false, excludeColumns);

but that resulted in no properties being omitted from the update

I was able to workaround it by using a different method:

updateMapping.UpdateProperties(prop => prop.IncludeInUpdates(!excludeColumns.Contains(prop.PropertyName)));

but you may want to consider renaming UpdatePropertiesExcluding

MoonStorm commented 5 months ago

You're right. The name of that method is a bit confusing.

Ideally you'd have the list of columns to exclude from the update at the beginning of the list of arguments, so it would read something like "Update all the properties, excluding 'Prop1', 'Prop2', with the following settings", unfortunately the variable list of params can only be added at the end of the method in C#.

Sorry about the confusion.