From e40bf5d228c9c9813872e4de5fa4727a424228eb Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 May 2022 15:18:07 +1000 Subject: [PATCH 1/5] Add trailing slash to GlotPress iOS `export-translations` URL This will avoid doubling up each request we make. More details at https://wordpress.slack.com/archives/C02RP50LK/p1651640726217119?thread_ts=1651607938.132789&cid=C02RP50LK --- .../wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb | 2 +- spec/ios_download_strings_files_from_glotpress_spec.rb | 2 +- spec/ios_l10n_helper_spec.rb | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index ad2e20280..a4cf5d841 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -143,7 +143,7 @@ def self.generate_strings_file_from_hash(translations:, output_path:) # 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)}") + 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 diff --git a/spec/ios_download_strings_files_from_glotpress_spec.rb b/spec/ios_download_strings_files_from_glotpress_spec.rb index 45fb23749..3d82f343c 100644 --- a/spec/ios_download_strings_files_from_glotpress_spec.rb +++ b/spec/ios_download_strings_files_from_glotpress_spec.rb @@ -7,7 +7,7 @@ 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) + stub_request(:get, "#{gp_fake_url}/#{locale}/default/export-translations/").with(query: query) end describe 'downloading export files from GlotPress' do diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index 812636a6f..7c0371c75 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -190,7 +190,7 @@ def file_encoding(path) describe 'request query parameters' do it 'passes the expected params when no filters are provided' do # Arrange - stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations").with(query: { format: 'strings' }).to_return(body: 'content') + stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations/").with(query: { format: 'strings' }).to_return(body: 'content') dest = StringIO.new # Act described_class.download_glotpress_export_file(project_url: gp_fake_url, locale: 'fr', filters: nil, destination: dest) @@ -201,7 +201,7 @@ def file_encoding(path) it 'passes the expected params when a list of filters is provided' do # Arrange - stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations") + stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations/") .with(query: { format: 'strings', 'filters[status]': 'current', 'filters[term]': 'foobar' }).to_return(body: 'content') dest = StringIO.new # Act @@ -221,7 +221,7 @@ def file_encoding(path) # Note: in practice it seems that GlotPress's `.strings` exports are using UTF-8 (but served as `application/octet-stream`) # but it does not hurt to ensure the download to a file can work with UTF-16 (and copy the binary stream verbatim) body = File.read(fixture('Localizable-utf16.strings')) - stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations").with(query: { format: 'strings' }).to_return(body: body) + stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations/").with(query: { format: 'strings' }).to_return(body: body) dest = File.join(tmp_dir, 'export.strings') # Act described_class.download_glotpress_export_file(project_url: gp_fake_url, locale: 'fr', filters: nil, destination: dest) @@ -236,7 +236,7 @@ def file_encoding(path) describe 'invalid parameters' do it 'prints an `UI.error` if passed a non-existing locale (or any other 404)' do # Arrange - stub = stub_request(:get, "#{gp_fake_url}/invalid/default/export-translations").with(query: { format: 'strings' }).to_return(status: [404, 'Not Found']) + 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) } dest = StringIO.new @@ -249,7 +249,7 @@ def file_encoding(path) it 'prints an `UI.error` if the destination cannot be written to' do # Arrange - stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations").with(query: { format: 'strings' }).to_return(body: 'content') + stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations/").with(query: { format: 'strings' }).to_return(body: 'content') error_messages = [] allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) } dest = '/these/are/not/the/droids/you/are/looking/for.strings' From 10839b74bcd71f7d5c5b5dd1437eb6f8d1af05f1 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 May 2022 15:43:58 +1000 Subject: [PATCH 2/5] Set specific User Agent when downloading iOS translations from GlotPress This way, people looking at the request logs (and machines, too, I hope) will know it is a legit source that's making the requests. The value originally suggested was `WordPress iOS App Release Packager; https://apps.wordpress.com/ ` but I chose a more generic one because this tooling is used for more than WordPress alone. See https://wordpress.slack.com/archives/C02RP50LK/p1651640726217119?thread_ts=1651607938.132789&cid=C02RP50LK --- .../helper/ios/ios_l10n_helper.rb | 6 +++++- spec/ios_l10n_helper_spec.rb | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index a4cf5d841..aff9f1691 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -144,8 +144,12 @@ def self.generate_strings_file_from_hash(translations:, output_path:) 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)}") + + # Set an unambiguous User Agent so GlotPress won't rate-limit us + options = { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + begin - IO.copy_stream(uri.open, destination) + IO.copy_stream(uri.open(options), destination) rescue StandardError => e UI.error "Error downloading locale `#{locale}` — #{e.message}" return nil diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index 7c0371c75..ef971b945 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -187,6 +187,27 @@ def file_encoding(path) describe '#download_glotpress_export_file' do let(:gp_fake_url) { 'https://stub.glotpress.com/rspec-fake-project' } + it 'sets a predefined User Agent so GlotPress will not rate-limit us' do + # Arrange + stub = stub_request(:get, "#{gp_fake_url}/fr/default/export-translations/") + .with( + query: { format: 'strings' }, + # Note that the syntax below merely checks that the given headers are present in the request, + # it does not restrict the request to have only those headers. + # + # See: + # - https://github.com/bblimke/webmock/tree/33d8810c2828fc17010e15cc3f21ad2c726a966f#matching-requests + # - https://github.com/bblimke/webmock/issues/276#issuecomment-28625436 + headers: { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + ).to_return(body: 'content') + dest = StringIO.new + # Act + described_class.download_glotpress_export_file(project_url: gp_fake_url, locale: 'fr', filters: nil, destination: dest) + # Assert + expect(stub).to have_been_made.once + expect(dest.string).to eq('content') + end + describe 'request query parameters' do it 'passes the expected params when no filters are provided' do # Arrange From 449159d0f948749961d175972489e73ca37b186e Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 May 2022 15:51:08 +1000 Subject: [PATCH 3/5] Add trailing slash to GlotPress Android `export-translations` URL This will avoid doubling up each request we make. More details at https://wordpress.slack.com/archives/C02RP50LK/p1651640726217119?thread_ts=1651607938.132789&cid=C02RP50LK --- .../helper/android/android_localize_helper.rb | 2 +- spec/android_localize_helper_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb index a4c16d6ed..7d29426f6 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb @@ -282,7 +282,7 @@ def self.download_from_glotpress(res_dir:, glotpress_project_url:, glotpress_fil # def self.download_glotpress_export_file(project_url:, locale:, filters:) query_params = filters.transform_keys { |k| "filters[#{k}]" }.merge(format: 'android') - uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations?#{URI.encode_www_form(query_params)}") + uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") begin uri.open { |f| Nokogiri::XML(f.read.gsub("\t", ' '), nil, Encoding::UTF_8.to_s) } rescue StandardError => e diff --git a/spec/android_localize_helper_spec.rb b/spec/android_localize_helper_spec.rb index b470bb760..480357f84 100644 --- a/spec/android_localize_helper_spec.rb +++ b/spec/android_localize_helper_spec.rb @@ -61,7 +61,7 @@ def generated_file(code) Dir[File.join(stubs_dir, '*.xml')].each do |path| # Each file in stubs_dir is a `{locale_code}.xml` whose content is what we want to use as stub for glotpress requests to `locale_code` locale_code = File.basename(path, '.xml') - url = "#{gp_fake_url.chomp('/')}/#{locale_code}/default/export-translations?filters%5Bstatus%5D=current&format=android" + url = "#{gp_fake_url.chomp('/')}/#{locale_code}/default/export-translations/?filters%5Bstatus%5D=current&format=android" stub_request(:get, url).to_return(status: 200, body: File.read(path)) end @@ -215,7 +215,7 @@ def generated_file(code) # Arrange: Prepare request stubs custom_gp_urls = TEST_LOCALES_MAP.map do |locale| - "#{gp_fake_url.chomp('/')}/#{locale[:glotpress]}/default/export-translations?filters%5Bstatus%5D=custom-status&filters%5Bwarnings%5D=yes&format=android" + "#{gp_fake_url.chomp('/')}/#{locale[:glotpress]}/default/export-translations/?filters%5Bstatus%5D=custom-status&filters%5Bwarnings%5D=yes&format=android" end custom_gp_urls.each do |url| stub_request(:get, url) @@ -244,7 +244,7 @@ def generated_file(code) statuses = %w[current waiting fuzzy] statuses.each do |status| stub_path = File.join(fixtures_dir, 'filters', "#{status}.xml") - stub_request(:get, "#{gp_fake_url.chomp('/')}/fakegploc/default/export-translations?filters%5Bstatus%5D=#{status}&format=android") + stub_request(:get, "#{gp_fake_url.chomp('/')}/fakegploc/default/export-translations/?filters%5Bstatus%5D=#{status}&format=android") .to_return(status: 200, body: File.read(stub_path)) end From faf730f55fa943f3d57468b1095d40f926eb36eb Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 May 2022 16:30:43 +1000 Subject: [PATCH 4/5] Set specific User Agent when downloading Android translations from GlotPress This way, people looking at the request logs (and machines, too, I hope) will know it is a legit source that's making the requests. The value originally suggested was `WordPress iOS App Release Packager; https://apps.wordpress.com/ ` but I chose a more generic one because this tooling is used for more than WordPress alone. See https://wordpress.slack.com/archives/C02RP50LK/p1651640726217119?thread_ts=1651607938.132789&cid=C02RP50LK --- .../helper/android/android_localize_helper.rb | 6 +++- spec/android_localize_helper_spec.rb | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb index 7d29426f6..262e073fe 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb @@ -283,8 +283,12 @@ def self.download_from_glotpress(res_dir:, glotpress_project_url:, glotpress_fil def self.download_glotpress_export_file(project_url:, locale:, filters:) query_params = filters.transform_keys { |k| "filters[#{k}]" }.merge(format: 'android') uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") + + # Set an unambiguous User Agent so GlotPress won't rate-limit us + options = { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + begin - uri.open { |f| Nokogiri::XML(f.read.gsub("\t", ' '), nil, Encoding::UTF_8.to_s) } + uri.open(options) { |f| Nokogiri::XML(f.read.gsub("\t", ' '), nil, Encoding::UTF_8.to_s) } rescue StandardError => e UI.error "Error downloading #{locale} - #{e.message}" return nil diff --git a/spec/android_localize_helper_spec.rb b/spec/android_localize_helper_spec.rb index 480357f84..4bbc16d9f 100644 --- a/spec/android_localize_helper_spec.rb +++ b/spec/android_localize_helper_spec.rb @@ -264,5 +264,36 @@ def generated_file(code) expect(File.read(generated_file_path)).to eq(expected_merged_content) end end + + it 'sets a predefined User Agent so GlotPress will not rate-limit us' do + # Arrange + # + # Note that in this test we don't care about what is downloaded or how it's processed, + # only whether the request is made with the expected User Agent. + # Therefore there is only minimum fixtures setup, unlike in the examples before + FileUtils.mkdir_p(File.dirname(generated_file(nil))) + FileUtils.cp(expected_file(nil), generated_file(nil)) + + stub = stub_request(:get, "#{gp_fake_url}fr/default/export-translations/") + .with( + query: { format: 'android', 'filters[status]': 'current' }, + # Note that the syntax below merely checks that the given headers are present in the request, + # it does not restrict the request to have only those headers. + # + # See: + # - https://github.com/bblimke/webmock/tree/33d8810c2828fc17010e15cc3f21ad2c726a966f#matching-requests + # - https://github.com/bblimke/webmock/issues/276#issuecomment-28625436 + headers: { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + ).to_return(status: 200, body: '') + + # Act + described_class.download_from_glotpress( + res_dir: tmpdir, + glotpress_project_url: gp_fake_url, + locales_map: TEST_LOCALES_MAP.select { |l| l[:android] == 'fr' } # only run for `fr` because we only stubbed that + ) + + expect(stub).to have_been_made.once + end end end From 1995bdd65158e0d1ab6573ba8c3dba2b12843503 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Wed, 4 May 2022 16:40:15 +1000 Subject: [PATCH 5/5] Extract shared User Agent value in constant --- .../helper/android/android_localize_helper.rb | 2 +- .../plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb | 2 +- lib/fastlane/plugin/wpmreleasetoolkit/helper/user_agent.rb | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 lib/fastlane/plugin/wpmreleasetoolkit/helper/user_agent.rb diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb index 262e073fe..beb99605a 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb @@ -285,7 +285,7 @@ def self.download_glotpress_export_file(project_url:, locale:, filters:) uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") # Set an unambiguous User Agent so GlotPress won't rate-limit us - options = { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + options = { 'User-Agent' => Wpmreleasetoolkit::USER_AGENT } begin uri.open(options) { |f| Nokogiri::XML(f.read.gsub("\t", ' '), nil, Encoding::UTF_8.to_s) } diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index aff9f1691..f09bbbc44 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -146,7 +146,7 @@ def self.download_glotpress_export_file(project_url:, locale:, filters:, destina uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations/?#{URI.encode_www_form(query_params)}") # Set an unambiguous User Agent so GlotPress won't rate-limit us - options = { 'User-Agent' => 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/' } + options = { 'User-Agent' => Wpmreleasetoolkit::USER_AGENT } begin IO.copy_stream(uri.open(options), destination) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/user_agent.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/user_agent.rb new file mode 100644 index 000000000..fa74cfd49 --- /dev/null +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/user_agent.rb @@ -0,0 +1,5 @@ +module Fastlane + module Wpmreleasetoolkit + USER_AGENT = 'Automattic App Release Automator; https://github.com/wordpress-mobile/release-toolkit/'.freeze + end +end