Moving functionality out of the FormatHandler into the Format class.

The Format class now takes care of deciding whether it's supported,
and it also does method dispatch to classes and instances as necessary.

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

4 files changed, 252 insertions, 113 deletions

diff --git a/lib/puppet/network/format.rb b/lib/puppet/network/format.rb
index 7c678568d..832697836 100644
--- a/lib/puppet/network/format.rb
+++ b/lib/puppet/network/format.rb
@@ -23,5 +23,42 @@ class Puppet::Network::Format
         end
 
         instance_eval(&block) if block_given?
+
+        @intern_method = "from_%s" % name
+        @render_method = "to_%s" % name
+        @intern_multiple_method = "from_multiple_%s" % name
+        @render_multiple_method = "to_multiple_%s" % name
+    end
+
+    def intern(klass, text)
+        return klass.send(intern_method, text) if klass.respond_to?(intern_method)
+        raise NotImplementedError
+    end
+
+    def intern_multiple(klass, text)
+        return klass.send(intern_multiple_method, text) if klass.respond_to?(intern_multiple_method)
+        raise NotImplementedError
+    end
+
+    def render(instance)
+        return instance.send(render_method) if instance.respond_to?(render_method)
+        raise NotImplementedError
     end
+
+    def render_multiple(instances)
+        # This method implicitly assumes that all instances are of the same type.
+        return instances[0].class.send(render_multiple_method, instances) if instances[0].class.respond_to?(render_multiple_method)
+        raise NotImplementedError
+    end
+
+    def supported?(klass)
+        klass.respond_to?(intern_method) and
+            klass.respond_to?(intern_multiple_method) and
+            klass.respond_to?(render_multiple_method) and
+            klass.instance_methods.include?(render_method)
+    end
+
+    private
+
+    attr_reader :intern_method, :render_method, :intern_multiple_method, :render_multiple_method
 end
diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb
index 7e540708d..e42cc4c46 100644
--- a/lib/puppet/network/format_handler.rb
+++ b/lib/puppet/network/format_handler.rb
@@ -1,3 +1,4 @@
+require 'yaml'
 require 'puppet/network'
 require 'puppet/network/format'
 
@@ -23,31 +24,34 @@ module Puppet::Network::FormatHandler
         @formats[name]
     end
 
+    # Provide a list of all formats.
+    def self.formats
+        @formats.keys
+    end
+
     # Return a format capable of handling the provided mime type.
     def self.mime(mimetype)
         @formats.values.find { |format| format.mime == mimetype }
     end
 
     module ClassMethods
+        def format_handler
+            Puppet::Network::FormatHandler
+        end
+
         def convert_from(format, data)
             raise ArgumentError, "Format %s not supported" % format unless support_format?(format)
-            send("from_%s" % format, data)
+            format_handler.format(format).intern(self, data)
         end
 
         def convert_from_multiple(format, data)
-            if respond_to?("from_multiple_%s" % format)
-                send("from_multiple_%s" % format, data)
-            else
-                convert_from(format, data)
-            end
+            raise ArgumentError, "Format %s not supported" % format unless support_format?(format)
+            format_handler.format(format).intern_multiple(self, data)
         end
 
         def render_multiple(format, instances)
-            if respond_to?("to_multiple_%s" % format)
-                send("to_multiple_%s" % format, instances)
-            else
-                instances.send("to_%s" % format)
-            end
+            raise ArgumentError, "Format %s not supported" % format unless support_format?(format)
+            format_handler.format(format).render_multiple(instances)
         end
 
         def default_format
@@ -55,15 +59,19 @@ module Puppet::Network::FormatHandler
         end
 
         def support_format?(name)
-            respond_to?("from_%s" % name) and instance_methods.include?("to_%s" % name)
+            Puppet::Network::FormatHandler.format(name).supported?(self)
         end
 
         def supported_formats
-            instance = instance_methods.collect { |m| m =~ /^to_(.+)$/ and $1 }.compact
-            klass = methods.collect { |m| m =~ /^from_(.+)$/ and $1 }.compact
+            format_handler.formats.collect { |f| format_handler.format(f) }.find_all { |f| f.supported?(self) }.collect { |f| f.name }
+        end
 
-            # Return the intersection of the two lists.
-            return instance & klass
+        def from_marshal(text)
+            Marshal.load(text)
+        end
+
+        def from_yaml(text)
+            YAML.load(text)
         end
     end
 
@@ -75,12 +83,16 @@ module Puppet::Network::FormatHandler
                 format = self.class.default_format
             end
 
-            send("to_%s" % format)
+            Puppet::Network::FormatHandler.format(format).render(self)
         end
 
         def support_format?(name)
             self.class.support_format?(name)
         end
+
+        def to_marshal(instance)
+            Marshal.dump(instance)
+        end
     end
 end
 
diff --git a/spec/unit/network/format.rb b/spec/unit/network/format.rb
index 78e677e9b..34a841603 100755
--- a/spec/unit/network/format.rb
+++ b/spec/unit/network/format.rb
@@ -4,35 +4,129 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/network/format'
 
-describe Puppet::Network::Format do
-    it "should require a name" do
-        lambda { Puppet::Network::Format.new }.should raise_error(ArgumentError)
+# A class with all of the necessary
+# hooks.
+class FormatRenderer
+    def self.to_multiple_my_format(list)
     end
 
-    it "should be able to provide its name" do
-        Puppet::Network::Format.new(:my_format).name.should == :my_format
+    def self.from_multiple_my_format(text)
     end
 
-    it "should be able to set its mime type at initialization" do
-        format = Puppet::Network::Format.new(:my_format, :mime => "foo/bar")
-        format.mime.should == "foo/bar"
+    def self.from_my_format(text)
     end
 
-    it "should default to text plus the name of the format as the mime type" do
-        Puppet::Network::Format.new(:my_format).mime.should == "text/my_format"
+    def to_my_format
     end
+end
+
+describe Puppet::Network::Format do
+    describe "when initializing" do
+        it "should require a name" do
+            lambda { Puppet::Network::Format.new }.should raise_error(ArgumentError)
+        end
+
+        it "should be able to provide its name" do
+            Puppet::Network::Format.new(:my_format).name.should == :my_format
+        end
+
+        it "should be able to set its mime type at initialization" do
+            format = Puppet::Network::Format.new(:my_format, :mime => "foo/bar")
+            format.mime.should == "foo/bar"
+        end
 
-    it "should fail if unsupported options are provided" do
-        lambda { Puppet::Network::Format.new(:my_format, :foo => "bar") }.should raise_error(ArgumentError)
+        it "should default to text plus the name of the format as the mime type" do
+            Puppet::Network::Format.new(:my_format).mime.should == "text/my_format"
+        end
+
+        it "should fail if unsupported options are provided" do
+            lambda { Puppet::Network::Format.new(:my_format, :foo => "bar") }.should raise_error(ArgumentError)
+        end
     end
 
-    it "should support being confined" do
-        Puppet::Network::Format.new(:my_format).should respond_to(:confine)
+    describe "instances" do
+        before do
+            @format = Puppet::Network::Format.new(:my_format)
+        end
+
+        it "should support being confined" do
+            @format.should respond_to(:confine)
+        end
+
+        it "should not be considered suitable if confinement conditions are not met" do
+            @format.confine :true => false
+            @format.should_not be_suitable
+        end
+
+        it "should be able to determine if a class is supported" do
+            @format.should respond_to(:supported?)
+        end
+
+        it "should consider a class to be supported if it has the individual and multiple methods for rendering and interning" do
+            @format.should be_supported(FormatRenderer)
+        end
+
+        it "should not consider a class to be supported unless it has the individual and multiple methods for rendering and interning" do
+            Puppet::Network::Format.new(:yaml).should_not be_supported(String)
+        end
     end
 
-    it "should not be considered suitable if confinement conditions are not met" do
-        format = Puppet::Network::Format.new(:my_format)
-        format.confine :true => false
-        format.should_not be_suitable
+    describe "when converting between instances and formatted text" do
+        before do
+            @format = Puppet::Network::Format.new(:my_format)
+            @instance = FormatRenderer.new
+        end
+
+        it "should have a method for rendering a single instance" do
+            @format.should respond_to(:render)
+        end
+
+        it "should have a method for rendering multiple instances" do
+            @format.should respond_to(:render_multiple)
+        end
+
+        it "should have a method for interning text" do
+            @format.should respond_to(:intern)
+        end
+
+        it "should have a method for interning text into multiple instances" do
+            @format.should respond_to(:intern_multiple)
+        end
+
+        it "should return the results of calling the instance-specific render method if the method is present" do
+            @instance.expects(:to_my_format).returns "foo"
+            @format.render(@instance).should == "foo"
+        end
+
+        it "should return the results of calling the class-specific render_multiple method if the method is present" do
+            @instance.class.expects(:to_multiple_my_format).returns ["foo"]
+            @format.render_multiple([@instance]).should == ["foo"]
+        end
+
+        it "should return the results of calling the class-specific intern method if the method is present" do
+            FormatRenderer.expects(:from_my_format).with("foo").returns @instance
+            @format.intern(FormatRenderer, "foo").should equal(@instance)
+        end
+
+        it "should return the results of calling the class-specific intern_multiple method if the method is present" do
+            FormatRenderer.expects(:from_multiple_my_format).with("foo").returns [@instance]
+            @format.intern_multiple(FormatRenderer, "foo").should == [@instance]
+        end
+
+        it "should fail if asked to render and the instance does not respond to 'to_<format>'" do
+            lambda { @format.render("foo") }.should raise_error(NotImplementedError)
+        end
+
+        it "should fail if asked to intern and the class does not respond to 'from_<format>'" do
+            lambda { @format.intern(String, "foo") }.should raise_error(NotImplementedError)
+        end
+
+        it "should fail if asked to intern multiple and the class does not respond to 'from_multiple_<format>'" do
+            lambda { @format.intern_multiple(String, "foo") }.should raise_error(NotImplementedError)
+        end
+
+        it "should fail if asked to render multiple and the instance does not respond to 'to_multiple_<format>'" do
+            lambda { @format.render_multiple(["foo", "bar"]) }.should raise_error(NotImplementedError)
+        end
     end
 end
diff --git a/spec/unit/network/format_handler.rb b/spec/unit/network/format_handler.rb
index d142c3177..5071196c3 100755
--- a/spec/unit/network/format_handler.rb
+++ b/spec/unit/network/format_handler.rb
@@ -6,26 +6,6 @@ require 'puppet/network/format_handler'
 
 class FormatTester
     extend Puppet::Network::FormatHandler
-
-    # Not a supported format; missing the 'to'
-    def self.from_nothing; end
-
-    # Not a supported format; missing the 'from'
-    def to_nowhere; end
-
-    # A largely functional format.
-    def self.from_good; end
-
-    def to_good; end
-
-    # A format that knows how to handle multiple instances specially.
-    def self.from_mults; end
-
-    def self.from_multiple_mults; end
-
-    def self.to_multiple_mults; end
-
-    def to_mults; end
 end
 
 describe Puppet::Network::FormatHandler do
@@ -36,68 +16,69 @@ describe Puppet::Network::FormatHandler do
         end
     end
 
-    it "should be able to test whether a format is supported" do
-        FormatTester.should respond_to(:support_format?)
-    end
-
-    it "should consider the format supported if it can convert from an instance to the format and from the format to an instance" do
-        FormatTester.should be_support_format(:good)
+    it "should be able to list supported formats" do
+        FormatTester.should respond_to(:supported_formats)
     end
 
-    it "should not consider the format supported unless it can convert the instance to the specified format" do
-        FormatTester.should_not be_support_format(:nothing)
+    it "should include all supported formats" do
+        one = stub 'supported', :supported? => true, :name => "one"
+        two = stub 'supported', :supported? => false, :name => "two"
+        three = stub 'supported', :supported? => true, :name => "three"
+        four = stub 'supported', :supported? => false, :name => "four"
+        Puppet::Network::FormatHandler.expects(:formats).returns %w{one two three four}
+        Puppet::Network::FormatHandler.expects(:format).with("one").returns one
+        Puppet::Network::FormatHandler.expects(:format).with("two").returns two
+        Puppet::Network::FormatHandler.expects(:format).with("three").returns three
+        Puppet::Network::FormatHandler.expects(:format).with("four").returns four
+        FormatTester.supported_formats.sort.should == %w{one three}.sort
     end
 
-    it "should not consider the format supported unless it can convert from the format to an instance" do
-        FormatTester.should_not be_support_format(:nowhere)
+    it "should return the first format as the default format" do
+        FormatTester.expects(:supported_formats).returns %w{one two}
+        FormatTester.default_format.should == "one"
     end
 
-    it "should be able to convert from a given format" do
-        FormatTester.should respond_to(:convert_from)
-    end
+    describe "when using formats" do
+        before do
+            @format = mock 'format'
+            Puppet::Network::FormatHandler.stubs(:format).with(:my_format).returns @format
+            @format.stubs(:supported?).returns true
+        end
 
-    it "should fail if asked to convert from an unsupported format" do
-        FormatTester.expects(:support_format?).with(:nope).returns false
-        lambda { FormatTester.convert_from(:nope, "mydata") }.should raise_error(ArgumentError)
-    end
+        it "should be able to test whether a format is supported" do
+            FormatTester.should respond_to(:support_format?)
+        end
 
-    it "should call the format-specific converter when asked to convert from a given format" do
-        FormatTester.expects(:from_good).with("mydata")
-        FormatTester.convert_from(:good, "mydata")
-    end
+        it "should use the Format to determine whether a given format is supported" do
+            @format.expects(:supported?).with(FormatTester)
+            FormatTester.support_format?(:my_format)
+        end
 
-    it "should be able to use a specific hook for converting into multiple instances" do
-        FormatTester.expects(:from_multiple_mults).with("mydata")
-        FormatTester.convert_from_multiple(:mults, "mydata")
-    end
+        it "should be able to convert from a given format" do
+            FormatTester.should respond_to(:convert_from)
+        end
 
-    it "should default to the normal conversion method when no special method is available" do
-        FormatTester.expects(:from_good).with("mydata")
-        FormatTester.convert_from_multiple(:good, "mydata")
-    end
+        it "should fail if asked to convert from an unsupported format" do
+            @format.expects(:supported?).with(FormatTester).returns false
+            lambda { FormatTester.convert_from(:my_format, "mydata") }.should raise_error(ArgumentError)
+        end
 
-    it "should be able to use a specific hook for rendering multiple instances" do
-        FormatTester.expects(:to_multiple_mults).with("mydata")
-        FormatTester.render_multiple(:mults, "mydata")
-    end
+        it "should call the format-specific converter when asked to convert from a given format" do
+            @format.expects(:intern).with(FormatTester, "mydata")
+            FormatTester.convert_from(:my_format, "mydata")
+        end
 
-    it "should use the instance method if no multiple-render hook is available" do
-        instances = mock 'instances'
-        instances.expects(:to_good)
-        FormatTester.render_multiple(:good, instances)
-    end
+        it "should be able to use a specific hook for converting into multiple instances" do
+            @format.expects(:intern_multiple).with(FormatTester, "mydata")
 
-    it "should be able to list supported formats" do
-        FormatTester.should respond_to(:supported_formats)
-    end
+            FormatTester.convert_from_multiple(:my_format, "mydata")
+        end
 
-    it "should include all formats that include both the to_ and from_ methods in the list of supported formats" do
-        FormatTester.supported_formats.sort.should == %w{good mults}.sort
-    end
+        it "should be able to use a specific hook for rendering multiple instances" do
+            @format.expects(:render_multiple).with("mydata")
 
-    it "should return the first format as the default format" do
-        FormatTester.expects(:supported_formats).returns %w{one two}
-        FormatTester.default_format.should == "one"
+            FormatTester.render_multiple(:my_format, "mydata")
+        end
     end
 
     describe "when managing formats" do
@@ -129,6 +110,20 @@ describe Puppet::Network::FormatHandler do
             format = Puppet::Network::FormatHandler.create(:by_name, :mime => "foo/bar")
             Puppet::Network::FormatHandler.mime("foo/bar").should equal(format)
         end
+
+        it "should be able to return all formats" do
+            one = stub 'one', :name => :one
+            two = stub 'two', :name => :two
+            Puppet::Network::Format.expects(:new).with(:one).returns(one)
+            Puppet::Network::Format.expects(:new).with(:two).returns(two)
+
+            Puppet::Network::FormatHandler.create(:one)
+            Puppet::Network::FormatHandler.create(:two)
+
+            list = Puppet::Network::FormatHandler.formats
+            list.should be_include(:one)
+            list.should be_include(:two)
+        end
     end
 
     describe "when an instance" do
@@ -136,14 +131,6 @@ describe Puppet::Network::FormatHandler do
             FormatTester.new.should respond_to(:support_format?)
         end
 
-        it "should consider the format supported if it can convert from an instance to the format and from the format to an instance" do
-            FormatTester.new.should be_support_format(:good)
-        end
-
-        it "should not consider the format supported unless it can convert from an instance to the format and from the format to an instance" do
-            FormatTester.new.should_not be_support_format(:nope)
-        end
-
         it "should be able to convert to a given format" do
             FormatTester.new.should respond_to(:render)
         end
@@ -155,15 +142,24 @@ describe Puppet::Network::FormatHandler do
         end
 
         it "should call the format-specific converter when asked to convert to a given format" do
+            format = stub 'rendering format', :supported? => true
+
+            Puppet::Network::FormatHandler.stubs(:format).with(:foo).returns format
+
             tester = FormatTester.new
-            tester.expects(:to_good)
-            tester.render(:good)
+            format.expects(:render).with(tester).returns "foo"
+
+            tester.render(:foo).should == "foo"
         end
 
         it "should render to the default format if no format is provided when rendering" do
+            format = stub 'rendering format', :supported? => true
+            Puppet::Network::FormatHandler.stubs(:format).with("foo").returns format
+
             FormatTester.expects(:default_format).returns "foo"
             tester = FormatTester.new
-            tester.expects(:to_foo)
+
+            format.expects(:render).with(tester)
             tester.render
         end
     end