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

feat(generate): structure for ordered keys #241

Closed
wants to merge 9 commits into from
Closed

Conversation

brandtkeller
Copy link
Member

Description

During a recent webinar - the formatting of OSCAL documents in Lula with regards to field order was mentioned as a point of confusion. While we do not want to stray far from the current generation logic which has no strict ties to order - providing the order of elements for the top-level models may clear up structure.

Recommend just the top-level models - open to feedback on ensuring we have error catching for non-deeply-equal items.

Related Issue

Fixes #240

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@mike-winberry
Copy link
Collaborator

highly recommend taking a look at libraries, because I don't think maintaining a map of the fields is gonna be a good long term solution, it puts us back to needing dev intervention for schema changes, we should be able to either maintain OG file order, which would be the computationally least complex solution I think, I had looked into this a month or two ago and had a partial solution using either https://github.com/elliotchance/orderedmap or some similar solution main...163-revise-fields-in-random-order-after-revision

@mike-winberry
Copy link
Collaborator

#163

^The original issue.

@mike-winberry
Copy link
Collaborator

also here is where we are sorting alphabetically,

slices.Sort(keys)
, idk if removing this would maintain the schema order, because I think that could depend on the marshal/unmarshalling method as well as the golang map ordering

@brandtkeller
Copy link
Member Author

highly recommend taking a look at libraries, because I don't think maintaining a map of the fields is gonna be a good long term solution, it puts us back to needing dev intervention for schema changes, we should be able to either maintain OG file order, which would be the computationally least complex solution I think, I had looked into this a month or two ago and had a partial solution using either https://github.com/elliotchance/orderedmap or some similar solution main...163-revise-fields-in-random-order-after-revision

also here is where we are sorting alphabetically,

slices.Sort(keys)

, idk if removing this would maintain the schema order, because I think that could depend on the marshal/unmarshalling method as well as the golang map ordering

All great items for context - I don't disagree that maintaining the map fields is not a great long term solution. Removing the sort results in the randomized output result with regards to order. Curious if there is another angle to approach this from - but that may be unnecessary work - the fields in the schema are listed alphabetically.

Overall just wanted to validate marshalling oscal models with fields in a specific order to validate they appear as expected. Generate is an operation that we seldomly run (IE going on 6 months since last OSCAL release). Curious how revise would respond coupled with this.

Overall not a pressing need -> more of an experiment to evaluate.

@mike-winberry
Copy link
Collaborator

could we just use the schema order (may need to back fix the doctor command to maintain the schema order), but since the schema is xml derived it may actually make sense they are ordered, [ ] will verify in the xml. In that case we wouldn't need to worry about hardcoding, and could pull the order directly from the schema applicable to the oscal document. thoughts?

@brandtkeller
Copy link
Member Author

Closing pull request as we orient to review metaschema for generation.

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

Successfully merging this pull request may close these issues.

Ability to establish known struct key order
2 participants