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

XML parsing fails #67

Open
Waitak opened this issue Apr 30, 2016 · 3 comments
Open

XML parsing fails #67

Waitak opened this issue Apr 30, 2016 · 3 comments

Comments

@Waitak
Copy link

Waitak commented Apr 30, 2016

XML parsing doesn't work at all. The struct tags for XML are all of the form

`xml:"tag"`

but the values are taken from the "value" attribute. The problem is that it is not possible to specify that the value for an element is taken from an attribute of the same element. It is only possible to specify that the value of a field of the element is taken from an attribute. As a result, the struct tags don't correspond to the XML format, and parsing fails. See this Stackoverflow discussion for details.

The solution seems to be to implement a custom unmarshaller/marshaller for XML that is aware of this usage of the "value" attribute.

@Waitak
Copy link
Author

Waitak commented Apr 30, 2016

A StackOverflow contributor made an excellent suggestion. If you were to include the attached file, and replace all of the string field types with StringField, and bool with BooleanField in all of the data types and resources the "value" attribute would work fine. You would also need to either cast StringFields to string when you need one, or just use StringField and BooleanField everywhere, and similarly for other types (Decimal, for example).

@cmoesel
Copy link
Contributor

cmoesel commented May 2, 2016

Thus far, XML support has not been a target feature for us. You'll find that we don't have XML tags at all, so any behavior you see is just based on Go's default marshaling/unmarshaling of structs. XML is one of those things we put on our "some day" list, but hasn't really received priority since it isn't required in any of our use cases.

A while back, I had to write a simple HQMF parser in Go and ran into the same basic problem with simple types having their values in "value" attributes. For that, I ended up implementing something similar to (but not exactly like) what you found on SO. It really is a shame that Go doesn't support just a tiny bit more complexity in their XML tags.

Since all of our model code is generated, we could generate custom marshalers and unmarshalers for every resource/type (without having to manually do it for every one). This would create a bunch more code than the StringField / BooleanField approach, but we'd be able to keep the built-in string and boolean types. While I like the clean marshaling/unmarshaling approach of StringField and BooleanField, I don't like requiring a cast every time we want to use a built-in primitive. I shudder to think of all the unit tests we'd need to fix where we do equals assertions on strings or booleans! So... I'd want to think more about it before deciding to take such an approach.

As I said, XML support hasn't really been on our priority list. At the same time, depending on the approach, it might be something that could be hammered out fairly quickly -- so I don't want to entirely put it off. What's your urgency on this? If we agreed on an approach, is it something you might be willing/able to submit a PR for? (That would require modifying the generator that is based on the ANT FHIR toolchain).

@Waitak
Copy link
Author

Waitak commented May 2, 2016

Thinking about it, it might be smarter to just bite the bullet and generate custom marshalers and unmarshalers for everything. It's ugly, but then generated code is generated code. On the other hand... I wonder if you could generate your own generic marshaler/unmarshaler, written to handle value= and then call that instead of the standard one? That'd be some guru-level hacking, but if you got it right, it'd be elegant, fast and hardly any extra code at all.

You'd definitely have to hand-implement the marshaler/unmarshaler for FHIRDateTime no matter which approach you take, but everything else should pretty much work.

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

No branches or pull requests

2 participants