Skip to content

Go struct members pointers #3241

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@

import org.openapitools.codegen.CliOption;
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.CodegenModel;
import org.openapitools.codegen.CodegenProperty;
import org.openapitools.codegen.CodegenType;
import org.openapitools.codegen.SupportingFile;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.util.List;
import java.util.Map;

public class GoClientCodegen extends AbstractGoCodegen {

Expand Down Expand Up @@ -99,6 +103,7 @@ public void processOpts() {
supportingFiles.add(new SupportingFile("go.mod.mustache", "", "go.mod"));
supportingFiles.add(new SupportingFile("go.sum", "", "go.sum"));
supportingFiles.add(new SupportingFile(".travis.yml", "", ".travis.yml"));
supportingFiles.add(new SupportingFile("utils.mustache", "", "utils.go"));

if (additionalProperties.containsKey(WITH_GO_CODEGEN_COMMENT)) {
setWithGoCodegenComment(Boolean.parseBoolean(additionalProperties.get(WITH_GO_CODEGEN_COMMENT).toString()));
Expand All @@ -122,6 +127,32 @@ public void processOpts() {
}
}

@Override
public Map<String, Object> postProcessModels(Map<String, Object> objs) {
objs = super.postProcessModels(objs);
List<Map<String, String>> imports = (List<Map<String, String>>) objs.get("imports");

boolean addedErrorsImport = false;
List<Map<String, Object>> models = (List<Map<String, Object>>) objs.get("models");
for (Map<String, Object> m : models) {
Object v = m.get("model");
if (v instanceof CodegenModel) {
CodegenModel model = (CodegenModel) v;
if (!model.isEnum) {
imports.add(createMapping("import", "encoding/json"));
}
for (CodegenProperty param : model.vars) {
if (!addedErrorsImport && param.required) {
imports.add(createMapping("import", "errors"));
addedErrorsImport = true;
}
}
}
}

return objs;
}

/**
* Configures the type of generator.
*
Expand Down
16 changes: 16 additions & 0 deletions modules/openapi-generator/src/main/resources/go/README.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,22 @@ r, err := client.Service.Operation(auth, args)
{{/isOAuth}}
{{/authMethods}}

## Documentation for Utility Methods

Due to the fact that model structure members are all pointers, this package contains
a number of utility functions to easily obtain pointers to values of basic types.
Each of these functions takes a value of the given basic type and returns a pointer to it:

* `PtrBool`
* `PtrInt`
* `PtrInt32`
* `PtrInt64`
* `PtrFloat`
* `PtrFloat32`
* `PtrFloat64`
* `PtrString`
* `PtrTime`

## Author

{{#apiInfo}}{{#apis}}{{^hasMore}}{{infoEmail}}
Expand Down
88 changes: 87 additions & 1 deletion modules/openapi-generator/src/main/resources/go/model.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,95 @@ type {{classname}} struct {
{{#description}}
// {{{description}}}
{{/description}}
{{name}} {{#isNullable}}*{{/isNullable}}{{{dataType}}} `json:"{{baseName}}{{^required}},omitempty{{/required}}"{{#withXml}} xml:"{{baseName}}{{#isXmlAttribute}},attr{{/isXmlAttribute}}"{{/withXml}}{{#vendorExtensions.x-go-custom-tag}} {{{.}}}{{/vendorExtensions.x-go-custom-tag}}`
{{name}} *{{{dataType}}} `json:"{{baseName}},omitempty"{{#withXml}} xml:"{{baseName}}{{#isXmlAttribute}},attr{{/isXmlAttribute}}"{{/withXml}}{{#vendorExtensions.x-go-custom-tag}} {{{.}}}{{/vendorExtensions.x-go-custom-tag}}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of adding a field isExplicitNull to mark it explicity? as a convention of go, it's a nullable if it's a pointer, otoh, a non-pointer field means it's a non-optional field

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is to be able to have a field that can be sent with null as a value. The whole point of this PR is that it's very hard in Go to have fields that allow setting either a zero value (0, false, empty string) or a null explicitly. This PR allows for various combinations of all of these features thanks to isExplicitNull and the custom marshalling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that {{#isNullable}} ... {{/isNullable}} has been removed. Does it mean model properties with or without nullable set to true are handled the same way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are handled explicitly in the custom marshalling method, see https://github.com/OpenAPITools/openapi-generator/pull/3241/files#diff-6403a68f6b1039398715805f9043d0a1R95 and following lines.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, it's breaking backward-compatibility. the field shouldn't twiddle between pointer and non-pointer at least

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've had a discussion about this with @wing328 and we'll probably create another go client generator to accept this change (and possibly other breaking changes) that would otherwise be unacceptable for 4.1.x and would have to wait for 5.0.0 - which would be a loooong time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkabrda For the new go generator, would you mind tagging it with "Experimental"

It would look like this in the new generator's constructor:

        generatorMetadata = GeneratorMetadata.newBuilder(generatorMetadata)
                .stability(Stability.EXPERIMENTAL)
               .build();

This will allow us to tag/filter generators to clearly present which ones are considered stable to users.

{{#isNullable}} isExplicitNull{{name}} bool `json:"-"{{#withXml}} xml:"-"{{/withXml}}`{{/isNullable}}
{{/vars}}
}
{{/isEnum}}

{{^isEnum}}
{{#vars}}
// Get{{name}} returns the {{name}} field if non-nil, zero value otherwise.
func (o *{{classname}}) Get{{name}}() {{dataType}} {
if o == nil || o.{{name}} == nil {
var ret {{dataType}}
return ret
}
return *o.{{name}}
}

// Get{{name}}Ok returns a tuple with the {{name}} field if it's non-nil, zero value otherwise
// and a boolean to check if the value has been set.
func (o *{{classname}}) Get{{name}}Ok() ({{dataType}}, bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name doesn't really sonuds ok.. in favor of others e.g. Get{{name}}IfExists..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer Get{{name}}Ok, since it returns two values - the value of {{name}} and whether or not retrieving it was ok. With IfExists, I would expect the function to return only the value of {{name}} if it exists. YMMV.

if o == nil || o.{{name}} == nil {
var ret {{dataType}}
return ret, false
}
return *o.{{name}}, true
}

// Has{{name}} returns a boolean if a field has been set.
func (o *{{classname}}) Has{{name}}() bool {
if o != nil && o.{{name}} != nil {
return true
}

return false
}

// Set{{name}} gets a reference to the given {{dataType}} and assigns it to the {{name}} field.
func (o *{{classname}}) Set{{name}}(v {{dataType}}) {
o.{{name}} = &v
}

{{#isNullable}}
// Set{{name}}ExplicitNull (un)sets {{name}} to be considered as explicit "null" value
// when serializing to JSON (pass true as argument to set this, false to unset)
// The {{name}} value is set to nil even if false is passed
func (o *{{classname}}) Set{{name}}ExplicitNull(b bool) {
o.{{name}} = nil
o.isExplicitNull{{name}} = b
}
{{/isNullable}}
{{/vars}}

func (o {{classname}}) MarshalJSON() ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. this should belong to another pull, discriminating the change..
  2. the custom marshal has to be a opt-in extension..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. As noted in one of the comments above, the custom marshalling is a core of this change without which the rest wouldn't work reasonably.
  2. Why?

toSerialize := map[string]interface{}{}
{{#vars}}
{{#required}}
{{! if argument is required and not nullable, it can't be nil}}
{{^isNullable}}
if o.{{name}} == nil {
return nil, errors.New("{{name}} is required and not nullable, but was not set on {{classname}}")
}{{/isNullable}}
{{! if argument is required and nullable, it *must* have isExplicitNull<name> set to true when it's nil}}
{{#isNullable}}
if o.{{name}} == nil && !o.isExplicitNull{{name}} {
return nil, errors.New("{{name}} is required and nullable, but it wasn't set to be explicit null")
}
{{/isNullable}}
{{/required}}
{{! if argument is nullable, only serialize it if it is nil *and* was explicitly set to nil}}
{{#isNullable}}
if o.{{name}} == nil {
if o.isExplicitNull{{name}} {
toSerialize["{{baseName}}"] = o.{{name}}
}
} else {
toSerialize["{{baseName}}"] = o.{{name}}
}
{{/isNullable}}
{{! if argument is not nullable, don't set it if it is nil}}
{{^isNullable}}
if o.{{name}} != nil {
toSerialize["{{baseName}}"] = o.{{name}}
}
{{/isNullable}}
{{/vars}}
return json.Marshal(toSerialize)
}

{{/isEnum}}

{{/model}}
{{/models}}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,50 @@

Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
{{#vars}}**{{name}}** | {{#isNullable}}Pointer to {{/isNullable}}{{#isPrimitiveType}}**{{{dataType}}}**{{/isPrimitiveType}}{{^isPrimitiveType}}[**{{{dataType}}}**]({{complexType}}.md){{/isPrimitiveType}} | {{description}} | {{^required}}[optional] {{/required}}{{#readOnly}}[readonly] {{/readOnly}}{{#defaultValue}}[default to {{{.}}}]{{/defaultValue}}
{{#vars}}**{{name}}** | Pointer to {{#isPrimitiveType}}**{{{dataType}}}**{{/isPrimitiveType}}{{^isPrimitiveType}}[**{{{dataType}}}**]({{complexType}}.md){{/isPrimitiveType}} | {{description}} | {{^required}}[optional] {{/required}}{{#readOnly}}[readonly] {{/readOnly}}{{#defaultValue}}[default to {{{.}}}]{{/defaultValue}}
{{/vars}}

{{^isEnum}}
## Methods

{{#vars}}
### Get{{name}}

`func (o *{{classname}}) Get{{name}}() {{dataType}}`

Get{{name}} returns the {{name}} field if non-nil, zero value otherwise.

### Get{{name}}Ok

`func (o *{{classname}}) Get{{name}}Ok() ({{dataType}}, bool)`

Get{{name}}Ok returns a tuple with the {{name}} field if it's non-nil, zero value otherwise
and a boolean to check if the value has been set.

### Has{{name}}

`func (o *{{classname}}) Has{{name}}() bool`

Has{{name}} returns a boolean if a field has been set.

### Set{{name}}

`func (o *{{classname}}) Set{{name}}(v {{dataType}})`

Set{{name}} gets a reference to the given {{dataType}} and assigns it to the {{name}} field.

{{#isNullable}}
### Set{{name}}ExplicitNull

`func (o *{{classname}}) Set{{name}}ExplicitNull(b bool)`

Set{{name}}ExplicitNull (un)sets {{name}} to be considered as explicit "null" value
when serializing to JSON (pass true as argument to set this, false to unset)
The {{name}} value is set to nil even if false is passed
{{/isNullable}}
{{/vars}}
{{/isEnum}}

[[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)

{{/model}}{{/models}}
31 changes: 31 additions & 0 deletions modules/openapi-generator/src/main/resources/go/utils.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{{>partial_header}}
package {{packageName}}

import "time"

// PtrBool is a helper routine that returns a pointer to given integer value.
func PtrBool(v bool) *bool { return &v }

// PtrInt is a helper routine that returns a pointer to given integer value.
func PtrInt(v int) *int { return &v }

// PtrInt32 is a helper routine that returns a pointer to given integer value.
func PtrInt32(v int32) *int32 { return &v }

// PtrInt64 is a helper routine that returns a pointer to given integer value.
func PtrInt64(v int64) *int64 { return &v }

// PtrFloat is a helper routine that returns a pointer to given float value.
func PtrFloat(v float32) *float32 { return &v }

// PtrFloat32 is a helper routine that returns a pointer to given float value.
func PtrFloat32(v float32) *float32 { return &v }

// PtrFloat64 is a helper routine that returns a pointer to given float value.
func PtrFloat64(v float64) *float64 { return &v }

// PtrString is a helper routine that returns a pointer to given string value.
func PtrString(v string) *string { return &v }

// PtrTime is helper routine that returns a pointer to given Time value.
func PtrTime(v time.Time) *time.Time { return &v }
30 changes: 18 additions & 12 deletions samples/client/petstore/go/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ func TestOAuth2(t *testing.T) {
tokenSource := cfg.TokenSource(createContext(nil), &tok)
auth := context.WithValue(context.Background(), sw.ContextOAuth2, tokenSource)

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(context.Background(), newPet)

Expand Down Expand Up @@ -71,8 +72,9 @@ func TestBasicAuth(t *testing.T) {
Password: "f4k3p455",
})

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(auth, newPet)

Expand Down Expand Up @@ -100,8 +102,9 @@ func TestBasicAuth(t *testing.T) {
func TestAccessToken(t *testing.T) {
auth := context.WithValue(context.Background(), sw.ContextAccessToken, "TESTFAKEACCESSTOKENISFAKE")

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(nil, newPet)

Expand Down Expand Up @@ -129,8 +132,9 @@ func TestAccessToken(t *testing.T) {
func TestAPIKeyNoPrefix(t *testing.T) {
auth := context.WithValue(context.Background(), sw.ContextAPIKey, sw.APIKey{Key: "TEST123"})

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(context.Background(), newPet)

Expand Down Expand Up @@ -163,8 +167,9 @@ func TestAPIKeyNoPrefix(t *testing.T) {
func TestAPIKeyWithPrefix(t *testing.T) {
auth := context.WithValue(context.Background(), sw.ContextAPIKey, sw.APIKey{Key: "TEST123", Prefix: "Bearer"})

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(nil, newPet)

Expand Down Expand Up @@ -196,8 +201,9 @@ func TestAPIKeyWithPrefix(t *testing.T) {

func TestDefaultHeader(t *testing.T) {

newPet := (sw.Pet{Id: 12992, Name: "gopher",
PhotoUrls: []string{"http://1.com", "http://2.com"}, Status: "pending", Tags: []sw.Tag{sw.Tag{Id: 1, Name: "tag2"}}})
newPet := (sw.Pet{Id: sw.PtrInt64(12992), Name: sw.PtrString("gopher"),
PhotoUrls: &[]string{"http://1.com", "http://2.com"}, Status: sw.PtrString("pending"),
Tags: &[]sw.Tag{sw.Tag{Id: sw.PtrInt64(1), Name: sw.PtrString("tag2")}}})

r, err := client.PetApi.AddPet(context.Background(), newPet)

Expand Down
4 changes: 2 additions & 2 deletions samples/client/petstore/go/fake_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ func TestPutBodyWithFileSchema(t *testing.T) {
return // early return to test compilation

schema := sw.FileSchemaTestClass{
File: sw.File{SourceURI: "https://example.com/image.png"},
Files: []sw.File{{SourceURI: "https://example.com/image.png"}}}
File: &sw.File{SourceURI: sw.PtrString("https://example.com/image.png")},
Files: &[]sw.File{{SourceURI: sw.PtrString("https://example.com/image.png")}}}

r, err := client.FakeApi.TestBodyWithFileSchema(context.Background(), schema)

Expand Down
16 changes: 16 additions & 0 deletions samples/client/petstore/go/go-petstore-withXml/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,22 @@ r, err := client.Service.Operation(auth, args)
```


## Documentation for Utility Methods

Due to the fact that model structure members are all pointers, this package contains
a number of utility functions to easily obtain pointers to values of basic types.
Each of these functions takes a value of the given basic type and returns a pointer to it:

* `PtrBool`
* `PtrInt`
* `PtrInt32`
* `PtrInt64`
* `PtrFloat`
* `PtrFloat32`
* `PtrFloat64`
* `PtrString`
* `PtrTime`

## Author


Expand Down
Loading