refactor: use the function options pattern for external functions (#31)

Author: teodora-sandu

README.md

@@ -15,51 +15,55 @@ $ go get github.com/snyk/code-client-go
 
 Use the HTTP client to make HTTP requests with configured retriable codes and authorisation headers for Snyk Rest APIs.
 
-Implement the `github.com/snyk/code-client-go/http.Config` interface to configure the Snyk Code API client from applications.
+You can either configure the client using the functional options pattern provided or by implementing the interfaces.
 
-Provide a net/http.Client factory to customize the underlying HTTP protocol behavior (timeouts, etc).
+Provide a `net/http.Client` factory to customize the underlying HTTP protocol behavior (timeouts, etc).
 
 ```go
 import (
     "net/http"
 
     "github.com/rs/zerolog"
-    codehttp "github.com/snyk/code-client-go/http"
+    codeClientHTTP "github.com/snyk/code-client-go/http"
+    codeClientObservability  "github.com/snyk/code-client-go/observability"
 )
 
 logger := zerlog.NewLogger(...)
-config := newConfigForMyApp()
-httpClient := codehttp.NewHTTPClient(logger, config, func() *http.Client { return http.DefaultClient }, codeInstrumentor, codeErrorReporter)
+instrumentor := codeClientObservability.NewInstrumentor()
+errorReporter := codeClientObservability.NewErrorReporter()
+httpClient := codeClientHTTP.NewHTTPClient(
+    func() *http.Client {
+        return &http.Client{
+            Timeout: time.Duration(1) * time.Second,
+        }
+    },
+    codeClientHTTP.WithRetryCount(1),
+    codeClientHTTP.WithLogger(logger),
+    codeClientHTTP.WithInstrumentor(instrumentor),
+    codeClientHTTP.WithErrorReporter(errorReporter),
+)
 ```
 
-The HTTP client exposes a `DoCall` function.
+The HTTP client exposes a `Do` function.
 
 ### Configuration
 
-Implement the `http.Config` interface to configure the Snyk Code API client from applications.
+Implement the `config.Config` interface to configure the Snyk Code API client from applications.
 
 ### Code Scanner
 
 Use the Code Scanner to trigger a scan for a Snyk Code workspace using the Bundle Manager created above.
+
 The Code Scanner exposes a `UploadAndAnalyze` function, which can be used like this:
 
 ```go
-import (
-    "net/http"
-    
-    "github.com/rs/zerolog"
-    code "github.com/snyk/code-client-go"
-)
-
-logger := zerlog.NewLogger(...)
 config := newConfigForMyApp()
-
 codeScanner := code.NewCodeScanner(
     httpClient,
-	config,
-    codeInstrumentor, 
-	codeErrorReporter,
-    logger,
+    config,
+    codeClientHTTP.WithLogger(logger),
+    codeClientHTTP.WithInstrumentor(instrumentor),
+    codeClientHTTP.WithErrorReporter(errorReporter),
 )
 code.UploadAndAnalyze(context.Background(), requestId, "path/to/workspace", channelForWalkingFiles, changedFiles)
 ```

http/http.go

@@ -33,12 +33,14 @@ type HTTPClient interface {
 	Do(req *http.Request) (*http.Response, error)
 }
 
+type HTTPClientFactory func() *http.Client
+
 type httpClient struct {
-	retryCount    int
-	clientFactory func() *http.Client
-	instrumentor  observability.Instrumentor
-	errorReporter observability.ErrorReporter
-	logger        *zerolog.Logger
+	retryCount        int
+	httpClientFactory HTTPClientFactory
+	instrumentor      observability.Instrumentor
+	errorReporter     observability.ErrorReporter
+	logger            *zerolog.Logger
 }
 
 type OptionFunc func(*httpClient)
@@ -68,18 +70,18 @@ func WithLogger(logger *zerolog.Logger) OptionFunc {
 }
 
 func NewHTTPClient(
-	clientFactory func() *http.Client,
+	httpClientFactory HTTPClientFactory,
 	options ...OptionFunc,
 ) HTTPClient {
 	nopLogger := zerolog.Nop()
 	instrumentor := observability.NewInstrumentor()
 	errorReporter := observability.NewErrorReporter(&nopLogger)
 	client := &httpClient{
-		retryCount:    3,
-		clientFactory: clientFactory,
-		instrumentor:  instrumentor,
-		errorReporter: errorReporter,
-		logger:        &nopLogger,
+		retryCount:        3,
+		httpClientFactory: httpClientFactory,
+		instrumentor:      instrumentor,
+		errorReporter:     errorReporter,
+		logger:            &nopLogger,
 	}
 
 	for _, option := range options {
@@ -137,7 +139,7 @@ func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) {
 		req.Body = reqBody
 		s.logger.Debug().Str("url", req.URL.String()).Str("snyk-request-id", requestId).Str("requestBody", string(reqBuf)).Msg("SEND TO REMOTE")
 	}
-	response, err := s.clientFactory().Do(req)
+	response, err := s.httpClientFactory().Do(req)
 	req.Body = copyReqBody
 	if response != nil {
 		var copyResBody io.ReadCloser

internal/analysis/analysis.go

@@ -53,11 +53,11 @@ type AnalysisOrchestrator interface {
 }
 
 func NewAnalysisOrchestrator(
+	config config.Config,
 	logger *zerolog.Logger,
 	httpClient codeClientHTTP.HTTPClient,
 	instrumentor observability.Instrumentor,
 	errorReporter observability.ErrorReporter,
-	config config.Config,
 ) *analysisOrchestrator {
 	return &analysisOrchestrator{
 		httpClient,

internal/analysis/analysis_test.go

@@ -70,7 +70,7 @@ func TestAnalysis_CreateWorkspace(t *testing.T) {
 
 	logger := zerolog.Nop()
 
-	analysisOrchestrator := analysis.NewAnalysisOrchestrator(&logger, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter)
 	_, err := analysisOrchestrator.CreateWorkspace(
 		context.Background(),
 		"4a72d1db-b465-4764-99e1-ecedad03b06a",
@@ -99,7 +99,7 @@ func TestAnalysis_CreateWorkspace_NotARepository(t *testing.T) {
 	logger := zerolog.Nop()
 
 	repoDir := t.TempDir()
-	analysisOrchestrator := analysis.NewAnalysisOrchestrator(&logger, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter)
 	_, err := analysisOrchestrator.CreateWorkspace(
 		context.Background(),
 		"4a72d1db-b465-4764-99e1-ecedad03b06a",
@@ -143,7 +143,7 @@ func TestAnalysis_CreateWorkspace_Failure(t *testing.T) {
 
 	logger := zerolog.Nop()
 
-	analysisOrchestrator := analysis.NewAnalysisOrchestrator(&logger, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter)
 	_, err := analysisOrchestrator.CreateWorkspace(
 		context.Background(),
 		"4a72d1db-b465-4764-99e1-ecedad03b06a",
@@ -240,7 +240,7 @@ func TestAnalysis_RunAnalysis(t *testing.T) {
 
 	logger := zerolog.Nop()
 
-	analysisOrchestrator := analysis.NewAnalysisOrchestrator(&logger, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter)
 	actual, err := analysisOrchestrator.RunAnalysis()
 	require.NoError(t, err)
 	assert.Equal(t, "COMPLETE", actual.Status)

internal/bundle/bundle.go

@@ -34,7 +34,7 @@ type Bundle interface {
 }
 
 type deepCodeBundle struct {
-	SnykCode      deepcode.SnykCodeClient
+	SnykCode      deepcode.DeepcodeClient
 	instrumentor  observability.Instrumentor
 	errorReporter observability.ErrorReporter
 	logger        *zerolog.Logger
@@ -46,7 +46,7 @@ type deepCodeBundle struct {
 }
 
 func NewBundle(
-	snykCode deepcode.SnykCodeClient,
+	snykCode deepcode.DeepcodeClient,
 	instrumentor observability.Instrumentor,
 	errorReporter observability.ErrorReporter,
 	logger *zerolog.Logger,

internal/bundle/bundle_manager.go

@@ -31,7 +31,7 @@ import (
 
 // TODO: add progress tracker for percentage progress
 type bundleManager struct {
-	SnykCode             deepcode.SnykCodeClient
+	deepcodeClient       deepcode.DeepcodeClient
 	instrumentor         observability.Instrumentor
 	errorReporter        observability.ErrorReporter
 	logger               *zerolog.Logger
@@ -57,13 +57,13 @@ type BundleManager interface {
 }
 
 func NewBundleManager(
+	deepcodeClient deepcode.DeepcodeClient,
 	logger *zerolog.Logger,
-	SnykCode deepcode.SnykCodeClient,
 	instrumentor observability.Instrumentor,
 	errorReporter observability.ErrorReporter,
 ) *bundleManager {
 	return &bundleManager{
-		SnykCode:             SnykCode,
+		deepcodeClient:       deepcodeClient,
 		instrumentor:         instrumentor,
 		errorReporter:        errorReporter,
 		logger:               logger,
@@ -133,10 +133,10 @@ func (b *bundleManager) Create(ctx context.Context,
 	var bundleHash string
 	var missingFiles []string
 	if len(fileHashes) > 0 {
-		bundleHash, missingFiles, err = b.SnykCode.CreateBundle(span.Context(), fileHashes)
+		bundleHash, missingFiles, err = b.deepcodeClient.CreateBundle(span.Context(), fileHashes)
 	}
 	bundle = NewBundle(
-		b.SnykCode,
+		b.deepcodeClient,
 		b.instrumentor,
 		b.errorReporter,
 		b.logger,
@@ -213,7 +213,7 @@ func (b *bundleManager) groupInBatches(
 
 func (b *bundleManager) IsSupported(ctx context.Context, file string) (bool, error) {
 	if b.supportedExtensions.Size() == 0 && b.supportedConfigFiles.Size() == 0 {
-		filters, err := b.SnykCode.GetFilters(ctx)
+		filters, err := b.deepcodeClient.GetFilters(ctx)
 		if err != nil {
 			b.logger.Error().Err(err).Msg("could not get filters")
 			return false, err