From 5330af3fa69d4c47437ce27480c7f3b74652b2ca Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Mon, 18 Apr 2016 21:52:43 -0300 Subject: [PATCH] Using single builder for push and tag events --- app/services/git_push_service.rb | 2 +- app/services/git_tag_push_service.rb | 2 +- doc/system_hooks/system_hooks.md | 40 +++++++++++++++++++++-- lib/gitlab/push_data_builder.rb | 21 +----------- spec/lib/gitlab/push_data_builder_spec.rb | 28 ++++------------ 5 files changed, 47 insertions(+), 46 deletions(-) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 25d8e2cf052..1e1be8cd04b 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -141,7 +141,7 @@ class GitPushService < BaseService def build_push_data_system_hook @push_data_system ||= Gitlab::PushDataBuilder. - build_system(@project, current_user, params[:oldrev], params[:newrev], params[:ref]) + build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], []) end def push_to_existing_branch? diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index f4a1d0da5d8..64271d8bc5c 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -38,6 +38,6 @@ class GitTagPushService < BaseService def build_system_push_data Gitlab::PushDataBuilder. - build_system(project, current_user, params[:oldrev], params[:newrev], params[:ref]) + build(project, current_user, params[:oldrev], params[:newrev], params[:ref], [], '') end end diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index 4c5a9e366cf..c44930a4ceb 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -4,6 +4,12 @@ Your GitLab instance can perform HTTP POST requests on the following events: `pr System hooks can be used, e.g. for logging or changing information in a LDAP server. +> **Note:** +> +> We follow the same structure from Webhooks for Push and Tag events, but we never display commits. +> +> Same deprecations from Webhooks are valid here. + ## Hooks request example **Request header**: @@ -276,7 +282,22 @@ X-Gitlab-Event: System Hook "visibility_level":0, "path_with_namespace":"mike/diaspora", "default_branch":"master", - } + "homepage":"http://example.com/mike/diaspora", + "url":"git@example.com:mike/diaspora.git", + "ssh_url":"git@example.com:mike/diaspora.git", + "http_url":"http://example.com/mike/diaspora.git" + }, + "repository":{ + "name": "Diaspora", + "url": "git@example.com:mike/diaspora.git", + "description": "", + "homepage": "http://example.com/mike/diaspora", + "git_http_url":"http://example.com/mike/diaspora.git", + "git_ssh_url":"git@example.com:mike/diaspora.git", + "visibility_level":0 + }, + "commits": [], + "total_commits_count": 0 } ``` @@ -314,6 +335,21 @@ X-Gitlab-Event: System Hook "visibility_level":0, "path_with_namespace":"jsmith/example", "default_branch":"master", - } + "homepage":"http://example.com/jsmith/example", + "url":"git@example.com:jsmith/example.git", + "ssh_url":"git@example.com:jsmith/example.git", + "http_url":"http://example.com/jsmith/example.git" + }, + "repository":{ + "name": "Example", + "url": "ssh://git@example.com/jsmith/example.git", + "description": "", + "homepage": "http://example.com/jsmith/example", + "git_http_url":"http://example.com/jsmith/example.git", + "git_ssh_url":"git@example.com:jsmith/example.git", + "visibility_level":0 + }, + "commits": [], + "total_commits_count": 0 } ``` diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb index eb8e45040bc..67622f321a6 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/push_data_builder.rb @@ -41,6 +41,7 @@ module Gitlab # Hash to be passed as post_receive_data data = { object_kind: type, + event_name: type, before: oldrev, after: newrev, ref: ref, @@ -62,26 +63,6 @@ module Gitlab data end - def build_system(project, user, oldrev, newrev, ref) - type = Gitlab::Git.tag_ref?(ref) ? 'tag_push' : 'push' - - data = { - event_name: type, - before: oldrev, - after: newrev, - ref: ref, - checkout_sha: checkout_sha(project.repository, newrev, ref), - user_id: user.id, - user_name: user.name, - user_email: user.email, - user_avatar: user.avatar_url, - project_id: project.id, - project: project.hook_attrs(backward: false) - } - - data - end - # This method provide a sample data generated with # existing project and commits to test webhooks def build_sample(project, user) diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/push_data_builder_spec.rb index d6fb65e6948..7fc34139eff 100644 --- a/spec/lib/gitlab/push_data_builder_spec.rb +++ b/spec/lib/gitlab/push_data_builder_spec.rb @@ -34,6 +34,12 @@ describe Gitlab::PushDataBuilder, lib: true do it { expect(data[:checkout_sha]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } it { expect(data[:after]).to eq('8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b') } it { expect(data[:ref]).to eq('refs/tags/v1.1.0') } + it { expect(data[:user_id]).to eq(user.id) } + it { expect(data[:user_name]).to eq(user.name) } + it { expect(data[:user_email]).to eq(user.email) } + it { expect(data[:user_avatar]).to eq(user.avatar_url) } + it { expect(data[:project_id]).to eq(project.id) } + it { expect(data[:project]).to be_a(Hash) } it { expect(data[:commits]).to be_empty } it { expect(data[:total_commits_count]).to be_zero } @@ -45,26 +51,4 @@ describe Gitlab::PushDataBuilder, lib: true do not_to raise_error end end - - describe '.build_system' do - let(:data) do - described_class.build_system(project, user, Gitlab::Git::BLANK_SHA, - '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b', - 'refs/tags/v1.1.0') - end - - it { expect(data).to be_a(Hash) } - it { expect(data[:before]).to eq(Gitlab::Git::BLANK_SHA) } - it { expect(data[:checkout_sha]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } - it { expect(data[:after]).to eq('8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b') } - it { expect(data[:ref]).to eq('refs/tags/v1.1.0') } - it { expect(data[:user_id]).to eq(user.id) } - it { expect(data[:user_name]).to eq(user.name) } - it { expect(data[:user_email]).to eq(user.email) } - it { expect(data[:user_avatar]).to eq(user.avatar_url) } - it { expect(data[:project_id]).to eq(project.id) } - it { expect(data[:project]).to be_a(Hash) } - - include_examples 'project hook data' - end end