Skip to content

[REQ][Go Client] Support circular schema definitions #5737

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

Open
grokify opened this issue Mar 28, 2020 · 4 comments
Open

[REQ][Go Client] Support circular schema definitions #5737

grokify opened this issue Mar 28, 2020 · 4 comments

Comments

@grokify
Copy link
Member

grokify commented Mar 28, 2020

Is your feature request related to a problem? Please describe.

Valid OpenAPI specs can have circular definitions, however, this is not currently supported in generated Go client SDKs.

Validity of OpenAPI circular definitions is discussed here:

Such definitions will also validate on the OAS3 validators by Mermade and APIDevTools.

Currently, when such a spec is used, the invalid recursive type compile error occurs.

# github.com/grokify/go-ringcentral/codegen/engagevoice
../../../../codegen/engagevoice/model_agent_account_access.go:12:6: invalid recursive type AgentAccountAccess
../../../../codegen/engagevoice/model_campaign.go:12:6: invalid recursive type Campaign
../../../../codegen/engagevoice/model_custom_dial_zone_groups.go:12:6: invalid recursive type CustomDialZoneGroups
../../../../codegen/engagevoice/model_quota_target.go:12:6: invalid recursive type QuotaTarget

Describe the solution you'd like

The problem with circular definitions occurs in Go because the compiler cannot determine the size of such a struct. The solution is to replace the nested struct with a pointer, who's size is known, as described here:

A pointer's size is known, but how big is something that contains itself? (And the inner struct contains itself as well, as does the inner inner struct, and so on.)

https://stackoverflow.com/a/8261789/1908967

This has been confirmed to compile by manually updating the generated model files.

Describe alternatives you've considered

None yet.

Additional context

  1. The use of pointers for nested structs is also being discussed in various JSON null value threads.
    1. [Go] Optional values should be pointers #522
    2. [BUG][go-experimental] Explicit nulls support is broken #5278
  2. Google's auto-generated Go client SDKs appear to always use pointers for nested structs - https://github.com/googleapis/google-api-go-client

Given how pointers are part of a long-running nullable discussion, one thought is to use a CLI parameter that will make all nested structs pointers as to make it an optional feature.

@grokify grokify changed the title [REQ][Go] Support circular schema definitions [REQ][Go Client] Support circular schema definitions Mar 28, 2020
@wing328
Copy link
Member

wing328 commented Mar 29, 2020

@grokify I wonder if you've tried the go-experimental generator to see whether it has the same issue.

@grokify
Copy link
Member Author

grokify commented Mar 29, 2020

@wing328 Not yet, but I'll check it out.

For interim use, I've created the following simple post processor. I'll look into this more after the current project ships.

@jonasknobloch
Copy link

jonasknobloch commented Nov 12, 2020

Ran into this issue using the current go generator (go-experimental at the time this issue was created). Fixed it by manually updating the generated model files.

@tony-scio
Copy link

We've been hacking the generated files with a simple minded shell script to add pointers. @grokify thanks for the more robust hack than ours. Would be great to see this fixed in the generator though.

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

No branches or pull requests

4 participants