From 32912fb5bd76bb4a931fb5e41930999afb110700 Mon Sep 17 00:00:00 2001 From: Seb C Date: Mon, 1 Aug 2022 19:49:32 +0100 Subject: [PATCH] Updates to RequestMutator (#53) Updates to RequestMutator Added NewBlankRequestMatcher Renamed NewDefaultRequestMatcher to NewStrictRequestMatcher README updates Added Test_Mutator_Multiple_On --- .golangci.yml | 4 +- README.md | 6 ++- cassette/cassette.go | 1 + cassette/track/http.go | 28 ++++++++++++-- cassette/track/mutator_test.go | 67 ++++++++++++++++++++++++++++++++++ cassette/track/track.go | 4 +- govcr.go | 2 +- matchers.go | 32 ++++++++++------ pcb.go | 1 + 9 files changed, 123 insertions(+), 22 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9dabeb9..13711a7 100644 --- a/.golangci.yml +++ b/.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 \ No newline at end of file + severity: info diff --git a/README.md b/README.md index f1ef41a..e1bb77a 100644 --- a/README.md +++ b/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. diff --git a/cassette/cassette.go b/cassette/cassette.go index 7c86670..774d8d0 100644 --- a/cassette/cassette.go +++ b/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()) } diff --git a/cassette/track/http.go b/cassette/track/http.go index 63c0c25..d4afbdc 100644 --- a/cassette/track/http.go +++ b/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)) } diff --git a/cassette/track/mutator_test.go b/cassette/track/mutator_test.go index 0d44c97..fb2ab64 100644 --- a/cassette/track/mutator_test.go +++ b/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 } diff --git a/cassette/track/track.go b/cassette/track/track.go index 519821d..8686c53 100644 --- a/cassette/track/track.go +++ b/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) } diff --git a/govcr.go b/govcr.go index e45d2fa..1b8cdc1 100644 --- a/govcr.go +++ b/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 diff --git a/matchers.go b/matchers.go index 5954c4d..00c6016 100644 --- a/matchers.go +++ b/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,15 +80,11 @@ 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, @@ -81,10 +92,6 @@ func NewMethodURLRequestMatcher(options ...DefaultRequestMatcherOptions) *Defaul }, } - 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 diff --git a/pcb.go b/pcb.go index 6f37848..71e2099 100644 --- a/pcb.go +++ b/pcb.go @@ -30,6 +30,7 @@ func (pcb *PrintedCircuitBoard) seekTrack(k7 *cassette.Cassette, httpRequest *ht return pcb.replayTrack(k7, trackNumber) } } + return nil, nil }