Manual order of paths is broken for no reason

[darklynx]

io.swagger.models.Swagger class supports preserved order for paths by using LinkedHashMap however the paths getter method has following magic inside:

public Map<String, Path> getPaths() {
    if (paths == null) {
        return null;
    }
    Map<String, Path> sorted = new LinkedHashMap<String, Path>();
    List<String> keys = new ArrayList<String>();
    keys.addAll(paths.keySet());
    Collections.sort(keys);

    for (String key : keys) {
        sorted.put(key, paths.get(key));
    }
    return sorted;
}

This completely breaks the original order of paths. I encountered this issue while generating swagger JSON from YAML description using swagger-codegen.

Are there any particular reason to sort the paths in the getter?

The quick replace of the getter with:

public Map<String, Path> getPaths() {
    return paths;
}

solved my problem, but maybe such fix could break some other projects/logic that I'm not aware of.

It would be great to have a possibility to preserve the original order of paths, because they are usually grouped and displayed in Swagger UI in the logical order, not in alphabetical.

[webron]

Whether we do it or not, you need to keep in mind there is just no ordering supported anyways. Everything is represented as a JSON object and not as a JSON array, and by definition, there's no order of properties within a JSON object and they can be parsed differently by different parsers.

If you want to control the order in the UI, you'd need to write your own sorter for it.

That said, it doesn't mean we won't change the sorting behavior in swagger-core, I'd just recommend not relying on it as the behavior you expect is simply not guaranteed in any way.

[darklynx]

Thank you for your quick reply!

I entirely understand that parsing logic could be tricky and does not have to be sequential, which would then break the originally defined order. I also understand that Swagger Spec which defines map to keep paths by definition does not guarantees the order in general.

This makes the sorting logic from the getter even more confusing.

However the proposed simplification from my message above has solved the issue for swagger-codegen project and out-coming JSON had the same order of paths as original YAML. The swagger-ui project that I'm using tends also to respect the order of paths in JSON file.

But I don't like to have a separate branch / fork for this small but important change. It would be great if the getter could be simplified in your original project, unless it breaks something that I'm not aware of.

Unfortunately I could not track down the reason for paths sorting in the commit history (neither in comments), and I might be missing some important reasons to sort the paths.

[webron]

@darklynx - @fehguy may have a reason for ordering that, not sure. He's away for a few days but should get back to you when he can. If there's no explicit reason for it, I don't see a reason not to simplify it.

[fehguy]

I believe the sorting was added because, given it is a map, the order would change just fetching the items twice in a row. Since the maps are unordered, I think it's best to leave this as such.

I do suggest using tags if you want to organize your resources better.

Please reopen if you strongly disagree and we can try to find some sort of solution.

[darklynx]

@fehguy thank you for your reply! I see your point.

But if you look at the implementation of public Swagger path(String key, Path path) method in Swagger class, it tends to use LinkedHashMap which would preserve the order paths gets added to Swagger.

This could have worked with YAML parser => JSON generator (I tested it). However the public Map<String, Path> getPaths() method breaks the order by sorting the output. Since your assumption is that the order cannot be guaranteed for paths why bother sorting them anyway?

On the other hand I do use tags in my descriptions of API, but cannot figure out how they could help to preserve an order within the tag group.

Actually it is a missing feature in the Path description to have some kind of sortOrder or weight parameter that would help to sort paths while displaying them. But this is, of course, out of scope of this particular issue, and I have already seen a feature request mentioning something like that.

Thanks a lot for the Swagger, you are doing a great job!
I can live with my personal generator and wait for the path sort order feature to make into the Swagger spec... ;)

[webron]

@darklynx - the path sort option was explicitly removed from the spec. If you'd like to see it re-added in the future, you'd better open a feature request in the swagger-spec project, but explain your reasoning well (because we had reasons to exclude it ;)). We can continue the discussion there and assess whether it should change back in a future version of the spec.

[darklynx]

@webron where can I read about the reasoning? :) I'd like to understand your position about ordering paths. I've read the discussion on this issue: #1050, but it does not explain why you've decided to get rid of manual ordering

[webron]

The discussion was in the 2.0 workgroup. If you email me, I'll try to find it and forward it to you. It may take a couple of days.

[paukiatwee]

I would like to suggest add new method Swagger.getPathsMap() to return original LinkedHashMap in order to access to ordered map.

@JsonIgnore annotation can add to Swagger.getPathsMap() in order to prevent serialized as json.

This is essential to preserve paths order defined in json/yaml file.

[2is10]

@webron

Everything is represented as a JSON object and not as a JSON array, and by definition, there's no order of properties within a JSON object and they can be parsed differently by different parsers.

The fact is that browsers (the most commonly used parsers of swagger JSON, no?) do preserve property order of objects, and this feature is part of the ES6 specification. Here’s an informative discussion of the reasons. I found this remark from Brendon Eich specifically noteworthy:

Engines are converging, inter-operation pressure points in one direction only: greater convergence and standardization. . . . Leaving things unspecified for too long was a failure on our part in tending the spec, or a trade-off (we had other things to do ;-). All water under the bridge, but we're not stepping back to unspecified behavior. Because engines aren't, because developers do not want.

@fehguy

I believe the sorting was added because, given it is a map, the order would change just fetching the items twice in a row. Since the maps are unordered, I think it's best to leave this as such.

What do you mean that “the order would change just fetching the items twice in a row”? I don't know of any widely used java.util.Map implementation whose order changes without any change to its contents. (I’m talking about a specific instance’s order. Even java.util.HashMap has a consistent order until modified.)

Also, your claim that “maps are unordered” is an over-generalization. From the Map Javadoc:

The order of a map is defined as the order in which the iterators on the map's collection views return their elements. Some map implementations, like the TreeMap class, make specific guarantees as to their order; others, like the HashMap class, do not.

Swagger allows callers to choose what kind of Map they want to use for paths:

    public void setPaths(Map<String, Path> paths) {
        this.paths = paths;
    }

If callers choose to use an ordered Map (as Swagger does by default), then why defeat that choice with an arbitrarily placed sort operation?

If fixing this issue is really as simple as removing a sort operation that has no reason to exist in the first place, I second @darklynx’s request for you to please consider it. Thanks.

[TehBakker]

Just to check, there is no way to customly order my paths to show them in a logical order in swagger ui ?
Also using codeGen so I can't use swagger annotation, just the swagger yaml specification.
And I don't want to use alpha order.

[darklynx]

funny, it got fixed with no complains in this PR #1951

I do not get why did we have to run such a long discussion here and a year later people just approve the suggested changes w/o arguing 🤷‍♂️