feat(api): improve codegen outputs (#41)

* Use `[]byte` for generated request bodies when the source schema has `format: byte`
* Gzip-compress request bodies when `Content-Encoding: gzip` is set
* Require that all models returned as error conditions in our API implement the `error` interface
* Move the implementation for `api.HealthCheck` out of `ping.go` and into `api/error.go`


Co-authored-by: William Baker <55118525+wbaker85@users.noreply.github.com>

etc/generate-openapi.sh

@@ -24,5 +24,5 @@ docker run --rm -it -u "$(id -u):$(id -g)" \
 
   # Clean up the generated code.
   cd "${ROOT_DIR}"
-  make fmt
+  >/dev/null make fmt
 )

internal/api/api_health.go

@@ -52,7 +52,6 @@ func (r ApiGetHealthRequest) ZapTraceSpan(zapTraceSpan string) ApiGetHealthReque
 	r.zapTraceSpan = &zapTraceSpan
 	return r
 }
-
 func (r ApiGetHealthRequest) GetZapTraceSpan() *string {
 	return r.zapTraceSpan
 }
@@ -147,7 +146,7 @@ func (a *HealthApiService) GetHealthExecute(r ApiGetHealthRequest) (HealthCheck,
 				newErr.error = err.Error()
 				return localVarReturnValue, localVarHTTPResponse, newErr
 			}
-			newErr.model = v
+			newErr.model = &v
 			return localVarReturnValue, localVarHTTPResponse, newErr
 		}
 		var v Error
@@ -156,7 +155,7 @@ func (a *HealthApiService) GetHealthExecute(r ApiGetHealthRequest) (HealthCheck,
 			newErr.error = err.Error()
 			return localVarReturnValue, localVarHTTPResponse, newErr
 		}
-		newErr.model = v
+		newErr.model = &v
 		return localVarReturnValue, localVarHTTPResponse, newErr
 	}
 

internal/api/api_setup.go

@@ -67,7 +67,6 @@ func (r ApiGetSetupRequest) ZapTraceSpan(zapTraceSpan string) ApiGetSetupRequest
 	r.zapTraceSpan = &zapTraceSpan
 	return r
 }
-
 func (r ApiGetSetupRequest) GetZapTraceSpan() *string {
 	return r.zapTraceSpan
 }
@@ -182,15 +181,14 @@ func (r ApiPostSetupRequest) OnboardingRequest(onboardingRequest OnboardingReque
 	r.onboardingRequest = &onboardingRequest
 	return r
 }
-
 func (r ApiPostSetupRequest) GetOnboardingRequest() *OnboardingRequest {
 	return r.onboardingRequest
 }
+
 func (r ApiPostSetupRequest) ZapTraceSpan(zapTraceSpan string) ApiPostSetupRequest {
 	r.zapTraceSpan = &zapTraceSpan
 	return r
 }
-
 func (r ApiPostSetupRequest) GetZapTraceSpan() *string {
 	return r.zapTraceSpan
 }
@@ -290,7 +288,7 @@ func (a *SetupApiService) PostSetupExecute(r ApiPostSetupRequest) (OnboardingRes
 			newErr.error = err.Error()
 			return localVarReturnValue, localVarHTTPResponse, newErr
 		}
-		newErr.model = v
+		newErr.model = &v
 		return localVarReturnValue, localVarHTTPResponse, newErr
 	}
 

internal/api/client.go

@@ -12,6 +12,7 @@ package api
 
 import (
 	"bytes"
+	"compress/gzip"
 	"context"
 	"encoding/json"
 	"encoding/xml"
@@ -297,7 +298,14 @@ func (c *APIClient) prepareRequest(
 
 	// Generate a new request
 	if body != nil {
-		localVarRequest, err = http.NewRequest(method, url.String(), body)
+		var b io.Reader = body
+		if enc, ok := headerParams["Content-Encoding"]; ok && enc == "gzip" {
+			b, err = compressWithGzip(b)
+			if err != nil {
+				return nil, err
+			}
+		}
+		localVarRequest, err = http.NewRequest(method, url.String(), b)
 	} else {
 		localVarRequest, err = http.NewRequest(method, url.String(), nil)
 	}
@@ -427,6 +435,20 @@ func setBody(body interface{}, contentType string) (bodyBuf *bytes.Buffer, err e
 	return bodyBuf, nil
 }
 
+func compressWithGzip(data io.Reader) (io.Reader, error) {
+	pr, pw := io.Pipe()
+	gw := gzip.NewWriter(pw)
+	var err error
+
+	go func() {
+		_, err = io.Copy(gw, data)
+		gw.Close()
+		pw.Close()
+	}()
+
+	return pr, err
+}
+
 // detectContentType method is used to figure out `Request.Body` content type for request header
 func detectContentType(body interface{}) string {
 	contentType := "text/plain; charset=utf-8"
@@ -506,11 +528,14 @@ func strlen(s string) int {
 type GenericOpenAPIError struct {
 	body  []byte
 	error string
-	model interface{}
+	model error
 }
 
 // Error returns non-empty string if there was an error.
 func (e GenericOpenAPIError) Error() string {
+	if e.model != nil {
+		return e.model.Error()
+	}
 	return e.error
 }
 

internal/api/client_internal_test.go

@@ -0,0 +1,52 @@
+package api
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNoGzipRequest(t *testing.T) {
+	client := APIClient{cfg: NewConfiguration()}
+	body := []byte("This should not get gzipped")
+	req, err := client.prepareRequest(
+		context.Background(),
+		"/foo", "POST", body,
+		map[string]string{},
+		nil, nil, "", "", nil,
+	)
+	require.NoError(t, err)
+	defer req.Body.Close()
+
+	out := bytes.Buffer{}
+	_, err = io.Copy(&out, req.Body)
+	require.NoError(t, err)
+
+	require.Equal(t, string(body), out.String())
+}
+
+func TestGzipRequest(t *testing.T) {
+	client := APIClient{cfg: NewConfiguration()}
+	body := []byte("This should get gzipped")
+	req, err := client.prepareRequest(
+		context.Background(),
+		"/foo", "POST", body,
+		map[string]string{"Content-Encoding": "gzip"},
+		nil, nil, "", "", nil,
+	)
+	require.NoError(t, err)
+	defer req.Body.Close()
+
+	out := bytes.Buffer{}
+	gzr, err := gzip.NewReader(req.Body)
+	require.NoError(t, err)
+	defer gzr.Close()
+	_, err = io.Copy(&out, gzr)
+	require.NoError(t, err)
+
+	require.Equal(t, string(body), out.String())
+}

internal/api/error.go

@@ -5,7 +5,8 @@ import (
 	"strings"
 )
 
-// Extension to let our API error type be used as a "standard" error.
+// Extensions to let our API error types be used as "standard" errors.
+
 func (o *Error) Error() string {
 	if o.Message != "" && o.Err != nil {
 		var b strings.Builder
@@ -20,3 +21,17 @@ func (o *Error) Error() string {
 	}
 	return fmt.Sprintf("<%s>", o.Code)
 }
+
+func (o *HealthCheck) Error() string {
+	if o.Status == HEALTHCHECKSTATUS_PASS {
+		// Make sure we aren't misusing HealthCheck responses.
+		panic("successful healthcheck used as an error!")
+	}
+	var message string
+	if o.Message != nil {
+		message = *o.Message
+	} else {
+		message = fmt.Sprintf("check %s failed", o.Name)
+	}
+	return fmt.Sprintf("health check failed: %s", message)
+}

internal/api/templates/README.md

@@ -13,11 +13,15 @@ multiple locations.
 
 `api.mustache`
 * Add `GetX()` methods for each request parameter `X`, for use in unit tests
+* Add checks for `isByteArray` to generate `[]byte` request fields instead of `*string`
+* Update creation of `GenericOpenAPIError` to track sub-error models by reference
 
 `client.mustache`
 * Removed use of `golang.org/x/oauth2` to avoid its heavy dependencies
 * Fixed error strings to be idiomatic according to staticcheck (lowercase, no punctuation)
 * Use `strings.EqualFold` instead of comparing two `strings.ToLower` calls
+* GZip request bodies when `Content-Encoding: gzip` is set
+* Update the `GenericOpenAPIError` type to enforce that error response models implement the `error` interface
 
 `configuration.mustache`
 * Deleted `ContextOAuth2` key to match modification in client

internal/api/templates/api.mustache

@@ -52,18 +52,19 @@ type {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request s
 	ApiService *{{classname}}Service
 {{/generateInterfaces}}
 {{#allParams}}
-	{{paramName}} {{^isPathParam}}*{{/isPathParam}}{{{dataType}}}
+	{{paramName}} {{#isByteArray}}[]byte{{/isByteArray}}{{^isByteArray}}{{^isPathParam}}*{{/isPathParam}}{{{dataType}}}{{/isByteArray}}
 {{/allParams}}
 }
-{{#allParams}}{{^isPathParam}}
+{{#allParams}}
-func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) {{vendorExtensions.x-export-param-name}}({{paramName}} {{{dataType}}}) {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request {
+func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) {{vendorExtensions.x-export-param-name}}({{paramName}} {{#isByteArray}}[]byte{{/isByteArray}}{{^isByteArray}}{{{dataType}}}{{/isByteArray}}) {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request {
-	r.{{paramName}} = &{{paramName}}
+	r.{{paramName}} = {{^isByteArray}}{{^isPathParam}}&{{/isPathParam}}{{/isByteArray}}{{paramName}}
 	return r
-}{{/isPathParam}}
+}
-
+func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) Get{{vendorExtensions.x-export-param-name}}() {{#isByteArray}}[]byte{{/isByteArray}}{{^isByteArray}}{{^isPathParam}}*{{/isPathParam}}{{{dataType}}}{{/isByteArray}} {
-func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) Get{{vendorExtensions.x-export-param-name}}() {{^isPathParam}}*{{/isPathParam}}{{{dataType}}} {
     return r.{{paramName}}
-}{{/allParams}}
+}
+
+{{/allParams}}
 
 func (r {{#structPrefix}}{{&classname}}{{/structPrefix}}Api{{operationId}}Request) Execute() ({{#returnType}}{{{.}}}, {{/returnType}}*_nethttp.Response, error) {
 	return r.ApiService.{{nickname}}Execute(r)
@@ -352,7 +353,7 @@ func (a *{{{classname}}}Service) {{nickname}}Execute(r {{#structPrefix}}{{&class
 				newErr.error = err.Error()
 				return {{#returnType}}localVarReturnValue, {{/returnType}}localVarHTTPResponse, newErr
 			}
-			newErr.model = v
+			newErr.model = &v
 			{{^-last}}
 			return {{#returnType}}localVarReturnValue, {{/returnType}}localVarHTTPResponse, newErr
 			{{/-last}}

internal/api/templates/client.mustache

@@ -3,6 +3,7 @@ package {{packageName}}
 
 import (
 	"bytes"
+	"compress/gzip"
 	"context"
 	"encoding/json"
 	"encoding/xml"
@@ -306,7 +307,14 @@ func (c *APIClient) prepareRequest(
 
 	// Generate a new request
 	if body != nil {
-		localVarRequest, err = http.NewRequest(method, url.String(), body)
+        var b io.Reader = body
+        if enc, ok := headerParams["Content-Encoding"]; ok && enc == "gzip" {
+            b, err = compressWithGzip(b)
+            if err != nil {
+                return nil, err
+            }
+        }
+		localVarRequest, err = http.NewRequest(method, url.String(), b)
 	} else {
 		localVarRequest, err = http.NewRequest(method, url.String(), nil)
 	}
@@ -477,6 +485,20 @@ func setBody(body interface{}, contentType string) (bodyBuf *bytes.Buffer, err e
 	return bodyBuf, nil
 }
 
+func compressWithGzip(data io.Reader) (io.Reader, error) {
+    pr, pw := io.Pipe()
+    gw := gzip.NewWriter(pw)
+    var err error
+
+    go func() {
+        _, err = io.Copy(gw, data)
+        gw.Close()
+        pw.Close()
+    }()
+
+    return pr, err
+}
+
 // detectContentType method is used to figure out `Request.Body` content type for request header
 func detectContentType(body interface{}) string {
 	contentType := "text/plain; charset=utf-8"
@@ -556,11 +578,14 @@ func strlen(s string) int {
 type GenericOpenAPIError struct {
 	body  []byte
 	error string
-	model interface{}
+	model error
 }
 
 // Error returns non-empty string if there was an error.
 func (e GenericOpenAPIError) Error() string {
+    if e.model != nil {
+        return e.model.Error()
+    }
     return e.error
 }
 

internal/ping.go

@@ -2,7 +2,6 @@ package internal
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/influxdata/influx-cli/v2/internal/api"
 )
@@ -13,21 +12,10 @@ func (c *CLI) Ping(ctx context.Context, client api.HealthApi) error {
 	if c.TraceId != "" {
 		req = req.ZapTraceSpan(c.TraceId)
 	}
-	resp, _, err := client.GetHealthExecute(req)
+	if _, _, err := client.GetHealthExecute(req); err != nil {
-	if err != nil {
+		return err
-		return fmt.Errorf("failed to make health check request: %w", err)
 	}
 
-	if resp.Status == api.HEALTHCHECKSTATUS_FAIL {
+	_, err := c.StdIO.Write([]byte("OK\n"))
-		var message string
-		if resp.Message != nil {
-			message = *resp.Message
-		} else {
-			message = fmt.Sprintf("check %s failed", resp.Name)
-		}
-		return fmt.Errorf("health check failed: %s", message)
-	}
-
-	_, err = c.StdIO.Write([]byte("OK\n"))
 	return err
 }

internal/ping_test.go

@@ -70,7 +70,7 @@ func Test_PingFailedStatus(t *testing.T) {
 	e := "I broke"
 	client := &mock.HealthApi{
 		GetHealthExecuteFn: func(api.ApiGetHealthRequest) (api.HealthCheck, *http.Response, error) {
-			return api.HealthCheck{Status: api.HEALTHCHECKSTATUS_FAIL, Message: &e}, nil, nil
+			return api.HealthCheck{}, nil, &api.HealthCheck{Status: api.HEALTHCHECKSTATUS_FAIL, Message: &e}
 		},
 	}
 
@@ -86,7 +86,7 @@ func Test_PingFailedStatusNoMessage(t *testing.T) {
 	name := "foo"
 	client := &mock.HealthApi{
 		GetHealthExecuteFn: func(api.ApiGetHealthRequest) (api.HealthCheck, *http.Response, error) {
-			return api.HealthCheck{Status: api.HEALTHCHECKSTATUS_FAIL, Name: name}, nil, nil
+			return api.HealthCheck{}, nil, &api.HealthCheck{Status: api.HEALTHCHECKSTATUS_FAIL, Name: name}
 		},
 	}