Browse Source

Fix cache_collection crashing when given an empty collection (#15921)

* Fix cache_collection crashing when given an empty collection

* Add tests
Claire 3 years ago
parent
commit
5027abecd1

+ 3 - 1
app/controllers/concerns/cache_concern.rb

@@ -31,7 +31,9 @@ module CacheConcern
   def cache_collection(raw, klass)
     return raw unless klass.respond_to?(:with_includes)
 
-    raw                    = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation)
+    raw = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation)
+    return [] if raw.empty?
+
     cached_keys_with_value = Rails.cache.read_multi(*raw).transform_keys(&:id)
     uncached_ids           = raw.map(&:id) - cached_keys_with_value.keys
 

+ 3 - 1
app/lib/entity_cache.rb

@@ -16,7 +16,9 @@ class EntityCache
   end
 
   def emoji(shortcodes, domain)
-    shortcodes   = Array(shortcodes)
+    shortcodes = Array(shortcodes)
+    return [] if shortcodes.empty?
+
     cached       = Rails.cache.read_multi(*shortcodes.map { |shortcode| to_key(:emoji, shortcode, domain) })
     uncached_ids = []
 

+ 40 - 0
spec/controllers/concerns/cache_concern_spec.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe CacheConcern, type: :controller do
+  controller(ApplicationController) do
+    include CacheConcern
+
+    def empty_array
+      render plain: cache_collection([], Status).size
+    end
+
+    def empty_relation
+      render plain: cache_collection(Status.none, Status).size
+    end
+  end
+
+  before do
+    routes.draw do
+      get  'empty_array' => 'anonymous#empty_array'
+      post 'empty_relation' => 'anonymous#empty_relation'
+    end
+  end
+
+  describe '#cache_collection' do
+    context 'given an empty array' do
+      it 'returns an empty array' do
+        get :empty_array
+        expect(response.body).to eq '0'
+      end
+    end
+
+    context 'given an empty relation' do
+      it 'returns an empty array' do
+        get :empty_relation
+        expect(response.body).to eq '0'
+      end
+    end
+  end
+end

+ 19 - 0
spec/lib/entity_cache_spec.rb

@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+RSpec.describe EntityCache do
+  let(:local_account)  { Fabricate(:account, domain: nil, username: 'alice') }
+  let(:remote_account) { Fabricate(:account, domain: 'remote.test', username: 'bob', url: 'https://remote.test/') }
+
+  describe '#emoji' do
+    subject { EntityCache.instance.emoji(shortcodes, domain) }
+
+    context 'called with an empty list of shortcodes' do
+      let(:shortcodes) { [] }
+      let(:domain)     { 'example.org' }
+
+      it 'returns an empty array' do
+        is_expected.to eq []
+      end
+    end
+  end
+end