Use functional options instead of builder pattern

awslabs · Feb 14, 2022 · 7ea3aa6 · 7ea3aa6
1 parent 022db7c
commit 7ea3aa6
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 14 deletions.
diff --git a/ecr-login/cli/docker-credential-ecr-login/main.go b/ecr-login/cli/docker-credential-ecr-login/main.go
@@ -19,7 +19,6 @@ import (
 	"os"
 
 	ecr "github.com/awslabs/amazon-ecr-credential-helper/ecr-login"
-	"github.com/awslabs/amazon-ecr-credential-helper/ecr-login/api"
 	"github.com/awslabs/amazon-ecr-credential-helper/ecr-login/config"
 	"github.com/awslabs/amazon-ecr-credential-helper/ecr-login/version"
 	"github.com/docker/docker-credential-helpers/credentials"
@@ -42,5 +41,5 @@ func main() {
 	}
 
 	config.SetupLogger()
-	credentials.Serve(ecr.NewECRHelper(api.DefaultClientFactory{}))
+	credentials.Serve(ecr.NewECRHelper())
 }
diff --git a/ecr-login/ecr.go b/ecr-login/ecr.go
@@ -31,16 +31,30 @@ type ECRHelper struct {
 	logger        *logrus.Logger
 }
 
-func NewECRHelper(cf api.ClientFactory) ECRHelper {
-	return ECRHelper{
-		clientFactory: cf,
-		logger:        logrus.StandardLogger(),
+type Option func(*ECRHelper)
+
+func WithClientFactory(clientFactory api.ClientFactory) Option {
+	return func(e *ECRHelper) {
+		e.clientFactory = clientFactory
+	}
+}
+
+func WithLogOutput(w io.Writer) Option {
+	return func(e *ECRHelper) {
+		e.logger.Out = w
 	}
 }
 
-func (self ECRHelper) WithLogOutput(w io.Writer) ECRHelper {
-	self.logger.Out = w
-	return self
+func NewECRHelper(opts ...Option) *ECRHelper {
+	e := &ECRHelper{
+		clientFactory: api.DefaultClientFactory{},
+		logger:        logrus.StandardLogger(),
+	}
+	for _, o := range opts {
+		o(e)
+	}
+
+	return e
 }
 
 // ensure ECRHelper adheres to the credentials.Helper interface

diff --git a/ecr-login/ecr_test.go b/ecr-login/ecr_test.go
@@ -36,7 +36,7 @@ func TestGetSuccess(t *testing.T) {
 	factory := &mock_api.MockClientFactory{}
 	client := &mock_api.MockClient{}
 
-	helper := NewECRHelper(factory)
+	helper := NewECRHelper(WithClientFactory(factory))
 
 	factory.NewClientFromRegionFn = func(_ string) ecr.Client { return client }
 	client.GetCredentialsFn = func(serverURL string) (*ecr.Auth, error) {
@@ -60,7 +60,7 @@ func TestGetError(t *testing.T) {
 	factory := &mock_api.MockClientFactory{}
 	client := &mock_api.MockClient{}
 
-	helper := NewECRHelper(factory)
+	helper := NewECRHelper(WithClientFactory(factory))
 
 	factory.NewClientFromRegionFn = func(_ string) ecr.Client { return client }
 	client.GetCredentialsFn = func(serverURL string) (*ecr.Auth, error) {
@@ -74,7 +74,7 @@ func TestGetError(t *testing.T) {
 }
 
 func TestGetNoMatch(t *testing.T) {
-	helper := NewECRHelper(nil)
+	helper := NewECRHelper(WithClientFactory(nil))
 
 	username, password, err := helper.Get("not-ecr-server-url")
 	assert.True(t, credentials.IsErrCredentialsNotFound(err))
@@ -86,7 +86,7 @@ func TestListSuccess(t *testing.T) {
 	factory := &mock_api.MockClientFactory{}
 	client := &mock_api.MockClient{}
 
-	helper := NewECRHelper(factory)
+	helper := NewECRHelper(WithClientFactory(factory))
 
 	factory.NewClientWithDefaultsFn = func() ecr.Client { return client }
 	client.ListCredentialsFn = func() ([]*ecr.Auth, error) {
@@ -107,7 +107,7 @@ func TestListFailure(t *testing.T) {
 	factory := &mock_api.MockClientFactory{}
 	client := &mock_api.MockClient{}
 
-	helper := NewECRHelper(factory)
+	helper := NewECRHelper(WithClientFactory(factory))
 
 	factory.NewClientWithDefaultsFn = func() ecr.Client { return client }
 	client.ListCredentialsFn = func() ([]*ecr.Auth, error) {