Repository: beatlabs/patron
Pull request #191 (2020-03-24 16:06:39)

Add validation for brokers in kafka.NewBuilder
## Which problem is this PR solving?

- It updates validation for the brokers list. Relates to #174 


codecov[bot]: # [Codecov](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=h1) Report
> Merging [#191](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=desc) into [master](https://codecov.io/gh/beatlabs/patron/commit/7c27b37aba9dc190efbf0c6a4c56660eea826980&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `94.44%`.

[![Impacted file tree graph](https://codecov.io/gh/beatlabs/patron/pull/191/graphs/tree.svg?width=650&height=150&src=pr&token=sxY15rXW1X)](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
+ Coverage   84.40%   84.41%   +0.01%     
==========================================
  Files          51       52       +1     
  Lines        3046     3074      +28     
==========================================
+ Hits         2571     2595      +24     
- Misses        397      399       +2     
- Partials       78       80       +2     
```


| [Impacted Files](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [client/amqp/amqp.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L2FtcXAvYW1xcC5nbw==) | `40.98% <ø> (ø)` | |
| [client/amqp/option.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L2FtcXAvb3B0aW9uLmdv) | `66.66% <ø> (ø)` | |
| [client/amqp/user.pb.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L2FtcXAvdXNlci5wYi5nbw==) | `26.92% <ø> (ø)` | |
| [client/http/option.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L2h0dHAvb3B0aW9uLmdv) | `100.00% <ø> (ø)` | |
| [client/kafka/kafka.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L2thZmthL2thZmthLmdv) | `84.12% <0.00%> (ø)` | |
| [client/redis/redis.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L3JlZGlzL3JlZGlzLmdv) | `44.44% <ø> (ø)` | |
| [client/sns/message.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L3Nucy9tZXNzYWdlLmdv) | `97.53% <ø> (ø)` | |
| [client/sns/publisher.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L3Nucy9wdWJsaXNoZXIuZ28=) | `93.33% <ø> (ø)` | |
| [client/sqs/message.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L3Nxcy9tZXNzYWdlLmdv) | `92.50% <ø> (ø)` | |
| [client/sqs/publisher.go](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree#diff-Y2xpZW50L3Nxcy9wdWJsaXNoZXIuZ28=) | `93.33% <ø> (ø)` | |
| ... and [31 more](https://codecov.io/gh/beatlabs/patron/pull/191/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=footer). Last update [ad3b80d...9eb38c9](https://codecov.io/gh/beatlabs/patron/pull/191?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/client/kafka/builder.go b/client/kafka/builder.go
index 836886e89..08ea7f6bc 100644
--- a/client/kafka/builder.go
+++ b/client/kafka/builder.go
@@ -9,6 +9,7 @@ import (
 	"github.com/beatlabs/patron/encoding"
 	"github.com/beatlabs/patron/encoding/json"
 	patronErrors "github.com/beatlabs/patron/errors"
+	"github.com/beatlabs/patron/internal/validation"
 	"github.com/beatlabs/patron/log"
 	"github.com/opentracing/opentracing-go"
 )
@@ -48,8 +49,8 @@ func NewBuilder(brokers []string) *AsyncBuilder {
 	cfg.Version = sarama.V0_11_0_0
 
 	errs := []error{}
-	if len(brokers) == 0 {
-		errs = append(errs, errors.New("brokers list is empty"))
+	if validation.IsStringSliceEmpty(brokers) {
+		errs = append(errs, errors.New("brokers are empty or have an empty value"))
 	}
 
 	return &AsyncBuilder{


Test patch:
diff --git a/client/kafka/builder_test.go b/client/kafka/builder_test.go
index e21e60d60..fc1bbb8eb 100644
--- a/client/kafka/builder_test.go
+++ b/client/kafka/builder_test.go
@@ -140,15 +140,16 @@ func TestBrokers(t *testing.T) {
 		{name: "single mock broker", args: args{brokers: []string{seed.Addr()}}, wantErr: false},
 		{name: "multiple mock brokers", args: args{brokers: []string{seed.Addr(), seed.Addr(), seed.Addr()}}, wantErr: false},
 		{name: "empty brokers list", args: args{brokers: []string{}}, wantErr: true},
+		{name: "brokers list with an empty value", args: args{brokers: []string{" ", "value"}}, wantErr: true},
 		{name: "nil brokers list", args: args{brokers: nil}, wantErr: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ab := NewBuilder(tt.args.brokers)
 			if tt.wantErr {
-				assert.Empty(t, ab.brokers)
+				assert.NotEmpty(t, ab.errors)
 			} else {
-				assert.NotEmpty(t, ab.brokers)
+				assert.Empty(t, ab.errors)
 			}
 		})
 	}