Scope ensures to only run if default_timezone actually altered

lib/github/sql.rb

@@ -167,16 +167,18 @@ def add(sql, extras = nil)
 
       query << " " unless query.empty?
 
-      if @force_tz
-        zone = ActiveRecord::Base.default_timezone
-        ActiveRecord::Base.default_timezone = @force_tz
-      end
+      begin
+        if @force_tz
+          zone = ActiveRecord::Base.default_timezone
+          ActiveRecord::Base.default_timezone = @force_tz
+        end
 
-      query << interpolate(sql.strip, extras)
+        query << interpolate(sql.strip, extras)
 
-      self
-    ensure
-      ActiveRecord::Base.default_timezone = zone if @force_tz
+        self
+      ensure
+        ActiveRecord::Base.default_timezone = zone if @force_tz
+      end
     end
 
     # Public: Add a chunk of SQL to the query, unless query generated so far is empty.
@@ -270,38 +272,40 @@ def results
       return @results if defined? @results
       return [] if frozen?
 
-      if @force_tz
-        zone = ActiveRecord::Base.default_timezone
-        ActiveRecord::Base.default_timezone = @force_tz
-      end
+      begin
+        if @force_tz
+          zone = ActiveRecord::Base.default_timezone
+          ActiveRecord::Base.default_timezone = @force_tz
+        end
 
-      case query
-      when /\ADELETE/i
-        @affected_rows = connection.delete(query, "#{self.class.name} Delete")
+        case query
+        when /\ADELETE/i
+          @affected_rows = connection.delete(query, "#{self.class.name} Delete")
 
-      when /\AINSERT/i
-        @last_insert_id = connection.insert(query, "#{self.class.name} Insert")
+        when /\AINSERT/i
+          @last_insert_id = connection.insert(query, "#{self.class.name} Insert")
 
-      when /\AUPDATE/i
-        @affected_rows = connection.update(query, "#{self.class.name} Update")
+        when /\AUPDATE/i
+          @affected_rows = connection.update(query, "#{self.class.name} Update")
 
-      when /\ASELECT/i
-        # Why not execute or select_rows? Because select_all hits the query cache.
-        @hash_results = connection.select_all(query, "#{self.class.name} Select")
-        @results = @hash_results.map(&:values)
+        when /\ASELECT/i
+          # Why not execute or select_rows? Because select_all hits the query cache.
+          @hash_results = connection.select_all(query, "#{self.class.name} Select")
+          @results = @hash_results.map(&:values)
 
-      else
-        @results = connection.execute(query, "#{self.class.name} Execute").to_a
-      end
+        else
+          @results = connection.execute(query, "#{self.class.name} Execute").to_a
+        end
 
-      @results ||= []
+        @results ||= []
 
-      retrieve_found_row_count
-      freeze
+        retrieve_found_row_count
+        freeze
 
-      @results
-    ensure
-      ActiveRecord::Base.default_timezone = zone if @force_tz
+        @results
+      ensure
+        ActiveRecord::Base.default_timezone = zone if @force_tz
+      end
     end
 
     # Public: If the query is a SELECT, return an array of hashes instead of an array of arrays.

test/github/sql_test.rb

@@ -213,4 +213,53 @@ def test_models
       ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS `repositories`")
     end
   end
+
+  def test_add_doesnt_modify_timezone_if_early_return_invoked
+    begin
+      original_default_timezone = ActiveRecord::Base.default_timezone
+      refute_nil original_default_timezone
+
+      ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS `repositories`")
+      ActiveRecord::Base.connection.execute <<-SQL
+        CREATE TABLE `repositories` (
+          `id` int(11) NOT NULL AUTO_INCREMENT,
+          `name` varchar(255) DEFAULT NULL,
+          PRIMARY KEY (`id`)
+        ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+      SQL
+
+      sql = GitHub::SQL.new("SELECT * FROM repositories WHERE id = ?", force_timezone: :local)
+      sql.add nil, id: 1
+
+      assert_equal original_default_timezone, ActiveRecord::Base.default_timezone
+    ensure
+      ActiveRecord::Base.default_timezone = original_default_timezone
+      ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS `repositories`")
+    end
+  end
+
+  def test_results_doesnt_modify_timezone_if_early_return_invoked
+    begin
+      original_default_timezone = ActiveRecord::Base.default_timezone
+      refute_nil original_default_timezone
+
+      ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS `repositories`")
+      ActiveRecord::Base.connection.execute <<-SQL
+        CREATE TABLE `repositories` (
+          `id` int(11) NOT NULL AUTO_INCREMENT,
+          `name` varchar(255) DEFAULT NULL,
+          PRIMARY KEY (`id`)
+        ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+      SQL
+
+      sql = GitHub::SQL.new("SELECT * FROM repositories LIMIT 1", force_timezone: :local)
+      sql.results
+      sql.results
+
+      assert_equal original_default_timezone, ActiveRecord::Base.default_timezone
+    ensure
+      ActiveRecord::Base.default_timezone = original_default_timezone
+      ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS `repositories`")
+    end
+  end
 end