DapperLib / Dapper.Contrib

Dapper community contributions - additional extensions for Dapper
Other
263 stars 96 forks source link

Dapper.Contrib does not pass Turkey Test #73

Open yusufozturk opened 6 years ago

yusufozturk commented 6 years ago

In the following code:

https://github.com/StackExchange/Dapper/blob/master/Dapper.Contrib/SqlMapperExtensions.cs#L117

There is a string.equals operation to find "id" in the all properties which fails in Turkey test. So operations are getting failed in all Turkish locale servers. "id" is "İD" or "ıd" in Turkish, so basically it fails for the "ID" column.

We have a custom "ToLower" extension which handles this issue. I don't know if there is another easy solution to work around.

rhubley commented 6 years ago

If you changed it to compare to "ID" instead of "id" does it work for you? Apparently ToUpper works better for Turkey, don't know if it fails for other cultures though.

yusufozturk commented 6 years ago

@rhubley

ToUpper can make it "İD", so it will fail again :)

I think the best way is adding culture to string.equal operation. For now, we wrote a custom convert method to change ıd to id, İD to ID etc.

canertosuner commented 6 years ago

@yusufozturk Could you share your ext. method ? or what does it exactly do for it ?

rhubley commented 6 years ago

Setting a culture is tricky, you would need your code and Database to share the same culture which might not always be the case.

Adondriel commented 6 years ago

We should probably address this by using whatever locale is used in the model object, by changing the hardcoded "id" to grabbing the variable name from the object itself. This would mean that if they define the model in turkish(is that the name of the language?) that should give us the correct variable to query against. (grabbed via the [Key] attribute, of course)

Edit: NVM, this is not needed. This is an internal thing. We are using the wrong method for comparing them, I think. see reference: https://msdn.microsoft.com/en-us/library/system.stringcomparer.currentcultureignorecase(v=vs.110).aspx