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 <noreply@anthropic.com>

Author: thatbudakguy

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)
 

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