diff --git a/CHANGELOG.md b/CHANGELOG.md index fa01a94..dceb7fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ # Changelog + * [Unreleased] Stop `Client.new` from modifying the params hash passed in, and accept frozen hashes * [Unreleased] Add native Extract API support via the `extract` method * [Unreleased] Apply the `timeout` option to persistent connections * [Unreleased] Add Image API support diff --git a/lib/serpapi/client.rb b/lib/serpapi/client.rb index b8ae22a..0f3941e 100644 --- a/lib/serpapi/client.rb +++ b/lib/serpapi/client.rb @@ -74,22 +74,20 @@ def initialize(params = {}) raise SerpApiError, 'params cannot be nil' if params.nil? raise SerpApiError, "params must be hash, not: #{params.class}" unless params.instance_of?(Hash) + # work on a copy so the caller's hash is never modified + # (dup, unlike clone, also returns an unfrozen copy of a frozen hash) + config = params.dup + # store client HTTP request timeout - @timeout = params[:timeout] || 120 + @timeout = config.delete(:timeout) || 120 @timeout.freeze # enable HTTP persistent mode - @persistent = true - @persistent = params[:persistent] if params.key?(:persistent) + @persistent = config.key?(:persistent) ? config.delete(:persistent) : true @persistent.freeze - # delete this client only configuration keys - %i[timeout persistent].each do |option| - params.delete(option) if params.key?(option) - end - # set default query parameters - @params = params.clone || {} + @params = config # track ruby library as a client for statistic purpose @params[:source] = 'serpapi-ruby:' << SerpApi::VERSION diff --git a/spec/serpapi/client/constructor_spec.rb b/spec/serpapi/client/constructor_spec.rb new file mode 100644 index 0000000..6d81883 --- /dev/null +++ b/spec/serpapi/client/constructor_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +# These specs only build clients, so they make no HTTP request to serpapi.com. +describe 'SerpApi::Client constructor' do + it 'does not modify the params hash passed by the caller' do + config = { engine: 'google', api_key: 'test_key', timeout: 10, persistent: false } + original = config.dup + + SerpApi::Client.new(config) + + expect(config).to eq(original) + end + + it 'gives the same settings to every client built from a reused config hash' do + config = { engine: 'google', api_key: 'test_key', timeout: 10, persistent: false } + + first = SerpApi::Client.new(config) + second = SerpApi::Client.new(config) + + expect(second.timeout).to eq(first.timeout) + expect(second.timeout).to eq(10) + expect(second.persistent).to be false + expect(second.socket).to be_nil + end + + it 'accepts a frozen params hash' do + config = { engine: 'google', api_key: 'test_key', timeout: 10, persistent: false }.freeze + + client = SerpApi::Client.new(config) + + expect(client.timeout).to eq(10) + expect(client.persistent).to be false + expect(client.engine).to eq('google') + end + + it 'keeps timeout and persistent out of the query parameters' do + client = SerpApi::Client.new(engine: 'google', api_key: 'test_key', timeout: 10, persistent: false) + + expect(client.params).not_to have_key(:timeout) + expect(client.params).not_to have_key(:persistent) + end + + it 'defaults to a 120 second timeout and persistent mode' do + client = SerpApi::Client.new(engine: 'google', api_key: 'test_key') + + expect(client.timeout).to eq(120) + expect(client.persistent).to be true + expect(client.socket).not_to be_nil + client.close + end +end