From aa53bb97537be7d8e643a954afed234b74a812db Mon Sep 17 00:00:00 2001 From: Sam Vevang Date: Wed, 26 Jul 2023 14:23:56 -0500 Subject: [PATCH 1/2] Facility to ignore 404 --- app/models/apple/api.rb | 32 +++++++++++++++-------- app/models/apple/podcast_delivery_file.rb | 2 +- test/models/apple/api_test.rb | 32 +++++++++++++++++++++++ 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/app/models/apple/api.rb b/app/models/apple/api.rb index 2c6f746db..0e3d16067 100644 --- a/app/models/apple/api.rb +++ b/app/models/apple/api.rb @@ -3,7 +3,7 @@ module Apple class Api ERROR_RETRIES = 3 - SUCCESS_CODES = %w[200 201].freeze + SUCCESS_CODES = [200, 201].freeze DEFAULT_BATCH_SIZE = 5 DEFAULT_WRITE_BATCH_SIZE = 1 @@ -155,8 +155,10 @@ def check_row(row_operation) true end - def ok_code(resp) - SUCCESS_CODES.include?(resp.code) + def ok_code(resp, ignore_not_found: false) + return true if ignore_not_found && resp.code.to_i == 404 + + SUCCESS_CODES.include?(resp.code.to_i) end def log_response_error(resp) @@ -188,13 +190,13 @@ def response(resp) resp end - def unwrap_response(resp) - raise Apple::ApiError.new("Apple Api Error", resp) unless ok_code(resp) + def unwrap_response(resp, ignore_not_found: false) + raise Apple::ApiError.new("Apple Api Error", resp) unless ok_code(resp, ignore_not_found: ignore_not_found) JSON.parse(resp.body) end - def unwrap_bridge_response(resp) + def unwrap_bridge_response(resp, ignore_not_found: false) raise Apple::ApiError.new("Apple Api Bridge Error", resp) unless ok_code(resp) parsed = JSON.parse(resp.body) @@ -205,7 +207,15 @@ def unwrap_bridge_response(resp) parsed.select { |row_operation| row_operation["api_response"][key] == true } end - (fixed_errs, remaining_errors) = yield(errs) + # ignore 404s if requested + errs = errs.reject { |err| err.dig("api_response", "val", "data", "status") == 404 } if ignore_not_found + + (fixed_errs, remaining_errors) = + if block_given? + yield(errs) + else + [[], errs] + end [oks + fixed_errs, remaining_errors] end @@ -231,16 +241,16 @@ def bridge_remote_and_unwrap(bridge_resource, bridge_options, batch_size: DEFAUL unwrap_bridge_response(resp, &block) end - def bridge_remote_and_retry(bridge_resource, bridge_options, batch_size: DEFAULT_BATCH_SIZE) + def bridge_remote_and_retry(bridge_resource, bridge_options, batch_size: DEFAULT_BATCH_SIZE, ignore_not_found: false) resp = bridge_remote(bridge_resource, bridge_options, batch_size: batch_size) - unwrap_bridge_response(resp) do |row_operation_errors| + unwrap_bridge_response(resp, ignore_not_found: ignore_not_found) do |row_operation_errors| retry_bridge_api_operation(bridge_resource, [], row_operation_errors) end end - def bridge_remote_and_retry!(bridge_resource, bridge_options, batch_size: DEFAULT_BATCH_SIZE) - (oks, errs) = bridge_remote_and_retry(bridge_resource, bridge_options, batch_size: batch_size) + def bridge_remote_and_retry!(bridge_resource, bridge_options, **args) + (oks, errs) = bridge_remote_and_retry(bridge_resource, bridge_options, **args) raise_bridge_api_error(errs) if errs.present? oks diff --git a/app/models/apple/podcast_delivery_file.rb b/app/models/apple/podcast_delivery_file.rb index b71b0230a..4e9c4a378 100644 --- a/app/models/apple/podcast_delivery_file.rb +++ b/app/models/apple/podcast_delivery_file.rb @@ -161,7 +161,7 @@ def self.get_podcast_delivery_files(api, pdfs) def self.get_podcast_delivery_files_via_deliveries(api, podcast_deliveries) delivery_files_response = api.bridge_remote_and_retry!("getPodcastDeliveryFiles", - get_delivery_podcast_delivery_files_bridge_params(podcast_deliveries), batch_size: 1) + get_delivery_podcast_delivery_files_bridge_params(podcast_deliveries), batch_size: 1, ignore_not_found: true) # Rather than mangling and persisting the enumerated view of the delivery files from the podcast delivery # Instead, re-fetch the podcast delivery file from the non-list podcast delivery file resource diff --git a/test/models/apple/api_test.rb b/test/models/apple/api_test.rb index 60120a99a..f3c7cfe39 100644 --- a/test/models/apple/api_test.rb +++ b/test/models/apple/api_test.rb @@ -193,4 +193,36 @@ api.stub(:log_response_error, ->(**) { raise "should not be called" }) { api.response(http_response) } end end + + describe "#unwrap_bridge_response" do + let(:ok_row) { {api_response: {ok: true, err: false, val: {data: {status: 200}}}}.with_indifferent_access } + let(:not_found_row) { {api_response: {ok: false, err: true, val: {data: {status: 404}}}}.with_indifferent_access } + + it "handles strings or integers" do + assert_equal [[], []], api.unwrap_bridge_response(OpenStruct.new(code: "200", body: [].to_json)) + assert_equal [[], []], api.unwrap_bridge_response(OpenStruct.new(code: 200, body: [].to_json)) + end + + it "groups 'ok' data into the ok response array" do + assert_equal [[ok_row], []], api.unwrap_bridge_response(OpenStruct.new(code: "201", body: [ok_row].to_json)) + end + + it "groups 'err' data into the err response array" do + assert_equal [[], [not_found_row]], api.unwrap_bridge_response(OpenStruct.new(code: "201", body: [not_found_row].to_json)) + end + + it "raises for a 4xx and 5xx _bridge_ reponse" do + # it raises Apple::ApiError + assert_raises Apple::ApiError do + api.unwrap_bridge_response(OpenStruct.new(code: "404", body: [].to_json)) + end + assert_raises Apple::ApiError do + api.unwrap_bridge_response(OpenStruct.new(code: "500", body: [].to_json)) + end + end + + it "can optionally treat a 404 as ok, excising the missing responses" do + assert_equal [[ok_row], []], api.unwrap_bridge_response(OpenStruct.new(code: "200", body: [not_found_row, ok_row].to_json), ignore_not_found: true) + end + end end From 01386d444a13181efb64712d2f46df17cb8b30c9 Mon Sep 17 00:00:00 2001 From: Sam Vevang Date: Wed, 26 Jul 2023 14:33:19 -0500 Subject: [PATCH 2/2] Set up left joins on api results filtering 404s --- app/models/apple/podcast_delivery_file.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/apple/podcast_delivery_file.rb b/app/models/apple/podcast_delivery_file.rb index 4e9c4a378..fa460b269 100644 --- a/app/models/apple/podcast_delivery_file.rb +++ b/app/models/apple/podcast_delivery_file.rb @@ -166,8 +166,9 @@ def self.get_podcast_delivery_files_via_deliveries(api, podcast_deliveries) # Rather than mangling and persisting the enumerated view of the delivery files from the podcast delivery # Instead, re-fetch the podcast delivery file from the non-list podcast delivery file resource formatted_bridge_params = - join_on(PODCAST_DELIVERY_ID_ATTR, podcast_deliveries, delivery_files_response) - .map do |(podcast_delivery, row)| + join_on(PODCAST_DELIVERY_ID_ATTR, podcast_deliveries, delivery_files_response, left_join: true).map do |(podcast_delivery, row)| + next if row.nil? + podcast_delivery_files_ids = row["api_response"]["val"]["data"].map do |podcast_delivery_file_data| podcast_delivery_file_data["id"] @@ -179,8 +180,9 @@ def self.get_podcast_delivery_files_via_deliveries(api, podcast_deliveries) end end .flatten + .compact - api.bridge_remote_and_retry!("getPodcastDeliveryFiles", formatted_bridge_params, batch_size: 1) + api.bridge_remote_and_retry!("getPodcastDeliveryFiles", formatted_bridge_params, batch_size: 1, ignore_not_found: true) end # Map across the podcast deliveries and get the bridge params for each