Refactor rabbitmq_plugins to share common code between exists? and instances

Author: nmaludy

lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb

@@ -4,20 +4,29 @@
 Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins, parent: Puppet::Provider::RabbitmqCli) do
   confine feature: :posix
 
-  def self.instances
-    plugin_list = run_with_retries do
-      # Pass in -E for explicitly enabled plugins as well as -e for implicitly enabled plugins
+  def self.plugin_list
+    list = run_with_retries do
+      # Starting in RabbitMQ 3.6.7 pass in -e to list both implicitly and explicitly enabled plugins.
+      # If you pass in -E instead, then only explicitly enabled plugins are listed.
       # Implicitly enabled plugins are those that were enabled as a dependency of another plugin/
       # If we do not pass in -e then the order if plugin installation matters within the puppet
       # code. Example, if Plugin A depends on Plugin B and we install Plugin B first it will
       # implicitly enable Plugin A. Then when we go to run Puppet a second time without the
       # -e parameter, we won't see Plugin A as being enabled so we'll try to install it again.
       # To preserve idempotency we should get all enabled plugins regardless of implicitly or
       # explicitly enabled.
-      rabbitmqplugins('list', '-E', '-e', '-m')
+      if Puppet::Util::Package.versioncmp(rabbitmq_version, '3.6.7') >= 0
+        # also pass in -q to suppress informational messages that break our parsing
+        rabbitmqplugins('list', '-e', '-m', '-q')
+      else
+        rabbitmqplugins('list', '-E', '-m')
+      end
     end
+    list.split(%r{\n})
+  end
 
-    plugin_list.split(%r{\n}).map do |line|
+  def self.instances
+    plugin_list.map do |line|
       raise Puppet::Error, "Cannot parse invalid plugins line: #{line}" unless line =~ %r{^(\S+)$}
       new(name: Regexp.last_match(1))
     end
@@ -40,6 +49,6 @@ def destroy
   end
 
   def exists?
-    run_with_retries { rabbitmqplugins('list', '-E', '-e', '-m') }.split(%r{\n}).include? resource[:name]
+    self.class.plugin_list.include? resource[:name]
   end
 end

spec/unit/puppet/provider/rabbitmq_plugin/rabbitmqctl_spec.rb

@@ -9,17 +9,84 @@
   end
   let(:provider) { provider_class.new(resource) }
 
-  it 'matches plugins' do
-    provider.expects(:rabbitmqplugins).with('list', '-E', '-e', '-m').returns("foo\n")
-    expect(provider.exists?).to eq(true)
-  end
-
   it 'calls rabbitmqplugins to enable when node not running' do
     provider.class.expects(:rabbitmq_running).returns false
     provider.expects(:rabbitmqplugins).with('enable', 'foo')
     provider.create
   end
 
+  describe '#instances' do
+    it 'exists' do
+      expect(provider_class).to respond_to :instances
+    end
+
+    # rubocop:disable RSpec/MultipleExpectation
+    it 'retrieves instances' do
+      provider.class.expects(:plugin_list).returns(%w[foo bar])
+      instances = provider_class.instances
+      expect(instances.size).to eq(2)
+      instances_cmp = instances.map { |prov| { name: prov.get(:name) } }
+      expect(instances_cmp).to eq(
+        [
+          { name: 'foo' },
+          { name: 'bar' }
+        ]
+      )
+    end
+    # rubocop:enable RSpec/MultipleExpectation
+
+    it 'raises error on invalid line' do
+      provider_class.expects(:plugin_list).returns(['  '])
+      expect { provider_class.instances }.to raise_error Puppet::Error, %r{Cannot parse invalid plugins line}
+    end
+  end
+
+  describe '#plugin_list' do
+    it 'exists' do
+      expect(provider_class).to respond_to :instances
+    end
+
+    context 'with RabbitMQ version >=3.6.7' do
+      it 'returns a list of plugins' do
+        provider.class.expects(:rabbitmq_version).returns '3.6.7'
+        provider.class.expects(:rabbitmqplugins).with('list', '-e', '-m', '-q').returns("foo\nbar\nbaz\n")
+        expect(provider.class.plugin_list).to eq(%w[foo bar baz])
+      end
+
+      it 'handles no training newline properly' do
+        provider.class.expects(:rabbitmq_version).returns '3.6.7'
+        provider.class.expects(:rabbitmqplugins).with('list', '-e', '-m', '-q').returns("foo\nbar")
+        expect(provider.class.plugin_list).to eq(%w[foo bar])
+      end
+    end
+
+    context 'with RabbitMQ version <3.6.7' do
+      it 'returns a list of plugins' do
+        provider.class.expects(:rabbitmq_version).returns '3.6.6'
+        provider.class.expects(:rabbitmqplugins).with('list', '-E', '-m').returns("foo\nbar\nbaz\n")
+        expect(provider.class.plugin_list).to eq(%w[foo bar baz])
+      end
+
+      it 'handles no training newline properly' do
+        provider.class.expects(:rabbitmq_version).returns '3.6.6'
+        provider.class.expects(:rabbitmqplugins).with('list', '-E', '-m').returns("foo\nbar")
+        expect(provider.class.plugin_list).to eq(%w[foo bar])
+      end
+    end
+  end
+
+  describe '#exists?' do
+    it 'matches existng plugins' do
+      provider_class.expects(:plugin_list).returns(%w[foo])
+      expect(provider.exists?).to eq(true)
+    end
+
+    it 'returns false for missing plugins' do
+      provider_class.expects(:plugin_list).returns(%w[bar])
+      expect(provider.exists?).to eq(false)
+    end
+  end
+
   context 'with RabbitMQ version >=3.4.0' do
     it 'calls rabbitmqplugins to enable' do
       provider.class.expects(:rabbitmq_version).returns '3.4.0'