Reduce code for building requests and decoding response bodies

[itchyny]

I noticed a REST API is not implemented in this official client (can you tell which one?). I was to implement to support the API, but I noticed that there are too many code duplication. Most client methods create a http request, serialize the payload and add content type header, send the request, and decode the response body. Some methods use c.PostJSON(...), but this does not support decoding response bodies.

Too much code duplication makes it boring to read and write.

One of the difficulties to deduplicate this kind of client code was how to decode the response body into each type. But Generics was out in Go 1.18 and it changed the world of Go. We can now use Generics in this library, you now support only Go 1.20 and 1.21. One of the unfamous limitation of it is that we cannot use it for struct methods, but I think this is a small thing, just pass the receiver as an argument.

Here's a list of client utilities I implemented in this PR. There are four requestX each method, and two variants for GET.

func requestGet[T any](client *Client, path string) (*T, error)
func requestGetWithParams[T any](client *Client, path string, params url.Values) (*T, error)
func requestGetAndReturnHeader[T any](client *Client, path string) (*T, http.Header, error)
func requestPost[T any](client *Client, path string, payload any) (*T, error)
func requestPut[T any](client *Client, path string, payload any) (*T, error)
func requestDelete[T any](client *Client, path string) (*T, error)

For example, we can implement some methods using the utilities as follows.

func (c *Client) GetOrg() (*Org, error) {
	return requestGet[Org](c, "/api/v0/org")
}

func (c *Client) CreateDowntime(param *Downtime) (*Downtime, error) {
	return requestPost[Downtime](c, "/api/v0/downtimes", param)
}

func (c *Client) FindServices() ([]*Service, error) {
	data, err := requestGet[struct {
		Services []*Service `json:"services"`
	}](c, "/api/v0/services")
	if err != nil {
		return nil, err
	}
	return data.Services, nil
}

func (c *Client) FetchLatestMetricValues(hostIDs []string, metricNames []string) (LatestMetricValues, error) {
	params := url.Values{}
	for _, hostID := range hostIDs {
		params.Add("hostId", hostID)
	}
	for _, metricName := range metricNames {
		params.Add("name", metricName)
	}

	data, err := requestGetWithParams[struct {
		LatestMetricValues LatestMetricValues `json:"tsdbLatest"`
	}](c, "/api/v0/tsdb/latest", params)
	if err != nil {
		return nil, err
	}
	return data.LatestMetricValues, nil
}

This is more declarative and neat isn't it? We don't need to care about closing the response, to duplicate code decoding the response body. Previously the code was like this...

func (c *Client) GetOrg() (*Org, error) {
	req, err := http.NewRequest("GET", c.urlFor("/api/v0/org").String(), nil)
	if err != nil {
		return nil, err
	}
	resp, err := c.Request(req)
	defer closeResponse(resp)
	if err != nil {
		return nil, err
	}
	var data Org
	err = json.NewDecoder(resp.Body).Decode(&data)
	if err != nil {
		return nil, err
	}
	return &data, nil
}

func (c *Client) CreateDowntime(param *Downtime) (*Downtime, error) {
	resp, err := c.PostJSON("/api/v0/downtimes", param)
	defer closeResponse(resp)
	if err != nil {
		return nil, err
	}

	var data Downtime
	err = json.NewDecoder(resp.Body).Decode(&data)
	if err != nil {
		return nil, err
	}
	return &data, nil
}

func (c *Client) FindServices() ([]*Service, error) {
	req, err := http.NewRequest("GET", c.urlFor("/api/v0/services").String(), nil)
	if err != nil {
		return nil, err
	}
	resp, err := c.Request(req)
	defer closeResponse(resp)
	if err != nil {
		return nil, err
	}

	var data struct {
		Services []*Service `json:"services"`
	}
	err = json.NewDecoder(resp.Body).Decode(&data)
	if err != nil {
		return nil, err
	}
	return data.Services, err
}

func (c *Client) FetchLatestMetricValues(hostIDs []string, metricNames []string) (LatestMetricValues, error) {
	v := url.Values{}
	for _, hostID := range hostIDs {
		v.Add("hostId", hostID)
	}
	for _, metricName := range metricNames {
		v.Add("name", metricName)
	}

	req, err := http.NewRequest("GET", fmt.Sprintf("%s?%s", c.urlFor("/api/v0/tsdb/latest").String(), v.Encode()), nil)
	if err != nil {
		return nil, err
	}
	resp, err := c.Request(req)
	defer closeResponse(resp)
	if err != nil {
		return nil, err
	}

	var data struct {
		LatestMetricValues LatestMetricValues `json:"tsdbLatest"`
	}
	err = json.NewDecoder(resp.Body).Decode(&data)
	if err != nil {
		return nil, err
	}

	return data.LatestMetricValues, err
}

My only concern is how this patch grows end users' executable file size. I tested only mkr, but file size growth is small (linux amd64 executable size: 10572K -> 10640K). I think this is acceptable, but if there's a case suffering from this patch, we can drop the type parameter from requestInternal, create the response value in requestGet, etc., and pass it by reference. This reduces the generated code for requestInternal.

I'm sorry for submitting a large refactoring patch. I don't mind you close this PR due to any reason, because I don't care how much code it is implemented by, as a package user. But hopefully, I would like to see some good news.

---

This is an off topic. I'm maintaining mackerel-client-rs. After two years of hiatus, I rebooted the implementation recently. Due to the richness of type system and my own macro definition, I can implement the client methods with fancy code. This leads me motivated to reduce the code duplication of the Go client.

Example of mackerel-client-rs code

pub async fn get_organization(&self) -> Result<Organization> {
    self.request(
        Method::GET,
        "/api/v0/org",
        query_params![],
        request_body![],
        response_body!(..),
    )
    .await
}

pub async fn create_downtime(&self, downtime_value: &DowntimeValue) -> Result<Downtime> {
    self.request(
        Method::POST,
        "/api/v0/downtimes",
        query_params![],
        request_body!(downtime_value),
        response_body!(..),
    )
    .await
}

pub async fn list_services(&self) -> Result<Vec<Service>> {
    self.request(
        Method::GET,
        "/api/v0/services",
        query_params![],
        request_body![],
        response_body! { services: Vec<Service> },
    )
    .await
}

pub async fn list_latest_host_metric_values(
    &self,
    host_ids: impl IntoIterator<Item = impl Into<HostId>>,
    metric_names: impl IntoIterator<Item = impl AsRef<str>>,
) -> Result<HashMap<HostId, HashMap<String, Option<MetricValue>>>> {
    self.request(
        Method::GET,
        "/api/v0/tsdb/latest",
        &host_ids
            .into_iter()
            .map(|host_id| ("hostId", host_id.into().to_string()))
            .chain(
                metric_names
                    .into_iter()
                    .map(|metric_name| ("name", metric_name.as_ref().to_owned())),
            )
            .collect::<Vec<_>>(),
        request_body![],
        response_body! { tsdbLatest: HashMap<HostId, HashMap<String, Option<MetricValue>>> },
    )
    .await
}

[yohfee]

Thank you for submitting great pull request!