Move git post push logic to service

app/models/project.rb

@@ -295,53 +295,6 @@ class Project < ActiveRecord::Base
     end
   end
 
-  # This method will be called after each post receive and only if the provided
-  # user is present in GitLab.
-  #
-  # All callbacks for post receive should be placed here.
-  def trigger_post_receive(oldrev, newrev, ref, user)
-    data = post_receive_data(oldrev, newrev, ref, user)
-
-    # Create satellite
-    self.satellite.create unless self.satellite.exists?
-
-    # Create push event
-    self.observe_push(data)
-
-    if push_to_branch? ref, oldrev
-      # Close merged MR
-      self.update_merge_requests(oldrev, newrev, ref, user)
-
-      # Execute web hooks
-      self.execute_hooks(data.dup)
-
-      # Execute project services
-      self.execute_services(data.dup)
-    end
-
-    # Discover the default branch, but only if it hasn't already been set to
-    # something else
-    if repository && default_branch.nil?
-      update_attributes(default_branch: self.repository.discover_default_branch)
-    end
-  end
-
-  def push_to_branch? ref, oldrev
-    ref_parts = ref.split('/')
-
-    # Return if this is not a push to a branch (e.g. new commits)
-    !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000")
-  end
-
-  def observe_push(data)
-    Event.create(
-      project: self,
-      action: Event::PUSHED,
-      data: data,
-      author_id: data[:user_id]
-    )
-  end
-
   def execute_hooks(data)
     hooks.each { |hook| hook.async_execute(data) }
   end
@@ -354,68 +307,12 @@ class Project < ActiveRecord::Base
     end
   end
 
-  # Produce a hash of post-receive data
-  #
-  # data = {
-  #   before: String,
-  #   after: String,
-  #   ref: String,
-  #   user_id: String,
-  #   user_name: String,
-  #   repository: {
-  #     name: String,
-  #     url: String,
-  #     description: String,
-  #     homepage: String,
-  #   },
-  #   commits: Array,
-  #   total_commits_count: Fixnum
-  # }
-  #
-  def post_receive_data(oldrev, newrev, ref, user)
-
-    push_commits = repository.commits_between(oldrev, newrev)
-
-    # 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,
-      repository: {
-        name: name,
-        url: url_to_repo,
-        description: description,
-        homepage: web_url,
-      },
-      commits: [],
-      total_commits_count: push_commits_count
-    }
-
-    # For perfomance purposes maximum 20 latest commits
-    # will be passed as post receive hook data.
-    #
-    push_commits_limited.each do |commit|
-      data[:commits] << {
-        id: commit.id,
-        message: commit.safe_message,
-        timestamp: commit.date.xmlschema,
-        url: "#{Gitlab.config.gitlab.url}/#{path_with_namespace}/commit/#{commit.id}",
-        author: {
-          name: commit.author_name,
-          email: commit.author_email
-        }
-      }
+  def discover_default_branch
+    # Discover the default branch, but only if it hasn't already been set to
+    # something else
+    if repository && default_branch.nil?
+      update_attributes(default_branch: self.repository.discover_default_branch)
     end
-
-    data
   end
 
   def update_merge_requests(oldrev, newrev, ref, user)
@@ -446,6 +343,10 @@ class Project < ActiveRecord::Base
     !repository || repository.empty?
   end
 
+  def ensure_satellite_exists
+    self.satellite.create unless self.satellite.exists?
+  end
+
   def satellite
     @satellite ||= Gitlab::Satellite::Satellite.new(self)
   end

app/services/git_push_service.rb

+class GitPushService
+  attr_accessor :project, :user, :push_data
+
+  # This method will be called after each git update
+  # and only if the provided user and project is present in GitLab.
+  #
+  # All callbacks for post receive action should be placed here.
+  #
+  # Now this method do next:
+  #  1. Ensure project satellite exists
+  #  2. Update merge requests
+  #  3. Execute project web hooks
+  #  4. Execute project services
+  #  5. Create Push Event
+  #
+  def execute(project, user, oldrev, newrev, ref)
+    @project, @user = project, user
+
+    # Collect data for this git push
+    @push_data = post_receive_data(oldrev, newrev, ref)
+
+    project.ensure_satellite_exists
+    project.discover_default_branch
+
+    if push_to_branch?(ref, oldrev)
+      project.update_merge_requests(oldrev, newrev, ref, @user)
+      project.execute_hooks(@push_data.dup)
+      project.execute_services(@push_data.dup)
+    end
+
+    create_push_event
+  end
+
+  protected
+
+  def create_push_event
+    Event.create(
+      project: project,
+      action: Event::PUSHED,
+      data: push_data,
+      author_id: push_data[:user_id]
+    )
+  end
+
+  # Produce a hash of post-receive data
+  #
+  # data = {
+  #   before: String,
+  #   after: String,
+  #   ref: String,
+  #   user_id: String,
+  #   user_name: String,
+  #   repository: {
+  #     name: String,
+  #     url: String,
+  #     description: String,
+  #     homepage: String,
+  #   },
+  #   commits: Array,
+  #   total_commits_count: Fixnum
+  # }
+  #
+  def post_receive_data(oldrev, newrev, ref)
+    push_commits = project.repository.commits_between(oldrev, newrev)
+
+    # 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,
+      repository: {
+        name: project.name,
+        url: project.url_to_repo,
+        description: project.description,
+        homepage: project.web_url,
+      },
+      commits: [],
+      total_commits_count: push_commits_count
+    }
+
+    # For perfomance purposes maximum 20 latest commits
+    # will be passed as post receive hook data.
+    #
+    push_commits_limited.each do |commit|
+      data[:commits] << {
+        id: commit.id,
+        message: commit.safe_message,
+        timestamp: commit.date.xmlschema,
+        url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{commit.id}",
+        author: {
+          name: commit.author_name,
+          email: commit.author_email
+        }
+      }
+    end
+
+    data
+  end
+
+  def push_to_branch? ref, oldrev
+    ref_parts = ref.split('/')
+
+    # Return if this is not a push to a branch (e.g. new commits)
+    !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000")
+  end
+end

app/workers/post_receive.rb

@@ -42,6 +42,6 @@ class PostReceive
       return false
     end
 
-    project.trigger_post_receive(oldrev, newrev, ref, user)
+    GitPushService.new.execute(project, user, oldrev, newrev, ref)
   end
 end

spec/models/project_hooks_spec.rb

-require 'spec_helper'
-
-describe Project, "Hooks" do
-  let(:project) { create(:project) }
-
-  before do
-    @key = create(:key, user: project.owner)
-    @user = @key.user
-    @key_id = @key.identifier
-  end
-
-  describe "Post Receive Event" do
-    it "should create push event" do
-      oldrev, newrev, ref = '00000000000000000000000000000000', 'newrev', 'refs/heads/master'
-      data = project.post_receive_data(oldrev, newrev, ref, @user)
-
-      project.observe_push(data)
-      event = Event.last
-
-      event.should_not be_nil
-      event.project.should == project
-      event.action.should == Event::PUSHED
-      event.data.should == data
-    end
-  end
-
-  describe "Project hooks" do
-    context "with no web hooks" do
-      it "raises no errors" do
-        lambda {
-          project.execute_hooks({})
-        }.should_not raise_error
-      end
-    end
-
-    context "with web hooks" do
-      before do
-        @project_hook = create(:project_hook)
-        @project_hook_2 = create(:project_hook)
-        project.hooks << [@project_hook, @project_hook_2]
-
-        stub_request(:post, @project_hook.url)
-        stub_request(:post, @project_hook_2.url)
-      end
-
-      it "executes multiple web hook" do
-        @project_hook.should_receive(:async_execute).once
-        @project_hook_2.should_receive(:async_execute).once
-
-        project.trigger_post_receive('oldrev', 'newrev', 'refs/heads/master', @user)
-      end
-    end
-
-    context "does not execute web hooks" do
-      before do
-        @project_hook = create(:project_hook)
-        project.hooks << [@project_hook]
-      end
-
-      it "when pushing a branch for the first time" do
-        @project_hook.should_not_receive(:execute)
-        project.trigger_post_receive('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user)
-      end
-
-      it "when pushing tags" do
-        @project_hook.should_not_receive(:execute)
-        project.trigger_post_receive('oldrev', 'newrev', 'refs/tags/v1.0.0', @user)
-      end
-    end
-
-    context "when pushing new branches" do
-
-    end
-
-    context "when gathering commit data" do
-      before do
-        @oldrev, @newrev, @ref = project.repository.fresh_commits(2).last.sha,
-          project.repository.fresh_commits(2).first.sha, 'refs/heads/master'
-        @commit = project.repository.fresh_commits(2).first
-
-        # Fill nil/empty attributes
-        project.description = "This is a description"
-
-        @data = project.post_receive_data(@oldrev, @newrev, @ref, @user)
-      end
-
-      subject { @data }
-
-      it { should include(before: @oldrev) }
-      it { should include(after: @newrev) }
-      it { should include(ref: @ref) }
-      it { should include(user_id: project.owner.id) }
-      it { should include(user_name: project.owner.name) }
-
-      context "with repository data" do
-        subject { @data[:repository] }
-
-        it { should include(name: project.name) }
-        it { should include(url: project.url_to_repo) }
-        it { should include(description: project.description) }
-        it { should include(homepage: project.web_url) }
-      end
-
-      context "with commits" do
-        subject { @data[:commits] }
-
-        it { should be_an(Array) }
-        it { should have(1).element }
-
-        context "the commit" do
-          subject { @data[:commits].first }
-
-          it { should include(id: @commit.id) }
-          it { should include(message: @commit.safe_message) }
-          it { should include(timestamp: @commit.date.xmlschema) }
-          it { should include(url: "#{Gitlab.config.gitlab.url}/#{project.code}/commit/#{@commit.id}") }
-
-          context "with a author" do
-            subject { @data[:commits].first[:author] }
-
-            it { should include(name: @commit.author_name) }
-            it { should include(email: @commit.author_email) }
-          end
-        end
-      end
-    end
-  end
-end

spec/models/project_spec.rb

@@ -72,11 +72,8 @@ describe Project do
     it { should respond_to(:url_to_repo) }
     it { should respond_to(:repo_exists?) }
     it { should respond_to(:satellite) }
-    it { should respond_to(:observe_push) }
     it { should respond_to(:update_merge_requests) }
     it { should respond_to(:execute_hooks) }
-    it { should respond_to(:post_receive_data) }
-    it { should respond_to(:trigger_post_receive) }
     it { should respond_to(:transfer) }
     it { should respond_to(:name_with_namespace) }
     it { should respond_to(:namespace_owner) }

spec/services/git_push_service.rb

+require 'spec_helper'
+
+describe GitPushService do
+  let (:user)          { create :user }
+  let (:project)       { create :project }
+  let (:service) { GitPushService.new }
+
+  before do
+    @oldrev = 'b98a310def241a6fd9c9a9a3e7934c48e498fe81'
+    @newrev = 'b19a04f53caeebf4fe5ec2327cb83e9253dc91bb'
+    @ref = 'refs/heads/master'
+  end
+
+  describe "Git Push Data" do
+    before do
+      service.execute(project, user, @oldrev, @newrev, @ref)
+      @push_data = service.push_data
+      @commit = project.repository.commit(@newrev)
+    end
+
+    subject { @push_data }
+
+    it { should include(before: @oldrev) }
+    it { should include(after: @newrev) }
+    it { should include(ref: @ref) }
+    it { should include(user_id: user.id) }
+    it { should include(user_name: user.name) }
+
+    context "with repository data" do
+      subject { @push_data[:repository] }
+
+      it { should include(name: project.name) }
+      it { should include(url: project.url_to_repo) }
+      it { should include(description: project.description) }
+      it { should include(homepage: project.web_url) }
+    end
+
+    context "with commits" do
+      subject { @push_data[:commits] }
+
+      it { should be_an(Array) }
+      it { should have(1).element }
+
+      context "the commit" do
+        subject { @push_data[:commits].first }
+
+        it { should include(id: @commit.id) }
+        it { should include(message: @commit.safe_message) }
+        it { should include(timestamp: @commit.date.xmlschema) }
+        it { should include(url: "#{Gitlab.config.gitlab.url}/#{project.code}/commit/#{@commit.id}") }
+
+        context "with a author" do
+          subject { @push_data[:commits].first[:author] }
+
+          it { should include(name: @commit.author_name) }
+          it { should include(email: @commit.author_email) }
+        end
+      end
+    end
+  end
+
+  describe "Push Event" do
+    before do
+      service.execute(project, user, @oldrev, @newrev, @ref)
+      @event = Event.last
+    end
+
+    it { @event.should_not be_nil }
+    it { @event.project.should == project }
+    it { @event.action.should == Event::PUSHED }
+    it { @event.data.should == service.push_data }
+  end
+
+  describe "Web Hooks" do
+    context "with web hooks" do
+      before do
+        @project_hook = create(:project_hook)
+        @project_hook_2 = create(:project_hook)
+        project.hooks << [@project_hook, @project_hook_2]
+
+        stub_request(:post, @project_hook.url)
+        stub_request(:post, @project_hook_2.url)
+      end
+
+      it "executes multiple web hook" do
+        @project_hook.should_receive(:async_execute).once
+        @project_hook_2.should_receive(:async_execute).once
+
+        service.execute(project, user, @oldrev, @newrev, @ref)
+      end
+    end
+
+    context "does not execute web hooks" do
+      before do
+        @project_hook = create(:project_hook)
+        project.hooks << [@project_hook]
+      end
+
+      it "when pushing a branch for the first time" do
+        @project_hook.should_not_receive(:execute)
+        service.execute(project, user, '00000000000000000000000000000000', 'newrev', 'refs/heads/master')
+      end
+
+      it "when pushing tags" do
+        @project_hook.should_not_receive(:execute)
+        service.execute(project, user, 'newrev', 'newrev', 'refs/tags/v1.0.0')
+      end
+    end
+  end
+end
+