diff --git a/internal/kosli/client.go b/internal/kosli/client.go new file mode 100644 index 000000000..c51a380da --- /dev/null +++ b/internal/kosli/client.go @@ -0,0 +1,81 @@ +// Package kosli is a client for the Kosli API, with one file per tag of the +// API's OpenAPI spec. +package kosli + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + + "github.com/kosli-dev/cli/internal/requests" +) + +// Sender sends one request to the Kosli API. *requests.Client satisfies it. +type Sender interface { + Do(*requests.RequestParams) (*requests.HTTPResponse, error) +} + +// Client calls the Kosli API for one organization on one host. +type Client struct { + sender Sender + host string + org string + token string +} + +// New returns a Client that sends its requests through sender. +func New(sender Sender, host, org, token string) *Client { + return &Client{sender: sender, host: host, org: org, token: token} +} + +// Result is what a call to the Kosli API returned. +type Result struct { + // Raw is the response body exactly as the server sent it, so a command + // can print it for --output json without re-encoding. + Raw []byte + // Created is true when the server created the resource (201) rather than + // updating one that already existed (200). + Created bool +} + +// Response is the Result of a read whose body decodes to T. The body is decoded +// only when Value is called, so a caller that prints Raw, as --output json +// does, is not failed by a body that T does not fit. +type Response[T any] struct { + Result +} + +// Value decodes the body. An empty body, as from a dry run, is T's zero value. +func (r *Response[T]) Value() (T, error) { + var value T + if len(r.Raw) == 0 { + return value, nil + } + if err := json.Unmarshal(r.Raw, &value); err != nil { + return value, fmt.Errorf("failed to decode the Kosli API response: %w", err) + } + return value, nil +} + +func (c *Client) endpoint(segments ...string) (string, error) { + return url.JoinPath(c.host, append([]string{"api/v2"}, segments...)...) +} + +func (c *Client) send(ctx context.Context, params *requests.RequestParams) (*Result, error) { + params.Context = ctx + params.Token = c.token + response, err := c.sender.Do(params) + if err != nil { + return nil, err + } + if response == nil { + // A dry run logs the request instead of sending it. + return &Result{}, nil + } + return &Result{ + Raw: []byte(response.Body), + Created: response.Resp.StatusCode == http.StatusCreated, + }, nil +} diff --git a/internal/kosli/client_test.go b/internal/kosli/client_test.go new file mode 100644 index 000000000..5a32ae7b9 --- /dev/null +++ b/internal/kosli/client_test.go @@ -0,0 +1,272 @@ +package kosli + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// received is what the fake server saw, so a test can assert on the request +// rather than only on what came back. +type received struct { + hits int + method string + rawPath string + authHeader string +} + +func newFakeServer(t *testing.T, status int, responseBody string) (*httptest.Server, *received) { + t.Helper() + seen := &received{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen.hits++ + seen.method = r.Method + seen.rawPath = r.URL.EscapedPath() + seen.authHeader = r.Header.Get("Authorization") + w.WriteHeader(status) + _, err := w.Write([]byte(responseBody)) + // assert, not require: FailNow must not be called off the test goroutine. + assert.NoError(t, err) + })) + t.Cleanup(server.Close) + return server, seen +} + +func newHTTPClient(t *testing.T) *requests.Client { + t.Helper() + // No retries, so a 409 or 5xx surfaces as itself instead of being retried away. + httpClient, err := requests.NewKosliClient("", 0, false, logger.NewLogger(io.Discard, io.Discard, false)) + require.NoError(t, err) + return httpClient +} + +func newTestClient(t *testing.T, host string) *Client { + t.Helper() + return New(newHTTPClient(t), host, "test-org", "test-token") +} + +func TestSendRequestsTheEndpointWithTheTokenAndReturnsTheRawBody(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, `{"name":"trail-1"}`) + client := newTestClient(t, server.URL) + + endpoint, err := client.endpoint("trails", "test-org", "flow-1", "trail-1") + require.NoError(t, err) + result, err := client.send(context.Background(), &requests.RequestParams{Method: http.MethodGet, URL: endpoint}) + + require.NoError(t, err) + require.Equal(t, http.MethodGet, seen.method) + require.Equal(t, "/api/v2/trails/test-org/flow-1/trail-1", seen.rawPath) + require.Equal(t, "Bearer test-token", seen.authHeader) + require.JSONEq(t, `{"name":"trail-1"}`, string(result.Raw)) +} + +func TestSendReportsA201AsCreated(t *testing.T) { + for _, tc := range []struct { + status int + created bool + }{ + {http.StatusCreated, true}, + {http.StatusOK, false}, + } { + t.Run(http.StatusText(tc.status), func(t *testing.T) { + server, _ := newFakeServer(t, tc.status, `"OK"`) + client := newTestClient(t, server.URL) + + endpoint, err := client.endpoint("flows", "test-org") + require.NoError(t, err) + result, err := client.send(context.Background(), &requests.RequestParams{Method: http.MethodPut, URL: endpoint}) + + require.NoError(t, err) + require.Equal(t, tc.created, result.Created) + }) + } +} + +func TestEndpointEscapesEachPathSegment(t *testing.T) { + client := New(nil, "https://app.kosli.com", "test-org", "test-token") + + endpoint, err := client.endpoint("trails", "test-org", "my flow", "trail?#1") + + require.NoError(t, err) + require.Equal(t, "https://app.kosli.com/api/v2/trails/test-org/my%20flow/trail%3F%231", endpoint) +} + +func TestSendReturnsAnAPIErrorThatTheStatusHelpersRecognise(t *testing.T) { + for _, tc := range []struct { + status int + is func(error) bool + }{ + {http.StatusNotFound, IsNotFound}, + // A 409 is retried until retries run out, which ends in a plain error, + // so only a request that opts out of conflict retries sees it. + {http.StatusConflict, IsConflict}, + {http.StatusForbidden, IsForbidden}, + } { + t.Run(http.StatusText(tc.status), func(t *testing.T) { + server, _ := newFakeServer(t, tc.status, `{"message":"nope"}`) + client := newTestClient(t, server.URL) + + endpoint, err := client.endpoint("flows", "test-org", "missing") + require.NoError(t, err) + _, err = client.send(context.Background(), &requests.RequestParams{ + Method: http.MethodGet, + URL: endpoint, + DisableConflictRetry: true, + }) + + var apiErr *APIError + require.ErrorAs(t, err, &apiErr) + require.Equal(t, tc.status, apiErr.StatusCode) + require.True(t, tc.is(err)) + }) + } +} + +func TestStatusHelpersDoNotMatchOtherStatusesOrOtherErrors(t *testing.T) { + serverError := &APIError{StatusCode: http.StatusInternalServerError} + for _, is := range []func(error) bool{IsNotFound, IsConflict, IsForbidden} { + require.False(t, is(serverError)) + require.False(t, is(errors.New("connection refused"))) + require.False(t, is(nil)) + } +} + +func TestDryRunStillSendsAGet(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, `{"name":"trail-1"}`) + client := New(DryRun(newHTTPClient(t)), server.URL, "test-org", "test-token") + + endpoint, err := client.endpoint("trails", "test-org", "flow-1", "trail-1") + require.NoError(t, err) + result, err := client.send(context.Background(), &requests.RequestParams{Method: http.MethodGet, URL: endpoint}) + + require.NoError(t, err) + require.Equal(t, 1, seen.hits) + require.JSONEq(t, `{"name":"trail-1"}`, string(result.Raw)) +} + +func TestDryRunStillSendsTheOtherSafeMethods(t *testing.T) { + for _, method := range []string{http.MethodHead, http.MethodOptions} { + t.Run(method, func(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, ``) + client := New(DryRun(newHTTPClient(t)), server.URL, "test-org", "test-token") + + endpoint, err := client.endpoint("flows", "test-org") + require.NoError(t, err) + _, err = client.send(context.Background(), &requests.RequestParams{Method: method, URL: endpoint}) + + require.NoError(t, err) + require.Equal(t, 1, seen.hits) + }) + } +} + +func TestDryRunLeavesTheCallersParamsUnchanged(t *testing.T) { + recorder := &recordingSender{} + params := &requests.RequestParams{Method: http.MethodPut, URL: "https://app.kosli.com/api/v2/flows/test-org"} + + _, err := DryRun(recorder).Do(params) + + require.NoError(t, err) + require.False(t, params.DryRun) + require.True(t, recorder.got.DryRun) +} + +// recordingSender keeps the params it was given instead of sending them. +type recordingSender struct { + got *requests.RequestParams +} + +func (s *recordingSender) Do(params *requests.RequestParams) (*requests.HTTPResponse, error) { + s.got = params + return nil, nil +} + +func TestDryRunDoesNotSendAWriteAndReturnsAnEmptyResult(t *testing.T) { + for _, method := range []string{http.MethodPut, http.MethodPost, http.MethodPatch, http.MethodDelete} { + t.Run(method, func(t *testing.T) { + server, seen := newFakeServer(t, http.StatusCreated, `"OK"`) + client := New(DryRun(newHTTPClient(t)), server.URL, "test-org", "test-token") + + endpoint, err := client.endpoint("flows", "test-org") + require.NoError(t, err) + result, err := client.send(context.Background(), &requests.RequestParams{ + Method: method, + URL: endpoint, + Payload: map[string]string{"name": "flow-1"}, + }) + + require.NoError(t, err) + require.Equal(t, 0, seen.hits) + require.Equal(t, &Result{}, result) + }) + } +} + +type namedThing struct { + Name string `json:"name"` +} + +func TestValueDecodesTheBody(t *testing.T) { + response := &Response[namedThing]{Result: Result{Raw: []byte(`{"name":"trail-1","unread":true}`)}} + + value, err := response.Value() + + require.NoError(t, err) + require.Equal(t, namedThing{Name: "trail-1"}, value) +} + +func TestValueOfADryRunResultIsZero(t *testing.T) { + response := &Response[namedThing]{} + + value, err := response.Value() + + require.NoError(t, err) + require.Equal(t, namedThing{}, value) +} + +func TestValueOfABodyThatDoesNotMatchIsAnErrorButRawIsStillThere(t *testing.T) { + // a caller printing Raw, as --output json does, must not be failed by a + // body the type does not fit + raw := []byte(`["not","an","object"]`) + response := &Response[namedThing]{Result: Result{Raw: raw}} + + _, err := response.Value() + + require.ErrorContains(t, err, "failed to decode the Kosli API response") + require.Equal(t, raw, response.Raw) +} + +func TestACancelledContextStopsTheRequestBeforeItReachesTheServer(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, `{}`) + client := newTestClient(t, server.URL) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + endpoint, err := client.endpoint("flows", "test-org") + require.NoError(t, err) + _, err = client.send(ctx, &requests.RequestParams{Method: http.MethodGet, URL: endpoint}) + + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, 0, seen.hits) +} + +func TestANilContextIsTreatedAsBackground(t *testing.T) { + server, seen := newFakeServer(t, http.StatusOK, `{}`) + client := newTestClient(t, server.URL) + + endpoint, err := client.endpoint("flows", "test-org") + require.NoError(t, err) + //nolint:staticcheck // a command's run called directly by a test has no context + _, err = client.send(nil, &requests.RequestParams{Method: http.MethodGet, URL: endpoint}) + + require.NoError(t, err) + require.Equal(t, 1, seen.hits) +} diff --git a/internal/kosli/dryrun.go b/internal/kosli/dryrun.go new file mode 100644 index 000000000..19b4e3031 --- /dev/null +++ b/internal/kosli/dryrun.go @@ -0,0 +1,27 @@ +package kosli + +import ( + "net/http" + + "github.com/kosli-dev/cli/internal/requests" +) + +// DryRun wraps next so that reads still reach the server but writes are only +// logged, with the payload that would have been sent. +func DryRun(next Sender) Sender { + return dryRunSender{next: next} +} + +type dryRunSender struct { + next Sender +} + +func (s dryRunSender) Do(params *requests.RequestParams) (*requests.HTTPResponse, error) { + switch params.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + return s.next.Do(params) + } + write := *params + write.DryRun = true + return s.next.Do(&write) +} diff --git a/internal/kosli/errors.go b/internal/kosli/errors.go new file mode 100644 index 000000000..399cd83b2 --- /dev/null +++ b/internal/kosli/errors.go @@ -0,0 +1,32 @@ +package kosli + +import ( + "errors" + "net/http" + + "github.com/kosli-dev/cli/internal/requests" +) + +// APIError is the error for a response with a status other than 200 or 201. +type APIError = requests.APIError + +// IsNotFound reports whether err is an APIError for a 404. +func IsNotFound(err error) bool { + return hasStatus(err, http.StatusNotFound) +} + +// IsConflict reports whether err is an APIError for a 409. +func IsConflict(err error) bool { + return hasStatus(err, http.StatusConflict) +} + +// IsForbidden reports whether err is an APIError for a 403, which is also what +// a beta endpoint answers to an organization without the feature enabled. +func IsForbidden(err error) bool { + return hasStatus(err, http.StatusForbidden) +} + +func hasStatus(err error, status int) bool { + var apiErr *APIError + return errors.As(err, &apiErr) && apiErr.StatusCode == status +} diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 8e7ebcc03..cea1f93cb 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -127,6 +127,9 @@ type RequestParams struct { Password string Token string DryRun bool + // Context cancels the request, including its retries. Nil means + // context.Background(). + Context context.Context // DisableConflictRetry stops the client retrying on HTTP 409 for this // request. By default 409 is retried (it signals a transient lock // conflict), but some endpoints use 409 for a permanent client error @@ -166,7 +169,11 @@ func (p *RequestParams) newHTTPRequest() (*http.Request, map[string]any, error) } } - req, err := http.NewRequest(p.Method, p.URL, body) + ctx := p.Context + if ctx == nil { + ctx = context.Background() + } + req, err := http.NewRequestWithContext(ctx, p.Method, p.URL, body) if err != nil { return nil, nil, fmt.Errorf("failed to create %s request to %s : %v", p.Method, p.URL, err) } @@ -301,7 +308,7 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) { resp, err := c.HttpClient.Do(req) if err != nil { // err from retryable client is detailed enough - return nil, fmt.Errorf("%v", err) + return nil, fmt.Errorf("%w", err) } defer func() {