Add latest changes from gitlab-org/gitlab@master

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
 

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

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

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 <EMAIL@ADDRESS>, 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"

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)

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 = {

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