AP-870: Make location requests OCLC limit configurable

.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

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

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/' %>

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']

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

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