Post Receive Refactored. Service hooks also triggered now

Dmitriy Zaporozhets
1 parent 95c23b2f
Showing 3 changed files with 86 additions and 57 deletions Show diff stats
app/roles/push_observer.rb
spec/models/project_hooks_spec.rb
spec/workers/post_receive_spec.rb
@@ -2,45 +2,83 @@
 #
 # Triggered by PostReceive job
 module PushObserver
-  def observe_push(oldrev, newrev, ref, user)
+  # 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)
-    Event.create(
-      project: self,
-      action: Event::Pushed,
-      data: data,
-      author_id: data[:user_id]
-    )
-  end
+    # Create push event
+    self.observe_push(data)
-  def update_merge_requests(oldrev, newrev, ref, user)
-    return true unless ref =~ /heads/
-    branch_name = ref.gsub("refs/heads/", "")
-    c_ids = self.commits_between(oldrev, newrev).map(&:id)
+    if push_to_branch? ref, oldrev
+      # Close merged MR
+      self.update_merge_requests(oldrev, newrev, ref, user)
-    # Update code for merge requests
-    mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all
-    mrs.each { |merge_request| merge_request.reload_code; merge_request.mark_as_unchecked }
+      # Execute web hooks
+      self.execute_hooks(data.dup)
-    # Close merge requests
-    mrs = self.merge_requests.opened.where(target_branch: branch_name).all
-    mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) }
-    mrs.each { |merge_request| merge_request.merge!(user.id) }
+      # Execute project services
+      self.execute_services(data.dup)
+    end
-    true
+    # Create satellite
+    self.satellite.create unless self.satellite.exists?
+
+    # Discover the default branch, but only if it hasn't already been set to
+    # something else
+    if default_branch.nil?
+      update_attributes(default_branch: discover_default_branch)
+    end
   end
-  def execute_hooks(oldrev, newrev, ref, user)
+  def push_to_branch? ref, oldrev
     ref_parts = ref.split('/')
     # Return if this is not a push to a branch (e.g. new commits)
-    return if ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000"
+    !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000")
+  end
-    data = post_receive_data(oldrev, newrev, ref, user)
+  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.execute(data) }
   end
+  def execute_services(data)
+    services.each do |service|
+
+      # Call service hook for service if it has one
+      service.service_hook.execute if service.service_hook
+    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 = commits_between(oldrev, newrev)
@@ -87,27 +125,20 @@ module PushObserver
     data
   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)
-    # Create push event
-    self.observe_push(oldrev, newrev, ref, user)
-
-    # Close merged MR
-    self.update_merge_requests(oldrev, newrev, ref, user)
+  def update_merge_requests(oldrev, newrev, ref, user)
+    return true unless ref =~ /heads/
+    branch_name = ref.gsub("refs/heads/", "")
+    c_ids = self.commits_between(oldrev, newrev).map(&:id)
-    # Execute web hooks
-    self.execute_hooks(oldrev, newrev, ref, user)
+    # Update code for merge requests
+    mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all
+    mrs.each { |merge_request| merge_request.reload_code; merge_request.mark_as_unchecked }
-    # Create satellite
-    self.satellite.create unless self.satellite.exists?
+    # Close merge requests
+    mrs = self.merge_requests.opened.where(target_branch: branch_name).all
+    mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) }
+    mrs.each { |merge_request| merge_request.merge!(user.id) }
-    # Discover the default branch, but only if it hasn't already been set to
-    # something else
-    if default_branch.nil?
-      update_attributes(default_branch: discover_default_branch)
-    end
+    true
   end
 end
@@ -11,13 +11,15 @@ describe Project, &quot;Hooks&quot; do
   describe "Post Receive Event" do
     it "should create push event" do
       oldrev, newrev, ref = '00000000000000000000000000000000', 'newrev', 'refs/heads/master'
-      project.observe_push(oldrev, newrev, ref, @user)
+      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 == project.post_receive_data(oldrev, newrev, ref, @user)
+      event.data.should == data
     end
   end
@@ -25,7 +27,7 @@ describe Project, &quot;Hooks&quot; do
     context "with no web hooks" do
       it "raises no errors" do
         lambda {
-          project.execute_hooks('oldrev', 'newrev', 'ref', @user)
+          project.execute_hooks({})
         }.should_not raise_error
       end
     end
@@ -41,7 +43,7 @@ describe Project, &quot;Hooks&quot; do
         @project_hook.should_receive(:execute).once
         @project_hook_2.should_receive(:execute).once
-        project.execute_hooks('oldrev', 'newrev', 'refs/heads/master', @user)
+        project.trigger_post_receive('oldrev', 'newrev', 'refs/heads/master', @user)
       end
     end
@@ -53,12 +55,12 @@ describe Project, &quot;Hooks&quot; do
       it "when pushing a branch for the first time" do
         @project_hook.should_not_receive(:execute)
-        project.execute_hooks('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user)
+        project.trigger_post_receive('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user)
       end
       it "when pushing tags" do
         @project_hook.should_not_receive(:execute)
-        project.execute_hooks('oldrev', 'newrev', 'refs/tags/v1.0.0', @user)
+        project.trigger_post_receive('oldrev', 'newrev', 'refs/tags/v1.0.0', @user)
       end
     end
@@ -27,16 +27,12 @@ describe PostReceive do
       PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false
     end
-    it "asks the project to execute web hooks" do
+    it "asks the project to trigger all hooks" do
       Project.stub(find_by_path: project)
-      project.should_receive(:execute_hooks).with('sha-old', 'sha-new', 'refs/heads/master', project.owner)
-
-      PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id)
-    end
-
-    it "asks the project to observe push/create event data" do
-      Project.stub(find_by_path: project)
-      project.should_receive(:observe_push).with('sha-old', 'sha-new', 'refs/heads/master', project.owner)
+      project.should_receive(:execute_hooks)
+      project.should_receive(:execute_services)
+      project.should_receive(:update_merge_requests)
+      project.should_receive(:observe_push)
       PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id)
     end