Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[SUGGESTION] Robust Json serialization #1793

Open
ConiJB opened this issue Aug 17, 2020 · 7 comments
Open

[SUGGESTION] Robust Json serialization #1793

ConiJB opened this issue Aug 17, 2020 · 7 comments

Comments

@ConiJB
Copy link

ConiJB commented Aug 17, 2020

Is your feature request related to a problem? Please describe.
When serializing large doubles using a different Json serializer (Google.Protobuf.JsonFormatter) the decimal places are omitted when the double has no rest. The LiteDB.JsonSerializer detects this as an int32 and throws an exception if this is larger than int32.MaxValue.
So this

 var jsonString = @"{ 'LargeDouble': 210000000000 }";
 var doc = LiteDB.JsonSerializer.Deserialize(jsonString).AsDocument;

throws a "System.OverflowException" with "Value was either too large or too small for an Int32". This behaviour is of course not wrong, but I think it's a very simple change to detect this and change the conversion to double.

Describe the solution you'd like
If you include a simple check in "Tokenizer.cs" at the end of "ReadNumber(ref bool db)" that tries to convert large numbers into Int32 and returns dbl=true if it cannot this serializes more robust for these edge cases.

  //Fallback if not correct serialized doubles
  if (!dbl && sb.Length >= 10)
  {
      if (!Int32.TryParse(sb.ToString(), out int _))
        dbl = true;
   }
  return sb.ToString();

Describe alternatives you've considered
I tried to bring Google.Protobuf.JsonFormatter to keep the decimal point for all doubles (same as Newtonsoft serializer behaves) but failed.

@lbnascimento
Copy link
Contributor

@ConiJB I think a better solution would be to change https://github.com/mbdavid/LiteDB/blob/5d88c20a14f962e685d255347b3d407d38538248/LiteDB/Document/Json/JsonReader.cs#L83 into something like this:

case TokenType.Int:
{
	try
	{
		return new BsonValue(Convert.ToInt32(token.Value, _numberFormat)); 
	}
	catch(OverflowException)
	{
		return new BsonValue(Convert.ToInt64(token.Value, _numberFormat)); 
	}
}

@nightroman
Copy link
Contributor

IMHO, exceptions are better be avoided. They are (1) expensive, (2) noise on debugging, (3) not recommended for normal scenarios in any case.

@lbnascimento
Copy link
Contributor

@nightroman You are correct, it is better to avoid using exceptions for flow control if possible (I forgot about that). That said, I believe the best solution would be this:

case TokenType.Int:
    {
        if (Int32.TryParse(token.Value, NumberStyles.Any, _numberFormat, out int result))
            return new BsonValue(result);
        else
            return new BsonValue(Int64.Parse(token.Value, NumberStyles.Any, _numberFormat));
    }

@ConiJB
Copy link
Author

ConiJB commented Aug 18, 2020

@lbnascimento thanks for the reply, it does make sense to use Int64 and not double as proposed. I also agree with @nightroman that exceptions are too expensive and should be avoided.
However with this approach I run into problems in my projects because when parsing the json string back into an IMessage (using Google.Protobuf.JsonParser) it refuses to parse as it expects a double but gets the "$numberlong".
But the Google.Protobuf.JsonParser created the problem to begin with, so I guess I need to work on that side anyway instead of trying to fix it in LiteDB.

@lbnascimento
Copy link
Contributor

@ConiJB If you really need a double, you could simply take the result of JsonSerializer.Deserialize(string), which is a BsonValue, and call .AsDouble. You can also check if the BsonValue contains a numeric value with .IsNumber.

@ConiJB
Copy link
Author

ConiJB commented Aug 19, 2020

@lbnascimento Thanks, unfortunatly that's not as easy: I am not deserializing single values but fairly large objects. I do not know beforehand which values and what kind of values are in those documents. Especially I do not know which values are supposed to be double and which ones Int64.

@lbnascimento
Copy link
Contributor

@ConiJB You could use our BsonMapper to convert BsonValues (returned by JsonSerializer.Deserialize(string)) into regular .NET objects. If your document has a known structure, you could create a class that represents it and deserialize using

var result = BsonMapper.Global.Deserialize<MyClass>(JsonSerializer.Deserialize(jsonString));

If it doesn't have a known structure, you could simply use Dictionary<string, object> as the type parameter for BsonMapper.Deserialize<T>.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants