From 56f130d9f2c0f3aa3787c4fea839e8dfcf22db13 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Tue, 21 Apr 2026 19:34:37 +0200 Subject: [PATCH] [#71645] Convert instance to semantic identifiers --- .../identifier_settings_form_component.rb | 13 +- app/models/projects/semantic_identifier.rb | 24 +++ .../work_package/semantic_identifier.rb | 10 + .../convert_project_to_semantic_service.rb | 118 +++++++++++ .../project_identifiers/identifier_autofix.rb | 2 + .../problematic_identifiers.rb | 30 +-- .../pending_projects_finder.rb | 60 ++++++ .../convert_instance_to_semantic_ids_job.rb | 9 +- .../convert_project_to_semantic_ids_job.rb | 43 ++++ .../finish_semantic_conversion_job.rb | 81 ++++++++ ...identifier_settings_form_component_spec.rb | 16 ++ .../work_package/semantic_identifier_spec.rb | 29 +++ ...onvert_project_to_semantic_service_spec.rb | 183 ++++++++++++++++++ .../problematic_identifiers_spec.rb | 34 +++- .../pending_projects_finder_spec.rb | 83 ++++++++ ...nvert_instance_to_semantic_ids_job_spec.rb | 66 +++++++ ...onvert_project_to_semantic_ids_job_spec.rb | 45 +++++ .../finish_semantic_conversion_job_spec.rb | 128 ++++++++++++ 18 files changed, 954 insertions(+), 20 deletions(-) create mode 100644 app/services/project_identifiers/convert_project_to_semantic_service.rb create mode 100644 app/services/project_identifiers/pending_projects_finder.rb create mode 100644 app/workers/project_identifiers/convert_project_to_semantic_ids_job.rb create mode 100644 app/workers/project_identifiers/finish_semantic_conversion_job.rb create mode 100644 spec/services/project_identifiers/convert_project_to_semantic_service_spec.rb create mode 100644 spec/services/project_identifiers/pending_projects_finder_spec.rb create mode 100644 spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb create mode 100644 spec/workers/project_identifiers/convert_project_to_semantic_ids_job_spec.rb create mode 100644 spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb diff --git a/app/components/work_packages/admin/settings/identifier_settings_form_component.rb b/app/components/work_packages/admin/settings/identifier_settings_form_component.rb index 5ddad8f2df5..856d38173f7 100644 --- a/app/components/work_packages/admin/settings/identifier_settings_form_component.rb +++ b/app/components/work_packages/admin/settings/identifier_settings_form_component.rb @@ -99,11 +99,22 @@ module WorkPackages def radio_button_options if change_in_progress? - { button_options: { disabled: true } } + { + values: identifier_values(checked: nil), + button_options: { disabled: true } + } + elsif completed? + { values: identifier_values(checked: Setting[:work_packages_identifier]) } else { button_options: { data: { action: "change->admin--work-packages-identifier#handleChange" } } } end end + + def identifier_values(checked:) + Setting::WorkPackageIdentifier::ALLOWED_VALUES.map do |v| + { name: v, value: v, checked: v == checked } + end + end end end end diff --git a/app/models/projects/semantic_identifier.rb b/app/models/projects/semantic_identifier.rb index 100f04de7f3..63ad6ee59dd 100644 --- a/app/models/projects/semantic_identifier.rb +++ b/app/models/projects/semantic_identifier.rb @@ -51,6 +51,23 @@ module Projects::SemanticIdentifier [seq, "#{identifier}-#{seq}"] end + # Returns the most-recent slug from FriendlyId history that is a valid semantic + # identifier and is not currently held by another project, or nil if none exists. + # Used by the backfill job to restore a prior semantic identifier instead of + # generating a fresh one, so existing WP identifiers and aliases remain correct. + def previous_semantic_identifier + candidates = previous_semantic_identifier_candidates + return nil if candidates.empty? + + taken = self.class + .where.not(id:) + .where("LOWER(identifier) IN (?)", candidates.map(&:downcase)) + .pluck(:identifier) + .to_set(&:downcase) + + candidates.find { |slug| taken.exclude?(slug.downcase) } + end + # Called after this project's identifier is renamed. Atomically: # 1. Appends new-prefix aliases for every WP that ever carried an old-prefix alias. # 2. Updates identifier on resident WPs to the new prefix. @@ -67,6 +84,13 @@ module Projects::SemanticIdentifier private + def previous_semantic_identifier_candidates + slugs + .order(created_at: :desc) + .pluck(:slug) + .select { |slug| ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers.valid_format?(slug) } + end + # For every alias row whose identifier starts with the old prefix, inserts a # corresponding row with the new prefix. This covers WPs still in the project # as well as any that have moved out but still carry old-prefix alias rows. diff --git a/app/models/work_package/semantic_identifier.rb b/app/models/work_package/semantic_identifier.rb index d3feb920782..01d3cd05ec2 100644 --- a/app/models/work_package/semantic_identifier.rb +++ b/app/models/work_package/semantic_identifier.rb @@ -50,6 +50,16 @@ module WorkPackage::SemanticIdentifier inverse_of: :work_package, dependent: :delete_all + scope :semantically_sequenced, -> { where.not(sequence_number: nil) } + scope :unsequenced, -> { where(sequence_number: nil) } + scope :non_semantic_of, ->(project) { + semantically_sequenced.where("identifier IS DISTINCT FROM (? || '-' || sequence_number::text)", project.identifier) + } + scope :non_semantic, -> { + joins(:project).semantically_sequenced + .where("work_packages.identifier IS DISTINCT FROM projects.identifier || '-' || work_packages.sequence_number::text") + } + after_create :allocate_and_register_semantic_id, if: -> { Setting::WorkPackageIdentifier.semantic? } end diff --git a/app/services/project_identifiers/convert_project_to_semantic_service.rb b/app/services/project_identifiers/convert_project_to_semantic_service.rb new file mode 100644 index 00000000000..14d0b126488 --- /dev/null +++ b/app/services/project_identifiers/convert_project_to_semantic_service.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module ProjectIdentifiers + # Brings a single project fully up to date for semantic identifier mode: + # + # 1. Fixes the project identifier if it is not in valid semantic format. + # 2. Rewrites stale WP identifiers whose prefix no longer matches the project. + # 3. Assigns sequence numbers to WPs that have none yet. + # 4. Seeds the alias table for all historical project identifier prefixes. + class ConvertProjectToSemanticService + def initialize(project) + @project = project + end + + def call + ApplicationRecord.transaction do + fix_identifier_if_needed + reset_stale_identifiers + backfill_missing_ids + seed_alias_table + end + end + + private + + attr_reader :project + + def fix_identifier_if_needed + # Pure format check — no DB queries. + return if ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers.valid_format?(project.identifier) + + # Serialize all concurrent identifier assignments with a transaction-level + # advisory lock. The lock is automatically released when the outer + # ApplicationRecord.transaction commits, so the next job waiting on it + # always reads a fully up-to-date exclusion set and can never generate a + # duplicate. Without this, parallel jobs can read the same exclusion set + # before any of them commits, then all pick the same candidate. + OpenProject::Mutex.with_advisory_lock( + Project, "semantic_identifier_generation", transaction: true + ) do + assign_semantic_identifier + end + end + + def assign_semantic_identifier + # Re-instantiate inside the lock so the exclusion set reflects all + # identifiers committed since this job started. + detector = ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers.new + generator = ProjectIdentifiers::IdentifierAutofix::ProjectIdentifierSuggestionGenerator + + # Prefer restoring the project's last known semantic identifier (from + # FriendlyId history) so that existing WP identifiers remain valid and + # aliases need no update. Fall back to generating a fresh suggestion. + new_identifier = project.previous_semantic_identifier || + generator.suggest_identifier(project.name, exclude: detector.exclusion_set) + + raise "Generated identifier is blank for project #{project.id}" if new_identifier.blank? + + project.identifier = new_identifier + # Bypass validation, because we're technically still in classic mode, so the model would be applying + # validation for classic identifiers. + project.save!(validate: false) + end + + def reset_stale_identifiers + # Fix WPs whose identifier does not exactly match the expected semantic identifier + # (caused by renames or cross-project moves in classic mode) + WorkPackage.where(project:).non_semantic_of(project).update_all(identifier: nil, sequence_number: nil) + end + + def backfill_missing_ids + WorkPackage.where(project:).unsequenced.find_each do |wp| + seq, identifier = project.allocate_wp_semantic_identifier! + wp.update_columns(sequence_number: seq, identifier:) + end + end + + def seed_alias_table + slug_prefixes = project.slugs.pluck(:slug) + return if slug_prefixes.empty? + + WorkPackage.where(project:).semantically_sequenced.in_batches do |batch| + alias_rows = batch.pluck(:id, :sequence_number) + .product(slug_prefixes) + .map { |(wp_id, seq), prefix| { identifier: "#{prefix}-#{seq}", work_package_id: wp_id } } + WorkPackageSemanticAlias.insert_all(alias_rows, unique_by: :identifier) if alias_rows.any? + end + end + end +end diff --git a/app/services/project_identifiers/identifier_autofix.rb b/app/services/project_identifiers/identifier_autofix.rb index eab2cf347a3..3481ca0e5a2 100644 --- a/app/services/project_identifiers/identifier_autofix.rb +++ b/app/services/project_identifiers/identifier_autofix.rb @@ -34,6 +34,8 @@ module ProjectIdentifiers GoodJob::Job .where(job_class: [ ProjectIdentifiers::ConvertInstanceToSemanticIdsJob.name, + ProjectIdentifiers::ConvertProjectToSemanticIdsJob.name, + ProjectIdentifiers::FinishSemanticConversionJob.name, ProjectIdentifiers::RevertInstanceToClassicIdsJob.name ]) .exists?(finished_at: nil) diff --git a/app/services/project_identifiers/identifier_autofix/problematic_identifiers.rb b/app/services/project_identifiers/identifier_autofix/problematic_identifiers.rb index a4d0b765f3a..b00aebaaa2d 100644 --- a/app/services/project_identifiers/identifier_autofix/problematic_identifiers.rb +++ b/app/services/project_identifiers/identifier_autofix/problematic_identifiers.rb @@ -63,6 +63,23 @@ module ProjectIdentifiers [:not_fully_uppercased, ->(id, _) { id != id.upcase }] ].freeze + # Returns a symbol classifying why the identifier violates the expected format, + # or nil if the identifier is format-valid. Pure in-memory check — no DB queries. + def self.format_error_reason(identifier) + FORMAT_RULES.each do |reason, check| + return reason if check.call(identifier, max_identifier_length) + end + nil + end + + def self.valid_format?(identifier) + format_error_reason(identifier).nil? + end + + def self.max_identifier_length + ProjectIdentifierSuggestionGenerator::IDENTIFIER_LENGTH[:max] + end + def scope @scope ||= exceeds_max_length .or(contains_non_alphanumeric) @@ -75,7 +92,7 @@ module ProjectIdentifiers # Returns a symbol classifying why the identifier is problematic. # Must handle all identifiers matched by #scope. def error_reason(identifier) - format_error_reason(identifier) || collision_error_reason(identifier) || :unknown + self.class.format_error_reason(identifier) || collision_error_reason(identifier) || :unknown end # Returns a Set of identifiers that must not be suggested for new assignments. @@ -95,20 +112,11 @@ module ProjectIdentifiers .to_set end - def exceeds_max_length = Project.where("length(identifier) > ?", max_identifier_length) + def exceeds_max_length = Project.where("length(identifier) > ?", self.class.max_identifier_length) def contains_non_alphanumeric = Project.where("identifier ~ ?", "[^a-zA-Z0-9_]") def starts_with_digit = Project.where("identifier ~ ?", "^[0-9]") def not_fully_uppercased = Project.where("identifier != UPPER(identifier)") - def max_identifier_length = ProjectIdentifierSuggestionGenerator::IDENTIFIER_LENGTH[:max] - - def format_error_reason(identifier) - FORMAT_RULES.each do |reason, check| - return reason if check.call(identifier, max_identifier_length) - end - nil - end - def collision_error_reason(identifier) if in_use_identifiers.include?(identifier) :in_use diff --git a/app/services/project_identifiers/pending_projects_finder.rb b/app/services/project_identifiers/pending_projects_finder.rb new file mode 100644 index 00000000000..8e9835e4a8a --- /dev/null +++ b/app/services/project_identifiers/pending_projects_finder.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module ProjectIdentifiers + # Returns the set of project IDs that still need backfilling before the + # instance can be switched to semantic identifier mode. Three buckets: + # + # * projects whose identifier is not in valid semantic format + # * projects that have work packages with no sequence_number yet + # * projects that have work packages whose identifier doesn't match + # the current project prefix (stale due to renames or cross-project moves) + module PendingProjectsFinder + def self.project_ids + projects_with_bad_identifier | projects_with_unsequenced_wps | projects_with_stale_wps + end + + class << self + private + + def projects_with_bad_identifier + ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers.new.scope.ids.to_set + end + + def projects_with_unsequenced_wps + WorkPackage.unsequenced.distinct.pluck(:project_id).to_set + end + + def projects_with_stale_wps + WorkPackage.non_semantic.distinct.pluck(:project_id).to_set + end + end + end +end diff --git a/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb b/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb index 29d6613aadf..c10dac35812 100644 --- a/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb +++ b/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb @@ -32,6 +32,13 @@ class ProjectIdentifiers::ConvertInstanceToSemanticIdsJob < ApplicationJob include GoodJob::ActiveJobExtensions::Concurrency good_job_control_concurrency_with(total_limit: 1) + queue_with_priority :above_normal - def perform(*); end + def perform + GoodJob::Batch.enqueue(on_success: ProjectIdentifiers::FinishSemanticConversionJob) do + ProjectIdentifiers::PendingProjectsFinder.project_ids.each do |project_id| + ProjectIdentifiers::ConvertProjectToSemanticIdsJob.perform_later(project_id) + end + end + end end diff --git a/app/workers/project_identifiers/convert_project_to_semantic_ids_job.rb b/app/workers/project_identifiers/convert_project_to_semantic_ids_job.rb new file mode 100644 index 00000000000..b44ac1c1bd6 --- /dev/null +++ b/app/workers/project_identifiers/convert_project_to_semantic_ids_job.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class ProjectIdentifiers::ConvertProjectToSemanticIdsJob < ApplicationJob + include GoodJob::ActiveJobExtensions::Concurrency + + good_job_control_concurrency_with(perform_limit: 5) + queue_with_priority :above_normal + retry_on StandardError, wait: :polynomially_longer, attempts: 8 + discard_on ActiveRecord::RecordNotFound + + def perform(project_id) + project = Project.find(project_id) + ProjectIdentifiers::ConvertProjectToSemanticService.new(project).call + end +end diff --git a/app/workers/project_identifiers/finish_semantic_conversion_job.rb b/app/workers/project_identifiers/finish_semantic_conversion_job.rb new file mode 100644 index 00000000000..dc14f3c5868 --- /dev/null +++ b/app/workers/project_identifiers/finish_semantic_conversion_job.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +# GoodJob on_success callback invoked after a ConvertInstanceToSemanticIdsJob +# batch completes. Performs up to MAX_SWEEPS synchronous sweeps to catch any +# projects created or modified during the batch run, then enables semantic mode. +# If projects still remain after MAX_SWEEPS sweeps a RuntimeError is raised to +# abort the job (leaving the setting as classic); the instance is either under +# active load or there is a code bug. +class ProjectIdentifiers::FinishSemanticConversionJob < ApplicationJob + queue_with_priority :high + + MAX_SWEEPS = 5 + + def perform(*) + corrective_sweep + set_semantic_mode! + end + + private + + def set_semantic_mode! + result = Settings::UpdateService + .new(user: User.system) + .call(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) + + raise "[FinishSemanticConversionJob] Failed to enable semantic mode: #{result.message}" unless result.success? + end + + def corrective_sweep + MAX_SWEEPS.times do + remaining = pending_project_ids + return if remaining.empty? + + remaining.each do |project_id| + project = Project.find_by(id: project_id) + next unless project + + ProjectIdentifiers::ConvertProjectToSemanticService.new(project).call + end + end + + return if pending_project_ids.empty? + + message = "[FinishSemanticConversionJob] Giving up after #{MAX_SWEEPS} sweeps — " \ + "projects still remain pending. The instance may be under active load or there is a bug." + Rails.logger.warn message + raise message + end + + def pending_project_ids + ProjectIdentifiers::PendingProjectsFinder.project_ids + end +end diff --git a/spec/components/work_packages/admin/settings/identifier_settings_form_component_spec.rb b/spec/components/work_packages/admin/settings/identifier_settings_form_component_spec.rb index 654f77505ae..ad6210226b5 100644 --- a/spec/components/work_packages/admin/settings/identifier_settings_form_component_spec.rb +++ b/spec/components/work_packages/admin/settings/identifier_settings_form_component_spec.rb @@ -104,6 +104,22 @@ RSpec.describe WorkPackages::Admin::Settings::IdentifierSettingsFormComponent, t render_component(component) expect(ProjectIdentifiers::IdentifierAutofix::PreviewQuery).not_to have_received(:new) end + + context "with semantic setting", with_settings: { work_packages_identifier: "semantic" } do + it "shows semantic as selected" do + render_component(component) + expect(page).to have_field("Project-based semantic identifiers", checked: true) + expect(page).to have_field("Instance-wide numerical sequence (default)", checked: false) + end + end + + context "with classic setting", with_settings: { work_packages_identifier: "classic" } do + it "shows classic as selected" do + render_component(component) + expect(page).to have_field("Instance-wide numerical sequence (default)", checked: true) + expect(page).to have_field("Project-based semantic identifiers", checked: false) + end + end end context "when state is :edit" do diff --git a/spec/models/work_package/semantic_identifier_spec.rb b/spec/models/work_package/semantic_identifier_spec.rb index f00b8e769d0..23df72c15fc 100644 --- a/spec/models/work_package/semantic_identifier_spec.rb +++ b/spec/models/work_package/semantic_identifier_spec.rb @@ -40,6 +40,35 @@ RSpec.describe WorkPackage::SemanticIdentifier do work_package end + describe ".semantically_sequenced" do + it "includes work packages with a sequence number" do + expect(WorkPackage.semantically_sequenced).to include(work_package) + end + + it "excludes work packages without a sequence number" do + wp = create(:work_package, project:) + wp.update_columns(sequence_number: nil) + expect(WorkPackage.semantically_sequenced).not_to include(wp) + end + end + + describe ".non_semantic_of" do + it "excludes work packages whose identifier matches the expected semantic format" do + expect(WorkPackage.non_semantic_of(project)).not_to include(work_package) + end + + it "includes work packages with a stale identifier" do + work_package.update_columns(identifier: "OLDPROJ-1") + expect(WorkPackage.non_semantic_of(project)).to include(work_package) + end + + it "excludes work packages without a sequence number" do + wp = create(:work_package, project:) + wp.update_columns(sequence_number: nil, identifier: nil) + expect(WorkPackage.non_semantic_of(project)).not_to include(wp) + end + end + describe "after_create registration" do it "assigns a sequence number" do expect(work_package.reload.sequence_number).to eq(1) diff --git a/spec/services/project_identifiers/convert_project_to_semantic_service_spec.rb b/spec/services/project_identifiers/convert_project_to_semantic_service_spec.rb new file mode 100644 index 00000000000..6e237c64574 --- /dev/null +++ b/spec/services/project_identifiers/convert_project_to_semantic_service_spec.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe ProjectIdentifiers::ConvertProjectToSemanticService, + with_settings: { work_packages_identifier: "classic" } do + describe "#call" do + context "when the project has a prior valid semantic identifier in FriendlyId history" do + let!(:project) do + create(:project).tap do |p| + p.update_columns(identifier: "my-app", wp_sequence_counter: 0) + FriendlyId::Slug.create!(sluggable: p, slug: "MYAPP") + end + end + + before { described_class.new(project).call } + + it "restores the prior semantic identifier instead of generating a new one" do + expect(project.reload.identifier).to eq("MYAPP") + end + end + + context "when the prior semantic identifier is already taken by another project" do + let!(:other) { create(:project).tap { |p| p.update_columns(identifier: "MYAPP") } } + let!(:project) do + create(:project).tap do |p| + p.update_columns(identifier: "my-app", wp_sequence_counter: 0) + FriendlyId::Slug.create!(sluggable: p, slug: "MYAPP") + end + end + + before { described_class.new(project).call } + + it "falls back to a freshly generated semantic identifier" do + identifier = project.reload.identifier + expect(identifier).not_to eq("my-app") + expect(identifier).not_to eq("MYAPP") + expect(identifier).to match(/\A[A-Z][A-Z0-9_]{1,9}\z/) + end + end + + context "when the generated identifier is blank" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "my-app") } + end + + before do + allow(ProjectIdentifiers::IdentifierAutofix::ProjectIdentifierSuggestionGenerator) + .to receive(:suggest_identifier) + .and_return(nil) + end + + it "raises a RuntimeError" do + expect { described_class.new(project).call } + .to raise_error(RuntimeError, /Generated identifier is blank/) + end + end + + context "when the project has a problematic identifier" do + let!(:project) { create(:project, name: "My Project") } + let!(:wp) { create(:work_package, project:) } + + before { described_class.new(project).call } + + it "renames the project to a valid semantic identifier" do + expect(project.reload.identifier).to match(/\A[A-Z][A-Z0-9_]{1,9}\z/) + end + + it "backfills WPs using the new identifier" do + expect(wp.reload.identifier).to eq("#{project.reload.identifier}-1") + end + end + + context "when the project already has a valid identifier" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "MYAPP", wp_sequence_counter: 0) } + end + let!(:wp) { create(:work_package, project:) } + + before { described_class.new(project).call } + + it "leaves the identifier unchanged" do + expect(project.reload.identifier).to eq("MYAPP") + end + end + + context "when a project has work packages without sequence numbers" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "MYAPP", wp_sequence_counter: 0) } + end + let!(:wp1) { create(:work_package, project:) } + let!(:wp2) { create(:work_package, project:) } + + before { described_class.new(project).call } + + it "assigns sequence numbers in id (oldest-first) order" do + expect(wp1.reload.sequence_number).to eq(1) + expect(wp2.reload.sequence_number).to eq(2) + end + + it "sets identifier on each WP using the project identifier" do + expect(wp1.reload.identifier).to eq("MYAPP-1") + expect(wp2.reload.identifier).to eq("MYAPP-2") + end + end + + context "when a WP was moved in from another project (identifier and seq are stale)" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "DEST", wp_sequence_counter: 1) } + end + let!(:wp) { create(:work_package, project:).tap { |w| w.update_columns(sequence_number: 4, identifier: "SOURCE-1") } } + + before { described_class.new(project).call } + + it "rewrites the identifier to match the current project prefix" do + expect(wp.reload.identifier).to eq("DEST-2") + end + end + + context "when a moved-in WP has a sequence_number higher than the project counter (counter underflow)" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "DEST", wp_sequence_counter: 0) } + end + let!(:moved_wp) { create(:work_package, project:).tap { |w| w.update_columns(sequence_number: 5, identifier: "SOURCE-5") } } + let!(:new_wp) { create(:work_package, project:).tap { |w| w.update_columns(sequence_number: nil, identifier: nil) } } + + before { described_class.new(project).call } + + it "correctly assigns the new sequence numbers" do + expect(moved_wp.reload.sequence_number).to eq(1) + expect(new_wp.reload.sequence_number).to eq(2) + end + end + + context "when seeding the alias table" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "CURR", wp_sequence_counter: 0) } + end + let!(:wp) { create(:work_package, project:) } + + before do + FriendlyId::Slug.where(sluggable: project).delete_all + FriendlyId::Slug.create!(sluggable: project, slug: "OLD") + FriendlyId::Slug.create!(sluggable: project, slug: "CURR") + described_class.new(project).call + end + + it "inserts alias rows for all historical prefixes" do + seq = wp.reload.sequence_number + expect(WorkPackageSemanticAlias.where(work_package_id: wp.id).pluck(:identifier)) + .to contain_exactly("OLD-#{seq}", "CURR-#{seq}") + end + end + end +end diff --git a/spec/services/project_identifiers/identifier_autofix/problematic_identifiers_spec.rb b/spec/services/project_identifiers/identifier_autofix/problematic_identifiers_spec.rb index 9074fc53935..bb76e6473f3 100644 --- a/spec/services/project_identifiers/identifier_autofix/problematic_identifiers_spec.rb +++ b/spec/services/project_identifiers/identifier_autofix/problematic_identifiers_spec.rb @@ -81,31 +81,47 @@ RSpec.describe ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers do end end - describe "#error_reason" do + describe ".format_error_reason" do it "returns :too_long when identifier exceeds max length" do - expect(analysis.error_reason("averylongidentifier")).to eq(:too_long) + expect(described_class.format_error_reason("averylongidentifier")).to eq(:too_long) end it "returns :numerical when identifier is purely numeric" do - expect(analysis.error_reason("12345")).to eq(:numerical) + expect(described_class.format_error_reason("12345")).to eq(:numerical) end it "returns :starts_with_number when identifier begins with a digit" do - expect(analysis.error_reason("1abc")).to eq(:starts_with_number) + expect(described_class.format_error_reason("1abc")).to eq(:starts_with_number) end it "returns :special_characters when identifier has non-alphanumeric chars" do - expect(analysis.error_reason("ab-c")).to eq(:special_characters) + expect(described_class.format_error_reason("ab-c")).to eq(:special_characters) end it "returns :not_fully_uppercased when identifier is lowercase but otherwise valid" do - expect(analysis.error_reason("proj")).to eq(:not_fully_uppercased) + expect(described_class.format_error_reason("proj")).to eq(:not_fully_uppercased) end it "returns :too_long with priority over :special_characters" do - expect(analysis.error_reason("my-very-long-identifier")).to eq(:too_long) + expect(described_class.format_error_reason("my-very-long-identifier")).to eq(:too_long) end + it "returns nil for a valid identifier" do + expect(described_class.format_error_reason("VALID")).to be_nil + end + end + + describe ".valid_format?" do + it "returns true for a valid identifier" do + expect(described_class.valid_format?("VALID")).to be(true) + end + + it "returns false for an identifier with format errors" do + expect(described_class.valid_format?("ab-c")).to be(false) + end + end + + describe "#error_reason" do it "returns :in_use when identifier belongs to a non-problematic project" do create_project_with_raw_identifier(name: "Taken", identifier: "TAKEN") expect(analysis.error_reason("TAKEN")).to eq(:in_use) @@ -121,6 +137,10 @@ RSpec.describe ProjectIdentifiers::IdentifierAutofix::ProblematicIdentifiers do it "returns :unknown when no classification matches" do expect(analysis.error_reason("VALID")).to eq(:unknown) end + + it "delegates format checks to .format_error_reason" do + expect(analysis.error_reason("ab-c")).to eq(:special_characters) + end end describe "#exclusion_set" do diff --git a/spec/services/project_identifiers/pending_projects_finder_spec.rb b/spec/services/project_identifiers/pending_projects_finder_spec.rb new file mode 100644 index 00000000000..5410b786120 --- /dev/null +++ b/spec/services/project_identifiers/pending_projects_finder_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe ProjectIdentifiers::PendingProjectsFinder, + with_settings: { work_packages_identifier: "classic" } do + describe ".project_ids" do + context "when everything is clean" do + it "returns an empty set" do + expect(described_class.project_ids).to be_empty + end + end + + context "when a project has a non-semantic identifier" do + let!(:project) { create(:project, name: "My Project") } + + it "includes that project" do + expect(described_class.project_ids).to include(project.id) + end + end + + context "when a project has a valid identifier but work packages without sequence numbers" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "VALID1") } + end + let!(:wp) { create(:work_package, project:) } + + it "includes that project" do + expect(described_class.project_ids).to include(project.id) + end + end + + context "when a project has a valid identifier but no work packages needing backfill" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "VALID1", wp_sequence_counter: 1) } + end + let!(:wp) { create(:work_package, project:).tap { |w| w.update_columns(sequence_number: 1, identifier: "VALID1-1") } } + + it "does not include that project" do + expect(described_class.project_ids).not_to include(project.id) + end + end + + context "when a project has work packages with stale identifiers" do + let!(:project) do + create(:project).tap { |p| p.update_columns(identifier: "DEST", wp_sequence_counter: 1) } + end + let!(:wp) { create(:work_package, project:).tap { |w| w.update_columns(sequence_number: 1, identifier: "SOURCE-1") } } + + it "includes that project" do + expect(described_class.project_ids).to include(project.id) + end + end + end +end diff --git a/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb b/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb new file mode 100644 index 00000000000..ab43f67bd50 --- /dev/null +++ b/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe ProjectIdentifiers::ConvertInstanceToSemanticIdsJob, + with_good_job_batches: [ + ProjectIdentifiers::FinishSemanticConversionJob, + ProjectIdentifiers::ConvertProjectToSemanticIdsJob + ] do + subject(:job) { described_class.new } + + describe "#perform" do + context "when there are projects to convert" do + before { allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[1, 2]) } + + it "enqueues one ConvertProjectToSemanticIdsJob per pending project" do + job.perform + expect(GoodJob::Job.where(job_class: ProjectIdentifiers::ConvertProjectToSemanticIdsJob.name).count).to eq(2) + end + + it "sets FinishSemanticConversionJob as the on_success callback" do + allow(GoodJob::Batch).to receive(:enqueue).and_call_original + job.perform + expect(GoodJob::Batch).to have_received(:enqueue) + .with(hash_including(on_success: ProjectIdentifiers::FinishSemanticConversionJob)) + end + end + + context "when there are no projects to convert" do + before { allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set.new) } + + it "does not enqueue any per-project jobs" do + job.perform + expect(GoodJob::Job.where(job_class: ProjectIdentifiers::ConvertProjectToSemanticIdsJob.name)).not_to exist + end + end + end +end diff --git a/spec/workers/project_identifiers/convert_project_to_semantic_ids_job_spec.rb b/spec/workers/project_identifiers/convert_project_to_semantic_ids_job_spec.rb new file mode 100644 index 00000000000..591e41c6ead --- /dev/null +++ b/spec/workers/project_identifiers/convert_project_to_semantic_ids_job_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe ProjectIdentifiers::ConvertProjectToSemanticIdsJob do + describe "#perform" do + it "delegates to ConvertProjectToSemanticService" do + project = create(:project) + service = instance_double(ProjectIdentifiers::ConvertProjectToSemanticService, call: nil) + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service) + + described_class.new.perform(project.id) + + expect(service).to have_received(:call) + end + end +end diff --git a/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb b/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb new file mode 100644 index 00000000000..673041901ab --- /dev/null +++ b/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "rails_helper" + +RSpec.describe ProjectIdentifiers::FinishSemanticConversionJob do + subject(:job) { described_class.new } + + let(:update_service) { instance_double(Settings::UpdateService, call: ServiceResult.success) } + + before do + allow(Settings::UpdateService).to receive(:new).with(user: User.system).and_return(update_service) + end + + describe "#perform" do + context "when no projects remain from the start" do + before { allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set.new) } + + it "enables semantic mode without running any conversion" do + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new) + job.perform + expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new) + expect(update_service).to have_received(:call) + .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) + end + end + + context "when projects are cleared after the first sweep" do + let(:project) { instance_double(Project) } + let(:service) { instance_double(ProjectIdentifiers::ConvertProjectToSemanticService, call: nil) } + + before do + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[1], Set.new) + allow(Project).to receive(:find_by).with(id: 1).and_return(project) + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service) + end + + it "runs one conversion sweep then enables semantic mode" do + job.perform + expect(service).to have_received(:call).once + expect(update_service).to have_received(:call) + .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) + end + end + + context "when projects are cleared on the last sweep" do + let(:project) { instance_double(Project) } + let(:service) { instance_double(ProjectIdentifiers::ConvertProjectToSemanticService, call: nil) } + + before do + pending_sets = Array.new(described_class::MAX_SWEEPS, Set[1]) + [Set.new] + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(*pending_sets) + allow(Project).to receive(:find_by).with(id: 1).and_return(project) + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service) + end + + it "enables semantic mode after the final sweep clears all projects" do + job.perform + expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times + expect(update_service).to have_received(:call) + .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) + end + end + + context "when projects still remain after all sweeps" do + let(:project) { instance_double(Project) } + let(:service) { instance_double(ProjectIdentifiers::ConvertProjectToSemanticService, call: nil) } + + before do + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[1]) + allow(Project).to receive(:find_by).with(id: 1).and_return(project) + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service) + end + + it "raises after MAX_SWEEPS sweeps, logging a warning and not enabling semantic mode" do + allow(Rails.logger).to receive(:warn) + give_up_pattern = /Giving up after #{described_class::MAX_SWEEPS} sweeps/o + + expect { job.perform }.to raise_error(RuntimeError, give_up_pattern) + expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times + expect(Rails.logger).to have_received(:warn).with(give_up_pattern) + expect(update_service).not_to have_received(:call) + end + end + + context "when a remaining project no longer exists" do + before do + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[99], Set.new) + allow(Project).to receive(:find_by).with(id: 99).and_return(nil) + allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new) + end + + it "skips the missing project and still enables semantic mode" do + job.perform + expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new) + expect(update_service).to have_received(:call) + .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) + end + end + end +end