From 12fe223664cd6a2fc1277de4f85b58c4b5e8a722 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Sat, 1 Dec 2018 13:02:06 +0100 Subject: [PATCH] Add DB indices to histories and tickets tables (fixes #2368) --- db/migrate/20120101000001_create_base.rb | 3 + db/migrate/20120101000010_create_ticket.rb | 2 + ...68_add_indices_to_histories_and_tickets.rb | 11 ++++ ...7_remove_invalid_user_foreign_keys_spec.rb | 8 +-- ...d_indices_to_histories_and_tickets_spec.rb | 55 +++++++++++++++++++ spec/support/db_migration.rb | 25 ++++++++- 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20181123000000_issue_2368_add_indices_to_histories_and_tickets.rb create mode 100644 spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 8f1f6fbda..b39974ac6 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -408,6 +408,9 @@ class CreateBase < ActiveRecord::Migration[4.2] add_index :histories, [:id_from] add_index :histories, [:value_from], length: 255 add_index :histories, [:value_to], length: 255 + add_index :histories, [:related_o_id] + add_index :histories, [:related_history_object_id] + add_index :histories, %i[o_id history_object_id related_o_id] add_foreign_key :histories, :history_types add_foreign_key :histories, :history_objects add_foreign_key :histories, :history_attributes diff --git a/db/migrate/20120101000010_create_ticket.rb b/db/migrate/20120101000010_create_ticket.rb index 07ee6371d..6604e2e13 100644 --- a/db/migrate/20120101000010_create_ticket.rb +++ b/db/migrate/20120101000010_create_ticket.rb @@ -111,6 +111,8 @@ class CreateTicket < ActiveRecord::Migration[4.2] add_index :tickets, [:pending_time] add_index :tickets, [:type] add_index :tickets, [:time_unit] + add_index :tickets, %i[group_id state_id] + add_index :tickets, %i[group_id state_id owner_id] add_foreign_key :tickets, :groups add_foreign_key :tickets, :users, column: :owner_id add_foreign_key :tickets, :users, column: :customer_id diff --git a/db/migrate/20181123000000_issue_2368_add_indices_to_histories_and_tickets.rb b/db/migrate/20181123000000_issue_2368_add_indices_to_histories_and_tickets.rb new file mode 100644 index 000000000..8d319f92e --- /dev/null +++ b/db/migrate/20181123000000_issue_2368_add_indices_to_histories_and_tickets.rb @@ -0,0 +1,11 @@ +class Issue2368AddIndicesToHistoriesAndTickets < ActiveRecord::Migration[5.1] + def up + return if !Setting.find_by(name: 'system_init_done') + + add_index :histories, :related_o_id if !index_exists?(:histories, :related_o_id) + add_index :histories, :related_history_object_id if !index_exists?(:histories, :related_history_object_id) + add_index :histories, %i[o_id history_object_id related_o_id] if !index_exists?(:histories, %i[o_id history_object_id related_o_id]) + add_index :tickets, %i[group_id state_id] if !index_exists?(:tickets, %i[group_id state_id]) + add_index :tickets, %i[group_id state_id owner_id] if !index_exists?(:tickets, %i[group_id state_id owner_id]) + end +end diff --git a/spec/db/migrate/issue_1977_remove_invalid_user_foreign_keys_spec.rb b/spec/db/migrate/issue_1977_remove_invalid_user_foreign_keys_spec.rb index ea99d6701..977694090 100644 --- a/spec/db/migrate/issue_1977_remove_invalid_user_foreign_keys_spec.rb +++ b/spec/db/migrate/issue_1977_remove_invalid_user_foreign_keys_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do context 'invalid User foreign key columns' do it 'cleans up OnlineNotification#user_id', db_strategy: :reset do - witout_foreign_key(:online_notifications, column: :user_id) + without_foreign_key(:online_notifications, column: :user_id) create(:online_notification, user_id: 1337) valid = create(:online_notification, user_id: existing_user_id) @@ -22,8 +22,8 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do end it 'cleans up RecentView#created_by_id', db_strategy: :reset do - witout_foreign_key(:online_notifications, column: :user_id) - witout_foreign_key(:recent_views, column: :created_by_id) + without_foreign_key(:online_notifications, column: :user_id) + without_foreign_key(:recent_views, column: :created_by_id) create(:recent_view, created_by_id: 1337) valid = create(:recent_view, created_by_id: existing_user_id) @@ -36,7 +36,7 @@ RSpec.describe Issue1977RemoveInvalidUserForeignKeys, type: :db_migration do end it 'cleans up Avatar#o_id', db_strategy: :reset do - witout_foreign_key(:online_notifications, column: :user_id) + without_foreign_key(:online_notifications, column: :user_id) create(:avatar, object_lookup_id: ObjectLookup.by_name('User'), o_id: 1337) valid_ticket = create(:avatar, object_lookup_id: ObjectLookup.by_name('Ticket'), o_id: 1337) diff --git a/spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb b/spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb new file mode 100644 index 000000000..4dca1e617 --- /dev/null +++ b/spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb @@ -0,0 +1,55 @@ +require 'rails_helper' + +RSpec.describe Issue2368AddIndicesToHistoriesAndTickets, type: :db_migration do + self.use_transactional_tests = false # see comments on #without_index method + + before { without_index(table, column: columns) } + + context 'for histories table' do + let(:table) { :histories } + + context 'and related_o_id column' do + let(:columns) { %i[related_o_id] } + + it 'adds an index' do + expect { migrate }.to change { index_exists?(table, columns) }.to(true) + end + end + + context 'and related_history_object_id column' do + let(:columns) { %i[related_history_object_id] } + + it 'adds an index' do + expect { migrate }.to change { index_exists?(table, columns) }.to(true) + end + end + + context 'and o_id, history_object_id, & related_o_id columns' do + let(:columns) { %i[o_id history_object_id related_o_id] } + + it 'adds a composite index' do + expect { migrate }.to change { index_exists?(table, columns) }.to(true) + end + end + end + + context 'for tickets table' do + let(:table) { :tickets } + + context 'and group_id & state_id columns' do + let(:columns) { %i[group_id state_id] } + + it 'adds a composite index' do + expect { migrate }.to change { index_exists?(table, columns) }.to(true) + end + end + + context 'and group_id, state_id, & owner_id columns' do + let(:columns) { %i[group_id state_id owner_id] } + + it 'adds a composite index' do + expect { migrate }.to change { index_exists?(table, columns) }.to(true) + end + end + end +end diff --git a/spec/support/db_migration.rb b/spec/support/db_migration.rb index 4ef73c015..c18aa79af 100644 --- a/spec/support/db_migration.rb +++ b/spec/support/db_migration.rb @@ -42,10 +42,10 @@ module DbMigrationHelper # @param [Symbol] column the name of the foreign_key column # # @example - # witout_foreign_key(:online_notifications, column: :user_id) + # without_foreign_key(:online_notifications, column: :user_id) # # @return [nil] - def witout_foreign_key(from_table, column:) + def without_foreign_key(from_table, column:) suppress_messages do break if !foreign_key_exists?(from_table, column: column) @@ -53,6 +53,25 @@ module DbMigrationHelper end end + # Helper method for setting up specs on DB migrations that add indices. + # Make sure to define type: :db_migration in your RSpec.describe call + # and add `self.use_transactional_tests = false` to your context. + # + # @param [Symbol] from_table the name of the table with the indexed column + # @param [Symbol] name(s) of indexed column(s) + # + # @example + # without_index(:online_notifications, column: :user_id) + # + # @return [nil] + def without_index(from_table, column:) + suppress_messages do + break if !index_exists?(from_table, column) + + remove_index(from_table, column: column) + end + end + # Enables the usage of `ActiveRecord::Migration` methods. # # @see ActiveRecord::Migration @@ -91,7 +110,7 @@ module DbMigrationHelper # # @return [nil] def adds_foreign_key(from_table, column:) - witout_foreign_key(from_table, column: column) + without_foreign_key(from_table, column: column) suppress_messages do expect do