Updates to RequestMutator (#53)

Updates to RequestMutator

Added NewBlankRequestMatcher
Renamed NewDefaultRequestMatcher to NewStrictRequestMatcher
README updates
Added Test_Mutator_Multiple_On

.golangci.yml

@@ -1741,7 +1741,7 @@ linters:
     # - nakedret
     - nestif
     - nilerr
-    - nilnil
+    #- nilnil
     # - nlreturn
     # - noctx
     - nolintlint
@@ -1885,4 +1885,4 @@ severity:
   rules:
     - linters:
       - dupl
-      severity: info
+      severity: info

README.md

@@ -27,7 +27,7 @@ On **subsequent executions** (unless you delete the cassette file), the HTTP cal
 
 Note:
 
-We use a "relaxed" request matcher because `example.com` inject a `Age` header that varies per-request. Without a mutator, govcr's default strict matcher would not match the track on the cassette and keep sending live requests (and record them to the cassette).
+We use a "relaxed" request matcher because `example.com` injects an "`Age`" header that varies per-request. Without a mutator, govcr's default strict matcher would not match the track on the cassette and keep sending live requests (and record them to the cassette).
 
 ## Install
 
@@ -114,10 +114,12 @@ You can create your own matcher on any part of the request and in any manner (li
 
 The live HTTP request and response traffic is protected against modifications. While **govcr** could easily support in-place mutation of the live traffic, this is not a goal.
 
-Nonetheless, **govcr** supports mutating tracks, either at recording time or at playback time.
+Nonetheless, **govcr** supports mutating tracks, either at **recording time** or at **playback time**.
 
 In either case, this is achieved with track `Mutators`.
 
+A `Mutator` can be combined with one or more `On` conditions. At present, all `On` conditions attached to a mutator must be true for the mutator to apply.
+
 A **track recording mutator** can change both the request and the response that will be persisted to the cassette.
 
 A **track replaying mutator** transforms the track after it was matched and retrieved from the cassette. It does not change the cassette file.

cassette/cassette.go

@@ -87,6 +87,7 @@ func (k7 *Cassette) NumberOfTracks() int32 {
 // ReplayTrack returns the specified track number, as recorded on cassette.
 func (k7 *Cassette) ReplayTrack(trackNumber int32) (*track.Track, error) {
 	if trackNumber >= k7.NumberOfTracks() {
+		//nolint: err113
 		return nil, fmt.Errorf("invalid track number %d (only %d available) (track #0 stands for first track)", trackNumber, k7.NumberOfTracks())
 	}
 

cassette/track/http.go

@@ -177,8 +177,18 @@ func cloneHTTPRequestBody(httpRequest *http.Request) []byte {
 
 	var httpBodyClone []byte
 	if httpRequest.Body != nil {
-		httpBodyClone, _ = ioutil.ReadAll(httpRequest.Body)
-		_ = httpRequest.Body.Close()
+		var err error
+
+		httpBodyClone, err = ioutil.ReadAll(httpRequest.Body)
+		if err != nil {
+			log.Println("cloneHTTPRequestBody - httpBodyClone:", err)
+		}
+
+		err = httpRequest.Body.Close()
+		if err != nil {
+			log.Println("cloneHTTPRequestBody - httpRequest.Body.Close:", err)
+		}
+
 		httpRequest.Body = ioutil.NopCloser(bytes.NewBuffer(httpBodyClone))
 	}
 
@@ -192,8 +202,18 @@ func cloneHTTPResponseBody(httpResponse *http.Response) []byte {
 
 	var httpBodyClone []byte
 	if httpResponse.Body != nil {
-		httpBodyClone, _ = ioutil.ReadAll(httpResponse.Body)
-		_ = httpResponse.Body.Close()
+		var err error
+
+		httpBodyClone, err = ioutil.ReadAll(httpResponse.Body)
+		if err != nil {
+			log.Println("cloneHTTPResponseBody - httpBodyClone:", err)
+		}
+
+		err = httpResponse.Body.Close()
+		if err != nil {
+			log.Println("cloneHTTPResponseBody - httpResponse.Body.Close:", err)
+		}
+
 		httpResponse.Body = ioutil.NopCloser(bytes.NewBuffer(httpBodyClone))
 	}
 

cassette/track/mutator_test.go

@@ -2,6 +2,7 @@ package track_test
 
 import (
 	"errors"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -101,4 +102,70 @@ func Test_Mutator_OnErr_WhenNoErr(t *testing.T) {
 	require.Nil(t, trk.ErrMsg)
 }
 
+func Test_Mutator_Multiple_On(t *testing.T) {
+	tt := map[string]struct {
+		mutatorOnFn func(track.Mutator) track.Mutator
+		wantMethod  string
+	}{
+		"2 On's, both matched": {
+			mutatorOnFn: func(m track.Mutator) track.Mutator {
+				return m.
+					OnRequestMethod(http.MethodPost).
+					OnNoErr()
+			},
+			wantMethod: http.MethodPost + " has been mutated",
+		},
+		"2 On's, 1st matches, 2nd does not": {
+			mutatorOnFn: func(m track.Mutator) track.Mutator {
+				return m.
+					OnRequestMethod(http.MethodPost).
+					OnErr()
+			},
+			wantMethod: http.MethodPost,
+		},
+		"2 On's, 1st does not matches, 2nd does": {
+			mutatorOnFn: func(m track.Mutator) track.Mutator {
+				return m.
+					OnRequestMethod(http.MethodGet).
+					OnNoErr()
+			},
+			wantMethod: http.MethodPost,
+		},
+		"2 On's, none matches": {
+			mutatorOnFn: func(m track.Mutator) track.Mutator {
+				return m.
+					OnRequestMethod(http.MethodGet).
+					OnErr()
+			},
+			wantMethod: http.MethodPost,
+		},
+	}
+
+	mutator := track.Mutator(
+		func(tk *track.Track) {
+			tk.Request.Method = tk.Request.Method + " has been mutated"
+		})
+
+	for name, tc := range tt {
+		name := name
+		tc := tc
+
+		t.Run(name, func(t *testing.T) {
+			trk := track.NewTrack(
+				&track.Request{
+					Method: http.MethodPost,
+				},
+				&track.Response{
+					Status: "BadStatus",
+				},
+				nil,
+			)
+
+			tc.mutatorOnFn(mutator)(trk)
+
+			require.Equal(t, tc.wantMethod, trk.Request.Method)
+		})
+	}
+}
+
 func strPtr(s string) *string { return &s }

cassette/track/track.go

@@ -92,10 +92,12 @@ func (trk *Track) ToErr() error {
 			Net:    "govcr",
 			Source: nil,
 			Addr:   nil,
-			Err:    errors.New(errType + ": " + errMsg),
+			//nolint: err113
+			Err: errors.New(errType + ": " + errMsg),
 		}
 	}
 
+	//nolint: err113
 	return errors.New(errType + ": " + errMsg)
 }
 

govcr.go

@@ -22,7 +22,7 @@ func NewVCR(settings ...Setting) *ControlPanel {
 
 	// use a default RequestMatcher if none provided
 	if vcrSettings.requestMatcher == nil {
-		vcrSettings.requestMatcher = NewDefaultRequestMatcher()
+		vcrSettings.requestMatcher = NewStrictRequestMatcher()
 	}
 
 	// create VCR's HTTP client

matchers.go

@@ -53,8 +53,23 @@ func WithRequestMatcherFunc(m RequestMatcherFunc) DefaultRequestMatcherOptions {
 	}
 }
 
-// NewDefaultRequestMatcher creates a new default implementation of RequestMatcher.
-func NewDefaultRequestMatcher(options ...DefaultRequestMatcherOptions) *DefaultRequestMatcher {
+// NewBlankRequestMatcher creates a new default implementation of RequestMatcher.
+// By default, it will always match any and all requests to a cassette track.
+// You should pass specific RequestMatcherFunc as options to customise its behaviour.
+// You can also use one of the predefined matchers such as those provided by
+// NewStrictRequestMatcher() or NewMethodURLRequestMatcher().
+func NewBlankRequestMatcher(options ...DefaultRequestMatcherOptions) *DefaultRequestMatcher {
+	drm := DefaultRequestMatcher{}
+
+	for _, option := range options {
+		option(&drm)
+	}
+
+	return &drm
+}
+
+// NewStrictRequestMatcher creates a new default implementation of RequestMatcher.
+func NewStrictRequestMatcher() *DefaultRequestMatcher {
 	drm := DefaultRequestMatcher{
 		matchers: []RequestMatcherFunc{
 			DefaultHeaderMatcher,
@@ -65,26 +80,18 @@ func NewDefaultRequestMatcher(options ...DefaultRequestMatcherOptions) *DefaultR
 		},
 	}
 
-	for _, option := range options {
-		option(&drm)
-	}
-
 	return &drm
 }
 
 // NewMethodURLRequestMatcher creates a new implementation of RequestMatcher based on Method and URL.
-func NewMethodURLRequestMatcher(options ...DefaultRequestMatcherOptions) *DefaultRequestMatcher {
+func NewMethodURLRequestMatcher() *DefaultRequestMatcher {
 	drm := DefaultRequestMatcher{
 		matchers: []RequestMatcherFunc{
 			DefaultMethodMatcher,
 			DefaultURLMatcher,
 		},
 	}
 
-	for _, option := range options {
-		option(&drm)
-	}
-
 	return &drm
 }
 
@@ -105,7 +112,7 @@ func DefaultMethodMatcher(httpRequest, trackRequest *track.Request) bool {
 // DefaultURLMatcher is the default implementation of URLMatcher.
 // Because this function is meant to be called from DefaultRequestMatcher.Match(),
 // it doesn't check for either argument to be nil. Match() takes care of it.
-//nolint:gocyclo
+//nolint:gocyclo,gocognit
 func DefaultURLMatcher(httpRequest, trackRequest *track.Request) bool {
 	httpURL := httpRequest.URL
 	if httpURL == nil {
@@ -142,6 +149,7 @@ func DefaultTrailerMatcher(httpRequest, trackRequest *track.Request) bool {
 	return areHTTPHeadersEqual(httpRequest.Trailer, trackRequest.Trailer)
 }
 
+//nolint:gocyclo,gocognit
 func areHTTPHeadersEqual(httpHeaders1, httpHeaders2 http.Header) bool {
 	if len(httpHeaders1) != len(httpHeaders2) {
 		return false

pcb.go

@@ -30,6 +30,7 @@ func (pcb *PrintedCircuitBoard) seekTrack(k7 *cassette.Cassette, httpRequest *ht
 			return pcb.replayTrack(k7, trackNumber)
 		}
 	}
+
 	return nil, nil
 }