fsprojects / FSharp.Data

F# Data: Library for Data Access
https://fsprojects.github.io/FSharp.Data
Other
806 stars 288 forks source link

Reading a DateTime fails when the DateTime.Kind property gets set to Unspecified #1437

Open rjmccann101 opened 2 years ago

rjmccann101 commented 2 years ago

When a .xsd file is used to create the XML TypeProvider type from a .xsd and when the .xsd uses xs:dateTime then if when the date is parsed into a DateTime structure it does not set the DateTime.Kind to something other than DateTimeKind.Unspecified an error will be raised when attempting to read the date value.

For example: given the following xsd fragment

<xs:element name="XMLTimeStamp" type="xs:dateTime" id="S1.2">
    <xs:annotation>
        <xs:documentation>Date and time that the XML was originally created.</xs:documentation>
    </xs:annotation>
</xs:element>

and the following xml

<XMLTimeStamp>2022-04-28T10:09:17</XMLTimeStamp>

the following error is generated when trying to access the XMLTimeStamp

Installed Packages
Fsharp.Data, 4.2.8
Error: System.Exception: Expecting DateTimeOffset in Value, got 2022-04-28T10:09:17
   at Microsoft.FSharp.Core.PrintfModule.PrintFormatToStringThenFail@1439.Invoke(String message) in D:\workspace\_work\1\s\src\fsharp\FSharp.Core\printf.fs:line 1439
   at System.Runtime.CompilerServices.RuntimeHelpers.DispatchTailCalls(IntPtr callersRetAddrSlot, IntPtr callTarget, IntPtr retVal)
   at FSharp.Data.Runtime.TextRuntime.GetNonOptionalValue[T](String name, FSharpOption`1 opt, FSharpOption`1 originalValue)
   at <StartupCode$FSI_0029>.$FSI_0029.main@()

The issue is the code in AsDateTimeOffset in FileConversions.fs

match ParseISO8601FormattedDateTime text cultureInfo with
| Some d when d.Kind <> DateTimeKind.Unspecified -> 
  match DateTimeOffset.TryParse(text, cultureInfo, dateTimeStyles) with
  | true, dto -> dto |> Some
  | false, _ -> None
| _ -> None

When the date is 2022-04-28T10:09:17 them d.Kind is set to DateTimeKind.Unspecified when parsed into a DateTime by ParseISO8601FormattedDateTime and the code returns None. Changing the date 2022-04-28T10:09:17Z causes the d.Kind to be set and the date gets returned correctly.

One possible solution would be to mimic what AsDateTime, which is used when type=xs:date, does which is to give the DateTime the DateTimeKind.Local Kind when none is specified. This would change the code to something like this:

match ParseISO8601FormattedDateTime text cultureInfo with
| Some d when d.Kind = DateTimeKind.Unspecified ->
        new DateTimeOffset(new DateTime(d.Ticks,DateTimeKind.Local)) |> Some
| Some d -> new DateTimeOffset(d) |> Some
| _ -> None

This also removes the double parsing of the date string that currently takes place.

An alternative and even simpler approach would be to ignore the fact that DateTimeKind.Unspecified is used, we still have a valid date and can construct a valid DateTimeOffset from it.

match ParseISO8601FormattedDateTime text cultureInfo with
| Some d -> new DateTimeOffset(d) |> Some
| _ -> None
rjmccann101 commented 2 years ago

So changing AsDateTimeOffset to parse dates without explicit TimeZone information doesn't work - it breaks lots of tests in other code because what couldn't be inferred as a DateTimeOffset now can be and types change from DateTime to DateTimeOffset.

nhirschey commented 2 years ago

I believe it comes from the XML type xs:datetime getting mapped to DateTimeOffset via XmlTypeCode.DateTime -> typeof<System.DateTimeOffset>.

Given the discussion of time zones specificions in XML here (? is this the best source) it might make sense for the type provider to conditionally infer xs:datetime to either be System.DateTime or System.DateTimeOffset based on the sample format, thought that's a bit annoying.

giacomociti commented 1 year ago

We had some discussions around this topic a long time ago (see #1181 and #980) and we settled on a sub-optimal solution also to avoid impacting the existing stuff. But I hope you can improve on this. My preferred workaround, in cases like this, is to avoid accessing the generated property and use instead theXElement property. The generated types in fact are just wrappers around and an XElement and exposing the underlying XML I think it's been a very pragmatic and effective choice, allowing to use raw XML when necessary.