diff --git a/cluster_agents.go b/cluster_agents.go index 36fba144d120e0e223f03f37f8f2d6eacf09e61d..20ba1f15804e8e54775e2bf8e350c2177fb27fd1 100644 --- a/cluster_agents.go +++ b/cluster_agents.go @@ -140,20 +140,12 @@ func (s *ClusterAgentsService) ListAgents(pid any, opt *ListAgentsOptions, optio if err != nil { return nil, nil, err } - uri := fmt.Sprintf("projects/%s/cluster_agents", PathEscape(project)) - req, err := s.client.NewRequest(http.MethodGet, uri, opt, options) - if err != nil { - return nil, nil, err - } - - var as []*Agent - resp, err := s.client.Do(req, &as) - if err != nil { - return nil, resp, err - } - - return as, resp, nil + return do[[]*Agent](s.client, + withPath("projects/%s/cluster_agents", project), + withAPIOpts(opt), + withRequestOpts(options...), + ) } func (s *ClusterAgentsService) GetAgent(pid any, id int, options ...RequestOptionFunc) (*Agent, *Response, error) { @@ -161,20 +153,11 @@ func (s *ClusterAgentsService) GetAgent(pid any, id int, options ...RequestOptio if err != nil { return nil, nil, err } - uri := fmt.Sprintf("projects/%s/cluster_agents/%d", PathEscape(project), id) - req, err := s.client.NewRequest(http.MethodGet, uri, nil, options) - if err != nil { - return nil, nil, err - } - - a := new(Agent) - resp, err := s.client.Do(req, a) - if err != nil { - return nil, resp, err - } - - return a, resp, nil + return do[*Agent](s.client, + withPath("projects/%s/cluster_agents/%d", project, id), + withRequestOpts(options...), + ) } // RegisterAgentOptions represents the available RegisterAgent() @@ -191,20 +174,13 @@ func (s *ClusterAgentsService) RegisterAgent(pid any, opt *RegisterAgentOptions, if err != nil { return nil, nil, err } - uri := fmt.Sprintf("projects/%s/cluster_agents", PathEscape(project)) - - req, err := s.client.NewRequest(http.MethodPost, uri, opt, options) - if err != nil { - return nil, nil, err - } - - a := new(Agent) - resp, err := s.client.Do(req, a) - if err != nil { - return nil, resp, err - } - return a, resp, nil + return do[*Agent](s.client, + withMethod(http.MethodPost), + withPath("projects/%s/cluster_agents", project), + withAPIOpts(opt), + withRequestOpts(options...), + ) } func (s *ClusterAgentsService) DeleteAgent(pid any, id int, options ...RequestOptionFunc) (*Response, error) { @@ -212,14 +188,13 @@ func (s *ClusterAgentsService) DeleteAgent(pid any, id int, options ...RequestOp if err != nil { return nil, err } - uri := fmt.Sprintf("projects/%s/cluster_agents/%d", PathEscape(project), id) - - req, err := s.client.NewRequest(http.MethodDelete, uri, nil, options) - if err != nil { - return nil, err - } - return s.client.Do(req, nil) + _, resp, err := do[none](s.client, + withMethod(http.MethodDelete), + withPath("projects/%s/cluster_agents/%d", project, id), + withRequestOpts(options...), + ) + return resp, err } // ListAgentTokensOptions represents the available ListAgentTokens() options. diff --git a/request_handler.go b/request_handler.go index 3c80321fb93b1621af8b4845b1149b9d0c206da3..6ead6434ccba881fe1fc0df7e6d0666183219654 100644 --- a/request_handler.go +++ b/request_handler.go @@ -1,66 +1,130 @@ package gitlab -// Internal utility functions for handling common request/response patterns +import ( + "fmt" + "net/http" + "reflect" +) -// doRequest handles requests that return a single object. -// This is a generic utility function for GitLab API endpoints that return a single resource. -// -// GitLab API docs: https://docs.gitlab.com/ee/api/ -func doRequest[T any]( - client *Client, - method, url string, - body any, - options ...RequestOptionFunc, -) (T, *Response, error) { - var result T - req, err := client.NewRequest(method, url, body, options) - if err != nil { - var zero T - return zero, nil, err +type doConfig struct { + method string + path string + apiOpts any + requestOpts []RequestOptionFunc +} + +type doOption func(c *doConfig) + +func withMethod(method string) doOption { + return func(c *doConfig) { + c.method = method } - resp, err := client.Do(req, &result) - if err != nil { - var zero T - return zero, resp, err +} + +func withPath(path string, args ...any) doOption { + return func(c *doConfig) { + as := make([]any, len(args)) + for i, a := range args { + switch v := a.(type) { + case string: + as[i] = PathEscape(v) + default: + as[i] = v + } + } + c.path = fmt.Sprintf(path, as...) } - return result, resp, nil } -// doRequestSlice handles requests that return a slice of objects. -// This is a generic utility function for GitLab API endpoints that return a slice of objects. -// -// GitLab API docs: https://docs.gitlab.com/ee/api/ -func doRequestSlice[T any]( - client *Client, - method, url string, - body any, - options ...RequestOptionFunc, -) ([]T, *Response, error) { - var result []T - req, err := client.NewRequest(method, url, body, options) - if err != nil { - return nil, nil, err +func withAPIOpts(o any) doOption { + return func(c *doConfig) { + c.apiOpts = o } - resp, err := client.Do(req, &result) - if err != nil { - return nil, resp, err +} + +func withRequestOpts(o ...RequestOptionFunc) doOption { + return func(c *doConfig) { + c.requestOpts = o } - return result, resp, nil } -// doRequestVoid handles requests that don't return data. -// This is a generic utility function for GitLab API endpoints that perform actions without returning response data. +// none is a sentinel type to signal that a request performed with do does not return a value. +type none struct{} + +// do constructs an API requests, performs it and processes the response. +// +// Use the opts to configure the request. +// If the response body shouldn't be handled, use the none sentinel type +// and ignore the first return argument. +// +// Example: +// +// // Get Request to return single *Agent: +// return do[*Agent](s.client, +// +// withPath("projects/%s/cluster_agents/%d", project, id), +// withRequestOpts(options...), +// +// ) // -// GitLab API docs: https://docs.gitlab.com/ee/api/ -func doRequestVoid( - client *Client, - method, url string, - body any, - options ...RequestOptionFunc, -) (*Response, error) { - req, err := client.NewRequest(method, url, body, options) +// // Get Request to return multiple []*Agents +// return do[[]*Agent](s.client, +// +// withPath("projects/%s/cluster_agents", project), +// withRequestOpts(options...), +// +// ) +// +// // Post Request to create Agent and return *Agents +// return do[*Agent](s.client, +// +// withMethod(http.MethodPost), +// withPath("projects/%s/cluster_agents", project), +// withAPIOpts(opt), +// withRequestOpts(options...), +// +// ) +// +// // Delete Request that returns nothing: +// _, resp, err := do[none](s.client, +// +// withMethod(http.MethodDelete), +// withPath("projects/%s/cluster_agents/%d", project, id), +// withRequestOpts(options...), +// +// ) +func do[T any](client *Client, opts ...doOption) (T, *Response, error) { + // default config + config := &doConfig{ + method: http.MethodGet, + apiOpts: nil, + } + + // apply options to config + for _, f := range opts { + f(config) + } + + req, err := client.NewRequest(config.method, config.path, config.apiOpts, config.requestOpts) if err != nil { - return nil, err + var z T + return z, nil, err + } + + var ( + as T + resp *Response + ) + if reflect.TypeOf(as) == reflect.TypeOf(none{}) { + resp, err = client.Do(req, nil) + } else { + resp, err = client.Do(req, &as) } - return client.Do(req, nil) + + if err != nil { + var z T + return z, resp, err + } + + return as, resp, nil } diff --git a/request_handler_test.go b/request_handler_test.go index 66f7eae303773251ac33939516f994c44bd8d99f..ff1b8432f66f2109cdd7ffab74183f6ddc9abc84 100644 --- a/request_handler_test.go +++ b/request_handler_test.go @@ -9,13 +9,12 @@ import ( "github.com/stretchr/testify/require" ) -// Test structs for testing -type TestUser struct { +type testUser struct { ID int64 `json:"id"` Name string `json:"name"` } -type TestProject struct { +type testProject struct { ID int64 `json:"id"` Name string `json:"name"` Description string `json:"description"` @@ -25,22 +24,20 @@ func TestDoRequestSuccess(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns a test user from testdata + // GIVEN path := "/api/v4/users/1" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) mustWriteHTTPResponse(t, w, "testdata/get_user.json") }) - // WHEN doRequest is called with valid parameters - user, resp, err := doRequest[*TestUser]( + // WHEN + user, resp, err := do[*testUser]( client, - http.MethodGet, - "users/1", - nil, + withPath("users/1"), ) - // THEN the request should succeed and return the expected user + // THEN assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) assert.Equal(t, int64(1), user.ID) @@ -51,12 +48,12 @@ func TestDoRequestPOSTWithBody(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that creates and returns a project + // GIVEN path := "/api/v4/projects" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodPost) - var reqBody TestProject + var reqBody testProject err := json.NewDecoder(r.Body).Decode(&reqBody) require.NoError(t, err) require.Equal(t, "New Project", reqBody.Name) @@ -66,21 +63,20 @@ func TestDoRequestPOSTWithBody(t *testing.T) { w.Write([]byte(`{"id": 1, "name": "New Project", "description": "Test project"}`)) }) - // GIVEN a project creation request body - requestBody := &TestProject{ + requestBody := &testProject{ Name: "New Project", Description: "Test project", } - // WHEN doRequest is called with POST method and body - project, resp, err := doRequest[*TestProject]( + // WHEN + project, resp, err := do[*testProject]( client, - http.MethodPost, - "projects", - requestBody, + withMethod(http.MethodPost), + withPath("projects"), + withAPIOpts(requestBody), ) - // THEN the project should be created successfully + // THEN assert.NoError(t, err) assert.Equal(t, 201, resp.StatusCode) assert.Equal(t, "New Project", project.Name) @@ -90,7 +86,7 @@ func TestDoRequestErrorResponse(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns a 404 error + // GIVEN path := "/api/v4/users/999" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) @@ -98,16 +94,13 @@ func TestDoRequestErrorResponse(t *testing.T) { w.Write([]byte(`{"message": "Not found"}`)) }) - // WHEN doRequest is called for a non-existent user - user, resp, err := doRequest[*TestUser]( + // WHEN + user, resp, err := do[*testUser]( client, - http.MethodGet, - "users/999", - nil, - nil, + withPath("users/99"), ) - // THEN the request should fail with a 404 error + // THEN assert.Error(t, err) assert.Equal(t, 404, resp.StatusCode) assert.Nil(t, user) @@ -117,28 +110,25 @@ func TestDoRequestSliceSuccess(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns a list of users from testdata + // GIVEN path := "/api/v4/users" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) mustWriteHTTPResponse(t, w, "testdata/list_users.json") }) - // WHEN doRequestSlice is called to fetch users - users, resp, err := doRequestSlice[TestUser]( + // WHEN + users, resp, err := do[[]testUser]( client, - http.MethodGet, - "users", - nil, - nil, + withPath("users"), ) - // THEN the request should succeed and return all users + // THEN assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) assert.Len(t, users, 3) - expectedUsers := []TestUser{ + expectedUsers := []testUser{ {ID: 1, Name: "Example User 1"}, {ID: 2, Name: "Example User 2"}, {ID: 3, Name: "Example User 3"}, @@ -153,7 +143,7 @@ func TestDoRequestSliceEmptySlice(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns an empty array + // GIVEN path := "/api/v4/users" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) @@ -161,16 +151,13 @@ func TestDoRequestSliceEmptySlice(t *testing.T) { w.Write([]byte("[]")) }) - // WHEN doRequestSlice is called on an endpoint with no data - users, resp, err := doRequestSlice[TestUser]( + // WHEN + users, resp, err := do[[]testUser]( client, - http.MethodGet, - "users", - nil, - nil, + withPath("users"), ) - // THEN the request should succeed and return an empty slice + // THEN assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) assert.Empty(t, users) @@ -180,7 +167,7 @@ func TestDoRequestSliceErrorResponse(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns a 500 error + // GIVEN path := "/api/v4/users" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) @@ -188,16 +175,13 @@ func TestDoRequestSliceErrorResponse(t *testing.T) { w.Write([]byte(`{"message": "Internal server error"}`)) }) - // WHEN doRequestSlice is called on a failing endpoint - users, resp, err := doRequestSlice[TestUser]( + // WHEN + users, resp, err := do[[]testUser]( client, - http.MethodGet, - "users", - nil, - nil, + withPath("users"), ) - // THEN the request should fail with a 500 error + // THEN assert.Error(t, err) assert.Equal(t, 500, resp.StatusCode) assert.Nil(t, users) @@ -207,23 +191,21 @@ func TestDoRequestVoidSuccessDELETE(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that accepts DELETE requests + // GIVEN path := "/api/v4/users/1" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodDelete) w.WriteHeader(204) // No Content }) - // WHEN doRequestVoid is called with DELETE method - resp, err := doRequestVoid( + // WHEN + _, resp, err := do[none]( client, - http.MethodDelete, - "users/1", - nil, - nil, + withMethod(http.MethodDelete), + withPath("users/1"), ) - // THEN the request should succeed with no content + // THEN assert.NoError(t, err) assert.Equal(t, 204, resp.StatusCode) } @@ -232,7 +214,7 @@ func TestDoRequestVoidSuccessPUT(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that accepts PUT requests with body validation + // GIVEN path := "/api/v4/merge_requests/1/approve" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodPut) @@ -245,19 +227,17 @@ func TestDoRequestVoidSuccessPUT(t *testing.T) { w.WriteHeader(200) }) - // GIVEN an approval request body requestBody := map[string]string{"action": "approve"} - // WHEN doRequestVoid is called with PUT method and body - resp, err := doRequestVoid( + // WHEN + _, resp, err := do[none]( client, - http.MethodPut, - "merge_requests/1/approve", - requestBody, - nil, + withMethod(http.MethodPut), + withPath("merge_requests/1/approve"), + withAPIOpts(requestBody), ) - // THEN the request should succeed + // THEN assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) } @@ -266,7 +246,7 @@ func TestDoRequestVoidErrorResponse(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that returns a 403 forbidden error + // GIVEN path := "/api/v4/users/1" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodDelete) @@ -274,16 +254,14 @@ func TestDoRequestVoidErrorResponse(t *testing.T) { w.Write([]byte(`{"message": "Forbidden"}`)) }) - // WHEN doRequestVoid is called on a forbidden operation - resp, err := doRequestVoid( + // WHEN + _, resp, err := do[none]( client, - http.MethodDelete, - "users/1", - nil, - nil, + withMethod(http.MethodDelete), + withPath("users/1"), ) - // THEN the request should fail with a 403 error + // THEN assert.Error(t, err) assert.Equal(t, 403, resp.StatusCode) } @@ -292,8 +270,7 @@ func TestRequestHandlerWithOptions(t *testing.T) { t.Parallel() mux, client := setup(t) - // GIVEN a mock server that validates query parameters and headers - // from options + // GIVEN path := "/api/v4/users" mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, http.MethodGet) @@ -304,22 +281,19 @@ func TestRequestHandlerWithOptions(t *testing.T) { mustWriteHTTPResponse(t, w, "testdata/list_users_public_email.json") }) - // GIVEN request options with pagination and custom header options := []RequestOptionFunc{ WithOffsetPaginationParameters(2), WithHeader("X-Test-Header", "test-value"), } - // WHEN doRequestSlice is called with request options - users, resp, err := doRequestSlice[TestUser]( + // WHEN + users, resp, err := do[[]testUser]( client, - http.MethodGet, - "users", - nil, - options..., + withPath("users"), + withRequestOpts(options...), ) - // THEN the request should succeed with options applied + // THEN assert.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) assert.Len(t, users, 1)