Skip to content

Bring back GitLab attr_encrypted tests

Matthias Käppler requested to merge 372221-attr-encrypted-tests into master

What does this MR do and why?

Closes #375712 (closed)

When we vendored attr_encrypted in !98528 (merged), some tests for monkey-patches we had removed or rolled into the library were also removed:

  • spec/initializers/attr_encrypted_no_db_connection_spec.rb
  • spec/initializers/attr_encrypted_thread_safe_spec.rb

The latter had already been fixed upstream, and tests were added by the library authors. But the former issue is unresolved in the upstream project so we had to fix and test this ourselves. This MR brings back these tests, which is to make sure that encrypted fields that are not mapped to a DB column should not have synthesized methods.

I had to rewrite the original tests using MiniTest but I hope nothing got lost in translation; here is the original spec for reference:

# frozen_string_literal: true

require 'spec_helper'

RSpec.describe 'GitLab monkey-patches to AttrEncrypted' do
  describe '#attribute_instance_methods_as_symbols_available?' do
    let(:klass) do
      Class.new(ActiveRecord::Base) do
        # We need some sort of table to work on
        self.table_name = 'projects'

        attr_encrypted :foo
      end
    end

    it 'returns false' do
      expect(ActiveRecord::Base.__send__(:attribute_instance_methods_as_symbols_available?)).to be_falsy
    end

    it 'does not define virtual attributes' do
      instance = klass.new

      aggregate_failures do
        %w[
          encrypted_foo encrypted_foo=
          encrypted_foo_iv encrypted_foo_iv=
          encrypted_foo_salt encrypted_foo_salt=
        ].each do |method_name|
          expect(instance).not_to respond_to(method_name)
        end
      end
    end

    it 'calls attr_changed? method with kwargs' do
      obj = klass.new

      expect(obj.foo_changed?).to eq(false)
    end
  end
end

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #372221 (closed)

Edited by Matthias Käppler

Merge request reports

Loading