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

please user OrderedMap instead of the golang's map #645

Open
ddzrc opened this issue Oct 22, 2022 · 11 comments · May be fixed by #695
Open

please user OrderedMap instead of the golang's map #645

ddzrc opened this issue Oct 22, 2022 · 11 comments · May be fixed by #695

Comments

@ddzrc
Copy link

ddzrc commented Oct 22, 2022

The order of properties after each parsing is inconsistent.

@fenollp
Copy link
Collaborator

fenollp commented Oct 23, 2022

Go maps iteration order is random, so I don't see why you mention deserialization?

Are you asking for this lib to use orderedmaps instead of maps so that operations can be deterministic? What's your use case?
Do you see non-deterministic operations within the lib (that would be a bug)?

Also, what implementation of orderedmap would you recommend and why.

@zhangyongding
Copy link

@fenollp
Copy link
Collaborator

fenollp commented Nov 30, 2022

See also https://github.com/wk8/go-ordered-map
v2 requires go18, which isn't a full year old https://go.dev/blog/go1.18

@fenollp fenollp linked a pull request Dec 4, 2022 that will close this issue
@chippyash
Copy link

We have noticed that not only is the ordering inconsistent, which can make it a mess for people trying to read the yaml files, but the output yaml will also break a swaggerhub display. By default, yaml is output in alphabetical order. This places 'components' before 'paths' and that will break some definitions. All the definitions are there, and it does validate in swaggerhub BUT the display does not show the definitions. We've seen this so far with 'allOf'

      schema:
        allOf:
          - $ref: '#/components/schemas/Guid'
          - title: Incident guid

If we manually move the 'components' section to be after 'paths', then all is ok again.

@fenollp
Copy link
Collaborator

fenollp commented Dec 5, 2022

Thanks for your feedback! We've got a couple github issues related to serialization order (and non-determinism must be annihilated) and I'm happy to hit two birds with one stone!

Note: solving this issue depends on wk8/go-ordered-map#14 (comment) if anyone want to help there ;)

@codestation
Copy link

Note: solving this issue depends on wk8/go-ordered-map#14 (comment) if anyone want to help there ;)

Seems like this is already implemented on wk8/go-ordered-map#16

@fenollp
Copy link
Collaborator

fenollp commented Mar 19, 2023

@chippyash Serialization order of fields is already adaptable. See openapi3.go and the MarshalJSON func implementation there. You should be able to reorder the fields there. I'll review your PR (provided you add a test for this ofc)

@codestation Look at my draft PR you'll see that's already being used :)

@fenollp
Copy link
Collaborator

fenollp commented Mar 19, 2023

FYI progress on this is blocked by #763
If anyone wants to take a look at the failing tests there...

@chippyash
Copy link

@fenollp Any progress on this please?

@fenollp
Copy link
Collaborator

fenollp commented May 13, 2023

Hi @chippyash Not much progress to report. Still blocked on #763 Comments welcomed!

@ashb
Copy link

ashb commented Jan 15, 2024

The reason I'd like this: we generate our spec file and add it to git, and right now if we use MarshalJSON() as it is, the order of parameters etc changes every time the command is run.

(I'm not expecting anyone to magically fix this, just providing reason for having order preserving.)

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

Successfully merging a pull request may close this issue.

6 participants