From 08d1ac69df51ee66f18c91716bf5d051c0cc3eda Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 23 Dec 2024 18:34:15 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../ci/job_token/authorizations_compactor.rb | 2 +- lib/gitlab/i18n/po_linter.rb | 17 +- lib/gitlab/i18n/translation_entry.rb | 32 +++ spec/fixtures/spaces.po | 30 +++ spec/lib/gitlab/i18n/po_linter_spec.rb | 37 ++- .../lib/gitlab/i18n/translation_entry_spec.rb | 216 ++++++++++++++++++ .../authorizations_compactor_spec.rb | 14 ++ 7 files changed, 344 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/spaces.po diff --git a/app/models/ci/job_token/authorizations_compactor.rb b/app/models/ci/job_token/authorizations_compactor.rb index df24293922b..6485c2371ba 100644 --- a/app/models/ci/job_token/authorizations_compactor.rb +++ b/app/models/ci/job_token/authorizations_compactor.rb @@ -27,7 +27,7 @@ module Ci end origin_project_id_batches.each do |batch| - projects = Project.where(id: batch) + projects = Project.where(id: batch).includes(:project_namespace) origin_project_traversal_ids += projects.map { |p| p.project_namespace.traversal_ids } end diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb index b85fd991a94..36de3123460 100644 --- a/lib/gitlab/i18n/po_linter.rb +++ b/lib/gitlab/i18n/po_linter.rb @@ -76,13 +76,28 @@ module Gitlab validate_html(errors, entry) validate_translation(errors, entry) validate_namespace(errors, entry) + validate_spaces(errors, entry) errors end + def validate_spaces(errors, entry) + if entry.translations_contain_leading_space? + errors << 'has leading space. Remove it from the translation' + end + + if entry.translations_contain_trailing_space? + errors << 'has trailing space. Remove it from the translation' + end + + if entry.translations_contain_multiple_spaces? + errors << 'has different sets of consecutive multiple spaces. Make them consistent with source string' + end + end + def validate_namespace(errors, entry) if entry.translations_contain_namespace? - errors << 'contains a namespace, remove it from the translation. For more information see ' \ + errors << 'contains a namespace. Remove it from the translation. For more information see ' \ 'https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings' end end diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb index 22ec6e8dbe8..51bf08d24ae 100644 --- a/lib/gitlab/i18n/translation_entry.rb +++ b/lib/gitlab/i18n/translation_entry.rb @@ -6,6 +6,8 @@ module Gitlab PERCENT_REGEX = /(?:^|[^%])%(?!{\w*}|[a-z%])/ ANGLE_BRACKET_REGEX = /[<>]/ NAMESPACE_REGEX = /^((?u)\w+|\s)*\|/ + SPACE_REGEX = /[\p{Separator}\u0009-\u000d\u001c-\u001f\u0085\u180e]/ + MULTIPLE_CONSECUTIVE_SPACES_REGEX = /#{SPACE_REGEX}{2,}/o attr_reader :nplurals, :entry_data @@ -22,6 +24,10 @@ module Gitlab @plural_id ||= Array(entry_data[:msgid_plural]).join end + def msgid_without_namespace + @msgid_without_namespace ||= msgid.sub(NAMESPACE_REGEX, '') + end + def has_plural? plural_id.present? end @@ -77,6 +83,32 @@ module Gitlab string =~ NAMESPACE_REGEX end + def translations_contain_leading_space? + all_translations.any? { |translation| contains_leading_space?(translation) } + end + + def contains_leading_space?(translation) + translation.match?(/\A,?#{SPACE_REGEX}/o) && !msgid_without_namespace.match?(/\A,?#{SPACE_REGEX}/o) + end + + def translations_contain_trailing_space? + all_translations.any? { |translation| contains_trailing_space?(translation) } + end + + def contains_trailing_space?(translation) + translation.match?(/#{SPACE_REGEX}\Z/o) && !msgid_without_namespace.match?(/#{SPACE_REGEX}\Z/o) + end + + def translations_contain_multiple_spaces? + all_translations.any? { |translation| contains_multiple_spaces?(translation) } + end + + def contains_multiple_spaces?(translation) + msgid_matches = msgid_without_namespace.scan(MULTIPLE_CONSECUTIVE_SPACES_REGEX) + translation_matches = translation.scan(MULTIPLE_CONSECUTIVE_SPACES_REGEX) + msgid_matches.sort != translation_matches.sort + end + def msgid_contains_unescaped_chars? contains_unescaped_chars?(msgid) end diff --git a/spec/fixtures/spaces.po b/spec/fixtures/spaces.po new file mode 100644 index 00000000000..db676d8c20a --- /dev/null +++ b/spec/fixtures/spaces.po @@ -0,0 +1,30 @@ +# Spanish translations for gitlab package. +# Copyright (C) 2017 THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the gitlab package. +# FIRST AUTHOR , 2017. +# +msgid "" +msgstr "" +"Project-Id-Version: gitlab 1.0.0\n" +"Report-Msgid-Bugs-To: \n" +"Language-Team: Spanish\n" +"Language: es\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=2; plural=n != 1;\n" +"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" + +msgid "1 commit" +msgstr " 1 cambio" + +msgid "With plural" +msgid_plural "with plurals" +msgstr[0] " first" +msgstr[1] "second" + +msgid "User" +msgstr "Usuario " + +msgid "Hello there world" +msgstr "Hello a todos mundo" diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index ed29c7c4e37..e4cbf0eddce 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -52,14 +52,46 @@ RSpec.describe Gitlab::I18n::PoLinter do it 'has an error for translation with namespace' do message_id = "404|Not found" - expected_message = "contains a namespace, remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings" + expected_message = "contains a namespace. Remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings" expect(errors[message_id]).to include(expected_message) end it 'has an error for plural translation with namespace' do message_id = "CommitHistory|1 commit" - expected_message = "contains a namespace, remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings" + expected_message = "contains a namespace. Remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings" + + expect(errors[message_id]).to include(expected_message) + end + end + + context 'for a translations with spaces' do + let(:po_path) { 'spec/fixtures/spaces.po' } + + it 'has an error for translation with a leading space' do + message_id = "1 commit" + expected_message = "has leading space. Remove it from the translation" + + expect(errors[message_id]).to include(expected_message) + end + + it 'has an error for plural translation with a leading space' do + message_id = "With plural" + expected_message = "has leading space. Remove it from the translation" + + expect(errors[message_id]).to include(expected_message) + end + + it 'has an error for translation with a trailing space' do + message_id = "User" + expected_message = "has trailing space. Remove it from the translation" + + expect(errors[message_id]).to include(expected_message) + end + + it 'has an error for translation with a multiple spaces not present in source string' do + message_id = "Hello there world" + expected_message = "has different sets of consecutive multiple spaces. Make them consistent with source string" expect(errors[message_id]).to include(expected_message) end @@ -233,6 +265,7 @@ RSpec.describe Gitlab::I18n::PoLinter do expect(linter).to receive(:validate_unescaped_chars).with([], fake_entry) expect(linter).to receive(:validate_translation).with([], fake_entry) expect(linter).to receive(:validate_namespace).with([], fake_entry) + expect(linter).to receive(:validate_spaces).with([], fake_entry) expect(linter).to receive(:validate_html).with([], fake_entry) linter.validate_entry(fake_entry) diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb index ddb72335ef9..b2b7ad8ec56 100644 --- a/spec/lib/gitlab/i18n/translation_entry_spec.rb +++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb @@ -133,6 +133,222 @@ RSpec.describe Gitlab::I18n::TranslationEntry do end end + describe '#translations_contain_leading_space' do + it 'is true when msgstr starts with a space and msgid does not' do + data = { + msgid: 'Hello world', + msgstr: ' Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_truthy + end + + it 'is true when msgstr starts with a space, msgid does not and msgid contains namespace' do + data = { + msgid: 'General|Hello world', + msgstr: ' Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_truthy + end + + it 'is false when msgstr and msgid both start with a space and msgid contains namespace' do + data = { + msgid: 'General| Hello world', + msgstr: ' Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr and msgid both start with a space' do + data = { + msgid: ' Hello world', + msgstr: ' Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr starts with a space and msgid starts with a comma followed by space' do + data = { + msgid: ', or ', + msgstr: ' eller ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr and msgid both start with a comma followed by space' do + data = { + msgid: ', or ', + msgstr: ', eller ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr starts with a comma and a space and msgid starts with a comma' do + data = { + msgid: ' or ', + msgstr: ', eller ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr and msgid both do not start with a space' do + data = { + msgid: 'Hello world', + msgstr: 'Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + + it 'is false when msgstr does not contain a space but msgid does' do + data = { + msgid: 'Hello world', + msgstr: 'Ahojsvěte' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_leading_space?).to be_falsy + end + end + + describe '#translations_contain_trailing_space' do + it 'is true when msgstr ends with a space and msgid does not' do + data = { + msgid: 'Hello world', + msgstr: 'Ahoj světe ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_truthy + end + + it 'is true when msgstr ends with a space, msgid does not and msgid contains namespace' do + data = { + msgid: 'General|Hello world', + msgstr: 'Ahoj světe ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_truthy + end + + it 'is false when msgstr and msgid both end with a space and msgid contains namespace' do + data = { + msgid: 'General|Hello world ', + msgstr: 'Ahoj světe ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_falsy + end + + it 'is false when msgstr and msgid both end with a space' do + data = { + msgid: 'Hello world ', + msgstr: 'Ahoj světe ' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_falsy + end + + it 'is false when msgstr and msgid both do not end with a space' do + data = { + msgid: 'Hello world', + msgstr: 'Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_falsy + end + + it 'is false when msgstr does not contain a space but msgid does' do + data = { + msgid: 'Hello world', + msgstr: 'Ahojsvěte' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_trailing_space?).to be_falsy + end + end + + describe '#translations_contain_multiple_spaces' do + it 'is true when msgstr contains multiple spaces and msgid does not' do + data = { + msgid: 'Hello world', + msgstr: 'Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_truthy + end + + it 'is true when msgstr and msgid contain different amounts of multiple consecutive spaces' do + data = { + msgid: 'Hello world and all', + msgstr: 'Ahoj světe a všichni' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_truthy + end + + it 'is true when msgstr and msgid contain different amounts of multiple consecutive spaces and chunks' do + data = { + msgid: 'Hello world and all', + msgstr: 'Ahoj světe a všichni' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_truthy + end + + it 'is false when msgstr and msgid contain the same chunks of multiple spaces at different places' do + data = { + msgid: 'Hello world and all', + msgstr: "Ahoj světe a všichni" + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_falsy + end + + it 'is false when msgstr and msgid contain the same chunks of multiple spaces at the same places' do + data = { + msgid: 'Hello world and all', + msgstr: 'Ahoj světe a všichni' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_falsy + end + + it 'is false when msgstr and msgid contain one chunk of multiple spaces at the same place' do + data = { + msgid: 'Hello world', + msgstr: 'Ahoj světe' + } + entry = described_class.new(entry_data: data, nplurals: 2) + + expect(entry.translations_contain_multiple_spaces?).to be_falsy + end + end + describe '#translations_contain_namespace' do it 'is true when the msgstr contains namespace' do data = { diff --git a/spec/models/ci/job_token/authorizations_compactor_spec.rb b/spec/models/ci/job_token/authorizations_compactor_spec.rb index 0ad1e92d2f2..f67c461a8ff 100644 --- a/spec/models/ci/job_token/authorizations_compactor_spec.rb +++ b/spec/models/ci/job_token/authorizations_compactor_spec.rb @@ -112,4 +112,18 @@ RSpec.describe Ci::JobToken::AuthorizationsCompactor, feature_category: :secrets end end end + + describe '#origin_traversal_ids' do + it 'does not cause N+1 queries when loading projects' do + accessed_project_control = create(:project) + create(:ci_job_token_authorization, origin_project: pns1.project, accessed_project: accessed_project_control, + last_authorized_at: 1.day.ago) + compactor_control = described_class.new(accessed_project_control.id) + control = ActiveRecord::QueryRecorder.new do + compactor_control.origin_project_traversal_ids + end + + expect { compactor.origin_project_traversal_ids }.not_to exceed_query_limit(control) + end + end end