This module is now used by the client and server side, rather than having a Handler module that's 90% server functionality but also used by the client.
While we don't automatically get api choice from this, it at least provides a pattern for how we'll handle API development over time. Signed-off-by: Luke Kanies <[email protected]> --- lib/puppet/indirector/rest.rb | 7 +-- lib/puppet/network/http/api.rb | 4 + lib/puppet/network/http/api/v1.rb | 60 ++++++++++++++++++ lib/puppet/network/http/handler.rb | 61 +----------------- spec/unit/indirector/rest.rb | 4 +- spec/unit/network/http/api/v1.rb | 122 ++++++++++++++++++++++++++++++++++++ spec/unit/network/http/handler.rb | 108 +------------------------------- 7 files changed, 197 insertions(+), 169 deletions(-) create mode 100644 lib/puppet/network/http/api.rb create mode 100644 lib/puppet/network/http/api/v1.rb create mode 100644 spec/unit/network/http/api/v1.rb diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 38ecdb7..c3d689e 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -2,11 +2,11 @@ require 'net/http' require 'uri' require 'puppet/network/http_pool' -require 'puppet/network/http/handler' +require 'puppet/network/http/api/v1' # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus - include Puppet::Network::HTTP::Handler + include Puppet::Network::HTTP::API::V1 class << self attr_reader :server_setting, :port_setting @@ -64,9 +64,6 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end def find(request) - p model - p indirection - p indirection.model deserialize network(request).get(indirection2uri(request), headers) end diff --git a/lib/puppet/network/http/api.rb b/lib/puppet/network/http/api.rb new file mode 100644 index 0000000..8b1b747 --- /dev/null +++ b/lib/puppet/network/http/api.rb @@ -0,0 +1,4 @@ +require 'puppet/network/http' + +class Puppet::Network::HTTP::API +end diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb new file mode 100644 index 0000000..2ee1a81 --- /dev/null +++ b/lib/puppet/network/http/api/v1.rb @@ -0,0 +1,60 @@ +require 'puppet/network/http/api' + +module Puppet::Network::HTTP::API::V1 + # How we map http methods and the indirection name in the URI + # to an indirection method. + METHOD_MAP = { + "GET" => { + :plural => :search, + :singular => :find + }, + "PUT" => { + :singular => :save + }, + "DELETE" => { + :singular => :destroy + } + } + + def uri2indirection(http_method, uri, params) + environment, indirection, key = uri.split("/", 4)[1..-1] # the first field is always nil because of the leading slash + + raise ArgumentError, "The environment must be purely alphanumeric, not '%s'" % environment unless environment =~ /^\w+$/ + raise ArgumentError, "The indirection name must be purely alphanumeric, not '%s'" % indirection unless indirection =~ /^\w+$/ + + method = indirection_method(http_method, indirection) + + params[:environment] = environment + + raise ArgumentError, "No request key specified in %s" % uri if key == "" or key.nil? + + key = URI.unescape(key) + + Puppet::Indirector::Request.new(indirection, method, key, params) + end + + def indirection2uri(request) + indirection = request.method == :search ? request.indirection_name.to_s + "s" : request.indirection_name.to_s + "/#{request.environment.to_s}/#{indirection}/#{request.escaped_key}#{request.query_string}" + end + + def indirection_method(http_method, indirection) + unless METHOD_MAP[http_method] + raise ArgumentError, "No support for http method %s" % http_method + end + + unless method = METHOD_MAP[http_method][plurality(indirection)] + raise ArgumentError, "No support for plural %s operations" % http_method + end + + return method + end + + def plurality(indirection) + result = (indirection == handler.to_s + "s") ? :plural : :singular + + indirection.sub!(/s$/, '') if result + + result + end +end diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 610aa0a..76f07ed 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -1,22 +1,10 @@ module Puppet::Network::HTTP end -module Puppet::Network::HTTP::Handler +require 'puppet/network/http/api/v1' - # How we map http methods and the indirection name in the URI - # to an indirection method. - METHOD_MAP = { - "GET" => { - :plural => :search, - :singular => :find - }, - "PUT" => { - :singular => :save - }, - "DELETE" => { - :singular => :destroy - } - } +module Puppet::Network::HTTP::Handler + include Puppet::Network::HTTP::API::V1 attr_reader :model, :server, :handler @@ -54,51 +42,10 @@ module Puppet::Network::HTTP::Handler send("do_%s" % indirection_request.method, indirection_request, request, response) rescue Exception => e + puts e.backtrace return do_exception(response, e) end - def uri2indirection(http_method, uri, params) - environment, indirection, key = uri.split("/", 4)[1..-1] # the first field is always nil because of the leading slash - - raise ArgumentError, "The environment must be purely alphanumeric, not '%s'" % environment unless environment =~ /^\w+$/ - raise ArgumentError, "The indirection name must be purely alphanumeric, not '%s'" % indirection unless indirection =~ /^\w+$/ - - method = indirection_method(http_method, indirection) - - params[:environment] = environment - - raise ArgumentError, "No request key specified in %s" % uri if key == "" or key.nil? - - key = URI.unescape(key) - - Puppet::Indirector::Request.new(indirection, method, key, params) - end - - def indirection2uri(request) - indirection = request.method == :search ? request.indirection_name.to_s + "s" : request.indirection_name.to_s - "/#{request.environment.to_s}/#{indirection}/#{request.escaped_key}#{request.query_string}" - end - - def indirection_method(http_method, indirection) - unless METHOD_MAP[http_method] - raise ArgumentError, "No support for http method %s" % http_method - end - - unless method = METHOD_MAP[http_method][plurality(indirection)] - raise ArgumentError, "No support for plural %s operations" % http_method - end - - return method - end - - def plurality(indirection) - result = (indirection == handler.to_s + "s") ? :plural : :singular - - indirection.sub!(/s$/, '') if result - - result - end - # Set the response up, with the body and status. def set_response(response, body, status = 200) raise NotImplementedError diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index 9305a71..d135b8d 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -46,8 +46,8 @@ describe Puppet::Indirector::REST do @searcher.stubs(:model).returns @model end - it "should include the Http Handler module" do - Puppet::Indirector::REST.ancestors.should be_include(Puppet::Network::HTTP::Handler) + it "should include the v1 REST API module" do + Puppet::Indirector::REST.ancestors.should be_include(Puppet::Network::HTTP::API::V1) end it "should have a method for specifying what setting a subclass should use to retrieve its server" do diff --git a/spec/unit/network/http/api/v1.rb b/spec/unit/network/http/api/v1.rb new file mode 100644 index 0000000..fc284de --- /dev/null +++ b/spec/unit/network/http/api/v1.rb @@ -0,0 +1,122 @@ +#!/usr/bin/env ruby + +Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") } + +require 'puppet/network/http/api/v1' + +class V1RestApiTester + include Puppet::Network::HTTP::API::V1 +end + +describe Puppet::Network::HTTP::API::V1 do + before do + @tester = V1RestApiTester.new + end + + it "should be able to convert a URI into a request" do + @tester.should respond_to(:uri2indirection) + end + + it "should be able to convert a request into a URI" do + @tester.should respond_to(:indirection2uri) + end + + describe "when converting a URI into a request" do + before do + @tester.stubs(:handler).returns "foo" + end + + it "should require the http method, the URI, and the query parameters" do + # Not a terribly useful test, but an important statement for the spec + lambda { @tester.uri2indirection("/foo") }.should raise_error(ArgumentError) + end + + it "should use the first field of the URI as the environment" do + @tester.uri2indirection("GET", "/env/foo/bar", {}).environment.should == Puppet::Node::Environment.new("env") + end + + it "should fail if the environment is not alphanumeric" do + lambda { @tester.uri2indirection("GET", "/env ness/foo/bar", {}) }.should raise_error(ArgumentError) + end + + it "should use the environment from the URI even if one is specified in the parameters" do + @tester.uri2indirection("GET", "/env/foo/bar", {:environment => "otherenv"}).environment.should == Puppet::Node::Environment.new("env") + end + + it "should use the second field of the URI as the indirection name" do + @tester.uri2indirection("GET", "/env/foo/bar", {}).indirection_name.should == :foo + end + + it "should fail if the indirection name is not alphanumeric" do + lambda { @tester.uri2indirection("GET", "/env/foo ness/bar", {}) }.should raise_error(ArgumentError) + end + + it "should use the remainder of the URI as the indirection key" do + @tester.uri2indirection("GET", "/env/foo/bar", {}).key.should == "bar" + end + + it "should support the indirection key being a /-separated file path" do + @tester.uri2indirection("GET", "/env/foo/bee/baz/bomb", {}).key.should == "bee/baz/bomb" + end + + it "should fail if no indirection key is specified" do + lambda { @tester.uri2indirection("GET", "/env/foo/", {}) }.should raise_error(ArgumentError) + lambda { @tester.uri2indirection("GET", "/env/foo", {}) }.should raise_error(ArgumentError) + end + + it "should choose 'find' as the indirection method if the http method is a GET and the indirection name is singular" do + @tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find + end + + it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do + @tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search + end + + it "should choose 'delete' as the indirection method if the http method is a DELETE and the indirection name is singular" do + @tester.uri2indirection("DELETE", "/env/foo/bar", {}).method.should == :destroy + end + + it "should choose 'save' as the indirection method if the http method is a PUT and the indirection name is singular" do + @tester.uri2indirection("PUT", "/env/foo/bar", {}).method.should == :save + end + + it "should fail if an indirection method cannot be picked" do + lambda { @tester.uri2indirection("UPDATE", "/env/foo/bar", {}) }.should raise_error(ArgumentError) + end + + it "should URI unescape the indirection key" do + escaped = URI.escape("foo bar") + @tester.uri2indirection("GET", "/env/foo/#{escaped}", {}).key.should == "foo bar" + end + end + + describe "when converting a request into a URI" do + before do + @request = Puppet::Indirector::Request.new(:foo, :find, "with spaces", :foo => :bar, :environment => "myenv") + end + + it "should use the environment as the first field of the URI" do + @tester.indirection2uri(@request).split("/")[1].should == "myenv" + end + + it "should use the indirection as the second field of the URI" do + @tester.indirection2uri(@request).split("/")[2].should == "foo" + end + + it "should pluralize the indirection name if the method is 'search'" do + @request.stubs(:method).returns :search + @tester.indirection2uri(@request).split("/")[2].should == "foos" + end + + it "should use the escaped key as the remainder of the URI" do + escaped = URI.escape("with spaces") + @tester.indirection2uri(@request).split("/")[3].sub(/\?.+/, '').should == escaped + end + + it "should add the query string to the URI" do + @request.expects(:query_string).returns "?query" + @tester.indirection2uri(@request).should =~ /\?query$/ + end + end + +end diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index aa3e13d..85149b6 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -11,111 +11,9 @@ describe Puppet::Network::HTTP::Handler do before do @handler = HttpHandled.new end - - it "should be able to convert a URI into a request" do - @handler.should respond_to(:uri2indirection) - end - - it "should be able to convert a request into a URI" do - @handler.should respond_to(:indirection2uri) - end - - describe "when converting a URI into a request" do - before do - @handler.stubs(:handler).returns "foo" - end - - it "should require the http method, the URI, and the query parameters" do - # Not a terribly useful test, but an important statement for the spec - lambda { @handler.uri2indirection("/foo") }.should raise_error(ArgumentError) - end - - it "should use the first field of the URI as the environment" do - @handler.uri2indirection("GET", "/env/foo/bar", {}).environment.should == Puppet::Node::Environment.new("env") - end - - it "should fail if the environment is not alphanumeric" do - lambda { @handler.uri2indirection("GET", "/env ness/foo/bar", {}) }.should raise_error(ArgumentError) - end - - it "should use the environment from the URI even if one is specified in the parameters" do - @handler.uri2indirection("GET", "/env/foo/bar", {:environment => "otherenv"}).environment.should == Puppet::Node::Environment.new("env") - end - - it "should use the second field of the URI as the indirection name" do - @handler.uri2indirection("GET", "/env/foo/bar", {}).indirection_name.should == :foo - end - - it "should fail if the indirection name is not alphanumeric" do - lambda { @handler.uri2indirection("GET", "/env/foo ness/bar", {}) }.should raise_error(ArgumentError) - end - - it "should use the remainder of the URI as the indirection key" do - @handler.uri2indirection("GET", "/env/foo/bar", {}).key.should == "bar" - end - - it "should support the indirection key being a /-separated file path" do - @handler.uri2indirection("GET", "/env/foo/bee/baz/bomb", {}).key.should == "bee/baz/bomb" - end - - it "should fail if no indirection key is specified" do - lambda { @handler.uri2indirection("GET", "/env/foo/", {}) }.should raise_error(ArgumentError) - lambda { @handler.uri2indirection("GET", "/env/foo", {}) }.should raise_error(ArgumentError) - end - - it "should choose 'find' as the indirection method if the http method is a GET and the indirection name is singular" do - @handler.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find - end - - it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do - @handler.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search - end - - it "should choose 'delete' as the indirection method if the http method is a DELETE and the indirection name is singular" do - @handler.uri2indirection("DELETE", "/env/foo/bar", {}).method.should == :destroy - end - - it "should choose 'save' as the indirection method if the http method is a PUT and the indirection name is singular" do - @handler.uri2indirection("PUT", "/env/foo/bar", {}).method.should == :save - end - - it "should fail if an indirection method cannot be picked" do - lambda { @handler.uri2indirection("UPDATE", "/env/foo/bar", {}) }.should raise_error(ArgumentError) - end - - it "should URI unescape the indirection key" do - escaped = URI.escape("foo bar") - @handler.uri2indirection("GET", "/env/foo/#{escaped}", {}).key.should == "foo bar" - end - end - - describe "when converting a request into a URI" do - before do - @request = Puppet::Indirector::Request.new(:foo, :find, "with spaces", :foo => :bar, :environment => "myenv") - end - - it "should use the environment as the first field of the URI" do - @handler.indirection2uri(@request).split("/")[1].should == "myenv" - end - - it "should use the indirection as the second field of the URI" do - @handler.indirection2uri(@request).split("/")[2].should == "foo" - end - - it "should pluralize the indirection name if the method is 'search'" do - @request.stubs(:method).returns :search - @handler.indirection2uri(@request).split("/")[2].should == "foos" - end - - it "should use the escaped key as the remainder of the URI" do - escaped = URI.escape("with spaces") - @handler.indirection2uri(@request).split("/")[3].sub(/\?.+/, '').should == escaped - end - - it "should add the query string to the URI" do - @request.expects(:query_string).returns "?query" - @handler.indirection2uri(@request).should =~ /\?query$/ - end + + it "should include the v1 REST API" do + Puppet::Network::HTTP::Handler.ancestors.should be_include(Puppet::Network::HTTP::API::V1) end it "should have a method for initializing" do -- 1.6.1 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
