diff options
author | Robert Speicher <robert@gitlab.com> | 2018-09-14 17:38:29 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2018-09-14 17:38:29 +0000 |
commit | 165e9a99c0363934e7676f3968e974af73c6cf95 (patch) | |
tree | ddeb4bbd45faaf3a8cd3efc8e28dd22c55a6f617 | |
parent | 69925767b7ac49b59c0292608b6655edb117e31d (diff) | |
parent | ebe89e8bd259a848d737b33a793298220054a023 (diff) | |
download | gitlab-ce-165e9a99c0363934e7676f3968e974af73c6cf95.tar.gz |
Merge branch '50824-fix-prepend-concern' into 'master'
CE: Properly implement prepending for Concern
See merge request gitlab-org/gitlab-ce!21444
-rw-r--r-- | app/models/concerns/protected_branch_access.rb | 11 | ||||
-rw-r--r-- | app/models/concerns/protected_tag_access.rb | 3 | ||||
-rw-r--r-- | config/initializers/0_as_concern.rb | 22 | ||||
-rw-r--r-- | lib/gitlab/patch/prependable.rb | 65 | ||||
-rw-r--r-- | lib/gitlab/utils/override.rb | 12 | ||||
-rw-r--r-- | spec/lib/gitlab/patch/prependable_spec.rb | 234 | ||||
-rw-r--r-- | spec/support/helpers/stub_configuration.rb | 3 |
7 files changed, 318 insertions, 32 deletions
diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb index 744f7f48dc8..58761fce952 100644 --- a/app/models/concerns/protected_branch_access.rb +++ b/app/models/concerns/protected_branch_access.rb @@ -2,18 +2,17 @@ module ProtectedBranchAccess extend ActiveSupport::Concern + include ProtectedRefAccess included do - include ProtectedRefAccess - belongs_to :protected_branch delegate :project, to: :protected_branch + end - def check_access(user) - return false if access_level == Gitlab::Access::NO_ACCESS + def check_access(user) + return false if access_level == Gitlab::Access::NO_ACCESS - super - end + super end end diff --git a/app/models/concerns/protected_tag_access.rb b/app/models/concerns/protected_tag_access.rb index 04bd54d6b1c..3f5696c0749 100644 --- a/app/models/concerns/protected_tag_access.rb +++ b/app/models/concerns/protected_tag_access.rb @@ -2,10 +2,9 @@ module ProtectedTagAccess extend ActiveSupport::Concern + include ProtectedRefAccess included do - include ProtectedRefAccess - belongs_to :protected_tag delegate :project, to: :protected_tag diff --git a/config/initializers/0_as_concern.rb b/config/initializers/0_as_concern.rb index 40232bd6252..ff132547225 100644 --- a/config/initializers/0_as_concern.rb +++ b/config/initializers/0_as_concern.rb @@ -1,25 +1,7 @@ -# This module is based on: https://gist.github.com/bcardarella/5735987 - -module Prependable - def prepend_features(base) - if base.instance_variable_defined?(:@_dependencies) - base.instance_variable_get(:@_dependencies) << self - false - else - return false if base < self - - super - base.singleton_class.send(:prepend, const_get('ClassMethods')) if const_defined?(:ClassMethods) - @_dependencies.each { |dep| base.send(:prepend, dep) } # rubocop:disable Gitlab/ModuleWithInstanceVariables - base.class_eval(&@_included_block) if instance_variable_defined?(:@_included_block) # rubocop:disable Gitlab/ModuleWithInstanceVariables - end - end -end +# frozen_string_literal: true module ActiveSupport module Concern - prepend Prependable - - alias_method :prepended, :included + prepend Gitlab::Patch::Prependable end end diff --git a/lib/gitlab/patch/prependable.rb b/lib/gitlab/patch/prependable.rb new file mode 100644 index 00000000000..a9f6cfb19cb --- /dev/null +++ b/lib/gitlab/patch/prependable.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +# We're patching `ActiveSupport::Concern` in +# config/initializers/0_as_concern.rb +# +# We want to patch `ActiveSupport::Concern` for two reasons: +# 1. Allow defining class methods via: `class_methods` method +# 2. Allow `prepended do; end` work like `included do; end` +# If we don't need anything above, we don't need this patch nor the concern! + +# rubocop:disable Gitlab/ModuleWithInstanceVariables +module Gitlab + module Patch + module Prependable + class MultiplePrependedBlocks < StandardError + def initialize + super "Cannot define multiple 'prepended' blocks for a Concern" + end + end + + def prepend_features(base) + return false if prepended?(base) + + super + + if const_defined?(:ClassMethods) + klass_methods = const_get(:ClassMethods) + base.singleton_class.prepend klass_methods + base.instance_variable_set(:@_prepended_class_methods, klass_methods) + end + + if instance_variable_defined?(:@_prepended_block) + base.class_eval(&@_prepended_block) + end + + true + end + + def class_methods + super + + if instance_variable_defined?(:@_prepended_class_methods) + const_get(:ClassMethods).prepend @_prepended_class_methods + end + end + + def prepended(base = nil, &block) + if base.nil? + raise MultiplePrependedBlocks if + instance_variable_defined?(:@_prepended_block) + + @_prepended_block = block + else + super + end + end + + def prepended?(base) + index = base.ancestors.index(base) + + base.ancestors[0...index].index(self) + end + end + end +end diff --git a/lib/gitlab/utils/override.rb b/lib/gitlab/utils/override.rb index 7b2a62fed48..d00921e6cdc 100644 --- a/lib/gitlab/utils/override.rb +++ b/lib/gitlab/utils/override.rb @@ -89,15 +89,19 @@ module Gitlab def included(base = nil) super - queue_verification(base) + queue_verification(base) if base end - alias_method :prepended, :included + def prepended(base = nil) + super + + queue_verification(base) if base + end - def extended(mod) + def extended(mod = nil) super - queue_verification(mod.singleton_class) + queue_verification(mod.singleton_class) if mod end def queue_verification(base) diff --git a/spec/lib/gitlab/patch/prependable_spec.rb b/spec/lib/gitlab/patch/prependable_spec.rb new file mode 100644 index 00000000000..725d733d176 --- /dev/null +++ b/spec/lib/gitlab/patch/prependable_spec.rb @@ -0,0 +1,234 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +# Patching ActiveSupport::Concern +require_relative '../../../../config/initializers/0_as_concern' + +describe Gitlab::Patch::Prependable do + before do + @prepended_modules = [] + end + + let(:ee) do + # So that block in Module.new could see them + prepended_modules = @prepended_modules + + Module.new do + extend ActiveSupport::Concern + + class_methods do + def class_name + super.tr('C', 'E') + end + end + + this = self + prepended do + prepended_modules << [self, this] + end + + def name + super.tr('c', 'e') + end + end + end + + let(:ce) do + # So that block in Module.new could see them + prepended_modules = @prepended_modules + ee_ = ee + + Module.new do + extend ActiveSupport::Concern + prepend ee_ + + class_methods do + def class_name + 'CE' + end + end + + this = self + prepended do + prepended_modules << [self, this] + end + + def name + 'ce' + end + end + end + + describe 'a class including a concern prepending a concern' do + subject { Class.new.include(ce) } + + it 'returns values from prepended module ee' do + expect(subject.new.name).to eq('ee') + expect(subject.class_name).to eq('EE') + end + + it 'has the expected ancestors' do + expect(subject.ancestors.take(3)).to eq([subject, ee, ce]) + expect(subject.singleton_class.ancestors.take(3)) + .to eq([subject.singleton_class, + ee.const_get(:ClassMethods), + ce.const_get(:ClassMethods)]) + end + + it 'prepends only once even if called twice' do + 2.times { ce.prepend(ee) } + + subject + + expect(@prepended_modules).to eq([[ce, ee]]) + end + + context 'overriding methods' do + before do + subject.module_eval do + def self.class_name + 'Custom' + end + + def name + 'custom' + end + end + end + + it 'returns values from the class' do + expect(subject.new.name).to eq('custom') + expect(subject.class_name).to eq('Custom') + end + end + end + + describe 'a class prepending a concern prepending a concern' do + subject { Class.new.prepend(ce) } + + it 'returns values from prepended module ee' do + expect(subject.new.name).to eq('ee') + expect(subject.class_name).to eq('EE') + end + + it 'has the expected ancestors' do + expect(subject.ancestors.take(3)).to eq([ee, ce, subject]) + expect(subject.singleton_class.ancestors.take(3)) + .to eq([ee.const_get(:ClassMethods), + ce.const_get(:ClassMethods), + subject.singleton_class]) + end + + it 'prepends only once' do + subject.prepend(ce) + + expect(@prepended_modules).to eq([[ce, ee], [subject, ce]]) + end + end + + describe 'a class prepending a concern' do + subject do + ee_ = ee + + Class.new do + prepend ee_ + + def self.class_name + 'CE' + end + + def name + 'ce' + end + end + end + + it 'returns values from prepended module ee' do + expect(subject.new.name).to eq('ee') + expect(subject.class_name).to eq('EE') + end + + it 'has the expected ancestors' do + expect(subject.ancestors.take(2)).to eq([ee, subject]) + expect(subject.singleton_class.ancestors.take(2)) + .to eq([ee.const_get(:ClassMethods), + subject.singleton_class]) + end + + it 'prepends only once' do + subject.prepend(ee) + + expect(@prepended_modules).to eq([[subject, ee]]) + end + end + + describe 'simple case' do + subject do + foo_ = foo + + Class.new do + prepend foo_ + + def value + 10 + end + end + end + + let(:foo) do + Module.new do + extend ActiveSupport::Concern + + prepended do + def self.class_value + 20 + end + end + + def value + super * 10 + end + end + end + + context 'class methods' do + it "has a method" do + expect(subject).to respond_to(:class_value) + end + + it 'can execute a method' do + expect(subject.class_value).to eq(20) + end + end + + context 'instance methods' do + it "has a method" do + expect(subject.new).to respond_to(:value) + end + + it 'chains a method execution' do + expect(subject.new.value).to eq(100) + end + end + end + + context 'having two prepended blocks' do + subject do + Module.new do + extend ActiveSupport::Concern + + prepended do + end + + prepended do + end + end + end + + it "raises an error" do + expect { subject } + .to raise_error(described_class::MultiplePrependedBlocks) + end + end +end diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index 8475f91799b..776119564ec 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -1,5 +1,8 @@ require 'active_support/core_ext/hash/transform_values' require 'active_support/hash_with_indifferent_access' +require 'active_support/dependencies' + +require_dependency 'gitlab' module StubConfiguration def stub_application_setting(messages) |