Merge branch '30526-c-be-wiki-activity-Pushes' into 'master'

#216014 [BE] Wiki Activity on Project, Group and User Activity Pages"

Closes #30526

See merge request gitlab-org/gitlab!26624

app/models/event.rb

@@ -96,6 +96,8 @@ class Event < ApplicationRecord
   end
 
   scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }
+  scope :for_wiki_meta, ->(meta) { where(target_type: 'WikiPage::Meta', target_id: meta.id) }
+  scope :created_at, ->(time) { where(created_at: time) }
 
   # Authors are required as they're used to display who pushed data.
   #

app/models/wiki_page.rb

@@ -295,6 +295,10 @@ class WikiPage
     'wiki_page'
   end
 
+  def version_commit_timestamp
+    version&.commit&.committed_date
+  end
+
   private
 
   def serialize_front_matter(hash)

app/models/wiki_page/meta.rb

@@ -5,6 +5,7 @@ class WikiPage
     include Gitlab::Utils::StrongMemoize
 
     CanonicalSlugConflictError = Class.new(ActiveRecord::RecordInvalid)
+    WikiPageInvalid = Class.new(ArgumentError)
 
     self.table_name = 'wiki_page_meta'
 
@@ -23,46 +24,62 @@ class WikiPage
 
     alias_method :resource_parent, :project
 
-    # Return the (updated) WikiPage::Meta record for a given wiki page
-    #
-    # If none is found, then a new record is created, and its fields are set
-    # to reflect the wiki_page passed.
-    #
-    # @param [String] last_known_slug
-    # @param [WikiPage] wiki_page
-    #
-    # As with all `find_or_create` methods, this one raises errors on
-    # validation issues.
-    def self.find_or_create(last_known_slug, wiki_page)
-      project = wiki_page.wiki.project
-      known_slugs = [last_known_slug, wiki_page.slug].compact.uniq
-      raise 'no slugs!' if known_slugs.empty?
-
-      transaction do
-        found = find_by_canonical_slug(known_slugs, project)
-        meta = found || create(title: wiki_page.title, project_id: project.id)
-
-        meta.update_state(found.nil?, known_slugs, wiki_page)
-
-        # We don't need to run validations here, since find_by_canonical_slug
-        # guarantees that there is no conflict in canonical_slug, and DB
-        # constraints on title and project_id enforce our other invariants
-        # This saves us a query.
-        meta
+    class << self
+      # Return the (updated) WikiPage::Meta record for a given wiki page
+      #
+      # If none is found, then a new record is created, and its fields are set
+      # to reflect the wiki_page passed.
+      #
+      # @param [String] last_known_slug
+      # @param [WikiPage] wiki_page
+      #
+      # This method raises errors on validation issues.
+      def find_or_create(last_known_slug, wiki_page)
+        raise WikiPageInvalid unless wiki_page.valid?
+
+        project = wiki_page.wiki.project
+        known_slugs = [last_known_slug, wiki_page.slug].compact.uniq
+        raise 'No slugs found! This should not be possible.' if known_slugs.empty?
+
+        transaction do
+          updates = wiki_page_updates(wiki_page)
+          found = find_by_canonical_slug(known_slugs, project)
+          meta = found || create!(updates.merge(project_id: project.id))
+
+          meta.update_state(found.nil?, known_slugs, wiki_page, updates)
+
+          # We don't need to run validations here, since find_by_canonical_slug
+          # guarantees that there is no conflict in canonical_slug, and DB
+          # constraints on title and project_id enforce our other invariants
+          # This saves us a query.
+          meta
+        end
       end
-    end
 
-    def self.find_by_canonical_slug(canonical_slug, project)
-      meta, conflict = with_canonical_slug(canonical_slug)
-        .where(project_id: project.id)
-        .limit(2)
+      def find_by_canonical_slug(canonical_slug, project)
+        meta, conflict = with_canonical_slug(canonical_slug)
+          .where(project_id: project.id)
+          .limit(2)
 
-      if conflict.present?
-        meta.errors.add(:canonical_slug, 'Duplicate value found')
-        raise CanonicalSlugConflictError.new(meta)
+        if conflict.present?
+          meta.errors.add(:canonical_slug, 'Duplicate value found')
+          raise CanonicalSlugConflictError.new(meta)
+        end
+
+        meta
       end
 
-      meta
+      private
+
+      def wiki_page_updates(wiki_page)
+        last_commit_date = wiki_page.version_commit_timestamp || Time.now.utc
+
+        {
+          title: wiki_page.title,
+          created_at: last_commit_date,
+          updated_at: last_commit_date
+        }
+      end
     end
 
     def canonical_slug
@@ -85,24 +102,21 @@ class WikiPage
       @canonical_slug = slug
     end
 
-    def update_state(created, known_slugs, wiki_page)
-      update_wiki_page_attributes(wiki_page)
+    def update_state(created, known_slugs, wiki_page, updates)
+      update_wiki_page_attributes(updates)
       insert_slugs(known_slugs, created, wiki_page.slug)
       self.canonical_slug = wiki_page.slug
     end
 
-    def update_columns(attrs = {})
-      super(attrs.reverse_merge(updated_at: Time.now.utc))
-    end
-
-    def self.update_all(attrs = {})
-      super(attrs.reverse_merge(updated_at: Time.now.utc))
-    end
-
     private
 
-    def update_wiki_page_attributes(page)
-      update_columns(title: page.title) unless page.title == title
+    def update_wiki_page_attributes(updates)
+      # Remove all unnecessary updates:
+      updates.delete(:updated_at) if updated_at == updates[:updated_at]
+      updates.delete(:created_at) if created_at <= updates[:created_at]
+      updates.delete(:title) if title == updates[:title]
+
+      update_columns(updates) unless updates.empty?
     end
 
     def insert_slugs(strings, is_new, canonical_slug)

app/services/event_create_service.rb

@@ -85,18 +85,40 @@ class EventCreateService
   # Create a new wiki page event
   #
   # @param [WikiPage::Meta] wiki_page_meta The event target
-  # @param [User] current_user The event author
+  # @param [User] author The event author
   # @param [Integer] action One of the Event::WIKI_ACTIONS
-  def wiki_event(wiki_page_meta, current_user, action)
+  #
+  # @return a tuple of event and either :found or :created
+  def wiki_event(wiki_page_meta, author, action)
     return unless Feature.enabled?(:wiki_events)
 
     raise IllegalActionError, action unless Event::WIKI_ACTIONS.include?(action)
 
-    create_record_event(wiki_page_meta, current_user, action)
+    if duplicate = existing_wiki_event(wiki_page_meta, action)
+      return duplicate
+    end
+
+    event = create_record_event(wiki_page_meta, author, action)
+    # Ensure that the event is linked in time to the metadata, for non-deletes
+    unless action == Event::DESTROYED
+      time_stamp = wiki_page_meta.updated_at
+      event.update_columns(updated_at: time_stamp, created_at: time_stamp)
+    end
+
+    event
   end
 
   private
 
+  def existing_wiki_event(wiki_page_meta, action)
+    if action == Event::DESTROYED
+      most_recent = Event.for_wiki_meta(wiki_page_meta).recent.first
+      return most_recent if most_recent.present? && most_recent.action == action
+    else
+      Event.for_wiki_meta(wiki_page_meta).created_at(wiki_page_meta.updated_at).first
+    end
+  end
+
   def create_record_event(record, current_user, status)
     create_event(record.resource_parent, current_user, status, target_id: record.id, target_type: record.class.name)
   end

app/services/git/wiki_push_service.rb

@@ -2,8 +2,63 @@
 
 module Git
   class WikiPushService < ::BaseService
+    # Maximum number of change events we will process on any single push
+    MAX_CHANGES = 100
+
     def execute
-      # This is used in EE
+      process_changes
+    end
+
+    private
+
+    def process_changes
+      return unless can_process_wiki_events?
+
+      push_changes.take(MAX_CHANGES).each do |change| # rubocop:disable CodeReuse/ActiveRecord
+        next unless change.page.present?
+
+        response = create_event_for(change)
+        log_error(response.message) if response.error?
+      end
+    end
+
+    def can_process_wiki_events?
+      Feature.enabled?(:wiki_events) && Feature.enabled?(:wiki_events_on_git_push, project)
+    end
+
+    def push_changes
+      default_branch_changes.flat_map do |change|
+        raw_changes(change).map { |raw| Git::WikiPushService::Change.new(wiki, change, raw) }
+      end
+    end
+
+    def raw_changes(change)
+      wiki.repository.raw.raw_changes_between(change[:oldrev], change[:newrev])
+    end
+
+    def wiki
+      project.wiki
+    end
+
+    def create_event_for(change)
+      event_service.execute(change.last_known_slug, change.page, change.event_action)
+    end
+
+    def event_service
+      @event_service ||= WikiPages::EventCreateService.new(current_user)
+    end
+
+    def on_default_branch?(change)
+      project.wiki.default_branch == ::Gitlab::Git.branch_name(change[:ref])
+    end
+
+    # See: [Gitlab::GitPostReceive#changes]
+    def changes
+      params[:changes] || []
+    end
+
+    def default_branch_changes
+      @default_branch_changes ||= changes.select { |change| on_default_branch?(change) }
     end
   end
 end

app/services/git/wiki_push_service/change.rb

+# frozen_string_literal: true
+
+module Git
+  class WikiPushService
+    class Change
+      include Gitlab::Utils::StrongMemoize
+
+      # @param [ProjectWiki] wiki
+      # @param [Hash] change - must have keys `:oldrev` and `:newrev`
+      # @param [Gitlab::Git::RawDiffChange] raw_change
+      def initialize(project_wiki, change, raw_change)
+        @wiki, @raw_change, @change = project_wiki, raw_change, change
+      end
+
+      def page
+        strong_memoize(:page) { wiki.find_page(slug, revision) }
+      end
+
+      # See [Gitlab::Git::RawDiffChange#extract_operation] for the
+      # definition of the full range of operation values.
+      def event_action
+        case raw_change.operation
+        when :added
+          Event::CREATED
+        when :deleted
+          Event::DESTROYED
+        else
+          Event::UPDATED
+        end
+      end
+
+      def last_known_slug
+        strip_extension(raw_change.old_path || raw_change.new_path)
+      end
+
+      private
+
+      attr_reader :raw_change, :change, :wiki
+
+      def filename
+        return raw_change.old_path if deleted?
+
+        raw_change.new_path
+      end
+
+      def slug
+        strip_extension(filename)
+      end
+
+      def revision
+        return change[:oldrev] if deleted?
+
+        change[:newrev]
+      end
+
+      def deleted?
+        raw_change.operation == :deleted
+      end
+
+      def strip_extension(filename)
+        return unless filename
+
+        File.basename(filename, File.extname(filename))
+      end
+    end
+  end
+end

app/services/wiki_pages/base_service.rb

@@ -46,12 +46,9 @@ module WikiPages
     def create_wiki_event(page)
       return unless ::Feature.enabled?(:wiki_events)
 
-      slug = slug_for_page(page)
+      response = WikiPages::EventCreateService.new(current_user).execute(slug_for_page(page), page, event_action)
 
-      Event.transaction do
-        wiki_page_meta = WikiPage::Meta.find_or_create(slug, page)
-        EventCreateService.new.wiki_event(wiki_page_meta, current_user, event_action)
-      end
+      log_error(response.message) if response.error?
     end
 
     def slug_for_page(page)

app/services/wiki_pages/event_create_service.rb

+# frozen_string_literal: true
+
+module WikiPages
+  class EventCreateService
+    # @param [User] author The event author
+    def initialize(author)
+      raise ArgumentError, 'author must not be nil' unless author
+
+      @author = author
+    end
+
+    def execute(slug, page, action)
+      return ServiceResponse.success(message: 'No event created as `wiki_events` feature is disabled') unless ::Feature.enabled?(:wiki_events)
+
+      event = Event.transaction do
+        wiki_page_meta = WikiPage::Meta.find_or_create(slug, page)
+
+        ::EventCreateService.new.wiki_event(wiki_page_meta, author, action)
+      end
+
+      ServiceResponse.success(payload: { event: event })
+    rescue ::EventCreateService::IllegalActionError, ::ActiveRecord::ActiveRecordError => e
+      ServiceResponse.error(message: e.message, payload: { error: e })
+    end
+
+    private
+
+    attr_reader :author
+  end
+end

changelogs/unreleased/30526-c-be-wiki-activity-Pushes.yml

+---
+title: Create Wiki activity events on pushes to Wiki git repository
+merge_request: 26624
+author:
+type: added

ee/app/services/ee/git/wiki_push_service.rb

@@ -3,16 +3,16 @@
 module EE
   module Git
     module WikiPushService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
       def execute
         super
 
         return unless project.use_elasticsearch?
 
-        # Check if one of the changes we got was for the default branch. If it was, trigger an ES update
-        params[:changes].each do |change|
-          branch_name = ::Gitlab::Git.ref_name(change[:ref])
-          next unless project.wiki.default_branch == branch_name
-
+        # For all changes on the default branch (usually master) trigger an ES update
+        default_branch_changes.each do |change|
           project.wiki.index_wiki_blobs(change[:newrev])
         end
       end

ee/spec/services/ee/git/wiki_push_service_spec.rb

@@ -23,6 +23,10 @@ describe Git::WikiPushService do
     describe 'when changes include master ref' do
       let(:changes) { +"123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag\n423423 797823 refs/heads/master" }
 
+      before do
+        allow(project.wiki.repository.raw).to receive(:raw_changes_between).once.with('423423', '797823').and_return([])
+      end
+
       it 'triggers a wiki update' do
         expect(project.wiki).to receive(:index_wiki_blobs).with("797823")
 
@@ -44,6 +48,7 @@ describe Git::WikiPushService do
   context 'when elasticsearch is disabled' do
     before do
       stub_ee_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
+      allow(project.wiki.repository.raw).to receive(:raw_changes_between).once.with('423423', '797823').and_return([])
     end
 
     describe 'when changes include master ref' do

ee/spec/workers/post_receive_spec.rb

@@ -99,82 +99,94 @@ describe PostReceive do
   describe '#process_wiki_changes' do
     let(:gl_repository) { "wiki-#{project.id}" }
 
-    it 'calls Geo::RepositoryUpdatedService when running on a Geo primary node' do
-      allow(Gitlab::Geo).to receive(:primary?) { true }
-
-      expect_any_instance_of(::Geo::RepositoryUpdatedService).to receive(:execute)
+    it 'calls Git::WikiPushService#process_changes' do
+      expect_any_instance_of(::Git::WikiPushService).to receive(:process_changes)
 
       described_class.new.perform(gl_repository, key_id, base64_changes)
     end
 
-    it 'does not call Geo::RepositoryUpdatedService when not running on a Geo primary node' do
-      allow(Gitlab::Geo).to receive(:primary?) { false }
+    context 'assuming calls to process_changes are successful' do
+      before do
+        allow_any_instance_of(Git::WikiPushService).to receive(:process_changes)
+      end
 
-      expect_any_instance_of(::Geo::RepositoryUpdatedService).not_to receive(:execute)
+      it 'calls Geo::RepositoryUpdatedService when running on a Geo primary node' do
+        allow(Gitlab::Geo).to receive(:primary?) { true }
 
-      described_class.new.perform(gl_repository, key_id, base64_changes)
-    end
+        expect_any_instance_of(::Geo::RepositoryUpdatedService).to receive(:execute)
 
-    it 'triggers wiki index update when ElasticSearch is enabled and pushed to master', :elastic do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+        described_class.new.perform(gl_repository, key_id, base64_changes)
+      end
 
-      expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
+      it 'does not call Geo::RepositoryUpdatedService when not running on a Geo primary node' do
+        allow(Gitlab::Geo).to receive(:primary?) { false }
 
-      described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
-    end
+        expect_any_instance_of(::Geo::RepositoryUpdatedService).not_to receive(:execute)
 
-    it 'does not trigger wiki index update when Elasticsearch is enabled and not pushed to master', :elastic do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+        described_class.new.perform(gl_repository, key_id, base64_changes)
+      end
 
-      expect_any_instance_of(ProjectWiki).not_to receive(:index_wiki_blobs)
+      it 'triggers wiki index update when ElasticSearch is enabled and pushed to master', :elastic do
+        stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
 
-      described_class.new.perform(gl_repository, key_id, base64_changes)
-    end
+        expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
 
-    context 'when limited indexing is on', :elastic do
-      before do
-        stub_ee_application_setting(
-          elasticsearch_search: true,
-          elasticsearch_indexing: true,
-          elasticsearch_limit_indexing: true
-        )
+        described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
       end
 
-      context 'when the project is not enabled specifically' do
-        it 'does not trigger wiki index update' do
-          expect_any_instance_of(ProjectWiki).not_to receive(:index_wiki_blobs)
+      it 'does not trigger wiki index update when Elasticsearch is enabled and not pushed to master', :elastic do
+        stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
 
-          described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
-        end
+        expect_any_instance_of(ProjectWiki).not_to receive(:index_wiki_blobs)
+
+        described_class.new.perform(gl_repository, key_id, base64_changes)
       end
 
-      context 'when a project is enabled specifically' do
+      context 'when limited indexing is on', :elastic do
         before do
-          create :elasticsearch_indexed_project, project: project
+          stub_ee_application_setting(
+            elasticsearch_search: true,
+            elasticsearch_indexing: true,
+            elasticsearch_limit_indexing: true
+          )
         end
 
-        it 'triggers wiki index update' do
-          expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
+        context 'when the project is not enabled specifically' do
+          it 'does not trigger wiki index update' do
+            expect_any_instance_of(ProjectWiki).not_to receive(:index_wiki_blobs)
 
-          described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
+            described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
+          end
         end
-      end
 
-      context 'when a group is enabled' do
-        let(:user) { create(:user) }
-        let(:group) { create(:group) }
-        let(:project) { create(:project, :wiki_repo, group: group) }
-        let(:key) { create(:key, user: user) }
+        context 'when a project is enabled specifically' do
+          before do
+            create :elasticsearch_indexed_project, project: project
+          end
 
-        before do
-          create :elasticsearch_indexed_namespace, namespace: group
-          group.add_owner(user)
+          it 'triggers wiki index update' do
+            expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
+
+            described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
+          end
         end
 
-        it 'triggers wiki index update' do
-          expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
+        context 'when a group is enabled' do
+          let(:user) { create(:user) }
+          let(:group) { create(:group) }
+          let(:project) { create(:project, :wiki_repo, group: group) }
+          let(:key) { create(:key, user: user) }
+
+          before do
+            create :elasticsearch_indexed_namespace, namespace: group
+            group.add_owner(user)
+          end
 
-          described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
+          it 'triggers wiki index update' do
+            expect_any_instance_of(ProjectWiki).to receive(:index_wiki_blobs)
+
+            described_class.new.perform(gl_repository, key_id, base64_changes_with_master)
+          end
         end
       end
     end

spec/factories/design_management/versions.rb

@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :design_version, class: 'DesignManagement::Version' do
-    sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") }
+    sha
     issue { designs.first&.issue || create(:issue) }
     author { issue&.author || create(:user) }
 

spec/factories/git_wiki_commit_details.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :git_wiki_commit_details, class: 'Gitlab::Git::Wiki::CommitDetails' do
+    skip_create
+
+    transient do
+      author { create(:user) }
+    end
+
+    sequence(:message) { |n| "Commit message #{n}" }
+
+    initialize_with { new(author.id, author.username, author.name, author.email, message) }
+  end
+end

spec/factories/sequences.rb

@@ -12,4 +12,5 @@ FactoryBot.define do
   sequence(:branch) { |n| "my-branch-#{n}" }
   sequence(:past_time) { |n| 4.hours.ago + (2 * n).seconds }
   sequence(:iid)
+  sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") }
 end

spec/factories/wiki_pages.rb

@@ -66,5 +66,6 @@ FactoryBot.define do
   end
 
   sequence(:wiki_page_title) { |n| "Page #{n}" }
+  sequence(:wiki_filename) { |n| "Page_#{n}.md" }
   sequence(:sluggified_title) { |n| "slug-#{n}" }
 end

spec/models/event_spec.rb

@@ -84,6 +84,21 @@ describe Event do
     end
   end
 
+  describe 'scopes' do
+    describe 'created_at' do
+      it 'can find the right event' do
+        time = 1.day.ago
+        event = create(:event, created_at: time)
+        false_positive = create(:event, created_at: 2.days.ago)
+
+        found = described_class.created_at(time)
+
+        expect(found).to include(event)
+        expect(found).not_to include(false_positive)
+      end
+    end
+  end
+
   describe "Push event" do
     let(:project) { create(:project, :private) }
     let(:user) { project.owner }
@@ -511,6 +526,14 @@ describe Event do
         expect(described_class.not_wiki_page).to match_array(non_wiki_events)
       end
     end
+
+    describe '.for_wiki_meta' do
+      it 'finds events for a given wiki page metadata object' do
+        event = events.select(&:wiki_page?).first
+
+        expect(described_class.for_wiki_meta(event.target)).to contain_exactly(event)
+      end
+    end
   end
 
   describe '#wiki_page and #wiki_page?' do

spec/models/wiki_page/meta_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe WikiPage::Meta do
-  let_it_be(:project) { create(:project) }
+  let_it_be(:project) { create(:project, :wiki_repo) }
   let_it_be(:other_project) { create(:project) }
 
   describe 'Associations' do
@@ -169,8 +169,11 @@ describe WikiPage::Meta do
       described_class.find_or_create(last_known_slug, wiki_page)
     end
 
-    def create_previous_version(title = old_title, slug = last_known_slug)
-      create(:wiki_page_meta, title: title, project: project, canonical_slug: slug)
+    def create_previous_version(title: old_title, slug: last_known_slug, date: wiki_page.version.commit.committed_date)
+      create(:wiki_page_meta,
+             title: title, project: project,
+             created_at: date, updated_at: date,
+             canonical_slug: slug)
     end
 
     def create_context
@@ -198,6 +201,8 @@ describe WikiPage::Meta do
           title: wiki_page.title,
           project: wiki_page.wiki.project
         )
+        expect(meta.updated_at).to eq(wiki_page.version.commit.committed_date)
+        expect(meta.created_at).not_to be_after(meta.updated_at)
         expect(meta.slugs.where(slug: last_known_slug)).to exist
         expect(meta.slugs.canonical.where(slug: wiki_page.slug)).to exist
       end
@@ -209,22 +214,32 @@ describe WikiPage::Meta do
       end
     end
 
-    context 'the slug is too long' do
-      let(:last_known_slug) { FFaker::Lorem.characters(2050) }
+    context 'there are problems' do
+      context 'the slug is too long' do
+        let(:last_known_slug) { FFaker::Lorem.characters(2050) }
 
-      it 'raises an error' do
-        expect { find_record }.to raise_error ActiveRecord::ValueTooLong
+        it 'raises an error' do
+          expect { find_record }.to raise_error ActiveRecord::ValueTooLong
+        end
       end
-    end
 
-    context 'a conflicting record exists' do
-      before do
-        create(:wiki_page_meta, project: project, canonical_slug: last_known_slug)
-        create(:wiki_page_meta, project: project, canonical_slug: current_slug)
+      context 'a conflicting record exists' do
+        before do
+          create(:wiki_page_meta, project: project, canonical_slug: last_known_slug)
+          create(:wiki_page_meta, project: project, canonical_slug: current_slug)
+        end
+
+        it 'raises an error' do
+          expect { find_record }.to raise_error(ActiveRecord::RecordInvalid)
+        end
       end
 
-      it 'raises an error' do
-        expect { find_record }.to raise_error(ActiveRecord::RecordInvalid)
+      context 'the wiki page is not valid' do
+        let(:wiki_page) { build(:wiki_page, project: project, title: nil) }
+
+        it 'raises an error' do
+          expect { find_record }.to raise_error(described_class::WikiPageInvalid)
+        end
       end
     end
 
@@ -258,6 +273,17 @@ describe WikiPage::Meta do
       end
     end
 
+    context 'the commit happened a day ago' do
+      before do
+        allow(wiki_page.version.commit).to receive(:committed_date).and_return(1.day.ago)
+      end
+
+      include_examples 'metadata examples' do
+        # Identical to the base case.
+        let(:query_limit) { 5 }
+      end
+    end
+
     context 'the last_known_slug is the same as the current slug, as on creation' do
       let(:last_known_slug) { current_slug }
 
@@ -292,6 +318,33 @@ describe WikiPage::Meta do
       end
     end
 
+    context 'a record exists in the DB, but we need to update timestamps' do
+      let(:last_known_slug) { current_slug }
+      let(:old_title) { title }
+
+      before do
+        create_previous_version(date: 1.week.ago)
+      end
+
+      include_examples 'metadata examples' do
+        # We need the query, and the update
+        # SAVEPOINT active_record_2
+        #
+        # SELECT * FROM wiki_page_meta
+        #   INNER JOIN wiki_page_slugs
+        #     ON wiki_page_slugs.wiki_page_meta_id = wiki_page_meta.id
+        #   WHERE wiki_page_meta.project_id = ?
+        #     AND wiki_page_slugs.canonical = TRUE
+        #     AND wiki_page_slugs.slug = ?
+        #   LIMIT 2
+        #
+        # UPDATE wiki_page_meta SET updated_at = ?date WHERE id = ?id
+        #
+        # RELEASE SAVEPOINT active_record_2
+        let(:query_limit) { 4 }
+      end
+    end
+
     context 'we need to update the slug, but not the title' do
       let(:old_title) { title }
 
@@ -359,14 +412,14 @@ describe WikiPage::Meta do
     end
 
     context 'we want to change the slug back to a previous version' do
-      let(:slug_1) { 'foo' }
-      let(:slug_2) { 'bar' }
+      let(:slug_1) { generate(:sluggified_title) }
+      let(:slug_2) { generate(:sluggified_title) }
 
       let(:wiki_page) { create(:wiki_page, title: slug_1, project: project) }
       let(:last_known_slug) { slug_2 }
 
       before do
-        meta = create_previous_version(title, slug_1)
+        meta = create_previous_version(title: title, slug: slug_1)
         meta.canonical_slug = slug_2
       end
 

spec/models/wiki_page_spec.rb

@@ -844,6 +844,20 @@ describe WikiPage do
     end
   end
 
+  describe '#version_commit_timestamp' do
+    context 'for a new page' do
+      it 'returns nil' do
+        expect(new_page.version_commit_timestamp).to be_nil
+      end
+    end
+
+    context 'for page that exists' do
+      it 'returns the timestamp of the commit' do
+        expect(existing_page.version_commit_timestamp).to eq(existing_page.version.commit.committed_date)
+      end
+    end
+  end
+
   private
 
   def get_slugs(page_or_dir)

spec/services/event_create_service_spec.rb

@@ -162,16 +162,25 @@ describe EventCreateService do
       context "The action is #{action}" do
         let(:event) { service.wiki_event(meta, user, action) }
 
-        it 'creates the event' do
+        it 'creates the event', :aggregate_failures do
           expect(event).to have_attributes(
             wiki_page?: true,
             valid?: true,
             persisted?: true,
             action: action,
-            wiki_page: wiki_page
+            wiki_page: wiki_page,
+            author: user
           )
         end
 
+        it 'is idempotent', :aggregate_failures do
+          expect { event }.to change(Event, :count).by(1)
+          duplicate = nil
+          expect { duplicate = service.wiki_event(meta, user, action) }.not_to change(Event, :count)
+
+          expect(duplicate).to eq(event)
+        end
+
         context 'the feature is disabled' do
           before do
             stub_feature_flags(wiki_events: false)

spec/services/git/wiki_push_service/change_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Git::WikiPushService::Change do
+  subject { described_class.new(project_wiki, change, raw_change) }
+
+  let(:project_wiki) { double('ProjectWiki') }
+  let(:raw_change) { double('RawChange', new_path: new_path, old_path: old_path, operation: operation) }
+  let(:change) { { oldrev: generate(:sha), newrev: generate(:sha) } }
+
+  let(:new_path) do
+    case operation
+    when :deleted
+      nil
+    else
+      generate(:wiki_filename)
+    end
+  end
+
+  let(:old_path) do
+    case operation
+    when :added
+      nil
+    when :deleted, :renamed
+      generate(:wiki_filename)
+    else
+      new_path
+    end
+  end
+
+  describe '#page' do
+    context 'the page does not exist' do
+      before do
+        expect(project_wiki).to receive(:find_page).with(String, String).and_return(nil)
+      end
+
+      %i[added deleted renamed modified].each do |op|
+        context "the operation is #{op}" do
+          let(:operation) { op }
+
+          it { is_expected.to have_attributes(page: be_nil) }
+        end
+      end
+    end
+
+    context 'the page can be found' do
+      let(:wiki_page) { double('WikiPage') }
+
+      before do
+        expect(project_wiki).to receive(:find_page).with(slug, revision).and_return(wiki_page)
+      end
+
+      context 'the page has been deleted' do
+        let(:operation) { :deleted }
+        let(:slug) { old_path.chomp('.md') }
+        let(:revision) { change[:oldrev] }
+
+        it { is_expected.to have_attributes(page: wiki_page) }
+      end
+
+      %i[added renamed modified].each do |op|
+        let(:operation) { op }
+        let(:slug) { new_path.chomp('.md') }
+        let(:revision) { change[:newrev] }
+
+        it { is_expected.to have_attributes(page: wiki_page) }
+      end
+    end
+  end
+
+  describe '#last_known_slug' do
+    context 'the page has been created' do
+      let(:operation) { :added }
+
+      it { is_expected.to have_attributes(last_known_slug: new_path.chomp('.md')) }
+    end
+
+    %i[renamed modified deleted].each do |op|
+      context "the operation is #{op}" do
+        let(:operation) { op }
+
+        it { is_expected.to have_attributes(last_known_slug: old_path.chomp('.md')) }
+      end
+    end
+  end
+
+  describe '#event_action' do
+    context 'the page is deleted' do
+      let(:operation) { :deleted }
+
+      it { is_expected.to have_attributes(event_action: Event::DESTROYED) }
+    end
+
+    context 'the page is added' do
+      let(:operation) { :added }
+
+      it { is_expected.to have_attributes(event_action: Event::CREATED) }
+    end
+
+    %i[renamed modified].each do |op|
+      context "the page is #{op}" do
+        let(:operation) { op }
+
+        it { is_expected.to have_attributes(event_action: Event::UPDATED) }
+      end
+    end
+  end
+end

spec/services/git/wiki_push_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Git::WikiPushService, services: true do
+  include RepoHelpers
+
+  let_it_be(:key_id) { create(:key, user: current_user).shell_id }
+  let_it_be(:project) { create(:project, :wiki_repo) }
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:git_wiki) { project.wiki.wiki }
+  let_it_be(:repository) { git_wiki.repository }
+
+  describe '#execute' do
+    context 'the push contains more than the permitted number of changes' do
+      def run_service
+        process_changes { described_class::MAX_CHANGES.succ.times { write_new_page } }
+      end
+
+      it 'creates only MAX_CHANGES events' do
+        expect { run_service }.to change(Event, :count).by(described_class::MAX_CHANGES)
+      end
+    end
+
+    context 'default_branch collides with a tag' do
+      it 'creates only one event' do
+        base_sha = current_sha
+        write_new_page
+
+        service = create_service(base_sha, ['refs/heads/master', 'refs/tags/master'])
+
+        expect { service.execute }.to change(Event, :count).by(1)
+      end
+    end
+
+    describe 'successfully creating events' do
+      let(:count) { Event::WIKI_ACTIONS.size }
+
+      def run_service
+        wiki_page_a = create(:wiki_page, project: project)
+        wiki_page_b = create(:wiki_page, project: project)
+
+        process_changes do
+          write_new_page
+          update_page(wiki_page_a.title)
+          delete_page(wiki_page_b.page.path)
+        end
+      end
+
+      it 'creates one event for every wiki action' do
+        expect { run_service }.to change(Event, :count).by(count)
+      end
+
+      it 'handles all known actions' do
+        run_service
+
+        expect(Event.last(count).pluck(:action)).to match_array(Event::WIKI_ACTIONS)
+      end
+    end
+
+    context 'two pages have been created' do
+      def run_service
+        process_changes do
+          write_new_page
+          write_new_page
+        end
+      end
+
+      it 'creates two events' do
+        expect { run_service }.to change(Event, :count).by(2)
+      end
+
+      it 'creates two metadata records' do
+        expect { run_service }.to change(WikiPage::Meta, :count).by(2)
+      end
+
+      it 'creates appropriate events' do
+        run_service
+
+        expect(Event.last(2)).to all(have_attributes(wiki_page?: true, action: Event::CREATED))
+      end
+    end
+
+    context 'a non-page file as been added' do
+      it 'does not create events, or WikiPage metadata' do
+        expect do
+          process_changes { write_non_page }
+        end.not_to change { [Event.count, WikiPage::Meta.count] }
+      end
+    end
+
+    context 'one page, and one non-page have been created' do
+      def run_service
+        process_changes do
+          write_new_page
+          write_non_page
+        end
+      end
+
+      it 'creates a wiki page creation event' do
+        expect { run_service }.to change(Event, :count).by(1)
+
+        expect(Event.last).to have_attributes(wiki_page?: true, action: Event::CREATED)
+      end
+
+      it 'creates one metadata record' do
+        expect { run_service }.to change(WikiPage::Meta, :count).by(1)
+      end
+    end
+
+    context 'one page has been added, and then updated' do
+      def run_service
+        process_changes do
+          title = write_new_page
+          update_page(title)
+        end
+      end
+
+      it 'creates just a single event' do
+        expect { run_service }.to change(Event, :count).by(1)
+      end
+
+      it 'creates just one metadata record' do
+        expect { run_service }.to change(WikiPage::Meta, :count).by(1)
+      end
+
+      it 'creates a new wiki page creation event' do
+        run_service
+
+        expect(Event.last).to have_attributes(
+          wiki_page?: true,
+          action: Event::CREATED
+        )
+      end
+    end
+
+    context 'when a page we already know about has been updated' do
+      let(:wiki_page) { create(:wiki_page, project: project) }
+
+      before do
+        create(:wiki_page_meta, :for_wiki_page, wiki_page: wiki_page)
+      end
+
+      def run_service
+        process_changes { update_page(wiki_page.title) }
+      end
+
+      it 'does not create a new meta-data record' do
+        expect { run_service }.not_to change(WikiPage::Meta, :count)
+      end
+
+      it 'creates a new event' do
+        expect { run_service }.to change(Event, :count).by(1)
+      end
+
+      it 'adds an update event' do
+        run_service
+
+        expect(Event.last).to have_attributes(
+          wiki_page?: true,
+          action: Event::UPDATED
+        )
+      end
+    end
+
+    context 'when a page we do not know about has been updated' do
+      def run_service
+        wiki_page = create(:wiki_page, project: project)
+        process_changes { update_page(wiki_page.title) }
+      end
+
+      it 'creates a new meta-data record' do
+        expect { run_service }.to change(WikiPage::Meta, :count).by(1)
+      end
+
+      it 'creates a new event' do
+        expect { run_service }.to change(Event, :count).by(1)
+      end
+
+      it 'adds an update event' do
+        run_service
+
+        expect(Event.last).to have_attributes(
+          wiki_page?: true,
+          action: Event::UPDATED
+        )
+      end
+    end
+
+    context 'when a page we do not know about has been deleted' do
+      def run_service
+        wiki_page = create(:wiki_page, project: project)
+        process_changes { delete_page(wiki_page.page.path) }
+      end
+
+      it 'create a new meta-data record' do
+        expect { run_service }.to change(WikiPage::Meta, :count).by(1)
+      end
+
+      it 'creates a new event' do
+        expect { run_service }.to change(Event, :count).by(1)
+      end
+
+      it 'adds an update event' do
+        run_service
+
+        expect(Event.last).to have_attributes(
+          wiki_page?: true,
+          action: Event::DESTROYED
+        )
+      end
+    end
+
+    it 'calls log_error for every event we cannot create' do
+      base_sha = current_sha
+      count = 3
+      count.times { write_new_page }
+      message = 'something went very very wrong'
+      allow_next_instance_of(WikiPages::EventCreateService, current_user) do |service|
+        allow(service).to receive(:execute)
+          .with(String, WikiPage, Integer)
+          .and_return(ServiceResponse.error(message: message))
+      end
+
+      service = create_service(base_sha)
+
+      expect(service).to receive(:log_error).exactly(count).times.with(message)
+
+      service.execute
+    end
+
+    describe 'feature flags' do
+      shared_examples 'a no-op push' do
+        it 'does not create any events' do
+          expect { process_changes { write_new_page } }.not_to change(Event, :count)
+        end
+
+        it 'does not even look for events to process' do
+          base_sha = current_sha
+          write_new_page
+
+          service = create_service(base_sha)
+
+          expect(service).not_to receive(:changed_files)
+
+          service.execute
+        end
+      end
+
+      context 'the wiki_events feature is disabled' do
+        before do
+          stub_feature_flags(wiki_events: false)
+        end
+
+        it_behaves_like 'a no-op push'
+      end
+
+      context 'the wiki_events_on_git_push feature is disabled' do
+        before do
+          stub_feature_flags(wiki_events_on_git_push: false)
+        end
+
+        it_behaves_like 'a no-op push'
+
+        context 'but is enabled for a given project' do
+          before do
+            stub_feature_flags(wiki_events_on_git_push: { enabled: true, thing: project })
+          end
+
+          it 'creates events' do
+            expect { process_changes { write_new_page } }.to change(Event, :count).by(1)
+          end
+        end
+      end
+    end
+  end
+
+  # In order to construct the correct GitPostReceive object that represents the
+  # changes we are applying, we need to describe the changes between old-ref and
+  # new-ref. Old ref (the base sha) we have to capture before we perform any
+  # changes. Once the changes have been applied, we can execute the service to
+  # process them.
+  def process_changes(&block)
+    base_sha = current_sha
+    yield
+    create_service(base_sha).execute
+  end
+
+  def create_service(base, refs = ['refs/heads/master'])
+    changes = post_received(base, refs).changes
+    described_class.new(project, current_user, changes: changes)
+  end
+
+  def post_received(base, refs)
+    change_str = refs.map { |ref| +"#{base} #{current_sha} #{ref}" }.join("\n")
+    post_received = ::Gitlab::GitPostReceive.new(project, key_id, change_str, {})
+    allow(post_received).to receive(:identify).with(key_id).and_return(current_user)
+
+    post_received
+  end
+
+  def current_sha
+    repository.gitaly_ref_client.find_branch('master')&.dereferenced_target&.id || Gitlab::Git::BLANK_SHA
+  end
+
+  # It is important not to re-use the WikiPage services here, since they create
+  # events - these helper methods below are intended to simulate actions on the repo
+  # that have not gone through our services.
+
+  def write_new_page
+    generate(:wiki_page_title).tap { |t| git_wiki.write_page(t, 'markdown', 'Hello', commit_details) }
+  end
+
+  # We write something to the wiki-repo that is not a page - as, for example, an
+  # attachment. This will appear as a raw-diff change, but wiki.find_page will
+  # return nil.
+  def write_non_page
+    params = {
+      file_name: 'attachment.log',
+      file_content: 'some stuff',
+      branch_name: 'master'
+    }
+    ::Wikis::CreateAttachmentService.new(project, project.owner, params).execute
+  end
+
+  def update_page(title)
+    page = git_wiki.page(title: title)
+    git_wiki.update_page(page.path, title, 'markdown', 'Hey', commit_details)
+  end
+
+  def delete_page(path)
+    git_wiki.delete_page(path, commit_details)
+  end
+
+  def commit_details
+    create(:git_wiki_commit_details, author: current_user)
+  end
+end

spec/services/wiki_pages/event_create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe WikiPages::EventCreateService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+
+  subject { described_class.new(user) }
+
+  describe '#execute' do
+    let_it_be(:page) { create(:wiki_page, project: project) }
+    let(:slug) { generate(:sluggified_title) }
+    let(:action) { Event::CREATED }
+    let(:response) { subject.execute(slug, page, action) }
+
+    context 'feature flag is not enabled' do
+      before do
+        stub_feature_flags(wiki_events: false)
+      end
+
+      it 'does not error' do
+        expect(response).to be_success
+          .and have_attributes(message: /No event created/)
+      end
+
+      it 'does not create an event' do
+        expect { response }.not_to change(Event, :count)
+      end
+    end
+
+    context 'the user is nil' do
+      subject { described_class.new(nil) }
+
+      it 'raises an error on construction' do
+        expect { subject }.to raise_error ArgumentError
+      end
+    end
+
+    context 'the action is illegal' do
+      let(:action) { Event::WIKI_ACTIONS.max + 1 }
+
+      it 'returns an error' do
+        expect(response).to be_error
+      end
+
+      it 'does not create an event' do
+        expect { response }.not_to change(Event, :count)
+      end
+
+      it 'does not create a metadata record' do
+        expect { response }.not_to change(WikiPage::Meta, :count)
+      end
+    end
+
+    it 'returns a successful response' do
+      expect(response).to be_success
+    end
+
+    context 'the action is a deletion' do
+      let(:action) { Event::DESTROYED }
+
+      it 'does not synchronize the wiki metadata timestamps with the git commit' do
+        expect_next_instance_of(WikiPage::Meta) do |instance|
+          expect(instance).not_to receive(:synch_times_with_page)
+        end
+
+        response
+      end
+    end
+
+    it 'creates a wiki page event' do
+      expect { response }.to change(Event, :count).by(1)
+    end
+
+    it 'returns an event in the payload' do
+      expect(response.payload).to include(event: have_attributes(author: user, wiki_page?: true, action: action))
+    end
+
+    it 'records the slug for the page' do
+      response
+      meta = WikiPage::Meta.find_or_create(page.slug, page)
+
+      expect(meta.slugs.pluck(:slug)).to include(slug)
+    end
+  end
+end

spec/workers/post_receive_spec.rb

@@ -299,6 +299,31 @@ describe PostReceive do
       end
     end
 
+    context "master" do
+      let(:default_branch) { 'master' }
+      let(:oldrev) { '012345' }
+      let(:newrev) { '6789ab' }
+      let(:changes) do
+        <<~EOF
+            #{oldrev} #{newrev} refs/heads/#{default_branch}
+            123456 789012 refs/heads/tést2
+        EOF
+      end
+
+      let(:raw_repo) { double('RawRepo') }
+
+      it 'processes the changes on the master branch' do
+        expect_next_instance_of(Git::WikiPushService) do |service|
+          expect(service).to receive(:process_changes).and_call_original
+        end
+        expect(project.wiki).to receive(:default_branch).twice.and_return(default_branch)
+        expect(project.wiki.repository).to receive(:raw).and_return(raw_repo)
+        expect(raw_repo).to receive(:raw_changes_between).once.with(oldrev, newrev).and_return([])
+
+        perform
+      end
+    end
+
     context "branches" do
       let(:changes) do
         <<~EOF
@@ -307,6 +332,12 @@ describe PostReceive do
         EOF
       end
 
+      before do
+        allow_next_instance_of(Git::WikiPushService) do |service|
+          allow(service).to receive(:process_changes)
+        end
+      end
+
       it 'expires the branches cache' do
         expect(project.wiki.repository).to receive(:expire_branches_cache).once