-
Notifications
You must be signed in to change notification settings - Fork 9
Respect floating DATE-TIME values during parsing #52
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -415,7 +415,7 @@ defmodule ICal.Deserialize do | |
|
|
||
| # convert a string to a proper timezone, this includes ones with a / | ||
| # but also ones like Central Standard Time, so we try our best to normalize those | ||
| # all else fails, assume UTC | ||
| # all else fails, use the calendar default timezone if one was declared | ||
| def to_timezone(timezone, default \\ "Etc/UTC") | ||
| def to_timezone(nil, default), do: default | ||
|
|
||
|
|
@@ -427,6 +427,9 @@ defmodule ICal.Deserialize do | |
| ICal.Deserialize.Timezone.windows_to_olson(timezone) != nil -> | ||
| ICal.Deserialize.Timezone.windows_to_olson(timezone) | ||
|
|
||
| is_nil(default) -> | ||
| nil | ||
|
|
||
| true -> | ||
| default | ||
| end | ||
|
|
@@ -440,7 +443,8 @@ defmodule ICal.Deserialize do | |
|
|
||
| It returns `nil` for ill-formed dates or datetime strings. | ||
| """ | ||
| @spec to_date(String.t() | nil, map, ICal.t()) :: Date.t() | DateTime.t() | nil | ||
| @spec to_date(String.t() | nil, map, ICal.t()) :: | ||
| Date.t() | DateTime.t() | NaiveDateTime.t() | nil | ||
| def to_date(nil, _params, _calendar), do: nil | ||
|
Comment on lines
+446
to
448
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change to the API. which will break code that does not currently expect / handle NaiveDateTimes. This would mean either bumping the major version of the library (which can be done), or finding a way to avoid introducing a new incompatible data type. It probably requires the new type, so this will also need a bump to 3.0 in the mix.exs. |
||
|
|
||
| def to_date(date_string, %{"VALUE" => "DATE"}, _calendar) do | ||
|
|
@@ -457,11 +461,21 @@ defmodule ICal.Deserialize do | |
| end | ||
|
|
||
| def to_date(date_string, %{"TZID" => timezone}, %ICal{default_timezone: default_timezone}) do | ||
| # Microsoft Outlook calendar .ICS files report times in Greenwich Standard Time (UTC +0) | ||
| # so just convert this to UTC | ||
| timezone = to_timezone(timezone, default_timezone) | ||
|
|
||
| to_date_in_timezone(date_string, timezone) | ||
| if timezone == nil do | ||
| to_local_date(date_string) | ||
| else | ||
| to_date_in_timezone(date_string, timezone) | ||
| end | ||
| end | ||
|
|
||
| def to_date(<<_::binary-size(15), "Z">> = date_string, _params, _calendar) do | ||
| to_date_in_timezone(date_string, "Etc/UTC") | ||
| end | ||
|
|
||
| def to_date(date_string, _params, %ICal{default_timezone: nil}) do | ||
| to_local_date(date_string) | ||
|
Comment on lines
+466
to
+478
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would probably be easier to add this to def to_date_in_timezone(date_string, timezone) do
# datetime in the form "{YYYY}{0M}{0D}T{h24}{m}{s}[Z]"
with <<y::binary-size(4), m::binary-size(2), d::binary-size(2), ?T, t_h::binary-size(2),
t_m::binary-size(2), t_s::binary-size(2), rest::binary>>
when rest == "" or rest == "Z" <- date_string,
{year, ""} <- Integer.parse(y),
{month, ""} <- Integer.parse(m),
{day, ""} <- Integer.parse(d),
{hour, ""} <- Integer.parse(t_h),
{minute, ""} <- Integer.parse(t_m),
{second, ""} <- Integer.parse(t_s) do
to_date_in_timezone(year, month, day, hour, minute, second, rest, timezone)
else
_ -> nil
end
end
defp to_date_in_timezone(year, month, day, hour, minute, second, "Z", _timezone) do
to_date_in_timezone(year, month, day, hour, minute, second, "Z", "Etc/UTC")
end
defp to_date_in_timezone(year, month, day, hour, minute, second, _suffix, nil) do
case NaiveDateTime.new(year, month, day, hour, minute, second) do
{:ok, datetime} -> datetime
_ -> nil
end
end
defp to_date_in_timezone(year, month, day, hour, minute, second, _suffix, timezone) do
with {:ok, date} <- Date.new(year, month, day),
{:ok, time} <- Time.new(hour, minute, second) do
ICal.as_valid_datetime(date, time, timezone)
else
_ -> nil
end
end
@doc "Parses a local date string as a NaiveDatetime, returning `nil` on failure"
@spec to_local_date(String.t()) :: NaiveDateTime.t() | nil
def to_local_date(date_string) do
# Ensure there is no trailing 'Z' by only passing in the first 15 characters.
to_date_in_timezone(String.slice(date_string, 0, 15), nil)
end |
||
| end | ||
|
|
||
| def to_date(date_string, _params, %ICal{default_timezone: default_timezone}) do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,14 +39,14 @@ defmodule ICal.Event do | |
|
|
||
| @type t :: %__MODULE__{ | ||
| uid: String.t() | nil, | ||
| dtstamp: DateTime.t() | nil, | ||
| created: DateTime.t() | nil, | ||
| dtstart: Date.t() | DateTime.t() | nil, | ||
| dtend: Date.t() | DateTime.t() | nil, | ||
| modified: Date.t() | nil, | ||
| recurrence_id: Date.t() | nil, | ||
| exdates: [Date.t() | DateTime.t()], | ||
| rdates: [Date.t() | DateTime.t() | ICal.period()], | ||
| dtstamp: DateTime.t() | NaiveDateTime.t() | nil, | ||
| created: DateTime.t() | NaiveDateTime.t() | nil, | ||
| dtstart: Date.t() | DateTime.t() | NaiveDateTime.t() | nil, | ||
| dtend: Date.t() | DateTime.t() | NaiveDateTime.t() | nil, | ||
| modified: Date.t() | DateTime.t() | NaiveDateTime.t() | nil, | ||
| recurrence_id: Date.t() | DateTime.t() | NaiveDateTime.t() | nil, | ||
| exdates: [Date.t() | DateTime.t() | NaiveDateTime.t()], | ||
| rdates: [Date.t() | DateTime.t() | NaiveDateTime.t() | ICal.period()], | ||
|
Comment on lines
+42
to
+49
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing all these types added, it would probably make sense to introduce custom types in @type rfc5455_datetime :: DateTime.t() | NativeDateTime.t()
@type maybe_rfc5455_datetime :: nil | rfc5455_datetime
@type rfc5455_date :: Date.t() | rfc5455_datetime
@type maybe_rfc5455_date :: nil | rfc5455_dateThis would make things more succinct and less prone to accidental omissions, as we could then right this as: dtstamp: ICal.maybe_rfc5455_datetime(),
created: ICal.maybe_rfc5455_datetime(),
dtstart: ICal.maybe_rfc5455_date(),
exdates: [ICal.rfc5455_date],wdyt? |
||
| rrule: ICal.Recurrence.t() | nil, | ||
| class: String.t() | nil, | ||
| description: String.t() | nil, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,6 +201,50 @@ defmodule ICal.DeserializeTest do | |
| assert ~D[1998-01-19] == ICal.Deserialize.to_date("19980119", %{"VALUE" => "DATE"}, %ICal{}) | ||
| end | ||
|
|
||
| test "to_date/3 parses a floating DATE-TIME as a naive datetime" do | ||
| assert ~N[1998-01-19 02:00:00] == | ||
| ICal.Deserialize.to_date("19980119T020000", %{}, %ICal{}) | ||
| end | ||
|
|
||
| test "to_date/3 parses a floating DATE-TIME in the calendar default timezone when set" do | ||
| result = | ||
| ICal.Deserialize.to_date( | ||
| "19980119T020000", | ||
| %{}, | ||
| %ICal{default_timezone: "America/Chicago"} | ||
| ) | ||
|
|
||
| assert %DateTime{ | ||
| year: 1998, | ||
| month: 1, | ||
| day: 19, | ||
| hour: 2, | ||
| minute: 0, | ||
| second: 0, | ||
| time_zone: "America/Chicago" | ||
| } = result | ||
| end | ||
|
|
||
| test "to_date/3 parses a UTC DATE-TIME as a UTC datetime" do | ||
| assert ~U[1998-01-19 02:00:00Z] == | ||
| ICal.Deserialize.to_date("19980119T020000Z", %{}, %ICal{}) | ||
| end | ||
|
|
||
| test "to_date/3 parses a DATE-TIME with TZID in that timezone" do | ||
| result = | ||
| ICal.Deserialize.to_date("19980119T020000", %{"TZID" => "America/Chicago"}, %ICal{}) | ||
|
|
||
| assert %DateTime{ | ||
| year: 1998, | ||
| month: 1, | ||
| day: 19, | ||
| hour: 2, | ||
| minute: 0, | ||
| second: 0, | ||
| time_zone: "America/Chicago" | ||
| } = result | ||
| end | ||
|
|
||
| test "to_date/3 returns nil for an unparseable VALUE=DATE string" do | ||
| assert nil == ICal.Deserialize.to_date("garbage", %{"VALUE" => "DATE"}, %ICal{}) | ||
| end | ||
|
|
@@ -298,8 +342,31 @@ defmodule ICal.DeserializeTest do | |
| # olson | ||
| assert event.dtend.time_zone == "America/Chicago" | ||
|
|
||
| # unrecognized tz | ||
| assert event.dtstamp.time_zone == "Etc/UTC" | ||
| # unrecognized tz without a calendar default timezone | ||
| assert event.dtstamp == ~N[2222-12-24 08:30:00] | ||
| end | ||
|
|
||
| test "unrecognized TZID falls back to the calendar default timezone when set" do | ||
| ics = """ | ||
| BEGIN:VCALENDAR | ||
| X-WR-TIMEZONE:Europe/Zurich | ||
| BEGIN:VEVENT | ||
| DTSTAMP;TZID=Garbage:22221224T083000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """ | ||
|
Comment on lines
+350
to
+357
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be put into a file in |
||
|
|
||
| %ICal{events: [event]} = ICal.from_ics(ics) | ||
|
|
||
| assert %DateTime{ | ||
| year: 2222, | ||
| month: 12, | ||
| day: 24, | ||
| hour: 8, | ||
| minute: 30, | ||
| second: 0, | ||
| time_zone: "Europe/Zurich" | ||
| } = event.dtstamp | ||
| end | ||
|
|
||
| test "with CR+LF line endings" do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,7 @@ defmodule ICalTest do | |
| assert calendar.version == "2.0" | ||
| assert calendar.product_id == "-//Elixir ICal//EN" | ||
| assert calendar.method == "REQUEST" | ||
| assert calendar.default_timezone == "Etc/UTC" | ||
| assert calendar.default_timezone == nil | ||
| assert calendar.custom_properties == %{} | ||
| end | ||
|
|
||
|
|
@@ -77,14 +77,37 @@ defmodule ICalTest do | |
| assert Fixtures.calendar(:custom_properties) == calendar | ||
| end | ||
|
|
||
| test "ICal with custom tz alter dates" do | ||
| test "ICal with custom tz does not alter UTC dates" do | ||
| dtstamp = ~U[2015-12-24 08:00:00Z] | ||
|
|
||
| %ICal{events: [%ICal.Event{dtstamp: parsed_date}]} = | ||
| Helper.test_data("custom_calendar_tz") |> ICal.from_ics() | ||
|
|
||
| assert dtstamp != parsed_date | ||
| assert parsed_date.time_zone == "Europe/Zurich" | ||
| assert dtstamp == parsed_date | ||
| end | ||
|
|
||
| test "ICal with custom tz applies default timezone to floating dates" do | ||
| ics = """ | ||
| BEGIN:VCALENDAR | ||
| X-WR-TIMEZONE:Europe/Zurich | ||
| BEGIN:VEVENT | ||
| UID:1 | ||
| DTSTART:20151224T080000 | ||
| END:VEVENT | ||
| END:VCALENDAR | ||
| """ | ||
|
Comment on lines
+90
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be put into a file in test/data/ and loaded with Helper.test_data/1 |
||
|
|
||
| %ICal{events: [%ICal.Event{dtstart: parsed_date}]} = ICal.from_ics(ics) | ||
|
|
||
| assert %DateTime{ | ||
| year: 2015, | ||
| month: 12, | ||
| day: 24, | ||
| hour: 8, | ||
| minute: 0, | ||
| second: 0, | ||
| time_zone: "Europe/Zurich" | ||
| } = parsed_date | ||
| end | ||
|
|
||
| test "ICal.to_ics/1 of a calendar with an event, as in README" do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -499,7 +499,7 @@ defmodule ICal.Test.Fixtures do | |
| } | ||
| } | ||
| ], | ||
| default_timezone: "Etc/UTC" | ||
| default_timezone: nil | ||
| } | ||
| end | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
defaultis nil, then returningdefaultwill in thetruebranch will returnnil. So I think this change can be removed?