From cf3665ba83fd75f6f45f84025cb6a203fd0391ec Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 8 Feb 2023 11:28:50 -0500 Subject: [PATCH 01/15] implement retry behavior with repetitive code --- .../usps_in_person_proofing/mock/fixtures.rb | 4 + .../request_expired_token_response.json | 5 + .../usps_in_person_proofing/proofer.rb | 50 ++- .../usps_in_person_proofing/proofer_spec.rb | 295 ++++++++++-------- spec/support/usps_ipp_helper.rb | 36 +++ 5 files changed, 252 insertions(+), 138 deletions(-) create mode 100644 app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json diff --git a/app/services/usps_in_person_proofing/mock/fixtures.rb b/app/services/usps_in_person_proofing/mock/fixtures.rb index 51ca638275c..45f7191dd86 100644 --- a/app/services/usps_in_person_proofing/mock/fixtures.rb +++ b/app/services/usps_in_person_proofing/mock/fixtures.rb @@ -5,6 +5,10 @@ def self.internal_server_error_response load_response_fixture('internal_server_error_response.json') end + def self.request_expired_token_response + load_response_fixture('request_expired_token_response.json') + end + def self.request_token_response load_response_fixture('request_token_response.json') end diff --git a/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json b/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json new file mode 100644 index 00000000000..94f268da06c --- /dev/null +++ b/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json @@ -0,0 +1,5 @@ +{ + "token_type": "Bearer", + "access_token": "==PZWyMP2ZHGOIeTd17YomIf7XjZUL4G93dboY1pTsuTJN0s9BwMYvOcIS9B3gRvloK2sroi9uFXdXrFuly7==", + "expires_in": 0 +} diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index a2d6a0a61bd..a08ea064cda 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -18,11 +18,17 @@ def request_facilities(location) zipCode: location.zip_code, }.to_json - facilities = parse_facilities( - faraday.post(url, body, dynamic_headers) do |req| + begin + response = faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_facilities' } + end.body + rescue Faraday::ForbiddenError + retrieve_token! + response = faraday.post(url, body, dynamic_headers) do |req| req.options.context = { service_name: 'usps_facilities' } - end.body, - ) + end.body + end + facilities = parse_facilities(response) dedupe_facilities(facilities) end @@ -56,8 +62,15 @@ def request_enroll(applicant) IPPAssuranceLevel: '1.5', } - res = faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enroll' } + begin + res = faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enroll' } + end + rescue Faraday::ForbiddenError + retrieve_token! + res = faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enroll' } + end end Response::RequestEnrollResponse.new(res.body) end @@ -78,9 +91,15 @@ def request_proofing_results(unique_id, enrollment_code) enrollmentCode: enrollment_code, } - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_proofing_results' } - end.body + begin + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_proofing_results' } + end.body + rescue Faraday::TimeoutError + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_proofing_results' } + end.body + end end # Makes HTTP request to retrieve enrollment code @@ -97,9 +116,16 @@ def request_enrollment_code(unique_id) uniqueID: unique_id, } - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enrollment_code' } - end.body + begin + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enrollment_code' } + end.body + rescue Faraday::ForbiddenError + retrieve_token! + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enrollment_code' } + end.body + end end # Makes a request to retrieve a new OAuth token, caches it, and returns it. Tokens have diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index bca90d0a4dd..31fa6067a23 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -106,9 +106,6 @@ def check_facility(facility) end describe '#request_facilities' do - before do - stub_request_token - end let(:location) do double( 'Location', @@ -118,24 +115,44 @@ def check_facility(facility) zip_code: Faker::Address.zip_code, ) end + context 'when the token is valid' do + before do + stub_request_token + end - it 'returns facilities' do - stub_request_facilities - facilities = subject.request_facilities(location) + it 'returns facilities' do + stub_request_facilities + facilities = subject.request_facilities(location) - check_facility(facilities[0]) + check_facility(facilities[0]) + end + + it 'does not return duplicates' do + stub_request_facilities_with_duplicates + facilities = subject.request_facilities(location) + + expect(facilities.length).to eq(9) + expect( + facilities.count do |post_office| + post_office.address == '3775 INDUSTRIAL BLVD' + end, + ).to eq(1) + end end - it 'does not return duplicates' do - stub_request_facilities_with_duplicates - facilities = subject.request_facilities(location) + context 'when the token is expired' do + before do + stub_expired_request_token + stub_request_facilities_with_expired_token + stub_request_facilities + end + + it 'fetches a new token and retries the attempt' do + facilities = subject.request_facilities(location) - expect(facilities.length).to eq(9) - expect( - facilities.count do |post_office| - post_office.address == '3775 INDUSTRIAL BLVD' - end, - ).to eq(1) + expect(facilities.length).to eq(10) + check_facility(facilities[0]) + end end end @@ -149,10 +166,8 @@ def check_facility(facility) end describe '#request_enroll' do - it 'returns enrollment information' do - stub_request_token - stub_request_enroll - applicant = double( + let(:applicant) do + double( 'applicant', address: Faker::Address.street_address, city: Faker::Address.city, @@ -163,144 +178,172 @@ def check_facility(facility) email: Faker::Internet.safe_email, unique_id: '123456789', ) - - enrollment = subject.request_enroll(applicant) - expect(enrollment.enrollment_code).to be_present - expect(enrollment.response_message).to be_present end + context 'when the token is valid' do + before do + stub_request_token + end + it 'returns enrollment information' do + stub_request_enroll - it 'returns 400 error' do - stub_request_token - stub_request_enroll_bad_request_response - applicant = double( - 'applicant', - address: Faker::Address.street_address, - city: Faker::Address.city, - state: Faker::Address.state_abbr, - zip_code: Faker::Address.zip_code, - first_name: Faker::Name.first_name, - last_name: Faker::Name.last_name, - email: Faker::Internet.safe_email, - unique_id: '123456789', - ) + enrollment = subject.request_enroll(applicant) + expect(enrollment.enrollment_code).to be_present + expect(enrollment.response_message).to be_present + end - expect { subject.request_enroll(applicant) }.to raise_error( - an_instance_of(Faraday::BadRequestError). - and(having_attributes( - response: include( - body: include( - 'responseMessage' => 'Sponsor for sponsorID 5 not found', + it 'returns 400 error' do + stub_request_enroll_bad_request_response + + expect { subject.request_enroll(applicant) }.to raise_error( + an_instance_of(Faraday::BadRequestError). + and(having_attributes( + response: include( + body: include( + 'responseMessage' => 'Sponsor for sponsorID 5 not found', + ), ), - ), - )), - ) - end + )), + ) + end - it 'returns 500 error' do - stub_request_token - stub_request_enroll_internal_server_error_response - applicant = double( - 'applicant', - address: Faker::Address.street_address, - city: Faker::Address.city, - state: Faker::Address.state_abbr, - zip_code: Faker::Address.zip_code, - first_name: Faker::Name.first_name, - last_name: Faker::Name.last_name, - email: Faker::Internet.safe_email, - unique_id: '123456789', - ) + it 'returns 500 error' do + stub_request_enroll_internal_server_error_response - expect { subject.request_enroll(applicant) }.to raise_error( - an_instance_of(Faraday::ServerError). - and(having_attributes( - response: include( - body: include( - 'responseMessage' => 'An internal error occurred processing the request', + expect { subject.request_enroll(applicant) }.to raise_error( + an_instance_of(Faraday::ServerError). + and(having_attributes( + response: include( + body: include( + 'responseMessage' => 'An internal error occurred processing the request', + ), ), - ), - )), - ) + )), + ) + end end - end - describe '#request_proofing_results' do - it 'returns failed enrollment information' do - stub_request_token - stub_request_failed_proofing_results - - applicant = double( - 'applicant', - unique_id: '123456789', - enrollment_code: '123456789', - ) + context 'when the token is expired' do + before do + stub_expired_request_token + stub_request_enroll_expired_token + stub_request_enroll + end - proofing_results = subject.request_proofing_results( - applicant.unique_id, - applicant.enrollment_code, - ) - expect(proofing_results['status']).to eq 'In-person failed' - expect(proofing_results['fraudSuspected']).to eq false + it 'fetches a new token and retries the attempt' do + enrollment = subject.request_enroll(applicant) + expect(enrollment.enrollment_code).to be_present + expect(enrollment.response_message).to be_present + end end + end - it 'returns passed enrollment information' do - stub_request_token - stub_request_passed_proofing_results - - applicant = double( + describe '#request_proofing_results' do + let(:applicant) do + double( 'applicant', unique_id: '123456789', enrollment_code: '123456789', ) - - proofing_results = subject.request_proofing_results( - applicant.unique_id, - applicant.enrollment_code, - ) - expect(proofing_results['status']).to eq 'In-person passed' - expect(proofing_results['fraudSuspected']).to eq false end + context 'when the token is valid' do + before do + stub_request_token + end + it 'returns failed enrollment information' do + stub_request_failed_proofing_results - it 'returns in-progress enrollment information' do - stub_request_token - stub_request_in_progress_proofing_results + proofing_results = subject.request_proofing_results( + applicant.unique_id, + applicant.enrollment_code, + ) + expect(proofing_results['status']).to eq 'In-person failed' + expect(proofing_results['fraudSuspected']).to eq false + end - applicant = double( - 'applicant', - unique_id: '123456789', - enrollment_code: '123456789', - ) + it 'returns passed enrollment information' do + stub_request_passed_proofing_results - expect do - subject.request_proofing_results( + proofing_results = subject.request_proofing_results( applicant.unique_id, applicant.enrollment_code, ) - end.to raise_error( - an_instance_of(Faraday::BadRequestError). - and(having_attributes( - response: include( - body: include( - 'responseMessage' => 'Customer has not been to a post office to complete IPP', + expect(proofing_results['status']).to eq 'In-person passed' + expect(proofing_results['fraudSuspected']).to eq false + end + + it 'returns in-progress enrollment information' do + stub_request_in_progress_proofing_results + + expect do + subject.request_proofing_results( + applicant.unique_id, + applicant.enrollment_code, + ) + end.to raise_error( + an_instance_of(Faraday::BadRequestError). + and(having_attributes( + response: include( + body: include( + 'responseMessage' => 'Customer has not been to a post office to complete IPP', + ), ), - ), - )), - ) + )), + ) + end + end + context 'when the token is expired' do + before do + stub_expired_request_token + stub_request_proofing_results_with_forbidden_error + stub_request_passed_proofing_results + end + + it 'fetches a new token and retries the attempt' do + proofing_results = subject.request_proofing_results( + applicant.unique_id, + applicant.enrollment_code, + ) + + expect(proofing_results['status']).to eq 'In-person passed' + expect(proofing_results['fraudSuspected']).to eq false + end end end describe '#request_enrollment_code' do - it 'returns enrollment information' do - stub_request_token - stub_request_enrollment_code - applicant = double( + let(:applicant) do + double( 'applicant', unique_id: '123456789', ) + end + context 'when the token is valid' do + before do + stub_request_token + end + + it 'returns enrollment information' do + stub_request_enrollment_code - enrollment = subject.request_enrollment_code(applicant) - expect(enrollment['enrollmentCode']).to be_present - expect(enrollment['responseMessage']).to be_present + enrollment = subject.request_enrollment_code(applicant) + expect(enrollment['enrollmentCode']).to be_present + expect(enrollment['responseMessage']).to be_present + end + end + + context 'when the token is expired' do + before do + stub_expired_request_token + end + + it 'fetches a new token and retries the attempt' do + stub_request_enrollment_code_with_forbidden_error + stub_request_enrollment_code + + enrollment = subject.request_enrollment_code(applicant) + expect(enrollment['enrollmentCode']).to be_present + expect(enrollment['responseMessage']).to be_present + end end end end diff --git a/spec/support/usps_ipp_helper.rb b/spec/support/usps_ipp_helper.rb index 5259b7e1487..939b6e71753 100644 --- a/spec/support/usps_ipp_helper.rb +++ b/spec/support/usps_ipp_helper.rb @@ -1,4 +1,12 @@ module UspsIppHelper + def stub_expired_request_token + stub_request(:post, %r{/oauth/authenticate}).to_return( + status: 200, + body: UspsInPersonProofing::Mock::Fixtures.request_expired_token_response, + headers: { 'content-type' => 'application/json' }, + ) + end + def stub_request_token stub_request(:post, %r{/oauth/authenticate}).to_return( status: 200, @@ -23,6 +31,13 @@ def stub_request_facilities_with_duplicates ) end + def stub_request_facilities_with_expired_token + stub_request( + :post, + %r{/ivs-ippaas-api/IPPRest/resources/rest/getIppFacilityList}, + ).to_raise(Faraday::ForbiddenError) + end + def stub_request_enroll stub_request(:post, %r{/ivs-ippaas-api/IPPRest/resources/rest/optInIPPApplicant}).to_return( status: 200, @@ -31,6 +46,13 @@ def stub_request_enroll ) end + def stub_request_enroll_expired_token + stub_request( + :post, + %r{/ivs-ippaas-api/IPPRest/resources/rest/optInIPPApplicant}, + ).to_raise(Faraday::ForbiddenError) + end + def stub_request_enroll_timeout_error stub_request( :post, @@ -196,6 +218,13 @@ def request_in_progress_proofing_results_args } end + def stub_request_proofing_results_with_forbidden_error + stub_request( + :post, + %r{/ivs-ippaas-api/IPPRest/resources/rest/getProofingResults}, + ).to_raise(Faraday::ForbiddenError) + end + def stub_request_proofing_results_with_timeout_error stub_request( :post, @@ -238,4 +267,11 @@ def stub_request_enrollment_code headers: { 'content-type' => 'application/json' }, ) end + + def stub_request_enrollment_code_with_forbidden_error + stub_request( + :post, + %r{/ivs-ippaas-api/IPPRest/resources/rest/requestEnrollmentCode}, + ).to_raise(Faraday::ForbiddenError) + end end From e50ace84848c1fdff95148e945d3ae51ec42d9dd Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 8 Feb 2023 16:00:50 -0500 Subject: [PATCH 02/15] remove begin/rescue blocks and get failing tests --- .../usps_in_person_proofing/proofer.rb | 50 +++++-------------- .../usps_in_person_proofing/proofer_spec.rb | 6 +++ 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index a08ea064cda..0969eff5056 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -18,16 +18,10 @@ def request_facilities(location) zipCode: location.zip_code, }.to_json - begin - response = faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_facilities' } - end.body - rescue Faraday::ForbiddenError - retrieve_token! - response = faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_facilities' } - end.body - end + response = faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_facilities' } + end.body + facilities = parse_facilities(response) dedupe_facilities(facilities) end @@ -62,15 +56,8 @@ def request_enroll(applicant) IPPAssuranceLevel: '1.5', } - begin - res = faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enroll' } - end - rescue Faraday::ForbiddenError - retrieve_token! - res = faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enroll' } - end + res = faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enroll' } end Response::RequestEnrollResponse.new(res.body) end @@ -91,15 +78,9 @@ def request_proofing_results(unique_id, enrollment_code) enrollmentCode: enrollment_code, } - begin - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_proofing_results' } - end.body - rescue Faraday::TimeoutError - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_proofing_results' } - end.body - end + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_proofing_results' } + end.body end # Makes HTTP request to retrieve enrollment code @@ -116,16 +97,9 @@ def request_enrollment_code(unique_id) uniqueID: unique_id, } - begin - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enrollment_code' } - end.body - rescue Faraday::ForbiddenError - retrieve_token! - faraday.post(url, body, dynamic_headers) do |req| - req.options.context = { service_name: 'usps_enrollment_code' } - end.body - end + faraday.post(url, body, dynamic_headers) do |req| + req.options.context = { service_name: 'usps_enrollment_code' } + end.body end # Makes a request to retrieve a new OAuth token, caches it, and returns it. Tokens have diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 31fa6067a23..6ae498f39df 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -148,6 +148,8 @@ def check_facility(facility) end it 'fetches a new token and retries the attempt' do + expect(subject).to receive(:token).exactly(2).times + facilities = subject.request_facilities(location) expect(facilities.length).to eq(10) @@ -230,7 +232,9 @@ def check_facility(facility) end it 'fetches a new token and retries the attempt' do + expect(subject).to receive(:token).exactly(2).times enrollment = subject.request_enroll(applicant) + expect(enrollment.enrollment_code).to be_present expect(enrollment.response_message).to be_present end @@ -340,7 +344,9 @@ def check_facility(facility) stub_request_enrollment_code_with_forbidden_error stub_request_enrollment_code + expect(subject).to receive(:token).exactly(2).times enrollment = subject.request_enrollment_code(applicant) + expect(enrollment['enrollmentCode']).to be_present expect(enrollment['responseMessage']).to be_present end From c5cfe1fb4204b92c8e1a88211c972bd05f318c15 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Thu, 9 Feb 2023 09:38:19 -0500 Subject: [PATCH 03/15] add retry options to faraday. tests still fail. --- app/services/usps_in_person_proofing/proofer.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index 0969eff5056..3c67687e3c0 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -133,6 +133,15 @@ def faraday conn.options.open_timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.write_timeout = IdentityConfig.store.usps_ipp_request_timeout + # If the request fails because the token is expired, get a new token before retrying the request + retry_options = { + retry_statuses: [403], + retry_block: lambda do |env:, options:, retry_count:, exception:, will_retry_in:| + token + end, + } + conn.request :retry, retry_options + # Log request metrics conn.request :instrumentation, name: 'request_metric.faraday' From be8bbcb096f7ba9cd32ad9c2e9e85b49290c8b72 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Thu, 9 Feb 2023 15:36:53 -0500 Subject: [PATCH 04/15] refactor implementation & spec --- app/services/usps_in_person_proofing/proofer.rb | 16 +++++++++------- .../usps_in_person_proofing/proofer_spec.rb | 6 +++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index 3c67687e3c0..e40a4c2d7cd 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -127,19 +127,21 @@ def token private def faraday + # If the request fails because the token is expired, get a new token before retrying request + retry_options = { + exceptions: [Faraday::ForbiddenError], + retry_statuses: [403], + retry_block: lambda do |env:, options:, retry_count:, exception:, will_retry_in:| + token + end, + } + Faraday.new(headers: request_headers) do |conn| conn.options.timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.read_timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.open_timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.write_timeout = IdentityConfig.store.usps_ipp_request_timeout - # If the request fails because the token is expired, get a new token before retrying the request - retry_options = { - retry_statuses: [403], - retry_block: lambda do |env:, options:, retry_count:, exception:, will_retry_in:| - token - end, - } conn.request :retry, retry_options # Log request metrics diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 6ae498f39df..169560adb38 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -148,7 +148,7 @@ def check_facility(facility) end it 'fetches a new token and retries the attempt' do - expect(subject).to receive(:token).exactly(2).times + expect(subject).to receive(:token).twice facilities = subject.request_facilities(location) @@ -232,7 +232,7 @@ def check_facility(facility) end it 'fetches a new token and retries the attempt' do - expect(subject).to receive(:token).exactly(2).times + expect(subject).to receive(:token).twice enrollment = subject.request_enroll(applicant) expect(enrollment.enrollment_code).to be_present @@ -344,7 +344,7 @@ def check_facility(facility) stub_request_enrollment_code_with_forbidden_error stub_request_enrollment_code - expect(subject).to receive(:token).exactly(2).times + expect(subject).to receive(:token).twice enrollment = subject.request_enrollment_code(applicant) expect(enrollment['enrollmentCode']).to be_present From ccce955521e70ffe28a4b3905805c19522bdf6cf Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Fri, 10 Feb 2023 10:01:17 -0500 Subject: [PATCH 05/15] changelog:Bug Fixes, In-person proofing, refresh USPS auth token when request fails From 598c8ed13a0590b7880297590db8869f998d6589 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Fri, 10 Feb 2023 11:05:51 -0500 Subject: [PATCH 06/15] attempt broader retry behavior. tests still fail. --- app/services/usps_in_person_proofing/proofer.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index e40a4c2d7cd..a580c060bb7 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -129,8 +129,6 @@ def token def faraday # If the request fails because the token is expired, get a new token before retrying request retry_options = { - exceptions: [Faraday::ForbiddenError], - retry_statuses: [403], retry_block: lambda do |env:, options:, retry_count:, exception:, will_retry_in:| token end, From 95f6f10e6b286cee46fbd1a998ffbfaafd0a91ed Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Mon, 13 Feb 2023 14:40:37 -0500 Subject: [PATCH 07/15] attempt to refresh token before pinging USPS api. more tests fail --- .../usps_in_person_proofing/proofer.rb | 12 ++----- .../usps_in_person_proofing/proofer_spec.rb | 32 +++++++++++++++++-- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index a580c060bb7..ebc4645a7b7 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -127,21 +127,12 @@ def token private def faraday - # If the request fails because the token is expired, get a new token before retrying request - retry_options = { - retry_block: lambda do |env:, options:, retry_count:, exception:, will_retry_in:| - token - end, - } - Faraday.new(headers: request_headers) do |conn| conn.options.timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.read_timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.open_timeout = IdentityConfig.store.usps_ipp_request_timeout conn.options.write_timeout = IdentityConfig.store.usps_ipp_request_timeout - conn.request :retry, retry_options - # Log request metrics conn.request :instrumentation, name: 'request_metric.faraday' @@ -162,6 +153,9 @@ def faraday # already cached. # @return [Hash] Headers to add to USPS requests def dynamic_headers + if Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) != -2 && Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) <= 0 + retrieve_token! + end { 'Authorization' => token, 'RequestID' => request_id, diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 169560adb38..8941ffab99f 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -141,17 +141,35 @@ def check_facility(facility) end context 'when the token is expired' do + AUTH_TOKEN_CACHE_KEY = :usps_ippaas_api_auth_token + let!(:redis) do + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url) + end before do + redis.write( + AUTH_TOKEN_CACHE_KEY, + UspsInPersonProofing::Mock::Fixtures.request_expired_token_response, expires_in: 0 + ) + allow(Rails).to receive(:cache).and_return( + redis, + ) stub_expired_request_token - stub_request_facilities_with_expired_token stub_request_facilities end - it 'fetches a new token and retries the attempt' do - expect(subject).to receive(:token).twice + it 'fetches a new token' do + root_url = 'http://my.root.url' + usps_ipp_sponsor_id = 1 + + expect(IdentityConfig.store).to receive(:usps_ipp_root_url). + and_return(root_url) + expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). + and_return(usps_ipp_sponsor_id) facilities = subject.request_facilities(location) + expect(WebMock).to have_requested(:post, "#{root_url}/oauth/authenticate").twice + expect(facilities.length).to eq(10) check_facility(facilities[0]) end @@ -232,6 +250,14 @@ def check_facility(facility) end it 'fetches a new token and retries the attempt' do + root_url = 'http://my.root.url' + usps_ipp_sponsor_id = 1 + + expect(IdentityConfig.store).to receive(:usps_ipp_root_url). + and_return(root_url) + expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). + and_return(usps_ipp_sponsor_id) + expect(subject).to receive(:token).twice enrollment = subject.request_enroll(applicant) From a2192de1b5e21b88f8fd89a61a3a5f193ab1e033 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Tue, 14 Feb 2023 11:34:38 -0500 Subject: [PATCH 08/15] got first test to pass, need to get other expired token tests passing --- .../request_expired_token_response.json | 2 +- .../usps_in_person_proofing/proofer_spec.rb | 34 +++++++++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json b/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json index 94f268da06c..0b69559b9a8 100644 --- a/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json +++ b/app/services/usps_in_person_proofing/mock/responses/request_expired_token_response.json @@ -1,5 +1,5 @@ { "token_type": "Bearer", - "access_token": "==PZWyMP2ZHGOIeTd17YomIf7XjZUL4G93dboY1pTsuTJN0s9BwMYvOcIS9B3gRvloK2sroi9uFXdXrFuly7==", + "access_token": "==BAAyMP2ZHGOIeTd17YomIf7XjZUL4G93dboY1pTsuTJN0s9BwMYvOcIS9B3gRvloK2sroi9uFXdXrFuly7==", "expires_in": 0 } diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 8941ffab99f..cb052207951 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -141,31 +141,45 @@ def check_facility(facility) end context 'when the token is expired' do - AUTH_TOKEN_CACHE_KEY = :usps_ippaas_api_auth_token - let!(:redis) do - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url) - end + let(:cache) { double(ActiveSupport::Cache::MemoryStore) } + let(:redis) { double(Redis) } before do - redis.write( - AUTH_TOKEN_CACHE_KEY, - UspsInPersonProofing::Mock::Fixtures.request_expired_token_response, expires_in: 0 - ) allow(Rails).to receive(:cache).and_return( - redis, + cache, ) + allow(cache).to receive(:redis).and_return(redis) + allow(redis).to receive(:ttl).and_return(0) + stub_expired_request_token + stub_request_token stub_request_facilities end + # TODO + # 1. Need to make check for expires_at have some kind of a buffer. Can't do it at zero. + # 2. We are making two calls to Redis. If it fails before the first and two calls, then the expiry is not set. That means we can't solely rely on the Redis expiry. (Tim also mentioned this in our slack thread earlier.) I will refactor make one call to Redis. it 'fetches a new token' do root_url = 'http://my.root.url' usps_ipp_sponsor_id = 1 expect(IdentityConfig.store).to receive(:usps_ipp_root_url). - and_return(root_url) + and_return(root_url).exactly(3).times expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). and_return(usps_ipp_sponsor_id) + expect(cache).to receive(:write).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(String), + hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), + ).twice + + expect(redis).to receive(:expireat).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(Integer), + ).twice + + expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + facilities = subject.request_facilities(location) expect(WebMock).to have_requested(:post, "#{root_url}/oauth/authenticate").twice From 57b3bc079a327540a7066853940c4f61db4d3df2 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 15 Feb 2023 14:56:49 -0500 Subject: [PATCH 09/15] get all expired token tests to pass --- .../usps_in_person_proofing/proofer_spec.rb | 137 ++++++++++++++---- 1 file changed, 107 insertions(+), 30 deletions(-) diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index cb052207951..3f7f9eae2d5 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -50,32 +50,6 @@ ) end - it 'reuses the cached auth token on subsequent requests' do - applicant = double( - 'applicant', - address: Faker::Address.street_address, - city: Faker::Address.city, - state: Faker::Address.state_abbr, - zip_code: Faker::Address.zip_code, - first_name: Faker::Name.first_name, - last_name: Faker::Name.last_name, - email: Faker::Internet.safe_email, - unique_id: '123456789', - ) - stub_request_token - stub_request_enroll - - subject.request_enroll(applicant) - subject.request_enroll(applicant) - subject.request_enroll(applicant) - - expect(WebMock).to have_requested(:post, %r{/oauth/authenticate}).once - expect(WebMock).to have_requested( - :post, - %r{/ivs-ippaas-api/IPPRest/resources/rest/optInIPPApplicant}, - ).times(3) - end - context 'when using redis as a backing store' do before do |ex| allow(Rails).to receive(:cache).and_return( @@ -83,6 +57,32 @@ ) end + it 'reuses the cached auth token on subsequent requests' do + applicant = double( + 'applicant', + address: Faker::Address.street_address, + city: Faker::Address.city, + state: Faker::Address.state_abbr, + zip_code: Faker::Address.zip_code, + first_name: Faker::Name.first_name, + last_name: Faker::Name.last_name, + email: Faker::Internet.safe_email, + unique_id: '123456789', + ) + stub_request_token + stub_request_enroll + + subject.request_enroll(applicant) + subject.request_enroll(applicant) + subject.request_enroll(applicant) + + expect(WebMock).to have_requested(:post, %r{/oauth/authenticate}).once + expect(WebMock).to have_requested( + :post, + %r{/ivs-ippaas-api/IPPRest/resources/rest/optInIPPApplicant}, + ).times(3) + end + it 'manually sets the expiration' do stub_request_token subject.retrieve_token! @@ -117,6 +117,9 @@ def check_facility(facility) end context 'when the token is valid' do before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_token end @@ -157,7 +160,7 @@ def check_facility(facility) # TODO # 1. Need to make check for expires_at have some kind of a buffer. Can't do it at zero. - # 2. We are making two calls to Redis. If it fails before the first and two calls, then the expiry is not set. That means we can't solely rely on the Redis expiry. (Tim also mentioned this in our slack thread earlier.) I will refactor make one call to Redis. + # 2. We are making two calls to Redis. If it fails before the first and two calls, then the expiry is not set. That means we can't solely rely on the Redis expiry. (Tim also mentioned this in our slack thread earlier.) I will refactor to make one call to Redis. it 'fetches a new token' do root_url = 'http://my.root.url' usps_ipp_sponsor_id = 1 @@ -200,6 +203,8 @@ def check_facility(facility) end describe '#request_enroll' do + let(:cache) { double(ActiveSupport::Cache::MemoryStore) } + let(:redis) { double(Redis) } let(:applicant) do double( 'applicant', @@ -215,6 +220,9 @@ def check_facility(facility) end context 'when the token is valid' do before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_token end it 'returns enrollment information' do @@ -258,6 +266,12 @@ def check_facility(facility) context 'when the token is expired' do before do + allow(Rails).to receive(:cache).and_return( + cache, + ) + allow(cache).to receive(:redis).and_return(redis) + allow(redis).to receive(:ttl).and_return(0) + stub_expired_request_token stub_request_enroll_expired_token stub_request_enroll @@ -268,11 +282,22 @@ def check_facility(facility) usps_ipp_sponsor_id = 1 expect(IdentityConfig.store).to receive(:usps_ipp_root_url). - and_return(root_url) + and_return(root_url).exactly(3).times expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). and_return(usps_ipp_sponsor_id) - expect(subject).to receive(:token).twice + expect(cache).to receive(:write).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(String), + hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), + ).twice + + expect(redis).to receive(:expireat).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(Integer), + ).twice + + expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) enrollment = subject.request_enroll(applicant) expect(enrollment.enrollment_code).to be_present @@ -291,6 +316,9 @@ def check_facility(facility) end context 'when the token is valid' do before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_token end it 'returns failed enrollment information' do @@ -336,13 +364,34 @@ def check_facility(facility) end end context 'when the token is expired' do + let(:cache) { double(ActiveSupport::Cache::MemoryStore) } + let(:redis) { double(Redis) } before do + allow(Rails).to receive(:cache).and_return( + cache, + ) + allow(cache).to receive(:redis).and_return(redis) + allow(redis).to receive(:ttl).and_return(0) + stub_expired_request_token stub_request_proofing_results_with_forbidden_error stub_request_passed_proofing_results end it 'fetches a new token and retries the attempt' do + expect(cache).to receive(:write).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(String), + hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), + ).twice + + expect(redis).to receive(:expireat).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(Integer), + ).twice + + expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + proofing_results = subject.request_proofing_results( applicant.unique_id, applicant.enrollment_code, @@ -363,6 +412,9 @@ def check_facility(facility) end context 'when the token is valid' do before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_token end @@ -376,15 +428,40 @@ def check_facility(facility) end context 'when the token is expired' do + let(:cache) { double(ActiveSupport::Cache::MemoryStore) } + let(:redis) { double(Redis) } before do + allow(Rails).to receive(:cache).and_return( + cache, + ) + allow(cache).to receive(:redis).and_return(redis) + allow(redis).to receive(:ttl).and_return(0) + stub_expired_request_token end it 'fetches a new token and retries the attempt' do + usps_ipp_sponsor_id = 1 + root_url = 'http://my.root.url' + expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). + and_return(usps_ipp_sponsor_id) + stub_request_enrollment_code_with_forbidden_error stub_request_enrollment_code - expect(subject).to receive(:token).twice + expect(cache).to receive(:write).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(String), + hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), + ).twice + + expect(redis).to receive(:expireat).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(Integer), + ).twice + + expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + enrollment = subject.request_enrollment_code(applicant) expect(enrollment['enrollmentCode']).to be_present From e8d2f0737ff30b770084ff19cf7701d620ee0b46 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 15 Feb 2023 15:28:49 -0500 Subject: [PATCH 10/15] DRY up proofer test setup --- .../usps_in_person_proofing/proofer_spec.rb | 109 ++++++------------ 1 file changed, 38 insertions(+), 71 deletions(-) diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 3f7f9eae2d5..6be51227172 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -4,6 +4,8 @@ include UspsIppHelper let(:subject) { UspsInPersonProofing::Proofer.new } + let(:root_url) { 'http://my.root.url' } + let(:usps_ipp_sponsor_id) { 1 } describe '#retrieve_token!' do it 'sets token and token_expires_at' do @@ -16,7 +18,6 @@ it 'calls the endpoint with the expected params' do stub_request_token - root_url = 'http://my.root.url' username = 'test username' password = 'test password' client_id = 'test client id' @@ -105,6 +106,36 @@ def check_facility(facility) expect(facility.zip_code_5).to be_present end + def set_up_expired_token(cache, redis) + allow(Rails).to receive(:cache).and_return( + cache, + ) + allow(cache).to receive(:redis).and_return(redis) + allow(redis).to receive(:ttl).and_return(0) + + stub_expired_request_token + end + + def check_for_token_refresh_and_method_call(cache, redis) + expect(IdentityConfig.store).to receive(:usps_ipp_root_url). + and_return(root_url).exactly(3).times + expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). + and_return(usps_ipp_sponsor_id) + + expect(cache).to receive(:write).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(String), + hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), + ).twice + + expect(redis).to receive(:expireat).with( + UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, + an_instance_of(Integer), + ).twice + + expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + end + describe '#request_facilities' do let(:location) do double( @@ -147,13 +178,7 @@ def check_facility(facility) let(:cache) { double(ActiveSupport::Cache::MemoryStore) } let(:redis) { double(Redis) } before do - allow(Rails).to receive(:cache).and_return( - cache, - ) - allow(cache).to receive(:redis).and_return(redis) - allow(redis).to receive(:ttl).and_return(0) - - stub_expired_request_token + set_up_expired_token(cache, redis) stub_request_token stub_request_facilities end @@ -162,26 +187,7 @@ def check_facility(facility) # 1. Need to make check for expires_at have some kind of a buffer. Can't do it at zero. # 2. We are making two calls to Redis. If it fails before the first and two calls, then the expiry is not set. That means we can't solely rely on the Redis expiry. (Tim also mentioned this in our slack thread earlier.) I will refactor to make one call to Redis. it 'fetches a new token' do - root_url = 'http://my.root.url' - usps_ipp_sponsor_id = 1 - - expect(IdentityConfig.store).to receive(:usps_ipp_root_url). - and_return(root_url).exactly(3).times - expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). - and_return(usps_ipp_sponsor_id) - - expect(cache).to receive(:write).with( - UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, - an_instance_of(String), - hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), - ).twice - - expect(redis).to receive(:expireat).with( - UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, - an_instance_of(Integer), - ).twice - - expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + check_for_token_refresh_and_method_call(cache, redis) facilities = subject.request_facilities(location) @@ -266,38 +272,13 @@ def check_facility(facility) context 'when the token is expired' do before do - allow(Rails).to receive(:cache).and_return( - cache, - ) - allow(cache).to receive(:redis).and_return(redis) - allow(redis).to receive(:ttl).and_return(0) - - stub_expired_request_token + set_up_expired_token(cache, redis) stub_request_enroll_expired_token stub_request_enroll end it 'fetches a new token and retries the attempt' do - root_url = 'http://my.root.url' - usps_ipp_sponsor_id = 1 - - expect(IdentityConfig.store).to receive(:usps_ipp_root_url). - and_return(root_url).exactly(3).times - expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). - and_return(usps_ipp_sponsor_id) - - expect(cache).to receive(:write).with( - UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, - an_instance_of(String), - hash_including(expires_at: an_instance_of(ActiveSupport::TimeWithZone)), - ).twice - - expect(redis).to receive(:expireat).with( - UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY, - an_instance_of(Integer), - ).twice - - expect(cache).to receive(:read).with(UspsInPersonProofing::Proofer::AUTH_TOKEN_CACHE_KEY) + check_for_token_refresh_and_method_call(cache, redis) enrollment = subject.request_enroll(applicant) expect(enrollment.enrollment_code).to be_present @@ -367,13 +348,7 @@ def check_facility(facility) let(:cache) { double(ActiveSupport::Cache::MemoryStore) } let(:redis) { double(Redis) } before do - allow(Rails).to receive(:cache).and_return( - cache, - ) - allow(cache).to receive(:redis).and_return(redis) - allow(redis).to receive(:ttl).and_return(0) - - stub_expired_request_token + set_up_expired_token(cache, redis) stub_request_proofing_results_with_forbidden_error stub_request_passed_proofing_results end @@ -431,18 +406,10 @@ def check_facility(facility) let(:cache) { double(ActiveSupport::Cache::MemoryStore) } let(:redis) { double(Redis) } before do - allow(Rails).to receive(:cache).and_return( - cache, - ) - allow(cache).to receive(:redis).and_return(redis) - allow(redis).to receive(:ttl).and_return(0) - - stub_expired_request_token + set_up_expired_token(cache, redis) end it 'fetches a new token and retries the attempt' do - usps_ipp_sponsor_id = 1 - root_url = 'http://my.root.url' expect(IdentityConfig.store).to receive(:usps_ipp_sponsor_id). and_return(usps_ipp_sponsor_id) From aa9b3e60c5a67cfc8de7fefdc1c45eb95f72fc83 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 15 Feb 2023 18:00:07 -0500 Subject: [PATCH 11/15] add buffer to expires_at conditional --- app/services/usps_in_person_proofing/proofer.rb | 2 +- spec/services/usps_in_person_proofing/proofer_spec.rb | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index ebc4645a7b7..02a0fe86ef9 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -153,7 +153,7 @@ def faraday # already cached. # @return [Hash] Headers to add to USPS requests def dynamic_headers - if Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) != -2 && Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) <= 0 + if Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) != -2 && Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) <= 0.5 # rubocop:disable Layout/LineLength retrieve_token! end { diff --git a/spec/services/usps_in_person_proofing/proofer_spec.rb b/spec/services/usps_in_person_proofing/proofer_spec.rb index 6be51227172..77451b38d03 100644 --- a/spec/services/usps_in_person_proofing/proofer_spec.rb +++ b/spec/services/usps_in_person_proofing/proofer_spec.rb @@ -183,9 +183,6 @@ def check_for_token_refresh_and_method_call(cache, redis) stub_request_facilities end - # TODO - # 1. Need to make check for expires_at have some kind of a buffer. Can't do it at zero. - # 2. We are making two calls to Redis. If it fails before the first and two calls, then the expiry is not set. That means we can't solely rely on the Redis expiry. (Tim also mentioned this in our slack thread earlier.) I will refactor to make one call to Redis. it 'fetches a new token' do check_for_token_refresh_and_method_call(cache, redis) From 210a72a20ed66a0322cbe4e9e73a92cac2d6c4e6 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Thu, 16 Feb 2023 11:49:00 -0500 Subject: [PATCH 12/15] fix broken tests by adding redis cache setup --- .../controllers/idv/review_controller_spec.rb | 3 + .../get_usps_proofing_results_job_spec.rb | 61 +++++++++++++++++++ spec/lib/tasks/dev_rake_spec.rb | 3 + 3 files changed, 67 insertions(+) diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index e8343696ff0..63408298675 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -362,6 +362,9 @@ def show context 'user picked phone confirmation' do before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) idv_session.address_verification_mechanism = 'phone' idv_session.vendor_phone_confirmation = true idv_session.user_phone_confirmation = true diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index c2d0d81c951..079d8ef219d 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -1,6 +1,11 @@ require 'rails_helper' RSpec.shared_examples 'enrollment_with_a_status_update' do |passed:, status:, response_json:| + before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) + end it 'logs a message with common attributes' do freeze_time do pending_enrollment.update( @@ -43,6 +48,9 @@ context 'email_analytics_attributes' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_passed_proofing_results end it 'logs message with email analytics attributes' do @@ -85,6 +93,10 @@ response_message: nil, response_status_code: nil| it 'logs an error message and leaves the enrollment and profile pending' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) + job.perform(Time.zone.now) pending_enrollment.reload @@ -123,6 +135,9 @@ RSpec.shared_examples 'enrollment_encountering_an_error_that_has_a_nil_response' do |error_type:| it 'logs that response is not present' do expect(NewRelic::Agent).to receive(:notice_error).with(instance_of(error_type)) + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) job.perform(Time.zone.now) @@ -251,6 +266,9 @@ end it 'logs a message with counts of various outcomes when the job completes' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) allow(InPersonEnrollment).to receive(:needs_usps_status_check). and_return(pending_enrollments) stub_request_proofing_results_with_responses( @@ -329,6 +347,11 @@ end describe 'sending emails' do + before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) + end it 'sends proofing failed email on response with failed status' do stub_request_failed_proofing_results @@ -448,6 +471,9 @@ ) it 'logs details about the success' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -470,6 +496,9 @@ context 'when an enrollment fails' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_failed_proofing_results end @@ -503,6 +532,9 @@ context 'when an enrollment fails and fraud is suspected' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_failed_suspected_fraud_proofing_results end @@ -548,6 +580,9 @@ ) it 'logs a message about the unsupported ID' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) job.perform Time.zone.now expect(job_analytics).to have_logged_event( @@ -562,6 +597,9 @@ context 'when an enrollment expires' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_expired_proofing_results end @@ -603,6 +641,9 @@ allow(IdentityConfig.store).to( receive(:in_person_enrollment_validity_in_days).and_return(30), ) + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -621,6 +662,11 @@ end context 'when an enrollment is reported as invalid' do + before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) + end context 'when an enrollment code is invalid' do # this enrollment code is hardcoded into the fixture # request_unexpected_invalid_enrollment_code_response.json @@ -692,6 +738,9 @@ ) it 'logs the status received' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) job.perform(Time.zone.now) pending_enrollment.reload @@ -719,6 +768,9 @@ context 'when USPS returns an unexpected 400 status code' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_proofing_results_with_responses( { status: 400, @@ -763,6 +815,9 @@ ) it 'logs the error to NewRelic' do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) expect(NewRelic::Agent).to receive(:notice_error).with(instance_of(Faraday::ClientError)) job.perform(Time.zone.now) end @@ -770,6 +825,9 @@ context 'when USPS returns a 5xx status code' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_proofing_results_internal_server_error end @@ -789,6 +847,9 @@ context 'when there is no status update' do before(:each) do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) stub_request_in_progress_proofing_results end diff --git a/spec/lib/tasks/dev_rake_spec.rb b/spec/lib/tasks/dev_rake_spec.rb index 3fba0d4927b..96b3eb42800 100644 --- a/spec/lib/tasks/dev_rake_spec.rb +++ b/spec/lib/tasks/dev_rake_spec.rb @@ -106,6 +106,9 @@ describe 'dev:random_in_person_users' do before(:each) do allow(IdentityConfig.store).to receive(:usps_mock_fallback).and_return(false) + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) ENV['VERIFIED'] = nil Rake::Task['dev:random_in_person_users'].reenable end From c3474b1a2922d994dd56eafb855c13dfcca77860 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Thu, 16 Feb 2023 12:25:15 -0500 Subject: [PATCH 13/15] fix broken enrollment helper tests by adding caching setup --- .../usps_in_person_proofing/enrollment_helper_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb b/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb index 9944f472aab..5ca6594595d 100644 --- a/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb +++ b/spec/services/usps_in_person_proofing/enrollment_helper_spec.rb @@ -44,6 +44,11 @@ end context 'an establishing enrollment record exists for the user' do + before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) + end it 'updates the existing enrollment record' do expect(user.in_person_enrollments.length).to eq(1) From 57fdca18ad9fc9645153eb0a226a0f789b164035 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 22 Feb 2023 10:51:43 -0500 Subject: [PATCH 14/15] respond to feedback --- .../usps_in_person_proofing/proofer.rb | 4 +- .../get_usps_proofing_results_job_spec.rb | 63 +------------------ 2 files changed, 6 insertions(+), 61 deletions(-) diff --git a/app/services/usps_in_person_proofing/proofer.rb b/app/services/usps_in_person_proofing/proofer.rb index 02a0fe86ef9..9825ae2676d 100644 --- a/app/services/usps_in_person_proofing/proofer.rb +++ b/app/services/usps_in_person_proofing/proofer.rb @@ -1,6 +1,7 @@ module UspsInPersonProofing class Proofer AUTH_TOKEN_CACHE_KEY = :usps_ippaas_api_auth_token + AUTH_TOKEN_REFRESH_THRESHOLD = 5 # Makes HTTP request to get nearby in-person proofing facilities # Requires address, city, state and zip code. @@ -153,7 +154,8 @@ def faraday # already cached. # @return [Hash] Headers to add to USPS requests def dynamic_headers - if Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) != -2 && Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) <= 0.5 # rubocop:disable Layout/LineLength + token_remaining_time = Rails.cache.redis.ttl(AUTH_TOKEN_CACHE_KEY) + if token_remaining_time != -2 && token_remaining_time <= AUTH_TOKEN_REFRESH_THRESHOLD retrieve_token! end { diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 079d8ef219d..ef02e149cd7 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -1,11 +1,6 @@ require 'rails_helper' RSpec.shared_examples 'enrollment_with_a_status_update' do |passed:, status:, response_json:| - before do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) - end it 'logs a message with common attributes' do freeze_time do pending_enrollment.update( @@ -48,9 +43,6 @@ context 'email_analytics_attributes' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_passed_proofing_results end it 'logs message with email analytics attributes' do @@ -93,10 +85,6 @@ response_message: nil, response_status_code: nil| it 'logs an error message and leaves the enrollment and profile pending' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) - job.perform(Time.zone.now) pending_enrollment.reload @@ -135,10 +123,6 @@ RSpec.shared_examples 'enrollment_encountering_an_error_that_has_a_nil_response' do |error_type:| it 'logs that response is not present' do expect(NewRelic::Agent).to receive(:notice_error).with(instance_of(error_type)) - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) - job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -162,6 +146,9 @@ let(:job_analytics) { FakeAnalytics.new } before do + allow(Rails).to receive(:cache).and_return( + ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), + ) ActiveJob::Base.queue_adapter = :test allow(job).to receive(:analytics).and_return(job_analytics) allow(IdentityConfig.store).to receive(:get_usps_proofing_results_job_reprocess_delay_minutes). @@ -266,9 +253,6 @@ end it 'logs a message with counts of various outcomes when the job completes' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) allow(InPersonEnrollment).to receive(:needs_usps_status_check). and_return(pending_enrollments) stub_request_proofing_results_with_responses( @@ -347,11 +331,6 @@ end describe 'sending emails' do - before do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) - end it 'sends proofing failed email on response with failed status' do stub_request_failed_proofing_results @@ -471,9 +450,6 @@ ) it 'logs details about the success' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -496,9 +472,6 @@ context 'when an enrollment fails' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_failed_proofing_results end @@ -532,9 +505,6 @@ context 'when an enrollment fails and fraud is suspected' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_failed_suspected_fraud_proofing_results end @@ -580,9 +550,6 @@ ) it 'logs a message about the unsupported ID' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) job.perform Time.zone.now expect(job_analytics).to have_logged_event( @@ -597,9 +564,6 @@ context 'when an enrollment expires' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_expired_proofing_results end @@ -641,9 +605,6 @@ allow(IdentityConfig.store).to( receive(:in_person_enrollment_validity_in_days).and_return(30), ) - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -663,9 +624,6 @@ context 'when an enrollment is reported as invalid' do before do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) end context 'when an enrollment code is invalid' do # this enrollment code is hardcoded into the fixture @@ -738,9 +696,6 @@ ) it 'logs the status received' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) job.perform(Time.zone.now) pending_enrollment.reload @@ -768,9 +723,6 @@ context 'when USPS returns an unexpected 400 status code' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_proofing_results_with_responses( { status: 400, @@ -815,9 +767,6 @@ ) it 'logs the error to NewRelic' do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) expect(NewRelic::Agent).to receive(:notice_error).with(instance_of(Faraday::ClientError)) job.perform(Time.zone.now) end @@ -825,9 +774,6 @@ context 'when USPS returns a 5xx status code' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_proofing_results_internal_server_error end @@ -847,9 +793,6 @@ context 'when there is no status update' do before(:each) do - allow(Rails).to receive(:cache).and_return( - ActiveSupport::Cache::RedisCacheStore.new(url: IdentityConfig.store.redis_throttle_url), - ) stub_request_in_progress_proofing_results end From 0cd7644df11709455a15eb63b14a31824506bfb5 Mon Sep 17 00:00:00 2001 From: Eileen McFarland Date: Wed, 22 Feb 2023 14:52:38 -0500 Subject: [PATCH 15/15] remove no-op --- spec/jobs/get_usps_proofing_results_job_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index ef02e149cd7..bf5558be7ac 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -623,8 +623,6 @@ end context 'when an enrollment is reported as invalid' do - before do - end context 'when an enrollment code is invalid' do # this enrollment code is hardcoded into the fixture # request_unexpected_invalid_enrollment_code_response.json