Fix issues found during the connection-reuse review

.github/workflows/ci.yml

@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 name: CI
 
 on:
@@ -12,11 +10,12 @@ jobs:
   test:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
-        ruby-version: ["3.0", "3.1", "3.2", "3.3"]
+        ruby-version: ["3.0", "3.1", "3.2", "3.3", "3.4", "4.0"]
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v5
 
       - name: Set up Ruby ${{ matrix.ruby-version }}
         uses: ruby/setup-ruby@v1

.github/workflows/release.yml

@@ -4,8 +4,6 @@ on:
   push:
     tags:
       - "v*"
-    branches:
-      - main
   workflow_dispatch:
     inputs:
       bump:
@@ -25,7 +23,7 @@ jobs:
       contents: write
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v5
 
       - name: Set up Ruby
         uses: ruby/setup-ruby@v1
@@ -67,7 +65,7 @@ jobs:
       id-token: write
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v5
         with:
           ref: ${{ github.event_name == 'workflow_dispatch' && 'main' || github.ref }}
 

CHANGELOG.md

@@ -0,0 +1,26 @@
+# Change Log
+
+## Unreleased
+
+### Added
+
+- Connection reuse: an `IPData::Client` keeps one persistent HTTPS connection (HTTP/1.1 keep-alive) for its lifetime instead of opening a new TLS connection for every request. Create one client and reuse it. New option `keep_alive_timeout:` (seconds, default 60) for the client-side idle timeout, and new method `Client#close` to finish the connection at shutdown; the client reconnects on demand. A client is safe to share between threads (requests are serialised on its connection) and reconnects in a forked child without closing the parent's socket.
+- `Response#is_satellite` and `Response#is_anycast`: the API sends these top-level booleans only when they are true; the readers return `false` when the key is absent, while `response["is_satellite"]`, `key?` and `to_h` reflect the raw response. Both names can be requested through `fields:` and `select_field:`.
+- `threat.blocklists` entries carry `first_seen`, `last_seen` (`"YYYY-MM-DD"`, UTC) and `days_seen` when the feed reports them; some feeds omit all three.
+
+### Changed
+
+- `timeout:` now also applies to writes (previously only to connecting and reading).
+- `bulk_lookup` (`POST /bulk`) is retried once, on a fresh connection, when a reused connection died before any response byte arrived; `lookup` (`GET`) was already retried once by `Net::HTTP`. Nothing is retried on an HTTP error status.
+- The `User-Agent` header is now `ipdata-ruby/<version> Ruby/<ruby-version> (<platform>)` instead of `ruby-ipdata/...`.
+- Gem metadata: the homepage is now https://ipdata.co; `source_code_uri` keeps pointing at this repository.
+
+### Fixed
+
+- `use_ssl` follows the scheme of `base_url:`, so plain `http://` base URLs work; it used to be always on.
+- `lookup(select_field: "city")` without an IP address requests `/city` instead of `//city`.
+- A `base_url:` with a path prefix (for example `https://example.com/ipdata`) keeps that prefix in front of every request path instead of losing it.
+
+## 0.1.5
+
+Initial release on RubyGems: `lookup` for the caller's own IP or a given IP with `fields:` and `select_field:`, `bulk_lookup` for up to 100 addresses, `Client.eu` for the EU endpoint, an in-memory LRU cache with TTL, typed errors (`AuthenticationError`, `ForbiddenError`, `BadRequestError`) and no runtime dependencies.

ipdata.gemspec

@@ -12,7 +12,7 @@ Gem::Specification.new do |spec|
   spec.description   = "Official Ruby SDK for the ipdata.co API. " \
                         "Look up geolocation, threat intelligence, ASN, company, " \
                         "carrier, currency, timezone and language data for any IP address."
-  spec.homepage      = "https://github.com/ipdata/ruby"
+  spec.homepage      = "https://ipdata.co"
   spec.license       = "MIT"
 
   spec.required_ruby_version = ">= 3.0"
@@ -25,8 +25,8 @@ Gem::Specification.new do |spec|
     "documentation_uri" => "https://docs.ipdata.co"
   }
 
-  spec.files = Dir["lib/**/*.rb", "LICENSE", "README.md"]
+  spec.files = Dir["lib/**/*.rb", "CHANGELOG.md", "LICENSE", "README.md"]
   spec.require_paths = ["lib"]
 
-  # Zero runtime dependencies — uses only Ruby stdlib (net/http, json, uri, monitor)
+  # Zero runtime dependencies — uses only Ruby stdlib (net/http, openssl, json, uri, monitor)
 end

lib/ipdata/client.rb

@@ -42,7 +42,8 @@ class Client
     # @param api_key [String] Your ipdata API key
     # @param cache [IPData::Cache, nil] Optional LRU cache instance
     # @param timeout [Integer] HTTP open, read and write timeout in seconds (default: 30)
-    # @param base_url [String, nil] Override the base URL (default: global endpoint)
+    # @param base_url [String, nil] Override the base URL (default: global endpoint).
+    #   A path prefix, if the URL has one, is kept in front of every request path
     # @param keep_alive_timeout [Integer] Seconds an idle connection is kept before
     #   the next request opens a new one (default: 60)
     def initialize(api_key, cache: nil, timeout: 30, base_url: nil, keep_alive_timeout: 60)
@@ -53,6 +54,7 @@ def initialize(api_key, cache: nil, timeout: 30, base_url: nil, keep_alive_timeo
       @timeout = timeout
       @keep_alive_timeout = keep_alive_timeout
       @base_uri = URI.parse(base_url || BASE_URL)
+      @base_path = @base_uri.path.chomp("/")
       @mutex = Mutex.new
       @connection = nil
       @connection_pid = nil
@@ -74,23 +76,18 @@ def self.eu(api_key, **opts)
     # @param select_field [String, nil] A single field to extract (returns raw value)
     # @return [Response, Object] Response object, or raw value if select_field is used
     def lookup(ip = nil, fields: nil, select_field: nil)
-      if select_field
-        path = "/#{ip}/#{select_field}"
-        data = get(path)
-        return data
-      end
+      return get(request_path(ip, select_field)) if select_field
 
       cache_key = build_cache_key(ip, fields)
       if @cache
         cached = @cache.get(cache_key)
         return cached if cached
       end
 
-      path = ip ? "/#{ip}" : "/"
       params = {}
       params["fields"] = fields.join(",") if fields && !fields.empty?
 
-      data = get(path, params)
+      data = get(request_path(ip), params)
       response = Response.new(data)
 
       @cache&.set(cache_key, response)
@@ -110,7 +107,7 @@ def bulk_lookup(ips, fields: nil)
       params = {}
       params["fields"] = fields.join(",") if fields && !fields.empty?
 
-      data = post("/bulk", ips, params)
+      data = post(request_path("bulk"), ips, params)
       data.map { |entry| Response.new(entry) }
     end
 
@@ -127,7 +124,7 @@ def close
     private
 
     def user_agent
-      @user_agent ||= "ruby-ipdata/#{VERSION} Ruby/#{RUBY_VERSION} (#{RUBY_PLATFORM})"
+      @user_agent ||= "ipdata-ruby/#{VERSION} Ruby/#{RUBY_VERSION} (#{RUBY_PLATFORM})"
     end
 
     def build_cache_key(ip, fields)
@@ -136,6 +133,13 @@ def build_cache_key(ip, fields)
       key
     end
 
+    # Path of a request made of +segments+, under the path prefix of the base
+    # URL if it has one. Nil segments are skipped, so a lookup of the caller's
+    # own IP uses "/" and its single-field form "/<field>", not "//<field>".
+    def request_path(*segments)
+      "#{@base_path}/#{segments.compact.join('/')}"
+    end
+
     def build_uri(path, params = {})
       uri = @base_uri.dup
       uri.path = path

test/test_client.rb

@@ -31,6 +31,27 @@ def test_custom_base_url
     assert_instance_of IPData::Client, client
   end
 
+  def test_base_url_path_prefix_is_kept_in_request_paths
+    prefixed = -> { IPData::Client.new(API_KEY, base_url: "https://gateway.example.com/ipdata") }
+
+    assert_request(:get, "/ipdata/8.8.8.8", query: { "api-key" => API_KEY }) { prefixed.call.lookup("8.8.8.8") }
+    assert_request(:get, "/ipdata/", query: { "api-key" => API_KEY }) { prefixed.call.lookup }
+    assert_request(:get, "/ipdata/8.8.8.8/city", query: { "api-key" => API_KEY }, body: '"Mountain View"') do
+      assert_equal "Mountain View", prefixed.call.lookup("8.8.8.8", select_field: "city")
+    end
+    assert_request(:post, "/ipdata/bulk", query: { "api-key" => API_KEY }, body: JSON.generate(SAMPLE_BULK_RESPONSE)) do
+      assert_equal 2, prefixed.call.bulk_lookup(["8.8.8.8", "1.1.1.1"]).size
+    end
+  end
+
+  def test_base_url_trailing_slash_does_not_double_the_path_separator
+    client = IPData::Client.new(API_KEY, base_url: "https://gateway.example.com/ipdata/")
+    assert_request(:get, "/ipdata/8.8.8.8", query: { "api-key" => API_KEY }) { client.lookup("8.8.8.8") }
+
+    client = IPData::Client.new(API_KEY, base_url: "https://api.ipdata.co/")
+    assert_request(:get, "/8.8.8.8", query: { "api-key" => API_KEY }) { client.lookup("8.8.8.8") }
+  end
+
   def test_keep_alive_timeout_option
     client = IPData::Client.new(API_KEY, keep_alive_timeout: 30)
     assert_instance_of IPData::Client, client
@@ -68,6 +89,12 @@ def test_lookup_with_select_field
     end
   end
 
+  def test_lookup_with_select_field_and_no_ip_uses_a_single_segment_path
+    assert_request(:get, "/city", query: { "api-key" => API_KEY }, body: '"Mountain View"') do
+      assert_equal "Mountain View", @client.lookup(select_field: "city")
+    end
+  end
+
   # --- bulk_lookup ---
 
   def test_bulk_lookup
@@ -119,7 +146,7 @@ def test_eu_client_uses_eu_base_url
     client = IPData::Client.eu(API_KEY)
 
     called = false
-    Net::HTTP.stub(:new, lambda { |host, port|
+    with_stub(Net::HTTP, :new, lambda { |host, port|
       assert_equal "eu-api.ipdata.co", host
       assert_equal 443, port
       called = true
@@ -155,7 +182,7 @@ def test_lookup_uses_cache
     client = IPData::Client.new(API_KEY, cache: cache)
     mock = mock_http(stub_http_response)
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       # First call hits the API
       client.lookup("8.8.8.8")
       assert_equal 1, mock.requests.size
@@ -171,7 +198,7 @@ def test_select_field_does_not_use_cache
     client = IPData::Client.new(API_KEY, cache: cache)
     mock = mock_http(stub_http_response(body: '"Mountain View"'))
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       client.lookup("8.8.8.8", select_field: "city")
       client.lookup("8.8.8.8", select_field: "city")
       assert_equal 2, mock.requests.size
@@ -183,7 +210,7 @@ def test_different_fields_have_different_cache_keys
     client = IPData::Client.new(API_KEY, cache: cache)
     mock = mock_http(stub_http_response)
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       client.lookup("8.8.8.8", fields: ["ip"])
       client.lookup("8.8.8.8", fields: ["ip", "city"])
       assert_equal 2, mock.requests.size
@@ -199,11 +226,11 @@ def test_user_agent_header
       captured_request = req
     end
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       @client.lookup("8.8.8.8")
     end
 
-    assert_match(/ruby-ipdata\/#{IPData::VERSION}/, captured_request["User-Agent"])
+    assert_match(%r{\Aipdata-ruby/#{Regexp.escape(IPData::VERSION)} }, captured_request["User-Agent"])
     assert_match(/Ruby\/#{RUBY_VERSION}/, captured_request["User-Agent"])
   end
 
@@ -216,7 +243,7 @@ def test_accept_json_header
       captured_request = req
     end
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       @client.lookup("8.8.8.8")
     end
 
@@ -310,7 +337,7 @@ def test_pid_change_reconnects_without_finishing_the_inherited_connection
     connections = stub_connections do
       @client.lookup("8.8.8.8")
       # Pretend the client was inherited by a forked child process.
-      Process.stub(:pid, Process.pid + 1) { @client.lookup("8.8.8.8") }
+      with_stub(Process, :pid, Process.pid + 1) { @client.lookup("8.8.8.8") }
     end
 
     assert_equal 2, connections.size
@@ -324,7 +351,7 @@ def test_pid_change_reconnects_without_finishing_the_inherited_connection
   def test_close_in_a_forked_child_leaves_the_inherited_connection_alone
     connections = stub_connections { @client.lookup("8.8.8.8") }
 
-    Process.stub(:pid, Process.pid + 1) { @client.close }
+    with_stub(Process, :pid, Process.pid + 1) { @client.close }
 
     assert connections.first.started?
     assert_equal 0, connections.first.finish_count
@@ -529,12 +556,11 @@ def assert_request(method, path, query: {}, body: nil, request_body: nil)
       captured_request = req
     end
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock }) do
       yield
     end
 
     assert captured_request, "Expected HTTP request to be made"
-    uri = URI.parse(captured_request.path.to_s + "?" + (captured_request.instance_variable_get(:@query) || ""))
 
     # Check path
     request_uri = URI.parse(captured_request.uri.to_s)
@@ -556,7 +582,7 @@ def assert_request(method, path, query: {}, body: nil, request_body: nil)
   def assert_error_response(status, body, error_class)
     http_response = stub_http_response(status: status.to_s, body: body)
 
-    Net::HTTP.stub(:new, ->(_h, _p) { mock_http(http_response) }) do
+    with_stub(Net::HTTP, :new, ->(_h, _p) { mock_http(http_response) }) do
       error = assert_raises(error_class) { @client.lookup("8.8.8.8") }
       assert_equal status, error.status_code
     end
@@ -575,7 +601,7 @@ def stub_connections(*outcomes)
     outcomes = [stub_http_response] if outcomes.empty?
     connections = []
 
-    Net::HTTP.stub(:new, lambda { |host, port|
+    with_stub(Net::HTTP, :new, lambda { |host, port|
       http = mock_http(outcomes.fetch(connections.size, outcomes.last))
       http.address = host
       http.port = port

test/test_helper.rb

@@ -4,8 +4,30 @@
 require "ipdata"
 require "minitest/autorun"
 
-# Shared test fixtures for the ipdata test suite.
+# Shared test fixtures and helpers for the ipdata test suite.
 module IPDataFixtures
+  # Replace the singleton method +name+ of +receiver+ for the duration of the
+  # block, then restore the original. A +replacement+ that responds to +call+
+  # is invoked with the arguments of each call; any other value is returned
+  # as is. This mirrors Object#stub from minitest/mock, which Minitest 6 no
+  # longer ships (Ruby 4.0 bundles Minitest 6), so the suite does not depend
+  # on it.
+  def with_stub(receiver, name, replacement)
+    singleton = receiver.singleton_class
+    original = :"__original_#{name}"
+    singleton.alias_method(original, name)
+    singleton.define_method(name) do |*args, **kwargs, &block|
+      replacement.respond_to?(:call) ? replacement.call(*args, **kwargs, &block) : replacement
+    end
+    yield
+  ensure
+    if singleton.method_defined?(original) || singleton.private_method_defined?(original)
+      singleton.remove_method(name)
+      singleton.alias_method(name, original)
+      singleton.remove_method(original)
+    end
+  end
+
   SAMPLE_RESPONSE = {
     "ip" => "8.8.8.8",
     "is_eu" => false,