Commit ad0905ab authored by Ruth Linehan's avatar Ruth Linehan

(PE-18347) Handle multiple arities for `get` in http client class

Prior to Puppet Server 5, Puppet Server's http client class had an
implementation of `get` with an arity of 2 (where `path` and `headers` are the
arguments). However, in Ruby Puppet and in Puppet Server 5, the http client
class has an implementation of `get` with an arity of 3 (with arguments
`path`, `headers`, and `options`).

Since we do not know under which version of Puppet Server or the Ruby Puppet
master this terminus code will be run under, this commit adds a
`multi_arity_get` helper function that inspects the `http_instance` class to
determine the arity of `get` before calling it with the appropriate arguments
for its arity.
parent a98f40a4
......@@ -48,7 +48,10 @@ class Puppet::Node::Facts::Puppetdb < Puppet::Indirector::REST
response = Http.action("/pdb/query/v4/nodes/#{CGI.escape(request.key)}/facts", :query) do |http_instance, path|
profile("Query for nodes facts: #{URI.unescape(path)}",
[:puppetdb, :facts, :find, :query_nodes, request.key]) do
http_instance.get(path, headers, {:metric_id => [:puppetdb, :facts, :find, request.key]})
Http.multi_arity_get(http_instance,
path,
headers,
{ :metric_id => [:puppetdb, :facts, :find, request.key] })
end
end
log_x_deprecation_header(response)
......@@ -116,7 +119,10 @@ class Puppet::Node::Facts::Puppetdb < Puppet::Indirector::REST
response = Http.action("/pdb/query/v4/nodes?query=#{query_param}", :query) do |http_instance, path|
profile("Fact query request: #{URI.unescape(path)}",
[:puppetdb, :facts, :search, :query_request, request.key]) do
http_instance.get(path, headers, {:metric_id => [:puppetdb, :facts, :search]})
Http.multi_arity_get(http_instance,
path,
headers,
{ :metric_id => [:puppetdb, :facts, :search] })
end
end
log_x_deprecation_header(response)
......
......@@ -30,7 +30,10 @@ class Puppet::Resource::Puppetdb < Puppet::Indirector::REST
response = Http.action("/pdb/query/v4/resources?query=#{query_param}", :query) do |http_instance, path|
profile("Resources query: #{URI.unescape(path)}",
[:puppetdb, :resource, :search, :query, request.key]) do
http_instance.get(path, headers, { :metric_id => [:puppetdb, :resource, :search, request.key] })
Http.multi_arity_get(http_instance,
path,
headers,
{ :metric_id => [:puppetdb, :resource, :search, request.key] })
end
end
......
......@@ -222,6 +222,29 @@ module Puppet::Util::Puppetdb
@@last_good_query_server_url_index.reset(0)
end
# This function is a bit of a hack to get around the fact that prior to
# Puppet Server 5, Puppet Server's http client's `get` method only took
# two arguments (both required), whereas the Ruby implementation takes
# three arguments (two are optional).
#
# Since we don't have any guarantees over what version of Puppet/Puppet
# Server this terminus code will run under, this method inspects the `get`
# method to check its arity, and then calls it with the correct number of
# arguments.
def self.multi_arity_get(http_instance, path, headers, options={})
# `parameters()` returns the parameters of a method as an array of
# tuples, with a tuple for each parameter, e.g. `[[:req :a], [:opt
# :b]]`. Counting this array gives the arity.
arity = http_instance.class.instance_method(:get).parameters.count
if arity == 2
http_instance.get(path, headers)
elsif arity == 3
http_instance.get(path, headers, options)
else
raise Puppet::Error,
"Http client `get` method expected to have arity 2 or 3 but was arity #{arity}"
end
end
end
class NotFoundError < Puppet::Error
......
......@@ -13,6 +13,14 @@ require 'puppet'
require 'puppet/util/puppetdb'
require 'puppet/util/log'
# A mock class for an http_instance so that the `multi_arity_get` function
# will work in tests.
class HttpClientTest
def get(path, headers, options)
end
end
def create_environmentdir(environment)
envdir = File.join(Puppet[:environmentpath], environment)
if not Dir.exists?(envdir)
......
......@@ -13,7 +13,7 @@ describe Puppet::Node::Facts::Puppetdb do
CommandReplaceFacts = Puppet::Util::Puppetdb::CommandNames::CommandReplaceFacts
let(:http) {stub 'http'}
let(:http) { HttpClientTest.new }
before :each do
Puppet::Util::Puppetdb.config.stubs(:server_urls).returns [URI("https://localhost:8282")]
......
......@@ -28,7 +28,7 @@ describe Puppet::Resource::Puppetdb do
response.stubs(:body).returns '[]'
query = CGI.escape(["and", ["=", "type", "exec"], ["=", "exported", true], ["not", ["=", "certname", "default.local"]]].to_json)
http = stub 'http'
http = HttpClientTest.new
Puppet::Network::HttpPool.stubs(:http_instance).returns(http)
http.stubs(:get).with("/pdb/query/v4/resources?query=#{query}", subject.headers, options).returns response
......@@ -42,7 +42,7 @@ describe Puppet::Resource::Puppetdb do
response.stubs(:body).returns '[]'
query = CGI.escape(["and", ["=", "type", "exec"], ["=", "exported", true], ["not", ["=", "certname", "default.local"]]].to_json)
http = stub 'http'
http = HttpClientTest.new
Puppet::Network::HttpPool.stubs(:http_instance).returns(http)
http.stubs(:get).with("/pdb/query/v4/resources?query=#{query}", subject.headers, options).returns response
......@@ -89,6 +89,7 @@ describe Puppet::Resource::Puppetdb do
response.stubs(:body).returns body
http = stub 'http'
http = HttpClientTest.new
Puppet::Network::HttpPool.stubs(:http_instance).returns(http)
http.stubs(:get).with("/pdb/query/v4/resources?query=#{CGI.escape(query.to_json)}", subject.headers, options).returns response
end
......
......@@ -383,4 +383,63 @@ describe Puppet::Util::Puppetdb::Http do
end
end
describe "#multi_arity_get" do
let(:path) { "/path" }
let(:headers) { {"header" => "foo"} }
let(:options) { {"options" => "abc"} }
describe "when http client class has .get with arity 2" do
let(:http_client_class) { Class.new do
def get(path, headers)
end
end }
let(:http_instance) { http_client_class.new }
it "calls .get with path and headers" do
http_instance.expects(:get).with(path, headers)
described_class.multi_arity_get(http_instance, path, headers, options)
end
end
describe "when http client class has .get with arity 3" do
let(:http_client_class) { Class.new do
def get(path, headers, options)
end
end }
let(:http_instance) { http_client_class.new }
it "calls .get with path, headers, and options" do
http_instance.expects(:get).with(path, headers, options)
described_class.multi_arity_get(http_instance, path, headers, options)
end
end
describe "when http client class has .get with arity 3 with defaults" do
let(:http_client_class) { Class.new do
def get(path, headers, options = {})
end
end }
let(:http_instance) { http_client_class.new }
it "calls .get with path, headers, and options" do
http_instance.expects(:get).with(path, headers, options)
described_class.multi_arity_get(http_instance, path, headers, options)
end
end
describe "when http client class has .get with arity not 2 or 3" do
let(:http_client_class) { Class.new do
def get(wrong)
end
end }
let(:http_instance) { http_client_class.new }
it "raises an exception" do
expect {
described_class.multi_arity_get(http_instance, path, headers, options)
}.to raise_error Puppet::Error, /Http client `get`/
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment