-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eliminate spurious ToString calls during deserialization #873
Eliminate spurious ToString calls during deserialization #873
Conversation
…ializer.cs to avoid calling .ToString() on the supplied value unless absolutely necessary. Also eliminated repeated calls to type.GetTypeInfo() on UWP, and simplified the code structure, collapsing away repeated #if !WINDOWS_UWP blocks. Added test Does_Not_ToString_Complex_Objects_During_Deserialization to RestSharp.Tests/JsonTests.cs. Made the JsonDeserializer.ConvertValue method internal so that it can be accessed by the test.
Just to clarify that parenthesized last sentence in the opening comment on the PR:
If you have a data structure like this:
...and you make a call to a service endpoint, and it returns the following JSON:
Then in the course of deserializing the request for the
The data structure does not need to recursively nest, of course; the members of The reason the code this PR is changing creates these strings is that it might be deserializing a primitive value, or an enum member name, in which case it needs the value in string form. However, if the code flow skips over all those branches and ends up at the call to The changes in this PR eliminate all this unnecessary JSON serialization. The value is only converted to a string if the data type is one of the simpler, primitive types, which pretty much guarantees that it won't be a |
@hallem I notice you merged in a whole bunch of changes back in April, and these are the most recent changes merged in. I was wondering whether this PR might be under consideration? :-) |
The main issue here is that SimpleJson.NET is not being updated for four years. We plan to move back to JSON.NET. |
Okay, cool beans :-) As long as the problem goes away in the end! |
I noticed some time ago that the
ConvertValue
method inJsonDeserializer
always.ToString()
s the object before checking what kind of deserialization must be performed. Most of the types of deserialization do require the string, but the most frequent types of deserialization will not. This becomes pathological in the case of a deep, complex, typed object structure, because each layer gets passed throughConvertValue
separately. In this PR, I've reworked theConvertValue
method to avoid theToString
call unless it is strictly required. I added a custom-builtLazy
-type class, since as I understand itLazy<T>
does not exist in .NET 3.5. I also added a unit test that ensures the offendingToString
calls are not made. (For what it's worth, theToStringCounter
value on the originalConvertValue
code has value 121 at the end of the test!)