Refactoring the stomp client and tests a bit

The main goal of this refactor is to tell the client to
be resilient to failures (configured at initialization time),
and to send all messages as persistent messages (configured
for each message).

In the process, the client now parses the queue source URI
and handles each argument separately.  The tests are more
thorough, also.

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

2 files changed, 99 insertions, 34 deletions

diff --git a/lib/puppet/util/queue/stomp.rb b/lib/puppet/util/queue/stomp.rb
index 6f845c314..3a6a99ca2 100644
--- a/lib/puppet/util/queue/stomp.rb
+++ b/lib/puppet/util/queue/stomp.rb
@@ -1,5 +1,6 @@
 require 'puppet/util/queue'
 require 'stomp'
+require 'uri'
 
 # Implements the Ruby Stomp client as a queue type within the Puppet::Indirector::Queue::Client
 # registry, for use with the <tt>:queue</tt> indirection terminus type.
@@ -11,11 +12,20 @@ class Puppet::Util::Queue::Stomp
     attr_accessor :stomp_client
 
     def initialize
-        self.stomp_client = Stomp::Client.new( Puppet[:queue_source] )
+        begin
+            uri = URI.parse(Puppet[:queue_source])
+        rescue => detail
+            raise ArgumentError, "Could not create Stomp client instance - queue source %s is invalid: %s" % [Puppet[:queue_source], detail]
+        end
+        unless uri.scheme == "stomp"
+            raise ArgumentError, "Could not create Stomp client instance - queue source %s is not a Stomp URL: %s" % [Puppet[:queue_source], detail]
+        end
+
+        self.stomp_client = Stomp::Client.new(uri.user, uri.password, uri.host, uri.port, true)
     end
 
     def send_message(target, msg)
-        stomp_client.send(stompify_target(target), msg)
+        stomp_client.send(stompify_target(target), msg, :persistent => true)
     end
 
     def subscribe(target)
diff --git a/spec/unit/util/queue/stomp.rb b/spec/unit/util/queue/stomp.rb
index b0b86e4af..f2960b3f8 100755
--- a/spec/unit/util/queue/stomp.rb
+++ b/spec/unit/util/queue/stomp.rb
@@ -15,52 +15,107 @@ end
 describe 'Puppet::Util::Queue::Stomp' do
     confine "Missing Stomp" => Puppet.features.stomp?
 
-    before :all do
-        class Stomp::Client
-            include Mocha::Standalone
-            attr_accessor :queue_source
+    before do
+        # So we make sure we never create a real client instance.
+        # Otherwise we'll try to connect, and that's bad.
+        Stomp::Client.stubs(:new).returns stub("client")
+    end
 
-            def send(q, m)
-                'To %s: %s' % [q, m]
-            end
+    it 'should be registered with Puppet::Util::Queue as :stomp type' do
+        Puppet::Util::Queue.queue_type_to_class(:stomp).should == Puppet::Util::Queue::Stomp
+    end
 
-            def subscribe(q)
-                yield(stub(:body => 'subscribe: %s' % q))
-            end
+    describe "when initializing" do
+        it "should create a Stomp client instance" do
+            Stomp::Client.expects(:new).returns stub("stomp_client")
+            Puppet::Util::Queue::Stomp.new
+        end
+
+        it "should provide helpful failures when the queue source is not a valid source" do
+            # Stub rather than expect, so we can include the source in the error
+            Puppet.settings.stubs(:value).with(:queue_source).returns "-----"
+
+            lambda { Puppet::Util::Queue::Stomp.new }.should raise_error(ArgumentError)
+        end
 
-            def initialize(s)
-                self.queue_source = s
+        it "should fail unless the queue source is a stomp URL" do
+            # Stub rather than expect, so we can include the source in the error
+            Puppet.settings.stubs(:value).with(:queue_source).returns "http://foo/bar"
+
+            lambda { Puppet::Util::Queue::Stomp.new }.should raise_error(ArgumentError)
+        end
+
+        list = %w{user password host port}
+        {"user" => "myuser", "password" => "mypass", "host" => "foohost", "port" => 42}.each do |name, value|
+            it "should use the #{name} from the queue source as the queueing #{name}" do
+                Puppet.settings.expects(:value).with(:queue_source).returns "stomp://myuser:mypass@foohost:42/"
+
+                Stomp::Client.expects(:new).with { |*args| args[list.index(name)] == value }
+                Puppet::Util::Queue::Stomp.new
             end
         end
-    end
 
-    before :each do
-        Puppet.settings.stubs(:value).returns 'faux_queue_source'
-    end
+        it "should create a reliable client instance" do
+            Puppet.settings.expects(:value).with(:queue_source).returns "stomp://myuser@foohost:42/"
 
-    it 'should make send function like core Ruby instead of stomp client send method' do
-        o = Puppet::Util::Queue::Stomp.new
-        o.expects(:pants).with('foo').once
-        o.send(:pants, 'foo')
+            Stomp::Client.expects(:new).with { |*args| args[4] == true }
+            Puppet::Util::Queue::Stomp.new
+        end
     end
 
-    it 'should be registered with Puppet::Util::Queue as :stomp type' do
-        Puppet::Util::Queue.queue_type_to_class(:stomp).should == Puppet::Util::Queue::Stomp
+    describe "when sending a message" do
+        before do
+            @client = stub 'client'
+            Stomp::Client.stubs(:new).returns @client
+            @queue = Puppet::Util::Queue::Stomp.new
+        end
+
+        it "should send it to the queue client instance" do
+            @client.expects(:send).with { |queue, msg, options| msg == "Smite!" }
+            @queue.send_message('fooqueue', 'Smite!')
+        end
+
+        it "should send it to the transformed queue name" do
+            @client.expects(:send).with { |queue, msg, options| queue == "/queue/fooqueue" }
+            @queue.send_message('fooqueue', 'Smite!')
+        end
+
+        it "should send it as a persistent message" do
+            @client.expects(:send).with { |queue, msg, options| options[:persistent] == true }
+            @queue.send_message('fooqueue', 'Smite!')
+        end
     end
 
-    it 'should initialize using Puppet[:queue_source] for configuration' do
-        o = Puppet::Util::Queue::Stomp.new
-        o.stomp_client.queue_source.should == 'faux_queue_source'
+    describe "when subscribing to a queue" do
+        before do
+            @client = stub 'client'
+            Stomp::Client.stubs(:new).returns @client
+            @queue = Puppet::Util::Queue::Stomp.new
+        end
+
+        it "should subscribe via the queue client instance" do
+            @client.expects(:subscribe)
+            @queue.subscribe('fooqueue')
+        end
+
+        it "should subscribe to the transformed queue name" do
+            @client.expects(:subscribe).with("/queue/fooqueue")
+            @queue.subscribe('fooqueue')
+        end
+
+        it "should yield the body of any received message" do
+            message = mock 'message'
+            message.expects(:body).returns "mybody"
+
+            @client.expects(:subscribe).yields(message)
+
+            body = nil
+            @queue.subscribe('fooqueue') { |b| body = b }
+            body.should == "mybody"
+        end
     end
 
     it 'should transform the simple queue name to "/queue/<queue_name>"' do
         Puppet::Util::Queue::Stomp.new.stompify_target('blah').should == '/queue/blah'
     end
-
-    it 'should transform the queue name properly and pass along to superclass for send and subscribe' do
-        o = Puppet::Util::Queue::Stomp.new
-        o.send_message('fooqueue', 'Smite!').should == 'To /queue/fooqueue: Smite!'
-        o.subscribe('moodew') {|obj| obj}.should == 'subscribe: /queue/moodew'
-    end
 end
-