Merge branch '26325-system-hooks' into 'master'

Backport New SystemHook: `repository_update`

Closes #26325

See merge request !11140

19 files changed, 230 insertions, 11 deletions

diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index a119934febc..ccfe553c89e 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -60,6 +60,7 @@ class Admin::HooksController < Admin::ApplicationController
       :enable_ssl_verification,
       :push_events,
       :tag_push_events,
+      :repository_update_events,
       :token,
       :url
     )
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index 777bad1e724..c645805c6da 100644
--- a/app/models/hooks/system_hook.rb
+++ b/app/models/hooks/system_hook.rb
@@ -1,4 +1,9 @@
 class SystemHook < WebHook
+  scope :repository_update_hooks, ->  { where(repository_update_events: true) }
+
+  default_value_for :push_events, false
+  default_value_for :repository_update_events, true
+
   def async_execute(data, hook_name)
     Sidekiq::Client.enqueue(SystemHookWorker, id, data, hook_name)
   end
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 22a177ed367..7cf03aabd6f 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -10,6 +10,7 @@ class WebHook < ActiveRecord::Base
   default_value_for :tag_push_events, false
   default_value_for :build_events, false
   default_value_for :pipeline_events, false
+  default_value_for :repository_update_events, false
   default_value_for :enable_ssl_verification, true
 
   scope :push_hooks, -> { where(push_events: true) }
diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml
index 6217d5fb135..645005c6deb 100644
--- a/app/views/admin/hooks/_form.html.haml
+++ b/app/views/admin/hooks/_form.html.haml
@@ -18,19 +18,26 @@
       or adding ssh key. But you can also enable extra triggers like Push events.
 
     .prepend-top-default
+      = form.check_box :repository_update_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :repository_update_events, class: 'list-label' do
+          %strong Repository update events
+        %p.light
+          This URL will be triggered when repository is updated
+    %div
       = form.check_box :push_events, class: 'pull-left'
       .prepend-left-20
         = form.label :push_events, class: 'list-label' do
           %strong Push events
         %p.light
-          This url will be triggered by a push to the repository
+          This URL will be triggered for each branch updated to the repository
     %div
       = form.check_box :tag_push_events, class: 'pull-left'
       .prepend-left-20
         = form.label :tag_push_events, class: 'list-label' do
           %strong Tag push events
         %p.light
-          This url will be triggered when a new tag is pushed to the repository
+          This URL will be triggered when a new tag is pushed to the repository
 .form-group
   = form.label :enable_ssl_verification, 'SSL verification', class: 'control-label checkbox'
   .col-sm-10
diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml
index 71117758921..3338b677bf5 100644
--- a/app/views/admin/hooks/index.html.haml
+++ b/app/views/admin/hooks/index.html.haml
@@ -27,7 +27,7 @@
             = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'
           .monospace= hook.url
           %div
-            - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger|
+            - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger|
               - if hook.send(trigger)
                 %span.label.label-gray= trigger.titleize
             %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'}
diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb
index 127d8dfbb61..c29571d3c62 100644
--- a/app/workers/post_receive.rb
+++ b/app/workers/post_receive.rb
@@ -20,13 +20,32 @@ class PostReceive
       # Nothing defined here yet.
     else
       process_project_changes(post_received)
+      process_repository_update(post_received)
     end
   end
 
-  def process_project_changes(post_received)
-    post_received.changes.each do |change|
-      oldrev, newrev, ref = change.strip.split(' ')
+  def process_repository_update(post_received)
+    changes = []
+    refs = Set.new
+
+    post_received.changes_refs do |oldrev, newrev, ref|
+      @user ||= post_received.identify(newrev)
 
+      unless @user
+        log("Triggered hook for non-existing user \"#{post_received.identifier}\"")
+        return false
+      end
+
+      changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref)
+      refs << ref
+    end
+
+    hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, @user, changes, refs.to_a)
+    SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks)
+  end
+
+  def process_project_changes(post_received)
+    post_received.changes_refs do |oldrev, newrev, ref|
       @user ||= post_received.identify(newrev)
 
       unless @user
diff --git a/changelogs/unreleased/26325-system-hooks.yml b/changelogs/unreleased/26325-system-hooks.yml
new file mode 100644
index 00000000000..62b8adaeccd
--- /dev/null
+++ b/changelogs/unreleased/26325-system-hooks.yml
@@ -0,0 +1,4 @@
+---
+title: 'Backported new SystemHook event: `repository_update`'
+merge_request: 11140
+author:
diff --git a/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb b/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb
new file mode 100644
index 00000000000..0faea87a962
--- /dev/null
+++ b/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb
@@ -0,0 +1,15 @@
+class AddRepositoryUpdateEventsToWebHooks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default :web_hooks, :repository_update_events, :boolean, default: false, allow_null: false
+  end
+
+  def down
+    remove_column :web_hooks, :repository_update_events
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 60077ffd812..65eaccf766a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1404,6 +1404,7 @@ ActiveRecord::Schema.define(version: 20170508190732) do
     t.string "token"
     t.boolean "pipeline_events", default: false, null: false
     t.boolean "confidential_issues_events", default: false, null: false
+    t.boolean "repository_update_events", default: false, null: false
   end
 
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md
index ad5ffc84473..583ec5522fd 100644
--- a/doc/system_hooks/system_hooks.md
+++ b/doc/system_hooks/system_hooks.md
@@ -266,7 +266,8 @@ X-Gitlab-Event: System Hook
 
 ## Push events
 
-Triggered when you push to the repository except when pushing tags.
+Triggered when you push to the repository, except when pushing tags.
+It generates one event per modified branch.
 
 **Request header**:
 
@@ -332,6 +333,7 @@ X-Gitlab-Event: System Hook
 ## Tag events
 
 Triggered when you create (or delete) tags to the repository.
+It generates one event per modified tag.
 
 **Request header**:
 
@@ -381,3 +383,49 @@ X-Gitlab-Event: System Hook
   "total_commits_count": 0
 }
 ```
+## Repository Update events
+
+Triggered only once when you push to the repository (including tags).
+
+**Request header**:
+
+```
+X-Gitlab-Event: System Hook
+```
+
+**Request body:**
+
+```json
+{
+  "event_name": "repository_update",
+  "user_id": 1,
+  "user_name": "John Smith",
+  "user_email": "admin@example.com",
+  "user_avatar": "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80",
+  "project_id": 1,
+  "project": {
+    "name":"Example",
+    "description":"",
+    "web_url":"http://example.com/jsmith/example",
+    "avatar_url":null,
+    "git_ssh_url":"git@example.com:jsmith/example.git",
+    "git_http_url":"http://example.com/jsmith/example.git",
+    "namespace":"Jsmith",
+    "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",
+  },
+  "changes": [
+    {
+      "before":"8205ea8d81ce0c6b90fbe8280d118cc9fdad6130",
+      "after":"4045ea7a3df38697b3730a20fb73c8bed8a3e69e",
+      "ref":"refs/heads/master"
+    }
+  ],
+  "refs":["refs/heads/master"]
+}
+```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 00d494f02f5..3fc2b453eb6 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -53,7 +53,7 @@ module API
     end
 
     class Hook < Grape::Entity
-      expose :id, :url, :created_at, :push_events, :tag_push_events
+      expose :id, :url, :created_at, :push_events, :tag_push_events, :repository_update_events
       expose :enable_ssl_verification
     end
 
diff --git a/lib/gitlab/data_builder/repository.rb b/lib/gitlab/data_builder/repository.rb
new file mode 100644
index 00000000000..b42dc052949
--- /dev/null
+++ b/lib/gitlab/data_builder/repository.rb
@@ -0,0 +1,35 @@
+module Gitlab
+  module DataBuilder
+    module Repository
+      extend self
+
+      # Produce a hash of post-receive data
+      def update(project, user, changes, refs)
+        {
+          event_name: 'repository_update',
+
+          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,
+
+          changes: changes,
+
+          refs: refs
+        }
+      end
+
+      # Produce a hash of partial data for a single change
+      def single_change(oldrev, newrev, ref)
+        {
+          before: oldrev,
+          after: newrev,
+          ref: ref
+        }
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb
index 0e14253ab4e..742118b76a8 100644
--- a/lib/gitlab/git_post_receive.rb
+++ b/lib/gitlab/git_post_receive.rb
@@ -13,6 +13,16 @@ module Gitlab
       super(identifier, project, revision)
     end
 
+    def changes_refs
+      return enum_for(:changes_refs) unless block_given?
+
+      changes.each do |change|
+        oldrev, newrev, ref = change.strip.split(' ')
+
+        yield oldrev, newrev, ref
+      end
+    end
+
     private
 
     def deserialize_changes(changes)
diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb
new file mode 100644
index 00000000000..1d1070e90f4
--- /dev/null
+++ b/spec/controllers/admin/hooks_controller_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Admin::HooksController do
+  let(:admin) { create(:admin) }
+
+  before do
+    sign_in(admin)
+  end
+
+  describe 'POST #create' do
+    it 'sets all parameters' do
+      hook_params = {
+        enable_ssl_verification: true,
+        push_events: true,
+        tag_push_events: true,
+        repository_update_events: true,
+        token: "TEST TOKEN",
+        url: "http://example.com"
+      }
+
+      post :create, hook: hook_params
+
+      expect(response).to have_http_status(302)
+      expect(SystemHook.all.size).to eq(1)
+      expect(SystemHook.first).to have_attributes(hook_params)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index f63fb7aeec6..2b95f76e045 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -317,6 +317,7 @@ ProjectHook:
 - token
 - group_id
 - confidential_issues_events
+- repository_update_events
 ProtectedBranch:
 - id
 - project_id
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index 8acec805584..4340170888d 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -1,6 +1,19 @@
 require "spec_helper"
 
 describe SystemHook, models: true do
+  context 'default attributes' do
+    let(:system_hook) { build(:system_hook) }
+
+    it 'sets defined default parameters' do
+      attrs = {
+        push_events: false,
+        repository_update_events: true,
+        enable_ssl_verification: true
+      }
+      expect(system_hook).to have_attributes(attrs)
+    end
+  end
+
   describe "execute" do
     let(:system_hook) { create(:system_hook) }
     let(:user)        { create(:user) }
@@ -105,4 +118,12 @@ describe SystemHook, models: true do
       ).once
     end
   end
+
+  describe '.repository_update_hooks' do
+    it 'returns hooks for repository update events only' do
+      hook = create(:system_hook, repository_update_events: true)
+      create(:system_hook, repository_update_events: false)
+      expect(SystemHook.repository_update_hooks).to eq([hook])
+    end
+  end
 end
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index c7b84173570..2eb191d6049 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -32,8 +32,9 @@ describe API::SystemHooks do
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
         expect(json_response.first['url']).to eq(hook.url)
-        expect(json_response.first['push_events']).to be true
+        expect(json_response.first['push_events']).to be false
         expect(json_response.first['tag_push_events']).to be false
+        expect(json_response.first['repository_update_events']).to be true
       end
     end
   end
diff --git a/spec/requests/api/v3/system_hooks_spec.rb b/spec/requests/api/v3/system_hooks_spec.rb
index 72c7d14b8ba..ae427541abb 100644
--- a/spec/requests/api/v3/system_hooks_spec.rb
+++ b/spec/requests/api/v3/system_hooks_spec.rb
@@ -31,8 +31,9 @@ describe API::V3::SystemHooks do
         expect(response).to have_http_status(200)
         expect(json_response).to be_an Array
         expect(json_response.first['url']).to eq(hook.url)
-        expect(json_response.first['push_events']).to be true
+        expect(json_response.first['push_events']).to be false
         expect(json_response.first['tag_push_events']).to be false
+        expect(json_response.first['repository_update_events']).to be true
       end
     end
   end
diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb
index 0260416dbe2..3289c2df1fb 100644
--- a/spec/workers/post_receive_spec.rb
+++ b/spec/workers/post_receive_spec.rb
@@ -9,7 +9,7 @@ describe PostReceive do
   let(:key) { create(:key, user: project.owner) }
   let(:key_id) { key.shell_id }
 
-  context "as a resque worker" do
+  context "as a sidekiq worker" do
     it "reponds to #perform" do
       expect(described_class.new).to respond_to(:perform)
     end
@@ -93,6 +93,27 @@ describe PostReceive do
     end
   end
 
+  describe '#process_repository_update' do
+    let(:changes) {'123456 789012 refs/heads/tést'}
+    let(:fake_hook_data) do
+      { event_name: 'repository_update' }
+    end
+
+    before do
+      allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner)
+      allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data)
+      # silence hooks so we can isolate
+      allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true)
+      allow(subject).to receive(:process_project_changes).and_return(true)
+    end
+
+    it 'calls SystemHooksService' do
+      expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true)
+
+      subject.perform(pwd(project), key_id, base64_changes)
+    end
+  end
+
   context "webhook" do
     it "fetches the correct project" do
       expect(Project).to receive(:find_by).with(id: project.id.to_s)