diff --git a/.env.sample b/.env.sample index 81cc6343..9f5f230a 100644 --- a/.env.sample +++ b/.env.sample @@ -7,3 +7,4 @@ FRAMEWORK_ALMA_SANDBOX_KEY=KEY_GOES_HERE LIT_TIND_API_KEY=KEY_GOES_HERE # set a warning message when we know we're getting rate limited by OCLC FRAMEWORK_LOCATION_REQUESTS_ALERT="Location Requests that involve WorldCat lookups are currently being rate limited by OCLC. We have contacted OCLC support and are awaiting resolution." +FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERS=10000 diff --git a/app/models/location_request.rb b/app/models/location_request.rb index ab01487a..5b145eba 100644 --- a/app/models/location_request.rb +++ b/app/models/location_request.rb @@ -8,7 +8,7 @@ class LocationRequest < ActiveRecord::Base # Batch size for inserting LocationRecords BATCH_SIZE = 10_000 - MAX_OCLC_NUMBERS = 50_000 + MAX_OCLC_NUMBERS = Rails.application.config.location_requests_max_oclc_numbers RESULT_ARGS = %i[oclc_number wc_symbols wc_error ht_record_url ht_error].freeze MSG_NO_OCLC_NUMBERS = 'No OCLC numbers found in input spreadsheet'.freeze diff --git a/config/altmedia.yml b/config/altmedia.yml index 63a9f876..c945d123 100644 --- a/config/altmedia.yml +++ b/config/altmedia.yml @@ -16,6 +16,7 @@ default: &default alma_api_key: <%= ENV["FRAMEWORK_ALMA_API_KEY"].presence || 'fake-api-key' %> alma_sandbox_key: <%= ENV["FRAMEWORK_ALMA_SANDBOX_KEY"].presence || 'fake-api-key' %> location_requests_alert: <%= ENV["FRAMEWORK_LOCATION_REQUESTS_ALERT"] %> + location_requests_max_oclc_numbers: <%= Integer(ENV["FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERS"].presence || 10_000) %> paypal_payflow_url: <%= ENV["PAYPAL_PAYFLOW_URL"] || 'https://payflowlink.paypal.com' %> paypal_payflow_login: <%= ENV["PAYPAL_PAYFLOW_LOGIN"] || 'ucblibrary' %> tind_base_uri: <%= ENV["LIT_TIND_BASE_URL"] || 'https://digicoll.lib.berkeley.edu/' %> diff --git a/config/application.rb b/config/application.rb index e80b167b..1e3d3451 100644 --- a/config/application.rb +++ b/config/application.rb @@ -87,6 +87,7 @@ def log_active_storage_root!(active_storage_root) # alert message on location requests when we know we're being rate limited config.location_requests_alert = config.altmedia['location_requests_alert'] + config.location_requests_max_oclc_numbers = config.altmedia['location_requests_max_oclc_numbers'] # Tind set values for marc inserts config.tind_resource_types = config.tind_marc['resource_types'] diff --git a/spec/config/altmedia_spec.rb b/spec/config/altmedia_spec.rb new file mode 100644 index 00000000..e3f41980 --- /dev/null +++ b/spec/config/altmedia_spec.rb @@ -0,0 +1,37 @@ +require 'rails_helper' + +RSpec.describe 'altmedia configuration' do + + def with_location_requests_max_oclc_numbers(max_oclc_numbers) + env_var = 'FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERS' + original_max_oclc_numbers = ENV.fetch(env_var, nil) + set_env(env_var, max_oclc_numbers) + yield + ensure + set_env(env_var, original_max_oclc_numbers) + end + + def set_env(env_var, value) + value ? ENV[env_var] = value : ENV.delete(env_var) + end + + def altmedia_config + Rails.application.config_for(:altmedia) + end + + it 'defaults the location requests max OCLC numbers to 10,000' do + with_location_requests_max_oclc_numbers(nil) do + expect(altmedia_config[:location_requests_max_oclc_numbers]).to eq(10_000) + end + end + + it 'reads the location requests max OCLC numbers from FRAMEWORK_LOCATION_REQUESTS_MAX_OCLC_NUMBERS' do + with_location_requests_max_oclc_numbers('123') do + expect(altmedia_config[:location_requests_max_oclc_numbers]).to eq(123) + end + end + + it 'exposes the location requests max OCLC numbers through Rails configuration' do + expect(Rails.application.config.location_requests_max_oclc_numbers).to eq(Rails.application.config.altmedia[:location_requests_max_oclc_numbers]) + end +end diff --git a/spec/models/location_request_spec.rb b/spec/models/location_request_spec.rb index 92c21ea1..42fefa5f 100644 --- a/spec/models/location_request_spec.rb +++ b/spec/models/location_request_spec.rb @@ -38,6 +38,23 @@ def assert_same_contents(expected_path, actual_attachment) expect(actual_blob).to eq(expected_blob) end + def uploaded_file_with_oclc_numbers(oclc_numbers) + Dir.mktmpdir(File.basename(__FILE__)) do |tmpdir| + original_path = 'spec/data/location/input-file-empty.xlsx' + new_path = File.join(tmpdir, "#{oclc_numbers.size}.xlsx") + + ss = BerkeleyLibrary::Util::XLSX::Spreadsheet.new(original_path) + c_index = ss.find_column_index_by_header!(BerkeleyLibrary::Location::Constants::OCLC_COL_HEADER) + oclc_numbers.each_with_index do |oclc_num, i| + r_index = 1 + i # skip header row + ss.set_value_at(r_index, c_index, oclc_num) + end + ss.save_as(new_path) + + uploaded_file_from(new_path, mime_type: mime_type_xlsx) + end + end + # ------------------------------- # setup / teardown @@ -83,6 +100,13 @@ def assert_same_contents(expected_path, actual_attachment) end end + describe '.max_oclc_numbers' do + it 'returns the configured max OCLC numbers formatted for display' do + expect(LocationRequest::MAX_OCLC_NUMBERS).to eq(Rails.application.config.location_requests_max_oclc_numbers) + expect(LocationRequest.max_oclc_numbers).to eq(Rails.application.config.location_requests_max_oclc_numbers.to_fs(:delimited)) + end + end + describe :create do it 'accepts an input file' do req = LocationRequest.create!(**valid_attributes) @@ -321,34 +345,33 @@ def assert_same_contents(expected_path, actual_attachment) expect(req.location_records.count).to eq(expected_count) end + it 'raises ArgumentError when the input contains more OCLC numbers than configured' do + stub_const('LocationRequest::MAX_OCLC_NUMBERS', 2) + + attributes = valid_attributes.except(:input_file) + attributes[:input_file] = uploaded_file_with_oclc_numbers(%w[1 2 3]) + req = LocationRequest.create!(**attributes) + + expect { req.ensure_location_records! } + .to raise_error(ArgumentError, I18n.t('location_request.errors.max_oclc_numbers', max: '2')) + + expect(req.location_records).not_to exist + end + it 'handles large numbers of records' do # NOTE: tested with up to 1 million, but it's slow (~4 minutes) - expected_count = 15_000 + expected_count = LocationRequest::BATCH_SIZE + 1 + stub_const('LocationRequest::MAX_OCLC_NUMBERS', expected_count) oclc_numbers = Array.new(expected_count) { |i| (expected_count + i).to_s } oclc_numbers.shuffle! - Dir.mktmpdir(File.basename(__FILE__)) do |tmpdir| - original_path = 'spec/data/location/input-file-empty.xlsx' - new_path = File.join(tmpdir, "#{expected_count}.xlsx") - - ss = BerkeleyLibrary::Util::XLSX::Spreadsheet.new(original_path) - c_index = ss.find_column_index_by_header!(BerkeleyLibrary::Location::Constants::OCLC_COL_HEADER) - oclc_numbers.each_with_index do |oclc_num, i| - r_index = 1 + i # skip header row - ss.set_value_at(r_index, c_index, oclc_num) - end - ss.save_as(new_path) - - input_file = uploaded_file_from(new_path, mime_type: mime_type_xlsx) - - attributes = valid_attributes.except(:input_file) - attributes[:input_file] = input_file + attributes = valid_attributes.except(:input_file) + attributes[:input_file] = uploaded_file_with_oclc_numbers(oclc_numbers) - req = LocationRequest.create!(**attributes) - req.ensure_location_records! + req = LocationRequest.create!(**attributes) + req.ensure_location_records! - expect(req.location_records.count).to eq(expected_count) - end + expect(req.location_records.count).to eq(expected_count) end end