defenseunicorns / go-oscal

Repository for the generation of OSCAL data types
Apache License 2.0
18 stars 1 forks source link

bug(generate): Json omitempty requires pointers. #131

Closed mike-winberry closed 7 months ago

mike-winberry commented 9 months ago

Problem: When using oscal as json and consuming go-oscal types, omitempty fields are still initialized due to non-primitive nested types not being pointers.

Potential bug (needs input). When using json.Unmarshal/Marshal omitempty fields are still present due to the sub types not being pointers. This is not a problem with yaml, due to differences in how yaml (gopkg.in/yaml.v3) handles marshaling the data.

Potential Solutions:

lstanden commented 8 months ago

I've got an experimental branch that adds an optional implementation using generics. The test suite currently fails, but that's kind of expected given the API to access those optionals. https://github.com/lstanden/go-oscal/tree/add-optional

This would obviously be a breaking change, so wanted to check in before I invest a lot of time in this.

mike-winberry commented 8 months ago

I've got an experimental branch that adds an optional implementation using generics. The test suite currently fails, but that's kind of expected given the API to access those optionals. https://github.com/lstanden/go-oscal/tree/add-optional

This would obviously be a breaking change, so wanted to check in before I invest a lot of time in this.

Thanks for taking a look at this. I know right now this is a lower priority for the team, but I brought it up today and plan on playing with it a bit this weekend/next week. I wouldn't worry about going farther right now until we can dig in a bit and see if this is the solution we want to move forward with.

Additionally if you are using go-oscal and running into this issue, we would love some context (if possible) so we can re-prioritize if necessary, right now yaml has been our target for first class, but would like to solve this sooner rather than later.

The only thing I have to add is that we may lean towards the pointer solution just because it is more conventional as an expectation, but I do like some of the additional things this solution provides.

lstanden commented 8 months ago

I’m not currently working with it, but I was taking a look as something I wanted to use to get a jump start on loading OSCAL data into a database.