GitLab wurde aktualisiert. Dank regelmäßiger Updates bleibt das THM GitLab sicher und Sie profitieren von den neuesten Funktionen. Vielen Dank für Ihre Geduld.

Commit 9d742e61 authored by Duana Saskia's avatar Duana Saskia
Browse files

Refactor: move active hook filter to TriggerableHooks

parent c3229760
......@@ -28,6 +28,12 @@ def hooks_for(trigger)
public_send(trigger) # rubocop:disable GitlabSecurity/PublicSend
end
def select_active(hooks_scope, data)
select do |hook|
ActiveHookFilter.new(hook).matches?(hooks_scope, data)
end
end
private
def triggerable_hooks(hooks)
......
......@@ -1163,7 +1163,7 @@ def owner
def execute_hooks(data, hooks_scope = :push_hooks)
run_after_commit_or_now do
hooks.hooks_for(hooks_scope).select {|hook| ActiveHookFilter.new(hook).matches?(hooks_scope, data)}.each do |hook|
hooks.hooks_for(hooks_scope).select_active(hooks_scope, data).each do |hook|
hook.async_execute(data, hooks_scope.to_s)
end
SystemHooksService.new.execute_hooks(data, hooks_scope)
......
......@@ -16,6 +16,7 @@ def validate_each(record, attribute, value)
if value.present?
value_without_wildcards = value.tr('*', 'x')
unless Gitlab::GitRefValidator.validate(value_without_wildcards)
record.errors[attribute] << "is not a valid branch name"
end
......
......@@ -40,4 +40,28 @@ class TestableHook < WebHook
end
end
end
describe '.select_active' do
it 'returns hooks that match the active filter' do
TestableHook.create!(url: 'http://example1.com', push_events: true)
TestableHook.create!(url: 'http://example2.com', push_events: true)
filter1 = double(:filter1)
filter2 = double(:filter2)
allow(ActiveHookFilter).to receive(:new).exactly(2).times.and_return(filter1, filter2)
expect(filter1).to receive(:matches?).and_return(true)
expect(filter2).to receive(:matches?).and_return(false)
hooks = TestableHook.push_hooks.order_id_asc
expect(hooks.select_active(:push_hooks, {})).to eq [hooks.first]
end
it 'returns empty list if no hooks match the active filter' do
TestableHook.create!(url: 'http://example1.com', push_events: true)
filter = double(:filter)
allow(ActiveHookFilter).to receive(:new).and_return(filter)
expect(filter).to receive(:matches?).and_return(false)
expect(TestableHook.push_hooks.select_active(:push_hooks, {})).to eq []
end
end
end
......@@ -11,28 +11,29 @@
context 'branch filter is specified' do
let(:branch_filter) { 'master' }
it 'returns true if branch matches' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
end
it 'returns false if branch does not match' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to eq(false)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false
end
it 'returns false if ref is nil' do
expect(filter.matches?(:push_hooks, {})).to eq(false)
expect(filter.matches?(:push_hooks, {})).to be false
end
context 'branch filter contains wildcard' do
let(:branch_filter) { 'features/*' }
it 'returns true if branch matches' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to eq(true)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to eq(true)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to be true
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to be true
end
it 'returns false if branch does not match' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(false)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false
end
end
end
......@@ -41,7 +42,7 @@
let(:branch_filter) { nil }
it 'returns true' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
end
end
......@@ -49,7 +50,7 @@
let(:branch_filter) { '' }
it 'acts like branch is not specified' do
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
end
end
end
......@@ -60,7 +61,7 @@
end
it 'returns true as branch filters are not yet supported for these' do
expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to eq(true)
expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true
end
end
end
......
......@@ -3672,36 +3672,36 @@ def domain_variable
describe '#execute_hooks' do
let(:data) { { ref: 'refs/heads/master', data: 'data' } }
it 'executes active projects hooks with the specified scope' do
expect_any_instance_of(ActiveHookFilter).to receive(:matches?)
.with(:tag_push_hooks, data)
.and_return(true)
hook = create(:project_hook, merge_requests_events: false, tag_push_events: true)
hook = create(:project_hook, merge_requests_events: false, push_events: true)
expect(ProjectHook).to receive(:select_active)
.with(:push_hooks, data)
.and_return([hook])
project = create(:project, hooks: [hook])
expect_any_instance_of(ProjectHook).to receive(:async_execute).once
project.execute_hooks(data, :tag_push_hooks)
project.execute_hooks(data, :push_hooks)
end
it 'does not execute project hooks that dont match the specified scope' do
hook = create(:project_hook, merge_requests_events: true, tag_push_events: false)
hook = create(:project_hook, merge_requests_events: true, push_events: false)
project = create(:project, hooks: [hook])
expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
project.execute_hooks(data, :tag_push_hooks)
project.execute_hooks(data, :push_hooks)
end
it 'does not execute project hooks which are not active' do
expect_any_instance_of(ActiveHookFilter).to receive(:matches?)
.with(:tag_push_hooks, data)
.and_return(false)
hook = create(:project_hook, tag_push_events: true)
hook = create(:project_hook, push_events: true)
expect(ProjectHook).to receive(:select_active)
.with(:push_hooks, data)
.and_return([])
project = create(:project, hooks: [hook])
expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
project.execute_hooks(data, :tag_push_hooks)
project.execute_hooks(data, :push_hooks)
end
it 'executes the system hooks with the specified scope' do
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment