From fefb98e9815de5e79e1490702678e29414e9a58a Mon Sep 17 00:00:00 2001 From: Nick Budak Date: Thu, 17 Sep 2026 14:24:11 -0700 Subject: [PATCH] Apply crawl_delay per request instead of per page Moves the requests a harvest makes into an HttpClient that waits out the crawl delay before each one. Previously the delay was applied once per page of search results, so a Blacklight 7 harvest could issue 100 document requests back to back before it took effect. Each request continues to get its own connection rather than reusing one, which is likewise more likely to be accepted by a WAF; the client is now the single place that behavior is decided. Fractional delays work too. A response class builds a client for itself when it isn't given one, and passes along the logger it was given so request logging doesn't quietly go somewhere else. Harvester request specs now stub HTTP with webmock rather than mocking Net::HTTP.get, since the request path is no longer a single class method. Closes #209 Co-Authored-By: Claude Opus 5 --- README.md | 4 +- lib/geo_combine/geo_blacklight_harvester.rb | 65 +++++-- .../geo_blacklight_harvester_spec.rb | 173 ++++++++++++++---- 3 files changed, 190 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 822ea77..5472ca6 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,9 @@ end ##### Crawl Delays (default: none) -Crawl delays can be configured (in seconds) either globally for all sites or on a per-site basis. This will cause a delay for that number of seconds between each search results page (note that Blacklight 7 necessitates a lot of requests per results page and this only causes the delay per page of results) +Crawl delays can be configured (in seconds, and fractions of a second are allowed) either globally for all sites or on a per-site basis. The harvester waits out the delay before every request it makes, not just before each page of search results -- which matters because Blacklight 7 and above needs a request per document, so one page of results is many requests. Each request also gets its own connection instead of reusing one. Together, pacing requests and connecting fresh make a harvest much less likely to be turned away by a WAF or other bot mitigation. + +Be aware that this makes a harvest take considerably longer than it did when the delay applied per page: a one second delay against a Blacklight 7 site with 10,000 records is around three hours of waiting. Lower the delay if that matters more to you than getting past bot mitigation. ##### Solr's commitWithin (default: 5000 milliseconds) diff --git a/lib/geo_combine/geo_blacklight_harvester.rb b/lib/geo_combine/geo_blacklight_harvester.rb index 72d1b19..adc4722 100644 --- a/lib/geo_combine/geo_blacklight_harvester.rb +++ b/lib/geo_combine/geo_blacklight_harvester.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'net/http' require 'geo_combine/logger' module GeoCombine @@ -13,7 +14,7 @@ module GeoCombine # end # The class configuration also allows for various other things to be configured: # - A debug parameter to print out details of what is being harvested and indexed - # - crawl delays for each page of results (globally or on a per site basis) + # - crawl delays between requests (globally or on a per site basis) # - Solr's commitWithin parameter (defaults to 5000) # - A document transformer proc to modify a document before indexing (defaults to removing _version_, score, and timestamp) # Example: GeoCombine::GeoBlacklightHarvester.new('SITE').index @@ -74,6 +75,41 @@ def each_document(&block) each_page { |documents| documents.each(&block) } end + ## + # Makes the requests for a harvest, waiting out the configured crawl delay + # before each one. Each request gets its own connection instead of reusing + # one; a request that is both paced and freshly connected is much less + # likely to be turned away by a WAF or other bot mitigation. + class HttpClient + attr_reader :crawl_delay + + def initialize(crawl_delay: nil, logger: GeoCombine::Logger.logger) + @crawl_delay = crawl_delay&.to_f + @logger = logger + end + + # Fetch a URL and parse the JSON response body + def get_json(url) + JSON.parse(get(url)) + end + + private + + # Fetch a URL and return the response body + def get(url) + throttle + Net::HTTP.get_response(URI(url)).body + end + + # Wait out the crawl delay, if one is configured + def throttle + return unless crawl_delay + + @logger.debug "waiting #{crawl_delay}s before the next request" + sleep(crawl_delay) + end + end + ## # A "factory" class to determine the blacklight response version to use class BlacklightResponseVersionFactory @@ -91,12 +127,13 @@ def self.call(json) end class LegacyBlacklightResponse - attr_reader :base_url + attr_reader :base_url, :client attr_accessor :response, :page - def initialize(response:, base_url:, logger: GeoCombine::Logger.logger) + def initialize(response:, base_url:, logger: GeoCombine::Logger.logger, client: HttpClient.new(logger:)) @base_url = base_url @response = response + @client = client @page = 1 @logger = logger end @@ -113,7 +150,7 @@ def documents @logger.debug "fetching page #{page} @ #{url}" begin - self.response = JSON.parse(Net::HTTP.get(URI(url))) + self.response = client.get_json(url) rescue StandardError => e @logger.error "request for #{url} failed with #{e}" self.response = nil @@ -139,12 +176,13 @@ def total_pages ## # Class to return documents from the Blacklight API (v7 and above) class ModernBlacklightResponse - attr_reader :base_url + attr_reader :base_url, :client attr_accessor :response, :page - def initialize(response:, base_url:, logger: GeoCombine::Logger.logger) + def initialize(response:, base_url:, logger: GeoCombine::Logger.logger, client: HttpClient.new(logger:)) @base_url = base_url @response = response + @client = client @page = 1 @logger = logger end @@ -164,7 +202,7 @@ def documents self.page += 1 @logger.debug "fetching page #{page} @ #{url}" begin - self.response = JSON.parse(Net::HTTP.get(URI(url))) + self.response = client.get_json(url) rescue StandardError => e @logger.error "Request for #{url} failed with #{e}" self.response = nil @@ -177,7 +215,7 @@ def documents def documents_from_urls(urls) @logger.debug "fetching #{urls.count} documents for page #{page}" urls.map do |url| - JSON.parse(Net::HTTP.get(URI("#{url}/raw"))) + client.get_json("#{url}/raw") rescue StandardError => e @logger.error "fetching \"#{url}/raw\" failed with #{e}" @@ -193,16 +231,19 @@ def each_page return to_enum(:each_page) unless block_given? @logger.debug "fetching page 1 @ #{base_url}&page=1" - response = JSON.parse(Net::HTTP.get(URI("#{base_url}&page=1"))) + response = client.get_json("#{base_url}&page=1") response_class = BlacklightResponseVersionFactory.call(response) - response_class.new(response:, base_url:, logger: @logger).documents.each do |documents| + response_class.new(response:, base_url:, client:, logger: @logger).documents.each do |documents| yield documents.map { |document| self.class.document_transformer&.call(document) }.compact - - sleep(crawl_delay.to_i) if crawl_delay end end + # The client used to make requests for this site + def client + @client ||= HttpClient.new(crawl_delay:, logger: @logger) + end + def base_url "#{site[:host]}?#{default_params.to_query}" end diff --git a/spec/lib/geo_combine/geo_blacklight_harvester_spec.rb b/spec/lib/geo_combine/geo_blacklight_harvester_spec.rb index 23457a8..ca9510f 100644 --- a/spec/lib/geo_combine/geo_blacklight_harvester_spec.rb +++ b/spec/lib/geo_combine/geo_blacklight_harvester_spec.rb @@ -9,20 +9,23 @@ let(:logger) { instance_double(Logger, warn: nil, info: nil, error: nil, debug: nil) } let(:site_key) { :INSTITUTION } + let(:base_url) { 'https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100' } let(:stub_json_response) { '{}' } let(:stub_solr_connection) { double('RSolr::Connection') } + let(:site_config) do + { host: 'https://example.com/', params: { f: { dct_provenance_s: ['INSTITUTION'] } } } + end + let(:config) { { INSTITUTION: site_config } } - before do - allow(described_class).to receive(:config).and_return({ - INSTITUTION: { - host: 'https://example.com/', - params: { - f: { dct_provenance_s: ['INSTITUTION'] } - } - } - }) + # Requests are stubbed with webmock; make sure none of them escape + around do |example| + WebMock.disable_net_connect! + example.run + WebMock.allow_net_connect! end + before { allow(described_class).to receive(:config).and_return(config) } + describe '.configure' do around do |example| previous = described_class.instance_variable_get(:@config) @@ -53,9 +56,7 @@ describe '#index' do before do - expect(Net::HTTP).to receive(:get).with( - URI('https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100&page=1') - ).and_return(stub_json_response) + stub_request(:get, "#{base_url}&page=1").to_return(body: stub_json_response) allow(RSolr).to receive(:connect).and_return(stub_solr_connection) end @@ -121,11 +122,7 @@ end describe '#each_document' do - before do - expect(Net::HTTP).to receive(:get).with( - URI('https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100&page=1') - ).and_return(stub_json_response) - end + before { stub_request(:get, "#{base_url}&page=1").to_return(body: stub_json_response) } let(:docs) { [{ 'layer_slug_s' => 'abc-123', 'score' => 0.1 }, { 'layer_slug_s' => 'abc-321' }] } let(:transformed_docs) { [{ 'layer_slug_s' => 'abc-123' }, { 'layer_slug_s' => 'abc-321' }] } @@ -154,6 +151,91 @@ end end + describe 'HttpClient' do + let(:client) { described_class::HttpClient.new(crawl_delay:, logger:) } + let(:crawl_delay) { 1 } + + before do + stub_request(:get, 'https://example.com/catalog/abc-123/raw').to_return(body: '{"id":"abc-123"}') + stub_request(:get, 'https://example.com/catalog/abc-321/raw').to_return(body: '{"id":"abc-321"}') + allow(client).to receive(:sleep) + end + + it 'parses the JSON response body' do + expect(client.get_json('https://example.com/catalog/abc-123/raw')).to eq('id' => 'abc-123') + end + + it 'waits out the crawl delay before every request, not just every page of results' do + client.get_json('https://example.com/catalog/abc-123/raw') + client.get_json('https://example.com/catalog/abc-321/raw') + + expect(client).to have_received(:sleep).with(1.0).twice + end + + it 'uses a new connection for each request' do + allow(Net::HTTP).to receive(:get_response).and_call_original + + client.get_json('https://example.com/catalog/abc-123/raw') + client.get_json('https://example.com/catalog/abc-321/raw') + + # Net::HTTP.get_response opens and closes a connection per call, rather + # than holding one open across requests the way a WAF tends to dislike + expect(Net::HTTP).to have_received(:get_response).twice + end + + context 'when the crawl delay is fractional' do + let(:crawl_delay) { '0.5' } + + it 'waits that fraction of a second' do + client.get_json('https://example.com/catalog/abc-123/raw') + + expect(client).to have_received(:sleep).with(0.5) + end + end + + context 'when no crawl delay is configured' do + let(:crawl_delay) { nil } + + it 'does not wait between requests' do + client.get_json('https://example.com/catalog/abc-123/raw') + + expect(client).not_to have_received(:sleep) + end + end + end + + describe 'crawl delay configuration' do + let(:client) { instance_double(described_class::HttpClient) } + + before do + allow(described_class::HttpClient).to receive(:new).and_return(client) + allow(client).to receive(:get_json).and_return( + { 'response' => { 'docs' => [], 'pages' => { 'current_page' => 1, 'total_pages' => 1 } } } + ) + end + + context 'when the site configures a crawl delay' do + let(:site_config) { super().merge(crawl_delay: 2) } + let(:config) { { crawl_delay: 1, INSTITUTION: site_config } } + + it 'prefers the site crawl delay over the global one' do + harvester.each_document.to_a + + expect(described_class::HttpClient).to have_received(:new).with(crawl_delay: 2, logger:) + end + end + + context 'when only a global crawl delay is configured' do + let(:config) { { crawl_delay: 1, INSTITUTION: site_config } } + + it 'uses the global crawl delay' do + harvester.each_document.to_a + + expect(described_class::HttpClient).to have_received(:new).with(crawl_delay: 1, logger:) + end + end + end + describe 'BlacklightResponseVersionFactory' do let(:version_class) { described_class::BlacklightResponseVersionFactory.call(json) } @@ -190,12 +272,17 @@ { 'response' => { 'docs' => second_docs, 'pages' => { 'current_page' => 2, 'total_pages' => 2 } } } end + it 'gives the client it builds by default the logger it was given' do + allow(described_class::HttpClient).to receive(:new).and_call_original + + described_class::LegacyBlacklightResponse.new(response: stub_first_response, base_url:, logger:) + + expect(described_class::HttpClient).to have_received(:new).with(logger:) + end + describe '#documents' do it 'pages through the response and returns all the documents' do - expect(Net::HTTP).to receive(:get).with( - URI('https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100&page=2') - ).and_return(stub_second_response.to_json) - base_url = 'https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100' + stub_request(:get, "#{base_url}&page=2").to_return(body: stub_second_response.to_json) docs = described_class::LegacyBlacklightResponse.new(response: stub_first_response, base_url:).documents @@ -203,8 +290,7 @@ end it 'stops paging and logs when a request fails' do - allow(Net::HTTP).to receive(:get).and_raise(SocketError, 'no route to host') - base_url = 'https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100' + stub_request(:get, "#{base_url}&page=2").to_raise(SocketError.new('no route to host')) docs = described_class::LegacyBlacklightResponse.new(response: stub_first_response, base_url:, logger:).documents @@ -217,9 +303,10 @@ describe 'ModernBlacklightResponse' do before do allow(RSolr).to receive(:connect).and_return(stub_solr_connection) - expect(Net::HTTP).to receive(:get).with( - URI('https://example.com/catalog.json?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&per_page=100&page=2&format=json') - ).and_return(second_results_response.to_json) + stub_request( + :get, + 'https://example.com/catalog.json?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&per_page=100&page=2&format=json' + ).to_return(body: second_results_response.to_json) end let(:first_results_response) do @@ -237,15 +324,22 @@ ] } end + it 'gives the client it builds by default the logger it was given' do + allow(described_class::HttpClient).to receive(:new).and_call_original + + described_class::ModernBlacklightResponse.new(response: first_results_response, base_url:, logger:) + + expect(described_class::HttpClient).to have_received(:new).with(logger:) + end + describe '#documents' do it 'pages through the response and fetches documents for each "link" on the response data' do %w[abc-123 abc-321 xyz-123 xyz-321].each do |id| - expect(Net::HTTP).to receive(:get).with( - URI("https://example.com/catalog/#{id}/raw") - ).and_return({ 'layer_slug_s' => id }.to_json) + stub_request(:get, "https://example.com/catalog/#{id}/raw").to_return( + body: { 'layer_slug_s' => id }.to_json + ) end - base_url = 'https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100' docs = described_class::ModernBlacklightResponse.new(response: first_results_response, base_url:).documents @@ -262,7 +356,6 @@ described_class::ModernBlacklightResponse.new(response:, base_url:, logger:).documents.to_a end - let(:base_url) { 'https://example.com?f%5Bdct_provenance_s%5D%5B%5D=INSTITUTION&format=json&per_page=100' } let(:next_url) { 'https://example.com/catalog.json?page=2' } before { allow(RSolr).to receive(:connect).and_return(stub_solr_connection) } @@ -276,10 +369,12 @@ end before do - allow(Net::HTTP).to receive(:get).with(URI('https://example.com/catalog/abc-123/raw')) - .and_return({ 'layer_slug_s' => 'abc-123' }.to_json) - allow(Net::HTTP).to receive(:get).with(URI('https://example.com/catalog/abc-321/raw')) - .and_raise(SocketError, 'connection reset') + stub_request(:get, 'https://example.com/catalog/abc-123/raw').to_return( + body: { 'layer_slug_s' => 'abc-123' }.to_json + ) + stub_request(:get, 'https://example.com/catalog/abc-321/raw').to_raise( + SocketError.new('connection reset') + ) end it 'drops that document and keeps the rest' do @@ -299,10 +394,10 @@ end before do - allow(Net::HTTP).to receive(:get).with(URI('https://example.com/catalog/abc-123/raw')) - .and_return({ 'layer_slug_s' => 'abc-123' }.to_json) - allow(Net::HTTP).to receive(:get).with(URI("#{next_url}&format=json")) - .and_raise(SocketError, 'no route to host') + stub_request(:get, 'https://example.com/catalog/abc-123/raw').to_return( + body: { 'layer_slug_s' => 'abc-123' }.to_json + ) + stub_request(:get, "#{next_url}&format=json").to_raise(SocketError.new('no route to host')) end it 'stops paging and returns what it already had' do