Skip to content

Conversation

@brjohnstmsft
Copy link
Member

…it tests

NOTE: I am not incrementing the NuGet package version because there is
another PR coming very soon and there's no point in shipping a new SDK
version that will only live for one day.

Apparently deserialization of DateTime in Search results has been broken
since the Search SDK first shipped (DateTimeOffset deserialization works
fine). The root cause is the "double deserialization" we need to do in order
to "unflatten" the search response JSON. First we deserialize to JObject and
capture the OData annotations (for things like @search.highlight,
@search.score, etc.). This results in the DateTime fields being deserialized
into DateTimeOffsets. Next, we deserialize again into the user's model class,
which was resulting in an InvalidCastException because JSON.NET can't cast
DateTimeOffset to DateTime. The fix is to add some read logic to
DateTimeConverter (and a lot of new test coverage).

I also took the opportunity to fix some unit tests that weren't running
correctly in other time zones. This bug was reported back in June:

#1202

…it tests

NOTE: I am not incrementing the NuGet package version because there is
another PR coming very soon and there's no point in shipping a new SDK
version that will only live for one day.

Apparently deserialization of DateTime in Search results has been broken
since the Search SDK first shipped (DateTimeOffset deserialization works
fine). The root cause is the "double deserialization" we need to do in order
to "unflatten" the search response JSON. First we deserialize to JObject and
capture the OData annotations (for things like @search.highlight,
@search.score, etc.). This results in the DateTime fields being deserialized
into DateTimeOffsets. Next, we deserialize again into the user's model class,
which was resulting in an InvalidCastException because JSON.NET can't cast
DateTimeOffset to DateTime. The fix is to add some read logic to
DateTimeConverter (and a lot of new test coverage).

I also took the opportunity to fix some unit tests that weren't running
correctly in other time zones. This bug was reported back in June:

Azure#1202
@azurecla
Copy link

Hi @brjohnstmsft, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (brjohnst). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, or work for Microsoft Open Technologies, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

ogail added a commit that referenced this pull request Aug 19, 2015
Search SDK: Fixing DateTime deserialization and time zone-specific un…
@ogail ogail merged commit df50486 into Azure:master Aug 19, 2015
@brjohnstmsft brjohnstmsft deleted the datetimefix-pr branch August 19, 2015 22:13
brjohnstmsft added a commit to brjohnstmsft/azure-sdk-for-net that referenced this pull request Aug 21, 2015
Used git cherry-pick to move these recent PRs from the master branch to the
AutoRest branch:

Azure#1384
Azure#1378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants