summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2018-04-06 10:57:19 +0000
committerSean McGivern <sean@mcgivern.me.uk>2018-04-06 10:57:19 +0000
commit41ee09c9a06b41d904977def6e7824928f27596a (patch)
treeb7e8afd7953b5c92cbcee71d5c1fcef2178d0a31
parent52b232ca5d18ffc08299f5bb0ef3be46d2fecca3 (diff)
parent20695052db6860620365af9b86ab301759d1405d (diff)
downloadgitlab-ce-41ee09c9a06b41d904977def6e7824928f27596a.tar.gz
Merge branch 'blackst0ne-rails5-fix-type-cast-for-from-database' into 'master'blackst0ne-rails5-update-skip-befor-action-in-authenticates-with-two-factor-concern
[Rails5] Update `type_cast_*_database` methods See merge request gitlab-org/gitlab-ce!18188
-rw-r--r--app/models/merge_request_diff_commit.rb2
-rw-r--r--config/initializers/active_record_array_type_casting.rb31
-rw-r--r--lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb2
-rw-r--r--lib/gitlab/database/sha_attribute.rb51
-rw-r--r--spec/lib/gitlab/database/sha_attribute_spec.rb8
-rw-r--r--spec/models/merge_request_diff_commit_spec.rb6
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