Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore order of query params of a request for recordings and playbacks #12

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 19 additions & 9 deletions implementation/playback/playback.go
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
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
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)
}