Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go codegen using joinSchema creates an empty interface #124

Closed
IfSentient opened this issue Apr 3, 2023 · 5 comments · Fixed by #127
Closed

Go codegen using joinSchema creates an empty interface #124

IfSentient opened this issue Apr 3, 2023 · 5 comments · Fixed by #127
Assignees

Comments

@IfSentient
Copy link
Contributor

If I have a field which is declared by both the joinSchema and the actual schema in a lineage, the go codegen for that field selector ends up being an empty interface. I logged the actual cue value at the selector, and it has all the combined fields, so I'm not 100% sure what's happening behind the scenes. Example:

lin: thema.#Lineage & {
    name: "foo"
    joinSchema: {
        bar: {
            joinField: string
            ...
        }
    }
    seqs: [{
        schemas: [{
            bar: {
                schemaField: string
            }
        }]
    }]
}

Logging the full schema's cue.Value before passing it off to the theme codegen:

{
    bar: {
        joinField: string
        schemaField: string
    }
}

I then use

gocode.GenerateTypesOpenAPI(sch, &gocode.TypeConfigOpenAPI{
	Config: &openapi.Config{
		RootName: "foo",
		Subpath:  cue.MakePath(cue.Str("bar")),
	},
	PackageName: "foo",
	ApplyFuncs:  []dstutil.ApplyFunc{codegen.PrefixDropper(meta.Name)},
})

The file contents I get back are:

package foo

// Bar defines model for Bar.
type Bar = interface{}

Rather than the expected

type Bar struct {
    JoinField       string `json:"joinField"`
    SchemaField string `json:"schemaField"`
@IfSentient
Copy link
Contributor Author

Some more info after messing around trying to bypass this issue by hacking the values in the CUE linage at runtime before the codegen: using the cue.Value.FillPath on a schema in the lineage runs into the same issue. When printing out the whole lineage's value with Format, I noticed the FillPath adds the value by doing a standard join in the CUE ({} & {}), and the codegen still results in an empty interface, much like the joinSchema problem.

Example:

	metadataVal := g.rt.Context().CompileString(`{
	uid: string
	finalizers: [...string]
}`)
	sch := parsedLin.First()
	for sch != nil {
		v = v.FillPath(cue.MakePath(cue.Str("lineage"), cue.Str("seqs"), cue.Index(int(sch.Version()[0])), cue.Str("schemas"), cue.Index(int(sch.Version()[1])), cue.Str("metadata")), metadataVal)
		sch = sch.Successor()
	}
	st := cueFmtState{} // have to use this with v.Format, because a fmt.Println(v) doesn't show the whole picture
	v.Format(&st, 'v')
	fmt.Println("under: ", string(st.Bytes()))

Standard out:

lineage:  thema.#Lineage & {
	name: "issue"
	seqs: [{
		schemas: [{
			spec: {
				title:       string
				description: string
				status:      string
			}
			metadata: {
				foo: string
			}
			status: {
				foo: string
			}
		}]
	}] & [{
		schemas: [{
			metadata?: {
				uid: string
				finalizers: [...string]
			}
		}, ...]
	}, ...]
}

This is similar to printing the value from the joinSchema, within encoding/gocode/openapi/oapi.go:GenerateSchema
Same input, but instead using a joinSchema of:

{
	metadata: {
		uid: string
		finalizers: [...string]
	}
	spec: {...}
	[string]: {...}
}

With a schema of:

{
	spec: {
		title: string
		description: string
		status: string
		foo: [...string]
	}
	metadata: {
		foo: string
	}
	status: {
		foo: string
	}
}

Produces this Format output for the lineage:

{
	metadata: {
		uid: string
		finalizers: [...string]
	}
	spec: {
		...
	}
	[string]: {
		...
	}
} & {
	spec: {
		title:       string
		description: string
		status:      string
		foo: [...string]
	}
	metadata: {
		foo: string
	}
	status: {
		foo: string
	}
}

So it seems likely that the problem comes from having joined objects in the codegen. Interestingly, doing a Format on just the metadata field (the new under in GenerateSchema) produces:

{
	uid: string
	foo: string
	finalizers: [...string]
	...
}

So it seems possible that the problem is actually in the cue openapi.Generate function. Will do some additional investigation.

@IfSentient
Copy link
Contributor Author

OK, so the problem comes down to this: the openAPI encoder merges the properties, but not the required parts. This generates an openAPI YAML that looks like this:

openapi: 3.0.0
info:
  title: Metadata
  version: "0.0"
paths: {}
components:
  schemas:
    Metadata:
      type: object
      properties:
        uid:
          type: string
        finalizers:
          type: array
          items:
            type: string
        foo:
          description: 'bar: bool'
          type: string
      allOf:
        - required:
            - uid
            - finalizers
        - required:
            - foo

It doesn't seem 100% clear to me from the openAPI docs if this is a correct usage of allOf, but it does render properly in the swagger editor. In any case, deepmap/oapi-codegen treats allOf as always containing multiple whole schemas, not being a partial, so we end up with an empty schema, which gives us the interface{}.

@IfSentient
Copy link
Contributor Author

It looks like there is an open issue and related PR in deepmap/oapi-codegen to resolve this issue with allOf. The diff in that PR addresses the issue with the thema go codegen.

@sdboyer
Copy link
Contributor

sdboyer commented Apr 11, 2023

One quick comment - it occurs to me that we could resolve this issue, by deciding #76 in the direction of saying that joinSchema is always, only the equivalent of must(). Then, the author has to explicitly write those joinSchema fields.

This would make lineage authoring less magical, which i think could improve UX.

@IfSentient IfSentient self-assigned this Apr 11, 2023
@sdboyer
Copy link
Contributor

sdboyer commented Apr 11, 2023

I think this is probably the straw that leads us to fork oapi-codegen. Best to do it in an internal directory within thema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants