Skip to content

Commit

Permalink
Ignore order of query params of a request for recordings and playbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
mplushnikov committed Nov 1, 2022
1 parent 86fa277 commit 7219220
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 10 deletions.
28 changes: 19 additions & 9 deletions implementation/playback/playback.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,34 @@ func New(recorderPath string) aurestclientapi.Client {
}

func (c *PlaybackImpl) Perform(ctx context.Context, method string, requestUrl string, _ interface{}, response *aurestclientapi.ParsedResponse) error {
filename, err := aurestrecorder.ConstructFilenameV2(method, requestUrl)
filename, err := aurestrecorder.ConstructFilenameV3(method, requestUrl)
if err != nil {
return err
}

jsonBytes, err := os.ReadFile(c.RecorderPath + filename)
if err != nil {
// try old filename for compatibility (cannot fail if ConstructFilenameV2 didn't)
filenameOld, _ := aurestrecorder.ConstructFilename(method, requestUrl)
filenameOldV1, _ := aurestrecorder.ConstructFilename(method, requestUrl)

jsonBytesOld, errWithOldFilename := os.ReadFile(c.RecorderPath + filenameOld)
if errWithOldFilename != nil {
// but return original error if that also fails
return err
jsonBytesOldV1, errWithOldFilenameV1 := os.ReadFile(c.RecorderPath + filenameOldV1)
if errWithOldFilenameV1 != nil {
// try old filename for compatibility (cannot fail if ConstructFilenameV2 didn't)
filenameOldV2, _ := aurestrecorder.ConstructFilenameV2(method, requestUrl)

jsonBytesOldV2, errWithOldFilenameV2 := os.ReadFile(c.RecorderPath + filenameOldV2)
if errWithOldFilenameV2 != nil {
// but return original error if that also fails
return err
} else {
aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename (v2) '%s', please move to '%s'", filenameOldV2, filename)
filename = filenameOldV2
jsonBytes = jsonBytesOldV2
}
} else {
aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename '%s', please move to '%s'", filenameOld, filename)
filename = filenameOld
jsonBytes = jsonBytesOld
aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename (v1) '%s', please move to '%s'", filenameOldV1, filename)
filename = filenameOldV1
jsonBytes = jsonBytesOldV1
}
}

Expand Down
26 changes: 25 additions & 1 deletion implementation/recorder/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type RecorderData struct {
func (c *RecorderImpl) Perform(ctx context.Context, method string, requestUrl string, requestBody interface{}, response *aurestclientapi.ParsedResponse) error {
responseErr := c.Wrapped.Perform(ctx, method, requestUrl, requestBody, response)
if c.RecorderPath != "" {
filename, err := ConstructFilenameV2(method, requestUrl)
filename, err := ConstructFilenameV3(method, requestUrl)
if err == nil {
recording := RecorderData{
Method: method,
Expand Down Expand Up @@ -111,3 +111,27 @@ func ConstructFilenameV2(method string, requestUrl string) (string, error) {
filename := fmt.Sprintf("request_%s_%s_%s.json", m, p, q)
return filename, nil
}

func ConstructFilenameV3(method string, requestUrl string) (string, error) {
parsedUrl, err := url.Parse(requestUrl)
if err != nil {
return "", err
}

m := strings.ToLower(method)
p := url.QueryEscape(parsedUrl.EscapedPath())
if len(p) > 120 {
p = string([]byte(p)[:120])
}
p = strings.ReplaceAll(p, "%2F", "-")
p = strings.TrimLeft(p, "-")
p = strings.TrimRight(p, "-")

// we have to ensure the filenames don't get too long. git for windows only supports 260 character paths
md5sumOverQuery := md5.Sum([]byte(parsedUrl.Query().Encode()))
q := hex.EncodeToString(md5sumOverQuery[:])
q = q[:8]

filename := fmt.Sprintf("request_%s_%s_%s.json", m, p, q)
return filename, nil
}
26 changes: 26 additions & 0 deletions implementation/recorder/recorder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,29 @@ func TestConstructFilenameV2Short(t *testing.T) {
require.Nil(t, err)
require.Equal(t, expected, actual)
}

func TestConstructFilenameV3Long(t *testing.T) {
requestUrl := "https://some.super.long.server.name.that.hopefully.does.not.exist/api/rest/v1/v2/v3/v4/this/is/intentionally/very/very/very/very/long/djkfjhdalsfhdsahjflkdjsahfkjlsdhafjkdshafkjlsdahf/asdfjkldsahfkjlfad/dskjfhjkdsfahlk/sdafjkhsdafklhreuih/dfgjgkjlhgjlkhg?asjdfhlkhewuirfhkjsdhlk=kjhrelrukihsjd&fsdkjhfdjklhsdf=werjkyewuiryuweiry&sfuyfddsjkhjkldsfhldkfs=sdjhdflksjhfdhsf"
actual, err := ConstructFilenameV3("GET", requestUrl)
expected := "request_get_api-rest-v1-v2-v3-v4-this-is-intentionally-very-very-very-very-long-djkfjhdalsfhdsahjflkd_fb2e3656.json"
require.Nil(t, err)
require.Equal(t, expected, actual)
}

func TestConstructFilenameV3Short(t *testing.T) {
requestUrl := "https://some.short.server.name/api/rest/v1/kittens"
actual, err := ConstructFilenameV3("GET", requestUrl)
expected := "request_get_api-rest-v1-kittens_d41d8cd9.json"
require.Nil(t, err)
require.Equal(t, expected, actual)
}

func TestConstructEqualFilenameV3ForDifferentQueryParameterOrder(t *testing.T) {
requestUrl1 := "https://some.short.server.name/api/rest/v1/kittens?a=123&z=o&v=666"
actual1, _ := ConstructFilenameV3("GET", requestUrl1)

requestUrl2 := "https://some.short.server.name/api/rest/v1/kittens?z=o&v=666&a=123"
actual2, _ := ConstructFilenameV3("GET", requestUrl2)

require.Equal(t, actual1, actual2)
}

0 comments on commit 7219220

Please sign in to comment.