Refactor the URI path building code

[favonia]

Description

Refactor the URI building code. My only hesitation:

1. Is cloudflare_experimentnal.go the right place for the new utility function buildURI?
2. In tunnel.go, params seems to be in both the URL and the payload. Is that intended?

Has your change been tested?

Yes.

Types of changes

What sort of change does your code introduce/modify?

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.
• This change is using publicly documented (api.cloudflare.com or developers.cloudflare.com) and stable APIs.

[jacobbednarz]

1. Is cloudflare_experimentnal.go the right place for the new utility function buildURI?

Probably not; this file contains the code for the proposed changes to the client itself. I would say drop this in the utils. This is applicable irrespective of the client in use.

2. In tunnel.go, params seems to be in both the URL and the payload. Is that intended?

Yep! The intention behind this is that we're moving towards a model where methods don't have an endless or unknown amount of parameters. Ideally, all filtering/customisation should be done using the ListParams of the service. This is also helpful when we have a service that is available at the user, zone and account levels and a ListParams struct can look like this:

type FooServiceListParams struct {
    UserID    *string
    ZoneID    *string
    AccountID *string
}

func (s *FooService) List(ctx context.Context, params FooServiceListParams) {}

Instead of having three different (and confusing) methods, there is one that knows based on all the fields in the struct, which endpoint to target.

c, err := cloudflare.NewExperimental(...)

// get all user LBs
lb1, _ := c.LoadBalancer.List(ctx, LBServiceListParams{UserID: "foo" })

// get all zone LBs
lb2, _ := c.LoadBalancer.List(ctx, LBServiceListParams{ZoneID: "bar" })

// get all account LBs
lb3, _ := c.LoadBalancer.List(ctx, LBServiceListParams{AccountID: "baz" })

// get all zone _and_ account LBs
lb4, _ := c.LoadBalancer.List(ctx, LBServiceListParams{ZoneID: "bar", AccountID: "baz" })

How the endpoint chooses to filter will be based on the service team but we will reuse the same struct (unless we find a good reason where this doesn't fit).

[jacobbednarz]

i just added some test coverage to this but i'm 👍 on the change. @favonia are you happy for this to be merged now?

[favonia]

i just added some test coverage to this but i'm +1 on the change. @favonia are you happy for this to be merged now?

Yes. The testing looks great!