Refactor GitTagPushService and fig tags_push system event hook

app/services/git_tag_push_service.rb

@@ -1,17 +1,16 @@
-class GitTagPushService
-  attr_accessor :project, :user, :push_data
+class GitTagPushService < BaseService
+  attr_accessor :push_data
 
-  def execute(project, user, oldrev, newrev, ref)
+  def execute
     project.repository.before_push_tag
 
-    @project, @user = project, user
-    @push_data = build_push_data(oldrev, newrev, ref)
+    @push_data = build_push_data
 
-    EventCreateService.new.push(project, user, @push_data)
+    EventCreateService.new.push(project, current_user, @push_data)
     SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks)
     project.execute_hooks(@push_data.dup, :tag_push_hooks)
     project.execute_services(@push_data.dup, :tag_push_hooks)
-    CreateCommitBuildsService.new.execute(project, @user, @push_data)
+    CreateCommitBuildsService.new.execute(project, current_user, @push_data)
     ProjectCacheWorker.perform_async(project.id)
 
     true
@@ -19,14 +18,14 @@ class GitTagPushService
 
   private
 
-  def build_push_data(oldrev, newrev, ref)
+  def build_push_data
     commits = []
     message = nil
 
-    if !Gitlab::Git.blank_ref?(newrev)
-      tag_name = Gitlab::Git.ref_name(ref)
+    if !Gitlab::Git.blank_ref?(params[:newrev])
+      tag_name = Gitlab::Git.ref_name(params[:ref])
       tag = project.repository.find_tag(tag_name)
-      if tag && tag.target == newrev
+      if tag && tag.target == params[:newrev]
         commit = project.commit(tag.target)
         commits = [commit].compact
         message = tag.message
@@ -34,11 +33,11 @@ class GitTagPushService
     end
 
     Gitlab::PushDataBuilder.
-      build(project, user, oldrev, newrev, ref, commits, message)
+      build(project, current_user, params[:oldrev], params[:newrev], params[:ref], commits, message)
   end
 
-  def build_system_push_data(oldrev, newrev, ref)
+  def build_system_push_data
     Gitlab::PushDataBuilder.
-      build_system(project, user, oldrev, newrev, ref)
+      build_system(project, current_user, params[:oldrev], params[:newrev], params[:ref])
   end
 end

app/workers/post_receive.rb

@@ -39,7 +39,7 @@ class PostReceive
       end
 
       if Gitlab::Git.tag_ref?(ref)
-        GitTagPushService.new.execute(post_received.project, @user, oldrev, newrev, ref)
+        GitTagPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute
       elsif Gitlab::Git.branch_ref?(ref)
         GitPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute
       end
@@ -47,7 +47,7 @@ class PostReceive
   end
 
   private
-  
+
   def log(message)
     Gitlab::GitLogger.error("POST-RECEIVE: #{message}")
   end

spec/services/git_tag_push_service_spec.rb

@@ -5,19 +5,17 @@ describe GitTagPushService, services: true do
 
   let(:user) { create :user }
   let(:project) { create :project }
-  let(:service) { GitTagPushService.new }
+  let(:service) { GitTagPushService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) }
 
-  before do
-    @oldrev = Gitlab::Git::BLANK_SHA
-    @newrev = "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" # gitlab-test: git rev-parse refs/tags/v1.1.0
-    @ref = 'refs/tags/v1.1.0'
-  end
+  let(:oldrev) { Gitlab::Git::BLANK_SHA }
+  let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0
+  let(:ref) { 'refs/tags/v1.1.0' }
 
   describe "Git Tag Push Data" do
     before do
-      service.execute(project, user, @oldrev, @newrev, @ref)
+      service.execute
       @push_data = service.push_data
-      @tag_name = Gitlab::Git.ref_name(@ref)
+      @tag_name = Gitlab::Git.ref_name(ref)
       @tag = project.repository.find_tag(@tag_name)
       @commit = project.commit(@tag.target)
     end
@@ -25,9 +23,9 @@ describe GitTagPushService, services: true do
     subject { @push_data }
 
     it { is_expected.to include(object_kind: 'tag_push') }
-    it { is_expected.to include(ref: @ref) }
-    it { is_expected.to include(before: @oldrev) }
-    it { is_expected.to include(after: @newrev) }
+    it { is_expected.to include(ref: ref) }
+    it { is_expected.to include(before: oldrev) }
+    it { is_expected.to include(after: newrev) }
     it { is_expected.to include(message: @tag.message) }
     it { is_expected.to include(user_id: user.id) }
     it { is_expected.to include(user_name: user.name) }
@@ -80,9 +78,11 @@ describe GitTagPushService, services: true do
 
   describe "Webhooks" do
     context "execute webhooks" do
+      let(:service) { GitTagPushService.new(project, user, oldrev: 'oldrev', newrev: 'newrev', ref: 'refs/tags/v1.0.0') }
+
       it "when pushing tags" do
         expect(project).to receive(:execute_hooks)
-        service.execute(project, user, 'oldrev', 'newrev', 'refs/tags/v1.0.0')
+        service.execute
       end
     end
   end