refactor: update client code+tests to use Requester interface (#549)

Refactor client test framework to use the requester interface.
canonical · Jan 13, 2025 · 4db6a03 · 4db6a03
1 parent 05ea5bc
commit 4db6a03
Showing 12 changed files with 252 additions and 95 deletions.
diff --git a/client/changes.go b/client/changes.go
@@ -16,6 +16,7 @@ package client
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/url"
@@ -100,7 +101,15 @@ func (client *Client) Change(id string) (*Change, error) {
 	}
 
 	var chgd changeAndData
-	_, err := client.doSync("GET", "/v1/changes/"+id, nil, nil, nil, &chgd)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/changes/" + id,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&chgd)
 	if err != nil {
 		return nil, err
 	}
@@ -126,7 +135,17 @@ func (client *Client) Abort(id string) (*Change, error) {
 	}
 
 	var chg Change
-	if _, err := client.doSync("POST", "/v1/changes/"+id, nil, nil, &body, &chg); err != nil {
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "POST",
+		Path:   "/v1/changes/" + id,
+		Body:   &body,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&chg)
+	if err != nil {
 		return nil, err
 	}
 
@@ -173,7 +192,16 @@ func (client *Client) Changes(opts *ChangesOptions) ([]*Change, error) {
 	}
 
 	var chgds []changeAndData
-	_, err := client.doSync("GET", "/v1/changes", query, nil, nil, &chgds)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/changes",
+		Query:  query,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&chgds)
 	if err != nil {
 		return nil, err
 	}
@@ -209,7 +237,16 @@ func (client *Client) WaitChange(id string, opts *WaitChangeOptions) (*Change, e
 		query.Set("timeout", opts.Timeout.String())
 	}
 
-	_, err := client.doSync("GET", "/v1/changes/"+id+"/wait", query, nil, nil, &chgd)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/changes/" + id + "/wait",
+		Query:  query,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&chgd)
 	if err != nil {
 		return nil, err
 	}

diff --git a/client/checks.go b/client/checks.go
@@ -15,6 +15,7 @@
 package client
 
 import (
+	"context"
 	"net/url"
 )
 
@@ -84,7 +85,16 @@ func (client *Client) Checks(opts *ChecksOptions) ([]*CheckInfo, error) {
 		query["names"] = opts.Names
 	}
 	var checks []*CheckInfo
-	_, err := client.doSync("GET", "/v1/checks", query, nil, nil, &checks)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/checks",
+		Query:  query,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&checks)
 	if err != nil {
 		return nil, err
 	}

diff --git a/client/client.go b/client/client.go
@@ -399,48 +399,6 @@ func decodeInto(reader io.Reader, v interface{}) error {
 	return nil
 }
 
-func (client *Client) doSync(method, path string, query url.Values, headers map[string]string, body io.Reader, v interface{}) (*RequestResponse, error) {
-	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
-		Type:    SyncRequest,
-		Method:  method,
-		Path:    path,
-		Query:   query,
-		Headers: headers,
-		Body:    body,
-	})
-	if err != nil {
-		return nil, err
-	}
-	if v != nil {
-		err = resp.DecodeResult(v)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return resp, nil
-}
-
-func (client *Client) doAsync(method, path string, query url.Values, headers map[string]string, body io.Reader, v interface{}) (*RequestResponse, error) {
-	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
-		Type:    AsyncRequest,
-		Method:  method,
-		Path:    path,
-		Query:   query,
-		Headers: headers,
-		Body:    body,
-	})
-	if err != nil {
-		return nil, err
-	}
-	if v != nil {
-		err = resp.DecodeResult(v)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return resp, nil
-}
-
 // A response produced by the REST API will usually fit in this
 // (exceptions are the icons/ endpoints obvs)
 type response struct {
@@ -522,7 +480,16 @@ type SysInfo struct {
 func (client *Client) SysInfo() (*SysInfo, error) {
 	var sysInfo SysInfo
 
-	if _, err := client.doSync("GET", "/v1/system-info", nil, nil, nil, &sysInfo); err != nil {
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/system-info",
+	})
+	if err != nil {
+		return nil, fmt.Errorf("cannot obtain system details: %w", err)
+	}
+	err = resp.DecodeResult(&sysInfo)
+	if err != nil {
 		return nil, fmt.Errorf("cannot obtain system details: %w", err)
 	}
 
@@ -544,7 +511,19 @@ func (client *Client) DebugPost(action string, params interface{}, result interf
 		return err
 	}
 
-	_, err = client.doSync("POST", "/v1/debug", nil, nil, bytes.NewReader(body), result)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "POST",
+		Path:   "/v1/debug",
+		Body:   bytes.NewReader(body),
+	})
+	if err != nil {
+		return err
+	}
+	err = resp.DecodeResult(result)
+	if err != nil {
+		return err
+	}
 	return err
 }
 
@@ -554,8 +533,16 @@ func (client *Client) DebugGet(action string, result interface{}, params map[str
 	for k, v := range params {
 		urlParams.Set(k, v)
 	}
-	_, err := client.doSync("GET", "/v1/debug", urlParams, nil, nil, &result)
-	return err
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:   SyncRequest,
+		Method: "GET",
+		Path:   "/v1/debug",
+		Query:  urlParams,
+	})
+	if err != nil {
+		return err
+	}
+	return resp.DecodeResult(&result)
 }
 
 type defaultRequester struct {

diff --git a/client/client_test.go b/client/client_test.go
@@ -16,6 +16,7 @@ package client_test
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -101,7 +102,12 @@ func (cs *clientSuite) TestNewBaseURLError(c *C) {
 
 func (cs *clientSuite) TestClientDoReportsErrors(c *C) {
 	cs.err = errors.New("ouchie")
-	err := cs.cli.Do("GET", "/", nil, nil, nil)
+	_, err := cs.cli.Requester().Do(context.Background(), &client.RequestOptions{
+		Type:   client.RawRequest,
+		Method: "GET",
+		Path:   "/",
+	})
+	c.Assert(err, NotNil)
 	c.Check(err, ErrorMatches, "cannot communicate with server: ouchie")
 	if cs.doCalls < 2 {
 		c.Fatalf("do did not retry")
@@ -127,7 +133,15 @@ func (cs *clientSuite) TestClientWorks(c *C) {
 	var v []int
 	cs.rsp = `[1,2]`
 	reqBody := io.NopCloser(strings.NewReader(""))
-	err := cs.cli.Do("GET", "/this", nil, reqBody, &v)
+	resp, err := cs.cli.Requester().Do(context.Background(), &client.RequestOptions{
+		Type:   client.RawRequest,
+		Method: "GET",
+		Path:   "/this",
+		Body:   reqBody,
+	})
+	c.Check(err, IsNil)
+	dec := json.NewDecoder(resp.Body)
+	err = dec.Decode(&v)
 	c.Check(err, IsNil)
 	c.Check(v, DeepEquals, []int{1, 2})
 	c.Assert(cs.req, NotNil)
@@ -138,8 +152,11 @@ func (cs *clientSuite) TestClientWorks(c *C) {
 }
 
 func (cs *clientSuite) TestClientDefaultsToNoAuthorization(c *C) {
-	var v string
-	_ = cs.cli.Do("GET", "/this", nil, nil, &v)
+	_, _ = cs.cli.Requester().Do(context.Background(), &client.RequestOptions{
+		Type:   client.RawRequest,
+		Method: "GET",
+		Path:   "/this",
+	})
 	c.Assert(cs.req, NotNil)
 	authorization := cs.req.Header.Get("Authorization")
 	c.Check(authorization, Equals, "")
@@ -288,9 +305,15 @@ func (cs *clientSuite) TestUserAgent(c *C) {
 	c.Assert(err, IsNil)
 	cli.SetDoer(cs)
 
+	resp, err := cli.Requester().Do(context.Background(), &client.RequestOptions{
+		Type:   client.RawRequest,
+		Method: "GET",
+		Path:   "/",
+	})
+	c.Assert(err, IsNil)
 	var v string
-	_ = cli.Do("GET", "/", nil, nil, &v)
-	c.Assert(cs.req, NotNil)
+	err = resp.DecodeResult(&v)
+	c.Assert(err, NotNil)
 	c.Check(cs.req.Header.Get("User-Agent"), Equals, "some-agent/9.87")
 }
 

diff --git a/client/exec.go b/client/exec.go
@@ -16,6 +16,7 @@ package client
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -152,7 +153,17 @@ func (client *Client) Exec(opts *ExecOptions) (*ExecProcess, error) {
 		"Content-Type": "application/json",
 	}
 	var result execResult
-	resp, err := client.doAsync("POST", "/v1/exec", nil, headers, &body, &result)
+	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
+		Type:    AsyncRequest,
+		Method:  "POST",
+		Path:    "/v1/exec",
+		Headers: headers,
+		Body:    &body,
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = resp.DecodeResult(&result)
 	if err != nil {
 		return nil, err
 	}

diff --git a/client/export_test.go b/client/export_test.go
@@ -17,8 +17,6 @@ package client
 import (
 	"context"
 	"fmt"
-	"io"
-	"net/url"
 )
 
 var (
@@ -30,30 +28,12 @@ func (client *Client) SetDoer(d doer) {
 	client.Requester().(*defaultRequester).doer = d
 }
 
-// TODO: Clean up tests to use the new Requester API. Tests do not generate a client.response type
-// reply in the body while SyncRequest or AsyncRequest responses assume the JSON body can be
-// unmarshalled into client.response.
-func (client *Client) Do(method, path string, query url.Values, body io.Reader, v interface{}) error {
+func (client *Client) FakeAsyncRequest() (changeId string, err error) {
 	resp, err := client.Requester().Do(context.Background(), &RequestOptions{
-		Type:    RawRequest,
-		Method:  method,
-		Path:    path,
-		Query:   query,
-		Headers: nil,
-		Body:    body,
+		Type:   AsyncRequest,
+		Method: "GET",
+		Path:   "/v1/async-test",
 	})
-	if err != nil {
-		return err
-	}
-	err = decodeInto(resp.Body, v)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
-func (client *Client) FakeAsyncRequest() (changeId string, err error) {
-	resp, err := client.doAsync("GET", "/v1/async-test", nil, nil, nil, nil)
 	if err != nil {
 		return "", fmt.Errorf("cannot do async test: %v", err)
 	}