diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml deleted file mode 100644 index c2cc666..0000000 --- a/.github/workflows/codeql-analysis.yml +++ /dev/null @@ -1,71 +0,0 @@ -# For most projects, this workflow file will not need changing; you simply need -# to commit it to your repository. -# -# You may wish to alter this file to override the set of languages analyzed, -# or to provide custom queries or build logic. -# -# ******** NOTE ******** -# We have attempted to detect the languages in your repository. Please check -# the `language` matrix defined below to confirm you have the correct set of -# supported CodeQL languages. -# -name: "CodeQL" - -on: - push: - branches: [ main ] - pull_request: - # The branches below must be a subset of the branches above - branches: [ main ] - schedule: - - cron: '24 19 * * 2' - -jobs: - analyze: - name: Analyze - runs-on: ubuntu-latest - permissions: - actions: read - contents: read - security-events: write - - strategy: - fail-fast: false - matrix: - language: [ 'go' ] - # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python' ] - # Learn more: - # https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#changing-the-languages-that-are-analyzed - - steps: - - name: Checkout repository - uses: actions/checkout@v2 - - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v1 - with: - languages: ${{ matrix.language }} - # If you wish to specify custom queries, you can do so here or in a config file. - # By default, queries listed here will override any specified in a config file. - # Prefix the list here with "+" to use these queries and those in the config file. - # queries: ./path/to/local/query, your-org/your-repo/queries@main - - # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). - # If this step fails, then you should remove it and run the build manually (see below) - - name: Autobuild - uses: github/codeql-action/autobuild@v1 - - # ℹī¸ Command-line programs to run using the OS shell. - # 📚 https://git.io/JvXDl - - # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines - # and modify them (or add more) to build your code if your project - # uses a compiled language - - #- run: | - # make bootstrap - # make release - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index ac868e1..60c189e 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -20,7 +20,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: 1.17 + go-version: '1.18' - name: Build run: go build -v ./... diff --git a/go.mod b/go.mod index 59a1746..d06aae2 100644 --- a/go.mod +++ b/go.mod @@ -1,16 +1,17 @@ module github.com/StephanHCB/go-autumn-restclient -go 1.17 +go 1.18 require ( github.com/StephanHCB/go-autumn-logging v0.3.0 github.com/go-http-utils/headers v0.0.0-20181008091004-fed159eddc2a - github.com/stretchr/testify v1.8.1 - github.com/tidwall/tinylru v1.1.0 + github.com/stretchr/testify v1.8.4 + github.com/tidwall/tinylru v1.2.1 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index a069d30..317f400 100644 --- a/go.sum +++ b/go.sum @@ -9,13 +9,19 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/tidwall/tinylru v1.1.0 h1:XY6IUfzVTU9rpwdhKUF6nQdChgCdGjkMfLzbWyiau6I= github.com/tidwall/tinylru v1.1.0/go.mod h1:3+bX+TJ2baOLMWTnlyNWHh4QMnFyARg2TLTQ6OFbzw8= +github.com/tidwall/tinylru v1.2.1 h1:VgBr72c2IEr+V+pCdkPZUwiQ0KJknnWIYbhxAVkYfQk= +github.com/tidwall/tinylru v1.2.1/go.mod h1:9bQnEduwB6inr2Y7AkBP7JPgCkyrhTV/ZpX0oOOpBI4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/implementation/playback/playback.go b/implementation/playback/playback.go index ca85d70..8d9d364 100644 --- a/implementation/playback/playback.go +++ b/implementation/playback/playback.go @@ -14,84 +14,98 @@ import ( const PlaybackRewritePathEnvVariable = "GO_AUTUMN_RESTCLIENT_PLAYBACK_REWRITE_PATH" type PlaybackImpl struct { - RecorderPath string - RecorderRewritePath string - // Now is exposed so tests can fixate the time by overwriting this field - Now func() time.Time + RecorderPath string + RecorderRewritePath string + ConstructFilenameCandidates []aurestrecorder.ConstructFilenameFunction + Now func() time.Time +} + +type PlaybackOptions struct { + // ConstructFilenameCandidates contains filename constructor functions. + // + // The first one is considered "canonical", for all others, a log entry is printed that instructs the user + // to rename the file. + ConstructFilenameCandidates []aurestrecorder.ConstructFilenameFunction + NowFunc func() time.Time } // New builds a new http client simulator based on playback. // // Use this in your tests. -func New(recorderPath string) aurestclientapi.Client { +// +// You can optionally add a PlaybackOptions instance to your call. The ... is really just so it's an optional argument. +func New(recorderPath string, additionalOptions ...PlaybackOptions) aurestclientapi.Client { + filenameCandidates := []aurestrecorder.ConstructFilenameFunction{ + aurestrecorder.ConstructFilenameV3WithBody, + aurestrecorder.ConstructFilenameWithBody, + aurestrecorder.ConstructFilenameV2WithBody, + } + nowFunc := time.Now + for _, o := range additionalOptions { + if len(o.ConstructFilenameCandidates) > 0 { + filenameCandidates = o.ConstructFilenameCandidates + } + if o.NowFunc != nil { + nowFunc = o.NowFunc + } + } return &PlaybackImpl{ - RecorderPath: recorderPath, - RecorderRewritePath: os.Getenv(PlaybackRewritePathEnvVariable), - Now: time.Now, + RecorderPath: recorderPath, + RecorderRewritePath: os.Getenv(PlaybackRewritePathEnvVariable), + ConstructFilenameCandidates: filenameCandidates, + Now: nowFunc, } } -func (c *PlaybackImpl) Perform(ctx context.Context, method string, requestUrl string, _ interface{}, response *aurestclientapi.ParsedResponse) error { - filename, err := aurestrecorder.ConstructFilenameV3(method, requestUrl) - if err != nil { - return err - } +func (c *PlaybackImpl) Perform(ctx context.Context, method string, requestUrl string, requestBody interface{}, response *aurestclientapi.ParsedResponse) error { + canonicalFilename := "" + var originalError error + for i, constructFilenameCandidate := range c.ConstructFilenameCandidates { + filename, err := constructFilenameCandidate(method, requestUrl, requestBody) + if err != nil { + return err + } + if i == 0 { + canonicalFilename = filename + } - jsonBytes, err := os.ReadFile(filepath.Join(c.RecorderPath, filename)) - if err != nil { - // try old filename for compatibility (cannot fail if ConstructFilenameV2 didn't) - filenameOldV1, _ := aurestrecorder.ConstructFilename(method, requestUrl) + jsonBytes, err := os.ReadFile(filepath.Join(c.RecorderPath, filename)) + if err != nil { + if i == 0 { + originalError = err + } + } else { + // successfully read + if i > 0 { + aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename '%s', please move to '%s'", filename, canonicalFilename) + } - jsonBytesOldV1, errWithOldFilenameV1 := os.ReadFile(filepath.Join(c.RecorderPath, filenameOldV1)) - if errWithOldFilenameV1 != nil { - // try old filename for compatibility (cannot fail if ConstructFilenameV2 didn't) - filenameOldV2, _ := aurestrecorder.ConstructFilenameV2(method, requestUrl) + _ = c.rewriteFileIfConfigured(ctx, filename, canonicalFilename) - jsonBytesOldV2, errWithOldFilenameV2 := os.ReadFile(filepath.Join(c.RecorderPath, filenameOldV2)) - if errWithOldFilenameV2 != nil { - // but return original error if that also fails + recording := aurestrecorder.RecorderData{} + err = json.Unmarshal(jsonBytes, &recording) + if err != nil { return err - } else { - aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename (v2) '%s', please move to '%s'", filenameOldV2, filename) + } - _ = c.rewriteFileIfConfigured(ctx, filenameOldV2, filename) + response.Header = recording.ParsedResponse.Header + response.Status = recording.ParsedResponse.Status + response.Time = c.Now() - filename = filenameOldV2 - jsonBytes = jsonBytesOldV2 + // cannot just assign the body, need to re-parse into the existing pointer - using a json round trip + bodyJsonBytes, err := json.Marshal(recording.ParsedResponse.Body) + if err != nil { + return err + } + err = json.Unmarshal(bodyJsonBytes, response.Body) + if err != nil { + return err } - } else { - aulogging.Logger.Ctx(ctx).Info().Printf("use of deprecated recorder filename (v1) '%s', please move to '%s'", filenameOldV1, filename) - - _ = c.rewriteFileIfConfigured(ctx, filenameOldV1, filename) - filename = filenameOldV1 - jsonBytes = jsonBytesOldV1 + return recording.Error } - } else { - _ = c.rewriteFileIfConfigured(ctx, filename, filename) } - - recording := aurestrecorder.RecorderData{} - err = json.Unmarshal(jsonBytes, &recording) - if err != nil { - return err - } - - response.Header = recording.ParsedResponse.Header - response.Status = recording.ParsedResponse.Status - response.Time = c.Now() - - // cannot just assign the body, need to re-parse into the existing pointer - using a json round trip - bodyJsonBytes, err := json.Marshal(recording.ParsedResponse.Body) - if err != nil { - return err - } - err = json.Unmarshal(bodyJsonBytes, response.Body) - if err != nil { - return err - } - - return recording.Error + return originalError } func (c *PlaybackImpl) rewriteFileIfConfigured(ctx context.Context, fileNameFrom string, fileNameTo string) error { diff --git a/implementation/recorder/recorder.go b/implementation/recorder/recorder.go index f4fd698..fe76bd3 100644 --- a/implementation/recorder/recorder.go +++ b/implementation/recorder/recorder.go @@ -12,13 +12,20 @@ import ( "strings" ) +type ConstructFilenameFunction func(method string, requestUrl string, requestBody interface{}) (string, error) + type RecorderImpl struct { - Wrapped aurestclientapi.Client - RecorderPath string + Wrapped aurestclientapi.Client + RecorderPath string + ConstructFilenameFunc ConstructFilenameFunction } const RecorderPathEnvVariable = "GO_AUTUMN_RESTCLIENT_RECORDER_PATH" +type RecorderOptions struct { + ConstructFilenameFunc ConstructFilenameFunction +} + // New builds a new http recorder. // // Insert this into your stack just above the actual http client. @@ -26,16 +33,25 @@ const RecorderPathEnvVariable = "GO_AUTUMN_RESTCLIENT_RECORDER_PATH" // Normally it does nothing, but if you set the environment variable RecorderPathEnvVariable to a path to a directory, // it will write response recordings for your requests that you can then play back using aurestplayback.PlaybackImpl // in your tests. -func New(wrapped aurestclientapi.Client) aurestclientapi.Client { +// +// You can optionally add a RecorderOptions instance to your call. The ... is really just so it's an optional argument. +func New(wrapped aurestclientapi.Client, additionalOptions ...RecorderOptions) aurestclientapi.Client { recorderPath := os.Getenv(RecorderPathEnvVariable) if recorderPath != "" { if !strings.HasSuffix(recorderPath, "/") { recorderPath += "/" } } + filenameFunc := ConstructFilenameV3WithBody + for _, o := range additionalOptions { + if o.ConstructFilenameFunc != nil { + filenameFunc = o.ConstructFilenameFunc + } + } return &RecorderImpl{ - Wrapped: wrapped, - RecorderPath: recorderPath, + Wrapped: wrapped, + RecorderPath: recorderPath, + ConstructFilenameFunc: filenameFunc, } } @@ -50,7 +66,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 := ConstructFilenameV3(method, requestUrl) + filename, err := c.ConstructFilenameFunc(method, requestUrl, requestBody) if err == nil { recording := RecorderData{ Method: method, @@ -88,6 +104,10 @@ func ConstructFilename(method string, requestUrl string) (string, error) { return filename, nil } +func ConstructFilenameWithBody(method string, requestUrl string, _ interface{}) (string, error) { + return ConstructFilename(method, requestUrl) +} + func ConstructFilenameV2(method string, requestUrl string) (string, error) { parsedUrl, err := url.Parse(requestUrl) if err != nil { @@ -112,6 +132,10 @@ func ConstructFilenameV2(method string, requestUrl string) (string, error) { return filename, nil } +func ConstructFilenameV2WithBody(method string, requestUrl string, _ interface{}) (string, error) { + return ConstructFilenameV2(method, requestUrl) +} + func ConstructFilenameV3(method string, requestUrl string) (string, error) { parsedUrl, err := url.Parse(requestUrl) if err != nil { @@ -135,3 +159,7 @@ func ConstructFilenameV3(method string, requestUrl string) (string, error) { filename := fmt.Sprintf("request_%s_%s_%s.json", m, p, q) return filename, nil } + +func ConstructFilenameV3WithBody(method string, requestUrl string, _ interface{}) (string, error) { + return ConstructFilenameV3(method, requestUrl) +} diff --git a/implementation/recorder/recorder_test.go b/implementation/recorder/recorder_test.go index b79e943..ed7637d 100644 --- a/implementation/recorder/recorder_test.go +++ b/implementation/recorder/recorder_test.go @@ -7,7 +7,7 @@ import ( func TestConstructFilenameLong(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 := ConstructFilename("GET", requestUrl) + actual, err := ConstructFilenameWithBody("GET", requestUrl, nil) expected := "request_get_%2Fapi%2Frest%2Fv1%2Fv2%2Fv3%2Fv4%2Fthis%2Fis%2Fintentionally%2Fvery%2Fvery%2Fvery%2Fvery%2Flong%2Fdjkfjhdalsfhdsahjflkd_fb2e3656d88910ffc49023f99f5e0df6.json" require.Nil(t, err) require.Equal(t, expected, actual) @@ -15,7 +15,7 @@ func TestConstructFilenameLong(t *testing.T) { func TestConstructFilenameShort(t *testing.T) { requestUrl := "https://some.short.server.name/api/rest/v1/kittens" - actual, err := ConstructFilename("GET", requestUrl) + actual, err := ConstructFilenameWithBody("GET", requestUrl, nil) expected := "request_get_%2Fapi%2Frest%2Fv1%2Fkittens_d41d8cd98f00b204e9800998ecf8427e.json" require.Nil(t, err) require.Equal(t, expected, actual) @@ -23,7 +23,7 @@ func TestConstructFilenameShort(t *testing.T) { func TestConstructFilenameV2Long(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 := ConstructFilenameV2("GET", requestUrl) + actual, err := ConstructFilenameV2WithBody("GET", requestUrl, nil) 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) @@ -31,7 +31,7 @@ func TestConstructFilenameV2Long(t *testing.T) { func TestConstructFilenameV2Short(t *testing.T) { requestUrl := "https://some.short.server.name/api/rest/v1/kittens" - actual, err := ConstructFilenameV2("GET", requestUrl) + actual, err := ConstructFilenameV2WithBody("GET", requestUrl, nil) expected := "request_get_api-rest-v1-kittens_d41d8cd9.json" require.Nil(t, err) require.Equal(t, expected, actual) @@ -39,7 +39,7 @@ func TestConstructFilenameV2Short(t *testing.T) { 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) + actual, err := ConstructFilenameV3WithBody("GET", requestUrl, nil) 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) @@ -47,7 +47,7 @@ func TestConstructFilenameV3Long(t *testing.T) { func TestConstructFilenameV3Short(t *testing.T) { requestUrl := "https://some.short.server.name/api/rest/v1/kittens" - actual, err := ConstructFilenameV3("GET", requestUrl) + actual, err := ConstructFilenameV3WithBody("GET", requestUrl, nil) expected := "request_get_api-rest-v1-kittens_d41d8cd9.json" require.Nil(t, err) require.Equal(t, expected, actual) @@ -55,10 +55,10 @@ func TestConstructFilenameV3Short(t *testing.T) { 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) + actual1, _ := ConstructFilenameV3WithBody("GET", requestUrl1, nil) requestUrl2 := "https://some.short.server.name/api/rest/v1/kittens?z=o&v=666&a=123" - actual2, _ := ConstructFilenameV3("GET", requestUrl2) + actual2, _ := ConstructFilenameV3WithBody("GET", requestUrl2, nil) require.Equal(t, actual1, actual2) }