From 022ee0c0c91d30dea3c1c0472525e86ec8379827 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 11 Dec 2018 09:43:34 +0100 Subject: [PATCH] don't filter tags by taggable type Due to performance reasons we cannot use the type filter on the tags. The table for ActsAsTaggableOn is too big and too unoptimized, such that the queries time out on production. See the discussion https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19740#note_120087938 for more info. --- app/controllers/admin/runners_controller.rb | 2 +- .../acts_as_taggable_on/tags_finder.rb | 13 +------- .../acts_as_taggable_on/tags_finder_spec.rb | 30 +++++++++---------- 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index dd6b3c98496..8a00408001e 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -49,7 +49,7 @@ class Admin::RunnersController < Admin::ApplicationController end def tag_list - tags = Autocomplete::ActsAsTaggableOn::TagsFinder.new(taggable_type: Ci::Runner, params: params).execute + tags = Autocomplete::ActsAsTaggableOn::TagsFinder.new(params: params).execute render json: ActsAsTaggableOn::TagSerializer.new.represent(tags) end diff --git a/app/finders/autocomplete/acts_as_taggable_on/tags_finder.rb b/app/finders/autocomplete/acts_as_taggable_on/tags_finder.rb index 81e22c85d34..71dffb72dce 100644 --- a/app/finders/autocomplete/acts_as_taggable_on/tags_finder.rb +++ b/app/finders/autocomplete/acts_as_taggable_on/tags_finder.rb @@ -5,30 +5,19 @@ module Autocomplete class TagsFinder LIMIT = 20 - def initialize(taggable_type:, params:) - @taggable_type = taggable_type + def initialize(params:) @params = params end def execute @tags = ::ActsAsTaggableOn::Tag.all - filter_by_taggable_type! search! limit! @tags end - def filter_by_taggable_type! - # rubocop: disable CodeReuse/ActiveRecord - @tags = @tags - .joins(:taggings) - .where(taggings: { taggable_type: @taggable_type.name }) - .distinct - # rubocop: enable CodeReuse/ActiveRecord - end - def search! search = @params[:search] diff --git a/spec/finders/autocomplete/acts_as_taggable_on/tags_finder_spec.rb b/spec/finders/autocomplete/acts_as_taggable_on/tags_finder_spec.rb index 9d1fac20362..857f6bba7e6 100644 --- a/spec/finders/autocomplete/acts_as_taggable_on/tags_finder_spec.rb +++ b/spec/finders/autocomplete/acts_as_taggable_on/tags_finder_spec.rb @@ -6,10 +6,10 @@ describe Autocomplete::ActsAsTaggableOn::TagsFinder do describe '#execute' do context 'with empty params' do it 'returns all tags' do - create :ci_runner, tag_list: ['tag1'] - create :ci_runner, tag_list: ['tag2'] + ActsAsTaggableOn::Tag.create!(name: 'tag1') + ActsAsTaggableOn::Tag.create!(name: 'tag2') - tags = described_class.new(taggable_type: Ci::Runner, params: {}).execute.map(&:name) + tags = described_class.new(params: {}).execute.map(&:name) expect(tags).to match_array %w(tag1 tag2) end @@ -18,10 +18,10 @@ describe Autocomplete::ActsAsTaggableOn::TagsFinder do context 'filter by search' do context 'with an empty search term' do it 'returns an empty collection' do - create :ci_runner, tag_list: ['tag1'] - create :ci_runner, tag_list: ['tag2'] + ActsAsTaggableOn::Tag.create!(name: 'tag1') + ActsAsTaggableOn::Tag.create!(name: 'tag2') - tags = described_class.new(taggable_type: Ci::Runner, params: { search: '' }).execute.map(&:name) + tags = described_class.new(params: { search: '' }).execute.map(&:name) expect(tags).to be_empty end @@ -29,10 +29,10 @@ describe Autocomplete::ActsAsTaggableOn::TagsFinder do context 'with a search containing 2 characters' do it 'returns the tag that strictly matches the search term' do - create :ci_runner, tag_list: ['t1'] - create :ci_runner, tag_list: ['t11'] + ActsAsTaggableOn::Tag.create!(name: 't1') + ActsAsTaggableOn::Tag.create!(name: 't11') - tags = described_class.new(taggable_type: Ci::Runner, params: { search: 't1' }).execute.map(&:name) + tags = described_class.new(params: { search: 't1' }).execute.map(&:name) expect(tags).to match_array ['t1'] end @@ -40,10 +40,10 @@ describe Autocomplete::ActsAsTaggableOn::TagsFinder do context 'with a search containing 3 characters' do it 'returns the tag that partially matches the search term' do - create :ci_runner, tag_list: ['tag1'] - create :ci_runner, tag_list: ['tag11'] + ActsAsTaggableOn::Tag.create!(name: 'tag1') + ActsAsTaggableOn::Tag.create!(name: 'tag11') - tags = described_class.new(taggable_type: Ci::Runner, params: { search: 'ag1' }).execute.map(&:name) + tags = described_class.new(params: { search: 'ag1' }).execute.map(&:name) expect(tags).to match_array %w(tag1 tag11) end @@ -54,10 +54,10 @@ describe Autocomplete::ActsAsTaggableOn::TagsFinder do it 'limits the result set by the limit constant' do stub_const("#{described_class}::LIMIT", 1) - create :ci_runner, tag_list: ['tag1'] - create :ci_runner, tag_list: ['tag2'] + ActsAsTaggableOn::Tag.create!(name: 'tag1') + ActsAsTaggableOn::Tag.create!(name: 'tag2') - tags = described_class.new(taggable_type: Ci::Runner, params: { search: 'tag' }).execute + tags = described_class.new(params: { search: 'tag' }).execute expect(tags.count).to eq 1 end