Merge branch '30526-a-be-wiki-activity-Models' into 'master'

#30526 (A) [BE] Wiki Events (models)

See merge request gitlab-org/gitlab!26529

app/models/event.rb

@@ -6,6 +6,7 @@ class Event < ApplicationRecord
   include Presentable
   include DeleteWithLimit
   include CreatedAtFilterable
+  include Gitlab::Utils::StrongMemoize
 
   default_scope { reorder(nil) }
 
@@ -42,7 +43,8 @@ class Event < ApplicationRecord
     note:           Note,
     project:        Project,
     snippet:        Snippet,
-    user:           User
+    user:           User,
+    wiki:           WikiPage::Meta
   ).freeze
 
   RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour
@@ -79,6 +81,7 @@ class Event < ApplicationRecord
   scope :recent, -> { reorder(id: :desc) }
   scope :code_push, -> { where(action: PUSHED) }
   scope :merged, -> { where(action: MERGED) }
+  scope :for_wiki_page, -> { where(target_type: WikiPage::Meta.name) }
 
   scope :with_associations, -> do
     # We're using preload for "push_event_payload" as otherwise the association
@@ -197,6 +200,14 @@ class Event < ApplicationRecord
     created_action? && !target && target_type.nil?
   end
 
+  def created_wiki_page?
+    wiki_page? && action == CREATED
+  end
+
+  def updated_wiki_page?
+    wiki_page? && action == UPDATED
+  end
+
   def created_target?
     created_action? && target
   end
@@ -217,6 +228,10 @@ class Event < ApplicationRecord
     target_type == "MergeRequest"
   end
 
+  def wiki_page?
+    target_type == WikiPage::Meta.name
+  end
+
   def milestone
     target if milestone?
   end
@@ -229,6 +244,14 @@ class Event < ApplicationRecord
     target if merge_request?
   end
 
+  def wiki_page
+    strong_memoize(:wiki_page) do
+      next unless wiki_page?
+
+      ProjectWiki.new(project, author).find_page(target.canonical_slug)
+    end
+  end
+
   def note
     target if note?
   end
@@ -250,6 +273,10 @@ class Event < ApplicationRecord
       'destroyed'
     elsif commented_action?
       "commented on"
+    elsif created_wiki_page?
+      'created'
+    elsif updated_wiki_page?
+      'updated'
     elsif created_project_action?
       created_project_action_name
     else
@@ -362,6 +389,8 @@ class Event < ApplicationRecord
                         :read_snippet
                       elsif milestone?
                         :read_milestone
+                      elsif wiki_page?
+                        :read_wiki
                       end
                     end
   end

app/models/project_wiki.rb

@@ -19,10 +19,11 @@ class ProjectWiki
   DIRECTION_DESC = 'desc'
   DIRECTION_ASC = 'asc'
 
+  attr_reader :project, :user
+
   # Returns a string describing what went wrong after
   # an operation fails.
   attr_reader :error_message
-  attr_reader :project
 
   def initialize(project, user = nil)
     @project = project
@@ -196,9 +197,9 @@ class ProjectWiki
 
   def commit_details(action, message = nil, title = nil)
     commit_message = message.presence || default_message(action, title)
-    git_user = Gitlab::Git::User.from_gitlab(@user)
+    git_user = Gitlab::Git::User.from_gitlab(user)
 
-    Gitlab::Git::Wiki::CommitDetails.new(@user.id,
+    Gitlab::Git::Wiki::CommitDetails.new(user.id,
                                          git_user.username,
                                          git_user.name,
                                          git_user.email,
@@ -206,7 +207,7 @@ class ProjectWiki
   end
 
   def default_message(action, title)
-    "#{@user.username} #{action} page: #{title}"
+    "#{user.username} #{action} page: #{title}"
   end
 
   def update_project_activity

app/models/wiki_page.rb

@@ -21,6 +21,14 @@ class WikiPage
     ActiveModel::Name.new(self, nil, 'wiki')
   end
 
+  def eql?(other)
+    return false unless other.present? && other.is_a?(self.class)
+
+    slug == other.slug && wiki.project == other.wiki.project
+  end
+
+  alias_method :==, :eql?
+
   # Sorts and groups pages by directory.
   #
   # pages - an array of WikiPage objects.
@@ -58,6 +66,7 @@ class WikiPage
 
   # The GitLab ProjectWiki instance.
   attr_reader :wiki
+  delegate :project, to: :wiki
 
   # The raw Gitlab::Git::WikiPage instance.
   attr_reader :page
@@ -70,6 +79,10 @@ class WikiPage
     Gitlab::HookData::WikiPageBuilder.new(self).build
   end
 
+  # Construct a new WikiPage
+  #
+  # @param [ProjectWiki] wiki
+  # @param [Gitlab::Git::WikiPage] page
   def initialize(wiki, page = nil)
     @wiki       = wiki
     @page       = page

app/models/wiki_page/meta.rb

+# frozen_string_literal: true
+
+class WikiPage
+  class Meta < ApplicationRecord
+    include Gitlab::Utils::StrongMemoize
+
+    CanonicalSlugConflictError = Class.new(ActiveRecord::RecordInvalid)
+
+    self.table_name = 'wiki_page_meta'
+
+    belongs_to :project
+
+    has_many :slugs, class_name: 'WikiPage::Slug', foreign_key: 'wiki_page_meta_id', inverse_of: :wiki_page_meta
+    has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+
+    validates :title, presence: true
+    validates :project_id, presence: true
+    validate :no_two_metarecords_in_same_project_can_have_same_canonical_slug
+
+    scope :with_canonical_slug, ->(slug) do
+      joins(:slugs).where(wiki_page_slugs: { canonical: true, slug: slug })
+    end
+
+    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
+      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)
+
+      if conflict.present?
+        meta.errors.add(:canonical_slug, 'Duplicate value found')
+        raise CanonicalSlugConflictError.new(meta)
+      end
+
+      meta
+    end
+
+    def canonical_slug
+      strong_memoize(:canonical_slug) { slugs.canonical.first&.slug }
+    end
+
+    def canonical_slug=(slug)
+      return if @canonical_slug == slug
+
+      if persisted?
+        transaction do
+          slugs.canonical.update_all(canonical: false)
+          page_slug = slugs.create_with(canonical: true).find_or_create_by(slug: slug)
+          page_slug.update_columns(canonical: true) unless page_slug.canonical?
+        end
+      else
+        slugs.new(slug: slug, canonical: true)
+      end
+
+      @canonical_slug = slug
+    end
+
+    def update_state(created, known_slugs, wiki_page)
+      update_wiki_page_attributes(wiki_page)
+      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
+    end
+
+    def insert_slugs(strings, is_new, canonical_slug)
+      creation = Time.now.utc
+
+      slug_attrs = strings.map do |slug|
+        {
+          wiki_page_meta_id: id,
+          slug: slug,
+          canonical: (is_new && slug == canonical_slug),
+          created_at: creation,
+          updated_at: creation
+        }
+      end
+      slugs.insert_all(slug_attrs) unless !is_new && slug_attrs.size == 1
+
+      @canonical_slug = canonical_slug if is_new || strings.size == 1
+    end
+
+    def no_two_metarecords_in_same_project_can_have_same_canonical_slug
+      return unless project_id.present? && canonical_slug.present?
+
+      offending = self.class.with_canonical_slug(canonical_slug).where(project_id: project_id)
+      offending = offending.where.not(id: id) if persisted?
+
+      if offending.exists?
+        errors.add(:canonical_slug, 'each page in a wiki must have a distinct canonical slug')
+      end
+    end
+  end
+end

app/models/wiki_page/slug.rb

+# frozen_string_literal: true
+
+class WikiPage
+  class Slug < ApplicationRecord
+    self.table_name = 'wiki_page_slugs'
+
+    belongs_to :wiki_page_meta, class_name: 'WikiPage::Meta', inverse_of: :slugs
+
+    validates :slug, presence: true, uniqueness: { scope: :wiki_page_meta_id }
+    validates :canonical, uniqueness: {
+          scope: :wiki_page_meta_id,
+          if: :canonical?,
+          message: 'Only one slug can be canonical per wiki metadata record'
+    }
+
+    scope :canonical, -> { where(canonical: true) }
+
+    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
+  end
+end

app/policies/wiki_page/meta_policy.rb

+# frozen_string_literal: true
+
+class WikiPage::MetaPolicy < BasePolicy
+  delegate { @subject.project }
+end

changelogs/unreleased/30526-a-be-wiki-activity-Models.yml

+---
+title: Adds wiki metadata models
+merge_request: 26529
+author:
+type: added

db/migrate/20200302152516_add_wiki_slug.rb

+# frozen_string_literal: true
+
+class AddWikiSlug < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    create_table :wiki_page_meta, id: :serial do |t|
+      t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false
+      t.timestamps_with_timezone null: false
+      t.string :title, null: false, limit: 255
+    end
+
+    create_table :wiki_page_slugs, id: :serial do |t|
+      t.boolean :canonical, default: false, null: false
+      t.references :wiki_page_meta, index: true, foreign_key: { on_delete: :cascade }, null: false
+      t.timestamps_with_timezone null: false
+      t.string :slug, null: false, limit: 2048
+      t.index [:slug, :wiki_page_meta_id], unique: true
+      t.index [:wiki_page_meta_id], name: 'one_canonical_wiki_page_slug_per_metadata', unique: true, where: "(canonical = true)"
+    end
+  end
+end

db/schema.rb

@@ -4668,6 +4668,25 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do
     t.index ["type"], name: "index_web_hooks_on_type"
   end
 
+  create_table "wiki_page_meta", id: :serial, force: :cascade do |t|
+    t.bigint "project_id", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "title", limit: 255, null: false
+    t.index ["project_id"], name: "index_wiki_page_meta_on_project_id"
+  end
+
+  create_table "wiki_page_slugs", id: :serial, force: :cascade do |t|
+    t.boolean "canonical", default: false, null: false
+    t.bigint "wiki_page_meta_id", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "slug", limit: 2048, null: false
+    t.index ["slug", "wiki_page_meta_id"], name: "index_wiki_page_slugs_on_slug_and_wiki_page_meta_id", unique: true
+    t.index ["wiki_page_meta_id"], name: "index_wiki_page_slugs_on_wiki_page_meta_id"
+    t.index ["wiki_page_meta_id"], name: "one_canonical_wiki_page_slug_per_metadata", unique: true, where: "(canonical = true)"
+  end
+
   create_table "x509_certificates", force: :cascade do |t|
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
@@ -5212,6 +5231,8 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do
   add_foreign_key "vulnerability_scanners", "projects", on_delete: :cascade
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
   add_foreign_key "web_hooks", "projects", name: "fk_0c8ca6d9d1", on_delete: :cascade
+  add_foreign_key "wiki_page_meta", "projects", on_delete: :cascade
+  add_foreign_key "wiki_page_slugs", "wiki_page_meta", column: "wiki_page_meta_id", on_delete: :cascade
   add_foreign_key "x509_certificates", "x509_issuers", on_delete: :cascade
   add_foreign_key "x509_commit_signatures", "projects", on_delete: :cascade
   add_foreign_key "x509_commit_signatures", "x509_certificates", on_delete: :cascade

lib/gitlab/hook_data/wiki_page_builder.rb

@@ -12,6 +12,10 @@ module Gitlab
             'content' => absolute_image_urls(wiki_page.content)
           )
       end
+
+      def uploads_prefix
+        ''
+      end
     end
   end
 end

spec/factories/events.rb

@@ -22,6 +22,16 @@ FactoryBot.define do
       action { Event::CLOSED }
       target factory: :closed_issue
     end
+
+    factory :wiki_page_event do
+      action { Event::CREATED }
+
+      transient do
+        wiki_page { create(:wiki_page, project: project) }
+      end
+
+      target { create(:wiki_page_meta, :for_wiki_page, wiki_page: wiki_page) }
+    end
   end
 
   factory :push_event, class: 'PushEvent' do

spec/factories/wiki_pages.rb

@@ -5,17 +5,22 @@ require 'ostruct'
 FactoryBot.define do
   factory :wiki_page do
     transient do
+      title { generate(:wiki_page_title) }
+      content { 'Content for wiki page' }
+      format { 'markdown' }
+      project { create(:project) }
       attrs do
         {
-          title: 'Title.with.dot',
-          content: 'Content for wiki page',
-          format: 'markdown'
+          title: title,
+          content: content,
+          format: format
         }
       end
     end
 
     page { OpenStruct.new(url_path: 'some-name') }
-    association :wiki, factory: :project_wiki, strategy: :build
+    wiki { build(:project_wiki, project: project) }
+
     initialize_with { new(wiki, page) }
 
     before(:create) do |page, evaluator|
@@ -25,5 +30,48 @@ FactoryBot.define do
     to_create do |page|
       page.create
     end
+
+    trait :with_real_page do
+      project { create(:project, :repository) }
+
+      page do
+        wiki.create_page(title, content)
+        page_title, page_dir = wiki.page_title_and_dir(title)
+        wiki.wiki.page(title: page_title, dir: page_dir, version: nil)
+      end
+    end
   end
+
+  factory :wiki_page_meta, class: 'WikiPage::Meta' do
+    title { generate(:wiki_page_title) }
+    project { create(:project) }
+
+    trait :for_wiki_page do
+      transient do
+        wiki_page { create(:wiki_page, project: project) }
+      end
+
+      project { @overrides[:wiki_page]&.project || create(:project) }
+      title { wiki_page.title }
+
+      initialize_with do
+        raise 'Metadata only available for valid pages' unless wiki_page.valid?
+
+        WikiPage::Meta.find_or_create(wiki_page.slug, wiki_page)
+      end
+    end
+  end
+
+  factory :wiki_page_slug, class: 'WikiPage::Slug' do
+    wiki_page_meta { create(:wiki_page_meta) }
+    slug { generate(:sluggified_title) }
+    canonical { false }
+
+    trait :canonical do
+      canonical { true }
+    end
+  end
+
+  sequence(:wiki_page_title) { |n| "Page #{n}" }
+  sequence(:sluggified_title) { |n| "slug-#{n}" }
 end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -40,6 +40,14 @@ Event:
 - updated_at
 - action
 - author_id
+WikiPage::Meta:
+- id
+- title
+- project_id
+WikiPage::Slug:
+- id
+- wiki_page_meta_id
+- slug
 PushEventPayload:
 - commit_count
 - action

spec/models/wiki_page/slug_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe WikiPage::Slug do
+  let_it_be(:meta) { create(:wiki_page_meta) }
+
+  describe 'Associations' do
+    it { is_expected.to belong_to(:wiki_page_meta) }
+
+    it 'refers correctly to the wiki_page_meta' do
+      created = create(:wiki_page_slug, wiki_page_meta: meta)
+
+      expect(created.reload.wiki_page_meta).to eq(meta)
+    end
+  end
+
+  describe 'scopes' do
+    describe 'canonical' do
+      subject { described_class.canonical }
+
+      context 'there are no slugs' do
+        it { is_expected.to be_empty }
+      end
+
+      context 'there are some non-canonical slugs' do
+        before do
+          create(:wiki_page_slug)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'there is at least one canonical slugs' do
+        before do
+          create(:wiki_page_slug, :canonical)
+        end
+
+        it { is_expected.not_to be_empty }
+      end
+    end
+  end
+
+  describe 'Validations' do
+    let(:canonical) { false }
+
+    subject do
+      build(:wiki_page_slug, canonical: canonical, wiki_page_meta: meta)
+    end
+
+    it { is_expected.to validate_presence_of(:slug) }
+    it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:wiki_page_meta_id) }
+
+    describe 'only_one_slug_can_be_canonical_per_meta_record' do
+      context 'there are no other slugs' do
+        it { is_expected.to be_valid }
+
+        context 'the current slug is canonical' do
+          let(:canonical) { true }
+
+          it { is_expected.to be_valid }
+        end
+      end
+
+      context 'there are other slugs, but they are not canonical' do
+        before do
+          create(:wiki_page_slug, wiki_page_meta: meta)
+        end
+
+        it { is_expected.to be_valid }
+
+        context 'the current slug is canonical' do
+          let(:canonical) { true }
+
+          it { is_expected.to be_valid }
+        end
+      end
+
+      context 'there is already a canonical slug' do
+        before do
+          create(:wiki_page_slug, canonical: true, wiki_page_meta: meta)
+        end
+
+        it { is_expected.to be_valid }
+
+        context 'the current slug is canonical' do
+          let(:canonical) { true }
+
+          it { is_expected.not_to be_valid }
+        end
+      end
+    end
+  end
+end

spec/models/wiki_page_spec.rb

@@ -606,12 +606,36 @@ describe WikiPage do
       expect(subject).to eq(subject)
     end
 
-    it 'returns false for updated wiki page' do
+    it 'returns true for updated wiki page' do
       subject.update(content: "Updated content")
-      updated_page = wiki.find_page('test page')
+      updated_page = wiki.find_page(existing_page.slug)
 
       expect(updated_page).not_to be_nil
-      expect(updated_page).not_to eq(subject)
+      expect(updated_page).to eq(subject)
+    end
+
+    it 'returns false for a completely different wiki page' do
+      other_page = create(:wiki_page)
+
+      expect(subject.slug).not_to eq(other_page.slug)
+      expect(subject.project).not_to eq(other_page.project)
+      expect(subject).not_to eq(other_page)
+    end
+
+    it 'returns false for page with different slug on same project' do
+      other_page = create(:wiki_page, project: subject.project)
+
+      expect(subject.slug).not_to eq(other_page.slug)
+      expect(subject.project).to eq(other_page.project)
+      expect(subject).not_to eq(other_page)
+    end
+
+    it 'returns false for page with the same slug on a different project' do
+      other_page = create(:wiki_page, title: existing_page.slug)
+
+      expect(subject.slug).to eq(other_page.slug)
+      expect(subject.project).not_to eq(other_page.project)
+      expect(subject).not_to eq(other_page)
     end
   end