diff options
6 files changed, 66 insertions, 34 deletions
diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index b75387e236e..1c2e57bb01f 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -17,7 +17,7 @@ class MergeRequestDiffCommit < ActiveRecord::Base commit_hash.merge( merge_request_diff_id: merge_request_diff_id, relative_order: index, - sha: sha_attribute.type_cast_for_database(sha), + sha: sha_attribute.serialize(sha), # rubocop:disable Cop/ActiveRecordSerialize authored_date: Gitlab::Database.sanitize_timestamp(commit_hash[:authored_date]), committed_date: Gitlab::Database.sanitize_timestamp(commit_hash[:committed_date]) ) diff --git a/config/initializers/active_record_array_type_casting.rb b/config/initializers/active_record_array_type_casting.rb index d94d592add6..a149e048ee2 100644 --- a/config/initializers/active_record_array_type_casting.rb +++ b/config/initializers/active_record_array_type_casting.rb @@ -1,20 +1,23 @@ -module ActiveRecord - class PredicateBuilder - class ArrayHandler - module TypeCasting - def call(attribute, value) - # This is necessary because by default ActiveRecord does not respect - # custom type definitions (like our `ShaAttribute`) when providing an - # array in `where`, like in `where(commit_sha: [sha1, sha2, sha3])`. - model = attribute.relation&.engine - type = model.user_provided_columns[attribute.name] if model - value = value.map { |value| type.type_cast_for_database(value) } if type +# Remove this initializer when upgraded to Rails 5.0 +unless Gitlab.rails5? + module ActiveRecord + class PredicateBuilder + class ArrayHandler + module TypeCasting + def call(attribute, value) + # This is necessary because by default ActiveRecord does not respect + # custom type definitions (like our `ShaAttribute`) when providing an + # array in `where`, like in `where(commit_sha: [sha1, sha2, sha3])`. + model = attribute.relation&.engine + type = model.user_provided_columns[attribute.name] if model + value = value.map { |value| type.type_cast_for_database(value) } if type - super(attribute, value) + super(attribute, value) + end end - end - prepend TypeCasting + prepend TypeCasting + end end end end diff --git a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb index fd5cbf76e47..a357538a885 100644 --- a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb +++ b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb @@ -96,7 +96,7 @@ module Gitlab commit_hash.merge( merge_request_diff_id: merge_request_diff.id, relative_order: index, - sha: sha_attribute.type_cast_for_database(sha) + sha: sha_attribute.serialize(sha) ) end diff --git a/lib/gitlab/database/sha_attribute.rb b/lib/gitlab/database/sha_attribute.rb index d9400e04b83..b2d8ee81977 100644 --- a/lib/gitlab/database/sha_attribute.rb +++ b/lib/gitlab/database/sha_attribute.rb @@ -1,12 +1,20 @@ module Gitlab module Database - BINARY_TYPE = if Gitlab::Database.postgresql? - # PostgreSQL defines its own class with slightly different - # behaviour from the default Binary type. - ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Bytea - else - ActiveRecord::Type::Binary - end + BINARY_TYPE = + if Gitlab::Database.postgresql? + # PostgreSQL defines its own class with slightly different + # behaviour from the default Binary type. + ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Bytea + else + # In Rails 5.0 `Type` has been moved from `ActiveRecord` to `ActiveModel` + # https://github.com/rails/rails/commit/9cc8c6f3730df3d94c81a55be9ee1b7b4ffd29f6#diff-f8ba7983a51d687976e115adcd95822b + # Remove this method and leave just `ActiveModel::Type::Binary` when removing Gitlab.rails5? code. + if Gitlab.rails5? + ActiveModel::Type::Binary + else + ActiveRecord::Type::Binary + end + end # Class for casting binary data to hexadecimal SHA1 hashes (and vice-versa). # @@ -16,18 +24,39 @@ module Gitlab class ShaAttribute < BINARY_TYPE PACK_FORMAT = 'H*'.freeze - # Casts binary data to a SHA1 in hexadecimal. + # It is called from activerecord-4.2.10/lib/active_record internal methods. + # Remove this method when removing Gitlab.rails5? code. def type_cast_from_database(value) - value = super + unpack_sha(super) + end + + # It is called from activerecord-4.2.10/lib/active_record internal methods. + # Remove this method when removing Gitlab.rails5? code. + def type_cast_for_database(value) + serialize(value) + end + # It is called from activerecord-5.0.6/lib/active_record/attribute.rb + # Remove this method when removing Gitlab.rails5? code.. + def deserialize(value) + value = Gitlab.rails5? ? super : method(:type_cast_from_database).super_method.call(value) + + unpack_sha(value) + end + + # Rename this method to `deserialize(value)` removing Gitlab.rails5? code. + # Casts binary data to a SHA1 in hexadecimal. + def unpack_sha(value) + # Uncomment this line when removing Gitlab.rails5? code. + # value = super value ? value.unpack(PACK_FORMAT)[0] : nil end # Casts a SHA1 in hexadecimal to the proper binary format. - def type_cast_for_database(value) + def serialize(value) arg = value ? [value].pack(PACK_FORMAT) : nil - super(arg) + Gitlab.rails5? ? super(arg) : method(:type_cast_for_database).super_method.call(arg) end end end diff --git a/spec/lib/gitlab/database/sha_attribute_spec.rb b/spec/lib/gitlab/database/sha_attribute_spec.rb index 62c1d37ea1c..778bfa2cc47 100644 --- a/spec/lib/gitlab/database/sha_attribute_spec.rb +++ b/spec/lib/gitlab/database/sha_attribute_spec.rb @@ -19,15 +19,15 @@ describe Gitlab::Database::ShaAttribute do let(:attribute) { described_class.new } - describe '#type_cast_from_database' do + describe '#deserialize' do it 'converts the binary SHA to a String' do - expect(attribute.type_cast_from_database(binary_from_db)).to eq(sha) + expect(attribute.deserialize(binary_from_db)).to eq(sha) end end - describe '#type_cast_for_database' do + describe '#serialize' do it 'converts a SHA String to binary data' do - expect(attribute.type_cast_for_database(sha).to_s).to eq(binary_sha) + expect(attribute.serialize(sha).to_s).to eq(binary_sha) end end end diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb index 7709cf43200..8c01a7ac18f 100644 --- a/spec/models/merge_request_diff_commit_spec.rb +++ b/spec/models/merge_request_diff_commit_spec.rb @@ -36,7 +36,7 @@ describe MergeRequestDiffCommit do "committer_email": "dmitriy.zaporozhets@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, - "sha": sha_attribute.type_cast_for_database('5937ac0a7beb003549fc5fd26fc247adbce4a52e') + "sha": sha_attribute.serialize("5937ac0a7beb003549fc5fd26fc247adbce4a52e") }, { "message": "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n", @@ -48,7 +48,7 @@ describe MergeRequestDiffCommit do "committer_email": "dmitriy.zaporozhets@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 1, - "sha": sha_attribute.type_cast_for_database('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + "sha": sha_attribute.serialize("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } ] end @@ -79,7 +79,7 @@ describe MergeRequestDiffCommit do "committer_email": "alejorro70@gmail.com", "merge_request_diff_id": merge_request_diff_id, "relative_order": 0, - "sha": sha_attribute.type_cast_for_database('ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69') + "sha": sha_attribute.serialize("ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69") }] end |