From 068517694113264c03368c608f84cecf8d6ecd15 Mon Sep 17 00:00:00 2001 From: favonia Date: Wed, 25 May 2022 20:03:58 -0500 Subject: [PATCH 1/2] Refactor the URI path building code --- access_application.go | 11 ++--------- access_bookmark.go | 11 ++--------- access_group.go | 20 ++++++++------------ access_policy.go | 22 +++++++++------------- account_members.go | 13 +++---------- accounts.go | 11 +---------- filter.go | 20 +++++++------------- firewall_rules.go | 11 ++--------- images.go | 11 ++--------- notifications.go | 13 ++----------- pages_project.go | 11 ++--------- rate_limiting.go | 11 ++--------- teams_list.go | 14 +++++--------- tunnel.go | 11 ++--------- tunnel_routes.go | 12 ++---------- utils.go | 13 +++++++++++++ zones.go | 10 +--------- 17 files changed, 65 insertions(+), 160 deletions(-) create mode 100644 utils.go diff --git a/access_application.go b/access_application.go index bd872e351..0a64814e8 100644 --- a/access_application.go +++ b/access_application.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -96,15 +95,9 @@ func (api *API) ZoneLevelAccessApplications(ctx context.Context, zoneID string, } func (api *API) accessApplications(ctx context.Context, id string, pageOpts PaginationOptions, routeRoot RouteRoot) ([]AccessApplication, ResultInfo, error) { - uri := fmt.Sprintf("/%s/%s/access/apps", routeRoot, id) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/%s/%s/access/apps", routeRoot, id), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []AccessApplication{}, ResultInfo{}, err } diff --git a/access_bookmark.go b/access_bookmark.go index 5eb4d5f01..8ebf4603c 100644 --- a/access_bookmark.go +++ b/access_bookmark.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -52,15 +51,9 @@ func (api *API) ZoneLevelAccessBookmarks(ctx context.Context, zoneID string, pag } func (api *API) accessBookmarks(ctx context.Context, id string, pageOpts PaginationOptions, routeRoot RouteRoot) ([]AccessBookmark, ResultInfo, error) { - uri := fmt.Sprintf("/%s/%s/access/bookmarks", routeRoot, id) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/%s/%s/access/bookmarks", routeRoot, id), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []AccessBookmark{}, ResultInfo{}, err } diff --git a/access_group.go b/access_group.go index 795349c00..4c45e21d9 100644 --- a/access_group.go +++ b/access_group.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -214,19 +213,16 @@ func (api *API) ZoneLevelAccessGroups(ctx context.Context, zoneID string, pageOp } func (api *API) accessGroups(ctx context.Context, id string, pageOpts PaginationOptions, routeRoot RouteRoot) ([]AccessGroup, ResultInfo, error) { - uri := fmt.Sprintf( - "/%s/%s/access/groups", - routeRoot, - id, + uri := buildURI( + fmt.Sprintf( + "/%s/%s/access/groups", + routeRoot, + id, + ), + pageOpts, ) - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []AccessGroup{}, ResultInfo{}, err } diff --git a/access_policy.go b/access_policy.go index 937fb60cb..82718c1bc 100644 --- a/access_policy.go +++ b/access_policy.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -77,20 +76,17 @@ func (api *API) ZoneLevelAccessPolicies(ctx context.Context, zoneID, application } func (api *API) accessPolicies(ctx context.Context, id string, applicationID string, pageOpts PaginationOptions, routeRoot RouteRoot) ([]AccessPolicy, ResultInfo, error) { - uri := fmt.Sprintf( - "/%s/%s/access/apps/%s/policies", - routeRoot, - id, - applicationID, + uri := buildURI( + fmt.Sprintf( + "/%s/%s/access/apps/%s/policies", + routeRoot, + id, + applicationID, + ), + pageOpts, ) - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []AccessPolicy{}, ResultInfo{}, err } diff --git a/account_members.go b/account_members.go index 1046b5a79..7f0f5cbba 100644 --- a/account_members.go +++ b/account_members.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -62,15 +61,9 @@ func (api *API) AccountMembers(ctx context.Context, accountID string, pageOpts P return []AccountMember{}, ResultInfo{}, errors.New(errMissingAccountID) } - uri := fmt.Sprintf("/accounts/%s/members", accountID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/accounts/%s/members", accountID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []AccountMember{}, ResultInfo{}, err } @@ -96,7 +89,7 @@ func (api *API) CreateAccountMemberWithStatus(ctx context.Context, accountID str uri := fmt.Sprintf("/accounts/%s/members", accountID) - var newMember = AccountMemberInvitation{ + newMember := AccountMemberInvitation{ Email: emailAddress, Roles: roles, Status: status, diff --git a/accounts.go b/accounts.go index 2e5b0f878..a8708fac6 100644 --- a/accounts.go +++ b/accounts.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -59,15 +58,7 @@ type AccountsListParams struct { // // API reference: https://api.cloudflare.com/#accounts-list-accounts func (api *API) Accounts(ctx context.Context, params AccountsListParams) ([]Account, ResultInfo, error) { - uri := "/accounts" - - v, _ := query.Values(params) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, buildURI("/accounts", params), nil) if err != nil { return []Account{}, ResultInfo{}, err } diff --git a/filter.go b/filter.go index 184807ceb..784036f1d 100644 --- a/filter.go +++ b/filter.go @@ -7,7 +7,6 @@ import ( "net/http" "net/url" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -85,15 +84,9 @@ func (api *API) Filter(ctx context.Context, zoneID, filterID string) (Filter, er // // API reference: https://developers.cloudflare.com/firewall/api/cf-filters/get/#get-all-filters func (api *API) Filters(ctx context.Context, zoneID string, pageOpts PaginationOptions) ([]Filter, error) { - uri := fmt.Sprintf("/zones/%s/filters", zoneID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/zones/%s/filters", zoneID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []Filter{}, err } @@ -209,11 +202,12 @@ func (api *API) DeleteFilters(ctx context.Context, zoneID string, filterIDs []st q.Add("id", id) } - queryParams := "?" + q.Encode() + uri := (&url.URL{ + Path: fmt.Sprintf("/zones/%s/filters", zoneID), + RawQuery: q.Encode(), + }).String() - uri := fmt.Sprintf("/zones/%s/filters", zoneID) - - _, err := api.makeRequestContext(ctx, http.MethodDelete, uri+queryParams, nil) + _, err := api.makeRequestContext(ctx, http.MethodDelete, uri, nil) if err != nil { return err } diff --git a/firewall_rules.go b/firewall_rules.go index cd49438b0..0e19a5e4b 100644 --- a/firewall_rules.go +++ b/firewall_rules.go @@ -8,7 +8,6 @@ import ( "net/url" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -45,15 +44,9 @@ type FirewallRuleResponse struct { // // API reference: https://developers.cloudflare.com/firewall/api/cf-firewall-rules/get/#get-all-rules func (api *API) FirewallRules(ctx context.Context, zoneID string, pageOpts PaginationOptions) ([]FirewallRule, error) { - uri := fmt.Sprintf("/zones/%s/firewall/rules", zoneID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/zones/%s/firewall/rules", zoneID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []FirewallRule{}, err } diff --git a/images.go b/images.go index 852eb528f..b8e96fd53 100644 --- a/images.go +++ b/images.go @@ -10,7 +10,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -204,15 +203,9 @@ func (api *API) CreateImageDirectUploadURL(ctx context.Context, accountID string // // API Reference: https://api.cloudflare.com/#cloudflare-images-list-images func (api *API) ListImages(ctx context.Context, accountID string, pageOpts PaginationOptions) ([]Image, error) { - uri := fmt.Sprintf("/accounts/%s/images/v1", accountID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/accounts/%s/images/v1", accountID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []Image{}, err } diff --git a/notifications.go b/notifications.go index 80e73d0bc..6d449c3d3 100644 --- a/notifications.go +++ b/notifications.go @@ -6,8 +6,6 @@ import ( "fmt" "net/http" "time" - - "github.com/google/go-querystring/query" ) // NotificationMechanismData holds a single public facing mechanism data @@ -424,15 +422,8 @@ type AlertHistoryFilter struct { // // API Reference: https://api.cloudflare.com/#notification-history-list-history func (api *API) ListNotificationHistory(ctx context.Context, accountID string, alertHistoryFilter AlertHistoryFilter) ([]NotificationHistory, ResultInfo, error) { - v, _ := query.Values(alertHistoryFilter) - - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - baseURL := fmt.Sprintf("/accounts/%s/alerting/v3/history", accountID) - res, err := api.makeRequestContext(ctx, http.MethodGet, baseURL+queryParams, nil) + uri := buildURI(fmt.Sprintf("/accounts/%s/alerting/v3/history", accountID), alertHistoryFilter) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []NotificationHistory{}, ResultInfo{}, err } diff --git a/pages_project.go b/pages_project.go index f79dcdd2f..a03f2ce49 100644 --- a/pages_project.go +++ b/pages_project.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -120,15 +119,9 @@ type pagesProjectListResponse struct { // // API reference: https://api.cloudflare.com/#pages-project-get-projects func (api *API) ListPagesProjects(ctx context.Context, accountID string, pageOpts PaginationOptions) ([]PagesProject, ResultInfo, error) { - uri := fmt.Sprintf("/accounts/%s/pages/projects", accountID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/accounts/%s/pages/projects", accountID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []PagesProject{}, ResultInfo{}, err } diff --git a/rate_limiting.go b/rate_limiting.go index e97dc1ff6..d8fcc798d 100644 --- a/rate_limiting.go +++ b/rate_limiting.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -106,15 +105,9 @@ func (api *API) CreateRateLimit(ctx context.Context, zoneID string, limit RateLi // // API reference: https://api.cloudflare.com/#rate-limits-for-a-zone-list-rate-limits func (api *API) ListRateLimits(ctx context.Context, zoneID string, pageOpts PaginationOptions) ([]RateLimit, ResultInfo, error) { - uri := fmt.Sprintf("/zones/%s/rate_limits", zoneID) - - v, _ := query.Values(pageOpts) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } + uri := buildURI(fmt.Sprintf("/zones/%s/rate_limits", zoneID), pageOpts) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []RateLimit{}, ResultInfo{}, err } diff --git a/teams_list.go b/teams_list.go index 799fcfcd9..93b829d4e 100644 --- a/teams_list.go +++ b/teams_list.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -125,15 +124,12 @@ func (api *API) TeamsListItems(ctx context.Context, params TeamsListItemsParams) return []TeamsListItem{}, ResultInfo{}, ErrMissingListID } - v, _ := query.Values(params) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - uri := fmt.Sprintf("/%s/%s/gateway/lists/%s/items", AccountRouteRoot, params.AccountID, params.ListID) + uri := buildURI( + fmt.Sprintf("/%s/%s/gateway/lists/%s/items", AccountRouteRoot, params.AccountID, params.ListID), + params, + ) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, nil) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil) if err != nil { return []TeamsListItem{}, ResultInfo{}, err } diff --git a/tunnel.go b/tunnel.go index fa9624ed1..2204b7ac8 100644 --- a/tunnel.go +++ b/tunnel.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -102,15 +101,9 @@ func (api *API) Tunnels(ctx context.Context, params TunnelListParams) ([]Tunnel, return []Tunnel{}, ErrMissingAccountID } - v, _ := query.Values(params) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - uri := fmt.Sprintf("/accounts/%s/cfd_tunnel", params.AccountID) + uri := buildURI(fmt.Sprintf("/accounts/%s/cfd_tunnel", params.AccountID), params) - res, err := api.makeRequestContextWithHeaders(ctx, http.MethodGet, uri+queryParams, nil, nil) + res, err := api.makeRequestContextWithHeaders(ctx, http.MethodGet, uri, nil, nil) if err != nil { return []Tunnel{}, err } diff --git a/tunnel_routes.go b/tunnel_routes.go index c77bcc160..96a9ff57a 100644 --- a/tunnel_routes.go +++ b/tunnel_routes.go @@ -8,7 +8,6 @@ import ( "net/url" "time" - "github.com/google/go-querystring/query" "github.com/pkg/errors" ) @@ -76,15 +75,8 @@ func (api *API) ListTunnelRoutes(ctx context.Context, params TunnelRoutesListPar return []TunnelRoute{}, ErrMissingAccountID } - v, _ := query.Values(params) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - uri := fmt.Sprintf("/%s/%s/teamnet/routes", AccountRouteRoot, params.AccountID) - res, err := api.makeRequestContext(ctx, http.MethodGet, uri+queryParams, params) - + uri := buildURI(fmt.Sprintf("/%s/%s/teamnet/routes", AccountRouteRoot, params.AccountID), params) + res, err := api.makeRequestContext(ctx, http.MethodGet, uri, params) if err != nil { return []TunnelRoute{}, err } diff --git a/utils.go b/utils.go new file mode 100644 index 000000000..1e04f8342 --- /dev/null +++ b/utils.go @@ -0,0 +1,13 @@ +package cloudflare + +import ( + "net/url" + + "github.com/google/go-querystring/query" +) + +// buildURI assembles the base path and queries. +func buildURI(path string, options interface{}) string { + v, _ := query.Values(options) + return (&url.URL{Path: path, RawQuery: v.Encode()}).String() +} diff --git a/zones.go b/zones.go index 2b7578efb..ecd4d7836 100644 --- a/zones.go +++ b/zones.go @@ -6,8 +6,6 @@ import ( "fmt" "net/http" "regexp" - - "github.com/google/go-querystring/query" ) type ZoneIdentifier string @@ -91,13 +89,7 @@ func (s *ZonesService) Get(ctx context.Context, zoneID ZoneIdentifier) (Zone, er // // API reference: https://api.cloudflare.com/#zone-list-zones func (s *ZonesService) List(ctx context.Context, params *ZoneParams) ([]Zone, *ResultInfo, error) { - v, _ := query.Values(params) - queryParams := v.Encode() - if queryParams != "" { - queryParams = "?" + queryParams - } - - res, _ := s.client.Call(ctx, http.MethodGet, "/zones"+queryParams, nil) + res, _ := s.client.Call(ctx, http.MethodGet, buildURI("/zones", params), nil) var r ZonesResponse err := json.Unmarshal(res, &r) From c713e854588f381f80b1106341f10ad7e996c5fb Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Fri, 27 May 2022 09:26:58 +1000 Subject: [PATCH 2/2] add test coverage for buildURI --- utils_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 utils_test.go diff --git a/utils_test.go b/utils_test.go new file mode 100644 index 000000000..5973d8c72 --- /dev/null +++ b/utils_test.go @@ -0,0 +1,38 @@ +package cloudflare + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type testExample struct { + A string `url:"a,omitempty"` + C string `url:"c,omitempty"` + + PaginationOptions +} + +func Test_buildURI(t *testing.T) { + tests := map[string]struct { + path string + params interface{} + want string + }{ + "multi level path without params": {path: "/accounts/foo", params: testExample{}, want: "/accounts/foo"}, + "multi level path with params": {path: "/zones/foo", params: testExample{A: "b"}, want: "/zones/foo?a=b"}, + "multi level path with multiple params": {path: "/zones/foo", params: testExample{A: "b", C: "d"}, want: "/zones/foo?a=b&c=d"}, + "multi level path with nested fields": {path: "/zones/foo", params: testExample{A: "b", C: "d", PaginationOptions: PaginationOptions{PerPage: 10}}, want: "/zones/foo?a=b&c=d&per_page=10"}, + "single level path without params": {path: "/foo", params: testExample{}, want: "/foo"}, + "single level path with params": {path: "/bar", params: testExample{C: "d"}, want: "/bar?c=d"}, + "single level path with multiple params": {path: "/foo", params: testExample{A: "b", C: "d"}, want: "/foo?a=b&c=d"}, + "single level path with nested fields": {path: "/foo", params: testExample{A: "b", C: "d", PaginationOptions: PaginationOptions{PerPage: 10}}, want: "/foo?a=b&c=d&per_page=10"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := buildURI(tc.path, tc.params) + assert.Equal(t, tc.want, got) + }) + } +}