Merge pull request #803 from nemupm/error_with_nil_or_empty_topic

Fix confusing error message with nil or empty string topic

Author: dasch

lib/kafka/cluster.rb

@@ -45,6 +45,10 @@ def add_target_topics(topics)
         new_topics = topics - @target_topics
 
         unless new_topics.empty?
+          if new_topics.any? { |topic| topic.nil? or topic.empty? }
+            raise ArgumentError, "Topic must not be nil or empty"
+          end
+
           @logger.info "New topics added to target list: #{new_topics.to_a.join(', ')}"
 
           @target_topics.merge(new_topics)

spec/cluster_spec.rb

@@ -1,18 +1,18 @@
 # frozen_string_literal: true
 
 describe Kafka::Cluster do
-  describe "#get_leader" do
-    let(:broker) { double(:broker) }
-    let(:broker_pool) { double(:broker_pool) }
+  let(:broker) { double(:broker) }
+  let(:broker_pool) { double(:broker_pool) }
 
-    let(:cluster) {
-      Kafka::Cluster.new(
-        seed_brokers: [URI("kafka://test1:9092")],
-        broker_pool: broker_pool,
-        logger: LOGGER,
-      )
-    }
+  let(:cluster) {
+    Kafka::Cluster.new(
+      seed_brokers: [URI("kafka://test1:9092")],
+      broker_pool: broker_pool,
+      logger: LOGGER,
+    )
+  }
 
+  describe "#get_leader" do
     before do
       allow(broker_pool).to receive(:connect) { broker }
       allow(broker).to receive(:disconnect)
@@ -89,4 +89,18 @@
       }.to raise_exception(Kafka::ConnectionError)
     end
   end
+
+  describe "#add_target_topics" do
+    it "raises ArgumentError if the topic is nil" do
+      expect {
+        cluster.add_target_topics([nil])
+      }.to raise_exception(ArgumentError)
+    end
+
+    it "raises ArgumentError if the topic is empty" do
+      expect {
+        cluster.add_target_topics([""])
+      }.to raise_exception(ArgumentError)
+    end
+  end
 end