Commit d6ee99f3 authored by Markus Koller's avatar Markus Koller

Merge branch '290290-key-builder-class' into 'master'

Resolve "Expose webhook data for Key via its own data-builder class"

See merge request gitlab-org/gitlab!59608
parents 94ac9d12 13488d11
# frozen_string_literal: true
class SystemHooksService
BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES = [GroupMember, Group, ProjectMember, User, Project].freeze
def execute_hooks_for(model, event)
data = build_event_data(model, event)
......@@ -22,39 +20,6 @@ class SystemHooksService
private
def build_event_data(model, event)
# return entire event data from its builder class, if available.
return builder_driven_event_data(model, event) if builder_driven_event_data_available?(model)
data = {
event_name: build_event_name(model, event),
created_at: model.created_at&.xmlschema,
updated_at: model.updated_at&.xmlschema
}
case model
when Key
data.merge!(
key: model.key,
id: model.id
)
if model.user
data[:username] = model.user.username
end
end
data
end
def build_event_name(model, event)
"#{model.class.name.downcase}_#{event}"
end
def builder_driven_event_data_available?(model)
model.class.in?(BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES)
end
def builder_driven_event_data(model, event)
builder_class = case model
when GroupMember
Gitlab::HookData::GroupMemberBuilder
......@@ -66,6 +31,8 @@ class SystemHooksService
Gitlab::HookData::UserBuilder
when Project
Gitlab::HookData::ProjectBuilder
when Key
Gitlab::HookData::KeyBuilder
end
builder_class.new(model).build(event)
......
# frozen_string_literal: true
module Gitlab
module HookData
class KeyBuilder < BaseBuilder
alias_method :key, :object
# Sample data
# {
# event_name: "key_create",
# created_at: "2021-04-19T06:13:24Z",
# updated_at: "2021-04-19T06:13:24Z",
# key: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQClDn/5BaESHlSb3NxQtiUc0BXgK6lsqdAUIdS3lwZ2gbACDhtoLYnc+qhZ4b8gWzE+2A8RmkvLe98T7noRoW4DAYs67NSqMs/kXd2ESPNV8qqv0u7tCxPz+c7DaYp2oC/avlxVQ2AeULZLCEwalYZ7irde0EZMeTwNIRu5s88gOw== dummy@gitlab.com",
# id: 1,
# username: "johndoe"
# }
def build(event)
[
event_data(event),
timestamps_data,
key_data,
user_data
].reduce(:merge)
end
private
def key_data
{
key: key.key,
id: key.id
}
end
def user_data
user = key.user
return {} unless user
{
username: user.username
}
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::HookData::KeyBuilder do
let_it_be(:personal_key) { create(:personal_key) }
let_it_be(:other_key) { create(:key) }
describe '#build' do
let(:data) { described_class.new(key).build(event) }
let(:event_name) { data[:event_name] }
let(:common_attributes) do
[
:event_name, :created_at, :updated_at, :key, :id
]
end
shared_examples_for 'includes the required attributes' do
it 'includes the required attributes' do
expect(data.keys).to contain_exactly(*attributes)
expect(data[:key]).to eq(key.key)
expect(data[:id]).to eq(key.id)
expect(data[:created_at]).to eq(key.created_at.xmlschema)
expect(data[:updated_at]).to eq(key.updated_at.xmlschema)
end
end
context 'for keys that belong to a user' do
let(:key) { personal_key }
let(:attributes) { common_attributes.append(:username) }
context 'data' do
context 'on create' do
let(:event) { :create }
it { expect(event_name).to eq('key_create') }
it { expect(data[:username]).to eq(key.user.username) }
it_behaves_like 'includes the required attributes'
end
context 'on destroy' do
let(:event) { :destroy }
it { expect(event_name).to eq('key_destroy') }
it { expect(data[:username]).to eq(key.user.username) }
it_behaves_like 'includes the required attributes'
end
end
end
context 'for keys that do not belong to a user' do
let(:key) { other_key }
let(:attributes) { common_attributes }
context 'data' do
context 'on create' do
let(:event) { :create }
it { expect(event_name).to eq('key_create') }
it_behaves_like 'includes the required attributes'
end
context 'on destroy' do
let(:event) { :destroy }
it { expect(event_name).to eq('key_destroy') }
it_behaves_like 'includes the required attributes'
end
end
end
end
end
......@@ -3,133 +3,68 @@
require 'spec_helper'
RSpec.describe SystemHooksService do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:project_member) { create(:project_member) }
let(:key) { create(:key, user: user) }
let(:deploy_key) { create(:key) }
let(:group) { create(:group) }
let(:group_member) { create(:group_member) }
context 'event data' do
it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
it { expect(event_data(project, :create)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
it { expect(event_data(project, :update)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) }
it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
it { expect(event_data(project_member, :update)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) }
it { expect(event_data(key, :create)).to include(:username, :key, :id) }
it { expect(event_data(key, :destroy)).to include(:username, :key, :id) }
it { expect(event_data(deploy_key, :create)).to include(:key, :id) }
it { expect(event_data(deploy_key, :destroy)).to include(:key, :id) }
it do
project.old_path_with_namespace = 'renamed_from_path'
expect(event_data(project, :rename)).to include(
:event_name, :name, :created_at, :updated_at, :path, :project_id,
:owner_name, :owner_email, :project_visibility,
:old_path_with_namespace
)
describe '#execute_hooks_for' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project) }
let_it_be(:group_member) { create(:group_member, source: group, user: user) }
let_it_be(:project_member) { create(:project_member, source: project, user: user) }
let_it_be(:key) { create(:key, user: user) }
let_it_be(:deploy_key) { create(:key) }
let(:event) { :create }
using RSpec::Parameterized::TableSyntax
where(:model_name, :builder_class) do
:group_member | Gitlab::HookData::GroupMemberBuilder
:group | Gitlab::HookData::GroupBuilder
:project_member | Gitlab::HookData::ProjectMemberBuilder
:user | Gitlab::HookData::UserBuilder
:project | Gitlab::HookData::ProjectBuilder
:key | Gitlab::HookData::KeyBuilder
:deploy_key | Gitlab::HookData::KeyBuilder
end
it do
project.old_path_with_namespace = 'transferred_from_path'
expect(event_data(project, :transfer)).to include(
:event_name, :name, :created_at, :updated_at, :path, :project_id,
:owner_name, :owner_email, :project_visibility,
:old_path_with_namespace
)
end
it do
expect(event_data(group, :create)).to include(
:event_name, :name, :created_at, :updated_at, :path, :group_id
)
end
with_them do
it 'builds the data with the relevant builder class and then calls #execute_hooks with the obtained data' do
data = double
model = public_send(model_name)
it do
expect(event_data(group, :destroy)).to include(
:event_name, :name, :created_at, :updated_at, :path, :group_id
)
end
expect_next_instance_of(builder_class, model) do |builder|
expect(builder).to receive(:build).with(event).and_return(data)
end
it do
expect(event_data(group_member, :create)).to include(
:event_name, :created_at, :updated_at, :group_name, :group_path,
:group_id, :user_id, :user_username, :user_name, :user_email, :group_access
)
end
service = described_class.new
it do
expect(event_data(group_member, :destroy)).to include(
:event_name, :created_at, :updated_at, :group_name, :group_path,
:group_id, :user_id, :user_username, :user_name, :user_email, :group_access
)
end
it do
expect(event_data(group_member, :update)).to include(
:event_name, :created_at, :updated_at, :group_name, :group_path,
:group_id, :user_id, :user_username, :user_name, :user_email, :group_access
)
end
it 'includes the correct project visibility level' do
data = event_data(project, :create)
expect(data[:project_visibility]).to eq('private')
end
expect_next_instance_of(SystemHooksService) do |system_hook_service|
expect(system_hook_service).to receive(:execute_hooks).with(data)
end
it 'handles nil datetime columns' do
user.update!(created_at: nil, updated_at: nil)
data = event_data(user, :destroy)
expect(data[:created_at]).to be(nil)
expect(data[:updated_at]).to be(nil)
service.execute_hooks_for(model, event)
end
end
end
context 'group_rename' do
it 'contains old and new path' do
allow(group).to receive(:path_before_last_save).and_return('old-path')
describe '#execute_hooks' do
let(:data) { { key: :value } }
data = event_data(group, :rename)
subject { described_class.new.execute_hooks(data) }
expect(data).to include(:event_name, :name, :created_at, :updated_at, :full_path, :path, :group_id, :old_path, :old_full_path)
expect(data[:path]).to eq(group.path)
expect(data[:full_path]).to eq(group.path)
expect(data[:old_path]).to eq(group.path_before_last_save)
expect(data[:old_full_path]).to eq(group.path_before_last_save)
end
it 'executes system hooks with the given data' do
hook = create(:system_hook)
it 'contains old and new full_path for subgroup' do
subgroup = create(:group, parent: group)
allow(subgroup).to receive(:path_before_last_save).and_return('old-path')
allow(SystemHook).to receive_message_chain(:hooks_for, :find_each).and_yield(hook)
data = event_data(subgroup, :rename)
expect(hook).to receive(:async_execute).with(data, 'system_hooks')
expect(data[:full_path]).to eq(subgroup.full_path)
expect(data[:old_path]).to eq('old-path')
end
subject
end
end
context 'event names' do
it { expect(event_name(project, :create)).to eq "project_create" }
it { expect(event_name(project, :destroy)).to eq "project_destroy" }
it { expect(event_name(project, :rename)).to eq "project_rename" }
it { expect(event_name(project, :transfer)).to eq "project_transfer" }
it { expect(event_name(project, :update)).to eq "project_update" }
it { expect(event_name(key, :create)).to eq 'key_create' }
it { expect(event_name(key, :destroy)).to eq 'key_destroy' }
end
it 'executes FileHook with the given data' do
expect(Gitlab::FileHook).to receive(:execute_all_async).with(data)
def event_data(*args)
SystemHooksService.new.send :build_event_data, *args
end
def event_name(*args)
SystemHooksService.new.send :build_event_name, *args
subject
end
end
end
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