Merge branch 'refactor_push_data_builder' into 'master'

Refactor push data builder

See merge request !1384

13 files changed, 122 insertions, 85 deletions

diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index ef4d2609147..b2ce99aeb45 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -24,8 +24,7 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def test
-    data = GitPushService.new.sample_data(project, current_user)
-
+    data = Gitlab::PushDataBuilder.build_sample(project, current_user)
     @service.execute(data)
 
     redirect_to :back
diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index 162ddef0fec..64b820160d3 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -13,6 +13,7 @@ class Projects::TagsController < Projects::ApplicationController
   def create
     result = CreateTagService.new(@project, current_user).
       execute(params[:tag_name], params[:ref], params[:message])
+
     if result[:status] == :success
       @tag = result[:tag]
       redirect_to project_tags_path(@project)
diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb
index 9b2a2270233..041c2287c36 100644
--- a/app/services/create_tag_service.rb
+++ b/app/services/create_tag_service.rb
@@ -21,10 +21,15 @@ class CreateTagService < BaseService
     new_tag = repository.find_tag(tag_name)
 
     if new_tag
+      if project.gitlab_ci?
+        push_data = create_push_data(project, current_user, new_tag)
+        project.gitlab_ci_service.async_execute(push_data)
+      end
+
       Event.create_ref_event(project, current_user, new_tag, 'add', 'refs/tags')
-      return success(new_tag)
+      success(new_tag)
     else
-      return error('Invalid reference name')
+      error('Invalid reference name')
     end
   end
 
@@ -33,4 +38,9 @@ class CreateTagService < BaseService
     out[:tag] = branch
     out
   end
+
+  def create_push_data(project, user, tag)
+    Gitlab::PushDataBuilder.
+      build(project, user, Gitlab::Git::BLANK_SHA, tag.target, 'refs/tags/' + tag.name, [])
+  end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 529af1970f6..a9ea7daabc8 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -52,16 +52,6 @@ class GitPushService
     end
   end
 
-  # This method provide a sample data
-  # generated with post_receive_data method
-  # for given project
-  #
-  def sample_data(project, user)
-    @project, @user = project, user
-    @push_commits = project.repository.commits(project.default_branch, nil, 3)
-    post_receive_data(@push_commits.last.id, @push_commits.first.id, "refs/heads/#{project.default_branch}")
-  end
-
   protected
 
   def create_push_event(push_data)
@@ -112,58 +102,9 @@ class GitPushService
     end
   end
 
-  # Produce a hash of post-receive data
-  #
-  # data = {
-  #   before: String,
-  #   after: String,
-  #   ref: String,
-  #   user_id: String,
-  #   user_name: String,
-  #   project_id: String,
-  #   repository: {
-  #     name: String,
-  #     url: String,
-  #     description: String,
-  #     homepage: String,
-  #   },
-  #   commits: Array,
-  #   total_commits_count: Fixnum
-  # }
-  #
   def post_receive_data(oldrev, newrev, ref)
-    # Total commits count
-    push_commits_count = push_commits.size
-
-    # Get latest 20 commits ASC
-    push_commits_limited = push_commits.last(20)
-
-    # Hash to be passed as post_receive_data
-    data = {
-      before: oldrev,
-      after: newrev,
-      ref: ref,
-      user_id: user.id,
-      user_name: user.name,
-      project_id: project.id,
-      repository: {
-        name: project.name,
-        url: project.url_to_repo,
-        description: project.description,
-        homepage: project.web_url,
-      },
-      commits: [],
-      total_commits_count: push_commits_count
-    }
-
-    # For performance purposes maximum 20 latest commits
-    # will be passed as post receive hook data.
-    #
-    push_commits_limited.each do |commit|
-      data[:commits] << commit.hook_attrs(project)
-    end
-
-    data
+    Gitlab::PushDataBuilder.
+      build(project, user, oldrev, newrev, ref, push_commits)
   end
 
   def push_to_existing_branch?(ref, oldrev)
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index bacd39bf1c4..c24809ad607 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -19,20 +19,8 @@ class GitTagPushService
   private
 
   def create_push_data(oldrev, newrev, ref)
-    data = {
-      ref: ref,
-      before: oldrev,
-      after: newrev,
-      user_id: user.id,
-      user_name: user.name,
-      project_id: project.id,
-      repository: {
-        name: project.name,
-        url: project.url_to_repo,
-        description: project.description,
-        homepage: project.web_url
-      }
-    }
+    Gitlab::PushDataBuilder.
+      build(project, user, oldrev, newrev, ref, [])
   end
 
   def create_push_event
diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb
index 17d86a7a274..21ec2c01cb8 100644
--- a/app/services/test_hook_service.rb
+++ b/app/services/test_hook_service.rb
@@ -1,6 +1,6 @@
 class TestHookService
   def execute(hook, current_user)
-    data = GitPushService.new.sample_data(hook.project, current_user)
+    data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user)
     hook.execute(data)
   end
 end
diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb
new file mode 100644
index 00000000000..72c42a6a254
--- /dev/null
+++ b/lib/gitlab/push_data_builder.rb
@@ -0,0 +1,63 @@
+module Gitlab
+  class PushDataBuilder
+    # Produce a hash of post-receive data
+    #
+    # data = {
+    #   before: String,
+    #   after: String,
+    #   ref: String,
+    #   user_id: String,
+    #   user_name: String,
+    #   project_id: String,
+    #   repository: {
+    #     name: String,
+    #     url: String,
+    #     description: String,
+    #     homepage: String,
+    #   },
+    #   commits: Array,
+    #   total_commits_count: Fixnum
+    # }
+    #
+    def self.build(project, user, oldrev, newrev, ref, commits = [])
+      # Total commits count
+      commits_count = commits.size
+
+      # Get latest 20 commits ASC
+      commits_limited = commits.last(20)
+
+      # Hash to be passed as post_receive_data
+      data = {
+        before: oldrev,
+        after: newrev,
+        ref: ref,
+        user_id: user.id,
+        user_name: user.name,
+        project_id: project.id,
+        repository: {
+          name: project.name,
+          url: project.url_to_repo,
+          description: project.description,
+          homepage: project.web_url,
+        },
+        commits: [],
+        total_commits_count: commits_count
+      }
+
+      # For performance purposes maximum 20 latest commits
+      # will be passed as post receive hook data.
+      commits_limited.each do |commit|
+        data[:commits] << commit.hook_attrs(project)
+      end
+
+      data
+    end
+
+    # This method provide a sample data generated with
+    # existing project and commits to test web hooks
+    def self.build_sample(project, user)
+      commits = project.repository.commits(project.default_branch, nil, 3)
+      build(project, user, commits.last.id, commits.first.id, "refs/heads/#{project.default_branch}", commits)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/push_data_builder_spec.rb
new file mode 100644
index 00000000000..fbf767a167f
--- /dev/null
+++ b/spec/lib/gitlab/push_data_builder_spec.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe 'Gitlab::PushDataBuilder' do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+
+
+  describe :build_sample do
+    let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+
+    it { data.should be_a(Hash) }
+    it { data[:before].should == '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' }
+    it { data[:after].should == '5937ac0a7beb003549fc5fd26fc247adbce4a52e' }
+    it { data[:ref].should == 'refs/heads/master' }
+    it { data[:commits].size.should == 3 }
+    it { data[:total_commits_count].should == 3 }
+  end
+
+  describe :build do
+    let(:data) do
+      Gitlab::PushDataBuilder.build(project,
+                                    user,
+                                    Gitlab::Git::BLANK_SHA,
+                                    '5937ac0a7beb003549fc5fd26fc247adbce4a52e',
+                                    'refs/tags/v1.1.0')
+    end
+
+    it { data.should be_a(Hash) }
+    it { data[:before].should == Gitlab::Git::BLANK_SHA }
+    it { data[:after].should == '5937ac0a7beb003549fc5fd26fc247adbce4a52e' }
+    it { data[:ref].should == 'refs/tags/v1.1.0' }
+    it { data[:commits].should be_empty }
+    it { data[:total_commits_count].should be_zero }
+  end
+end
diff --git a/spec/models/assembla_service_spec.rb b/spec/models/assembla_service_spec.rb
index 4300090eb13..005dd41fea9 100644
--- a/spec/models/assembla_service_spec.rb
+++ b/spec/models/assembla_service_spec.rb
@@ -33,7 +33,7 @@ describe AssemblaService, models: true do
         token: 'verySecret',
         subdomain: 'project_name'
       )
-      @sample_data = GitPushService.new.sample_data(project, user)
+      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
       @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/flowdock_service_spec.rb b/spec/models/flowdock_service_spec.rb
index 5540f0fa988..ac156719b43 100644
--- a/spec/models/flowdock_service_spec.rb
+++ b/spec/models/flowdock_service_spec.rb
@@ -32,7 +32,7 @@ describe FlowdockService do
         service_hook: true,
         token: 'verySecret'
       )
-      @sample_data = GitPushService.new.sample_data(project, user)
+      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
       @api_url = 'https://api.flowdock.com/v1/git/verySecret'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/gemnasium_service_spec.rb b/spec/models/gemnasium_service_spec.rb
index 60ffa6f8b05..2c560c11dac 100644
--- a/spec/models/gemnasium_service_spec.rb
+++ b/spec/models/gemnasium_service_spec.rb
@@ -33,7 +33,7 @@ describe GemnasiumService do
         token: 'verySecret',
         api_key: 'GemnasiumUserApiKey'
       )
-      @sample_data = GitPushService.new.sample_data(project, user)
+      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
     end
     it "should call Gemnasium service" do
       Gemnasium::GitlabService.should_receive(:execute).with(an_instance_of(Hash)).once
diff --git a/spec/models/pushover_service_spec.rb b/spec/models/pushover_service_spec.rb
index 59db69d7572..f2813d66c7d 100644
--- a/spec/models/pushover_service_spec.rb
+++ b/spec/models/pushover_service_spec.rb
@@ -36,7 +36,7 @@ describe PushoverService do
     let(:pushover) { PushoverService.new }
     let(:user) { create(:user) }
     let(:project) { create(:project) }
-    let(:sample_data) { GitPushService.new.sample_data(project, user) }
+    let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
 
     let(:api_key) { 'verySecret' }
     let(:user_key) { 'verySecret' }
diff --git a/spec/models/slack_service_spec.rb b/spec/models/slack_service_spec.rb
index d4840391967..34594072409 100644
--- a/spec/models/slack_service_spec.rb
+++ b/spec/models/slack_service_spec.rb
@@ -34,7 +34,7 @@ describe SlackService do
     let(:slack)   { SlackService.new }
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
-    let(:sample_data) { GitPushService.new.sample_data(project, user) }
+    let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
     let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
 
     before do