droyo / go-xml

utility and code-generation libraries for XML
MIT License
304 stars 115 forks source link

xsdgen: omitempty and zero-value structs #32

Open droyo opened 7 years ago

droyo commented 7 years ago

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:

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 :)

ianlopshire commented 6 years ago

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:

homanchou commented 6 years ago

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!

droyo commented 6 years ago

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 commented 6 years ago

@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?

ianlopshire commented 6 years ago

@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.

leth commented 5 years ago

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.

droyo commented 5 years ago

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 commented 5 years ago

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.

kdeloach commented 2 years ago

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.

agusti-t commented 1 year ago

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