Merge branch 'group-wiki-repository-storage' into 'master'

Track group wiki storage in DB

See merge request gitlab-org/gitlab!31121

app/models/wiki.rb

@@ -44,11 +44,15 @@ class Wiki
   # Returns the Gitlab::Git::Wiki object.
   def wiki
     strong_memoize(:wiki) do
-      repository.create_if_not_exists
-      raise CouldNotCreateWikiError unless repository_exists?
-
+      create_wiki_repository
       Gitlab::Git::Wiki.new(repository.raw)
     end
+  end
+
+  def create_wiki_repository
+    repository.create_if_not_exists
+
+    raise CouldNotCreateWikiError unless repository_exists?
   rescue => err
     Gitlab::ErrorTracking.track_exception(err, wiki: {
       container_type: container.class.name,

db/migrate/20200430103158_create_group_wiki_repositories.rb

+# frozen_string_literal: true
+
+class CreateGroupWikiRepositories < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    create_table :group_wiki_repositories, id: false do |t|
+      t.bigint :shard_id, null: false, index: true
+      t.bigint :group_id, null: false, index: false, primary_key: true, default: nil
+
+      # The limit is added in db/migrate/20200511120430_add_group_wiki_repositories_disk_path_limit.rb
+      t.text :disk_path, null: false, index: { unique: true } # rubocop:disable Migration/AddLimitToTextColumns
+    end
+  end
+end

db/migrate/20200511121549_add_group_wiki_repositories_shard_id_foreign_key.rb

+# frozen_string_literal: true
+
+class AddGroupWikiRepositoriesShardIdForeignKey < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :group_wiki_repositories, :shards, on_delete: :restrict # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :group_wiki_repositories, :shards
+    end
+  end
+end

db/migrate/20200511121610_add_group_wiki_repositories_group_id_foreign_key.rb

+# frozen_string_literal: true
+
+class AddGroupWikiRepositoriesGroupIdForeignKey < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :group_wiki_repositories, :namespaces, column: :group_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :group_wiki_repositories, :namespaces, column: :group_id
+    end
+  end
+end

db/migrate/20200511121620_add_group_wiki_repositories_disk_path_limit.rb

+# frozen_string_literal: true
+
+class AddGroupWikiRepositoriesDiskPathLimit < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :group_wiki_repositories, :disk_path, 80
+  end
+
+  def down
+    remove_text_limit :group_wiki_repositories, :disk_path
+  end
+end

db/structure.sql

@@ -3167,6 +3167,13 @@ CREATE SEQUENCE public.group_import_states_group_id_seq
 
 ALTER SEQUENCE public.group_import_states_group_id_seq OWNED BY public.group_import_states.group_id;
 
+CREATE TABLE public.group_wiki_repositories (
+    shard_id bigint NOT NULL,
+    group_id bigint NOT NULL,
+    disk_path text NOT NULL,
+    CONSTRAINT check_07f1c81806 CHECK ((char_length(disk_path) <= 80))
+);
+
 CREATE TABLE public.historical_data (
     id integer NOT NULL,
     date date NOT NULL,
@@ -8312,6 +8319,9 @@ ALTER TABLE ONLY public.group_group_links
 ALTER TABLE ONLY public.group_import_states
     ADD CONSTRAINT group_import_states_pkey PRIMARY KEY (group_id);
 
+ALTER TABLE ONLY public.group_wiki_repositories
+    ADD CONSTRAINT group_wiki_repositories_pkey PRIMARY KEY (group_id);
+
 ALTER TABLE ONLY public.historical_data
     ADD CONSTRAINT historical_data_pkey PRIMARY KEY (id);
 
@@ -9723,6 +9733,10 @@ CREATE INDEX index_group_group_links_on_shared_with_group_id ON public.group_gro
 
 CREATE INDEX index_group_import_states_on_group_id ON public.group_import_states USING btree (group_id);
 
+CREATE UNIQUE INDEX index_group_wiki_repositories_on_disk_path ON public.group_wiki_repositories USING btree (disk_path);
+
+CREATE INDEX index_group_wiki_repositories_on_shard_id ON public.group_wiki_repositories USING btree (shard_id);
+
 CREATE INDEX index_identities_on_saml_provider_id ON public.identities USING btree (saml_provider_id) WHERE (saml_provider_id IS NOT NULL);
 
 CREATE INDEX index_identities_on_user_id ON public.identities USING btree (user_id);
@@ -11625,6 +11639,9 @@ ALTER TABLE ONLY public.cluster_providers_aws
 ALTER TABLE ONLY public.grafana_integrations
     ADD CONSTRAINT fk_rails_18d0e2b564 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.group_wiki_repositories
+    ADD CONSTRAINT fk_rails_19755e374b FOREIGN KEY (shard_id) REFERENCES public.shards(id) ON DELETE RESTRICT;
+
 ALTER TABLE ONLY public.open_project_tracker_data
     ADD CONSTRAINT fk_rails_1987546e48 FOREIGN KEY (service_id) REFERENCES public.services(id) ON DELETE CASCADE;
 
@@ -11673,6 +11690,9 @@ ALTER TABLE ONLY public.service_desk_settings
 ALTER TABLE ONLY public.group_custom_attributes
     ADD CONSTRAINT fk_rails_246e0db83a FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.group_wiki_repositories
+    ADD CONSTRAINT fk_rails_26f867598c FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.lfs_file_locks
     ADD CONSTRAINT fk_rails_27a1d98fa8 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
@@ -13758,6 +13778,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200429181335
 20200429181955
 20200429182245
+20200430103158
 20200505164958
 20200505171834
 20200505172405
@@ -13767,6 +13788,9 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200507221434
 20200508091106
 20200511092714
+20200511121549
+20200511121610
+20200511121620
 20200511145545
 20200512085150
 \.

ee/app/models/ee/group.rb

@@ -48,6 +48,8 @@ module EE
       delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true
       delegate :enforced_group_managed_accounts?, :enforced_sso?, to: :saml_provider, allow_nil: true
 
+      has_one :group_wiki_repository
+
       belongs_to :file_template_project, class_name: "Project"
 
       belongs_to :push_rule

ee/app/models/group_wiki.rb

@@ -3,6 +3,14 @@
 class GroupWiki < Wiki
   alias_method :group, :container
 
+  override :create_wiki_repository
+  def create_wiki_repository
+    super
+
+    storage_record = container.group_wiki_repository || container.build_group_wiki_repository
+    storage_record.update!(shard_name: repository_storage, disk_path: storage.disk_path)
+  end
+
   override :storage
   def storage
     @storage ||= Storage::Hashed.new(container, prefix: Storage::Hashed::GROUP_REPOSITORY_PATH_PREFIX)
@@ -10,9 +18,7 @@ class GroupWiki < Wiki
 
   override :repository_storage
   def repository_storage
-    # TODO: Add table to track storage
-    # https://gitlab.com/gitlab-org/gitlab/-/issues/207865
-    'default'
+    container.group_wiki_repository&.shard_name || self.class.pick_repository_storage
   end
 
   override :hashed_storage?

ee/app/models/group_wiki_repository.rb

+# frozen_string_literal: true
+
+class GroupWikiRepository < ApplicationRecord
+  include Shardable
+
+  belongs_to :group
+
+  validates :group, :disk_path, presence: true, uniqueness: true
+end

ee/changelogs/unreleased/group-wiki-repository-storage.yml

+---
+title: Track group wiki storage in DB
+merge_request: 31121
+author:
+type: other

ee/spec/models/group_spec.rb

@@ -18,6 +18,7 @@ describe Group do
     it { is_expected.to have_many(:ip_restrictions) }
     it { is_expected.to have_one(:dependency_proxy_setting) }
     it { is_expected.to have_one(:deletion_schedule) }
+    it { is_expected.to have_one(:group_wiki_repository) }
     it { is_expected.to belong_to(:push_rule) }
 
     it_behaves_like 'model with wiki' do

ee/spec/models/group_wiki_repository_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupWikiRepository do
+  describe 'associations' do
+    it { is_expected.to belong_to(:shard) }
+    it { is_expected.to belong_to(:group) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:shard) }
+    it { is_expected.to validate_presence_of(:group) }
+    it { is_expected.to validate_presence_of(:disk_path) }
+
+    context 'uniqueness' do
+      subject { described_class.new(shard: build(:shard), group: build(:group), disk_path: 'path') }
+
+      it { is_expected.to validate_uniqueness_of(:group) }
+      it { is_expected.to validate_uniqueness_of(:disk_path) }
+    end
+  end
+end

ee/spec/models/group_wiki_spec.rb

@@ -11,6 +11,52 @@ describe GroupWiki do
       wiki_container.add_owner(user)
     end
 
+    describe '#create_wiki_repository' do
+      before do
+        # Don't actually create the repository, because we're using storage shards that don't exist.
+        allow(subject.repository).to receive(:create_if_not_exists)
+        allow(subject).to receive(:repository_exists?).and_return(true)
+      end
+
+      context 'when a tracking entry does not exist' do
+        let(:wiki_container) { wiki_container_without_repo }
+
+        it 'creates a new entry' do
+          expect { subject.create_wiki_repository }.to change(wiki_container, :group_wiki_repository)
+            .from(nil).to(kind_of(GroupWikiRepository))
+        end
+
+        it 'tracks the storage location' do
+          expect(subject).to receive(:repository_storage).and_return('foo')
+
+          subject.create_wiki_repository
+
+          expect(wiki_container.group_wiki_repository).to have_attributes(
+            disk_path: subject.storage.disk_path,
+            shard_name: 'foo'
+          )
+        end
+      end
+
+      context 'when a tracking entry exists' do
+        it 'does not create a new entry in the database' do
+          expect { subject.create_wiki_repository }.not_to change(wiki_container, :group_wiki_repository)
+        end
+
+        it 'updates the storage location' do
+          expect(subject).to receive(:repository_storage).and_return('foo')
+          expect(subject.storage).to receive(:disk_path).and_return('fancy/new/path')
+
+          subject.create_wiki_repository
+
+          expect(wiki_container.group_wiki_repository).to have_attributes(
+            disk_path: 'fancy/new/path',
+            shard_name: 'foo'
+          )
+        end
+      end
+    end
+
     describe '#storage' do
       it 'uses the group repository prefix' do
         expect(subject.storage.base_dir).to start_with('@groups/')
@@ -18,8 +64,19 @@ describe GroupWiki do
     end
 
     describe '#repository_storage' do
-      it 'returns the default storage' do
-        expect(subject.repository_storage).to eq('default')
+      context 'when a tracking entry does not exist' do
+        let(:wiki_container) { wiki_container_without_repo }
+
+        it 'returns the default shard' do
+          expect(subject.repository_storage).to eq('default')
+        end
+      end
+
+      context 'when a tracking entry exists' do
+        it 'returns the persisted shard if the repository is tracked' do
+          expect(wiki_container.group_wiki_repository).to receive(:shard_name).and_return('foo')
+          expect(subject.repository_storage).to eq('foo')
+        end
       end
     end