Fix Ci::Runner#contacted_at is sometimes a string which breaks runner admin pagefix-broken-admin-runner-page

2 files changed, 61 insertions, 7 deletions

diff --git a/app/models/concerns/redis_cacheable.rb b/app/models/concerns/redis_cacheable.rb
index b889f4202dc..0de97b4ba3c 100644
--- a/app/models/concerns/redis_cacheable.rb
+++ b/app/models/concerns/redis_cacheable.rb
@@ -15,7 +15,13 @@ module RedisCacheable
   end
 
   def cached_attribute(attribute)
-    (cached_attributes || {})[attribute]
+    value = (cached_attributes || {})[attribute]
+
+    if self.class.columns_hash[attribute.to_s]&.type == :datetime
+      value && Time.zone.parse(value)
+    else
+      value
+    end
   end
 
   def cache_attributes(values)
diff --git a/spec/models/concerns/redis_cacheable_spec.rb b/spec/models/concerns/redis_cacheable_spec.rb
index 3d7963120b6..07af90ff6cf 100644
--- a/spec/models/concerns/redis_cacheable_spec.rb
+++ b/spec/models/concerns/redis_cacheable_spec.rb
@@ -1,11 +1,59 @@
 require 'spec_helper'
 
+class TestClass
+  ColumnMock = Struct.new("ColumnMock", :type)
+  include RedisCacheable
+
+  cached_attr_reader :some_string, :some_time
+
+  def read_attribute(attribute)
+  end
+
+  def id
+    123
+  end
+
+  def self.columns_hash
+    {
+      "some_string" => ColumnMock.new(:string),
+      "some_time" => ColumnMock.new(:datetime)
+    }
+  end
+end
+
 describe RedisCacheable do
-  let(:model) { double }
+  let(:model) { TestClass.new }
 
-  before do
-    model.extend(described_class)
-    allow(model).to receive(:cache_attribute_key).and_return('key')
+  describe 'getter method' do
+    let(:payload) { { some_string: 'string value', some_time: '2018-05-04T07:29:50.548+02:00' } }
+
+    before do
+      Gitlab::Redis::SharedState.with do |redis|
+        expect(redis).to receive(:get).with('cache:TestClass:123:attributes')
+          .and_return(payload.to_json)
+      end
+    end
+
+    context 'when value is cached' do
+      it 'returns the string value' do
+        expect(model.some_string).to eq('string value')
+      end
+
+      context 'with a datetime type value' do
+        it 'converts the value into ActiveSupport::TimeWithZone' do
+          expect(model.some_time).to be_kind_of(ActiveSupport::TimeWithZone)
+          expect(model.some_time).to eq(Time.zone.parse('2018-05-04T07:29:50.548+02:00'))
+        end
+
+        context 'when value is nil' do
+          let(:payload) { { some_time: nil } }
+
+          it 'returns nil' do
+            expect(model.some_time).to be_nil
+          end
+        end
+      end
+    end
   end
 
   describe '#cached_attribute' do
@@ -15,7 +63,7 @@ describe RedisCacheable do
 
     it 'gets the cache attribute' do
       Gitlab::Redis::SharedState.with do |redis|
-        expect(redis).to receive(:get).with('key')
+        expect(redis).to receive(:get).with('cache:TestClass:123:attributes')
           .and_return(payload.to_json)
       end
 
@@ -30,7 +78,7 @@ describe RedisCacheable do
 
     it 'sets the cache attributes' do
       Gitlab::Redis::SharedState.with do |redis|
-        expect(redis).to receive(:set).with('key', values.to_json, anything)
+        expect(redis).to receive(:set).with('cache:TestClass:123:attributes', values.to_json, anything)
       end
 
       subject