oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

Optional parameter with default value should NOT be treated as pointers

Open ilya-direct opened this issue 4 years ago • 6 comments

Schema

schema:
  type: object
  properties:
      name:
          type: string
      comment:
          type: string
          default: ""
  required:
      - name

Generated as:

type AddItemJSONBody struct {
        Name    string `json:"name"`
	Comment    *string `json:"comment,omitempty"`
}

But it would be useful to be (without a pointer):

type AddItemJSONBody struct {
        Name    string `json:"name"`
	Comment    string `json:"comment,omitempty"`
}

within json.unmarshal empty comment should be assigned to default value.

PS pointer values must be checked for nil and that is an additional pain.

ilya-direct avatar Nov 09 '21 12:11 ilya-direct

I'm facing the same issue. Do you have any solution?

MateusFrFreitas avatar Sep 20 '22 14:09 MateusFrFreitas

I'm running into a similar issue. I've got an optional property that I want to be marshaled as a string same as Comment and I was hoping that setting x-nullable false would force the removal of the pointer.

Would that be a reasonable change to make? If x-nullable is false, but the value is not provided, then it should just fall back to the default if provided, or the zero value for the type, which the json marshalling should do automatically.

veleek avatar Oct 26 '22 14:10 veleek

It looks like this old PR also does something similar by adding support for x-go-nullable which forces a non-pointer when the value is false (or empty). #146

veleek avatar Oct 26 '22 15:10 veleek

@deepmap-marcinr Is there any way this could be merged/supported?

scottshotgg avatar Jan 25 '23 00:01 scottshotgg

This one is tricky, because the json unmarshaler is not aware of the fact that there should be a default value when unmarshaling. So, if we make a field a non-pointer type, then you will have no way to tell apart in your own code whether it's been provided but set to the zerovalue or not provided at all.

I would love all fields with defaults to be non-pointers, it's nicer to work with, however, before we have that, we need to have a spec-aware json marshaler. I've got the beginnings of one in an experimental code tree, but IMO, that's a hard requirement before we add undeterminable state.

So far, the choice has been to use uglier models but complete determinism.

deepmap-marcinr avatar Jan 25 '23 01:01 deepmap-marcinr

Thanks for the comment. I'll check out the experimental branch if I can find it.

For now I've forked and incorporated some of the changes from #146. This works for now; I would much rather work with pointers for simple types, especially since I've got a middleware that validates the JSON object itself before reaching any app code.

scottshotgg avatar Jan 26 '23 02:01 scottshotgg

I've always made good use of protobuf's accessors as a solution to this problem:

For either of these field definitions:

optional int32 birth_year = 1;
required int32 birth_year = 1;

the compiler generates a struct with an *int32 field named BirthYear and an accessor method GetBirthYear() which returns the int32 value in Artist or the default value if the field is unset. If the default is not explicitly set, the zero value of that type is used instead (0 for numbers, the empty string for strings).

I wonder if this would help here? I would appreciate having this for my use.

dnesting avatar May 22 '23 03:05 dnesting