Skip to content

Commit

Permalink
Treat empty entities as entities (#831)
Browse files Browse the repository at this point in the history
## Changes
To simplify codegen for Java, we'll start treating empty types as normal
entities in the OpenAPI code generator. The `(e *Entity) IsEmpty()` can
be used to determine if an entity is empty or not.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [ ] `make test` passing
- [ ] `make fmt` applied
- [ ] relevant integration tests applied
  • Loading branch information
mgyucht authored Feb 23, 2024
1 parent 56aea0b commit 4b1dee2
Show file tree
Hide file tree
Showing 45 changed files with 813 additions and 553 deletions.
16 changes: 10 additions & 6 deletions .codegen/api.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type {{.PascalName}}Interface interface {
{{range .Methods}}
{{- $hasWaiter := and .Wait (and (not .IsCrudRead) (not (eq .SnakeName "get_run"))) -}}
{{if not .Pagination}}{{.Comment "// " 80}}
{{.PascalName}}(ctx context.Context{{if .Request}}, {{if $hasWaiter}}{{.Request.CamelName}}{{else}}request{{end}} {{.Request.PascalName}}{{end}}) {{if $hasWaiter}}(*{{.Wait.PascalName}}{{with .Response}}[{{.PascalName}}]{{else}}[struct{}]{{end}}, error){{else}}{{ template "response-type" .}}{{end}}
{{.PascalName}}(ctx context.Context{{if .Request}}, {{if $hasWaiter}}{{.Request.CamelName}}{{else}}request{{end}} {{.Request.PascalName}}{{end}}) {{if $hasWaiter}}{{ template "wait-response-type" . }}{{else}}{{ template "response-type" .}}{{end}}
{{end}}

{{if $hasWaiter}}
Expand Down Expand Up @@ -218,14 +218,14 @@ func (w *{{.PascalName}}[R]) GetWithTimeout(timeout time.Duration) (*{{.Poll.Res
{{range .Methods}}
{{- $hasWaiter := and .Wait (and (not .IsCrudRead) (not (eq .SnakeName "get_run"))) -}}
{{if not .Pagination}}{{.Comment "// " 80}}
func (a *{{.Service.PascalName}}API) {{.PascalName}}(ctx context.Context{{if .Request}}, {{if $hasWaiter}}{{.Request.CamelName}}{{else}}request{{end}} {{.Request.PascalName}}{{end}}) {{if $hasWaiter}}(*{{.Wait.PascalName}}{{with .Response}}[{{.PascalName}}]{{else}}[struct{}]{{end}}, error){{else}}{{ template "response-type" .}}{{end}} {
func (a *{{.Service.PascalName}}API) {{.PascalName}}(ctx context.Context{{if .Request}}, {{if $hasWaiter}}{{.Request.CamelName}}{{else}}request{{end}} {{.Request.PascalName}}{{end}}) {{if $hasWaiter}}{{ template "wait-response-type" . }}{{else}}{{ template "response-type" .}}{{end}} {
{{if $hasWaiter -}}
{{if .Response}}{{.Response.CamelName}}, {{end}}err := a.impl.{{.PascalName}}(ctx{{if .Request}}, {{.Request.CamelName}}{{end}})
{{if not .Response.IsEmpty }}{{.Response.CamelName}}, {{end}}err := a.impl.{{.PascalName}}(ctx{{if .Request}}, {{.Request.CamelName}}{{end}})
if err != nil {
return nil, err
}
return &{{.Wait.PascalName}}{{with .Response}}[{{.PascalName}}]{{else}}[struct{}]{{end}}{
{{with .Response}}Response: {{.CamelName}},
return &{{.Wait.PascalName}}[{{with .Response}}{{if .IsEmpty}}struct{}{{else}}{{.PascalName}}{{end}}{{end}}]{
{{with .Response}}{{if not .IsEmpty}}Response: {{.CamelName}},{{end}}
{{- end}}{{range .Wait.Binding}}
{{.PollField.PascalName}}: {{.Bind.Of.CamelName}}.{{.Bind.PascalName}},{{end}}
Poll: func(timeout time.Duration, callback func(*{{.Wait.Poll.Response.PascalName}})) (*{{.Wait.Poll.Response.PascalName}}, error) {
Expand Down Expand Up @@ -423,4 +423,8 @@ func (a *{{.Service.Name}}API) {{.Shortcut.PascalName}}AndWait(ctx context.Conte
{{/* Note: we return the interface to make it possible to mock list responses more easily. */}}
{{- define "paginated-return-type" -}}
listing.Iterator[{{ template "type" .Pagination.Entity }}]
{{- end -}}
{{- end -}}

{{- define "wait-response-type" -}}
(*{{.Wait.PascalName}}[{{with .Response}}{{if .IsEmpty}}struct{}{{else}}{{.PascalName}}{{end}}{{end}}], error)
{{- end -}}
4 changes: 2 additions & 2 deletions .codegen/impl.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ func (a *{{.Service.CamelName}}Impl) {{.PascalName}}(ctx context.Context{{if .Re
{{- end }}

{{ define "response-type" -}}
{{ if .Response }}(
{{ if not .Response.IsEmpty }}(
{{- if not .Response.ArrayValue }}*{{end}}{{ template "type" .Response }}, error)
{{- else }}error
{{- end }}
{{- end }}

{{ define "response" -}}
{{ if .Response -}}
{{ if not .Response.IsEmpty -}}
{{ if not .Response.ArrayValue }}&{{end}}{{.Response.CamelName}}, {{end}}err
{{- end }}

Expand Down
10 changes: 4 additions & 6 deletions .codegen/model.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
"github.com/databricks/databricks-sdk-go/marshal"
)
{{range .Types}}
{{if .Fields -}}{{.Comment "// " 80}}
{{- if or .Fields .IsEmpty}}
{{.Comment "// " 80}}
type {{.PascalName}} struct {
{{- range .Fields}}
{{.Comment " // " 80}}
Expand Down Expand Up @@ -54,11 +55,8 @@ func (f *{{.PascalName}}) Set(v string) error {
// Type always returns {{.PascalName}} to satisfy [pflag.Value] interface
func (f *{{.PascalName}}) Type() string {
return "{{.PascalName}}"
}{{end}}
}
{{end}}
{{range .EmptyTypes}}
{{.Comment "// " 80}}
type {{.PascalName}} struct {}
{{end}}

{{- define "field-tag" -}}
Expand All @@ -70,6 +68,7 @@ type {{.PascalName}} struct {}

{{- define "type" -}}
{{- if not . }}any /* ERROR */
{{- else if .IsExternal }}{{.Package.Name}}.{{.PascalName}}
{{- else if .IsAny}}any
{{- else if .IsEmpty}}{{.PascalName}}
{{- else if .IsString}}string
Expand All @@ -80,7 +79,6 @@ type {{.PascalName}} struct {}
{{- else if .IsByteStream}}io.ReadCloser
{{- else if .ArrayValue }}[]{{template "type" .ArrayValue}}
{{- else if .MapValue }}map[string]{{template "type" .MapValue}}
{{- else if .IsExternal }}{{.Package.Name}}.{{.PascalName}}
{{- else if .IsObject }}{{.PascalName}}
{{- else if .Enum }}{{.PascalName}}
{{- else}}any /* MISSING TYPE */
Expand Down
17 changes: 16 additions & 1 deletion openapi/code/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ type Entity struct {
IsBool bool
IsString bool
IsByteStream bool
IsEmpty bool

// this field does not have a concrete type
IsAny bool
Expand Down Expand Up @@ -178,6 +177,22 @@ func (e *Entity) IsExternal() bool {
return e.Package != nil && len(e.Package.types) == 0
}

func (e *Entity) IsEmpty() bool {
return len(e.fields) == 0 &&
len(e.enum) == 0 &&
e.ArrayValue == nil &&
e.MapValue == nil &&
!e.IsInt &&
!e.IsInt64 &&
!e.IsFloat64 &&
!e.IsBool &&
!e.IsString &&
!e.IsByteStream &&
!e.IsAny &&
!e.IsComputed &&
!e.IsExternal()
}

func (e *Entity) RequiredFields() (fields []*Field) {
for _, r := range e.RequiredOrder {
v := e.fields[r]
Expand Down
4 changes: 2 additions & 2 deletions openapi/code/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestBasic(t *testing.T) {

require.Len(t, batch.Packages(), 1)
require.Len(t, batch.Services(), 1)
require.Len(t, batch.Types(), 17)
require.Len(t, batch.Types(), 19)
commands, ok := batch.packages["commands"]
require.True(t, ok)

Expand Down Expand Up @@ -60,7 +60,7 @@ func TestBasic(t *testing.T) {
assert.Equal(t, 0, len(wait.MessagePath()))

types := commands.Types()
assert.Equal(t, 17, len(types))
assert.Equal(t, 19, len(types))

command := types[2]
assert.Equal(t, "Command", command.PascalName())
Expand Down
3 changes: 1 addition & 2 deletions openapi/code/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ type Method struct {
// Request type representation
Request *Entity
// Response type representation
Response *Entity
EmptyResponseName Named
Response *Entity

// The style of the request, either "rpc" or "rest". See the documentation on
// Operation for more details.
Expand Down
42 changes: 11 additions & 31 deletions openapi/code/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type Package struct {
Components *openapi.Components
services map[string]*Service
types map[string]*Entity
emptyTypes []*Named
extImports map[string]*Entity
}

Expand Down Expand Up @@ -85,13 +84,6 @@ func (pkg *Package) Types() (types []*Entity) {
return types
}

// EmptyTypes returns sorted list of types without fields
func (pkg *Package) EmptyTypes() (types []*Named) {
types = append(types, pkg.emptyTypes...)
pascalNameSort(types)
return types
}

// HasPagination returns try if any service within this package has result
// iteration
func (pkg *Package) HasPagination() bool {
Expand Down Expand Up @@ -176,7 +168,7 @@ func (pkg *Package) schemaToEntity(s *openapi.Schema, path []string, hasName boo
e.Named.Name = strings.Join(path, "")
pkg.define(e)
}
e.IsEmpty = s.IsEmpty()
e.fields = map[string]*Field{}
e.IsAny = s.IsAny
e.IsComputed = s.IsComputed
e.RequiredOrder = s.Required
Expand Down Expand Up @@ -209,7 +201,6 @@ func (pkg *Package) schemaToEntity(s *openapi.Schema, path []string, hasName boo
// makeObject converts OpenAPI Schema into type representation
// processedEntities keeps track of the entities that are being generated to avoid infinite recursion.
func (pkg *Package) makeObject(e *Entity, s *openapi.Schema, path []string, processedEntities map[string]*Entity) *Entity {
e.fields = map[string]*Field{}
required := map[string]bool{}
for _, v := range s.Required {
required[v] = true
Expand Down Expand Up @@ -263,23 +254,20 @@ func (pkg *Package) localComponent(n *openapi.Node) string {
// definedEntity defines and returns the requested entity based on the schema.
// processedEntities keeps track of the entities that are being generated to avoid infinite recursion.
func (pkg *Package) definedEntity(name string, s *openapi.Schema, processedEntities map[string]*Entity) *Entity {
if s != nil {
if entity, ok := processedEntities[s.JsonPath]; ok {
// Return existing entity if it's already being generated.
return entity
}
}
if s == nil || s.IsEmpty() {
if s == nil {
entity := &Entity{
Named: Named{
Name: name,
Description: "",
},
IsEmpty: true,
fields: map[string]*Field{},
fields: map[string]*Field{},
}
return pkg.define(entity)
}
if entity, ok := processedEntities[s.JsonPath]; ok {
// Return existing entity if it's already being generated.
return entity
}

e := pkg.schemaToEntity(s, []string{name}, true, processedEntities)
if e == nil {
Expand All @@ -296,29 +284,21 @@ func (pkg *Package) definedEntity(name string, s *openapi.Schema, processedEntit
}

func (pkg *Package) define(entity *Entity) *Entity {
if entity.IsEmpty {
for _, e := range pkg.emptyTypes {
if e.PascalName() == entity.PascalName() {
return entity
}
}
pkg.emptyTypes = append(pkg.emptyTypes, &entity.Named)
return entity
}
_, defined := pkg.types[entity.Name]
k := entity.PascalName()
_, defined := pkg.types[k]
if defined {
//panic(fmt.Sprintf("%s is already defined", entity.Name))
return entity
}
if entity.Package == nil {
entity.Package = pkg
}
pkg.types[entity.Name] = entity
pkg.types[k] = entity
return entity
}

func (pkg *Package) updateType(entity *Entity) {
e, defined := pkg.types[entity.Name]
e, defined := pkg.types[entity.PascalName()]
if !defined {
return
}
Expand Down
35 changes: 4 additions & 31 deletions openapi/code/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ func (svc *Service) updateEntityTypeFromMimeType(entity *Entity, mimeType openap
// For request or response bodies that are not application/json, the body
// is modeled by a byte stream.
entity.IsByteStream = true
entity.IsEmpty = false
entity.IsAny = false
}

Expand Down Expand Up @@ -353,7 +352,7 @@ func (svc *Service) newRequest(params []openapi.Parameter, op *openapi.Operation
return request, mimeType, bodyField
}

func (svc *Service) newResponse(op *openapi.Operation) (*Entity, openapi.MimeType, *Field, Named, error) {
func (svc *Service) newResponse(op *openapi.Operation) (*Entity, openapi.MimeType, *Field, error) {
body := op.SuccessResponseBody(svc.Package.Components)
schema, mimeType := svc.getBaseSchemaAndMimeType(body)
name := op.Name()
Expand All @@ -378,41 +377,16 @@ func (svc *Service) newResponse(op *openapi.Operation) (*Entity, openapi.MimeTyp
}
}

// This next block of code is needed to make up for shortcomings in
// schemaToEntity. That function (and the Entity structure) assumes that all
// fields are part of the response schema. If we have fields part of the headers,
// we need to mark the response has non-empty and add it from the type map
if response.HasHeaderField() {
response.IsEmpty = false
svc.Package.define(response)
svc.removeFromEmptyList(response)
}

// We only support certain types of headers. Fail at build time if an unsupported type is found.
// We don't check this before because we need to ensure all referenced schemas have been defined.
if op.Responses["200"] != nil {
err := svc.validateHeaders(op.Responses["200"].Headers)
if err != nil {
return nil, "", nil, Named{}, err
return nil, "", nil, err
}
}

var emptyResponse Named
if response != nil && response.IsEmpty {
emptyResponse = response.Named
response = nil
}
return response, mimeType, bodyField, emptyResponse, nil
}

func (svc *Service) removeFromEmptyList(response *Entity) {
list := svc.Package.emptyTypes
for i, t := range list {
if t.Name == response.Name {
svc.Package.emptyTypes = append(list[:i], list[i+1:]...)
return
}
}
return response, mimeType, bodyField, nil
}

// ResponseHeaders are a map[string]*openapi.Parameter. The name is the key. This function converts
Expand Down Expand Up @@ -490,7 +464,7 @@ func (svc *Service) getPathStyle(op *openapi.Operation) openapi.PathStyle {
func (svc *Service) newMethod(verb, path string, params []openapi.Parameter, op *openapi.Operation) (*Method, error) {
methodName := op.Name()
request, reqMimeType, reqBodyField := svc.newRequest(params, op)
response, respMimeType, respBodyField, emptyResponse, err := svc.newResponse(op)
response, respMimeType, respBodyField, err := svc.newResponse(op)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -542,7 +516,6 @@ func (svc *Service) newMethod(verb, path string, params []openapi.Parameter, op
Request: request,
PathParts: svc.paramPath(path, request, params),
Response: response,
EmptyResponseName: emptyResponse,
PathStyle: requestStyle,
NameFieldPath: nameFieldPath,
IdFieldPath: idFieldPath,
Expand Down
2 changes: 1 addition & 1 deletion openapi/code/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (w *Wait) Binding() (binding []Binding) {
responseBind := true
bind := w.Method.wait.Bind
entity := w.Method.Response
if entity == nil {
if entity.IsEmpty() {
entity = w.Method.Request
responseBind = false
}
Expand Down
22 changes: 0 additions & 22 deletions openapi/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,28 +284,6 @@ func (s *Schema) IsArray() bool {
return s.ArrayValue != nil
}

func (s *Schema) IsEmpty() bool {
if s.IsMap() {
return false
}
if s.IsArray() {
return false
}
if s.IsObject() {
return false
}
if s.IsRef() {
return false
}
if s.IsAny {
return false
}
if s.Type == "object" || s.Type == "" {
return true
}
return false
}

type Parameter struct {
Node
Required bool `json:"required,omitempty"`
Expand Down
Loading

0 comments on commit 4b1dee2

Please sign in to comment.