Merge branch 'psk-move-wiki-page-concerns-back-to-models' into 'master'

Move wiki page concerns back inside models

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/169286



Merged-by: Stan Hu <stanhu@gmail.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Co-authored-by: Piotr Skorupa <pskorupa@gitlab.com>

.rubocop_todo/gitlab/bounded_contexts.yml

@@ -864,8 +864,6 @@ Gitlab/BoundedContexts:
     - 'app/models/concerns/has_repository.rb'
     - 'app/models/concerns/has_user_type.rb'
     - 'app/models/concerns/has_wiki.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
-    - 'app/models/concerns/has_wiki_page_slug_attributes.rb'
     - 'app/models/concerns/id_in_ordered.rb'
     - 'app/models/concerns/ignorable_columns.rb'
     - 'app/models/concerns/iid_routes.rb'

.rubocop_todo/gitlab/strong_memoize_attr.yml

@@ -86,7 +86,6 @@ Gitlab/StrongMemoizeAttr:
     - 'app/models/concerns/discussion_on_diff.rb'
     - 'app/models/concerns/has_repository.rb'
     - 'app/models/concerns/has_wiki.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/models/concerns/redis_cacheable.rb'
     - 'app/models/concerns/resolvable_discussion.rb'
     - 'app/models/concerns/security/latest_pipeline_information.rb'

.rubocop_todo/style/guard_clause.yml

@@ -70,7 +70,6 @@ Style/GuardClause:
     - 'app/models/concerns/cacheable_attributes.rb'
     - 'app/models/concerns/cascading_namespace_setting_attribute.rb'
     - 'app/models/concerns/deprecated_assignee.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/models/concerns/issuable_link.rb'
     - 'app/models/concerns/metric_image_uploading.rb'
     - 'app/models/concerns/milestoneish.rb'

.rubocop_todo/style/if_unless_modifier.yml

@@ -58,7 +58,6 @@ Style/IfUnlessModifier:
     - 'app/models/concerns/ci/artifactable.rb'
     - 'app/models/concerns/deprecated_assignee.rb'
     - 'app/models/concerns/group_descendant.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/models/concerns/ignorable_columns.rb'
     - 'app/models/concerns/issuable.rb'
     - 'app/models/concerns/issuable_link.rb'

.rubocop_todo/style/inline_disable_annotation.yml

@@ -390,7 +390,6 @@ Style/InlineDisableAnnotation:
     - 'app/models/concerns/from_set_operator.rb'
     - 'app/models/concerns/from_union.rb'
     - 'app/models/concerns/has_repository.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/models/concerns/ignorable_columns.rb'
     - 'app/models/concerns/integrations/reset_secret_fields.rb'
     - 'app/models/concerns/issuable.rb'

.rubocop_todo/style/redundant_parentheses.yml

@@ -5,7 +5,6 @@ Style/RedundantParentheses:
     - 'app/graphql/resolvers/concerns/caching_array_resolver.rb'
     - 'app/graphql/resolvers/tree_resolver.rb'
     - 'app/models/ci/build_metadata.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/services/lfs/file_transformer.rb'
     - 'app/services/members/invite_service.rb'
     - 'app/services/projects/lfs_pointers/lfs_object_download_list_service.rb'

.rubocop_todo/style/redundant_self.yml

@@ -39,7 +39,6 @@ Style/RedundantSelf:
     - 'app/models/concerns/featurable.rb'
     - 'app/models/concerns/has_user_type.rb'
     - 'app/models/concerns/has_wiki.rb'
-    - 'app/models/concerns/has_wiki_page_meta_attributes.rb'
     - 'app/models/concerns/ignorable_columns.rb'
     - 'app/models/concerns/integrations/base_data_fields.rb'
     - 'app/models/concerns/integrations/has_data_fields.rb'

app/models/concerns/has_wiki_page_meta_attributes.rb

-# frozen_string_literal: true
-
-module HasWikiPageMetaAttributes
-  extend ActiveSupport::Concern
-  include Gitlab::Utils::StrongMemoize
-
-  CanonicalSlugConflictError = Class.new(ActiveRecord::RecordInvalid)
-  WikiPageInvalid = Class.new(ArgumentError)
-
-  included do
-    has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
-
-    validates :title, length: { maximum: 255 }, allow_nil: false
-    validate :no_two_metarecords_in_same_container_can_have_same_canonical_slug
-
-    scope :with_canonical_slug, ->(slug) do
-      slug_table_name = klass.reflect_on_association(:slugs).table_name
-
-      joins(:slugs).where(slug_table_name => { canonical: true, slug: slug })
-    end
-  end
-
-  class_methods do
-    # 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?
-
-      container = wiki_page.wiki.container
-      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, container)
-        meta = found || create!(updates.merge(container_attrs(container)))
-
-        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/group_id enforce our other invariants
-        # This saves us a query.
-        meta
-      end
-    end
-
-    def find_by_canonical_slug(canonical_slug, container)
-      meta, conflict = with_canonical_slug(canonical_slug)
-        .where(container_attrs(container))
-        .limit(2)
-
-      if conflict.present?
-        meta.errors.add(:canonical_slug, 'Duplicate value found')
-        raise CanonicalSlugConflictError, meta
-      end
-
-      meta
-    end
-
-    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
-
-    def container_attrs(container)
-      return { project_id: container.id } if container.is_a?(Project)
-
-      { namespace_id: container.id } if container.is_a?(Group)
-    end
-  end
-
-  def canonical_slug
-    strong_memoize(:canonical_slug) { slugs.canonical.take&.slug }
-  end
-
-  # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  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
-  # rubocop:enable Gitlab/ModuleWithInstanceVariables
-
-  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
-
-  private
-
-  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)
-    creation = Time.current.utc
-
-    slug_attrs = strings.map do |slug|
-      slug_attributes(slug, canonical_slug, is_new, creation)
-    end
-    slugs.insert_all(slug_attrs) unless !is_new && slug_attrs.size == 1
-
-    @canonical_slug = canonical_slug if is_new || strings.size == 1 # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  end
-
-  def slug_attributes(slug, canonical_slug, is_new, creation)
-    {
-      slug: slug,
-      canonical: (is_new && slug == canonical_slug),
-      created_at: creation,
-      updated_at: creation
-    }.merge(slug_meta_attributes)
-  end
-
-  def slug_meta_attributes
-    { self.association(:slugs).reflection.foreign_key => id }
-  end
-
-  def no_two_metarecords_in_same_container_can_have_same_canonical_slug
-    container_id = attributes[container_key.to_s]
-
-    return unless container_id.present? && canonical_slug.present?
-
-    offending = self.class.with_canonical_slug(canonical_slug).where(container_key => container_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

app/models/concerns/has_wiki_page_slug_attributes.rb

-# frozen_string_literal: true
-
-module HasWikiPageSlugAttributes
-  extend ActiveSupport::Concern
-
-  included do
-    validates :slug, uniqueness: { scope: meta_foreign_key }
-    validates :slug, length: { maximum: 2048 }, allow_nil: false
-    validates :canonical, uniqueness: {
-      scope: meta_foreign_key,
-      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.current.utc))
-    end
-  end
-
-  def self.update_all(attrs = {})
-    super(attrs.reverse_merge(updated_at: Time.current.utc))
-  end
-end

app/models/wiki_page/meta.rb

@@ -2,24 +2,100 @@
 class WikiPage
   class Meta < ApplicationRecord
-    include HasWikiPageMetaAttributes
+    include Gitlab::Utils::StrongMemoize
     include Mentionable
     include Noteable
     self.table_name = 'wiki_page_meta'
+    CanonicalSlugConflictError = Class.new(ActiveRecord::RecordInvalid)
+    WikiPageInvalid = Class.new(ArgumentError)
+
     belongs_to :project, optional: true
     belongs_to :namespace, optional: true
+    has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent -- Technical debt
     has_many :slugs, class_name: 'WikiPage::Slug', foreign_key: 'wiki_page_meta_id', inverse_of: :wiki_page_meta
     has_many :notes, as: :noteable
     has_many :user_mentions, class_name: 'Wikis::UserMention', foreign_key: 'wiki_page_meta_id',
       inverse_of: :wiki_page_meta
+    validates :title, length: { maximum: 255 }, allow_nil: false
+    validate :no_two_metarecords_in_same_container_can_have_same_canonical_slug
     validate :project_or_namespace_present?
     alias_method :resource_parent, :project
+    scope :with_canonical_slug, ->(slug) do
+      slug_table_name = klass.reflect_on_association(:slugs).table_name
+
+      joins(:slugs).where(slug_table_name => { canonical: true, slug: slug })
+    end
+
+    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?
+
+        container = wiki_page.wiki.container
+        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, container)
+          meta = found || create!(updates.merge(container_attrs(container)))
+
+          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/group_id enforce our other invariants
+          # This saves us a query.
+          meta
+        end
+      end
+
+      def find_by_canonical_slug(canonical_slug, container)
+        meta, conflict = with_canonical_slug(canonical_slug)
+          .where(container_attrs(container))
+          .limit(2)
+
+        if conflict.present?
+          meta.errors.add(:canonical_slug, 'Duplicate value found')
+          raise CanonicalSlugConflictError, meta
+        end
+
+        meta
+      end
+
+      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
+
+      def container_attrs(container)
+        return { project_id: container.id } if container.is_a?(Project)
+
+        { namespace_id: container.id } if container.is_a?(Group)
+      end
+    end
+
     def container
       project || namespace
     end
@@ -37,6 +113,35 @@ def container_key
       for_group_wiki? ? :namespace_id : :project_id
     end
+    def canonical_slug
+      slugs.canonical.take&.slug
+    end
+    strong_memoize_attr :canonical_slug
+
+    # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Technical debt
+    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
+    # rubocop:enable Gitlab/ModuleWithInstanceVariables
+
+    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
+
     private
     def project_or_namespace_present?
@@ -44,5 +149,51 @@ def project_or_namespace_present?
       errors.add(:base, s_('Wiki|WikiPage::Meta should belong to either project or namespace.'))
     end
+
+    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)
+      creation = Time.current.utc
+
+      slug_attrs = strings.map do |slug|
+        slug_attributes(slug, canonical_slug, is_new, creation)
+      end
+      slugs.insert_all(slug_attrs) unless !is_new && slug_attrs.size == 1
+
+      @canonical_slug = canonical_slug if is_new || strings.size == 1 # rubocop:disable Gitlab/ModuleWithInstanceVariables -- Technical debt
+    end
+
+    def slug_attributes(slug, canonical_slug, is_new, creation)
+      {
+        slug: slug,
+        canonical: is_new && slug == canonical_slug,
+        created_at: creation,
+        updated_at: creation
+      }.merge(slug_meta_attributes)
+    end
+
+    def slug_meta_attributes
+      { association(:slugs).reflection.foreign_key => id }
+    end
+
+    def no_two_metarecords_in_same_container_can_have_same_canonical_slug
+      container_id = attributes[container_key.to_s]
+
+      return unless container_id.present? && canonical_slug.present?
+
+      offending = self.class.with_canonical_slug(canonical_slug).where(container_key => container_id)
+      offending = offending.where.not(id: id) if persisted?
+
+      return unless offending.exists?
+
+      errors.add(:canonical_slug, 'each page in a wiki must have a distinct canonical slug')
+    end
   end
 end

app/models/wiki_page/slug.rb

@@ -2,14 +2,26 @@
 class WikiPage
   class Slug < ApplicationRecord
-    def self.meta_foreign_key
-      :wiki_page_meta_id
-    end
-
-    include HasWikiPageSlugAttributes
-
     self.table_name = 'wiki_page_slugs'
     belongs_to :wiki_page_meta, class_name: 'WikiPage::Meta', inverse_of: :slugs
+
+    validates :slug, uniqueness: { scope: :wiki_page_meta_id }
+    validates :slug, length: { maximum: 2048 }, allow_nil: false
+    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 self.update_all(attrs = {})
+      super(attrs.reverse_merge(updated_at: Time.current.utc))
+    end
+
+    def update_columns(attrs = {})
+      super(attrs.reverse_merge(updated_at: Time.current.utc))
+    end
   end
 end