Added Cop to blacklist the use of serialize

This Cop blacklists the use of ActiveRecord's "serialize" method, except
for cases where we already use this.
This commit is contained in:
Yorick Peterse 2017-05-31 13:47:36 +02:00
parent 3d160480c0
commit cd74c1434e
No known key found for this signature in database
GPG key ID: EDD30D2BEB691AC9
18 changed files with 85 additions and 27 deletions

View file

@ -13,13 +13,13 @@ class ApplicationSetting < ActiveRecord::Base
[\r\n] # any number of newline characters
}x
serialize :restricted_visibility_levels
serialize :import_sources
serialize :disabled_oauth_sign_in_sources, Array
serialize :domain_whitelist, Array
serialize :domain_blacklist, Array
serialize :repository_storages
serialize :sidekiq_throttling_queues, Array
serialize :restricted_visibility_levels # rubocop:disable Cop/ActiverecordSerialize
serialize :import_sources # rubocop:disable Cop/ActiverecordSerialize
serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiverecordSerialize
serialize :domain_whitelist, Array # rubocop:disable Cop/ActiverecordSerialize
serialize :domain_blacklist, Array # rubocop:disable Cop/ActiverecordSerialize
serialize :repository_storages # rubocop:disable Cop/ActiverecordSerialize
serialize :sidekiq_throttling_queues, Array # rubocop:disable Cop/ActiverecordSerialize
cache_markdown_field :sign_in_text
cache_markdown_field :help_page_text

View file

@ -1,5 +1,5 @@
class AuditEvent < ActiveRecord::Base
serialize :details, Hash
serialize :details, Hash # rubocop:disable Cop/ActiverecordSerialize
belongs_to :user, foreign_key: :author_id

View file

@ -19,8 +19,8 @@ module Ci
)
end
serialize :options
serialize :yaml_variables, Gitlab::Serializer::Ci::Variables
serialize :options # rubocop:disable Cop/ActiverecordSerialize
serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiverecordSerialize
delegate :name, to: :project, prefix: true

View file

@ -6,7 +6,7 @@ module Ci
belongs_to :pipeline, foreign_key: :commit_id
has_many :builds
serialize :variables
serialize :variables # rubocop:disable Cop/ActiverecordSerialize
def user_variables
return [] unless variables

View file

@ -6,9 +6,9 @@ class DiffNote < Note
NOTEABLE_TYPES = %w(MergeRequest Commit).freeze
serialize :original_position, Gitlab::Diff::Position
serialize :position, Gitlab::Diff::Position
serialize :change_position, Gitlab::Diff::Position
serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize
serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize
serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize
validates :original_position, presence: true
validates :position, presence: true

View file

@ -26,7 +26,7 @@ class Event < ActiveRecord::Base
belongs_to :target, polymorphic: true
# For Hash only
serialize :data
serialize :data # rubocop:disable Cop/ActiverecordSerialize
# Callbacks
after_create :reset_project_activity

View file

@ -1,9 +1,9 @@
class WebHookLog < ActiveRecord::Base
belongs_to :web_hook
serialize :request_headers, Hash
serialize :request_data, Hash
serialize :response_headers, Hash
serialize :request_headers, Hash # rubocop:disable Cop/ActiverecordSerialize
serialize :request_data, Hash # rubocop:disable Cop/ActiverecordSerialize
serialize :response_headers, Hash # rubocop:disable Cop/ActiverecordSerialize
validates :web_hook, presence: true

View file

@ -7,7 +7,7 @@
class LegacyDiffNote < Note
include NoteOnDiff
serialize :st_diff
serialize :st_diff # rubocop:disable Cop/ActiverecordSerialize
validates :line_code, presence: true, line_code: true

View file

@ -21,7 +21,7 @@ class MergeRequest < ActiveRecord::Base
belongs_to :assignee, class_name: "User"
serialize :merge_params, Hash
serialize :merge_params, Hash # rubocop:disable Cop/ActiverecordSerialize
after_create :ensure_merge_request_diff, unless: :importing?
after_update :reload_diff_if_branch_changed

View file

@ -11,8 +11,8 @@ class MergeRequestDiff < ActiveRecord::Base
belongs_to :merge_request
serialize :st_commits
serialize :st_diffs
serialize :st_commits # rubocop:disable Cop/ActiverecordSerialize
serialize :st_diffs # rubocop:disable Cop/ActiverecordSerialize
state_machine :state, initial: :empty do
state :collected

View file

@ -3,7 +3,7 @@ class PersonalAccessToken < ActiveRecord::Base
include TokenAuthenticatable
add_authentication_token_field :token
serialize :scopes, Array
serialize :scopes, Array # rubocop:disable Cop/ActiverecordSerialize
belongs_to :user

View file

@ -10,7 +10,7 @@ class ProjectImportData < ActiveRecord::Base
insecure_mode: true,
algorithm: 'aes-256-cbc'
serialize :data, JSON
serialize :data, JSON # rubocop:disable Cop/ActiverecordSerialize
validates :project, presence: true

View file

@ -1,5 +1,5 @@
class SentNotification < ActiveRecord::Base
serialize :position, Gitlab::Diff::Position
serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiverecordSerialize
belongs_to :project
belongs_to :noteable, polymorphic: true

View file

@ -2,7 +2,7 @@
# and implement a set of methods
class Service < ActiveRecord::Base
include Sortable
serialize :properties, JSON
serialize :properties, JSON # rubocop:disable Cop/ActiverecordSerialize
default_value_for :active, false
default_value_for :push_events, true

View file

@ -40,7 +40,7 @@ class User < ActiveRecord::Base
otp_secret_encryption_key: Gitlab::Application.secrets.otp_key_base
devise :two_factor_backupable, otp_number_of_backup_codes: 10
serialize :otp_backup_codes, JSON
serialize :otp_backup_codes, JSON # rubocop:disable Cop/ActiverecordSerialize
devise :lockable, :recoverable, :rememberable, :trackable,
:validatable, :omniauthable, :confirmable, :registerable

View file

@ -0,0 +1,24 @@
module RuboCop
module Cop
# Cop that prevents the use of `serialize` in ActiveRecord models.
class ActiverecordSerialize < RuboCop::Cop::Cop
MSG = 'Do not store serialized data in the database, use separate columns and/or tables instead'.freeze
def on_send(node)
return unless in_models?(node)
add_offense(node, :selector) if node.children[1] == :serialize
end
def models_path
File.join(Dir.pwd, 'app', 'models')
end
def in_models?(node)
path = node.location.expression.source_buffer.name
path.start_with?(models_path)
end
end
end
end

View file

@ -1,5 +1,6 @@
require_relative 'cop/custom_error_class'
require_relative 'cop/gem_fetcher'
require_relative 'cop/activerecord_serialize'
require_relative 'cop/migration/add_column'
require_relative 'cop/migration/add_column_with_default_to_large_table'
require_relative 'cop/migration/add_concurrent_foreign_key'

View file

@ -0,0 +1,33 @@
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/activerecord_serialize'
describe RuboCop::Cop::ActiverecordSerialize do
include CopHelper
subject(:cop) { described_class.new }
context 'inside the app/models directory' do
it 'registers an offense when serialize is used' do
allow(cop).to receive(:in_models?).and_return(true)
inspect_source(cop, 'serialize :foo')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
end
end
end
context 'outside the app/models directory' do
it 'does nothing' do
allow(cop).to receive(:in_models?).and_return(false)
inspect_source(cop, 'serialize :foo')
expect(cop.offenses).to be_empty
end
end
end