From 9c233a9b00525699604c4954e3a196100f44c9f4 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 27 Feb 2019 22:09:14 +0100 Subject: [PATCH 1/2] Create separate models for different registries One day `file_registries` will be split up in type specific tables. See https://gitlab.com/gitlab-org/gitlab-ee/issues/10067 This change already defines separate models. So the code can operate like they are separate tables, using single-table inheritance magic. --- ee/app/models/geo/file_registry.rb | 10 +++++++ ee/app/models/geo/lfs_object_registry.rb | 9 ++++++ ee/app/models/geo/upload_registry.rb | 12 ++++++++ .../unreleased/geo-split-file-registry.yml | 5 ++++ ee/spec/models/geo/file_registry_spec.rb | 6 ++-- .../models/geo/lfs_object_registry_spec.rb | 16 ++++++++++ ee/spec/models/geo/upload_registry_spec.rb | 30 +++++++++++++++++++ 7 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 ee/app/models/geo/lfs_object_registry.rb create mode 100644 ee/app/models/geo/upload_registry.rb create mode 100644 ee/changelogs/unreleased/geo-split-file-registry.yml create mode 100644 ee/spec/models/geo/lfs_object_registry_spec.rb create mode 100644 ee/spec/models/geo/upload_registry_spec.rb diff --git a/ee/app/models/geo/file_registry.rb b/ee/app/models/geo/file_registry.rb index 1fef42277917a0..8c0de66eb4fae4 100644 --- a/ee/app/models/geo/file_registry.rb +++ b/ee/app/models/geo/file_registry.rb @@ -5,4 +5,14 @@ class Geo::FileRegistry < Geo::BaseRegistry scope :lfs_objects, -> { where(file_type: :lfs) } scope :attachments, -> { where(file_type: Geo::FileService::DEFAULT_OBJECT_TYPES) } + + self.inheritance_column = 'file_type' + + def self.find_sti_class(file_type) + if file_type == 'lfs' + Geo::LfsObjectRegistry + elsif Geo::FileService::DEFAULT_OBJECT_TYPES.include?(file_type.to_sym) + Geo::UploadRegistry + end + end end diff --git a/ee/app/models/geo/lfs_object_registry.rb b/ee/app/models/geo/lfs_object_registry.rb new file mode 100644 index 00000000000000..709fbb95c395d3 --- /dev/null +++ b/ee/app/models/geo/lfs_object_registry.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class Geo::LfsObjectRegistry < Geo::FileRegistry + belongs_to :lfs_object, foreign_key: :file_id, class_name: 'LfsObject' + + def self.sti_name + 'lfs' + end +end diff --git a/ee/app/models/geo/upload_registry.rb b/ee/app/models/geo/upload_registry.rb new file mode 100644 index 00000000000000..751aaae0d813e7 --- /dev/null +++ b/ee/app/models/geo/upload_registry.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Geo::UploadRegistry < Geo::FileRegistry + belongs_to :upload, foreign_key: :file_id + + def self.type_condition(table = arel_table) + sti_column = arel_attribute(inheritance_column, table) + sti_names = Geo::FileService::DEFAULT_OBJECT_TYPES + + sti_column.in(sti_names) + end +end diff --git a/ee/changelogs/unreleased/geo-split-file-registry.yml b/ee/changelogs/unreleased/geo-split-file-registry.yml new file mode 100644 index 00000000000000..feeda2e8a5dd0c --- /dev/null +++ b/ee/changelogs/unreleased/geo-split-file-registry.yml @@ -0,0 +1,5 @@ +--- +title: "Geo: Create separate models for different registries" +merge_request: 9755 +author: +type: added diff --git a/ee/spec/models/geo/file_registry_spec.rb b/ee/spec/models/geo/file_registry_spec.rb index 892aba3c9944a1..b533f9544063d5 100644 --- a/ee/spec/models/geo/file_registry_spec.rb +++ b/ee/spec/models/geo/file_registry_spec.rb @@ -6,13 +6,13 @@ describe '.failed' do it 'returns registries in the failed state' do - expect(described_class.failed).to contain_exactly(failed) + expect(described_class.failed).to match_ids(failed) end end describe '.synced' do it 'returns registries in the synced state' do - expect(described_class.synced).to contain_exactly(synced) + expect(described_class.synced).to match_ids(synced) end end @@ -21,7 +21,7 @@ set(:retry_tomorrow) { create(:geo_file_registry, retry_at: Date.tomorrow) } it 'returns registries in the synced state' do - expect(described_class.retry_due).not_to contain_exactly([retry_tomorrow]) + expect(described_class.retry_due).to match_ids([failed, synced, retry_yesterday]) end end end diff --git a/ee/spec/models/geo/lfs_object_registry_spec.rb b/ee/spec/models/geo/lfs_object_registry_spec.rb new file mode 100644 index 00000000000000..7dd54c032f445b --- /dev/null +++ b/ee/spec/models/geo/lfs_object_registry_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Geo::LfsObjectRegistry, :geo do + set(:lfs_registry) { create(:geo_file_registry, :lfs, :with_file) } + set(:attachment_registry) { create(:geo_file_registry, :attachment) } + + it 'only finds lfs registries' do + expect(described_class.all).to match_ids(lfs_registry) + end + + it 'finds associated LfsObject record' do + expect(described_class.find(lfs_registry.id).lfs_object).to be_an_instance_of(LfsObject) + end +end diff --git a/ee/spec/models/geo/upload_registry_spec.rb b/ee/spec/models/geo/upload_registry_spec.rb new file mode 100644 index 00000000000000..d5064091102360 --- /dev/null +++ b/ee/spec/models/geo/upload_registry_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Geo::UploadRegistry, :geo do + set(:lfs_registry) { create(:geo_file_registry, :lfs) } + set(:attachment_registry) { create(:geo_file_registry, :attachment, :with_file) } + set(:avatar_registry) { create(:geo_file_registry, :avatar) } + set(:file_registry) { create(:geo_file_registry, :file) } + set(:namespace_file_registry) { create(:geo_file_registry, :namespace_file) } + set(:personal_file_registry) { create(:geo_file_registry, :personal_file) } + set(:favicon_registry) { create(:geo_file_registry, :favicon) } + set(:import_export_registry) { create(:geo_file_registry, :import_export) } + + it 'finds all upload registries' do + expected = [attachment_registry, + avatar_registry, + file_registry, + namespace_file_registry, + personal_file_registry, + favicon_registry, + import_export_registry] + + expect(described_class.all).to match_ids(expected) + end + + it 'finds associated Upload record' do + expect(described_class.find(attachment_registry.id).upload).to be_an_instance_of(Upload) + end +end -- GitLab From 67cf88467c7864b308d614394f2b08fde6ac311a Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 12 Mar 2019 15:04:00 +0100 Subject: [PATCH 2/2] Make code fail when Rails gem is updated --- ee/app/models/geo/upload_registry.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ee/app/models/geo/upload_registry.rb b/ee/app/models/geo/upload_registry.rb index 751aaae0d813e7..fd0198c7a40ed3 100644 --- a/ee/app/models/geo/upload_registry.rb +++ b/ee/app/models/geo/upload_registry.rb @@ -3,6 +3,11 @@ class Geo::UploadRegistry < Geo::FileRegistry belongs_to :upload, foreign_key: :file_id + if Rails.gem_version >= Gem::Version.new('6.0') + raise '.type_condition was changed in Rails 6.0, please adapt this code accordingly' + # see https://github.com/rails/rails/commit/6a1a1e66ea7a917942bd8369fa8dbfedce391dab + end + def self.type_condition(table = arel_table) sti_column = arel_attribute(inheritance_column, table) sti_names = Geo::FileService::DEFAULT_OBJECT_TYPES -- GitLab