Add latest changes from gitlab-org/gitlab@master

10 files changed, 106 insertions, 78 deletions

diff --git a/app/models/ci/secure_file.rb b/app/models/ci/secure_file.rb
index df38398e5a9..1e6c48bbef5 100644
--- a/app/models/ci/secure_file.rb
+++ b/app/models/ci/secure_file.rb
@@ -17,20 +17,19 @@ module Ci
     validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT }
     validates :checksum, :file_store, :name, :project_id, presence: true
     validates :name, uniqueness: { scope: :project }
+
+    attribute :metadata, :ind_jsonb
     validates :metadata, json_schema: { filename: "ci_secure_file_metadata" }, allow_nil: true
 
+    attribute :file_store, default: -> { Ci::SecureFileUploader.default_store }
+    mount_file_store_uploader Ci::SecureFileUploader
+
     after_initialize :generate_key_data
     before_validation :assign_checksum
 
     scope :order_by_created_at, -> { order(created_at: :desc) }
     scope :project_id_in, ->(ids) { where(project_id: ids) }
 
-    serialize :metadata, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
-
-    attribute :file_store, default: -> { Ci::SecureFileUploader.default_store }
-
-    mount_file_store_uploader Ci::SecureFileUploader
-
     def checksum_algorithm
       CHECKSUM_ALGORITHM
     end
diff --git a/app/models/merge_request_context_commit.rb b/app/models/merge_request_context_commit.rb
index ebbdecf8aa7..281e11c7c13 100644
--- a/app/models/merge_request_context_commit.rb
+++ b/app/models/merge_request_context_commit.rb
@@ -12,7 +12,7 @@ class MergeRequestContextCommit < ApplicationRecord
   validates :sha, presence: true
   validates :sha, uniqueness: { message: 'has already been added' }
 
-  serialize :trailers, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
+  attribute :trailers, :ind_jsonb
   validates :trailers, json_schema: { filename: 'git_trailers' }
 
   # Sort by committed date in descending order to ensure latest commits comes on the top
diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb
index 152fb195c97..7e2efa2049b 100644
--- a/app/models/merge_request_diff_commit.rb
+++ b/app/models/merge_request_diff_commit.rb
@@ -35,7 +35,7 @@ class MergeRequestDiffCommit < ApplicationRecord
   sha_attribute :sha
   alias_attribute :id, :sha
 
-  serialize :trailers, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
+  attribute :trailers, :ind_jsonb
   validates :trailers, json_schema: { filename: 'git_trailers' }
 
   scope :with_users, -> { preload(:commit_author, :committer) }
diff --git a/config/initializers/active_record_data_types.rb b/config/initializers/active_record_data_types.rb
index 7f4bd32c221..b57d3bb5df1 100644
--- a/config/initializers/active_record_data_types.rb
+++ b/config/initializers/active_record_data_types.rb
@@ -5,6 +5,9 @@
 
 require 'active_record/connection_adapters/postgresql_adapter'
 
+ActiveRecord::Type.register(:ind_jsonb, Gitlab::Database::Type::IndifferentJsonb)
+ActiveRecord::Type.register(:sym_jsonb, Gitlab::Database::Type::SymbolizedJsonb)
+
 module ActiveRecord::ConnectionAdapters::PostgreSQL::OID
   # Add the class `DateTimeWithTimeZone` so we can map `timestamptz` to it.
   class DateTimeWithTimeZone < DateTime
diff --git a/config/initializers/types.rb b/config/initializers/types.rb
deleted file mode 100644
index 4a20e257469..00000000000
--- a/config/initializers/types.rb
+++ /dev/null
@@ -1,3 +0,0 @@
-# frozen_string_literal: true
-
-ActiveRecord::Type.register(:sym_jsonb, Gitlab::Database::Type::SymbolizedJsonb)
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index 5764c876e4d..fdd9c5f29fb 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -1120,11 +1120,11 @@ class AddOptionsToBuildMetadata < Gitlab::Database::Migration[2.0]
 end
 ```
 
-You have to use a serializer to provide a translation layer:
+By default hash keys will be strings. Optionally you can add a custom data type to provide different access to keys.
 
 ```ruby
 class BuildMetadata
-  serialize :config_options, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
+  attribute :config_options, :ind_jsonb # for indifferent accesss or :sym_jsonb if you need symbols only as keys.
 end
 ```
 
diff --git a/lib/gitlab/database/type/indifferent_jsonb.rb b/lib/gitlab/database/type/indifferent_jsonb.rb
new file mode 100644
index 00000000000..69bbcb383ba
--- /dev/null
+++ b/lib/gitlab/database/type/indifferent_jsonb.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    module Type
+      # Extends Rails' Jsonb data type to deserialize it into indifferent access Hash.
+      #
+      # Example:
+      #
+      #   class SomeModel < ApplicationRecord
+      #     # some_model.a_field is of type `jsonb`
+      #     attribute :a_field, :ind_jsonb
+      #   end
+      class IndifferentJsonb < ::ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Jsonb
+        def type
+          :ind_jsonb
+        end
+
+        def deserialize(value)
+          data = super
+          return unless data
+
+          ::Gitlab::Utils.deep_indifferent_access(data)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/serializers/json.rb b/lib/serializers/json.rb
deleted file mode 100644
index 6564f53d2da..00000000000
--- a/lib/serializers/json.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-# frozen_string_literal: true
-
-module Serializers
-  # Make the resulting hash have deep indifferent access
-  class Json
-    class << self
-      def dump(obj)
-        obj
-      end
-
-      def load(data)
-        return if data.nil?
-
-        Gitlab::Utils.deep_indifferent_access(data)
-      end
-    end
-  end
-end
diff --git a/spec/lib/gitlab/database/type/indifferent_jsonb_spec.rb b/spec/lib/gitlab/database/type/indifferent_jsonb_spec.rb
new file mode 100644
index 00000000000..6d27cbe180d
--- /dev/null
+++ b/spec/lib/gitlab/database/type/indifferent_jsonb_spec.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Database::Type::IndifferentJsonb do
+  let(:type) { described_class.new }
+
+  describe '#deserialize' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { type.deserialize(json) }
+
+    where(:json, :value) do
+      nil                                   | nil
+      '{"key":"value"}'                     | { key: 'value' }
+      '{"key":[1,2,3]}'                     | { key: [1, 2, 3] }
+      '{"key":{"subkey":"value"}}'          | { key: { subkey: 'value' } }
+      '{"key":{"a":[{"b":"c"},{"d":"e"}]}}' | { key: { a: [{ b: 'c' }, { d: 'e' }] } }
+    end
+
+    with_them do
+      it { is_expected.to match(value) }
+      it { is_expected.to match(value&.deep_stringify_keys) }
+    end
+  end
+
+  context 'when used by a model' do
+    let(:model) do
+      Class.new(ApplicationRecord) do
+        self.table_name = :_test_indifferent_jsonb
+
+        attribute :options, :ind_jsonb
+      end
+    end
+
+    let(:record) do
+      model.create!(name: 'test', options: { key: 'value' })
+    end
+
+    before do
+      model.connection.execute(<<~SQL)
+        CREATE TABLE _test_indifferent_jsonb(
+          id serial NOT NULL PRIMARY KEY,
+          name text,
+          options jsonb);
+      SQL
+
+      model.reset_column_information
+    end
+
+    it { expect(record.options).to match({ key: 'value' }) }
+    it { expect(record.options).to match({ 'key' => 'value' }) }
+
+    it 'ignores changes to other attributes' do
+      record.name = 'other test'
+
+      expect(record.changes).to match('name' => ['test', 'other test'])
+    end
+
+    it 'tracks changes to options' do
+      record.options = { key: 'other value' }
+
+      expect(record.changes).to match('options' => [{ 'key' => 'value' }, { 'key' => 'other value' }])
+    end
+  end
+end
diff --git a/spec/lib/serializers/json_spec.rb b/spec/lib/serializers/json_spec.rb
deleted file mode 100644
index 96a57cde056..00000000000
--- a/spec/lib/serializers/json_spec.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-require 'oj'
-
-RSpec.describe Serializers::Json do
-  describe '.dump' do
-    let(:obj) { { key: "value" } }
-
-    subject { described_class.dump(obj) }
-
-    it 'returns a hash' do
-      is_expected.to eq(obj)
-    end
-  end
-
-  describe '.load' do
-    let(:data_string) { '{"key":"value","variables":[{"key":"VAR1","value":"VALUE1"}]}' }
-    let(:data_hash) { Gitlab::Json.parse(data_string) }
-
-    context 'when loading a hash' do
-      subject { described_class.load(data_hash) }
-
-      it 'decodes a string' do
-        is_expected.to be_a(Hash)
-      end
-
-      it 'allows to access with symbols' do
-        expect(subject[:key]).to eq('value')
-        expect(subject[:variables].first[:key]).to eq('VAR1')
-      end
-
-      it 'allows to access with strings' do
-        expect(subject["key"]).to eq('value')
-        expect(subject["variables"].first["key"]).to eq('VAR1')
-      end
-    end
-
-    context 'when loading a nil' do
-      subject { described_class.load(nil) }
-
-      it 'returns nil' do
-        is_expected.to be_nil
-      end
-    end
-  end
-end