Merge branch 'blackst0ne-rails5-avoid-type-casting-in-uniqueness-validator' into 'master'

[Rails5] Avoid type casting in uniqueness validator

See merge request gitlab-org/gitlab-ce!18470

Gemfile.rails5.lock

@@ -69,7 +69,7 @@ GEM
       unf
     ast (2.4.0)
     atomic (1.1.100)
-    attr_encrypted (3.0.3)
+    attr_encrypted (3.1.0)
       encryptor (~> 3.0.0)
     attr_required (1.0.1)
     autoprefixer-rails (8.1.0.1)
@@ -291,9 +291,9 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.94.0)
+    gitaly-proto (0.97.0)
       google-protobuf (~> 3.1)
-      grpc (~> 1.0)
+      grpc (~> 1.10)
     github-linguist (5.3.3)
       charlock_holmes (~> 0.7.5)
       escape_utils (~> 1.1.0)
@@ -304,6 +304,17 @@ GEM
       flowdock (~> 0.7)
       gitlab-grit (>= 2.4.1)
       multi_json
+    gitlab-gollum-lib (4.2.7.1)
+      gemojione (~> 3.2)
+      github-markup (~> 1.6)
+      gollum-grit_adapter (~> 1.0)
+      nokogiri (>= 1.6.1, < 2.0)
+      rouge (~> 2.1)
+      sanitize (~> 2.1)
+      stringex (~> 2.6)
+    gitlab-gollum-rugged_adapter (0.4.4)
+      mime-types (>= 1.15)
+      rugged (~> 0.25)
     gitlab-grit (2.8.2)
       charlock_holmes (~> 0.6)
       diff-lcs (~> 1.1)
@@ -321,22 +332,8 @@ GEM
       rubyntlm (~> 0.5)
     globalid (0.4.1)
       activesupport (>= 4.2.0)
-    goldiloader (2.0.1)
-      activerecord (>= 4.2, < 5.2)
-      activesupport (>= 4.2, < 5.2)
     gollum-grit_adapter (1.0.1)
       gitlab-grit (~> 2.7, >= 2.7.1)
-    gollum-lib (4.2.7)
-      gemojione (~> 3.2)
-      github-markup (~> 1.6)
-      gollum-grit_adapter (~> 1.0)
-      nokogiri (>= 1.6.1, < 2.0)
-      rouge (~> 2.1)
-      sanitize (~> 2.1)
-      stringex (~> 2.6)
-    gollum-rugged_adapter (0.4.4)
-      mime-types (>= 1.15)
-      rugged (~> 0.25)
     gon (6.1.0)
       actionpack (>= 3.0)
       json
@@ -1009,7 +1006,7 @@ DEPENDENCIES
   asciidoctor (~> 1.5.6)
   asciidoctor-plantuml (= 0.0.8)
   asset_sync (~> 2.2.0)
-  attr_encrypted (~> 3.0.0)
+  attr_encrypted (~> 3.1.0)
   awesome_print (~> 1.2.0)
   babosa (~> 1.0.2)
   base32 (~> 0.3.0)
@@ -1069,15 +1066,14 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.94.0)
+  gitaly-proto (~> 0.97.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
+  gitlab-gollum-lib (~> 4.2)
+  gitlab-gollum-rugged_adapter (~> 0.4.4)
   gitlab-markup (~> 1.6.2)
   gitlab-styles (~> 2.3)
   gitlab_omniauth-ldap (~> 2.0.4)
-  goldiloader (~> 2.0)
-  gollum-lib (~> 4.2)
-  gollum-rugged_adapter (~> 0.4.4)
   gon (~> 6.1.0)
   google-api-client (~> 0.19.8)
   google-protobuf (= 3.5.1)

config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb

+# This is a monkey patch which must be removed when migrating to Rails 5.1 from 5.0.
+#
+# In Rails 5.0 there was introduced a bug which casts types in the uniqueness validator.
+# https://github.com/rails/rails/pull/23523/commits/811a4fa8eb6ceea841e61e8ac05747ffb69595ae
+#
+# That causes to bugs like this:
+#
+#     1) API::Users POST /user/:id/gpg_keys/:key_id/revoke when authenticated revokes existing key
+#     Failure/Error: let(:gpg_key) { create(:gpg_key, user: user) }
+#
+#     TypeError:
+#       can't cast Hash
+#     # ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in <top (required)>'
+#     # ./spec/requests/api/users_spec.rb:908:in `block (4 levels) in <top (required)>'
+#     # ------------------
+#     # --- Caused by: ---
+#     # TypeError:
+#     #   TypeError
+#     #   ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in <top (required)>'
+#
+# This bug was fixed in Rails 5.1 by https://github.com/rails/rails/pull/24745/commits/aa062318c451512035c10898a1af95943b1a3803
+
+if Gitlab.rails5?
+  ActiveSupport::Deprecation.warn("#{__FILE__} is a monkey patch which must be removed when upgrading to Rails 5.1")
+
+  if Rails.version.start_with?("5.1")
+    raise "Remove this monkey patch: #{__FILE__}"
+  end
+
+  # Copy-paste from https://github.com/kamipo/rails/blob/aa062318c451512035c10898a1af95943b1a3803/activerecord/lib/active_record/validations/uniqueness.rb
+  # including local fixes to make Rubocop happy again.
+  module ActiveRecord
+    module Validations
+      class UniquenessValidator < ActiveModel::EachValidator # :nodoc:
+        def validate_each(record, attribute, value)
+          finder_class = find_finder_class_for(record)
+          table = finder_class.arel_table
+          value = map_enum_attribute(finder_class, attribute, value)
+
+          relation = build_relation(finder_class, table, attribute, value)
+
+          if record.persisted?
+            if finder_class.primary_key
+              relation = relation.where.not(finder_class.primary_key => record.id_was || record.id)
+            else
+              raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.")
+            end
+          end
+
+          relation = scope_relation(record, table, relation)
+          relation = relation.merge(options[:conditions]) if options[:conditions]
+
+          if relation.exists?
+            error_options = options.except(:case_sensitive, :scope, :conditions)
+            error_options[:value] = value
+
+            record.errors.add(attribute, :taken, error_options)
+          end
+        rescue RangeError
+        end
+
+        protected
+
+        def build_relation(klass, table, attribute, value) #:nodoc:
+          if reflection = klass._reflect_on_association(attribute)
+            attribute = reflection.foreign_key
+            value = value.attributes[reflection.klass.primary_key] unless value.nil?
+          end
+
+          # the attribute may be an aliased attribute
+          if klass.attribute_alias?(attribute)
+            attribute = klass.attribute_alias(attribute)
+          end
+
+          attribute_name = attribute.to_s
+
+          column = klass.columns_hash[attribute_name]
+          cast_type = klass.type_for_attribute(attribute_name)
+
+          comparison =
+            if !options[:case_sensitive] && !value.nil?
+              # will use SQL LOWER function before comparison, unless it detects a case insensitive collation
+              klass.connection.case_insensitive_comparison(table, attribute, column, value)
+            else
+              klass.connection.case_sensitive_comparison(table, attribute, column, value)
+            end
+
+          if value.nil?
+            klass.unscoped.where(comparison)
+          else
+            bind = Relation::QueryAttribute.new(attribute_name, value, cast_type)
+            klass.unscoped.where(comparison, bind)
+          end
+        end
+      end
+    end
+  end
+end