Split translation & metadata entries into classes

8 files changed, 271 insertions, 238 deletions

diff --git a/lib/gitlab/i18n/metadata_entry.rb b/lib/gitlab/i18n/metadata_entry.rb
new file mode 100644
index 00000000000..3f9cbae62c8
--- /dev/null
+++ b/lib/gitlab/i18n/metadata_entry.rb
@@ -0,0 +1,24 @@
+module Gitlab
+  module I18n
+    class MetadataEntry < PoEntry
+      def expected_plurals
+        return nil unless plural_information
+
+        nplurals = plural_information['nplurals'].to_i
+        if nplurals > 0
+          nplurals
+        end
+      end
+
+      private
+
+      def plural_information
+        return @plural_information if defined?(@plural_information)
+
+        if plural_line = entry_data[:msgstr].detect { |metadata_line| metadata_line.starts_with?('Plural-Forms: ') }
+          @plural_information = Hash[plural_line.scan(/(\w+)=([^;\n]+)/)]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/i18n/po_entry.rb b/lib/gitlab/i18n/po_entry.rb
index f2a4bfbd1cd..014043cfdd6 100644
--- a/lib/gitlab/i18n/po_entry.rb
+++ b/lib/gitlab/i18n/po_entry.rb
@@ -1,96 +1,18 @@
 module Gitlab
   module I18n
     class PoEntry
-      attr_reader :entry_data
-
-      def initialize(entry_data)
-        @entry_data = entry_data
-      end
-
-      def msgid
-        entry_data[:msgid]
-      end
-
-      def metadata?
-        msgid.empty?
-      end
-
-      def plural_id
-        entry_data[:msgid_plural]
-      end
-
-      def plural?
-        plural_id.present?
-      end
-
-      def singular_translation
-        plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr]
-      end
-
-      def all_translations
-        @all_translations ||= entry_data.fetch_values(*translation_keys).reject(&:empty?)
-      end
-
-      def translated?
-        all_translations.any?
-      end
-
-      def plural_translations
-        return [] unless plural?
-        return [] unless translated?
-
-        # The singular translation is used if there's only translation. This is
-        # the case for languages without plurals.
-        return all_translations if all_translations.size == 1
-
-        entry_data.fetch_values(*plural_translation_keys)
-      end
-
-      def flag
-        entry_data[:flag]
-      end
-
-      def expected_plurals
-        return nil unless metadata?
-        return nil unless plural_information
-
-        nplurals = plural_information['nplurals'].to_i
-        if nplurals > 0
-          nplurals
+      def self.build(entry_data)
+        if entry_data[:msgid].empty?
+          MetadataEntry.new(entry_data)
+        else
+          TranslationEntry.new(entry_data)
         end
       end
 
-      # When a translation is a plural, but only has 1 translation, we could be
-      # talking about a language in which plural and singular is the same thing.
-      # In which case we always translate as a plural.
-      def has_singular?
-        !plural? || all_translations.size > 1
-      end
-
-      private
-
-      def plural_information
-        return nil unless metadata?
-        return @plural_information if defined?(@plural_information)
-
-        if plural_line = entry_data[:msgstr].detect { |metadata_line| metadata_line.starts_with?('Plural-Forms: ') }
-          @plural_information = Hash[plural_line.scan(/(\w+)=([^;\n]+)/)]
-        end
-      end
-
-      def plural_translation_keys
-        @plural_translation_keys ||= translation_keys.select do |key|
-          plural_index = key.scan(/\d+/).first.to_i
-          plural_index > 0
-        end
-      end
+      attr_reader :entry_data
 
-      def translation_keys
-        @translation_keys ||= if plural?
-                                entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ }
-                              else
-                                [:msgstr]
-                              end
+      def initialize(entry_data)
+        @entry_data = entry_data
       end
     end
   end
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index e7c92be1383..0dc96ac7b9b 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -3,7 +3,7 @@ require 'simple_po_parser'
 module Gitlab
   module I18n
     class PoLinter
-      attr_reader :po_path, :entries, :metadata, :locale
+      attr_reader :po_path, :translation_entries, :metadata_entry, :locale
 
       VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze
 
@@ -25,20 +25,23 @@ module Gitlab
       end
 
       def parse_po
-        @entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.new(data) }
-        @metadata = @entries.detect { |entry| entry.metadata? }
+        entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.build(data) }
+
+        # The first entry is the metadata entry if there is one.
+        # This is an entry when empty `msgid`
+        @metadata_entry = entries.shift if entries.first.is_a?(Gitlab::I18n::MetadataEntry)
+        @translation_entries = entries
+
         nil
       rescue SimplePoParser::ParserError => e
-        @entries = []
+        @translation_entries = []
         e.message
       end
 
       def validate_entries
         errors = {}
 
-        entries.each do |entry|
-          next if entry.metadata?
-
+        translation_entries.each do |entry|
           errors_for_entry = validate_entry(entry)
           errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any?
         end
@@ -58,11 +61,12 @@ module Gitlab
       end
 
       def validate_number_of_plurals(errors, entry)
-        return unless metadata&.expected_plurals
+        return unless metadata_entry&.expected_plurals
         return unless entry.translated?
 
-        if entry.plural? && entry.all_translations.size != metadata.expected_plurals
-          errors << "should have #{metadata.expected_plurals} #{'translations'.pluralize(metadata.expected_plurals)}"
+        if entry.plural? && entry.all_translations.size != metadata_entry.expected_plurals
+          errors << "should have #{metadata_entry.expected_plurals} "\
+                    "#{'translations'.pluralize(metadata_entry.expected_plurals)}"
         end
       end
 
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
new file mode 100644
index 00000000000..98095177994
--- /dev/null
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -0,0 +1,68 @@
+module Gitlab
+  module I18n
+    class TranslationEntry < PoEntry
+      def msgid
+        entry_data[:msgid]
+      end
+
+      def plural_id
+        entry_data[:msgid_plural]
+      end
+
+      def plural?
+        plural_id.present?
+      end
+
+      def singular_translation
+        plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr]
+      end
+
+      def all_translations
+        @all_translations ||= entry_data.fetch_values(*translation_keys).reject(&:empty?)
+      end
+
+      def translated?
+        all_translations.any?
+      end
+
+      def plural_translations
+        return [] unless plural?
+        return [] unless translated?
+
+        # The singular translation is used if there's only translation. This is
+        # the case for languages without plurals.
+        return all_translations if all_translations.size == 1
+
+        entry_data.fetch_values(*plural_translation_keys)
+      end
+
+      def flag
+        entry_data[:flag]
+      end
+
+      # When a translation is a plural, but only has 1 translation, we could be
+      # talking about a language in which plural and singular is the same thing.
+      # In which case we always translate as a plural.
+      def has_singular?
+        !plural? || all_translations.size > 1
+      end
+
+      private
+
+      def plural_translation_keys
+        @plural_translation_keys ||= translation_keys.select do |key|
+          plural_index = key.scan(/\d+/).first.to_i
+          plural_index > 0
+        end
+      end
+
+      def translation_keys
+        @translation_keys ||= if plural?
+                                entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ }
+                              else
+                                [:msgstr]
+                              end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/i18n/metadata_entry_spec.rb b/spec/lib/gitlab/i18n/metadata_entry_spec.rb
new file mode 100644
index 00000000000..5bc16e1806e
--- /dev/null
+++ b/spec/lib/gitlab/i18n/metadata_entry_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Gitlab::I18n::MetadataEntry do
+  describe '#expected_plurals' do
+    it 'returns the number of plurals' do
+      data = {
+        msgid: "",
+        msgstr: [
+          "",
+          "Project-Id-Version: gitlab 1.0.0\\n",
+          "Report-Msgid-Bugs-To: \\n",
+          "PO-Revision-Date: 2017-07-13 12:10-0500\\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",
+          "Last-Translator: Bob Van Landuyt <bob@gitlab.com>\\n",
+          "X-Generator: Poedit 2.0.2\\n"
+        ]
+      }
+      entry = described_class.new(data)
+
+      expect(entry.expected_plurals).to eq(2)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/i18n/po_entry_spec.rb b/spec/lib/gitlab/i18n/po_entry_spec.rb
index e671f3c17a1..4be1b3d4383 100644
--- a/spec/lib/gitlab/i18n/po_entry_spec.rb
+++ b/spec/lib/gitlab/i18n/po_entry_spec.rb
@@ -1,138 +1,18 @@
 require 'spec_helper'
 
 describe Gitlab::I18n::PoEntry do
-  describe '#singular_translation' do
-    it 'returns the normal `msgstr` for translations without plural' do
-      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
-      entry = described_class.new(data)
+  describe '.build' do
+    it 'builds a metadata entry when the msgid is empty' do
+      entry = described_class.build(msgid: '')
 
-      expect(entry.singular_translation).to eq('Bonjour monde')
+      expect(entry).to be_kind_of(Gitlab::I18n::MetadataEntry)
     end
 
-    it 'returns the first string for entries with plurals' do
-      data = {
-        msgid: 'Hello world',
-        msgid_plural: 'Hello worlds',
-        'msgstr[0]' => 'Bonjour monde',
-        'msgstr[1]' => 'Bonjour mondes'
-      }
-      entry = described_class.new(data)
+    it 'builds a translation entry when the msgid is empty' do
+      entry = described_class.build(msgid: 'Hello world')
 
-      expect(entry.singular_translation).to eq('Bonjour monde')
+      expect(entry).to be_kind_of(Gitlab::I18n::TranslationEntry)
     end
-  end
-
-  describe '#all_translations' do
-    it 'returns all translations for singular translations' do
-      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
-      entry = described_class.new(data)
-
-      expect(entry.all_translations).to eq(['Bonjour monde'])
-    end
-
-    it 'returns all translations when including plural translations' do
-      data = {
-        msgid: 'Hello world',
-        msgid_plural: 'Hello worlds',
-        'msgstr[0]' => 'Bonjour monde',
-        'msgstr[1]' => 'Bonjour mondes'
-      }
-      entry = described_class.new(data)
-
-      expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes'])
-    end
-  end
-
-  describe '#plural_translations' do
-    it 'returns all translations if there is only one plural' do
-      data = {
-        msgid: 'Hello world',
-        msgid_plural: 'Hello worlds',
-        'msgstr[0]' => 'Bonjour monde'
-      }
-      entry = described_class.new(data)
-
-      expect(entry.plural_translations).to eq(['Bonjour monde'])
-    end
-
-    it 'returns all translations except for the first one if there are multiple' do
-      data = {
-        msgid: 'Hello world',
-        msgid_plural: 'Hello worlds',
-        'msgstr[0]' => 'Bonjour monde',
-        'msgstr[1]' => 'Bonjour mondes',
-        'msgstr[2]' => 'Bonjour tous les mondes'
-      }
-      entry = described_class.new(data)
-
-      expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes'])
-    end
-  end
 
-  describe '#expected_plurals' do
-    it 'returns nil when the entry is an actual translation' do
-      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
-      entry = described_class.new(data)
-
-      expect(entry.expected_plurals).to be_nil
-    end
-
-    it 'returns the number of plurals' do
-      data = {
-        msgid: "",
-        msgstr: [
-          "",
-          "Project-Id-Version: gitlab 1.0.0\\n",
-          "Report-Msgid-Bugs-To: \\n",
-          "PO-Revision-Date: 2017-07-13 12:10-0500\\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",
-          "Last-Translator: Bob Van Landuyt <bob@gitlab.com>\\n",
-          "X-Generator: Poedit 2.0.2\\n"
-        ]
-      }
-      entry = described_class.new(data)
-
-      expect(entry.expected_plurals).to eq(2)
-    end
-  end
-
-  describe '#has_singular?' do
-    it 'has a singular when the translation is not pluralized' do
-      data = {
-        msgid: 'hello world',
-        msgstr: 'hello'
-      }
-      entry = described_class.new(data)
-
-      expect(entry).to have_singular
-    end
-
-    it 'has a singular when plural and singular are separately defined' do
-      data = {
-        msgid: 'hello world',
-        msgid_plural: 'hello worlds',
-        "msgstr[0]" => 'hello world',
-        "msgstr[1]" => 'hello worlds'
-      }
-      entry = described_class.new(data)
-
-      expect(entry).to have_singular
-    end
-
-    it 'does not have a separate singular if the plural string only has one translation' do
-      data = {
-        msgid: 'hello world',
-        msgid_plural: 'hello worlds',
-        "msgstr[0]" => 'hello worlds'
-      }
-      entry = described_class.new(data)
-
-      expect(entry).not_to have_singular
-    end
   end
 end
diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb
index 307ea8b2640..c40e8ff63bb 100644
--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -102,7 +102,8 @@ describe Gitlab::I18n::PoLinter do
       it 'fills in the entries' do
         linter.parse_po
 
-        expect(linter.entries).not_to be_empty
+        expect(linter.translation_entries).not_to be_empty
+        expect(linter.metadata_entry).to be_kind_of(Gitlab::I18n::MetadataEntry)
       end
 
       it 'does not have errors' do
@@ -120,21 +121,18 @@ describe Gitlab::I18n::PoLinter do
       it 'sets the entries to an empty array' do
         linter.parse_po
 
-        expect(linter.entries).to eq([])
+        expect(linter.translation_entries).to eq([])
       end
     end
   end
 
   describe '#validate_entries' do
-    it 'skips entries without a `msgid`' do
-      allow(linter).to receive(:entries) { [Gitlab::I18n::PoEntry.new({ msgid: "" })] }
-
-      expect(linter.validate_entries).to be_empty
-    end
-
     it 'keeps track of errors for entries' do
-      fake_invalid_entry = Gitlab::I18n::PoEntry.new({ msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" })
-      allow(linter).to receive(:entries) { [fake_invalid_entry] }
+      fake_invalid_entry = Gitlab::I18n::TranslationEntry.new(
+        msgid: "Hello %{world}",
+        msgstr: "Bonjour %{monde}"
+      )
+      allow(linter).to receive(:translation_entries) { [fake_invalid_entry] }
 
       expect(linter).to receive(:validate_entry)
                           .with(fake_invalid_entry)
@@ -161,9 +159,9 @@ describe Gitlab::I18n::PoLinter do
     it 'validates when there are an incorrect number of translations' do
       fake_metadata = double
       allow(fake_metadata).to receive(:expected_plurals).and_return(2)
-      allow(linter).to receive(:metadata).and_return(fake_metadata)
+      allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
 
-      fake_entry = Gitlab::I18n::PoEntry.new(
+      fake_entry = Gitlab::I18n::TranslationEntry.new(
         msgid: 'the singular',
         msgid_plural: 'the plural',
         'msgstr[0]' => 'the singular'
@@ -178,7 +176,7 @@ describe Gitlab::I18n::PoLinter do
 
   describe '#validate_variables' do
     it 'validates both signular and plural in a pluralized string when the entry has a singular' do
-      pluralized_entry = Gitlab::I18n::PoEntry.new({
+      pluralized_entry = Gitlab::I18n::TranslationEntry.new({
         msgid: 'Hello %{world}',
         msgid_plural: 'Hello all %{world}',
         'msgstr[0]' => 'Bonjour %{world}',
@@ -194,7 +192,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'only validates plural when there is no separate singular' do
-      pluralized_entry = Gitlab::I18n::PoEntry.new({
+      pluralized_entry = Gitlab::I18n::TranslationEntry.new({
         msgid: 'Hello %{world}',
         msgid_plural: 'Hello all %{world}',
         'msgstr[0]' => 'Bonjour %{world}'
@@ -207,7 +205,10 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'validates the message variables' do
-      entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello', msgstr: 'Bonjour' })
+      entry = Gitlab::I18n::TranslationEntry.new(
+        msgid: 'Hello',
+        msgstr: 'Bonjour'
+      )
 
       expect(linter).to receive(:validate_variables_in_message)
                           .with([], 'Hello', 'Bonjour')
diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb
new file mode 100644
index 00000000000..7d97942a1d5
--- /dev/null
+++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb
@@ -0,0 +1,106 @@
+require 'spec_helper'
+
+describe Gitlab::I18n::TranslationEntry do
+  describe '#singular_translation' do
+    it 'returns the normal `msgstr` for translations without plural' do
+      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
+      entry = described_class.new(data)
+
+      expect(entry.singular_translation).to eq('Bonjour monde')
+    end
+
+    it 'returns the first string for entries with plurals' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.singular_translation).to eq('Bonjour monde')
+    end
+  end
+
+  describe '#all_translations' do
+    it 'returns all translations for singular translations' do
+      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
+      entry = described_class.new(data)
+
+      expect(entry.all_translations).to eq(['Bonjour monde'])
+    end
+
+    it 'returns all translations when including plural translations' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes'])
+    end
+  end
+
+  describe '#plural_translations' do
+    it 'returns all translations if there is only one plural' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.plural_translations).to eq(['Bonjour monde'])
+    end
+
+    it 'returns all translations except for the first one if there are multiple' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes',
+        'msgstr[2]' => 'Bonjour tous les mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes'])
+    end
+  end
+
+  describe '#has_singular?' do
+    it 'has a singular when the translation is not pluralized' do
+      data = {
+        msgid: 'hello world',
+        msgstr: 'hello'
+      }
+      entry = described_class.new(data)
+
+      expect(entry).to have_singular
+    end
+
+    it 'has a singular when plural and singular are separately defined' do
+      data = {
+        msgid: 'hello world',
+        msgid_plural: 'hello worlds',
+        "msgstr[0]" => 'hello world',
+        "msgstr[1]" => 'hello worlds'
+      }
+      entry = described_class.new(data)
+
+      expect(entry).to have_singular
+    end
+
+    it 'does not have a separate singular if the plural string only has one translation' do
+      data = {
+        msgid: 'hello world',
+        msgid_plural: 'hello worlds',
+        "msgstr[0]" => 'hello worlds'
+      }
+      entry = described_class.new(data)
+
+      expect(entry).not_to have_singular
+    end
+  end
+end