JSON Marshal error on generated empty OptString struct

[exu]

What version of ogen are you using?

v0.54.1

Can this issue be reproduced with the latest version?

Yes just try to run this test in directory where API is generated with OptString

package api

import (
	"testing"

	"encoding/json"
)

func TestOpString(t *testing.T) {

	s := OptString{}

	_, err := json.Marshal(s)

	if err != nil {
		t.Fatalf("error should be nil, got %v", err)
	}

}

So for most use cases with fields set to not required when trying to marshal it (e.g. send through event bus) it's not possible to use ogen.

Getting output:

--- FAIL: TestOpString (0.00s)
    /Users/exu/code/kube/testkube-cloud-api/pkg/api/optstring_test.go:16: 
   error should be nil, got json: error calling MarshalJSON for type api.OptString: 
   unexpected end of JSON input

[ernado]

Encoding and decoding with encoding/json is not supported for OptString because following limitations: golang/go#11939

So it is impossible to represent OptString with encoding/json (pointers are used instead), that's why we are using go-faster/jx and custom code-generated json marshaling and unmarshaling.

[exu]

@ernado thanks for the quick response, so how should I deal with it, can I omit OptString and use pointers as most other generators do?
Or any other ideas on how to workaround this without changing the generator? (we have quite big API definition)

[exu]

Looks like just handling zeroed value works

Would it be a case to just do PR to the generator for Opt* structs?

// MarshalJSON implements stdjson.Marshaler.
func (s OptString) MarshalJSON() ([]byte, error) {
	if !s.Set {
		return []byte(`{}`), nil
	}
	e := jx.Encoder{}
	s.Encode(&e)
	return e.Bytes(), nil
}

Just is there a way to check if Type is optional in template?

[ernado]

What are you trying to do?

Using MarshalJSON on requests, responses or most generated types should work.

Returning {} is invalid, probably we should not generate MarshalJSON for OptString at all, it is impossible to express omitempty struct with encoding/json.

[exu]

I try to send generated struct through external library (NATS) and don't have control over marshaller. Additional info: Fields are not initialized.

[exu]

It's more about generating valid JSON - as looks like currently, we're ending with something like field: when the struct is uninitialized. It would be ok for me to just store pass it (without handling omitempty) I just need valid JSON and currently it's not.

[tdakkota]

I see there several ways to fix it:

1. Do not generate MarshalJSON/UnmarshalJSON for the Opt/NilOpt types.

   • Likely breaks existing code.
   • These types would be marshaled/unmarshaled by encoding/json, as JSON objects.

2. Return []byte("null").

   • Does not follow the optionals semantic.

3. Return an error in case if value is not set, add omitempty tag to optional fields.

   • Forces to add omitempty tag. Some people may use generated types
     in their own structs.
   • Would not work properly in some cases. For example: Set is false, but value is not zero.

[ernado]

@tdakkota omitempty does not work for structures, even if they are zero value.

[ernado]

The only valid solution is (1). Let's remove MarshalJSON/UnmarshalJSON for Opt* types, it should be only for full components (e.g. requests, responses).

[exu]

Maybe just make no 1 optional? To not break possible existing implementations

[art-frela]

Hi,

for objects workaround with overhead

...
// use generated method MarshalJSON.
tempBts, err := s.MarshalJSON()
if err != nil {
    return fmt.Errorf("marshal: %w", err)
}

var m map[string]interface{}

// unmarshal to map[string]interface{}
if er := json.Unmarshal(tempBts, &m); er != nil {
    return fmt.Errorf("umarshal to map: %w", er)
}

// use  std MarshalIndent for get ident json.
bts, err := json.MarshalIndent(m, "", "    ")
if err != nil {
    return fmt.Errorf("marshal: %w", err)
}

P.S. If all of the Opt fields filled, error not happen!
P.P.S. ogen --version // ogen version v0.72.1 (built with go1.21.0) darwin/amd64

[fiendish]

Just checking, is this still being worked on?

@tdakkota omitempty does not work for structures, even if they are zero value.

But they did recently add omitzero in golang/go#45669

The only valid solution is (1). Let's remove MarshalJSON/UnmarshalJSON for Opt* types

Wouldn't this alter data if the structs do have values?

Is this all because of avoiding pointers?

[ernado]

Yes, we avoid pointers for better semantics and removing NPE problem.

Will use omitzero when released tho. Finally.