Skip to content

Commit 6bf7157

Browse files
committed
Use an interface instead of the struct directly.
This can help with testing in implementing packages
1 parent 65b1d0f commit 6bf7157

18 files changed

+741
-346
lines changed

internal/generate/interfaces.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
package main
6+
7+
// Generate the Interfaces.go file.
8+
func generateInterfaces(file string, methods []methodTemplate) error {
9+
f, err := openGeneratedFile(file)
10+
if err != nil {
11+
return err
12+
}
13+
defer f.Close()
14+
15+
// build the Client interface
16+
f.WriteString("type Client interface {\n")
17+
for _, method := range methods {
18+
if method.IsListAll {
19+
f.WriteString("\t" + method.FunctionName + "(ctx context.Context, " + method.ParamsString + ") (" + method.ResponseType + ", error)\n")
20+
} else if method.ResponseType == "" {
21+
f.WriteString("\t" + method.FunctionName + "(ctx context.Context, " + method.ParamsString + ") error\n")
22+
} else {
23+
f.WriteString("\t" + method.FunctionName + "(ctx context.Context, " + method.ParamsString + ") (*" + method.ResponseType + ", error)\n")
24+
}
25+
}
26+
f.WriteString("}\n\n")
27+
28+
return nil
29+
}

internal/generate/main.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ func generateSDK() error {
4040
}
4141

4242
pathsFile := "../../oxide/paths.go"
43-
if err := generatePaths(pathsFile, spec); err != nil {
43+
methods, err := generatePaths(pathsFile, spec)
44+
if err != nil {
45+
return err
46+
}
47+
48+
interfacesFile := "../../oxide/interfaces.go"
49+
if err := generateInterfaces(interfacesFile, methods); err != nil {
4450
return err
4551
}
4652

internal/generate/main_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func Test_loadAPI(t *testing.T) {
2525
wantErr: "no such file or directory",
2626
},
2727
{
28-
name: "file does not exist",
28+
name: "invalid version",
2929
args: args{"generate/test_utils/INVALID_VERSION"},
3030
wantErr: "error loading openAPI spec from \"https://raw.githubusercontent.com/oxidecomputer/omicron//openapi/nexus.json\"",
3131
},

internal/generate/paths.go

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,15 @@ type paramsInfo struct {
4545
}
4646

4747
// Generate the paths.go file.
48-
func generatePaths(file string, spec *openapi3.T) error {
48+
func generatePaths(file string, spec *openapi3.T) ([]methodTemplate, error) {
4949
f, err := openGeneratedFile(file)
5050
if err != nil {
51-
return err
51+
return nil, err
5252
}
5353
defer f.Close()
5454

55+
methods := make([]methodTemplate, 0)
56+
5557
// Iterate over all the paths in the spec and write the methods.
5658
// We want to ensure we keep the order.
5759
keys := make([]string, 0)
@@ -66,78 +68,87 @@ func generatePaths(file string, spec *openapi3.T) error {
6668
continue
6769
}
6870

69-
err := buildPath(f, spec, path, p)
71+
tmpMethods, err := buildPath(f, spec, path, p)
7072
if err != nil {
71-
return err
73+
return nil, err
7274
}
75+
methods = append(methods, tmpMethods...)
7376
}
7477

75-
return nil
78+
return methods, nil
7679
}
7780

7881
// buildPath builds the given path as an http request to the given file.
79-
func buildPath(f *os.File, spec *openapi3.T, path string, p *openapi3.PathItem) error {
82+
func buildPath(f *os.File, spec *openapi3.T, path string, p *openapi3.PathItem) ([]methodTemplate, error) {
83+
methods := make([]methodTemplate, 0)
8084
if p.Get != nil {
81-
err := buildMethod(f, spec, http.MethodGet, path, p.Get, false)
85+
tmpMethods, err := buildMethod(f, spec, http.MethodGet, path, p.Get, false)
8286
if err != nil {
83-
return err
87+
return nil, err
8488
}
89+
methods = append(methods, tmpMethods...)
8590
}
8691

8792
if p.Post != nil {
88-
err := buildMethod(f, spec, http.MethodPost, path, p.Post, false)
93+
tmpMethods, err := buildMethod(f, spec, http.MethodPost, path, p.Post, false)
8994
if err != nil {
90-
return err
95+
return nil, err
9196
}
97+
methods = append(methods, tmpMethods...)
9298
}
9399

94100
if p.Put != nil {
95-
err := buildMethod(f, spec, http.MethodPut, path, p.Put, false)
101+
tmpMethods, err := buildMethod(f, spec, http.MethodPut, path, p.Put, false)
96102
if err != nil {
97-
return err
103+
return nil, err
98104
}
105+
methods = append(methods, tmpMethods...)
99106
}
100107

101108
if p.Delete != nil {
102-
err := buildMethod(f, spec, http.MethodDelete, path, p.Delete, false)
109+
tmpMethods, err := buildMethod(f, spec, http.MethodDelete, path, p.Delete, false)
103110
if err != nil {
104-
return err
111+
return nil, err
105112
}
113+
methods = append(methods, tmpMethods...)
106114
}
107115

108116
if p.Patch != nil {
109-
err := buildMethod(f, spec, http.MethodPatch, path, p.Patch, false)
117+
tmpMethods, err := buildMethod(f, spec, http.MethodPatch, path, p.Patch, false)
110118
if err != nil {
111-
return err
119+
return nil, err
112120
}
121+
methods = append(methods, tmpMethods...)
113122
}
114123

115124
if p.Head != nil {
116-
err := buildMethod(f, spec, http.MethodHead, path, p.Head, false)
125+
tmpMethods, err := buildMethod(f, spec, http.MethodHead, path, p.Head, false)
117126
if err != nil {
118-
return err
127+
return nil, err
119128
}
129+
methods = append(methods, tmpMethods...)
120130
}
121131

122132
if p.Options != nil {
123-
err := buildMethod(f, spec, http.MethodOptions, path, p.Options, false)
133+
tmpMethods, err := buildMethod(f, spec, http.MethodOptions, path, p.Options, false)
124134
if err != nil {
125-
return err
135+
return nil, err
126136
}
137+
methods = append(methods, tmpMethods...)
127138
}
128139

129-
return nil
140+
return methods, nil
130141
}
131142

132-
func buildMethod(f *os.File, spec *openapi3.T, method string, path string, o *openapi3.Operation, isGetAllPages bool) error {
143+
func buildMethod(f *os.File, spec *openapi3.T, method string, path string, o *openapi3.Operation, isGetAllPages bool) ([]methodTemplate, error) {
133144
respType, pagedRespType, err := getSuccessResponseType(o, isGetAllPages)
134145
if err != nil {
135-
return err
146+
return nil, err
136147
}
137148

138149
if len(o.Tags) == 0 || o.Tags[0] == "hidden" {
139150
fmt.Printf("[WARN] TODO: skipping operation %q, since it has no tag or is hidden\n", o.OperationID)
140-
return nil
151+
return nil, nil
141152
}
142153

143154
methodName := strcase.ToCamel(o.OperationID)
@@ -147,7 +158,7 @@ func buildMethod(f *os.File, spec *openapi3.T, method string, path string, o *op
147158

148159
if isGetAllPages {
149160
if pagedRespType == "" {
150-
return nil
161+
return nil, nil
151162
}
152163
}
153164

@@ -165,11 +176,11 @@ func buildMethod(f *os.File, spec *openapi3.T, method string, path string, o *op
165176

166177
pathParams, err := buildPathOrQueryParams("path", pInfo.parameters)
167178
if err != nil {
168-
return err
179+
return nil, err
169180
}
170181
queryParams, err := buildPathOrQueryParams("query", pInfo.parameters)
171182
if err != nil {
172-
return err
183+
return nil, err
173184
}
174185

175186
sanitisedDescription := strings.ReplaceAll(o.Description, "\n", "\n// ")
@@ -211,18 +222,22 @@ func buildMethod(f *os.File, spec *openapi3.T, method string, path string, o *op
211222
}
212223

213224
if err := writeTpl(f, config); err != nil {
214-
return err
225+
return nil, err
215226
}
216227

228+
methods := make([]methodTemplate, 0)
229+
methods = append(methods, config)
230+
217231
if pInfo.isPageResult && !isGetAllPages {
218232
// Run the method again with get all pages for ListAll methods.
219-
err := buildMethod(f, spec, method, path, o, true)
233+
allPagesMethods, err := buildMethod(f, spec, method, path, o, true)
220234
if err != nil {
221-
return err
235+
return nil, err
222236
}
237+
methods = append(methods, allPagesMethods...)
223238
}
224239

225-
return nil
240+
return methods, nil
226241
}
227242

228243
func getSuccessResponseType(o *openapi3.Operation, isGetAllPages bool) (string, string, error) {

internal/generate/paths_test.go

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,41 +14,69 @@ import (
1414

1515
func Test_generatePaths(t *testing.T) {
1616
file := "./test_utils/paths.json"
17-
pathsSpec, err := openapi3.NewLoader().LoadFromFile(file)
17+
spec, err := openapi3.NewLoader().LoadFromFile(file)
1818
if err != nil {
1919
t.Error(fmt.Errorf("error loading openAPI spec from %q: %v", file, err))
2020
}
2121

2222
type args struct {
23-
file string
24-
spec *openapi3.T
23+
pathsFile string
24+
interfacesFile string
25+
spec *openapi3.T
2526
}
2627
tests := []struct {
2728
name string
2829
args args
2930
wantErr string
3031
}{
3132
{
32-
name: "fail on non-existent file",
33-
args: args{"sdf/gdsf", pathsSpec},
33+
name: "fail on non-existent paths file",
34+
args: args{
35+
pathsFile: "sdf/gdsf",
36+
interfacesFile: "sdf/gdsf",
37+
spec: spec,
38+
},
39+
wantErr: "no such file or directory",
40+
},
41+
{
42+
name: "fail on non-existent interfaces file",
43+
args: args{
44+
pathsFile: "test_utils/paths_output",
45+
interfacesFile: "sdf/gdsf",
46+
spec: spec,
47+
},
3448
wantErr: "no such file or directory",
3549
},
3650
{
3751
name: "success",
38-
args: args{"test_utils/paths_output", pathsSpec},
52+
args: args{
53+
pathsFile: "test_utils/paths_output",
54+
interfacesFile: "test_utils/interfaces_output",
55+
spec: spec,
56+
},
3957
},
4058
}
4159
for _, tt := range tests {
4260
t.Run(tt.name, func(t *testing.T) {
4361
// TODO: For now the test is not properly generating the "ListAll" methods
4462
// This is because there is a separate check to the response type. The way this works
4563
// should be changed
46-
if err := generatePaths(tt.args.file, tt.args.spec); err != nil {
64+
methods, err := generatePaths(tt.args.pathsFile, tt.args.spec)
65+
if err != nil {
66+
assert.ErrorContains(t, err, tt.wantErr)
67+
return
68+
}
69+
70+
if err := generateInterfaces(tt.args.interfacesFile, methods); err != nil {
4771
assert.ErrorContains(t, err, tt.wantErr)
4872
return
4973
}
5074

51-
if err := compareFiles("test_utils/paths_output_expected", tt.args.file); err != nil {
75+
if err := compareFiles("test_utils/paths_output_expected", tt.args.pathsFile); err != nil {
76+
t.Error(err)
77+
}
78+
79+
if err := compareFiles("test_utils/interfaces_output_expected", tt.args.interfacesFile); err != nil {
5280
t.Error(err)
5381
}
5482
})

internal/generate/templates/listall_method.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{template "description" .}}func (c *Client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) ({{.ResponseType}}, error) { {{if .HasParams}}
1+
{{template "description" .}}func (c *client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) ({{.ResponseType}}, error) { {{if .HasParams}}
22
if err := params.Validate(); err != nil {
33
return nil, err
44
}{{end}}

internal/generate/templates/no_resptype_body_method.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{template "description" .}}func (c *Client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) error { {{if .HasParams}}
1+
{{template "description" .}}func (c *client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) error { {{if .HasParams}}
22
if err := params.Validate(); err != nil {
33
return err
44
}{{end}}{{if .IsAppJSON}}

internal/generate/templates/no_resptype_method.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{template "description" .}}func (c *Client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) error { {{if .HasParams}}
1+
{{template "description" .}}func (c *client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) error { {{if .HasParams}}
22
if err := params.Validate(); err != nil {
33
return err
44
}{{end}}

internal/generate/templates/resptype_body_method.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{template "description" .}}func (c *Client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) (*{{.ResponseType}}, error) { {{if .HasParams}}
1+
{{template "description" .}}func (c *client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) (*{{.ResponseType}}, error) { {{if .HasParams}}
22
if err := params.Validate(); err != nil {
33
return nil, err
44
}{{end}}{{if .IsAppJSON}}

internal/generate/templates/resptype_method.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{template "description" .}}func (c *Client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) (*{{.ResponseType}}, error) { {{if .HasParams}}
1+
{{template "description" .}}func (c *client) {{.FunctionName}}(ctx context.Context, {{.ParamsString}}) (*{{.ResponseType}}, error) { {{if .HasParams}}
22
if err := params.Validate(); err != nil {
33
return nil, err
44
}{{end}}

0 commit comments

Comments
 (0)