Merge branch 'split-events-into-push-events' into 'master'

Use a separate table for storing push events

See merge request !12463

app/controllers/dashboard/projects_controller.rb

@@ -52,8 +52,10 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   end
 
   def load_events
-    @events = Event.in_projects(load_projects(params.merge(non_public: true)))
-    @events = event_filter.apply_filter(@events).with_associations
-    @events = @events.limit(20).offset(params[:offset] || 0)
+    projects = load_projects(params.merge(non_public: true))
+
+    @events = EventCollection
+      .new(projects, offset: params[:offset].to_i, filter: event_filter)
+      .to_a
   end
 end

app/controllers/dashboard_controller.rb

@@ -29,9 +29,9 @@ class DashboardController < Dashboard::ApplicationController
         current_user.authorized_projects
       end
 
-    @events = Event.in_projects(projects)
-    @events = @event_filter.apply_filter(@events).with_associations
-    @events = @events.limit(20).offset(params[:offset] || 0)
+    @events = EventCollection
+      .new(projects, offset: params[:offset].to_i, filter: @event_filter)
+      .to_a
   end
 
   def set_show_full_reference

app/controllers/groups_controller.rb

@@ -160,9 +160,9 @@ class GroupsController < Groups::ApplicationController
   end
 
   def load_events
-    @events = Event.in_projects(@projects)
-    @events = event_filter.apply_filter(@events).with_associations
-    @events = @events.limit(20).offset(params[:offset] || 0)
+    @events = EventCollection
+      .new(@projects, offset: params[:offset].to_i, filter: event_filter)
+      .to_a
   end
 
   def user_actions

app/controllers/projects_controller.rb

@@ -301,10 +301,11 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def load_events
-    @events = @project.events.recent
-    @events = event_filter.apply_filter(@events).with_associations
-    limit = (params[:limit] || 20).to_i
-    @events = @events.limit(limit).offset(params[:offset] || 0)
+    projects = Project.where(id: @project.id)
+
+    @events = EventCollection
+      .new(projects, offset: params[:offset].to_i, filter: event_filter)
+      .to_a
   end
 
   def project_params

app/models/event.rb

@@ -48,6 +48,7 @@ class Event < ActiveRecord::Base
   belongs_to :author, class_name: "User"
   belongs_to :project
   belongs_to :target, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
+  has_one :push_event_payload, foreign_key: :event_id
 
   # For Hash only
   serialize :data # rubocop:disable Cop/ActiveRecordSerialize
@@ -55,19 +56,51 @@ class Event < ActiveRecord::Base
   # Callbacks
   after_create :reset_project_activity
   after_create :set_last_repository_updated_at, if: :push?
+  after_create :replicate_event_for_push_events_migration
 
   # Scopes
   scope :recent, -> { reorder(id: :desc) }
   scope :code_push, -> { where(action: PUSHED) }
 
-  scope :in_projects, ->(projects) do
-    where(project_id: projects.pluck(:id)).recent
+  scope :in_projects, -> (projects) do
+    sub_query = projects
+      .except(:order)
+      .select(1)
+      .where('projects.id = events.project_id')
+
+    where('EXISTS (?)', sub_query).recent
+  end
+
+  scope :with_associations, -> do
+    # We're using preload for "push_event_payload" as otherwise the association
+    # is not always available (depending on the query being built).
+    includes(:author, :project, project: :namespace)
+      .preload(:target, :push_event_payload)
   end
 
-  scope :with_associations, -> { includes(:author, :project, project: :namespace).preload(:target) }
   scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }
 
+  self.inheritance_column = 'action'
+
   class << self
+    def find_sti_class(action)
+      if action.to_i == PUSHED
+        PushEvent
+      else
+        Event
+      end
+    end
+
+    def subclass_from_attributes(attrs)
+      # Without this Rails will keep calling this method on the returned class,
+      # resulting in an infinite loop.
+      return unless self == Event
+
+      action = attrs.with_indifferent_access[inheritance_column].to_i
+
+      PushEvent if action == PUSHED
+    end
+
     # Update Gitlab::ContributionsCalendar#activity_dates if this changes
     def contributions
       where("action = ? OR (target_type IN (?) AND action IN (?)) OR (target_type = ? AND action = ?)",
@@ -290,6 +323,16 @@ class Event < ActiveRecord::Base
     @commits ||= (data[:commits] || []).reverse
   end
 
+  def commit_title
+    commit = commits.last
+
+    commit[:message] if commit
+  end
+
+  def commit_id
+    commit_to || commit_from
+  end
+
   def commits_count
     data[:total_commits_count] || commits.count || 0
   end
@@ -385,6 +428,16 @@ class Event < ActiveRecord::Base
     user ? author_id == user.id : false
   end
 
+  # We're manually replicating data into the new table since database triggers
+  # are not dumped to db/schema.rb. This could mean that a new installation
+  # would not have the triggers in place, thus losing events data in GitLab
+  # 10.0.
+  def replicate_event_for_push_events_migration
+    new_attributes = attributes.with_indifferent_access.except(:title, :data)
+
+    EventForMigration.create!(new_attributes)
+  end
+
   private
 
   def recent_update?

app/models/event_collection.rb

+# A collection of events to display in an event list.
+#
+# An EventCollection is meant to be used for displaying events to a user (e.g.
+# in a controller), it's not suitable for building queries that are used for
+# building other queries.
+class EventCollection
+  # To prevent users from putting too much pressure on the database by cycling
+  # through thousands of events we put a limit on the number of pages.
+  MAX_PAGE = 10
+
+  # projects - An ActiveRecord::Relation object that returns the projects for
+  #            which to retrieve events.
+  # filter - An EventFilter instance to use for filtering events.
+  def initialize(projects, limit: 20, offset: 0, filter: nil)
+    @projects = projects
+    @limit = limit
+    @offset = offset
+    @filter = filter
+  end
+
+  # Returns an Array containing the events.
+  def to_a
+    return [] if current_page > MAX_PAGE
+
+    relation = if Gitlab::Database.join_lateral_supported?
+                 relation_with_join_lateral
+               else
+                 relation_without_join_lateral
+               end
+
+    relation.with_associations.to_a
+  end
+
+  private
+
+  # Returns the events relation to use when JOIN LATERAL is not supported.
+  #
+  # This relation simply gets all the events for all authorized projects, then
+  # limits that set.
+  def relation_without_join_lateral
+    events = filtered_events.in_projects(projects)
+
+    paginate_events(events)
+  end
+
+  # Returns the events relation to use when JOIN LATERAL is supported.
+  #
+  # This relation is built using JOIN LATERAL, producing faster queries than a
+  # regular LIMIT + OFFSET approach.
+  def relation_with_join_lateral
+    projects_for_lateral = projects.select(:id).to_sql
+
+    lateral = filtered_events
+      .limit(limit_for_join_lateral)
+      .where('events.project_id = projects_for_lateral.id')
+      .to_sql
+
+    # The outer query does not need to re-apply the filters since the JOIN
+    # LATERAL body already takes care of this.
+    outer = base_relation
+      .from("(#{projects_for_lateral}) projects_for_lateral")
+      .joins("JOIN LATERAL (#{lateral}) AS #{Event.table_name} ON true")
+
+    paginate_events(outer)
+  end
+
+  def filtered_events
+    @filter ? @filter.apply_filter(base_relation) : base_relation
+  end
+
+  def paginate_events(events)
+    events.limit(@limit).offset(@offset)
+  end
+
+  def base_relation
+    # We want to have absolute control over the event queries being built, thus
+    # we're explicitly opting out of any default scopes that may be set.
+    Event.unscoped.recent
+  end
+
+  def limit_for_join_lateral
+    # Applying the OFFSET on the inside of a JOIN LATERAL leads to incorrect
+    # results. To work around this we need to increase the inner limit for every
+    # page.
+    #
+    # This means that on page 1 we use LIMIT 20, and an outer OFFSET of 0. On
+    # page 2 we use LIMIT 40 and an outer OFFSET of 20.
+    @limit + @offset
+  end
+
+  def current_page
+    (@offset / @limit) + 1
+  end
+
+  def projects
+    @projects.except(:order)
+  end
+end

app/models/event_for_migration.rb

+# This model is used to replicate events between the old "events" table and the
+# new "events_for_migration" table that will replace "events" in GitLab 10.0.
+class EventForMigration < ActiveRecord::Base
+  self.table_name = 'events_for_migration'
+end

app/models/push_event.rb

+class PushEvent < Event
+  # This validation exists so we can't accidentally use PushEvent with a
+  # different "action" value.
+  validate :validate_push_action
+
+  # Authors are required as they're used to display who pushed data.
+  #
+  # We're just validating the presence of the ID here as foreign key constraints
+  # should ensure the ID points to a valid user.
+  validates :author_id, presence: true
+
+  # The project is required to build links to commits, commit ranges, etc.
+  #
+  # We're just validating the presence of the ID here as foreign key constraints
+  # should ensure the ID points to a valid project.
+  validates :project_id, presence: true
+
+  # The "data" field must not be set for push events since it's not used and a
+  # waste of space.
+  validates :data, absence: true
+
+  # These fields are also not used for push events, thus storing them would be a
+  # waste.
+  validates :target_id, absence: true
+  validates :target_type, absence: true
+
+  def self.sti_name
+    PUSHED
+  end
+
+  def push?
+    true
+  end
+
+  def push_with_commits?
+    !!(commit_from && commit_to)
+  end
+
+  def tag?
+    return super unless push_event_payload
+
+    push_event_payload.tag?
+  end
+
+  def branch?
+    return super unless push_event_payload
+
+    push_event_payload.branch?
+  end
+
+  def valid_push?
+    return super unless push_event_payload
+
+    push_event_payload.ref.present?
+  end
+
+  def new_ref?
+    return super unless push_event_payload
+
+    push_event_payload.created?
+  end
+
+  def rm_ref?
+    return super unless push_event_payload
+
+    push_event_payload.removed?
+  end
+
+  def commit_from
+    return super unless push_event_payload
+
+    push_event_payload.commit_from
+  end
+
+  def commit_to
+    return super unless push_event_payload
+
+    push_event_payload.commit_to
+  end
+
+  def ref_name
+    return super unless push_event_payload
+
+    push_event_payload.ref
+  end
+
+  def ref_type
+    return super unless push_event_payload
+
+    push_event_payload.ref_type
+  end
+
+  def branch_name
+    return super unless push_event_payload
+
+    ref_name
+  end
+
+  def tag_name
+    return super unless push_event_payload
+
+    ref_name
+  end
+
+  def commit_title
+    return super unless push_event_payload
+
+    push_event_payload.commit_title
+  end
+
+  def commit_id
+    commit_to || commit_from
+  end
+
+  def commits_count
+    return super unless push_event_payload
+
+    push_event_payload.commit_count
+  end
+
+  def validate_push_action
+    return if action == PUSHED
+
+    errors.add(:action, "the action #{action.inspect} is not valid")
+  end
+end

app/models/push_event_payload.rb

+class PushEventPayload < ActiveRecord::Base
+  include ShaAttribute
+
+  belongs_to :event, inverse_of: :push_event_payload
+
+  validates :event_id, :commit_count, :action, :ref_type, presence: true
+  validates :commit_title, length: { maximum: 70 }
+
+  sha_attribute :commit_from
+  sha_attribute :commit_to
+
+  enum action: {
+    created: 0,
+    removed: 1,
+    pushed: 2
+  }
+
+  enum ref_type: {
+    branch: 0,
+    tag: 1
+  }
+end

app/services/event_create_service.rb

@@ -71,7 +71,14 @@ class EventCreateService
   end
 
   def push(project, current_user, push_data)
-    create_event(project, current_user, Event::PUSHED, data: push_data)
+    # We're using an explicit transaction here so that any errors that may occur
+    # when creating push payload data will result in the event creation being
+    # rolled back as well.
+    Event.transaction do
+      event = create_event(project, current_user, Event::PUSHED)
+
+      PushEventPayloadService.new(event, push_data).execute
+    end
 
     Users::ActivityService.new(current_user, 'push').execute
   end

app/services/push_event_payload_service.rb

+# Service class for creating push event payloads as stored in the
+# "push_event_payloads" table.
+#
+# Example:
+#
+#     data = Gitlab::DataBuilder::Push.build(...)
+#     event = Event.create(...)
+#
+#     PushEventPayloadService.new(event, data).execute
+class PushEventPayloadService
+  # event - The event this push payload belongs to.
+  # push_data - A Hash produced by `Gitlab::DataBuilder::Push.build` to use for
+  #             building the push payload.
+  def initialize(event, push_data)
+    @event = event
+    @push_data = push_data
+  end
+
+  # Creates and returns a new PushEventPayload row.
+  #
+  # This method will raise upon encountering validation errors.
+  #
+  # Returns an instance of PushEventPayload.
+  def execute
+    @event.build_push_event_payload(
+      commit_count: commit_count,
+      action: action,
+      ref_type: ref_type,
+      commit_from: commit_from_id,
+      commit_to: commit_to_id,
+      ref: trimmed_ref,
+      commit_title: commit_title,
+      event_id: @event.id
+    )
+
+    @event.push_event_payload.save!
+    @event.push_event_payload
+  end
+
+  # Returns the commit title to use.
+  #
+  # The commit title is limited to the first line and a maximum of 70
+  # characters.
+  def commit_title
+    commit = @push_data.fetch(:commits).last
+
+    return nil unless commit && commit[:message]
+
+    raw_msg = commit[:message]
+
+    # Find where the first line ends, without turning the entire message into an
+    # Array of lines (this is a waste of memory for large commit messages).
+    index = raw_msg.index("\n")
+    message = index ? raw_msg[0..index] : raw_msg
+
+    message.strip.truncate(70)
+  end
+
+  def commit_from_id
+    if create?
+      nil
+    else
+      revision_before
+    end
+  end
+
+  def commit_to_id
+    if remove?
+      nil
+    else
+      revision_after
+    end
+  end
+
+  def commit_count
+    @push_data.fetch(:total_commits_count)
+  end
+
+  def ref
+    @push_data.fetch(:ref)
+  end
+
+  def revision_before
+    @push_data.fetch(:before)
+  end
+
+  def revision_after
+    @push_data.fetch(:after)
+  end
+
+  def trimmed_ref
+    Gitlab::Git.ref_name(ref)
+  end
+
+  def create?
+    Gitlab::Git.blank_ref?(revision_before)
+  end
+
+  def remove?
+    Gitlab::Git.blank_ref?(revision_after)
+  end
+
+  def action
+    if create?
+      :created
+    elsif remove?
+      :removed
+    else
+      :pushed
+    end
+  end
+
+  def ref_type
+    if Gitlab::Git.tag_ref?(ref)
+      :tag
+    else
+      :branch
+    end
+  end
+end

app/views/events/_commit.html.haml

 %li.commit
   .commit-row-title
-    = link_to truncate_sha(commit[:id]), project_commit_path(project, commit[:id]), class: "commit-sha", alt: '', title: truncate_sha(commit[:id])
+    = link_to truncate_sha(event.commit_id), project_commit_path(project, event.commit_id), class: "commit-sha", alt: '', title: truncate_sha(event.commit_id)
     ·
-    = markdown event_commit_title(commit[:message]), project: project, pipeline: :single_line, author: event.author
+    = markdown event_commit_title(event.commit_title), project: project, pipeline: :single_line, author: event.author

app/views/events/_event_push.atom.haml

 %div{ xmlns: "http://www.w3.org/1999/xhtml" }
-  - event.commits.first(15).each do |commit|
-    %p
-      %strong= commit[:author][:name]
-      = link_to "(##{truncate_sha(commit[:id])})", project_commit_path(event.project, id: commit[:id])
-      %i
-        at
-        = commit[:timestamp].to_time.to_s(:short)
-    %blockquote= markdown(escape_once(commit[:message]), pipeline: :atom, project: event.project, author: event.author)
-  - if event.commits_count > 15
+  %p
+    %strong= event.author_name
+    = link_to "(#{truncate_sha(event.commit_id)})", project_commit_path(event.project, event.commit_id)
+    %i
+      at
+      = event.created_at.to_s(:short)
+  %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author)
+  - if event.commits_count > 1
     %p
       %i
         \... and
-        = pluralize(event.commits_count - 15, "more commit")
+        = pluralize(event.commits_count - 1, "more commit")

app/views/events/event/_push.html.haml

@@ -14,9 +14,7 @@
 - if event.push_with_commits?
   .event-body
     %ul.well-list.event_commits
-      - few_commits = event.commits[0...2]
-      - few_commits.each do |commit|
-        = render "events/commit", commit: commit, project: project, event: event
+      = render "events/commit", project: project, event: event
 
       - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user)
       - if event.commits_count > 1
@@ -44,9 +42,6 @@
           = link_to create_mr_path(project.default_branch, event.ref_name, project) do
             Create Merge Request
 - elsif event.rm_ref?
-  - repository = project.repository
-  - last_commit = repository.commit(event.commit_from)
-  - if last_commit
-    .event-body
-      %ul.well-list.event_commits
-        = render "events/commit", commit: last_commit, project: project, event: event
+  .event-body
+    %ul.well-list.event_commits
+      = render "events/commit", project: project, event: event

changelogs/unreleased/migrate-events-into-a-new-format.yml

+---
+title: Migrate events into a new format to reduce the storage necessary and improve performance
+merge_request:
+author:

changelogs/unreleased/use-a-specialized-class-for-querying-events.yml

+---
+title: Use a specialized class for querying events to improve performance
+merge_request:
+author:

db/migrate/20170608152747_prepare_events_table_for_push_events_migration.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class PrepareEventsTableForPushEventsMigration < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    # The order of these columns is deliberate and results in the following
+    # columns and sizes:
+    #
+    # * id (4 bytes)
+    # * project_id (4 bytes)
+    # * author_id (4 bytes)
+    # * target_id (4 bytes)
+    # * created_at (8 bytes)
+    # * updated_at (8 bytes)
+    # * action (2 bytes)
+    # * target_type (variable)
+    #
+    # Unfortunately we can't make the "id" column a bigint/bigserial as Rails 4
+    # does not support this properly.
+    create_table :events_for_migration do |t|
+      t.references :project,
+                   index: true,
+                   foreign_key: { on_delete: :cascade }
+
+      t.integer :author_id, index: true, null: false
+      t.integer :target_id
+
+      t.timestamps_with_timezone null: false
+
+      t.integer :action, null: false, limit: 2, index: true
+      t.string :target_type
+
+      t.index %i[target_type target_id]
+    end
+
+    # t.references doesn't like it when the column name doesn't make the table
+    # name so we have to add the foreign key separately.
+    add_concurrent_foreign_key(:events_for_migration, :users, column: :author_id)
+  end
+
+  def down
+    drop_table :events_for_migration
+  end
+end

db/migrate/20170608152748_create_push_event_payloads_tables.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CreatePushEventPayloadsTables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    create_table :push_event_payloads, id: false do |t|
+      t.bigint :commit_count, null: false
+
+      t.integer :event_id, null: false
+      t.integer :action, null: false, limit: 2
+      t.integer :ref_type, null: false, limit: 2
+
+      t.binary :commit_from
+      t.binary :commit_to
+
+      t.text :ref
+      t.string :commit_title, limit: 70
+
+      t.index :event_id, unique: true
+    end
+
+    # We're adding a foreign key to the _shadow_ table, and this is deliberate.
+    # By using the shadow table we don't have to recreate/revalidate this
+    # foreign key after swapping the "events_for_migration" and "events" tables.
+    #
+    # The "events_for_migration" table has a foreign key to "projects.id"
+    # ensuring that project removals also remove events from the shadow table
+    # (and thus also from this table).
+    add_concurrent_foreign_key(
+      :push_event_payloads,
+      :events_for_migration,
+      column: :event_id
+    )
+  end
+
+  def down
+    drop_table :push_event_payloads
+  end
+end

db/migrate/20170727123534_add_index_on_events_project_id_id.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndexOnEventsProjectIdId < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  COLUMNS = %i[project_id id].freeze
+  TABLES = %i[events events_for_migration].freeze
+
+  disable_ddl_transaction!
+
+  def up
+    TABLES.each do |table|
+      add_concurrent_index(table, COLUMNS) unless index_exists?(table, COLUMNS)
+
+      # We remove the index _after_ adding the new one since MySQL doesn't let
+      # you remove an index when a foreign key exists for the same column.
+      if index_exists?(table, :project_id)
+        remove_concurrent_index(table, :project_id)
+      end
+    end
+  end
+
+  def down
+    TABLES.each do |table|
+      unless index_exists?(table, :project_id)
+        add_concurrent_index(table, :project_id)
+      end
+
+      unless index_exists?(table, COLUMNS)
+        remove_concurrent_index(table, COLUMNS)
+      end
+    end
+  end
+end

db/post_migrate/20170627101016_schedule_event_migrations.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class ScheduleEventMigrations < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BUFFER_SIZE = 1000
+
+  disable_ddl_transaction!
+
+  class Event < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'events'
+  end
+
+  def up
+    jobs = []
+
+    Event.each_batch(of: 1000) do |relation|
+      min, max = relation.pluck('MIN(id), MAX(id)').first
+
+      if jobs.length == BUFFER_SIZE
+        # We push multiple jobs at a time to reduce the time spent in
+        # Sidekiq/Redis operations. We're using this buffer based approach so we
+        # don't need to run additional queries for every range.
+        BackgroundMigrationWorker.perform_bulk(jobs)
+        jobs.clear
+      end
+
+      jobs << ['MigrateEventsToPushEventPayloads', [min, max]]
+    end
+
+    BackgroundMigrationWorker.perform_bulk(jobs) unless jobs.empty?
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -530,10 +530,25 @@ ActiveRecord::Schema.define(version: 20170809142252) do
   add_index "events", ["action"], name: "index_events_on_action", using: :btree
   add_index "events", ["author_id"], name: "index_events_on_author_id", using: :btree
   add_index "events", ["created_at"], name: "index_events_on_created_at", using: :btree
-  add_index "events", ["project_id"], name: "index_events_on_project_id", using: :btree
+  add_index "events", ["project_id", "id"], name: "index_events_on_project_id_and_id", using: :btree
   add_index "events", ["target_id"], name: "index_events_on_target_id", using: :btree
   add_index "events", ["target_type"], name: "index_events_on_target_type", using: :btree
 
+  create_table "events_for_migration", force: :cascade do |t|
+    t.integer "project_id"
+    t.integer "author_id", null: false
+    t.integer "target_id"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.integer "action", limit: 2, null: false
+    t.string "target_type"
+  end
+
+  add_index "events_for_migration", ["action"], name: "index_events_for_migration_on_action", using: :btree
+  add_index "events_for_migration", ["author_id"], name: "index_events_for_migration_on_author_id", using: :btree
+  add_index "events_for_migration", ["project_id", "id"], name: "index_events_for_migration_on_project_id_and_id", using: :btree
+  add_index "events_for_migration", ["target_type", "target_id"], name: "index_events_for_migration_on_target_type_and_target_id", using: :btree
+
   create_table "feature_gates", force: :cascade do |t|
     t.string "feature_key", null: false
     t.string "key", null: false
@@ -1254,6 +1269,19 @@ ActiveRecord::Schema.define(version: 20170809142252) do
 
   add_index "protected_tags", ["project_id"], name: "index_protected_tags_on_project_id", using: :btree
 
+  create_table "push_event_payloads", id: false, force: :cascade do |t|
+    t.integer "commit_count", limit: 8, null: false
+    t.integer "event_id", null: false
+    t.integer "action", limit: 2, null: false
+    t.integer "ref_type", limit: 2, null: false
+    t.binary "commit_from"
+    t.binary "commit_to"
+    t.text "ref"
+    t.string "commit_title", limit: 70
+  end
+
+  add_index "push_event_payloads", ["event_id"], name: "index_push_event_payloads_on_event_id", unique: true, using: :btree
+
   create_table "redirect_routes", force: :cascade do |t|
     t.integer "source_id", null: false
     t.string "source_type", null: false
@@ -1654,6 +1682,8 @@ ActiveRecord::Schema.define(version: 20170809142252) do
   add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade
   add_foreign_key "environments", "projects", name: "fk_d1c8c1da6a", on_delete: :cascade
   add_foreign_key "events", "projects", name: "fk_0434b48643", on_delete: :cascade
+  add_foreign_key "events_for_migration", "projects", on_delete: :cascade
+  add_foreign_key "events_for_migration", "users", column: "author_id", name: "fk_edfd187b6f", on_delete: :cascade
   add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade
   add_foreign_key "gpg_keys", "users", on_delete: :cascade
   add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify
@@ -1696,6 +1726,7 @@ ActiveRecord::Schema.define(version: 20170809142252) do
   add_foreign_key "protected_tag_create_access_levels", "protected_tags"
   add_foreign_key "protected_tag_create_access_levels", "users"
   add_foreign_key "protected_tags", "projects", name: "fk_8e4af87648", on_delete: :cascade
+  add_foreign_key "push_event_payloads", "events_for_migration", column: "event_id", name: "fk_36c74129da", on_delete: :cascade
   add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade
   add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade
   add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade

doc/api/events.md

@@ -79,7 +79,6 @@ Example response:
     "target_id":160,
     "target_type":"Issue",
     "author_id":25,
-    "data":null,
     "target_title":"Qui natus eos odio tempore et quaerat consequuntur ducimus cupiditate quis.",
     "created_at":"2017-02-09T10:43:19.667Z",
     "author":{
@@ -99,7 +98,6 @@ Example response:
     "target_id":159,
     "target_type":"Issue",
     "author_id":21,
-    "data":null,
     "target_title":"Nostrum enim non et sed optio illo deleniti non.",
     "created_at":"2017-02-09T10:43:19.426Z",
     "author":{
@@ -151,7 +149,6 @@ Example response:
     "target_id": 830,
     "target_type": "Issue",
     "author_id": 1,
-    "data": null,
     "target_title": "Public project search field",
     "author": {
       "name": "Dmitriy Zaporozhets",
@@ -166,7 +163,7 @@ Example response:
   {
     "title": null,
     "project_id": 15,
-    "action_name": "opened",
+    "action_name": "pushed",
     "target_id": null,
     "target_type": null,
     "author_id": 1,
@@ -179,31 +176,14 @@ Example response:
       "web_url": "http://localhost:3000/root"
     },
     "author_username": "john",
-    "data": {
-      "before": "50d4420237a9de7be1304607147aec22e4a14af7",
-      "after": "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
-      "ref": "refs/heads/master",
-      "user_id": 1,
-      "user_name": "Dmitriy Zaporozhets",
-      "repository": {
-        "name": "gitlabhq",
-        "url": "git@dev.gitlab.org:gitlab/gitlabhq.git",
-        "description": "GitLab: self hosted Git management software. \r\nDistributed under the MIT License.",
-        "homepage": "https://dev.gitlab.org/gitlab/gitlabhq"
-      },
-      "commits": [
-        {
-          "id": "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
-          "message": "Add simple search to projects in public area",
-          "timestamp": "2013-05-13T18:18:08+00:00",
-          "url": "https://dev.gitlab.org/gitlab/gitlabhq/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428",
-          "author": {
-            "name": "Dmitriy Zaporozhets",
-            "email": "dmitriy.zaporozhets@gmail.com"
-          }
-        }
-      ],
-      "total_commits_count": 1
+    "push_data": {
+      "commit_count": 1,
+      "action": "pushed",
+      "ref_type": "branch",
+      "commit_from": "50d4420237a9de7be1304607147aec22e4a14af7",
+      "commit_to": "c5feabde2d8cd023215af4d2ceeb7a64839fc428",
+      "ref": "master",
+      "commit_title": "Add simple search to projects in public area"
     },
     "target_title": null
   },
@@ -214,7 +194,6 @@ Example response:
     "target_id": 840,
     "target_type": "Issue",
     "author_id": 1,
-    "data": null,
     "target_title": "Finish & merge Code search PR",
     "author": {
       "name": "Dmitriy Zaporozhets",
@@ -233,7 +212,6 @@ Example response:
     "target_id": 1312,
     "target_type": "Note",
     "author_id": 1,
-    "data": null,
     "target_title": null,
     "created_at": "2015-12-04T10:33:58.089Z",
     "note": {
@@ -305,7 +283,6 @@ Example response:
     "target_iid":160,
     "target_type":"Issue",
     "author_id":25,
-    "data":null,
     "target_title":"Qui natus eos odio tempore et quaerat consequuntur ducimus cupiditate quis.",
     "created_at":"2017-02-09T10:43:19.667Z",
     "author":{
@@ -326,7 +303,6 @@ Example response:
     "target_iid":159,
     "target_type":"Issue",
     "author_id":21,
-    "data":null,
     "target_title":"Nostrum enim non et sed optio illo deleniti non.",
     "created_at":"2017-02-09T10:43:19.426Z",
     "author":{

features/steps/shared/project.rb

@@ -71,28 +71,14 @@ module SharedProject
 
   step 'project "Shop" has push event' do
     @project = Project.find_by(name: "Shop")
-
-    data = {
-      before: Gitlab::Git::BLANK_SHA,
-      after: "6d394385cf567f80a8fd85055db1ab4c5295806f",
-      ref: "refs/heads/fix",
-      user_id: @user.id,
-      user_name: @user.name,
-      repository: {
-        name: @project.name,
-        url: "localhost/rubinius",
-        description: "",
-        homepage: "localhost/rubinius",
-        private: true
-      }
-    }
-
-    @event = Event.create(
-      project: @project,
-      action: Event::PUSHED,
-      data: data,
-      author_id: @user.id
-    )
+    @event = create(:push_event, project: @project, author: @user)
+
+    create(:push_event_payload,
+           event: @event,
+           action: :created,
+           commit_to: '6d394385cf567f80a8fd85055db1ab4c5295806f',
+           ref: 'fix',
+           commit_count: 1)
   end
 
   step 'I should see project "Shop" activity feed' do

lib/api/entities.rb

@@ -497,14 +497,24 @@ module API
       expose :author, using: Entities::UserBasic
     end
 
+    class PushEventPayload < Grape::Entity
+      expose :commit_count, :action, :ref_type, :commit_from, :commit_to
+      expose :ref, :commit_title
+    end
+
     class Event < Grape::Entity
       expose :title, :project_id, :action_name
       expose :target_id, :target_iid, :target_type, :author_id
-      expose :data, :target_title
+      expose :target_title
       expose :created_at
       expose :note, using: Entities::Note, if: ->(event, options) { event.note? }
       expose :author, using: Entities::UserBasic, if: ->(event, options) { event.author }
 
+      expose :push_event_payload,
+        as: :push_data,
+        using: PushEventPayload,
+        if: -> (event, _) { event.push? }
+
       expose :author_username do |event, options|
         event.author&.username
       end

lib/api/v3/entities.rb

@@ -25,14 +25,24 @@ module API
         expose(:downvote?)  { |note| false }
       end
 
+      class PushEventPayload < Grape::Entity
+        expose :commit_count, :action, :ref_type, :commit_from, :commit_to
+        expose :ref, :commit_title
+      end
+
       class Event < Grape::Entity
         expose :title, :project_id, :action_name
         expose :target_id, :target_type, :author_id
-        expose :data, :target_title
+        expose :target_title
         expose :created_at
         expose :note, using: Entities::Note, if: ->(event, options) { event.note? }
         expose :author, using: ::API::Entities::UserBasic, if: ->(event, options) { event.author }
 
+        expose :push_event_payload,
+          as: :push_data,
+          using: PushEventPayload,
+          if: -> (event, _) { event.push? }
+
         expose :author_username do |event, options|
           event.author&.username
         end

lib/gitlab/background_migration/migrate_events_to_push_event_payloads.rb

+module Gitlab
+  module BackgroundMigration
+    # Class that migrates events for the new push event payloads setup. All
+    # events are copied to a shadow table, and push events will also have a row
+    # created in the push_event_payloads table.
+    class MigrateEventsToPushEventPayloads
+      class Event < ActiveRecord::Base
+        self.table_name = 'events'
+
+        serialize :data
+
+        BLANK_REF = ('0' * 40).freeze
+        TAG_REF_PREFIX = 'refs/tags/'.freeze
+        MAX_INDEX = 69
+        PUSHED = 5
+
+        def push_event?
+          action == PUSHED && data.present?
+        end
+
+        def commit_title
+          commit = commits.last
+
+          return nil unless commit && commit[:message]
+
+          index = commit[:message].index("\n")
+          message = index ? commit[:message][0..index] : commit[:message]
+
+          message.strip.truncate(70)
+        end
+
+        def commit_from_sha
+          if create?
+            nil
+          else
+            data[:before]
+          end
+        end
+
+        def commit_to_sha
+          if remove?
+            nil
+          else
+            data[:after]
+          end
+        end
+
+        def data
+          super || {}
+        end
+
+        def commits
+          data[:commits] || []
+        end
+
+        def commit_count
+          data[:total_commits_count] || 0
+        end
+
+        def ref
+          data[:ref]
+        end
+
+        def trimmed_ref_name
+          if ref_type == :tag
+            ref[10..-1]
+          else
+            ref[11..-1]
+          end
+        end
+
+        def create?
+          data[:before] == BLANK_REF
+        end
+
+        def remove?
+          data[:after] == BLANK_REF
+        end
+
+        def push_action
+          if create?
+            :created
+          elsif remove?
+            :removed
+          else
+            :pushed
+          end
+        end
+
+        def ref_type
+          if ref.start_with?(TAG_REF_PREFIX)
+            :tag
+          else
+            :branch
+          end
+        end
+      end
+
+      class EventForMigration < ActiveRecord::Base
+        self.table_name = 'events_for_migration'
+      end
+
+      class PushEventPayload < ActiveRecord::Base
+        self.table_name = 'push_event_payloads'
+
+        enum action: {
+          created: 0,
+          removed: 1,
+          pushed: 2
+        }
+
+        enum ref_type: {
+          branch: 0,
+          tag: 1
+        }
+      end
+
+      # start_id - The start ID of the range of events to process
+      # end_id - The end ID of the range to process.
+      def perform(start_id, end_id)
+        return unless migrate?
+
+        find_events(start_id, end_id).each { |event| process_event(event) }
+      end
+
+      def process_event(event)
+        replicate_event(event)
+        create_push_event_payload(event) if event.push_event?
+      end
+
+      def replicate_event(event)
+        new_attributes = event.attributes
+          .with_indifferent_access.except(:title, :data)
+
+        EventForMigration.create!(new_attributes)
+      rescue ActiveRecord::InvalidForeignKey
+        # A foreign key error means the associated event was removed. In this
+        # case we'll just skip migrating the event.
+      end
+
+      def create_push_event_payload(event)
+        commit_from = pack(event.commit_from_sha)
+        commit_to = pack(event.commit_to_sha)
+
+        PushEventPayload.create!(
+          event_id: event.id,
+          commit_count: event.commit_count,
+          ref_type: event.ref_type,
+          action: event.push_action,
+          commit_from: commit_from,
+          commit_to: commit_to,
+          ref: event.trimmed_ref_name,
+          commit_title: event.commit_title
+        )
+      rescue ActiveRecord::InvalidForeignKey
+        # A foreign key error means the associated event was removed. In this
+        # case we'll just skip migrating the event.
+      end
+
+      def find_events(start_id, end_id)
+        Event
+          .where('NOT EXISTS (SELECT true FROM events_for_migration WHERE events_for_migration.id = events.id)')
+          .where(id: start_id..end_id)
+      end
+
+      def migrate?
+        Event.table_exists? && PushEventPayload.table_exists? &&
+          EventForMigration.table_exists?
+      end
+
+      def pack(value)
+        value ? [value].pack('H*') : nil
+      end
+    end
+  end
+end

lib/gitlab/database.rb

@@ -25,6 +25,10 @@ module Gitlab
       database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1]
     end
 
+    def self.join_lateral_supported?
+      postgresql? && version.to_f >= 9.3
+    end
+
     def self.nulls_last_order(field, direction = 'ASC')
       order = "#{field} #{direction}"
 

lib/gitlab/import_export/import_export.yml

@@ -3,18 +3,22 @@ project_tree:
   - labels:
       :priorities
   - milestones:
-    - :events
+    - events:
+      - :push_event_payload
   - issues:
-    - :events
+    - events:
+      - :push_event_payload
     - :timelogs
     - notes:
       - :author
-      - :events
+      - events:
+        - :push_event_payload
     - label_links:
       - label:
           :priorities
     - milestone:
-      - :events
+      - events:
+        - :push_event_payload
   - snippets:
     - :award_emoji
     - notes:
@@ -25,21 +29,25 @@ project_tree:
   - merge_requests:
     - notes:
       - :author
-      - :events
+      - events:
+        - :push_event_payload
     - merge_request_diff:
       - :merge_request_diff_commits
       - :merge_request_diff_files
-    - :events
+    - events:
+      - :push_event_payload
     - :timelogs
     - label_links:
       - label:
           :priorities
     - milestone:
-      - :events
+      - events:
+        - :push_event_payload
   - pipelines:
     - notes:
       - :author
-      - :events
+      - events:
+        - :push_event_payload
     - :stages
     - :statuses
   - :triggers
@@ -107,6 +115,8 @@ excluded_attributes:
   statuses:
     - :trace
     - :token
+  push_event_payload:
+    - :event_id
 
 methods:
   labels:

spec/controllers/users_controller_spec.rb

@@ -92,8 +92,14 @@ describe UsersController do
       before do
         sign_in(user)
         project.team << [user, :developer]
-        EventCreateService.new.push(project, user, [])
-        EventCreateService.new.push(forked_project, user, [])
+
+        push_data = Gitlab::DataBuilder::Push.build_sample(project, user)
+
+        fork_push_data = Gitlab::DataBuilder::Push
+          .build_sample(forked_project, user)
+
+        EventCreateService.new.push(project, user, push_data)
+        EventCreateService.new.push(forked_project, user, fork_push_data)
       end
 
       it 'includes forked projects' do

spec/factories/events.rb

@@ -2,6 +2,7 @@ FactoryGirl.define do
   factory :event do
     project
     author factory: :user
+    action Event::JOINED
 
     trait(:created)   { action Event::CREATED }
     trait(:updated)   { action Event::UPDATED }
@@ -20,4 +21,19 @@ FactoryGirl.define do
       target factory: :closed_issue
     end
   end
+
+  factory :push_event, class: PushEvent do
+    project factory: :project_empty_repo
+    author factory: :user
+    action Event::PUSHED
+  end
+
+  factory :push_event_payload do
+    event
+    commit_count 1
+    action :pushed
+    ref_type :branch
+    ref 'master'
+    commit_to '3cdce97ed87c91368561584e7358f4d46e3e173c'
+  end
 end

spec/features/calendar_spec.rb

@@ -42,14 +42,14 @@ feature 'Contributions Calendar', :js do
   end
 
   def push_code_contribution
-    push_params = {
-      project: contributed_project,
-      action: Event::PUSHED,
-      author_id: user.id,
-      data: { commit_count: 3 }
-    }
-
-    Event.create(push_params)
+    event = create(:push_event, project: contributed_project, author: user)
+
+    create(:push_event_payload,
+           event: event,
+           commit_from: '11f9ac0a48b62cef25eedede4c1819964f08d5ce',
+           commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+           commit_count: 3,
+           ref: 'master')
   end
 
   def note_comment_contribution

spec/features/dashboard/activity_spec.rb

@@ -23,27 +23,19 @@ feature 'Dashboard > Activity' do
       create(:merge_request, author: user, source_project: project, target_project: project)
     end
 
-    let(:push_event_data) do
-      {
-        before: Gitlab::Git::BLANK_SHA,
-        after: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e',
-        ref: 'refs/heads/new_design',
-        user_id: user.id,
-        user_name: user.name,
-        repository: {
-          name: project.name,
-          url: 'localhost/rubinius',
-          description: '',
-          homepage: 'localhost/rubinius',
-          private: true
-        }
-      }
-    end
-
     let(:note) { create(:note, project: project, noteable: merge_request) }
 
     let!(:push_event) do
-      create(:event, :pushed, data: push_event_data, project: project, author: user)
+      event = create(:push_event, project: project, author: user)
+
+      create(:push_event_payload,
+             event: event,
+             action: :created,
+             commit_to: '0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e',
+             ref: 'new_design',
+             commit_count: 1)
+
+      event
     end
 
     let!(:merged_event) do

spec/finders/contributed_projects_finder_spec.rb

@@ -14,8 +14,8 @@ describe ContributedProjectsFinder do
     private_project.add_developer(current_user)
     public_project.add_master(source_user)
 
-    create(:event, :pushed, project: public_project, target: public_project, author: source_user)
-    create(:event, :pushed, project: private_project, target: private_project, author: source_user)
+    create(:push_event, project: public_project, author: source_user)
+    create(:push_event, project: private_project, author: source_user)
   end
 
   describe 'without a current user' do

spec/lib/event_filter_spec.rb

@@ -5,7 +5,7 @@ describe EventFilter do
     let(:source_user) { create(:user) }
     let!(:public_project) { create(:project, :public) }
 
-    let!(:push_event)     { create(:event, :pushed,    project: public_project, target: public_project, author: source_user) }
+    let!(:push_event)     { create(:push_event,        project: public_project, author: source_user) }
     let!(:merged_event)   { create(:event, :merged,    project: public_project, target: public_project, author: source_user) }
     let!(:created_event)  { create(:event, :created,   project: public_project, target: public_project, author: source_user) }
     let!(:updated_event)  { create(:event, :updated,   project: public_project, target: public_project, author: source_user) }

spec/lib/gitlab/database_spec.rb

@@ -51,6 +51,28 @@ describe Gitlab::Database do
     end
   end
 
+  describe '.join_lateral_supported?' do
+    it 'returns false when using MySQL' do
+      allow(described_class).to receive(:postgresql?).and_return(false)
+
+      expect(described_class.join_lateral_supported?).to eq(false)
+    end
+
+    it 'returns false when using PostgreSQL 9.2' do
+      allow(described_class).to receive(:postgresql?).and_return(true)
+      allow(described_class).to receive(:version).and_return('9.2.1')
+
+      expect(described_class.join_lateral_supported?).to eq(false)
+    end
+
+    it 'returns true when using PostgreSQL 9.3.0 or newer' do
+      allow(described_class).to receive(:postgresql?).and_return(true)
+      allow(described_class).to receive(:version).and_return('9.3.0')
+
+      expect(described_class.join_lateral_supported?).to eq(true)
+    end
+  end
+
   describe '.nulls_last_order' do
     context 'when using PostgreSQL' do
       before do

spec/lib/gitlab/import_export/all_models.yml

@@ -22,6 +22,7 @@ events:
 - author
 - project
 - target
+- push_event_payload
 notes:
 - award_emoji
 - project
@@ -272,3 +273,5 @@ timelogs:
 - issue
 - merge_request
 - user
+push_event_payload:
+- event

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -36,6 +36,14 @@ Event:
 - updated_at
 - action
 - author_id
+PushEventPayload:
+- commit_count
+- action
+- ref_type
+- commit_from
+- commit_to
+- ref
+- commit_title
 Note:
 - id
 - note

spec/models/event_collection_spec.rb

+require 'spec_helper'
+
+describe EventCollection do
+  describe '#to_a' do
+    let(:project) { create(:project_empty_repo) }
+    let(:projects) { Project.where(id: project.id) }
+    let(:user) { create(:user) }
+
+    before do
+      20.times do
+        event = create(:push_event, project: project, author: user)
+
+        create(:push_event_payload, event: event)
+      end
+
+      create(:closed_issue_event, project: project, author: user)
+    end
+
+    it 'returns an Array of events' do
+      events = described_class.new(projects).to_a
+
+      expect(events).to be_an_instance_of(Array)
+    end
+
+    it 'applies a limit to the number of events' do
+      events = described_class.new(projects).to_a
+
+      expect(events.length).to eq(20)
+    end
+
+    it 'can paginate through events' do
+      events = described_class.new(projects, offset: 20).to_a
+
+      expect(events.length).to eq(1)
+    end
+
+    it 'returns an empty Array when crossing the maximum page number' do
+      events = described_class.new(projects, limit: 1, offset: 15).to_a
+
+      expect(events).to be_empty
+    end
+
+    it 'allows filtering of events using an EventFilter' do
+      filter = EventFilter.new(EventFilter.issue)
+      events = described_class.new(projects, filter: filter).to_a
+
+      expect(events.length).to eq(1)
+      expect(events[0].action).to eq(Event::CLOSED)
+    end
+  end
+end

spec/models/event_spec.rb

@@ -304,27 +304,15 @@ describe Event do
     end
   end
 
-  def create_push_event(project, user, attrs = {})
-    data = {
-      before: Gitlab::Git::BLANK_SHA,
-      after: "0220c11b9a3e6c69dc8fd35321254ca9a7b98f7e",
-      ref: "refs/heads/master",
-      user_id: user.id,
-      user_name: user.name,
-      repository: {
-        name: project.name,
-        url: "localhost/rubinius",
-        description: "",
-        homepage: "localhost/rubinius",
-        private: true
-      }
-    }
-
-    described_class.create({
-      project: project,
-      action: described_class::PUSHED,
-      data: data,
-      author_id: user.id
-    }.merge!(attrs))
+  def create_push_event(project, user)
+    event = create(:push_event, project: project, author: user)
+
+    create(:push_event_payload,
+           event: event,
+           commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+           commit_count: 0,
+           ref: 'master')
+
+    event
   end
 end

spec/models/members/project_member_spec.rb

@@ -149,7 +149,7 @@ describe ProjectMember do
   describe 'notifications' do
     describe '#after_accept_request' do
       it 'calls NotificationService.new_project_member' do
-        member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
+        member = create(:project_member, user: create(:user), requested_at: Time.now)
 
         expect_any_instance_of(NotificationService).to receive(:new_project_member)
 

spec/models/project_spec.rb

@@ -485,7 +485,7 @@ describe Project do
 
     describe 'last_activity' do
       it 'alias last_activity to last_event' do
-        last_event = create(:event, project: project)
+        last_event = create(:event, :closed, project: project)
 
         expect(project.last_activity).to eq(last_event)
       end
@@ -493,7 +493,7 @@ describe Project do
 
     describe 'last_activity_date' do
       it 'returns the creation date of the project\'s last event if present' do
-        new_event = create(:event, project: project, created_at: Time.now)
+        new_event = create(:event, :closed, project: project, created_at: Time.now)
 
         project.reload
         expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i)

spec/models/push_event_payload_spec.rb

+require 'spec_helper'
+
+describe PushEventPayload do
+  describe 'saving payloads' do
+    it 'does not allow commit messages longer than 70 characters' do
+      event = create(:push_event)
+      payload = build(:push_event_payload, event: event)
+
+      expect(payload).to be_valid
+
+      payload.commit_title = 'a' * 100
+
+      expect(payload).not_to be_valid
+    end
+  end
+end

spec/models/push_event_spec.rb

+require 'spec_helper'
+
+describe PushEvent do
+  let(:payload) { PushEventPayload.new }
+
+  let(:event) do
+    event = described_class.new
+
+    allow(event).to receive(:push_event_payload).and_return(payload)
+
+    event
+  end
+
+  describe '.sti_name' do
+    it 'returns Event::PUSHED' do
+      expect(described_class.sti_name).to eq(Event::PUSHED)
+    end
+  end
+
+  describe '#push?' do
+    it 'returns true' do
+      expect(event).to be_push
+    end
+  end
+
+  describe '#push_with_commits?' do
+    it 'returns true when both the first and last commit are present' do
+      allow(event).to receive(:commit_from).and_return('123')
+      allow(event).to receive(:commit_to).and_return('456')
+
+      expect(event).to be_push_with_commits
+    end
+
+    it 'returns false when the first commit is missing' do
+      allow(event).to receive(:commit_to).and_return('456')
+
+      expect(event).not_to be_push_with_commits
+    end
+
+    it 'returns false when the last commit is missing' do
+      allow(event).to receive(:commit_from).and_return('123')
+
+      expect(event).not_to be_push_with_commits
+    end
+  end
+
+  describe '#tag?' do
+    it 'returns true when pushing to a tag' do
+      allow(payload).to receive(:tag?).and_return(true)
+
+      expect(event).to be_tag
+    end
+
+    it 'returns false when pushing to a branch' do
+      allow(payload).to receive(:tag?).and_return(false)
+
+      expect(event).not_to be_tag
+    end
+  end
+
+  describe '#branch?' do
+    it 'returns true when pushing to a branch' do
+      allow(payload).to receive(:branch?).and_return(true)
+
+      expect(event).to be_branch
+    end
+
+    it 'returns false when pushing to a tag' do
+      allow(payload).to receive(:branch?).and_return(false)
+
+      expect(event).not_to be_branch
+    end
+  end
+
+  describe '#valid_push?' do
+    it 'returns true if a ref exists' do
+      allow(payload).to receive(:ref).and_return('master')
+
+      expect(event).to be_valid_push
+    end
+
+    it 'returns false when no ref is present' do
+      expect(event).not_to be_valid_push
+    end
+  end
+
+  describe '#new_ref?' do
+    it 'returns true when pushing a new ref' do
+      allow(payload).to receive(:created?).and_return(true)
+
+      expect(event).to be_new_ref
+    end
+
+    it 'returns false when pushing to an existing ref' do
+      allow(payload).to receive(:created?).and_return(false)
+
+      expect(event).not_to be_new_ref
+    end
+  end
+
+  describe '#rm_ref?' do
+    it 'returns true when removing an existing ref' do
+      allow(payload).to receive(:removed?).and_return(true)
+
+      expect(event).to be_rm_ref
+    end
+
+    it 'returns false when pushing to an existing ref' do
+      allow(payload).to receive(:removed?).and_return(false)
+
+      expect(event).not_to be_rm_ref
+    end
+  end
+
+  describe '#commit_from' do
+    it 'returns the first commit SHA' do
+      allow(payload).to receive(:commit_from).and_return('123')
+
+      expect(event.commit_from).to eq('123')
+    end
+  end
+
+  describe '#commit_to' do
+    it 'returns the last commit SHA' do
+      allow(payload).to receive(:commit_to).and_return('123')
+
+      expect(event.commit_to).to eq('123')
+    end
+  end
+
+  describe '#ref_name' do
+    it 'returns the name of the ref' do
+      allow(payload).to receive(:ref).and_return('master')
+
+      expect(event.ref_name).to eq('master')
+    end
+  end
+
+  describe '#ref_type' do
+    it 'returns the type of the ref' do
+      allow(payload).to receive(:ref_type).and_return('branch')
+
+      expect(event.ref_type).to eq('branch')
+    end
+  end
+
+  describe '#branch_name' do
+    it 'returns the name of the branch' do
+      allow(payload).to receive(:ref).and_return('master')
+
+      expect(event.branch_name).to eq('master')
+    end
+  end
+
+  describe '#tag_name' do
+    it 'returns the name of the tag' do
+      allow(payload).to receive(:ref).and_return('1.2')
+
+      expect(event.tag_name).to eq('1.2')
+    end
+  end
+
+  describe '#commit_title' do
+    it 'returns the commit message' do
+      allow(payload).to receive(:commit_title).and_return('foo')
+
+      expect(event.commit_title).to eq('foo')
+    end
+  end
+
+  describe '#commit_id' do
+    it 'returns the SHA of the last commit if present' do
+      allow(event).to receive(:commit_to).and_return('123')
+
+      expect(event.commit_id).to eq('123')
+    end
+
+    it 'returns the SHA of the first commit if the last commit is not present' do
+      allow(event).to receive(:commit_to).and_return(nil)
+      allow(event).to receive(:commit_from).and_return('123')
+
+      expect(event.commit_id).to eq('123')
+    end
+  end
+
+  describe '#commits_count' do
+    it 'returns the number of commits' do
+      allow(payload).to receive(:commit_count).and_return(1)
+
+      expect(event.commits_count).to eq(1)
+    end
+  end
+
+  describe '#validate_push_action' do
+    it 'adds an error when the action is not PUSHED' do
+      event.action = Event::CREATED
+      event.validate_push_action
+
+      expect(event.errors.count).to eq(1)
+    end
+  end
+end

spec/models/user_spec.rb

@@ -1291,7 +1291,7 @@ describe User do
     let!(:project2) { create(:project, forked_from_project: project3) }
     let!(:project3) { create(:project) }
     let!(:merge_request) { create(:merge_request, source_project: project2, target_project: project3, author: subject) }
-    let!(:push_event) { create(:event, :pushed, project: project1, target: project1, author: subject) }
+    let!(:push_event) { create(:push_event, project: project1, author: subject) }
     let!(:merge_event) { create(:event, :created, project: project3, target: merge_request, author: subject) }
 
     before do
@@ -1333,10 +1333,18 @@ describe User do
     subject { create(:user) }
     let!(:project1) { create(:project, :repository) }
     let!(:project2) { create(:project, :repository, forked_from_project: project1) }
-    let!(:push_data) do
-      Gitlab::DataBuilder::Push.build_sample(project2, subject)
+
+    let!(:push_event) do
+      event = create(:push_event, project: project2, author: subject)
+
+      create(:push_event_payload,
+             event: event,
+             commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+             commit_count: 0,
+             ref: 'master')
+
+      event
     end
-    let!(:push_event) { create(:event, :pushed, project: project2, target: project1, author: subject, data: push_data) }
 
     before do
       project1.team << [subject, :master]
@@ -1363,8 +1371,13 @@ describe User do
       expect(subject.recent_push(project1)).to eq(nil)
       expect(subject.recent_push(project2)).to eq(push_event)
 
-      push_data1 = Gitlab::DataBuilder::Push.build_sample(project1, subject)
-      push_event1 = create(:event, :pushed, project: project1, target: project1, author: subject, data: push_data1)
+      push_event1 = create(:push_event, project: project1, author: subject)
+
+      create(:push_event_payload,
+             event: push_event1,
+             commit_to: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+             commit_count: 0,
+             ref: 'master')
 
       expect(subject.recent_push([project1, project2])).to eq(push_event1) # Newest
     end

spec/requests/api/events_spec.rb

@@ -59,6 +59,34 @@ describe API::Events do
         expect(json_response.size).to eq(1)
       end
 
+      context 'when the list of events includes push events' do
+        let(:event) do
+          create(:push_event, author: user, project: private_project)
+        end
+
+        let!(:payload) { create(:push_event_payload, event: event) }
+        let(:payload_hash) { json_response[0]['push_data'] }
+
+        before do
+          get api("/users/#{user.id}/events?action=pushed", user)
+        end
+
+        it 'responds with HTTP 200 OK' do
+          expect(response).to have_http_status(200)
+        end
+
+        it 'includes the push payload as a Hash' do
+          expect(payload_hash).to be_an_instance_of(Hash)
+        end
+
+        it 'includes the push payload details' do
+          expect(payload_hash['commit_count']).to eq(payload.commit_count)
+          expect(payload_hash['action']).to eq(payload.action)
+          expect(payload_hash['ref_type']).to eq(payload.ref_type)
+          expect(payload_hash['commit_to']).to eq(payload.commit_to)
+        end
+      end
+
       context 'when there are multiple events from different projects' do
         let(:second_note) { create(:note_on_issue, project: create(:project)) }
 

spec/requests/api/v3/users_spec.rb

@@ -252,6 +252,31 @@ describe API::V3::Users do
     end
 
     context "as a user than can see the event's project" do
+      context 'when the list of events includes push events' do
+        let(:event) { create(:push_event, author: user, project: project) }
+        let!(:payload) { create(:push_event_payload, event: event) }
+        let(:payload_hash) { json_response[0]['push_data'] }
+
+        before do
+          get api("/users/#{user.id}/events?action=pushed", user)
+        end
+
+        it 'responds with HTTP 200 OK' do
+          expect(response).to have_http_status(200)
+        end
+
+        it 'includes the push payload as a Hash' do
+          expect(payload_hash).to be_an_instance_of(Hash)
+        end
+
+        it 'includes the push payload details' do
+          expect(payload_hash['commit_count']).to eq(payload.commit_count)
+          expect(payload_hash['action']).to eq(payload.action)
+          expect(payload_hash['ref_type']).to eq(payload.ref_type)
+          expect(payload_hash['commit_to']).to eq(payload.commit_to)
+        end
+      end
+
       context 'joined event' do
         it 'returns the "joined" event' do
           get v3_api("/users/#{user.id}/events", user)

spec/services/event_create_service_spec.rb

@@ -117,12 +117,52 @@ describe EventCreateService do
     let(:project) { create(:project) }
     let(:user) { create(:user) }
 
+    let(:push_data) do
+      {
+        commits: [
+          {
+            id: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+            message: 'This is a commit'
+          }
+        ],
+        before: '0000000000000000000000000000000000000000',
+        after: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+        total_commits_count: 1,
+        ref: 'refs/heads/my-branch'
+      }
+    end
+
     it 'creates a new event' do
-      expect { service.push(project, user, {}) }.to change { Event.count }
+      expect { service.push(project, user, push_data) }.to change { Event.count }
+    end
+
+    it 'creates the push event payload' do
+      expect(PushEventPayloadService).to receive(:new)
+        .with(an_instance_of(PushEvent), push_data)
+        .and_call_original
+
+      service.push(project, user, push_data)
     end
 
     it 'updates user last activity' do
-      expect { service.push(project, user, {}) }.to change { user_activity(user) }
+      expect { service.push(project, user, push_data) }
+        .to change { user_activity(user) }
+    end
+
+    it 'does not create any event data when an error is raised' do
+      payload_service = double(:service)
+
+      allow(payload_service).to receive(:execute)
+        .and_raise(RuntimeError)
+
+      allow(PushEventPayloadService).to receive(:new)
+        .and_return(payload_service)
+
+      expect { service.push(project, user, push_data) }
+        .to raise_error(RuntimeError)
+
+      expect(Event.count).to eq(0)
+      expect(PushEventPayload.count).to eq(0)
     end
   end
 

spec/services/git_push_service_spec.rb

@@ -141,10 +141,13 @@ describe GitPushService, services: true do
     let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) }
     let(:event) { Event.find_by_action(Event::PUSHED) }
 
-    it { expect(event).not_to be_nil }
+    it { expect(event).to be_an_instance_of(PushEvent) }
     it { expect(event.project).to eq(project) }
     it { expect(event.action).to eq(Event::PUSHED) }
-    it { expect(event.data).to eq(push_data) }
+    it { expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) }
+    it { expect(event.push_event_payload.commit_from).to eq(oldrev) }
+    it { expect(event.push_event_payload.commit_to).to eq(newrev) }
+    it { expect(event.push_event_payload.ref).to eq('master') }
 
     context "Updates merge requests" do
       it "when pushing a new branch for the first time" do

spec/services/push_event_payload_service_spec.rb

+require 'spec_helper'
+
+describe PushEventPayloadService do
+  let(:event) { create(:push_event) }
+
+  describe '#execute' do
+    let(:push_data) do
+      {
+        commits: [
+          {
+            id: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+            message: 'This is a commit'
+          }
+        ],
+        before: '0000000000000000000000000000000000000000',
+        after: '1cf19a015df3523caf0a1f9d40c98a267d6a2fc2',
+        total_commits_count: 1,
+        ref: 'refs/heads/my-branch'
+      }
+    end
+
+    it 'creates a new PushEventPayload row' do
+      payload = described_class.new(event, push_data).execute
+
+      expect(payload.commit_count).to eq(1)
+      expect(payload.action).to eq('created')
+      expect(payload.ref_type).to eq('branch')
+      expect(payload.commit_from).to be_nil
+      expect(payload.commit_to).to eq(push_data[:after])
+      expect(payload.ref).to eq('my-branch')
+      expect(payload.commit_title).to eq('This is a commit')
+      expect(payload.event_id).to eq(event.id)
+    end
+
+    it 'sets the push_event_payload association of the used event' do
+      payload = described_class.new(event, push_data).execute
+
+      expect(event.push_event_payload).to eq(payload)
+    end
+  end
+
+  describe '#commit_title' do
+    it 'returns nil if no commits were pushed' do
+      service = described_class.new(event, commits: [])
+
+      expect(service.commit_title).to be_nil
+    end
+
+    it 'returns a String limited to 70 characters' do
+      service = described_class.new(event, commits: [{ message: 'a' * 100 }])
+
+      expect(service.commit_title).to eq(('a' * 67) + '...')
+    end
+
+    it 'does not truncate the commit message if it is shorter than 70 characters' do
+      service = described_class.new(event, commits: [{ message: 'Hello' }])
+
+      expect(service.commit_title).to eq('Hello')
+    end
+
+    it 'includes the first line of a commit message if the message spans multiple lines' do
+      service = described_class
+        .new(event, commits: [{ message: "Hello\n\nworld" }])
+
+      expect(service.commit_title).to eq('Hello')
+    end
+  end
+
+  describe '#commit_from_id' do
+    it 'returns nil when creating a new ref' do
+      service = described_class.new(event, before: Gitlab::Git::BLANK_SHA)
+
+      expect(service.commit_from_id).to be_nil
+    end
+
+    it 'returns the ID of the first commit when pushing to an existing ref' do
+      service = described_class.new(event, before: '123')
+
+      expect(service.commit_from_id).to eq('123')
+    end
+  end
+
+  describe '#commit_to_id' do
+    it 'returns nil when removing an existing ref' do
+      service = described_class.new(event, after: Gitlab::Git::BLANK_SHA)
+
+      expect(service.commit_to_id).to be_nil
+    end
+  end
+
+  describe '#commit_count' do
+    it 'returns the number of commits' do
+      service = described_class.new(event, total_commits_count: 1)
+
+      expect(service.commit_count).to eq(1)
+    end
+
+    it 'raises when the push data does not contain the commits count' do
+      service = described_class.new(event, {})
+
+      expect { service.commit_count }.to raise_error(KeyError)
+    end
+  end
+
+  describe '#ref' do
+    it 'returns the name of the ref' do
+      service = described_class.new(event, ref: 'refs/heads/foo')
+
+      expect(service.ref).to eq('refs/heads/foo')
+    end
+
+    it 'raises when the push data does not contain the ref name' do
+      service = described_class.new(event, {})
+
+      expect { service.ref }.to raise_error(KeyError)
+    end
+  end
+
+  describe '#revision_before' do
+    it 'returns the revision from before the push' do
+      service = described_class.new(event, before: 'foo')
+
+      expect(service.revision_before).to eq('foo')
+    end
+
+    it 'raises when the push data does not contain the before revision' do
+      service = described_class.new(event, {})
+
+      expect { service.revision_before }.to raise_error(KeyError)
+    end
+  end
+
+  describe '#revision_after' do
+    it 'returns the revision from after the push' do
+      service = described_class.new(event, after: 'foo')
+
+      expect(service.revision_after).to eq('foo')
+    end
+
+    it 'raises when the push data does not contain the after revision' do
+      service = described_class.new(event, {})
+
+      expect { service.revision_after }.to raise_error(KeyError)
+    end
+  end
+
+  describe '#trimmed_ref' do
+    it 'returns the ref name without its prefix' do
+      service = described_class.new(event, ref: 'refs/heads/foo')
+
+      expect(service.trimmed_ref).to eq('foo')
+    end
+  end
+
+  describe '#create?' do
+    it 'returns true when creating a new ref' do
+      service = described_class.new(event, before: Gitlab::Git::BLANK_SHA)
+
+      expect(service.create?).to eq(true)
+    end
+
+    it 'returns false when pushing to an existing ref' do
+      service = described_class.new(event, before: 'foo')
+
+      expect(service.create?).to eq(false)
+    end
+  end
+
+  describe '#remove?' do
+    it 'returns true when removing an existing ref' do
+      service = described_class.new(event, after: Gitlab::Git::BLANK_SHA)
+
+      expect(service.remove?).to eq(true)
+    end
+
+    it 'returns false pushing to an existing ref' do
+      service = described_class.new(event, after: 'foo')
+
+      expect(service.remove?).to eq(false)
+    end
+  end
+
+  describe '#action' do
+    it 'returns :created when creating a ref' do
+      service = described_class.new(event, before: Gitlab::Git::BLANK_SHA)
+
+      expect(service.action).to eq(:created)
+    end
+
+    it 'returns :removed when removing an existing ref' do
+      service = described_class.new(event,
+                                    before: '123',
+                                    after: Gitlab::Git::BLANK_SHA)
+
+      expect(service.action).to eq(:removed)
+    end
+
+    it 'returns :pushed when pushing to an existing ref' do
+      service = described_class.new(event, before: '123', after: '456')
+
+      expect(service.action).to eq(:pushed)
+    end
+  end
+
+  describe '#ref_type' do
+    it 'returns :tag for a tag' do
+      service = described_class.new(event, ref: 'refs/tags/1.2')
+
+      expect(service.ref_type).to eq(:tag)
+    end
+
+    it 'returns :branch for a branch' do
+      service = described_class.new(event, ref: 'refs/heads/master')
+
+      expect(service.ref_type).to eq(:branch)
+    end
+  end
+end

spec/workers/prune_old_events_worker_spec.rb

@@ -2,9 +2,11 @@ require 'spec_helper'
 
 describe PruneOldEventsWorker do
   describe '#perform' do
-    let!(:expired_event) { create(:event, author_id: 0, created_at: 13.months.ago) }
-    let!(:not_expired_event) { create(:event, author_id: 0,  created_at: 1.day.ago) }
-    let!(:exactly_12_months_event) { create(:event, author_id: 0, created_at: 12.months.ago) }
+    let(:user) { create(:user) }
+
+    let!(:expired_event) { create(:event, :closed, author: user, created_at: 13.months.ago) }
+    let!(:not_expired_event) { create(:event, :closed, author: user,  created_at: 1.day.ago) }
+    let!(:exactly_12_months_event) { create(:event, :closed, author: user, created_at: 12.months.ago) }
 
     it 'prunes events older than 12 months' do
       expect { subject.perform }.to change { Event.count }.by(-1)