From fa6f19d1f83b68f2bb729be889ab7d66adbbedb8 Mon Sep 17 00:00:00 2001 From: dineshpanda Date: Fri, 30 Aug 2019 02:09:13 +0530 Subject: [PATCH] Remove dependency on IgnorableColumn concern --- app/models/application_setting.rb | 15 ++++--- app/models/ci/build.rb | 15 ++++--- app/models/ci/runner.rb | 3 +- app/models/concerns/ignorable_column.rb | 30 ------------- app/models/deploy_key.rb | 3 +- app/models/event.rb | 1 - app/models/merge_request_diff.rb | 1 - app/models/note.rb | 3 +- app/models/notification_setting.rb | 3 +- app/models/user.rb | 9 ++-- doc/development/what_requires_downtime.md | 12 ++--- spec/models/concerns/ignorable_column_spec.rb | 44 ------------------- 12 files changed, 29 insertions(+), 110 deletions(-) delete mode 100644 app/models/concerns/ignorable_column.rb delete mode 100644 spec/models/concerns/ignorable_column_spec.rb diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 2a99c6e5c59..8ff4cba2adb 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -4,7 +4,6 @@ class ApplicationSetting < ApplicationRecord include CacheableAttributes include CacheMarkdownField include TokenAuthenticatable - include IgnorableColumn include ChronicDurationAttribute add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption, default_enabled: true) ? :optional : :required } @@ -25,12 +24,14 @@ class ApplicationSetting < ApplicationRecord serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize - ignore_column :koding_url - ignore_column :koding_enabled - ignore_column :sentry_enabled - ignore_column :sentry_dsn - ignore_column :clientside_sentry_enabled - ignore_column :clientside_sentry_dsn + self.ignored_columns = %i[ + clientside_sentry_dsn + clientside_sentry_enabled + koding_enabled + koding_url + sentry_dsn + sentry_enabled + ] cache_markdown_field :sign_in_text cache_markdown_field :help_page_text diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 7930bef5cf2..6ff89666e53 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -11,19 +11,20 @@ module Ci include ObjectStorage::BackgroundMove include Presentable include Importable - include IgnorableColumn include Gitlab::Utils::StrongMemoize include Deployable include HasRef BuildArchivedError = Class.new(StandardError) - ignore_column :commands - ignore_column :artifacts_file - ignore_column :artifacts_metadata - ignore_column :artifacts_file_store - ignore_column :artifacts_metadata_store - ignore_column :artifacts_size + self.ignored_columns = %i[ + artifacts_file + artifacts_file_store + artifacts_metadata + artifacts_metadata_store + artifacts_size + commands + ] belongs_to :project, inverse_of: :builds belongs_to :runner diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 1c1c7a5ae7a..e0e905ebfa8 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -4,7 +4,6 @@ module Ci class Runner < ApplicationRecord extend Gitlab::Ci::Model include Gitlab::SQL::Pattern - include IgnorableColumn include RedisCacheable include ChronicDurationAttribute include FromUnion @@ -36,7 +35,7 @@ module Ci FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze - ignore_column :is_shared + self.ignored_columns = %i[is_shared] has_many :builds has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent diff --git a/app/models/concerns/ignorable_column.rb b/app/models/concerns/ignorable_column.rb deleted file mode 100644 index 3bec44dc79b..00000000000 --- a/app/models/concerns/ignorable_column.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -# Module that can be included into a model to make it easier to ignore database -# columns. -# -# Example: -# -# class User < ApplicationRecord -# include IgnorableColumn -# -# ignore_column :updated_at -# end -# -module IgnorableColumn - extend ActiveSupport::Concern - - class_methods do - def columns - super.reject { |column| ignored_columns.include?(column.name) } - end - - def ignored_columns - @ignored_columns ||= Set.new - end - - def ignore_column(*names) - ignored_columns.merge(names.map(&:to_s)) - end - end -end diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 0bd90bd28e3..06f8f31b8cc 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class DeployKey < Key - include IgnorableColumn include FromUnion has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -11,7 +10,7 @@ class DeployKey < Key scope :are_public, -> { where(public: true) } scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, :namespace] }) } - ignore_column :can_push + self.ignored_columns = %i[can_push] accepts_nested_attributes_for :deploy_keys_projects diff --git a/app/models/event.rb b/app/models/event.rb index 738080eb584..392d7368033 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -2,7 +2,6 @@ class Event < ApplicationRecord include Sortable - include IgnorableColumn include FromUnion default_scope { reorder(nil) } diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 2c9dbf2585c..2402fa8e38f 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -4,7 +4,6 @@ class MergeRequestDiff < ApplicationRecord include Sortable include Importable include ManualInverseAssociation - include IgnorableColumn include EachBatch include Gitlab::Utils::StrongMemoize include ObjectStorage::BackgroundMove diff --git a/app/models/note.rb b/app/models/note.rb index a12d1eb7243..c6ef91a27ad 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -14,7 +14,6 @@ class Note < ApplicationRecord include CacheMarkdownField include AfterCommitQueue include ResolvableNote - include IgnorableColumn include Editable include Gitlab::SQL::Pattern include ThrottledTouch @@ -34,7 +33,7 @@ class Note < ApplicationRecord end end - ignore_column :original_discussion_id + self.ignored_columns = %i[original_discussion_id] cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 8306b11a7b6..9882f1dc8b3 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -1,9 +1,8 @@ # frozen_string_literal: true class NotificationSetting < ApplicationRecord - include IgnorableColumn - ignore_column :events + self.ignored_columns = %i[events] enum level: { global: 3, watch: 2, participating: 1, mention: 4, disabled: 0, custom: 5 } diff --git a/app/models/user.rb b/app/models/user.rb index 6107aaa7fca..79c89461806 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,7 +13,6 @@ class User < ApplicationRecord include Sortable include CaseSensitivity include TokenAuthenticatable - include IgnorableColumn include FeatureGate include CreatedAtFilterable include BulkMemberAccessLoad @@ -24,9 +23,11 @@ class User < ApplicationRecord DEFAULT_NOTIFICATION_LEVEL = :participating - ignore_column :external_email - ignore_column :email_provider - ignore_column :authentication_token + self.ignored_columns = %i[ + authentication_token + email_provider + external_email + ] add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) } add_authentication_token_field :feed_token diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 944bf5900c5..b9d1b95a4d7 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -45,15 +45,12 @@ rule. The first step is to ignore the column in the application code. This is necessary because Rails caches the columns and re-uses this cache in various -places. This can be done by including the `IgnorableColumn` module into the -model, followed by defining the columns to ignore. For example, to ignore +places. This can be done by defining the columns to ignore. For example, to ignore `updated_at` in the User model you'd use the following: ```ruby -class User < ActiveRecord::Base - include IgnorableColumn - - ignore_column :updated_at +class User < ApplicationRecord + self.ignored_columns = %i[updated_at] end ``` @@ -64,8 +61,7 @@ column. Both these changes should be submitted in the same merge request. Once the changes from step 1 have been released & deployed you can set up a separate merge request that removes the ignore rule. This merge request can -simply remove the `ignore_column` line, and the `include IgnorableColumn` line -if no other `ignore_column` calls remain. +simply remove the `self.ignored_columns` line. ## Renaming Columns diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb deleted file mode 100644 index 6b82825d2cc..00000000000 --- a/spec/models/concerns/ignorable_column_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe IgnorableColumn do - let :base_class do - Class.new do - def self.columns - # This method does not have access to "double" - [ - Struct.new(:name).new('id'), - Struct.new(:name).new('title'), - Struct.new(:name).new('date') - ] - end - end - end - - let :model do - Class.new(base_class) do - include IgnorableColumn - end - end - - describe '.columns' do - it 'returns the columns, excluding the ignored ones' do - model.ignore_column(:title, :date) - - expect(model.columns.map(&:name)).to eq(%w(id)) - end - end - - describe '.ignored_columns' do - it 'returns a Set' do - expect(model.ignored_columns).to be_an_instance_of(Set) - end - - it 'returns the names of the ignored columns' do - model.ignore_column(:title, :date) - - expect(model.ignored_columns).to eq(Set.new(%w(title date))) - end - end -end