redis / redis-om-dotnet

Object mapping, and more, for Redis and .NET
MIT License
441 stars 74 forks source link

Incorrect behavior when updating decimal values #418

Closed axnetg closed 5 months ago

axnetg commented 6 months ago

Hello!

I am using Redis.OM 0.5.4 and I have found that when running on cultures where the decimal separator is not a dot the Update and UpdateAsync methods don't work correctly on decimal values.

Running the profiler showed that the method is sending the SET command serializing the decimal with a comma instead of a dot. I believe that this should be serialized using a culture-invariant approach to ensure consistent behavior.

Initially, it was difficult to detect this issue because Redis appears to interpret the value as an integer instead of producing an error.

Command captured by the profiler:

"EVALSHA" "118f0eee57ae15ddfd37bd680e8002358aaf7161" "1" "Person:01HF7QY15KWSKESTE6QD671HAD" "SET" "$.Height" "75,7"

Repro code:

using Redis.OM;
using Redis.OM.Modeling;

Thread.CurrentThread.CurrentCulture = new System.Globalization.CultureInfo("es-ES");

var provider = new RedisConnectionProvider("redis://localhost:6379");
await provider.Connection.CreateIndexAsync(typeof(Person));

var collection = provider.RedisCollection<Person>();
var person = new Person() { Name = "Steve", Height = 75.5m };
await collection.InsertAsync(person);

person = collection.First();
person.Height += 0.2m;
await collection.UpdateAsync(person);

[Document(StorageType = StorageType.Json)]
public class Person
{
    [RedisIdField]
    public string Id { get; set; }
    [Indexed]
    public string Name { get; set; }
    [Indexed]
    public decimal Height { get; set; }
}

When querying back the object the Height is 75 instead of 75.7. image

slorello89 commented 6 months ago

Had a similar issue with doubles some time ago, probably a similar fix - use InvarientCulture in the proper places to ensure that it's serialized/deserialized correctly.