Share GlotPress 429 auto retry from ASC metadata to iOS strings download

.ruby-version

@@ -1 +1 @@
-2.7.4
+3.2.2

Gemfile.lock

@@ -29,7 +29,7 @@ GEM
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
       tzinfo (~> 2.0)
-    addressable (2.8.4)
+    addressable (2.8.5)
       public_suffix (>= 2.0.2, < 6.0)
     algoliasearch (1.27.5)
       httpclient (~> 2.8, >= 2.8.3)
@@ -320,7 +320,7 @@ GEM
       trailblazer-option (>= 0.1.1, < 0.2.0)
       uber (< 0.2.0)
     retriable (3.1.2)
-    rexml (3.2.5)
+    rexml (3.2.6)
     rmagick (4.3.0)
     rouge (2.0.7)
     rspec (3.12.0)
@@ -392,7 +392,7 @@ GEM
       unf_ext
     unf_ext (0.0.8.2)
     unicode-display_width (2.4.2)
-    webmock (3.18.1)
+    webmock (3.19.1)
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)

lib/fastlane/plugin/wpmreleasetoolkit/helper/glotpress_downloader.rb

@@ -0,0 +1,52 @@
+require 'net/http'
+
+module Fastlane
+  module Helper
+    class GlotpressDownloader
+      AUTO_RETRY_SLEEP_TIME = 20
+      MAX_AUTO_RETRY_ATTEMPTS = 30
+
+      def initialize(
+        auto_retry: true,
+        auto_retry_sleep_time: 20,
+        auto_retry_max_attempts: 30
+      )
+        @auto_retry = auto_retry
+        @auto_retry_sleep_time = auto_retry_sleep_time
+        @auto_retry_max_attempts = auto_retry_max_attempts
+        @auto_retry_attempt_counter = 0
+      end
+
+      def download(glotpress_url, options = {})
+        uri = URI(glotpress_url)
+        response = Net::HTTP.get_response(uri, options)
+
+        case response.code
+        when '200' # All good pass the result along
+          response
+        when '301' # Follow the redirect
+          UI.message("Received 301 for `#{response.uri}`. Following redirect...")
+          download(response.header['location'])
+        when '429' # We got rate-limited, auto_retry or offer to try again with a prompt
+          if @auto_retry
+            if @auto_retry_attempt_counter < @auto_retry_max_attempts
+              UI.message("Received 429 for `#{response.uri}`. Auto retrying in #{@auto_retry_sleep_time} seconds...")
+              sleep(@auto_retry_sleep_time)
+              @auto_retry_attempt_counter += 1
+              download(response.uri)
+            else
+              UI.error("Abandoning `#{response.uri}` download after #{@auto_retry_attempt_counter} retries.")
+            end
+          elsif UI.confirm("Retry downloading `#{response.uri}` after receiving 429 from the API?")
+            download(response.uri)
+          else
+            UI.error("Abandoning `#{response.uri}` download as requested.")
+          end
+        else
+          message = "Received unexpected #{response.code} from request to URI #{response.uri}."
+          UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?")
+        end
+      end
+    end
+  end
+end

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

@@ -161,19 +161,34 @@ def self.generate_strings_file_from_hash(translations:, output_path:)
         #                               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:)
+        def self.download_glotpress_export_file(
+          project_url:,
+          locale:,
+          filters:,
+          destination:,
+          autoretry: true,
+          autoretry_count: 0,
+          autoretry_max: Fastlane::Helper::GlotpressDownloader::MAX_AUTO_RETRY_ATTEMPTS,
+          autoretry_sleep: Fastlane::Helper::GlotpressDownloader::AUTO_RETRY_SLEEP_TIME
+        )
           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)}")
 
+          downloader = Fastlane::Helper::GlotpressDownloader.new(
+            auto_retry: autoretry,
+            auto_retry_sleep_time: autoretry_sleep,
+            auto_retry_max_attempts: autoretry_max
+          )
+
           # Set an unambiguous User Agent so GlotPress won't rate-limit us
           options = { 'User-Agent' => Wpmreleasetoolkit::USER_AGENT }
 
+          response = downloader.download(uri.to_s, options)
+
           begin
-            IO.copy_stream(uri.open(options), destination)
+            IO.copy_stream(StringIO.new(response.body), destination)
           rescue StandardError => e
-            UI.error "Error downloading locale `#{locale}` — #{e.message} (#{uri})"
-            retry if e.is_a?(OpenURI::HTTPError) && UI.confirm("Retry downloading `#{locale}`?")
-            return nil
+            UI.error "Error saving locale `#{locale}` — #{e.message} (#{uri})"
           end
         end
       end

lib/fastlane/plugin/wpmreleasetoolkit/helper/metadata_download_helper.rb

@@ -4,23 +4,24 @@
 module Fastlane
   module Helper
     class MetadataDownloader
-      AUTO_RETRY_SLEEP_TIME = 20
-      MAX_AUTO_RETRY_ATTEMPTS = 30
-
       attr_reader :target_folder, :target_files
 
-      def initialize(target_folder, target_files, auto_retry)
+      def initialize(target_folder, target_files, auto_retry, auto_retry_sleep_time = 20, auto_retry_max_attempts = 30)
         @target_folder = target_folder
         @target_files = target_files
         @auto_retry = auto_retry
+        @auto_retry_sleep_time = auto_retry_sleep_time
         @alternates = {}
         @auto_retry_attempt_counter = 0
       end
 
       # Downloads data from GlotPress, in JSON format
       def download(target_locale, glotpress_url, is_source)
-        uri = URI(glotpress_url)
-        response = Net::HTTP.get_response(uri)
+        downloader = GlotpressDownloader.new(
+          auto_retry: @auto_retry,
+          auto_retry_sleep_time: @auto_retry_sleep_time
+        )
+        response = downloader.download(glotpress_url)
         handle_glotpress_download(response: response, locale: target_locale, is_source: is_source)
       end
 
@@ -112,22 +113,6 @@ def handle_glotpress_download(response:, locale:, is_source:)
           loc_data = JSON.parse(response.body) rescue loc_data = nil
           parse_data(locale, loc_data, is_source)
           reparse_alternates(target_locale, loc_data, is_source) unless @alternates.empty?
-        when '301'
-          # Follow the redirect
-          UI.message("Received 301 for `#{locale}`. Following redirect...")
-          download(locale, response.header['location'], is_source)
-        when '429'
-          # We got rate-limited, auto_retry or offer to try again with a prompt
-          if @auto_retry && @auto_retry_attempt_counter <= MAX_AUTO_RETRY_ATTEMPTS
-            UI.message("Received 429 for `#{locale}`. Auto retrying in #{AUTO_RETRY_SLEEP_TIME} seconds...")
-            sleep(AUTO_RETRY_SLEEP_TIME)
-            @auto_retry_attempt_counter += 1
-            download(locale, response.uri, is_source)
-          elsif UI.confirm("Retry downloading `#{locale}` after receiving 429 from the API?")
-            download(locale, response.uri, is_source)
-          else
-            UI.error("Abandoning `#{locale}` download as requested.")
-          end
         else
           message = "Received unexpected #{response.code} from request to URI #{response.uri}."
           UI.abort_with_message!(message) unless UI.confirm("#{message} Continue anyway?")

spec/glotpress_downloader_spec.rb

@@ -0,0 +1,116 @@
+require 'spec_helper'
+
+describe Fastlane::Helper::GlotpressDownloader do
+  describe 'downloading' do
+    context 'when auto retry is enabled' do
+      context 'when GlotPress returs a 200 code' do
+        it 'returns the response, without retrying' do
+          downloader = described_class.new(auto_retry: true)
+          fake_url = 'https://test.com'
+
+          stub_request(:get, fake_url).to_return(status: 200, body: 'OK')
+
+          response = downloader.download(fake_url)
+
+          expect(response.code).to eq('200')
+          expect(response.body).to eq('OK')
+          # Make sure there was no retry
+          assert_requested(:get, fake_url, times: 1)
+        end
+      end
+
+      context 'when GlotPress returs a 429 code' do
+        it 'retries automatically' do
+          sleep_time = 0.1
+          downloader = described_class.new(auto_retry: true, auto_retry_sleep_time: sleep_time)
+          fake_url = 'https://test.com'
+
+          count = 0
+          stub_request(:get, fake_url).to_return do
+            count += 1
+            if count == 1
+              { status: 429, body: 'Too Many Requests' }
+            else
+              { status: 200, body: 'OK' }
+            end
+          end
+
+          expect(Fastlane::UI).to receive(:message)
+            .with(/Received 429 for `#{fake_url}`. Auto retrying in #{sleep_time} seconds.../)
+
+          response = downloader.download(fake_url)
+
+          assert_requested(:get, fake_url, times: 2)
+          expect(response.code).to eq('200')
+        end
+
+        context 'when the maximum number of retries is reached' do
+          it 'aborts' do
+            sleep_time = 0.1
+            max_retries = 3
+            downloader = described_class.new(auto_retry: true, auto_retry_sleep_time: sleep_time, auto_retry_max_attempts: max_retries)
+            fake_url = 'https://test.com'
+
+            count = 0
+            stub_request(:get, fake_url).to_return do
+              count += 1
+              { status: 429, body: 'Too Many Requests' }
+            end
+
+            expect(Fastlane::UI).to receive(:message)
+              .with(/Received 429 for `#{fake_url}`. Auto retrying in #{sleep_time} seconds.../)
+              .exactly(max_retries).times
+
+            expect(Fastlane::UI).to receive(:error)
+              .with(/Abandoning `#{fake_url}` download after #{max_retries} retries./)
+
+            downloader.download(fake_url)
+
+            assert_requested(:get, fake_url, times: max_retries + 1) # the original request plus the retries
+          end
+        end
+      end
+    end
+
+    context 'when auto retry is disabled' do
+      context 'when GlotPress returs a 200 code' do
+        it 'returns the response, without retrying' do
+          downloader = described_class.new(auto_retry: false)
+          fake_url = 'https://test.com'
+
+          stub_request(:get, fake_url).to_return(status: 200, body: 'OK')
+
+          response = downloader.download(fake_url)
+
+          expect(response.code).to eq('200')
+          expect(response.body).to eq('OK')
+          # Make sure there was no retry
+          assert_requested(:get, fake_url, times: 1)
+        end
+      end
+
+      context 'when GlotPress returs a 429 code' do
+        it 'prompt the user for confirmation, ignoring the max auto retry parameter' do
+          sleep_time = 0.1
+          max_retries = 2
+          downloader = described_class.new(auto_retry: false, auto_retry_sleep_time: sleep_time, auto_retry_max_attempts: max_retries)
+          fake_url = 'https://test.com'
+
+          stub_request(:get, fake_url).to_return(status: 429, body: 'Too Many Requests')
+
+          count = 0
+          allow(Fastlane::UI).to receive(:confirm).with("Retry downloading `#{fake_url}` after receiving 429 from the API?") do
+            count += 1
+            # Simulate user retrying more times that the max retries, then aborting
+            count <= max_retries
+          end
+
+          # When the user aborts, with finish with an error
+          expect(Fastlane::UI).to receive(:error).with(/Abandoning `#{fake_url}` download as requested./)
+
+          downloader.download(fake_url)
+        end
+      end
+    end
+  end
+end

spec/ios_l10n_helper_spec.rb

@@ -259,14 +259,14 @@ def file_encoding(path)
         # Arrange
         stub = stub_request(:get, "#{gp_fake_url}/invalid/default/export-translations/").with(query: { format: 'strings' }).to_return(status: [404, 'Not Found'])
         error_messages = []
-        allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
+        allow(FastlaneCore::UI).to receive(:abort_with_message!) { |message| error_messages.append(message) }
         allow(FastlaneCore::UI).to receive(:confirm).and_return(false) # as we will be asked if we want to retry when getting a network error
         dest = StringIO.new
         # Act
         described_class.download_glotpress_export_file(project_url: gp_fake_url, locale: 'invalid', filters: nil, destination: dest)
         # Assert
         expect(stub).to have_been_made.once
-        expect(error_messages).to eq(["Error downloading locale `invalid` — 404 Not Found (#{gp_fake_url}/invalid/default/export-translations/?format=strings)"])
+        expect(error_messages).to eq(["Error downloading locale `invalid` — Received unexpected 404 from request to URI #{gp_fake_url}/invalid/default/export-translations/?format=strings"])
       end
 
       it 'prints an `UI.error` if the destination cannot be written to' do
@@ -280,7 +280,7 @@ def file_encoding(path)
         # Assert
         expect(stub).to have_been_made.once
         expect(File).not_to exist(dest)
-        expect(error_messages).to eq(["Error downloading locale `fr` — No such file or directory @ rb_sysopen - #{dest} (#{gp_fake_url}/fr/default/export-translations/?format=strings)"])
+        expect(error_messages).to eq(["Error saving locale `fr` — No such file or directory @ rb_sysopen - #{dest} (#{gp_fake_url}/fr/default/export-translations/?format=strings)"])
       end
     end
   end

spec/metadata_download_helper_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Fastlane::Helper::MetadataDownloader do
+  describe 'downloading from GlotPress' do
+    context 'when GlotPress returs a 429 code' do
+      it 'automatically retries' do
+        in_tmp_dir do |tmp_dir|
+          metadata_downloader = described_class.new(
+            tmp_dir,
+            { key: { desc: 'target-file-name.txt' } },
+            true,
+            0.1
+          )
+
+          fake_url = 'https://test.com'
+
+          count = 0
+          stub_request(:get, fake_url).to_return do
+            count += 1
+            if count == 1
+              { status: 429, body: 'Too Many Requests' }
+            else
+              { status: 200, body: 'OK' }
+            end
+          end
+
+          expect(Fastlane::UI).to receive(:message)
+            .with(/Received 429 for `#{fake_url}`. Auto retrying in 0.1 seconds.../)
+
+          expect(Fastlane::UI).to receive(:message)
+            .with(/No translation available for en-AU/)
+
+          expect(Fastlane::UI).to receive(:success)
+            .with(/Successfully downloaded `en-AU`./)
+
+          metadata_downloader.download('en-AU', fake_url, false)
+
+          assert_requested(:get, fake_url, times: 2)
+        end
+      end
+    end
+  end
+end

spec/spec_helper.rb

@@ -103,3 +103,14 @@ def with_tmp_file(named: nil, content: '')
 EMPTY_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'empty.json')
 PASSED_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'firebase', 'firebase-test-lab-run-passed.log')
 FAILED_FIREBASE_TEST_LOG_PATH = File.join(__dir__, 'test-data', 'firebase', 'firebase-test-lab-run-failure.log')
+
+# Monkey-patch WebMock to work around bug where response.uri is not set
+# See https://github.com/bblimke/webmock/issues/469
+WebMock::HttpLibAdapters::NetHttpAdapter.instance_variable_get(:@webMockNetHTTP).class_eval do
+  old_request = instance_method :request
+  define_method :request do |request, &block|
+    old_request.bind(self).call(request, &block).tap do |response|
+      response.uri = request.uri
+    end
+  end
+end