Sort methods to stabalize operation id generation

[bneijt]

The methods attribute accessed by the generate_unique_id function is modeled as a python set, which is not ordered and between runs might change. This results in the same fastapi generating different openapi specs if started multiple times.

This also relates to the bigger issue #4740 but is not a solution for that issue.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cf73051) 100.00% compared to head (ea7b56d) 100.00%.
Report is 1038 commits behind head on master.

❗ Current head ea7b56d differs from pull request most recent head d3d980b. Consider uploading reports for the commit d3d980b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            master     #5001    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          540       532     -8     
  Lines        13969     13672   -297     
==========================================
- Hits         13969     13672   -297     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

📝 Docs preview for commit 6379d7b at: https://62a06605ac380a00565f275f--fastapi.netlify.app

[JarroVGIT]

This would work and indeed solves a minor bug. You haven't supplied a test case for this though, might help in getting this PR approved by Tiangolo

[bneijt]

Because it's runtime random behavior, I find it hard to come up with a proper test case that will work across different python versions and will make sure we are actually doing the right thing instead of possibly passing because of shear luck. Consider the following output:

Python 3.8.9 (default, Dec 16 2021, 12:39:57) 
[GCC 11.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information
>>> b=set(["2","1","3"])
>>> list(b)[0]
'1'
>>> b=set(["1","2","3"])
>>> list(b)[0]
'3'
>>> b=set(["1","2","3"])
>>> list(b)[0]
'3'
>>> b=set(["1","2","3"])
>>> list(b)[0]
'3'
>>> b=set(["1","2","3"])
>>> list(b)[0]
'3'
>>> b=set(["1","2","4"])
>>> list(b)[0]
'4'

[JarroVGIT]

Yeah I was having a stroll outside right after I made that comment, and I had the exact same thought. You are right, I also couldn't find a way to properly test it.

That leaves me with the thought that somehow (if not through a specific test case) we would want to make it specific that there was a reason this line of code uses sorted. I can think of two possibilities:

1. Use a comment line in the code
2. Create a testcase with two lists (one reversed from the other) with explicit naming and pull that through the function. This doesn't test for sets, but it does make it explicit that despite different ordering in a Iterable, the outcome should be the same.

Let me know your thoughts!

[bneijt]

I've decided to add a comment.

I was writing the test case and came to the conclusion that there would be no way to write the test that would still make sense if APIRoute would be refactored to accept a single http method only.

Because I think that will probably end up being the outcome of #4740 I decided a comment would be best for now. Happy to hear from official reviewers on this.

[github-actions]

📝 Docs preview for commit ea7b56d at: https://62a3a49a1ffcb30a6f50af62--fastapi.netlify.app