diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index d861ef646f889fe7114afd3c603576ba453af53d..74ad78ff4c1daa5816638cf498ea05dee044bb1c 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -36,7 +36,7 @@ def index # rubocop: disable CodeReuse/ActiveRecord def starred @projects = load_projects(params.merge(starred: true)) - .includes(:forked_from_project, :topics) + .includes(:forked_from_project, :topics, :topics_acts_as_taggable) @groups = [] diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index dca3d12f3c99d3a70d6dd1b1fbdf871b7ee7d458..6bb1df9ee8e8bc00d39bee92b23befc9bb5cd4e2 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -180,7 +180,14 @@ def by_visibility_level(items) # rubocop: enable CodeReuse/ActiveRecord def by_topics(items) - params[:topic].present? ? items.tagged_with(params[:topic]) : items + return items unless params[:topic].present? + + topics = params[:topic].instance_of?(String) ? params[:topic].strip.split(/\s*,\s*/) : params[:topic] + topics.each do |topic| + items = items.with_topic(topic) + end + + items end def by_search(items) diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index af1f1c54ec2d390b49d55f9e3b058c3b4d513aeb..4530826dd859fb5fcb14da6a90d924c2d3dff8c2 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -406,6 +406,10 @@ def tag_list object.topic_list end + def topics + object.topic_list + end + private def project diff --git a/app/models/project.rb b/app/models/project.rb index 81b04e1316c7c45ac7e0bf4ea7f59aedd022e768..6fc0cb5017b8fcd829a659a504ba203806d4ce29 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -103,6 +103,8 @@ class Project < ApplicationRecord after_save :create_import_state, if: ->(project) { project.import? && project.import_state.nil? } + after_save :save_topics + after_create -> { create_or_load_association(:project_feature) } after_create -> { create_or_load_association(:ci_cd_settings) } @@ -127,12 +129,31 @@ class Project < ApplicationRecord after_initialize :use_hashed_storage after_create :check_repository_absence! + # Required during the `ActsAsTaggableOn::Tag -> Topic` migration + # TODO: remove 'acts_as_ordered_taggable_on' and ':topics_acts_as_taggable' in the further process of the migration + # https://gitlab.com/gitlab-org/gitlab/-/issues/335946 acts_as_ordered_taggable_on :topics + has_many :topics_acts_as_taggable, -> { order("#{ActsAsTaggableOn::Tagging.table_name}.id") }, + class_name: 'ActsAsTaggableOn::Tag', + through: :topic_taggings, + source: :tag + + has_many :project_topics, -> { order(:id) }, class_name: 'Projects::ProjectTopic' + has_many :topics, through: :project_topics, class_name: 'Projects::Topic' + + # Required during the `ActsAsTaggableOn::Tag -> Topic` migration + # TODO: remove 'topics' in the further process of the migration + # https://gitlab.com/gitlab-org/gitlab/-/issues/335946 + alias_method :topics_new, :topics + def topics + self.topics_acts_as_taggable + self.topics_new + end attr_accessor :old_path_with_namespace attr_accessor :template_name attr_writer :pipeline_status attr_accessor :skip_disk_validation + attr_writer :topic_list alias_attribute :title, :name @@ -623,6 +644,19 @@ def self.integration_association_name(name) joins(:service_desk_setting).where('service_desk_settings.project_key' => key) end + scope :with_topic, ->(topic_name) do + topic = Projects::Topic.find_by_name(topic_name) + acts_as_taggable_on_topic = ActsAsTaggableOn::Tag.find_by_name(topic_name) + + return none unless topic || acts_as_taggable_on_topic + + relations = [] + relations << where(id: topic.project_topics.select(:project_id)) if topic + relations << where(id: acts_as_taggable_on_topic.taggings.select(:taggable_id)) if acts_as_taggable_on_topic + + Project.from_union(relations) + end + enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } chronic_duration_attr :build_timeout_human_readable, :build_timeout, @@ -638,7 +672,7 @@ def self.integration_association_name(name) mount_uploader :bfg_object_map, AttachmentUploader def self.with_api_entity_associations - preload(:project_feature, :route, :topics, :group, :timelogs, namespace: [:route, :owner]) + preload(:project_feature, :route, :topics, :topics_acts_as_taggable, :group, :timelogs, namespace: [:route, :owner]) end def self.with_web_entity_associations @@ -2669,8 +2703,32 @@ def group_runners_enabled? ci_cd_settings.group_runners_enabled? end + def topic_list + self.topics.map(&:name) + end + private + def save_topics + return if @topic_list.nil? + + @topic_list = @topic_list.split(',') if @topic_list.instance_of?(String) + @topic_list = @topic_list.map(&:strip).uniq.reject(&:empty?) + + if @topic_list != self.topic_list || self.topics_acts_as_taggable.any? + self.topics_new.delete_all + self.topics = @topic_list.map { |topic| Projects::Topic.find_or_create_by(name: topic) } + + # Remove old topics (ActsAsTaggableOn::Tag) + # Required during the `ActsAsTaggableOn::Tag -> Topic` migration + # TODO: remove in the further process of the migration + # https://gitlab.com/gitlab-org/gitlab/-/issues/335946 + self.topic_taggings.clear + end + + @topic_list = nil + end + def find_integration(integrations, name) integrations.find { _1.to_param == name } end diff --git a/app/models/projects/project_topic.rb b/app/models/projects/project_topic.rb new file mode 100644 index 0000000000000000000000000000000000000000..d4b456ef48285a53870abb0e3d31975ecaf33ee4 --- /dev/null +++ b/app/models/projects/project_topic.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Projects + class ProjectTopic < ApplicationRecord + belongs_to :project + belongs_to :topic + end +end diff --git a/app/models/projects/topic.rb b/app/models/projects/topic.rb new file mode 100644 index 0000000000000000000000000000000000000000..a17aa550edb7637af0a1725c72a844f6fd51e4c6 --- /dev/null +++ b/app/models/projects/topic.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Projects + class Topic < ApplicationRecord + validates :name, presence: true, uniqueness: true, length: { maximum: 255 } + + has_many :project_topics, class_name: 'Projects::ProjectTopic' + has_many :projects, through: :project_topics + end +end diff --git a/db/migrate/20210729081351_create_topics.rb b/db/migrate/20210729081351_create_topics.rb new file mode 100644 index 0000000000000000000000000000000000000000..c6fdc6bb98a8cfedebf2f17870d1b7977fcf7c79 --- /dev/null +++ b/db/migrate/20210729081351_create_topics.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateTopics < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + def change + create_table_with_constraints :topics do |t| + t.text :name, null: false + t.text_limit :name, 255 + + t.index :name, unique: true + + t.timestamps_with_timezone + end + end +end diff --git a/db/migrate/20210729081739_create_project_topics.rb b/db/migrate/20210729081739_create_project_topics.rb new file mode 100644 index 0000000000000000000000000000000000000000..cbb8842f6536328904870c0c67230dcf9041a49d --- /dev/null +++ b/db/migrate/20210729081739_create_project_topics.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateProjectTopics < ActiveRecord::Migration[6.1] + def change + create_table :project_topics do |t| + t.bigint :project_id, null: false + t.bigint :topic_id, null: false + + t.index :project_id + t.index :topic_id + t.index [:project_id, :topic_id], unique: true + + t.timestamps_with_timezone + end + end +end diff --git a/db/migrate/20210729125641_add_foreign_key_to_project_on_project_topic.rb b/db/migrate/20210729125641_add_foreign_key_to_project_on_project_topic.rb new file mode 100644 index 0000000000000000000000000000000000000000..27cf5c60cf0e73a4f547c6e1e1bfaf82996aa24c --- /dev/null +++ b/db/migrate/20210729125641_add_foreign_key_to_project_on_project_topic.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddForeignKeyToProjectOnProjectTopic < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :project_topics, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :project_topics, column: :project_id + end + end +end diff --git a/db/migrate/20210729125659_add_foreign_key_to_topic_on_project_topic.rb b/db/migrate/20210729125659_add_foreign_key_to_topic_on_project_topic.rb new file mode 100644 index 0000000000000000000000000000000000000000..1ada08dca1a2d39ff7d029c8dee9de20407fd6e7 --- /dev/null +++ b/db/migrate/20210729125659_add_foreign_key_to_topic_on_project_topic.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddForeignKeyToTopicOnProjectTopic < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :project_topics, :topics, column: :topic_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :project_topics, column: :topic_id + end + end +end diff --git a/db/post_migrate/20210730104800_schedule_extract_project_topics_into_separate_table.rb b/db/post_migrate/20210730104800_schedule_extract_project_topics_into_separate_table.rb new file mode 100644 index 0000000000000000000000000000000000000000..bb498f89015b3e5221083c64f2a5d066b22ac2e7 --- /dev/null +++ b/db/post_migrate/20210730104800_schedule_extract_project_topics_into_separate_table.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class ScheduleExtractProjectTopicsIntoSeparateTable < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 1_000 + DELAY_INTERVAL = 2.minutes + MIGRATION = 'ExtractProjectTopicsIntoSeparateTable' + INDEX_NAME = 'tmp_index_taggings_on_id_where_taggable_type_project' + INDEX_CONDITION = "taggable_type = 'Project'" + + disable_ddl_transaction! + + class Tagging < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'taggings' + end + + def up + # this index is used in 20210730104800_schedule_extract_project_topics_into_separate_table + add_concurrent_index :taggings, :id, where: INDEX_CONDITION, name: INDEX_NAME + + queue_background_migration_jobs_by_range_at_intervals( + Tagging.where(taggable_type: 'Project'), + MIGRATION, + DELAY_INTERVAL, + batch_size: BATCH_SIZE, + track_jobs: true + ) + end + + def down + remove_concurrent_index_by_name :taggings, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210729081351 b/db/schema_migrations/20210729081351 new file mode 100644 index 0000000000000000000000000000000000000000..1f3974574438f3ca8431e541c400ed9bc2cb0371 --- /dev/null +++ b/db/schema_migrations/20210729081351 @@ -0,0 +1 @@ +17463867a8c14981386256dc90169fb879e1921d65eccca53eae576d49fba49d \ No newline at end of file diff --git a/db/schema_migrations/20210729081739 b/db/schema_migrations/20210729081739 new file mode 100644 index 0000000000000000000000000000000000000000..2215c7d7e666ff5a8ade66b0815edc68efb14d6f --- /dev/null +++ b/db/schema_migrations/20210729081739 @@ -0,0 +1 @@ +af7963d27bda6ef85fb5b5a06ecf1de14f21829eecdaf13e763aa9a6ffc2e83c \ No newline at end of file diff --git a/db/schema_migrations/20210729125641 b/db/schema_migrations/20210729125641 new file mode 100644 index 0000000000000000000000000000000000000000..e5ee4127656c45d7565eb1c4d26a71e824d74de0 --- /dev/null +++ b/db/schema_migrations/20210729125641 @@ -0,0 +1 @@ +b7bc495d010e0640b1145ca55f47696047fd4360d2dfc9a3da7941ab62840132 \ No newline at end of file diff --git a/db/schema_migrations/20210729125659 b/db/schema_migrations/20210729125659 new file mode 100644 index 0000000000000000000000000000000000000000..64c8cd0aeef876a9982bd7ed7879c305b99ff1a5 --- /dev/null +++ b/db/schema_migrations/20210729125659 @@ -0,0 +1 @@ +5826e87b2ce13d4951e9b8e774c87c29c6e0a0954a85d60ec68155f2c5cf3ccc \ No newline at end of file diff --git a/db/schema_migrations/20210730104800 b/db/schema_migrations/20210730104800 new file mode 100644 index 0000000000000000000000000000000000000000..d8e2986e9465b037ed5c9ee24dde100519bb12b2 --- /dev/null +++ b/db/schema_migrations/20210730104800 @@ -0,0 +1 @@ +7764c058665015707aff6e25ccbf60d4a329c67c16106b2ef523862ef82298b7 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index eee73636eb1565947dc05005924a6f159ec1156d..c73b585442a8c8c6bfbe7d6a6cd575d99d77425a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17333,6 +17333,23 @@ CREATE SEQUENCE project_statistics_id_seq ALTER SEQUENCE project_statistics_id_seq OWNED BY project_statistics.id; +CREATE TABLE project_topics ( + id bigint NOT NULL, + project_id bigint NOT NULL, + topic_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + +CREATE SEQUENCE project_topics_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE project_topics_id_seq OWNED BY project_topics.id; + CREATE TABLE project_tracing_settings ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -18789,6 +18806,23 @@ CREATE SEQUENCE token_with_ivs_id_seq ALTER SEQUENCE token_with_ivs_id_seq OWNED BY token_with_ivs.id; +CREATE TABLE topics ( + id bigint NOT NULL, + name text NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_7a90d4c757 CHECK ((char_length(name) <= 255)) +); + +CREATE SEQUENCE topics_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE topics_id_seq OWNED BY topics.id; + CREATE TABLE trending_projects ( id integer NOT NULL, project_id integer NOT NULL @@ -20727,6 +20761,8 @@ ALTER TABLE ONLY project_security_settings ALTER COLUMN project_id SET DEFAULT n ALTER TABLE ONLY project_statistics ALTER COLUMN id SET DEFAULT nextval('project_statistics_id_seq'::regclass); +ALTER TABLE ONLY project_topics ALTER COLUMN id SET DEFAULT nextval('project_topics_id_seq'::regclass); + ALTER TABLE ONLY project_tracing_settings ALTER COLUMN id SET DEFAULT nextval('project_tracing_settings_id_seq'::regclass); ALTER TABLE ONLY projects ALTER COLUMN id SET DEFAULT nextval('projects_id_seq'::regclass); @@ -20855,6 +20891,8 @@ ALTER TABLE ONLY todos ALTER COLUMN id SET DEFAULT nextval('todos_id_seq'::regcl ALTER TABLE ONLY token_with_ivs ALTER COLUMN id SET DEFAULT nextval('token_with_ivs_id_seq'::regclass); +ALTER TABLE ONLY topics ALTER COLUMN id SET DEFAULT nextval('topics_id_seq'::regclass); + ALTER TABLE ONLY trending_projects ALTER COLUMN id SET DEFAULT nextval('trending_projects_id_seq'::regclass); ALTER TABLE ONLY u2f_registrations ALTER COLUMN id SET DEFAULT nextval('u2f_registrations_id_seq'::regclass); @@ -22356,6 +22394,9 @@ ALTER TABLE ONLY project_settings ALTER TABLE ONLY project_statistics ADD CONSTRAINT project_statistics_pkey PRIMARY KEY (id); +ALTER TABLE ONLY project_topics + ADD CONSTRAINT project_topics_pkey PRIMARY KEY (id); + ALTER TABLE ONLY project_tracing_settings ADD CONSTRAINT project_tracing_settings_pkey PRIMARY KEY (id); @@ -22569,6 +22610,9 @@ ALTER TABLE ONLY todos ALTER TABLE ONLY token_with_ivs ADD CONSTRAINT token_with_ivs_pkey PRIMARY KEY (id); +ALTER TABLE ONLY topics + ADD CONSTRAINT topics_pkey PRIMARY KEY (id); + ALTER TABLE ONLY trending_projects ADD CONSTRAINT trending_projects_pkey PRIMARY KEY (id); @@ -24955,6 +24999,12 @@ CREATE INDEX index_project_statistics_on_storage_size_and_project_id ON project_ CREATE INDEX index_project_statistics_on_wiki_size_and_project_id ON project_statistics USING btree (wiki_size, project_id); +CREATE INDEX index_project_topics_on_project_id ON project_topics USING btree (project_id); + +CREATE UNIQUE INDEX index_project_topics_on_project_id_and_topic_id ON project_topics USING btree (project_id, topic_id); + +CREATE INDEX index_project_topics_on_topic_id ON project_topics USING btree (topic_id); + CREATE UNIQUE INDEX index_project_tracing_settings_on_project_id ON project_tracing_settings USING btree (project_id); CREATE INDEX index_projects_aimed_for_deletion ON projects USING btree (marked_for_deletion_at) WHERE ((marked_for_deletion_at IS NOT NULL) AND (pending_delete = false)); @@ -25459,6 +25509,8 @@ CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_plaintext_token ON token_with CREATE UNIQUE INDEX index_token_with_ivs_on_hashed_token ON token_with_ivs USING btree (hashed_token); +CREATE UNIQUE INDEX index_topics_on_name ON topics USING btree (name); + CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); CREATE INDEX index_u2f_registrations_on_key_handle ON u2f_registrations USING btree (key_handle); @@ -25793,6 +25845,8 @@ CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_root_namespaces ON na CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING btree (id) WHERE (state <> 2); +CREATE INDEX tmp_index_taggings_on_id_where_taggable_type_project ON taggings USING btree (id) WHERE ((taggable_type)::text = 'Project'::text); + CREATE UNIQUE INDEX uniq_pkgs_deb_grp_architectures_on_distribution_id_and_name ON packages_debian_group_architectures USING btree (distribution_id, name); CREATE UNIQUE INDEX uniq_pkgs_deb_grp_components_on_distribution_id_and_name ON packages_debian_group_components USING btree (distribution_id, name); @@ -26281,6 +26335,9 @@ ALTER TABLE ONLY ci_group_variables ALTER TABLE ONLY namespaces ADD CONSTRAINT fk_3448c97865 FOREIGN KEY (push_rule_id) REFERENCES push_rules(id) ON DELETE SET NULL; +ALTER TABLE ONLY project_topics + ADD CONSTRAINT fk_34af9ab07a FOREIGN KEY (topic_id) REFERENCES topics(id) ON DELETE CASCADE; + ALTER TABLE ONLY in_product_marketing_emails ADD CONSTRAINT fk_35c9101b63 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -26797,6 +26854,9 @@ ALTER TABLE ONLY label_links ALTER TABLE ONLY project_group_links ADD CONSTRAINT fk_daa8cee94c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY project_topics + ADD CONSTRAINT fk_db13576296 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY security_scans ADD CONSTRAINT fk_dbc89265b9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb index 01d71dc506338b76919b6984828109fba5c08c2e..847a45090c854bb0e43d62a13f65897627f88d49 100644 --- a/ee/lib/gitlab/elastic/search_results.rb +++ b/ee/lib/gitlab/elastic/search_results.rb @@ -28,7 +28,7 @@ def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE, preload_method: nil) case scope when 'projects' - eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics]) + eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics, :topics_acts_as_taggable]) when 'issues' eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: []) when 'merge_requests' diff --git a/lib/api/entities/basic_project_details.rb b/lib/api/entities/basic_project_details.rb index 0b231906ccd8d7e572d5e50cfd543c273cf4531f..49d20a309fc2f7b300d6c92f7da7827dc8a398b2 100644 --- a/lib/api/entities/basic_project_details.rb +++ b/lib/api/entities/basic_project_details.rb @@ -43,7 +43,7 @@ def self.preload_relation(projects_relation, options = {}) # N+1 is solved then by using `subject.topics.map(&:name)` # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555 projects_relation.preload(:project_feature, :route) - .preload(:import_state, :topics) + .preload(:import_state, :topics, :topics_acts_as_taggable) .preload(:auto_devops) .preload(namespace: [:route, :owner]) end diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 890b42ed8c8cc84e9eff18523edbd40d434cbc34..156870f0e1a4352d5e189a09609bf4f8fdf638ef 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -140,7 +140,7 @@ def self.preload_relation(projects_relation, options = {}) .preload(project_group_links: { group: :route }, fork_network: :root_project, fork_network_member: :forked_from_project, - forked_from_project: [:route, :topics, :group, :project_feature, namespace: [:route, :owner]]) + forked_from_project: [:route, :topics, :topics_acts_as_taggable, :group, :project_feature, namespace: [:route, :owner]]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb b/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb new file mode 100644 index 0000000000000000000000000000000000000000..15ce038321cefde95fce0350ab047a67b18beebf --- /dev/null +++ b/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # The class to extract the project topics into a separate `topics` table + class ExtractProjectTopicsIntoSeparateTable + # Temporary AR table for tags + class Tag < ActiveRecord::Base + self.table_name = 'tags' + end + + # Temporary AR table for taggings + class Tagging < ActiveRecord::Base + self.table_name = 'taggings' + belongs_to :tag + end + + # Temporary AR table for topics + class Topic < ActiveRecord::Base + self.table_name = 'topics' + end + + # Temporary AR table for project topics + class ProjectTopic < ActiveRecord::Base + self.table_name = 'project_topics' + belongs_to :topic + end + + def perform(start_id, stop_id) + Tagging.includes(:tag).where(taggable_type: 'Project', id: start_id..stop_id).each do |tagging| + topic = Topic.find_or_create_by(name: tagging.tag.name) + project_topic = ProjectTopic.find_or_create_by(project_id: tagging.taggable_id, topic: topic) + + tagging.delete if project_topic.persisted? + end + + mark_job_as_succeeded(start_id, stop_id) + end + + private + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + self.class.name.demodulize, + arguments + ) + end + end + end +end diff --git a/spec/factories/project_topics.rb b/spec/factories/project_topics.rb new file mode 100644 index 0000000000000000000000000000000000000000..60f5357d129e6c46d19bb9d273ac7a6cc19b7f18 --- /dev/null +++ b/spec/factories/project_topics.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :project_topic, class: 'Projects::ProjectTopic' do + association :project, factory: :project + association :topic, factory: :topic + end +end diff --git a/spec/factories/topics.rb b/spec/factories/topics.rb new file mode 100644 index 0000000000000000000000000000000000000000..e77441d9eae78617d5153d0900a438842dfee66b --- /dev/null +++ b/spec/factories/topics.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :topic, class: 'Projects::Topic' do + name { generate(:name) } + end +end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 21b5b2f6130c3db75b60a554352904f0062e65f9..1fdc05ca2d247331cf23eb88fec386efe9cbae15 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -135,6 +135,7 @@ describe 'filter by tags (deprecated)' do before do + public_project.reload public_project.topic_list = 'foo' public_project.save! end @@ -146,6 +147,7 @@ describe 'filter by topics' do before do + public_project.reload public_project.topic_list = 'foo, bar' public_project.save! end diff --git a/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d8f8fda3e579ec913d88207185729627aed6fccc --- /dev/null +++ b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::ExtractProjectTopicsIntoSeparateTable, schema: 20210730104800 do + it 'correctly extracts project topics into separate table' do + namespaces = table(:namespaces) + projects = table(:projects) + taggings = table(:taggings) + tags = table(:tags) + project_topics = table(:project_topics) + topics = table(:topics) + + namespace = namespaces.create!(name: 'foo', path: 'foo') + project = projects.create!(namespace_id: namespace.id) + tag_1 = tags.create!(name: 'Topic1') + tag_2 = tags.create!(name: 'Topic2') + tag_3 = tags.create!(name: 'Topic3') + topic_3 = topics.create!(name: 'Topic3') + tagging_1 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_1.id) + tagging_2 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_2.id) + other_tagging = taggings.create!(taggable_type: 'Other', taggable_id: project.id, context: 'topics', tag_id: tag_1.id) + tagging_3 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_3.id) + + subject.perform(tagging_1.id, tagging_3.id) + + # Tagging records + expect { tagging_1.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { tagging_2.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { other_tagging.reload }.not_to raise_error(ActiveRecord::RecordNotFound) + expect { tagging_3.reload }.to raise_error(ActiveRecord::RecordNotFound) + + # Topic records + topic_1 = topics.find_by(name: 'Topic1') + topic_2 = topics.find_by(name: 'Topic2') + expect(topics.all).to contain_exactly(topic_1, topic_2, topic_3) + + # ProjectTopic records + expect(project_topics.all.map(&:topic_id)).to contain_exactly(topic_1.id, topic_2.id, topic_3.id) + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 2b7138a7a10dcd9d7d7c56b3a599938d4238cd1b..d83a00589b125c4cc2a2e17c74a629d7ebb9e3c7 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -354,6 +354,8 @@ project: - taggings - base_tags - topic_taggings +- topics_acts_as_taggable +- project_topics - topics - chat_services - cluster diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d8f3a63d221137a1c239bcbb41a0454ab1165a64..71756cb25521ecbd63cc4f12502d2d72351552e8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7131,15 +7131,96 @@ def subject end describe 'topics' do - let_it_be(:project) { create(:project, topic_list: 'topic1, topic2, topic3') } + let_it_be(:project) { create(:project, name: 'topic-project', topic_list: 'topic1, topic2, topic3') } it 'topic_list returns correct string array' do - expect(project.topic_list).to match_array(%w[topic1 topic2 topic3]) + expect(project.topic_list).to eq(%w[topic1 topic2 topic3]) end - it 'topics returns correct tag records' do - expect(project.topics.first.class.name).to eq('ActsAsTaggableOn::Tag') - expect(project.topics.map(&:name)).to match_array(%w[topic1 topic2 topic3]) + it 'topics returns correct topic records' do + expect(project.topics.first.class.name).to eq('Projects::Topic') + expect(project.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end + + context 'topic_list=' do + using RSpec::Parameterized::TableSyntax + + where(:topic_list, :expected_result) do + ['topicA', 'topicB'] | %w[topicA topicB] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands + ['topicB', 'topicA'] | %w[topicB topicA] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands + [' topicC ', ' topicD '] | %w[topicC topicD] + ['topicE', 'topicF', 'topicE'] | %w[topicE topicF] # rubocop:disable Style/WordArray + ['topicE ', 'topicF', ' topicE'] | %w[topicE topicF] + 'topicA, topicB' | %w[topicA topicB] + 'topicB, topicA' | %w[topicB topicA] + ' topicC , topicD ' | %w[topicC topicD] + 'topicE, topicF, topicE' | %w[topicE topicF] + 'topicE , topicF, topicE' | %w[topicE topicF] + end + + with_them do + it 'set topics' do + project.topic_list = topic_list + project.save! + + expect(project.topics.map(&:name)).to eq(expected_result) + end + end + + it 'set topics if only the order is changed' do + project.topic_list = 'topicA, topicB' + project.save! + + expect(project.reload.topics.map(&:name)).to eq(%w[topicA topicB]) + + project.topic_list = 'topicB, topicA' + project.save! + + expect(project.reload.topics.map(&:name)).to eq(%w[topicB topicA]) + end + + it 'does not persist topics before project is saved' do + project.topic_list = 'topicA, topicB' + + expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end + + it 'does not update topics if project is not valid' do + project.name = nil + project.topic_list = 'topicA, topicB' + + expect(project.save).to be_falsy + expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end + end + + context 'during ExtractProjectTopicsIntoSeparateTable migration' do + before do + topic_a = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicA') + topic_b = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicB') + + project.reload.topics_acts_as_taggable = [topic_a, topic_b] + project.save! + project.reload + end + + it 'topic_list returns correct string array' do + expect(project.topic_list).to eq(%w[topicA topicB topic1 topic2 topic3]) + end + + it 'topics returns correct topic records' do + expect(project.topics.map(&:class)).to eq([ActsAsTaggableOn::Tag, ActsAsTaggableOn::Tag, Projects::Topic, Projects::Topic, Projects::Topic]) + expect(project.topics.map(&:name)).to eq(%w[topicA topicB topic1 topic2 topic3]) + end + + it 'topic_list= sets new topics and removes old topics' do + project.topic_list = 'new-topic1, new-topic2' + project.save! + project.reload + + expect(project.topics.map(&:class)).to eq([Projects::Topic, Projects::Topic]) + expect(project.topics.map(&:name)).to eq(%w[new-topic1 new-topic2]) + end end end diff --git a/spec/models/projects/project_topic_spec.rb b/spec/models/projects/project_topic_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c7a989040c705de5b9503f0fdfee85144452ea54 --- /dev/null +++ b/spec/models/projects/project_topic_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ProjectTopic do + let_it_be(:project_topic, reload: true) { create(:project_topic) } + + subject { project_topic } + + it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:topic) } + end +end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..409dc9327090a7672cd59888990e220c773666e5 --- /dev/null +++ b/spec/models/projects/topic_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Topic do + let_it_be(:topic, reload: true) { create(:topic) } + + subject { topic } + + it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to have_many(:project_topics) } + it { is_expected.to have_many(:projects) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index c392856312510733b333c6fa37cd1e50af26742f..cd8a56e59c435816a13f1ca2040549d26bb87879 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -86,7 +86,7 @@ subject(:project) { create_project(user, opts) } context "with 'topics' parameter" do - let(:opts) { { topics: 'topics' } } + let(:opts) { { name: 'topic-project', topics: 'topics' } } it 'keeps them as specified' do expect(project.topic_list).to eq(%w[topics]) @@ -94,7 +94,7 @@ end context "with 'topic_list' parameter" do - let(:opts) { { topic_list: 'topic_list' } } + let(:opts) { { name: 'topic-project', topic_list: 'topic_list' } } it 'keeps them as specified' do expect(project.topic_list).to eq(%w[topic_list]) @@ -102,7 +102,7 @@ end context "with 'tag_list' parameter (deprecated)" do - let(:opts) { { tag_list: 'tag_list' } } + let(:opts) { { name: 'topic-project', tag_list: 'tag_list' } } it 'keeps them as specified' do expect(project.topic_list).to eq(%w[tag_list])