Skip to content

Commit 8b32cd6

Browse files
maxatomemaxatome
committed
fix: BodyContainsBytes & BodyContainsString used with And/Or
Both "rearm" the body before (re-)reading it, so they can be used several times in a same matcher (combined with Or & And). At the same time, simplify the body copy and make the "rearming" more performant. Closes #145. Signed-off-by: Maxime Soulé <[email protected]>
1 parent 497153d commit 8b32cd6

File tree

4 files changed

+65
-58
lines changed

4 files changed

+65
-58
lines changed

‎export_test.go

-4
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,6 @@ func (b *bodyCopyOnRead) Body() io.ReadCloser {
3838
return b.body
3939
}
4040

41-
func (b *bodyCopyOnRead) Buf() []byte {
42-
return b.buf
43-
}
44-
4541
func (b *bodyCopyOnRead) Rearm() {
4642
b.rearm()
4743
}

‎match.go

+27-16
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ type MatcherFunc func(req *http.Request) bool
106106
func matcherFuncOr(mfs []MatcherFunc) MatcherFunc {
107107
return func(req *http.Request) bool {
108108
for _, mf := range mfs {
109+
rearmBody(req)
109110
if mf(req) {
110111
return true
111112
}
@@ -120,6 +121,7 @@ func matcherFuncAnd(mfs []MatcherFunc) MatcherFunc {
120121
}
121122
return func(req *http.Request) bool {
122123
for _, mf := range mfs {
124+
rearmBody(req)
123125
if !mf(req) {
124126
return false
125127
}
@@ -224,6 +226,7 @@ func NewMatcher(name string, fn MatcherFunc) Matcher {
224226
func BodyContainsBytes(subslice []byte) Matcher {
225227
return NewMatcher("",
226228
func(req *http.Request) bool {
229+
rearmBody(req)
227230
b, err := ioutil.ReadAll(req.Body)
228231
return err == nil && bytes.Contains(b, subslice)
229232
})
@@ -243,6 +246,7 @@ func BodyContainsBytes(subslice []byte) Matcher {
243246
func BodyContainsString(substr string) Matcher {
244247
return NewMatcher("",
245248
func(req *http.Request) bool {
249+
rearmBody(req)
246250
b, err := ioutil.ReadAll(req.Body)
247251
return err == nil && bytes.Contains(b, []byte(substr))
248252
})
@@ -465,27 +469,40 @@ func (m matchRouteKey) String() string {
465469
return m.RouteKey.String() + " <" + m.name + ">"
466470
}
467471

468-
// bodyCopyOnRead copies body content to buf on first Read(), except
472+
func rearmBody(req *http.Request) {
473+
if req != nil {
474+
if body, ok := req.Body.(interface{ rearm() }); ok {
475+
body.rearm()
476+
}
477+
}
478+
}
479+
480+
type buffer struct {
481+
*bytes.Reader
482+
}
483+
484+
func (b buffer) Close() error {
485+
return nil
486+
}
487+
488+
// bodyCopyOnRead mutates body into a buffer on first Read(), except
469489
// if body is nil or http.NoBody. In this case, EOF is returned for
470-
// each Read() and buf stays to nil.
490+
// each Read() and body stays untouched.
471491
type bodyCopyOnRead struct {
472492
body io.ReadCloser
473-
buf []byte
474493
}
475494

476495
func (b *bodyCopyOnRead) rearm() {
477-
if b.buf != nil {
478-
b.body = ioutil.NopCloser(bytes.NewReader(b.buf))
496+
if buf, ok := b.body.(buffer); ok {
497+
buf.Seek(0, io.SeekStart) //nolint:errcheck
479498
} // else b.body contains the original body, so don't touch
480499
}
481500

482501
func (b *bodyCopyOnRead) copy() {
483-
if b.buf == nil && b.body != nil && b.body != http.NoBody {
484-
var body bytes.Buffer
485-
io.Copy(&body, b.body) //nolint: errcheck
502+
if _, ok := b.body.(buffer); !ok && b.body != nil && b.body != http.NoBody {
503+
buf, _ := ioutil.ReadAll(b.body)
486504
b.body.Close()
487-
b.buf = body.Bytes()
488-
b.body = ioutil.NopCloser(bytes.NewReader(b.buf))
505+
b.body = buffer{bytes.NewReader(buf)}
489506
}
490507
}
491508

@@ -500,9 +517,3 @@ func (b *bodyCopyOnRead) Read(p []byte) (n int, err error) {
500517
func (b *bodyCopyOnRead) Close() error {
501518
return nil
502519
}
503-
504-
// Len returns the buffer total length, whatever the Read position in body is.
505-
func (b *bodyCopyOnRead) Len() int {
506-
b.copy()
507-
return len(b.buf)
508-
}

‎match_test.go

+19-36
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,30 @@ func TestNewMatcher(t *testing.T) {
8484
return req
8585
}
8686

87+
reqCopyBody := func(t testing.TB, body string, header ...string) *http.Request {
88+
req := req(t, body, header...)
89+
req.Body = httpmock.NewBodyCopyOnRead(req.Body)
90+
return req
91+
}
92+
8793
t.Run("BodyContainsBytes", func(t *testing.T) {
8894
m := httpmock.BodyContainsBytes([]byte("ip"))
8995
td.Cmp(t, m.Name(), autogenName)
9096
td.CmpTrue(t, m.Check(req(t, "pipo")))
9197
td.CmpFalse(t, m.Check(req(t, "bingo")))
98+
99+
td.CmpTrue(t, m.Check(reqCopyBody(t, "pipo")))
100+
td.CmpFalse(t, m.Check(reqCopyBody(t, "bingo")))
92101
})
93102

94103
t.Run("BodyContainsString", func(t *testing.T) {
95104
m := httpmock.BodyContainsString("ip")
96105
td.Cmp(t, m.Name(), autogenName)
97106
td.CmpTrue(t, m.Check(req(t, "pipo")))
98107
td.CmpFalse(t, m.Check(req(t, "bingo")))
108+
109+
td.CmpTrue(t, m.Check(reqCopyBody(t, "pipo")))
110+
td.CmpFalse(t, m.Check(reqCopyBody(t, "bingo")))
99111
})
100112

101113
t.Run("HeaderExists", func(t *testing.T) {
@@ -394,15 +406,17 @@ func TestBodyCopyOnRead(t *testing.T) {
394406
bc := httpmock.NewBodyCopyOnRead(body)
395407

396408
bc.Rearm()
397-
td.CmpNil(t, bc.Buf())
409+
td.Cmp(t, body, bc.Body(), "rearm didn't touch anything")
398410

399411
var buf [4]byte
400412
n, err := bc.Read(buf[:])
401413
td.CmpNoError(t, err)
402414
td.Cmp(t, n, 4)
403415
td.CmpString(t, buf[:], "BODY")
404416

405-
td.CmpString(t, bc.Buf(), "BODY", "Original body has been copied internally")
417+
td.Cmp(t, body, td.Not(bc.Body()), "Original body has been copied internally")
418+
419+
td.CmpNoError(t, bc.Body().Close()) // for coverage... :)
406420

407421
n, err = bc.Read(buf[:])
408422
td.Cmp(t, err, io.EOF)
@@ -435,55 +449,24 @@ func TestBodyCopyOnRead(t *testing.T) {
435449
bc := httpmock.NewBodyCopyOnRead(tc.body)
436450

437451
bc.Rearm()
438-
td.CmpNil(t, bc.Buf())
452+
td.Cmp(t, tc.body, bc.Body(), "rearm didn't touch anything")
439453

440454
var buf [4]byte
441455
n, err := bc.Read(buf[:])
442456
td.Cmp(t, err, io.EOF)
443457
td.Cmp(t, n, 0)
444-
td.CmpNil(t, bc.Buf())
445-
td.Cmp(t, bc.Body(), tc.body)
458+
td.Cmp(t, bc.Body(), tc.body, "body is not altered")
446459

447460
bc.Rearm()
448461

449462
n, err = bc.Read(buf[:])
450463
td.Cmp(t, err, io.EOF)
451464
td.Cmp(t, n, 0)
452-
td.CmpNil(t, bc.Buf())
453-
td.Cmp(t, bc.Body(), tc.body)
465+
td.Cmp(t, bc.Body(), tc.body, "body is not altered")
454466

455467
td.CmpNoError(t, bc.Close())
456468
})
457469
}
458-
459-
t.Run("len", func(t *testing.T) {
460-
testCases := []struct {
461-
name string
462-
bc interface{ Len() int }
463-
expected int
464-
}{
465-
{
466-
name: "nil",
467-
bc: httpmock.NewBodyCopyOnRead(nil),
468-
expected: 0,
469-
},
470-
{
471-
name: "no body",
472-
bc: httpmock.NewBodyCopyOnRead(http.NoBody),
473-
expected: 0,
474-
},
475-
{
476-
name: "filled",
477-
bc: httpmock.NewBodyCopyOnRead(ioutil.NopCloser(bytes.NewReader([]byte(`BODY`)))),
478-
expected: 4,
479-
},
480-
}
481-
for _, tc := range testCases {
482-
t.Run(tc.name, func(t *testing.T) {
483-
td.Cmp(t, tc.bc.Len(), tc.expected)
484-
})
485-
}
486-
})
487470
}
488471

489472
func TestExtractPackage(t *testing.T) {

‎transport_test.go

+19-2
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@ func TestRegisterMatcherResponder(t *testing.T) {
123123
}),
124124
NewStringResponder(200, "body-FOO"))
125125

126+
RegisterMatcherResponder("POST", "/foo",
127+
BodyContainsString("xxx").
128+
Or(BodyContainsString("yyy")).
129+
WithName("03-body-xxx|yyy"),
130+
NewStringResponder(200, "body-xxx|yyy"))
131+
126132
RegisterResponder("POST", "/foo", NewStringResponder(200, "default"))
127133

128134
RegisterNoResponder(NewNotFoundResponder(nil))
@@ -156,6 +162,16 @@ func TestRegisterMatcherResponder(t *testing.T) {
156162
body: "FOO",
157163
expectedBody: "body-FOO",
158164
},
165+
{
166+
name: "body xxx",
167+
body: "...xxx...",
168+
expectedBody: "body-xxx|yyy",
169+
},
170+
{
171+
name: "body yyy",
172+
body: "...yyy...",
173+
expectedBody: "body-xxx|yyy",
174+
},
159175
{
160176
name: "default",
161177
body: "ANYTHING",
@@ -194,12 +210,13 @@ func TestRegisterMatcherResponder(t *testing.T) {
194210
"text/plain",
195211
strings.NewReader("ANYTHING"),
196212
)
197-
assert.HasSuffix(err, `Responder not found for POST http://test.com/foo despite 3 matchers: ["00-header-foo=bar" "01-body-BAR" "02-body-FOO"]`)
213+
assert.HasSuffix(err, `Responder not found for POST http://test.com/foo despite 4 matchers: ["00-header-foo=bar" "01-body-BAR" "02-body-FOO" "03-body-xxx|yyy"]`)
198214
})
199215

200-
// Remove 2 matcher responders
216+
// Remove 3 matcher responders
201217
RegisterMatcherResponder("POST", "/foo", NewMatcher("01-body-BAR", nil), nil)
202218
RegisterMatcherResponder("POST", "/foo", NewMatcher("02-body-FOO", nil), nil)
219+
RegisterMatcherResponder("POST", "/foo", NewMatcher("03-body-xxx|yyy", nil), nil)
203220

204221
assert.Run("not found despite 1", func(assert *td.T) {
205222
_, err := http.Post(

0 commit comments

Comments
 (0)