Increase test coverage

README.md

@@ -595,6 +595,34 @@ rabbitmq_binding { 'myexchange@myqueue@myvhost':
 }
 ```
 
+```puppet
+rabbitmq_binding { 'binding 1':
+  source           => 'myexchange',
+  destination      => 'myqueue',
+  vhost            => 'myvhost',
+  user             => 'dan',
+  password         => 'bar',
+  destination_type => 'queue',
+  routing_key      => 'key1',
+  arguments        => {},
+  ensure           => present,
+}
+
+rabbitmq_binding { 'binding 2':
+  source           => 'myexchange',
+  destination      => 'myqueue',
+  vhost            => 'myvhost',
+  user             => 'dan',
+  password         => 'bar',
+  destination_type => 'queue',
+  routing_key      => 'key2',
+  arguments        => {},
+  ensure           => present,
+}
+
+```
+
+
 ### rabbitmq\_user\_permissions
 
 ```puppet

lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb

@@ -1,5 +1,7 @@
 require 'json'
 require 'puppet'
+require 'digest'
+
 Puppet::Type.type(:rabbitmq_binding).provide(:rabbitmqadmin) do
 
   if Puppet::PUPPETVERSION.to_f < 3
@@ -15,6 +17,9 @@
   end
   defaultfor :feature => :posix
 
+  # Without this, the composite namevar stuff doesn't work properly.
+  mk_resource_methods
+
   def should_vhost
     if @should_vhost
       @should_vhost
@@ -49,16 +54,17 @@ def self.instances
         else
           arguments = '{}'
         end
+        hashed_name = Digest::SHA256.hexdigest "%s@%s@%s@%s" % [source_name, destination_name, vhost, routing_key]
         unless(source_name.empty?)
           binding = {
             :source           => source_name,
-            :dest             => destination_name,
+            :destination      => destination_name,
             :vhost            => vhost,
             :destination_type => destination_type,
             :routing_key      => routing_key,
             :arguments        => JSON.parse(arguments),
             :ensure           => :present,
-            :name             => "%s@%s@%s@%s" % [source_name, destination_name, vhost, routing_key],
+            :name             => hashed_name,
           }
           resources << new(binding) if binding[:name]
         end
@@ -67,10 +73,12 @@ def self.instances
     resources
   end
 
+  # see
+  # https://github.com/puppetlabs/puppetlabs-netapp/blob/d0a655665463c69c932f835ba8756be32417a4e9/lib/puppet/provider/netapp_qtree/sevenmode.rb#L66-L73
   def self.prefetch(resources)
     bindings = instances
-    resources.keys.each do |name|
-      if provider = bindings.find{ |route| route.source == source && route.dest == dest && route.vhost == vhost && route.routing_key == routing_key }
+    resources.each do |name, res|
+      if provider = bindings.find{ |binding| binding.source == res[:source] && binding.destination == res[:destination] && binding.vhost == res[:vhost] && binding.routing_key == res[:routing_key] }
         resources[name].provider = provider
       end
     end
@@ -94,7 +102,7 @@ def create
       '-c',
       '/etc/rabbitmq/rabbitmqadmin.conf',
       "source=#{resource[:source]}",
-      "destination=#{resource[:dest]}",
+      "destination=#{resource[:destination]}",
       "arguments=#{arguments.to_json}",
       "routing_key=#{resource[:routing_key]}",
       "destination_type=#{resource[:destination_type]}"
@@ -104,7 +112,7 @@ def create
 
   def destroy
     vhost_opt = should_vhost ? "--vhost=#{should_vhost}" : ''
-    rabbitmqadmin('delete', 'binding', vhost_opt, "--user=#{resource[:user]}", "--password=#{resource[:password]}", '-c', '/etc/rabbitmq/rabbitmqadmin.conf', "source=#{resource[:source]}", "destination_type=#{resource[:destination_type]}", "destination=#{resource[:dest]}", "properties_key=#{resource[:routing_key]}")
+    rabbitmqadmin('delete', 'binding', vhost_opt, "--user=#{resource[:user]}", "--password=#{resource[:password]}", '-c', '/etc/rabbitmq/rabbitmqadmin.conf', "source=#{resource[:source]}", "destination_type=#{resource[:destination_type]}", "destination=#{resource[:destination]}", "properties_key=#{resource[:routing_key]}")
     @property_hash[:ensure] = :absent
   end
 

lib/puppet/type/rabbitmq_binding.rb

@@ -12,7 +12,7 @@
   end
 
   # Match patterns without '@' as arbitrary names; match patterns with
-  # src@dst@vhost to their named params for backwards compatibility.
+  # src@destination@vhost to their named params for backwards compatibility.
   def self.title_patterns
     [
       [
@@ -22,59 +22,59 @@ def self.title_patterns
         ]
       ],
       [
-        /^(\S+)@(\S+)@(\S+)$/m,
+        /^((\S+)@(\S+)@(\S+))$/m,
         [
+          [ :name ],
           [ :source ],
-          [ :dest ],
+          [ :destination],
           [ :vhost ]
         ]
       ]
     ]
   end
 
   newparam(:name) do
-    desc 'resource name, either source@dest@vhost or arbitrary name with params'
-    
+    desc 'resource name, either source@destination@vhost or arbitrary name with params'
+
     isnamevar
   end
 
-  newparam(:source) do
+  newproperty(:source) do
     desc 'source of binding'
 
     newvalues(/^\S+$/)
     isnamevar
   end
 
-  newparam(:dest, :namevar => true) do
+  newproperty(:destination) do
     desc 'destination of binding'
 
     newvalues(/^\S+$/)
     isnamevar
   end
 
-  newparam(:vhost, :namevar => true) do
+  newproperty(:vhost) do
     desc 'vhost'
 
     newvalues(/^\S+$/)
     defaultto('/')
     isnamevar
   end
 
-  newparam(:routing_key, :namevar => true) do
+  newproperty(:routing_key) do
     desc 'binding routing_key'
 
     newvalues(/^\S*$/)
-    defaultto('#')
     isnamevar
   end
 
-  newparam(:destination_type) do
+  newproperty(:destination_type) do
     desc 'binding destination_type'
     newvalues(/queue|exchange/)
     defaultto('queue')
   end
 
-  newparam(:arguments) do
+  newproperty(:arguments) do
     desc 'binding arguments'
     defaultto {}
     validate do |value|
@@ -113,7 +113,7 @@ def self.title_patterns
   autorequire(:rabbitmq_user_permissions) do
     [
       "#{self[:user]}@#{self[:source]}",
-      "#{self[:user]}@#{self[:dest]}"
+      "#{self[:user]}@#{self[:destination]}"
     ]
   end
 
@@ -122,11 +122,11 @@ def setup_autorequire(type)
     if type == 'exchange'
       rval = ["#{self[:source]}@#{self[:vhost]}"]
       if destination_type == type
-        rval.push("#{self[:dest]}@#{self[:vhost]}")
+        rval.push("#{self[:destination]}@#{self[:vhost]}")
       end
     else
       if destination_type == type
-        rval = ["#{self[:dest]}@#{self[:vhost]}"]
+        rval = ["#{self[:destination]}@#{self[:vhost]}"]
       else
         rval = []
       end
@@ -140,4 +140,12 @@ def validate_argument(argument)
     end
   end
 
+	# Validate that we have both source and destination now that these are not
+  # necessarily only coming from the resource title.
+  validate do
+    unless self[:source] and self[:destination]
+      raise ArgumentError, "Source and destination must both be defined."
+    end
+  end
+
 end

spec/acceptance/queue_spec.rb

@@ -78,7 +78,7 @@ class { '::rabbitmq':
   end
 
 
-  context "create multiple bindings when same source / dest / vhost but different routing keys" do
+  context "create multiple bindings when same source / destination / vhost but different routing keys" do
     it 'should run successfully' do
       pp = <<-EOS
       if $::osfamily == 'RedHat' {
@@ -125,7 +125,9 @@ class { '::rabbitmq':
 
       rabbitmq_binding { 'binding 1':
         source           => 'exchange1',
+        destination      => 'queue1',
         user             => 'dan',
+        vhost            => 'host1',
         password         => 'bar',
         destination_type => 'queue',
         routing_key      => 'test1',
@@ -134,7 +136,9 @@ class { '::rabbitmq':
 
       rabbitmq_binding { 'binding 2':
         source           => 'exchange1',
+        destination      => 'queue1',
         user             => 'dan',
+        vhost            => 'host1',
         password         => 'bar',
         destination_type => 'queue',
         routing_key      => 'test2',