Skip to content

Commit

Permalink
Merge pull request #312 from s4s7/feature/refactor-newRequest-func
Browse files Browse the repository at this point in the history 
Refactor newRequest function to remove error return value
  • Loading branch information
@youyuanwu
youyuanwu committed Mar 8, 2024
2 parents 475f357 + 1cc63fa commit 424b9cb
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 77 deletions.
28 changes: 12 additions & 16 deletions client/auth_info_test.go
View file
Expand Up @@ -18,17 +18,17 @@ import (
"net/http"
"testing"

"github.com/go-openapi/runtime"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/go-openapi/runtime"
)

func TestBasicAuth(t *testing.T) {
r, err := newRequest(http.MethodGet, "/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/", nil)

writer := BasicAuth("someone", "with a password")
err = writer.AuthenticateRequest(r, nil)
err := writer.AuthenticateRequest(r, nil)
require.NoError(t, err)

req := new(http.Request)
Expand All @@ -41,44 +41,40 @@ func TestBasicAuth(t *testing.T) {
}

func TestAPIKeyAuth_Query(t *testing.T) {
r, err := newRequest(http.MethodGet, "/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/", nil)

writer := APIKeyAuth("api_key", "query", "the-shared-key")
err = writer.AuthenticateRequest(r, nil)
err := writer.AuthenticateRequest(r, nil)
require.NoError(t, err)

assert.Equal(t, "the-shared-key", r.query.Get("api_key"))
}

func TestAPIKeyAuth_Header(t *testing.T) {
r, err := newRequest(http.MethodGet, "/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/", nil)

writer := APIKeyAuth("x-api-token", "header", "the-shared-key")
err = writer.AuthenticateRequest(r, nil)
err := writer.AuthenticateRequest(r, nil)
require.NoError(t, err)

assert.Equal(t, "the-shared-key", r.header.Get("x-api-token"))
}

func TestBearerTokenAuth(t *testing.T) {
r, err := newRequest(http.MethodGet, "/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/", nil)

writer := BearerToken("the-shared-token")
err = writer.AuthenticateRequest(r, nil)
err := writer.AuthenticateRequest(r, nil)
require.NoError(t, err)

assert.Equal(t, "Bearer the-shared-token", r.header.Get(runtime.HeaderAuthorization))
}

func TestCompose(t *testing.T) {
r, err := newRequest(http.MethodGet, "/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/", nil)

writer := Compose(APIKeyAuth("x-api-key", "header", "the-api-key"), APIKeyAuth("x-secret-key", "header", "the-secret-key"))
err = writer.AuthenticateRequest(r, nil)
err := writer.AuthenticateRequest(r, nil)
require.NoError(t, err)

assert.Equal(t, "the-api-key", r.header.Get("x-api-key"))
Expand Down
4 changes: 2 additions & 2 deletions client/request.go
View file
Expand Up @@ -36,7 +36,7 @@ import (
)

// NewRequest creates a new swagger http client request
func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) (*request, error) {
func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter) *request {
return &request{
pathPattern: pathPattern,
method: method,
Expand All @@ -45,7 +45,7 @@ func newRequest(method, pathPattern string, writer runtime.ClientRequestWriter)
query: make(url.Values),
timeout: DefaultTimeout,
getBody: getRequestBuffer,
}, nil
}
}

// Request represents a swagger client request.
Expand Down
80 changes: 28 additions & 52 deletions client/request_test.go
View file
Expand Up @@ -30,10 +30,11 @@ import (
"strings"
"testing"

"github.com/go-openapi/runtime"
"github.com/go-openapi/strfmt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/go-openapi/runtime"
)

var testProducers = map[string]runtime.Producer{
Expand All @@ -43,8 +44,7 @@ var testProducers = map[string]runtime.Producer{
}

func TestBuildRequest_SetHeaders(t *testing.T) {
r, err := newRequest(http.MethodGet, "/flats/{id}/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", nil)

// single value
_ = r.SetHeaderParam("X-Rate-Limit", "500")
Expand All @@ -58,16 +58,14 @@ func TestBuildRequest_SetHeaders(t *testing.T) {
}

func TestBuildRequest_SetPath(t *testing.T) {
r, err := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)

_ = r.SetPathParam("id", "1345")
assert.Equal(t, "1345", r.pathParams["id"])
}

func TestBuildRequest_SetQuery(t *testing.T) {
r, err := newRequest(http.MethodGet, "/flats/{id}/", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", nil)

// single value
_ = r.SetQueryParam("hello", "there")
Expand All @@ -80,8 +78,7 @@ func TestBuildRequest_SetQuery(t *testing.T) {

func TestBuildRequest_SetForm(t *testing.T) {
// non-multipart
r, err := newRequest(http.MethodPost, "/flats", nil)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats", nil)
_ = r.SetFormParam("hello", "world")
assert.Equal(t, "world", r.formFields.Get("hello"))
_ = r.SetFormParam("goodbye", "cruel", "world")
Expand All @@ -90,11 +87,10 @@ func TestBuildRequest_SetForm(t *testing.T) {

func TestBuildRequest_SetFile(t *testing.T) {
// needs to convert form to multipart
r, err := newRequest(http.MethodPost, "/flats/{id}/image", nil)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/image", nil)

// error if it isn't there
err = r.SetFileParam("not there", os.NewFile(0, "./i-dont-exist"))
err := r.SetFileParam("not there", os.NewFile(0, "./i-dont-exist"))
require.Error(t, err)

// error if it isn't a file
Expand Down Expand Up @@ -125,8 +121,7 @@ func mustGetFile(path string) *os.File {
}

func TestBuildRequest_SetBody(t *testing.T) {
r, err := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/?hello=world", nil)

bd := []struct{ Name, Hobby string }{{"Tom", "Organ trail"}, {"John", "Bird watching"}}

Expand All @@ -142,8 +137,7 @@ func TestBuildRequest_BuildHTTP_NoPayload(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
require.NoError(t, err)
Expand All @@ -162,8 +156,7 @@ func TestBuildRequest_BuildHTTP_Payload(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)

req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
Expand Down Expand Up @@ -197,8 +190,7 @@ func TestBuildRequest_BuildHTTP_SetsInAuth(t *testing.T) {
return nil
})

r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)

req, err := r.buildHTTP(runtime.JSONMime, "", testProducers, nil, auth)
Expand Down Expand Up @@ -228,8 +220,7 @@ func TestBuildRequest_BuildHTTP_XMLPayload(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.XMLMime)

req, err := r.BuildHTTP(runtime.XMLMime, "", testProducers, nil)
Expand All @@ -256,8 +247,7 @@ func TestBuildRequest_BuildHTTP_TextPayload(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.TextMime)

req, err := r.BuildHTTP(runtime.TextMime, "", testProducers, nil)
Expand All @@ -281,8 +271,7 @@ func TestBuildRequest_BuildHTTP_Form(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)

req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
Expand All @@ -305,8 +294,7 @@ func TestBuildRequest_BuildHTTP_Form_URLEncoded(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.URLencodedFormMime)

req, err := r.BuildHTTP(runtime.URLencodedFormMime, "", testProducers, nil)
Expand All @@ -330,8 +318,7 @@ func TestBuildRequest_BuildHTTP_Form_Content_Length(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)

req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
Expand All @@ -356,8 +343,7 @@ func TestBuildRequest_BuildHTTP_FormMultipart(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)

req, err := r.BuildHTTP(runtime.MultipartFormMime, "", testProducers, nil)
Expand Down Expand Up @@ -390,8 +376,7 @@ func TestBuildRequest_BuildHTTP_FormMultiples(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.MultipartFormMime)

req, err := r.BuildHTTP(runtime.MultipartFormMime, "", testProducers, nil)
Expand Down Expand Up @@ -438,8 +423,7 @@ func TestBuildRequest_BuildHTTP_Files(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
require.NoError(t, err)
Expand Down Expand Up @@ -492,8 +476,7 @@ func TestBuildRequest_BuildHTTP_Files_URLEncoded(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.URLencodedFormMime)
req, err := r.BuildHTTP(runtime.URLencodedFormMime, "", testProducers, nil)
require.NoError(t, err)
Expand Down Expand Up @@ -554,8 +537,7 @@ func TestBuildRequest_BuildHTTP_File_ContentType(t *testing.T) {

return nil
})
r, err := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodGet, "/flats/{id}/", reqWrtr)
_ = r.SetHeaderParam(runtime.HeaderContentType, runtime.JSONMime)
req, err := r.BuildHTTP(runtime.JSONMime, "", testProducers, nil)
require.NoError(t, err)
Expand Down Expand Up @@ -594,8 +576,7 @@ func TestBuildRequest_BuildHTTP_BasePath(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
require.NoError(t, err)
Expand All @@ -613,8 +594,7 @@ func TestBuildRequest_BuildHTTP_EscapedPath(t *testing.T) {
_ = req.SetHeaderParam("X-Rate-Limit", "200")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
require.NoError(t, err)
Expand All @@ -634,8 +614,7 @@ func TestBuildRequest_BuildHTTP_BasePathWithQueryParameters(t *testing.T) {
_ = req.SetPathParam("id", "1234")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?foo=bar", testProducers, nil)
require.NoError(t, err)
Expand All @@ -653,8 +632,7 @@ func TestBuildRequest_BuildHTTP_PathPatternWithQueryParameters(t *testing.T) {
_ = req.SetPathParam("id", "1234")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/?foo=bar", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/?foo=bar", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath", testProducers, nil)
require.NoError(t, err)
Expand All @@ -671,8 +649,7 @@ func TestBuildRequest_BuildHTTP_StaticParametersPathPatternPrevails(t *testing.T
_ = req.SetPathParam("id", "1234")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?hello=kitty", testProducers, nil)
require.NoError(t, err)
Expand All @@ -689,8 +666,7 @@ func TestBuildRequest_BuildHTTP_StaticParametersConflictClientPrevails(t *testin
_ = req.SetPathParam("id", "1234")
return nil
})
r, err := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)
require.NoError(t, err)
r := newRequest(http.MethodPost, "/flats/{id}/?hello=world", reqWrtr)

req, err := r.BuildHTTP(runtime.JSONMime, "/basepath?hello=kitty", testProducers, nil)
require.NoError(t, err)
Expand Down
12 changes: 5 additions & 7 deletions client/runtime.go
View file
Expand Up @@ -32,12 +32,13 @@ import (
"sync"
"time"

"github.com/go-openapi/strfmt"
"github.com/opentracing/opentracing-go"

"github.com/go-openapi/runtime"
"github.com/go-openapi/runtime/logger"
"github.com/go-openapi/runtime/middleware"
"github.com/go-openapi/runtime/yamlpc"
"github.com/go-openapi/strfmt"
"github.com/opentracing/opentracing-go"
)

const (
Expand Down Expand Up @@ -379,14 +380,11 @@ func (r *Runtime) EnableConnectionReuse() {
func (r *Runtime) createHttpRequest(operation *runtime.ClientOperation) (*request, *http.Request, error) { //nolint:revive,stylecheck
params, _, auth := operation.Params, operation.Reader, operation.AuthInfo

request, err := newRequest(operation.Method, operation.PathPattern, params)
if err != nil {
return nil, nil, err
}
request := newRequest(operation.Method, operation.PathPattern, params)

var accept []string
accept = append(accept, operation.ProducesMediaTypes...)
if err = request.SetHeaderParam(runtime.HeaderAccept, accept...); err != nil {
if err := request.SetHeaderParam(runtime.HeaderAccept, accept...); err != nil {
return nil, nil, err
}

Expand Down

0 comments on commit 424b9cb

Please sign in to comment.