[L10n Tooling: i3] Re-implement download-from-glotpress logic from .py and .swift scripts

.rubocop_todo.yml

@@ -146,6 +146,7 @@ RSpec/FilePath:
     - 'spec/ios_l10n_helper_spec.rb'
     - 'spec/ios_merge_strings_files_spec.rb'
     - 'spec/gp_update_metadata_source_spec.rb'
+    - 'spec/ios_download_strings_files_from_glotpress_spec.rb'
 
 # Offense count: 8
 # Cop supports --auto-correct.

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### New Features
 
 * Introduce new `ios_merge_strings_files` action. [#329]
+* Introduce new `ios_download_strings_files_from_glotpress` action. [#331]
 
 ### Bug Fixes
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_download_strings_files_from_glotpress.rb

@@ -0,0 +1,113 @@
+module Fastlane
+  module Actions
+    class IosDownloadStringsFilesFromGlotpressAction < Action
+      def self.run(params)
+        # TODO: Once we introduce the `Locale` POD via #296, check if the param is an array of locales and if so convert it to Hash{glotpress=>lproj}
+        locales = params[:locales]
+        download_dir = params[:download_dir]
+
+        UI.user_error!("The parent directory `#{download_dir}` (which contains all the `*.lproj` subdirectories) must already exist") unless Dir.exist?(download_dir)
+
+        locales.each do |glotpress_locale, lproj_name|
+          # Download the export in the proper `.lproj` directory
+          lproj_dir = File.join(download_dir, "#{lproj_name}.lproj")
+          destination = File.join(lproj_dir, "#{params[:table_basename]}.strings")
+          FileUtils.mkdir(lproj_dir) unless Dir.exist?(lproj_dir)
+
+          Fastlane::Helper::Ios::L10nHelper.download_glotpress_export_file(
+            project_url: params[:project_url],
+            locale: glotpress_locale,
+            filters: params[:filters],
+            destination: destination
+          )
+          # Do a quick check of the downloaded `.strings` file to ensure it looks valid
+          validate_strings_file(destination) unless params[:skip_file_validation]
+        end
+      end
+
+      # Validate that a `.strings` file downloaded from GlotPress seems valid and does not contain empty translations
+      def self.validate_strings_file(destination)
+        return unless File.exist?(destination) # If the file failed to download, don't try to validate an non-existing file. We'd already have a separate error for the download failure anyway.
+
+        translations = Fastlane::Helper::Ios::L10nHelper.read_strings_file_as_hash(path: destination)
+        empty_keys = translations.select { |_, value| value.nil? || value.empty? }.keys.sort
+        unless empty_keys.empty?
+          UI.error(
+            "Found empty translations in `#{destination}` for the following keys: #{empty_keys.inspect}.\n" \
+              + "This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.\n" \
+              + 'Please report this to the GlotPress team, and fix the file locally before continuing.'
+          )
+        end
+      rescue StandardError => e
+        UI.error("Error while validating the file exported from GlotPress (`#{destination}`) - #{e.message.chomp}")
+      end
+
+      #####################################################
+      # @!group Documentation
+      #####################################################
+
+      def self.description
+        'Downloads the `.strings` files from GlotPress for the various locales'
+      end
+
+      def self.details
+        <<~DETAILS
+          Downloads the `.strings` files from GlotPress for the various locales,
+          validates them, and saves them in the relevant `*.lproj` directories for each locale
+        DETAILS
+      end
+
+      def self.available_options
+        [
+          FastlaneCore::ConfigItem.new(key: :project_url,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_PROJECT_URL',
+                                       description: 'URL to the GlotPress project',
+                                       type: String),
+          FastlaneCore::ConfigItem.new(key: :locales,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_LOCALES',
+                                       description: 'The map of locales to download, each entry of the Hash corresponding to a { glotpress-locale-code => lproj-folder-basename } pair',
+                                       type: Hash), # TODO: also support an Array of `Locale` POD/struct type when we introduce it later (see #296)
+          FastlaneCore::ConfigItem.new(key: :download_dir,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_DOWNLOAD_DIR',
+                                       description: 'The parent directory containing all the `*.lproj` subdirectories in which the downloaded files will be saved',
+                                       type: String),
+          FastlaneCore::ConfigItem.new(key: :table_basename,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_TABLE_BASENAME',
+                                       description: 'The basename to save the `.strings` files under',
+                                       type: String,
+                                       optional: true,
+                                       default_value: 'Localizable'),
+          FastlaneCore::ConfigItem.new(key: :filters,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_FILTERS',
+                                       description: 'The GlotPress filters to use when requesting the translations export',
+                                       type: Hash,
+                                       optional: true,
+                                       default_value: { status: 'current' }),
+          FastlaneCore::ConfigItem.new(key: :skip_file_validation,
+                                       env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_SKIP_FILE_VALIDATION',
+                                       description: 'If true, skips the validation of `.strings` files after download',
+                                       type: Fastlane::Boolean,
+                                       optional: true,
+                                       default_value: false),
+        ]
+      end
+
+      def self.return_type
+        # Describes what type of data is expected to be returned
+        # see RETURN_TYPES in https://github.com/fastlane/fastlane/blob/master/fastlane/lib/fastlane/action.rb
+      end
+
+      def self.return_value
+        # Textual description of what the return value is
+      end
+
+      def self.authors
+        ['Automattic']
+      end
+
+      def self.is_supported?(platform)
+        [:ios, :mac].include?(platform)
+      end
+    end
+  end
+end

lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb

@@ -1,8 +1,9 @@
-require 'open3'
-require 'tempfile'
 require 'fileutils'
-require 'nokogiri'
 require 'json'
+require 'nokogiri'
+require 'open3'
+require 'open-uri'
+require 'tempfile'
 
 module Fastlane
   module Helper
@@ -116,6 +117,25 @@ def self.generate_strings_file_from_hash(translations:, output_path:)
           end
           File.write(output_path, builder.to_xml)
         end
+
+        # Downloads the export from GlotPress for a given locale and given filters.
+        #
+        # @param [String] project_url The URL to the GlotPress project to export from, e.g. `"https://translate.wordpress.org/projects/apps/ios/dev"`
+        # @param [String] locale The GlotPress locale code to download strings for.
+        # @param [Hash{Symbol=>String}] filters The hash of filters to apply when exporting from GlotPress.
+        #                               Typical examples include `{ status: 'current' }` or `{ status: 'review' }`.
+        # @param [String, IO] destination The path or `IO`-like instance, where to write the downloaded file on disk.
+        #
+        def self.download_glotpress_export_file(project_url:, locale:, filters:, destination:)
+          query_params = (filters || {}).transform_keys { |k| "filters[#{k}]" }.merge(format: 'strings')
+          uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations?#{URI.encode_www_form(query_params)}")
+          begin
+            IO.copy_stream(uri.open, destination)
+          rescue StandardError => e
+            UI.error "Error downloading locale `#{locale}` — #{e.message}"
+            return nil
+          end
+        end
       end
     end
   end

spec/ios_download_strings_files_from_glotpress_spec.rb

@@ -0,0 +1,184 @@
+require 'spec_helper'
+require 'tmpdir'
+
+describe Fastlane::Actions::IosDownloadStringsFilesFromGlotpressAction do
+  let(:test_data_dir) { File.join(File.dirname(__FILE__), 'test-data', 'translations', 'ios_generate_strings_file_from_code') }
+  let(:gp_fake_url) { 'https://stub.glotpress.com/rspec-fake-project' }
+  let(:locales_subset) { { 'fr-FR': 'fr', 'zh-cn': 'zh-Hans' } }
+
+  def gp_stub(locale:, query:)
+    stub_request(:get, "#{gp_fake_url}/#{locale}/default/export-translations").with(query: query)
+  end
+
+  describe 'downloading export files from GlotPress' do
+    def test_gp_download(filters:, tablename:, expected_gp_params:)
+      Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
+        # Arrange
+        stub_fr = gp_stub(locale: 'fr-FR', query: expected_gp_params).to_return(body: '"key" = "fr-copy";')
+        stub_zh = gp_stub(locale: 'zh-cn', query: expected_gp_params).to_return(body: '"key" = "zh-copy";')
+
+        # Act
+        # Note: The use of `.compact` here allows us to remove the keys (like `table_basename` and `filters`) from the `Hash` whose values are `nil`,
+        # so that we don't include those parameters at all in the action's call site -- making them use the default value from their respective
+        # `ConfigItem` (as opposed to passing a value of `nil` explicitly and overwrite the default value, which is not what we want to test).
+        run_described_fastlane_action({
+          project_url: gp_fake_url,
+          locales: locales_subset,
+          download_dir: tmp_dir,
+          table_basename: tablename,
+          filters: filters
+        }.compact)
+
+        # Assert
+        expect(stub_fr).to have_been_made.once
+        file_fr = File.join(tmp_dir, 'fr.lproj', "#{tablename || 'Localizable'}.strings")
+        expect(File).to exist(file_fr)
+        expect(File.read(file_fr)).to eq('"key" = "fr-copy";')
+
+        expect(stub_zh).to have_been_made.once
+        file_zh = File.join(tmp_dir, 'zh-Hans.lproj', "#{tablename || 'Localizable'}.strings")
+        expect(File).to exist(file_zh)
+        expect(File.read(file_zh)).to eq('"key" = "zh-copy";')
+      end
+    end
+
+    it 'downloads all the locales into the expected directories' do
+      test_gp_download(filters: nil, tablename: nil, expected_gp_params: { 'filters[status]': 'current', format: 'strings' })
+    end
+
+    it 'uses the proper filters when exporting the files from GlotPress' do
+      test_gp_download(
+        filters: { term: 'foo', status: 'review' },
+        tablename: nil,
+        expected_gp_params: { 'filters[term]': 'foo', 'filters[status]': 'review', format: 'strings' }
+      )
+    end
+
+    it 'uses a custom table name for the `.strings` files if provided' do
+      test_gp_download(
+        filters: nil,
+        tablename: 'MyApp',
+        expected_gp_params: { 'filters[status]': 'current', format: 'strings' }
+      )
+    end
+  end
+
+  describe 'error handling' do
+    it 'shows an error if an invalid locale is provided (404)' do
+      Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
+        # Arrange
+        stub = gp_stub(locale: 'unknown-locale', query: { 'filters[status]': 'current', format: 'strings' }).to_return(status: [404, 'Not Found'])
+        error_messages = []
+        allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
+
+        # Act
+        run_described_fastlane_action(
+          project_url: gp_fake_url,
+          locales: { 'unknown-locale': 'Base' },
+          download_dir: tmp_dir
+        )
+
+        # Assert
+        expect(stub).to have_been_made.once
+        expect(File).not_to exist(File.join(tmp_dir, 'Base.lproj', 'Localizable.strings'))
+        expect(error_messages).to eq(['Error downloading locale `unknown-locale` — 404 Not Found'])
+      end
+    end
+
+    it 'shows an error if the file cannot be written in the destination' do
+      # Arrange
+      download_dir = '/these/are/not/the/dirs/you/are/looking/for/'
+
+      # Act
+      act = lambda do
+        run_described_fastlane_action(
+          project_url: gp_fake_url,
+          locales: { 'fr-FR': 'fr' },
+          download_dir: download_dir
+        )
+      end
+
+      # Assert
+      # Note: FastlaneError is the exception raised by UI.user_error!
+      expect { act.call }.to raise_error(FastlaneCore::Interface::FastlaneError, "The parent directory `#{download_dir}` (which contains all the `*.lproj` subdirectories) must already exist")
+    end
+
+    it 'reports if a downloaded file is not a valid `.strings` file' do
+      Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
+        # Arrange
+        stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' }).to_return(body: 'some invalid strings file content')
+        error_messages = []
+        allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
+
+        # Act
+        run_described_fastlane_action(
+          project_url: gp_fake_url,
+          locales: { 'fr-FR': 'fr' },
+          download_dir: tmp_dir
+        )
+
+        # Assert
+        expect(stub).to have_been_made.once
+        file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
+        expect(File).to exist(file)
+        expected_error = 'Property List error: Unexpected character s at line 1 / JSON error: JSON text did not start with array or object and option to allow fragments not set.'
+        expect(error_messages.count).to eq(1)
+        expect(error_messages.first).to start_with("Error while validating the file exported from GlotPress (`#{file}`) - #{file}: #{expected_error}") # Different versions of `plutil` might append the line/column as well, but not all.
+      end
+    end
+
+    it 'reports if a downloaded file has empty translations' do
+      Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
+        # Arrange
+        stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' })
+               .to_return(body: ['"key1" = "value1";', '"key2" = "";', '"key3" = "";', '/* translators: use "" quotes please */', '"key4" = "value4";'].join("\n"))
+        error_messages = []
+        allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
+
+        # Act
+        run_described_fastlane_action(
+          project_url: gp_fake_url,
+          locales: { 'fr-FR': 'fr' },
+          download_dir: tmp_dir
+        )
+
+        # Assert
+        expect(stub).to have_been_made.once
+        file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
+        expect(File).to exist(file)
+        expected_error = <<~MSG.chomp
+          Found empty translations in `#{file}` for the following keys: ["key2", "key3"].
+          This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.
+          Please report this to the GlotPress team, and fix the file locally before continuing.
+        MSG
+        expect(error_messages).to eq([expected_error])
+      end
+    end
+
+    it 'does not report invalid downloaded files if `skip_file_validation:true`' do
+      Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
+        # Arrange
+        stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' }).to_return(body: 'some invalid strings file content')
+        error_messages = []
+        allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
+
+        # Act
+        act = lambda do
+          run_described_fastlane_action(
+            project_url: gp_fake_url,
+            locales: { 'fr-FR': 'fr' },
+            download_dir: tmp_dir,
+            skip_file_validation: true
+          )
+        end
+
+        # Assert
+        expect { act.call }.not_to raise_error
+        expect(stub).to have_been_made.once
+        file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
+        expect(File).to exist(file)
+        expect(error_messages).to eq([])
+      end
+    end
+  end
+end