diff --git a/app/controllers/registration_controller.rb b/app/controllers/registration_controller.rb index 1589129d..1bd40bf0 100644 --- a/app/controllers/registration_controller.rb +++ b/app/controllers/registration_controller.rb @@ -22,7 +22,7 @@ def show rescue Dynamoid::Errors::RecordNotFound render json: {}, status: :not_found rescue RegistrationError => e - render_error(e.http_status, e.error) + render_error(e.http_status, e.error, e.data) end def count @@ -79,7 +79,7 @@ def validate_update_request @user_id = params[:user_id] @competition_id = params[:competition_id] - RegistrationChecker.update_registration_allowed!(params, CompetitionApi.find!(@competition_id), @current_user) + RegistrationChecker.update_registration_allowed!(params, CompetitionApi.find(@competition_id), @current_user) rescue RegistrationError => e render_error(e.http_status, e.error, e.data) end @@ -102,7 +102,7 @@ def bulk_update def validate_bulk_update_request @competition_id = params.require('competition_id') - RegistrationChecker.bulk_update_allowed!(params, CompetitionApi.find!(@competition_id), @current_user) + RegistrationChecker.bulk_update_allowed!(params, CompetitionApi.find(@competition_id), @current_user) rescue BulkUpdateError => e render_error(e.http_status, e.errors) rescue NoMethodError @@ -158,7 +158,7 @@ def payment_ticket def validate_payment_ticket_request competition_id = params[:competition_id] - @competition = CompetitionApi.find!(competition_id) + @competition = CompetitionApi.find(competition_id) render_error(:forbidden, ErrorCodes::PAYMENT_NOT_ENABLED) unless @competition.using_wca_payment? @registration = Registration.find("#{competition_id}-#{@current_user}") @@ -223,6 +223,8 @@ def list_admin Metrics.registration_dynamodb_errors_counter.increment render json: { error: "Error getting registrations #{e}" }, status: :internal_server_error + rescue RegistrationError => e + render_error(e.http_status, e.error, e.data) end def import diff --git a/lib/competition_api.rb b/lib/competition_api.rb index 3bf9d9f3..ea87ad04 100644 --- a/lib/competition_api.rb +++ b/lib/competition_api.rb @@ -8,49 +8,29 @@ class CompetitionApi < WcaApi def self.find(competition_id) competition_json = fetch_competition(competition_id) CompetitionInfo.new(competition_json) - rescue RegistrationError - nil end def self.url(competition_id) "#{EnvConfig.WCA_HOST}/api/v0/competitions/#{competition_id}" end - def self.find!(competition_id) - competition_json = fetch_competition(competition_id) - CompetitionInfo.new(competition_json) - end - def self.fetch_qualifications(competition_id) - Rails.cache.fetch("#{competition_id}/qualifications", expires_in: 5.minutes) do - response = HTTParty.get("#{url(competition_id)}/qualifications") - case response.code - when 200 - @status = 200 - response.parsed_response - when 404 - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) - else - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX) - end + self.get_request("#{url(competition_id)}/qualifications") + rescue RegistrationError => e + if e.data[:http_code] == 404 + raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) + else + raise e end end private_class_method def self.fetch_competition(competition_id) - Rails.cache.fetch(competition_id, expires_in: 5.minutes) do - response = HTTParty.get(CompetitionApi.url(competition_id)) - case response.code - when 200 - response.parsed_response - when 404 - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) - else - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX) - end + self.get_request(url(competition_id)) + rescue RegistrationError => e + if e.data[:http_code] == 404 + raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) + else + raise e end end end diff --git a/lib/email_api.rb b/lib/email_api.rb index 50fa8090..d4b45192 100644 --- a/lib/email_api.rb +++ b/lib/email_api.rb @@ -9,22 +9,26 @@ def self.registration_email_path end def self.send_update_email(competition_id, user_id, status, current_user) - HTTParty.post(EmailApi.registration_email_path, headers: { WCA_API_HEADER => self.wca_token }, body: { - competition_id: competition_id, - user_id: user_id, - registration_status: status, - registration_action: 'update', - current_user: current_user, - }) + self.post_request( + EmailApi.registration_email_path, + { competition_id: competition_id, + user_id: user_id, + registration_status: status, + registration_action: 'update', + current_user: current_user }.to_json, + ) end def self.send_creation_email(competition_id, user_id) - HTTParty.post(EmailApi.registration_email_path, headers: { WCA_API_HEADER => self.wca_token }, body: { - competition_id: competition_id, - user_id: user_id, - registration_status: 'pending', - registration_action: 'create', - current_user: user_id, - }) + self.post_request( + EmailApi.registration_email_path, + { + competition_id: competition_id, + user_id: user_id, + registration_status: 'pending', + registration_action: 'create', + current_user: user_id, + }.to_json, + ) end end diff --git a/lib/error_codes.rb b/lib/error_codes.rb index 4ac1ccb6..bf4025a9 100644 --- a/lib/error_codes.rb +++ b/lib/error_codes.rb @@ -6,9 +6,10 @@ module ErrorCodes EXPIRED_TOKEN = -2 MISSING_AUTHENTICATION = -3 - # Competition Errors + # API Errors COMPETITION_NOT_FOUND = -1000 COMPETITION_API_5XX = -1001 + MONOLITH_API_ERROR = -1002 # User Errors USER_CANNOT_COMPETE = -2001 diff --git a/lib/payment_api.rb b/lib/payment_api.rb index ab6d416d..f40ce890 100644 --- a/lib/payment_api.rb +++ b/lib/payment_api.rb @@ -2,13 +2,10 @@ class PaymentApi < WcaApi def self.get_ticket(attendee_id, amount, currency_code, current_user) - response = HTTParty.post(payment_init_path, - body: { 'attendee_id' => attendee_id, 'amount' => amount, 'currency_code' => currency_code, 'current_user' => current_user }.to_json, - headers: { WCA_API_HEADER => self.wca_token, - 'Content-Type' => 'application/json' }) - unless response.ok? - raise 'Error from the payments service' - end + response = self.post_request( + payment_init_path, + { 'attendee_id' => attendee_id, 'amount' => amount, 'currency_code' => currency_code, 'current_user' => current_user }.to_json, + ) [response['client_secret'], response['id']] end diff --git a/lib/user_api.rb b/lib/user_api.rb index 9fa75813..281d9e59 100644 --- a/lib/user_api.rb +++ b/lib/user_api.rb @@ -21,15 +21,15 @@ def self.competitor_qualifications_path(user_id, date = nil) end def self.get_permissions(user_id) - HTTParty.get(permissions_path(user_id), headers: { WCA_API_HEADER => self.wca_token }) + self.get_request(permissions_path(user_id)) end def self.get_user_info_pii(user_ids) - HTTParty.post(UserApi.competitor_info_path, headers: { WCA_API_HEADER => self.wca_token }, body: { ids: user_ids.to_a }) + self.post_request(UserApi.competitor_info_path, { ids: user_ids.to_a }) end def self.qualifications(user_id, date = nil) - HTTParty.get(UserApi.competitor_qualifications_path(user_id, date), headers: { WCA_API_HEADER => self.wca_token }) + self.get_request(UserApi.competitor_qualifications_path(user_id, date)) end def self.can_compete?(user_id, competition_start_date) diff --git a/lib/wca_api.rb b/lib/wca_api.rb index 8c0a8763..b6353da1 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -21,4 +21,33 @@ def self.wca_token end end end + + def self.get_request(url) + Rails.cache.fetch(url, expires_in: 5.minutes) do + response = HTTParty.get(url, headers: { WCA_API_HEADER => self.wca_token }) + + if response.success? + response + else + raise RegistrationError.new( + 500, + ErrorCodes::MONOLITH_API_ERROR, + { url: "GET: #{url}", http_code: response.code, body: response.parsed_response }, + ) + end + end + end + + def self.post_request(url, body) + response = HTTParty.post(url, headers: { WCA_API_HEADER => self.wca_token, 'Content-Type' => 'application/json' }, body: body) + if response.success? + response + else + raise RegistrationError.new( + 500, + ErrorCodes::MONOLITH_API_ERROR, + { url: "POST: #{url}", http_code: response.code, body: response.parsed_response }, + ) + end + end end diff --git a/spec/controllers/registration_controller_spec.rb b/spec/controllers/registration_controller_spec.rb index 64056e39..8926c014 100644 --- a/spec/controllers/registration_controller_spec.rb +++ b/spec/controllers/registration_controller_spec.rb @@ -235,4 +235,74 @@ expect(response.code).to eq('400') end end + + describe '#API errors' do + describe 'raise Registration error when monolith API fails:' do + it 'UserApi::get_permissions' do + @registration_request = FactoryBot.build(:registration_request) + stub_request(:get, UserApi.permissions_path(@registration_request['user_id'])).to_return( + status: 502, + headers: { 'Content-Type' => 'application/json' }, + ) + + @competition = FactoryBot.build(:competition) + stub_request(:get, CompetitionApi.url(@competition['id'])).to_return( + status: 200, + body: @competition.except('qualifications').to_json, + headers: { 'Content-Type' => 'application/json' }, + ) + + stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json) + + request.headers['Authorization'] = @registration_request['jwt_token'] + post :create, params: @registration_request, as: :json + + expect(response.code).to eq('500') + end + + it 'UserApi::qualifications' do + @registration_request = FactoryBot.build(:registration_request) + stub_request(:get, UserApi.permissions_path(@registration_request['user_id'])).to_return( + status: 200, + body: FactoryBot.build(:permissions).to_json, + headers: { 'Content-Type' => 'application/json' }, + ) + + @competition = FactoryBot.build(:competition, :has_qualifications) + stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications')) + stub_json(CompetitionApi.url("#{@competition['id']}/qualifications"), 200, @competition['qualifications']) + + stub_request(:get, %r{#{Regexp.escape(EnvConfig.WCA_HOST)}/api/v0/results/\d+/qualification_data(\?date=\d{4}-\d{2}-\d{2})?}) + .to_return(status: 502) + + stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json) + + request.headers['Authorization'] = @registration_request['jwt_token'] + post :create, params: @registration_request, as: :json + + expect(response.code).to eq('500') + end + + it 'UserApi::get_user_info_pii' do + @competition = FactoryBot.build(:competition) + stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications')) + + @requesting_user_id = 1400 + stub_request(:get, UserApi.permissions_path(@requesting_user_id)).to_return( + status: 200, + body: FactoryBot.build(:permissions_response, organized_competitions: [@competition['id']]).to_json, + headers: { 'Content-Type' => 'application/json' }, + ) + + FactoryBot.create(:registration, registration_status: 'pending') + + stub_request(:post, UserApi.competitor_info_path).to_return(status: 502) + + request.headers['Authorization'] = fetch_jwt_token(@requesting_user_id) + get :list_admin, params: { competition_id: @competition['id'] } + + expect(response.code).to eq('500') + end + end + end end diff --git a/spec/lib/competition_api_spec.rb b/spec/lib/competition_api_spec.rb new file mode 100644 index 00000000..ef6a6802 --- /dev/null +++ b/spec/lib/competition_api_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe CompetitionApi do + it 'raises RegistrationError when 404 is returned' do + stub_request(:get, CompetitionApi.url('test')).to_return(status: 404) + + expect { + CompetitionApi.find('test') + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(404) + expect(error.error).to eq(ErrorCodes::COMPETITION_NOT_FOUND) + end + end + + it 'raises RegistrationError when 404 is returned' do + stub_request(:get, CompetitionApi.url('test')).to_return(status: 500) + + expect { + CompetitionApi.find('test') + }.to raise_error(RegistrationError) do |error| + expect(error.http_status).to eq(500) + expect(error.error).to eq(ErrorCodes::MONOLITH_API_ERROR) + end + end +end diff --git a/spec/support/stub_helper.rb b/spec/support/stub_helper.rb index c6f46bce..45cd7906 100644 --- a/spec/support/stub_helper.rb +++ b/spec/support/stub_helper.rb @@ -25,3 +25,11 @@ def stub_qualifications(payload = nil, qualification_data_date = nil) end end end + +def stub_qualification_error + url_regex = %r{#{Regexp.escape(EnvConfig.WCA_HOST)}/api/v0/results/\d+/qualification_data(\?date=\d{4}-\d{2}-\d{2})?} + + stub_request(:get, url_regex).to_return do |request| + { status: 502 } + end +end