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] optional boolean properties set to false are omitted when encoded #7391

Open
drennalls opened this issue Jan 12, 2018 · 13 comments · May be fixed by swagger-api/swagger-codegen-generators#1275

Comments

@drennalls
Copy link

Description

If an object defines an optional boolean property it does not end up in the JSON if the value is false

Swagger-codegen version

2.2.2

Swagger declaration file content or url
swagger: '2.0'
info:
  description: 'Demonstrate golang codegen bug'
  version: 1.0.0
schemes:
  - http
paths:
  /pet:
    post:
      summary: Add a new pet to the store
      description: ''
      operationId: addPet
      consumes:
        - application/json
      produces:
        - application/json
      parameters:
        - in: body
          name: body
          description: Pet object that needs to be added to the store
          required: true
          schema:
            $ref: '#/definitions/Pet'
      responses:
        '405':
          description: Invalid input
definitions:
  Pet:
    title: a Pet
    description: A pet for sale in the pet store
    type: object
    required:
      - name
    properties:
      id:
        type: integer
        format: int64
      name:
        type: string
        example: doggie
      vaccinated:
        type: boolean
        description: Indicates whether the Pet's vaccinations are up-to-date
Command line used for generation

java -jar target/swagger-codegen-cli.jar generate -i ./mypetstore.yaml -l go -o ./mypetstore-client-go

Steps to reproduce

Include an optional boolean property (like vaccinated in the API above), set to false and encode it. The field will be missing from the JSON.

Related issues/PRs

If the property is marked as required then things work as expected which I guess was a result of #4665

Suggest a fix/enhancement

Don't use omitempty for boolean properties, even if optional...
from https://golang.org/pkg/encoding/json/

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

..if you do then sending a false for optional properties is not possible.

@wing328
Copy link
Contributor

wing328 commented Jan 16, 2018

@drennalls Can you please try the latest stable version (2.3.0), which has the Go API client refactored?

@luism6n
Copy link

luism6n commented Jan 31, 2018

This happens in version 2.4.0-SNAPSHOT. Reproduced as follows:

Got petstore.yaml

And add:

...
      vaccinated:
        type: boolean
        description: indicate if pet is vaccinated
...

To the Pet object definition.

Generate the client:

$ ./run-in-docker.sh generate -i issue7391.yaml -l go -o /gen/out/issue7391 -DpackageName=issue7391
$ ln -s ${PWD}/out $(go env GOPATH)/src/out

Create a dummy server:

from flask import Flask, request

app = Flask(__name__)

@app.route('/pet', methods=['POST'])
def root():
    content = request.json
    print content
    return "all right"

app.run(port=8080)

Run a Go test:

package main

import (
	"fmt"
	"out/issue7391"
	"testing"
)

func TestPost(t *testing.T) {
	config := issue7391.NewConfiguration()
	config.BasePath = "http://localhost:8080"
	client := issue7391.NewAPIClient(config)

	pet := issue7391.Pet{
		Name: "john",
		// Vaccinated: false,
	}

	res, err := client.PetApi.AddPet(nil, pet)
	fmt.Printf("%+v\n%+v\n", res, err)
}

Running with or without commenting on that line, the server logs:

{u'name': u'john', u'photoUrls': None}
127.0.0.1 - - [31/Jan/2018 11:59:48] "POST /pet HTTP/1.1" 200 -

Manually changing the model_pet.go generated from:

...
	Vaccinated bool `json:"vaccinated,omitempty"`
...

to

...
	Vaccinated bool `json:"vaccinated"`
...

Again, running with or without commenting the Vaccinated: false line, the server logs:

127.0.0.1 - - [31/Jan/2018 11:59:58] "POST /pet HTTP/1.1" 200 -
{u'vaccinated': False, u'name': u'john', u'photoUrls': None}

So removing omitempty won't fix the problem, since the property will always be false if empty. Surprisingly, changing the property to:

...
	Vaccinated *bool `json:"vaccinated"`
...

Actualy works as intended. Maybe it's an option?

/cc @drennalls @wing328

@wing328
Copy link
Contributor

wing328 commented Jan 31, 2018

I thought removing omitempty would work. Can you also share the Go version you're using?

cc @antihax @bvwells

@luism6n
Copy link

luism6n commented Feb 1, 2018

Sure:

$ go version
go version go1.9.3 linux/amd64

@tenmozes
Copy link

@drennalls hello, setting default value should help

    vaccinated:
        type: boolean
        default:false
        description: indicate if pet is vaccinated

@wing328 wing328 modified the milestones: v2.4.0, Future Mar 16, 2018
@kevinmu
Copy link

kevinmu commented Mar 25, 2019

I seem to be running into this issue as well. Providing a default as @tenmozes suggested yields

 Vaccinated *bool `json:"vaccinated,omitempty"`

which does work. However, my question is whether we should also consider using this style even when a default is NOT provided?

Personally, I can't think of an instance where the existing

 Vaccinated bool `json:"vaccinated,omitempty"`

would be preferred over the pointer version that is generated when you provide a default. What do y'all think / can you think of one such scenario? In fact, wouldn't we run into this same issue with all scalar types (e.g., int, string)? Under the current scheme there is no way to pass 0 or an empty string unless you specify a default.

@etiennedi
Copy link

It seems the fix by setting a default outlined in #7391 (comment) does not work for fields of type number when the default is 0.

No default

The following:

    latitude:
        type: number
        description: the latitude in decimal format

leads to

Latitude float32 `json:"latitude,omitempty"` 

as expected.

Default != 0

Setting a default other than 0 will lead to the expecting pointer construct, such as:

    latitude:
        type: number
        default: 5
        description: the latitude in decimal format

leads to

Latitude *float32 `json:"latitude,omitempty"` 

as expected after knowing this fixes it for bool fields as well.

Default == 0

However, when the default is 0 (which is the default value of a float32, similar to how false is the default value of a bool), we don't end up with the pointer construct:

    latitude:
        type: number
        default: 0
        description: the latitude in decimal format

leads to

Latitude float32 `json:"latitude,omitempty"` 

So, fixing this by setting a default, does not work if the default is 0.

Gerrit91 added a commit to metal-stack/metal-go that referenced this issue Jul 19, 2019
Fixes a bug where auto acquisition of networks was not transmitted properly due to an issue in swagger codegen: swagger-api/swagger-codegen#7391
@InTheCloudDan
Copy link

I'm testing with version CLI 2.4.8 and it's no longer generating pointers for booleans when setting a default value.

@jake-scott
Copy link

I am also seeing the bool issue - on Golang 1.12.6 and go-swagger latest (c49ea4ca2).

The fix does seem to be making the struct member a pointer in the model - which as suggested, a default value does - and also making the property required, eg :

  GroupQueryResult:
    description: Result of a group query
    type: object
    required:
      - group
      - user
      - isMember
    properties:
      user:
        type: string
      group:
        type: string
      isMember:
        type: boolean

@wkgcass
Copy link

wkgcass commented Jan 22, 2020

any updates?

Gerrit91 added a commit to metal-stack/metal-api that referenced this issue Feb 14, 2020
y3ti added a commit to ghostery/APIv3-go-library that referenced this issue Nov 9, 2021
This is a dirty workaround for the problem with Golang empty values
and marshaling to JSON. The problem was described here:

https://willnorris.com/2014/05/go-rest-apis-and-pointers/

In this library all struct's fields should be pointers, not simple types
like `bool` or `string` because we can't say whether a value is empty
and we should omit it (`omitempty` field tag) or not.

This library is auto-generated by swagger codegen, and developers created
an issue for this problem:
swagger-api/swagger-codegen#7391

In this pull request we dirty fix our problem with only one field we want
to update (set value to false)
@cbrom
Copy link

cbrom commented Nov 17, 2021

Any updates?
It's a real problem when you are dealing with a complex and huge swagger api.

@cbrom
Copy link

cbrom commented Nov 29, 2021

@vvalchev Can you help please?

I tried but couldn't figure it out. Can you help me make boolean type not omitempty and be always a pointer type.

@cbrom
Copy link

cbrom commented Nov 30, 2021

@wkgcass Or anyone out there for solutions,

I used -t (template directory) while generating for golang. And under model.mustache, I specified

{{#description}} // {{{description}}} {{/description}} {{name}} {{^isEnum}}{{^isPrimitiveType}}{{^isContainer}}{{^isDateTime}}*{{/isDateTime}}{{/isContainer}}{{/isPrimitiveType}}{{/isEnum}}{{#isBoolean}}*{{/isBoolean}}{{{datatype}}} `json:"{{baseName}}{{^required}},omitempty{{/required}}"{{#withXml}} xml:"{{baseName}}"{{/withXml}}` {{/vars}} {{/isComposedModel}} }{{/isEnum}}{{/model}}{{/models}}

Notice {{#isBoolean}}*{{/isBoolean}}

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

Successfully merging a pull request may close this issue.

10 participants