(#7132) Reject 'summary' text with newlines embedded.

Our summary documentation is used to provide single-line context to faces,
actions, and other items.  To support this we hard-fail if someone tries to
use the summary to embed the long documentation, and point them to the right
place to add the extended text.

Reviewed-By: Max Martin <max@puppetlabs.com>

4 files changed, 75 insertions, 20 deletions

diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 5e9355061..4a36b509a 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -70,9 +70,16 @@ class Puppet::Interface
 
   attr_accessor :summary
   def summary(value = nil)
-    @summary = value unless value.nil?
+    value.nil? or summary = value
     @summary
   end
+  def summary=(value)
+    value = value.to_s
+    value =~ /\n/ and
+      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+
+    @summary = value
+  end
 
   attr_reader :name, :version
 
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index b94298963..d2d4facaa 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -24,8 +24,21 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :default, :summary
+  attr_accessor :default
 
+  ########################################################################
+  # Documentation stuff, whee!
+  attr_accessor :summary
+  def summary=(value)
+    value = value.to_s
+    value =~ /\n/ and
+      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+
+    @summary = value
+  end
+
+
+  ########################################################################
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
   # capabilities.   Heck, our initial plan was to have the faces delegate to
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 4be6a1c55..602e51314 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -348,4 +348,32 @@ describe Puppet::Interface::Action do
       end
     end
   end
+
+  context "documentation" do
+    subject do
+      face = Puppet::Interface.new(:action_documentation, '0.0.1') do
+        action :documentation do end
+      end
+      face.get_action(:documentation)
+    end
+
+    describe "#summary" do
+      it "should accept a summary" do
+        text = "this is my summary"
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should accept a long, long, long summary" do
+        text = "I never know when to stop with the word banana" + ("na" * 1000)
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should reject a summary with a newline" do
+        expect { subject.summary = "with\nnewlines" }.
+          to raise_error ArgumentError, /summary should be a single line/
+      end
+    end
+  end
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index e52b45d8a..036372e6a 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -50,24 +50,6 @@ describe Puppet::Interface do
       subject.new(:foo, '1.0.0').should respond_to(:summary).with(0).arguments
       subject.new(:foo, '1.0.0').should respond_to(:summary=).with(1).arguments
     end
-
-    it "should set the summary text" do
-      text = "hello, freddy, my little pal"
-      subject.define(:face_test_summary, '1.0.0') do
-        summary text
-      end
-      subject[:face_test_summary, '1.0.0'].summary.should == text
-    end
-
-    it "should support mutating the summary" do
-      text = "hello, freddy, my little pal"
-      subject.define(:face_test_summary, '1.0.0') do
-        summary text
-      end
-      subject[:face_test_summary, '1.0.0'].summary.should == text
-      subject[:face_test_summary, '1.0.0'].summary = text + text
-      subject[:face_test_summary, '1.0.0'].summary.should == text + text
-    end
   end
 
   describe "#initialize" do
@@ -204,4 +186,29 @@ describe Puppet::Interface do
       end
     end
   end
+
+  context "documentation" do
+    subject do
+      Puppet::Interface.new(:face_documentation, '0.0.1')
+    end
+
+    describe "#summary" do
+      it "should accept a summary" do
+        text = "this is my summary"
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should accept a long, long, long summary" do
+        text = "I never know when to stop with the word banana" + ("na" * 1000)
+        expect { subject.summary = text }.not_to raise_error
+        subject.summary.should == text
+      end
+
+      it "should reject a summary with a newline" do
+        expect { subject.summary = "with \n embedded \n newlines" }.
+          to raise_error ArgumentError, /summary should be a single line/
+      end
+    end
+  end
 end