Clockwork Time Zone fallback logic doesn't work as expected for Database Synced Events

[abhchand]

Firstly, a big thanks to the maintainers of Clockwork! The gem has been a staple of the Ruby community when it comes to scheduling jobs, and I know it takes time/effort to maintain.

Background

It appears that Clockwork uses the following fallback logic when determining what time zone to use

1. Per-event configured time zone
2. Clockwork configured time zone
3. System Time

That is evident by looking at these few lines from event.rb

module Clockwork
  class Event
    def initialize(...)
      # ...

      # Use whatever time zone was configured for this specific event
      # If not, fall back on the Clockwork configured time zone
      @timezone = options.fetch(:tz, @manager.config[:tz])
    end

    def convert_timezone(t)
      # Use one of the time zones set above
      # If not set at all, defer to just using `t`, which was originally set
      # as Time.now, which will reflect local system time
      @timezone ? t.in_time_zone(@timezone) : t
    end

    # ...
  end
end

The Issue

I have a database backed event and my model has a #tz method that will return the time zone for Clockwork to use.

The sync_database_events job periodically refreshes the list of events from the database and registers any updated events with the manager. You can see this in EventStore#register_with_manager, which further calls EventStore#options

options[:tz] = model.tz if model.respond_to?(:tz)

If my model has a nil time zone, this will set options[:tz] = nil since my model responds to :tz (as it should). So the key :tz will exist in the config, but with a nil value

When that event is registered it uses Hash#fetch which will pull a key back even if it has a nil value.

# example
options = { tz: nil }
@timezone = options.fetch(:tz, "foo")
puts @timezone # => nil

So @timezone ends up as nil and the whole event falls back on using system time. This violates the expectation set for the gem - if my model has a nil time zone it should fall back on to using the Clockwork configured timezone before it tries using the system time.

My model has to respond to a :tz method if I want to use time zone functionality, so there's no way I can have it return nothing.

Proposed Fixes

There's two possible fixes

1) Don't use fetch

@timezone = options[:tz] || @manager.config[:tz]

However if someone did intend to pass in a nil value, this would fallback to the Clockwork configured value which might not be what they expected.

2) Modify EventStore logic

options[:tz] = model.tz if model.respond_to?(:tz) && !model.tz.nil?

In my opinion this is simpler and fixes the core issue