go-xml icon indicating copy to clipboard operation
go-xml copied to clipboard

xsdgen: omitempty and zero-value structs

Open droyo opened this issue 6 years ago • 10 comments

We need a workaround for https://golang.org/issues/11939. Empty structs (full of zeroes) for which ,omitempty is present on all field tags should not show up in MarshalXML output, but they do. See https://play.golang.org/p/J-_l2JySA0 for a trivial example of the problem.

Solution 1: pointers for all

One solution would be to use pointers for everything. However, I don't want to do that, because it makes the resulting types more difficult to use, as each field access will require if v.FieldName != nil { ... guards.

Solution 2: Smarter MarshalXML methods

Another solution would be to make MarshalXML methods smarter. Ignoring struct embedding which will be gone when #31 is resolved, we can skip emitting the complexType T if the following conditions are satisfied:

  • All of T's fields use the ,omitempty option
  • All of T's fields are the zero value for the field's type

The second condition is kind of annoying to check. Something like this would cover most cases:

if (v == T{})

but it won't compile if T contains a slice, such as with list types or base64Binary derivatives.

Solution 3: Wait for a fix :)

droyo avatar Oct 17 '17 03:10 droyo

What if a nullable struct was generated for each type. The nullable struct would conditionally be used if a field is nullable or optional. Something like:

type Foo struct {
	StringField string
	IntField    int
}

type FooNullable struct {
	Foo
	Valid bool
}

type Bar {
	RequiredField Foo
	OptionalField FooNullable
}

I think this provides a number of advantages:

  • Prevents the need for nil guards
  • Explicitly set if a zero value should be marshaled
  • Explicitly see if a zero value was unmarshaled
  • Marshal and Unmarshal functions would be fairly trivial

ianlopshire avatar Nov 06 '17 21:11 ianlopshire

I've successfully generated structs from an xsd. However when I marshal the struct, which is full of optional fields it renders a huge xml file with even empty fields still rendering. If I'm interested in a solution now to ignore empty fields, what's the quickest way to implement solution 1? I think pointers for all struct fields is the easiest way? I much prefer using pointers as the way to leave a node undefined. Omitempty doesn't do what I want. Sometimes you need a zero value to come thru, such as setting quantity to 0, we don't want that to be ignored. Thanks!

homanchou avatar Nov 28 '17 22:11 homanchou

Using pointers for all struct fields is the easiest way, I think.

@homanchou , I've just pushed the branch omitempty-structs which makes this change. Can you give it a try and let me know if it meets your needs?

If this approach works out, I think I'd like to hide it behind a non-default config flag.

droyo avatar Dec 01 '17 00:12 droyo

@homanchou , have you been able to try the changes made in the omitempty-structs branch? If not, could you share the XSD and sample data you used, so I can make sure it fixes the issue?

droyo avatar Jan 08 '18 16:01 droyo

@droyo I've been able to use the omitempty-structs branch on a number of complex schemas with out any issues.

The xsd and wsdl files for the services I'v been working with can be found here.

ianlopshire avatar Jul 26 '18 16:07 ianlopshire

I have a similar issue, except the struct is a time.Time. I just tried the omitempty-structs branch, but it doesn't cover that scenario.

leth avatar Dec 21 '18 16:12 leth

There has been some movement on https://github.com/golang/go/issues/11939 which would resolve our problem. That said, that issue has been open for 3 years, and I don't think everyone will adopt Go 1.12 immediately, so it's still worth having a workaround here.

droyo avatar Jan 05 '19 16:01 droyo

the omitempty-structs branch implements solution 1, using pointers for all fields. this seems to work well for those who have used it, so i'm thinking we can move it into the master branch and put it behind a flag.

droyo avatar Jul 30 '19 23:07 droyo

I think it'd be immensely helpful to merge the omitempty-structs branch to resolve this. Currently, the generated output is far too noisy, especially for complex schemas. I don't mind adding null guards if necessary, since it allows you to differentiate between null and empty data.

For what it's worth, other projects like gqlgen make pointers the default for generated structs, and expose a setting to omit non-pointer structs.

kdeloach avatar Apr 08 '22 19:04 kdeloach

What happened with this issue? Would be great to have it merged.

agusti-t avatar Apr 17 '23 09:04 agusti-t