Merge pull request #12 from mpl-interhyp/ignore_order_of_query_params…

…_for_recording_name

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

implementation/playback/playback.go

@@ -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
 		}
 	}
 

implementation/recorder/recorder.go

@@ -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,
@@ -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
+}

implementation/recorder/recorder_test.go

@@ -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)
+}