From 5027abecd1e5e511064de75fb5248139e1c8fe23 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 18 Mar 2021 00:41:32 +0100 Subject: [PATCH] Fix cache_collection crashing when given an empty collection (#15921) * Fix cache_collection crashing when given an empty collection * Add tests --- app/controllers/concerns/cache_concern.rb | 4 +- app/lib/entity_cache.rb | 4 +- .../concerns/cache_concern_spec.rb | 40 +++++++++++++++++++ spec/lib/entity_cache_spec.rb | 19 +++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 spec/controllers/concerns/cache_concern_spec.rb create mode 100644 spec/lib/entity_cache_spec.rb diff --git a/app/controllers/concerns/cache_concern.rb b/app/controllers/concerns/cache_concern.rb index 3fb4b962a3..05e431b19a 100644 --- a/app/controllers/concerns/cache_concern.rb +++ b/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 diff --git a/app/lib/entity_cache.rb b/app/lib/entity_cache.rb index 5d51e85852..80b0046eea 100644 --- a/app/lib/entity_cache.rb +++ b/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 = [] diff --git a/spec/controllers/concerns/cache_concern_spec.rb b/spec/controllers/concerns/cache_concern_spec.rb new file mode 100644 index 0000000000..a34d7d7267 --- /dev/null +++ b/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 diff --git a/spec/lib/entity_cache_spec.rb b/spec/lib/entity_cache_spec.rb new file mode 100644 index 0000000000..43494bd92a --- /dev/null +++ b/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