From 6ac887df28dc696a76b41da74b58ade56c743ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio?= Date: Wed, 18 Jun 2025 00:31:13 +0300 Subject: [PATCH 1/3] Better preserver timezones in ical --- lib/ice_cube/builders/ical_builder.rb | 13 +++++- lib/ice_cube/parsers/ical_parser.rb | 39 ++++++++++++++--- lib/ice_cube/time_util.rb | 3 +- spec/examples/from_ical_spec.rb | 62 +++++++++++++++++++++++++++ spec/examples/to_ical_spec.rb | 16 +++---- 5 files changed, 118 insertions(+), 15 deletions(-) diff --git a/lib/ice_cube/builders/ical_builder.rb b/lib/ice_cube/builders/ical_builder.rb index 4af655ce..ce8171af 100644 --- a/lib/ice_cube/builders/ical_builder.rb +++ b/lib/ice_cube/builders/ical_builder.rb @@ -35,10 +35,21 @@ def self.ical_utc_format(time) def self.ical_format(time, force_utc) time = time.dup.utc if force_utc + + # Keep timezone. strftime will serializer short versions of time zone (eg. EEST), + # which are not reversivible, as there are many repeated abbreviated zones. This will result in + # issues in parsing + if time.respond_to?(:time_zone) + tz_id = time.time_zone.name + return ";TZID=#{tz_id}:#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%S")}" # local time specified" + end + if time.utc? ":#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%SZ")}" # utc time else - ";TZID=#{IceCube::I18n.l(time, format: "%Z:%Y%m%dT%H%M%S")}" # local time specified + # Warn %Z (capital) is OS dependent, and not unique (CST can be in Asia +0800, or Americas (-0500) + # at least %z allows recovery of accurate utc offset during parsing + ";TZID=#{time.strftime("%z")}:#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%S")}" # local time specified end end diff --git a/lib/ice_cube/parsers/ical_parser.rb b/lib/ice_cube/parsers/ical_parser.rb index 429bd84c..78ecfe94 100644 --- a/lib/ice_cube/parsers/ical_parser.rb +++ b/lib/ice_cube/parsers/ical_parser.rb @@ -4,18 +4,25 @@ def self.schedule_from_ical(ical_string, options = {}) data = {} ical_string.each_line do |line| (property, value) = line.split(":") - (property, _tzid) = property.split(";") + (property, tzid_param) = property.split(";") + + # Extract TZID if present + tzid = nil + if tzid_param && tzid_param.start_with?("TZID=") + tzid = tzid_param[5..-1] # Remove "TZID=" prefix + end + case property when "DTSTART" - data[:start_time] = TimeUtil.deserialize_time(value) + data[:start_time] = deserialize_time_with_tzid(value, tzid) when "DTEND" - data[:end_time] = TimeUtil.deserialize_time(value) + data[:end_time] = deserialize_time_with_tzid(value, tzid) when "RDATE" data[:rtimes] ||= [] - data[:rtimes] += value.split(",").map { |v| TimeUtil.deserialize_time(v) } + data[:rtimes] += value.split(",").map { |v| deserialize_time_with_tzid(v, tzid) } when "EXDATE" data[:extimes] ||= [] - data[:extimes] += value.split(",").map { |v| TimeUtil.deserialize_time(v) } + data[:extimes] += value.split(",").map { |v| deserialize_time_with_tzid(v, tzid) } when "DURATION" data[:duration] # FIXME when "RRULE" @@ -26,6 +33,28 @@ def self.schedule_from_ical(ical_string, options = {}) Schedule.from_hash data end + def self.deserialize_time_with_tzid(time_value, tzid) + if tzid.nil? || tzid.empty? + # No TZID, use standard deserialization + TimeUtil.deserialize_time(time_value) + elsif tzid.match?(/^[+-]\d{4}$/) + # TZID is an offset like +0300 or -0500 + # Parse the time and apply the offset + base_time = Time.strptime(time_value, "%Y%m%dT%H%M%S") + offset_hours = tzid[1..2].to_i + offset_minutes = tzid[3..4].to_i + offset_seconds = offset_hours * 3600 + offset_minutes * 60 + offset_seconds *= -1 if tzid[0] == "-" + Time.new(base_time.year, base_time.month, base_time.day, + base_time.hour, base_time.min, base_time.sec, offset_seconds) + else + # TZID is a timezone name - try to use it if possible + # For now, fall back to standard parsing + # TODO: Could be enhanced to support timezone names if TZInfo is available + TimeUtil.deserialize_time(time_value) + end + end + def self.rule_from_ical(ical) raise ArgumentError, "empty ical rule" if ical.nil? diff --git a/lib/ice_cube/time_util.rb b/lib/ice_cube/time_util.rb index a18f7758..cbf2dead 100644 --- a/lib/ice_cube/time_util.rb +++ b/lib/ice_cube/time_util.rb @@ -87,7 +87,8 @@ def self.serialize_time(time) case time when Time, Date if time.respond_to?(:time_zone) - {time: time.utc, zone: time.time_zone.name} + # avoid .utc as it changes the object timezone + {time: time.getutc, zone: time.time_zone.name} else time end diff --git a/spec/examples/from_ical_spec.rb b/spec/examples/from_ical_spec.rb index 2ab66c3c..c6e96527 100644 --- a/spec/examples/from_ical_spec.rb +++ b/spec/examples/from_ical_spec.rb @@ -429,5 +429,67 @@ def sorted_ical(ical) it_behaves_like "an invalid ical string" end end + + describe "timezone offset parsing" do + it "should correctly parse TZID with offset format" do + # Test parsing TZID with offset format like +0300 + ical_string = "DTSTART;TZID=+0300:20250618T001306" + + schedule = IceCube::Schedule.from_ical(ical_string) + + # Should preserve the +0300 offset (10800 seconds) + expect(schedule.start_time.utc_offset).to eq(10800) + + # Should parse the correct local time + expect(schedule.start_time.year).to eq(2025) + expect(schedule.start_time.month).to eq(6) + expect(schedule.start_time.day).to eq(18) + expect(schedule.start_time.hour).to eq(0) + expect(schedule.start_time.min).to eq(13) + expect(schedule.start_time.sec).to eq(6) + end + + it "should correctly parse TZID with negative offset format" do + # Test parsing TZID with negative offset format like -0500 + ical_string = "DTSTART;TZID=-0500:20250618T001306" + + schedule = IceCube::Schedule.from_ical(ical_string) + + puts "Parsed time (negative offset): #{schedule.start_time.inspect}" + puts "Parsed offset: #{schedule.start_time.utc_offset}" + puts "Expected offset: #{-5 * 3600} (-18000 seconds for -0500)" + + # Should preserve the -0500 offset (-18000 seconds) + expect(schedule.start_time.utc_offset).to eq(-18000) + + # Should parse the correct local time + expect(schedule.start_time.year).to eq(2025) + expect(schedule.start_time.month).to eq(6) + expect(schedule.start_time.day).to eq(18) + expect(schedule.start_time.hour).to eq(0) + expect(schedule.start_time.min).to eq(13) + expect(schedule.start_time.sec).to eq(6) + end + + it "should handle round-trip serialization with offset-based TZID" do + # Create a time with specific offset + original_time = Time.new(2025, 6, 18, 0, 13, 6, "+03:00") + original_schedule = IceCube::Schedule.new(original_time) + + # Convert to iCal + ical_string = original_schedule.to_ical + puts "Generated iCal: #{ical_string}" + + # Parse back + parsed_schedule = IceCube::Schedule.from_ical(ical_string) + + # Should maintain timezone information + expect(parsed_schedule.start_time.utc_offset).to eq(original_time.utc_offset) + expect(parsed_schedule.start_time.to_i).to eq(original_time.to_i) # Same instant + expect(parsed_schedule.start_time.hour).to eq(original_time.hour) # Same local hour + expect(parsed_schedule.start_time.min).to eq(original_time.min) # Same local minute + expect(parsed_schedule.start_time.sec).to eq(original_time.sec) # Same local second + end + end end end diff --git a/spec/examples/to_ical_spec.rb b/spec/examples/to_ical_spec.rb index 39dd5209..fe854965 100644 --- a/spec/examples/to_ical_spec.rb +++ b/spec/examples/to_ical_spec.rb @@ -97,7 +97,7 @@ it "should be able to serialize a base schedule to ical in local time" do Time.zone = "Eastern Time (US & Canada)" schedule = IceCube::Schedule.new(Time.zone.local(2010, 5, 10, 9, 0, 0)) - expect(schedule.to_ical).to eq("DTSTART;TZID=EDT:20100510T090000") + expect(schedule.to_ical).to eq("DTSTART;TZID=Eastern Time (US & Canada):20100510T090000") end it "should be able to serialize a base schedule to ical in UTC time" do @@ -110,7 +110,7 @@ schedule = IceCube::Schedule.new(Time.zone.local(2010, 5, 10, 9, 0, 0)) schedule.add_recurrence_rule IceCube::Rule.weekly # test equality - expectation = "DTSTART;TZID=PDT:20100510T090000\n" + expectation = "DTSTART;TZID=Pacific Time (US & Canada):20100510T090000\n" expectation << "RRULE:FREQ=WEEKLY" expect(schedule.to_ical).to eq(expectation) end @@ -120,7 +120,7 @@ schedule = IceCube::Schedule.new(Time.zone.local(2010, 10, 20, 4, 30, 0)) schedule.add_recurrence_rule IceCube::Rule.weekly.day_of_week(monday: [2, -1]) schedule.add_recurrence_rule IceCube::Rule.hourly - expectation = "DTSTART;TZID=EDT:20101020T043000\n" + expectation = "DTSTART;TZID=Eastern Time (US & Canada):20101020T043000\n" expectation << "RRULE:FREQ=WEEKLY;BYDAY=2MO,-1MO\n" expectation << "RRULE:FREQ=HOURLY" expect(schedule.to_ical).to eq(expectation) @@ -131,7 +131,7 @@ schedule = IceCube::Schedule.new(Time.zone.local(2010, 5, 10, 9, 0, 0)) schedule.add_exception_rule IceCube::Rule.weekly # test equality - expectation = "DTSTART;TZID=PDT:20100510T090000\n" + expectation = "DTSTART;TZID=Pacific Time (US & Canada):20100510T090000\n" expectation << "EXRULE:FREQ=WEEKLY" expect(schedule.to_ical).to eq(expectation) end @@ -141,7 +141,7 @@ schedule = IceCube::Schedule.new(Time.zone.local(2010, 10, 20, 4, 30, 0)) schedule.add_exception_rule IceCube::Rule.weekly.day_of_week(monday: [2, -1]) schedule.add_exception_rule IceCube::Rule.hourly - expectation = "DTSTART;TZID=EDT:20101020T043000\n" + expectation = "DTSTART;TZID=Eastern Time (US & Canada):20101020T043000\n" expectation << "EXRULE:FREQ=WEEKLY;BYDAY=2MO,-1MO\n" expectation << "EXRULE:FREQ=HOURLY" expect(schedule.to_ical).to eq(expectation) @@ -195,7 +195,7 @@ it "should default to to_ical using local time" do time = Time.now schedule = IceCube::Schedule.new(Time.now) - expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.zone}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false + expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false end it "should not have an rtime that duplicates start time" do @@ -208,8 +208,8 @@ it "should be able to receive a to_ical in utc time" do time = Time.now schedule = IceCube::Schedule.new(Time.now) - expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.zone}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false - expect(schedule.to_ical(false)).to eq("DTSTART;TZID=#{time.zone}:#{time.strftime("%Y%m%dT%H%M%S")}") + expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false + expect(schedule.to_ical(false)).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") expect(schedule.to_ical(true)).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") end From a082f98b1a29830d4bf0e38e9c4c155529d87f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio?= Date: Wed, 18 Jun 2025 00:47:57 +0300 Subject: [PATCH 2/3] Fix re-parsing of timezone --- lib/ice_cube/parsers/ical_parser.rb | 12 ++++++++---- spec/examples/from_ical_spec.rb | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/ice_cube/parsers/ical_parser.rb b/lib/ice_cube/parsers/ical_parser.rb index 78ecfe94..c5089a5c 100644 --- a/lib/ice_cube/parsers/ical_parser.rb +++ b/lib/ice_cube/parsers/ical_parser.rb @@ -48,10 +48,14 @@ def self.deserialize_time_with_tzid(time_value, tzid) Time.new(base_time.year, base_time.month, base_time.day, base_time.hour, base_time.min, base_time.sec, offset_seconds) else - # TZID is a timezone name - try to use it if possible - # For now, fall back to standard parsing - # TODO: Could be enhanced to support timezone names if TZInfo is available - TimeUtil.deserialize_time(time_value) + # TZID is a timezone name - Assume it's a valid timezone in a try-catch block + begin + TimeUtil.deserialize_time({time: time_value, zone: tzid}) + rescue ArgumentError + # If the timezone is invalid, fall back to standard deserialization + # Perhaps we want to log this? + TimeUtil.deserialize_time(time_value) + end end end diff --git a/spec/examples/from_ical_spec.rb b/spec/examples/from_ical_spec.rb index c6e96527..769451f7 100644 --- a/spec/examples/from_ical_spec.rb +++ b/spec/examples/from_ical_spec.rb @@ -110,7 +110,7 @@ module IceCube ICAL ical_string_with_multiple_rules = <<-ICAL.gsub(/^\s*/, "") - DTSTART;TZID=CDT:20151005T195541 + DTSTART;TZID=America/Chicago:20151005T195541 RRULE:FREQ=WEEKLY;BYDAY=MO,TU RRULE:FREQ=WEEKLY;INTERVAL=2;WKST=SU;BYDAY=FR ICAL From 38e2f0887a3a265083879d2b7b0dfe102a7209c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio?= Date: Thu, 3 Jul 2025 14:49:10 +0300 Subject: [PATCH 3/3] Improve timezone handling in iCal: convert local time to UTC when no timezone info is present and update related specs --- lib/ice_cube/builders/ical_builder.rb | 7 +-- lib/ice_cube/parsers/ical_parser.rb | 10 ----- spec/examples/from_ical_spec.rb | 62 --------------------------- spec/examples/to_ical_spec.rb | 14 +++--- 4 files changed, 11 insertions(+), 82 deletions(-) diff --git a/lib/ice_cube/builders/ical_builder.rb b/lib/ice_cube/builders/ical_builder.rb index ce8171af..dce90694 100644 --- a/lib/ice_cube/builders/ical_builder.rb +++ b/lib/ice_cube/builders/ical_builder.rb @@ -47,9 +47,10 @@ def self.ical_format(time, force_utc) if time.utc? ":#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%SZ")}" # utc time else - # Warn %Z (capital) is OS dependent, and not unique (CST can be in Asia +0800, or Americas (-0500) - # at least %z allows recovery of accurate utc offset during parsing - ";TZID=#{time.strftime("%z")}:#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%S")}" # local time specified + # Convert to UTC as TZID=+xxxx format is not recognized by JS libraries + warn "IceCube: Time object does not have timezone info. Assuming UTC: #{caller(1..1).first}" + utc_time = time.dup.utc + ":#{IceCube::I18n.l(utc_time, format: "%Y%m%dT%H%M%SZ")}" # converted to utc time end end diff --git a/lib/ice_cube/parsers/ical_parser.rb b/lib/ice_cube/parsers/ical_parser.rb index c5089a5c..6df66cc5 100644 --- a/lib/ice_cube/parsers/ical_parser.rb +++ b/lib/ice_cube/parsers/ical_parser.rb @@ -37,16 +37,6 @@ def self.deserialize_time_with_tzid(time_value, tzid) if tzid.nil? || tzid.empty? # No TZID, use standard deserialization TimeUtil.deserialize_time(time_value) - elsif tzid.match?(/^[+-]\d{4}$/) - # TZID is an offset like +0300 or -0500 - # Parse the time and apply the offset - base_time = Time.strptime(time_value, "%Y%m%dT%H%M%S") - offset_hours = tzid[1..2].to_i - offset_minutes = tzid[3..4].to_i - offset_seconds = offset_hours * 3600 + offset_minutes * 60 - offset_seconds *= -1 if tzid[0] == "-" - Time.new(base_time.year, base_time.month, base_time.day, - base_time.hour, base_time.min, base_time.sec, offset_seconds) else # TZID is a timezone name - Assume it's a valid timezone in a try-catch block begin diff --git a/spec/examples/from_ical_spec.rb b/spec/examples/from_ical_spec.rb index 769451f7..4ef584cd 100644 --- a/spec/examples/from_ical_spec.rb +++ b/spec/examples/from_ical_spec.rb @@ -429,67 +429,5 @@ def sorted_ical(ical) it_behaves_like "an invalid ical string" end end - - describe "timezone offset parsing" do - it "should correctly parse TZID with offset format" do - # Test parsing TZID with offset format like +0300 - ical_string = "DTSTART;TZID=+0300:20250618T001306" - - schedule = IceCube::Schedule.from_ical(ical_string) - - # Should preserve the +0300 offset (10800 seconds) - expect(schedule.start_time.utc_offset).to eq(10800) - - # Should parse the correct local time - expect(schedule.start_time.year).to eq(2025) - expect(schedule.start_time.month).to eq(6) - expect(schedule.start_time.day).to eq(18) - expect(schedule.start_time.hour).to eq(0) - expect(schedule.start_time.min).to eq(13) - expect(schedule.start_time.sec).to eq(6) - end - - it "should correctly parse TZID with negative offset format" do - # Test parsing TZID with negative offset format like -0500 - ical_string = "DTSTART;TZID=-0500:20250618T001306" - - schedule = IceCube::Schedule.from_ical(ical_string) - - puts "Parsed time (negative offset): #{schedule.start_time.inspect}" - puts "Parsed offset: #{schedule.start_time.utc_offset}" - puts "Expected offset: #{-5 * 3600} (-18000 seconds for -0500)" - - # Should preserve the -0500 offset (-18000 seconds) - expect(schedule.start_time.utc_offset).to eq(-18000) - - # Should parse the correct local time - expect(schedule.start_time.year).to eq(2025) - expect(schedule.start_time.month).to eq(6) - expect(schedule.start_time.day).to eq(18) - expect(schedule.start_time.hour).to eq(0) - expect(schedule.start_time.min).to eq(13) - expect(schedule.start_time.sec).to eq(6) - end - - it "should handle round-trip serialization with offset-based TZID" do - # Create a time with specific offset - original_time = Time.new(2025, 6, 18, 0, 13, 6, "+03:00") - original_schedule = IceCube::Schedule.new(original_time) - - # Convert to iCal - ical_string = original_schedule.to_ical - puts "Generated iCal: #{ical_string}" - - # Parse back - parsed_schedule = IceCube::Schedule.from_ical(ical_string) - - # Should maintain timezone information - expect(parsed_schedule.start_time.utc_offset).to eq(original_time.utc_offset) - expect(parsed_schedule.start_time.to_i).to eq(original_time.to_i) # Same instant - expect(parsed_schedule.start_time.hour).to eq(original_time.hour) # Same local hour - expect(parsed_schedule.start_time.min).to eq(original_time.min) # Same local minute - expect(parsed_schedule.start_time.sec).to eq(original_time.sec) # Same local second - end - end end end diff --git a/spec/examples/to_ical_spec.rb b/spec/examples/to_ical_spec.rb index fe854965..063a855e 100644 --- a/spec/examples/to_ical_spec.rb +++ b/spec/examples/to_ical_spec.rb @@ -192,10 +192,10 @@ expect(schedule.duration).to eq(3600) end - it "should default to to_ical using local time" do + it "should default to to_ical using UTC when there is no timezone info" do time = Time.now - schedule = IceCube::Schedule.new(Time.now) - expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false + schedule = IceCube::Schedule.new(time) + expect(schedule.to_ical).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") # converts local to UTC end it "should not have an rtime that duplicates start time" do @@ -207,10 +207,10 @@ it "should be able to receive a to_ical in utc time" do time = Time.now - schedule = IceCube::Schedule.new(Time.now) - expect(schedule.to_ical).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") # default false - expect(schedule.to_ical(false)).to eq("DTSTART;TZID=#{time.strftime("%z")}:#{time.strftime("%Y%m%dT%H%M%S")}") - expect(schedule.to_ical(true)).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") + schedule = IceCube::Schedule.new(time) + expect(schedule.to_ical).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") # converts local to UTC + expect(schedule.to_ical(false)).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") # still converts local to UTC + expect(schedule.to_ical(true)).to eq("DTSTART:#{time.utc.strftime("%Y%m%dT%H%M%S")}Z") # force UTC end it "should be able to serialize to ical with an until date" do