Dapper-Extensions icon indicating copy to clipboard operation
Dapper-Extensions copied to clipboard

Issue in ReflectionHelper.cs

Open sstoenner opened this issue 4 years ago • 4 comments

On line 326 in function GetParameter of file ReflectionHelper.cs you have this code: var entityPropertyName = propertyName.Split('_').Last(); and it is this variable that is used to search for the mapped property - WHY? This causes an exception if any field-name of the entity contains an underscore... Please fix by just using the property name.

sstoenner avatar Oct 28 '21 18:10 sstoenner

Confirmed this as a breaking change on my side as well.
I had several legacy classes with properties which contain _ as part of their names and this caused a major refactor on the entire system.

Here's the core that triggers this https://github.com/tmsmith/Dapper-Extensions/blame/master/DapperExtensions/ReflectionHelper.cs#L326

Here's a previous PR that got closed - though not sure if it would help or make things worse: https://github.com/tmsmith/Dapper-Extensions/pull/261/files

This was introduced 6 months ago on the latest changes. Previous code works fine. Although I get the idea for this change, I believe that just splitting through the name is not safe. You can't expect a string property to have a child object there. Maybe we should remove types like int, string and others and only apply this to custom types, then? Or have an empty interface on the type we want this split/dive to be applied?

ALMMa avatar Dec 19 '21 11:12 ALMMa

I accept suggestions when you have reference maps if this is changed. Basically, when you have a joint table the properties will be like Class1_PropertyName.

The change in the closed PR it would try to find "Class1_PropertyName" classmapper and it wouldn't find it. But try to get the classmapper of PropertyName works.

Other chars can create SQL issues also.

For 1 level property, changing last to first would fix but what about multilevel?

Let's take this example:

Client -> Orders -> Items

When getting Client I have a list of Orders. So the SQL will have the client columns, them the orders columns like "Orders_?" where ? will be the column name.

Adding the items to it we would have "Orders_Items_?".

I invite you guys to please help me and provide a solution to this issue without forgeting the possible reference maps.

valfrid-ly avatar Feb 19 '22 22:02 valfrid-ly

IMO, the thing was that this is a major breaking change and we were not aware of it 🤣

TBH, so far, the feeling was that this was not intentionally introduced into DapperExtensions. I don't have strong feeling for either keeping the split on _ or changing it to something else. I would try to keep it simple and rely on the _ then.

Requires refactor, but doable. I just applied to an entire system by the time I was checking these. Has been working fine ever since.

@sstoenner your thoughts?

ALMMa avatar Apr 13 '22 23:04 ALMMa

Hello. I am using PostgreSQL, all tables and columns contain _ Current I have changed ReflectionHelper.cs to

string[] entityPropertyNames = propertyName.Split('_');

var entityPropertyName = entityPropertyNames[entityPropertyNames.Length - 1];

IMemberMap propertyMap = map.Properties.SingleOrDefault(p => p.Name == entityPropertyName);

for (int i = entityPropertyNames.Length - 2; i>=0 && propertyMap == null; i--)
{
    entityPropertyName = entityPropertyNames[i] + "_" + entityPropertyName;
    propertyMap = map.Properties.SingleOrDefault(p => p.Name == entityPropertyName);
}

if (propertyMap == null)
    throw new NullReferenceException(String.Format("{0} was not found for {1}", entityPropertyName, entityType));

Please review it.

hatuan avatar Aug 21 '23 17:08 hatuan