Adding explicit optional attribute to indirection requests

Previously, any option that had a setter was treated
as an attribute, but now we're specifying the list of
attributes settable via options.

We also have a to_hash method that will take all of the
options and all of those attributes and join them
back into a hash.  This method is used by the REST Handler
module, since it uses the indirection request internally.

Signed-off-by: Luke Kanies <luke@madstop.com>

2 files changed, 41 insertions, 9 deletions

diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
index 5c577a05d..2ffed60e2 100644
--- a/lib/puppet/indirector/request.rb
+++ b/lib/puppet/indirector/request.rb
@@ -11,6 +11,8 @@ class Puppet::Indirector::Request
 
     attr_reader :indirection_name
 
+    OPTION_ATTRIBUTES = [:ip, :node, :authenticated, :ignore_terminus, :ignore_cache, :instance, :environment]
+
     # Is this an authenticated request?
     def authenticated?
         # Double negative, so we just get true or false
@@ -56,15 +58,9 @@ class Puppet::Indirector::Request
         self.indirection_name = indirection_name
         self.method = method
 
-        @options = options.inject({}) do |result, ary|
-            param, value = ary
-            if respond_to?(param.to_s + "=")
-                send(param.to_s + "=", value)
-            else
-                result[param] = value
-            end
-            result
-        end
+        set_attributes(options)
+
+        @options = options.inject({}) { |hash, ary| hash[ary[0].to_sym] = ary[1]; hash }
 
         if key.is_a?(String) or key.is_a?(Symbol)
             # If the request key is a URI, then we need to treat it specially,
@@ -130,6 +126,17 @@ class Puppet::Indirector::Request
         end.join("&")
     end
 
+    def to_hash
+        result = options.dup
+
+        OPTION_ATTRIBUTES.each do |attribute|
+            if value = send(attribute)
+                result[attribute] = value
+            end
+        end
+        result
+    end
+
     def to_s
         return uri if uri
         return "/%s/%s" % [indirection_name, key]
@@ -137,6 +144,15 @@ class Puppet::Indirector::Request
 
     private
 
+    def set_attributes(options)
+        OPTION_ATTRIBUTES.each do |attribute|
+            if options.include?(attribute)
+                send(attribute.to_s + "=", options[attribute])
+                options.delete(attribute)
+            end
+        end
+    end
+
     # Parse the key as a URI, setting attributes appropriately.
     def set_uri_key(key)
         @uri = key
diff --git a/spec/unit/indirector/request.rb b/spec/unit/indirector/request.rb
index e2a871a5f..47ffc319c 100755
--- a/spec/unit/indirector/request.rb
+++ b/spec/unit/indirector/request.rb
@@ -80,6 +80,14 @@ describe Puppet::Indirector::Request do
             Puppet::Indirector::Request.new(:ind, :method, :key, :foo => "bar").options.should be_instance_of(Hash)
         end
 
+        it "should treat options other than :ip, :node, and :authenticated as options rather than attributes" do
+            Puppet::Indirector::Request.new(:ind, :method, :key, :server => "bar").options[:server].should == "bar"
+        end
+
+        it "should normalize options to use symbols as keys" do
+            Puppet::Indirector::Request.new(:ind, :method, :key, "foo" => "bar").options[:foo].should == "bar"
+        end
+
         describe "and the request key is a URI" do
             describe "and the URI is a 'file' URI" do
                 before do
@@ -216,6 +224,14 @@ describe Puppet::Indirector::Request do
         Puppet::Indirector::Request.new(:myind, :find, "my key" ).environment.should equal(Puppet::Node::Environment.new)
     end
 
+    it "should support converting its options to a hash" do
+        Puppet::Indirector::Request.new(:myind, :find, "my key" ).should respond_to(:to_hash)
+    end
+
+    it "should include all of its attributes when its options are converted to a hash" do
+        Puppet::Indirector::Request.new(:myind, :find, "my key", :node => 'foo').to_hash[:node].should == 'foo'
+    end
+
     describe "when building a query string from its options" do
         before do
             @request = Puppet::Indirector::Request.new(:myind, :find, "my key")