feat: Refactor repositories download contents

[stevehipwell]

This PR refactors the behaviour of DownloadContents & DownloadContentsWithMeta with the former now being a direct passthrough to the latter as the only difference was the signature. The code has been refactored to use the API directly instead of via an unnecessary layer of indirection.

I've added an OpenAPI update to this PR as it proves that the updated code works against GitHub.

This change is required for #4151.

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.77%. Comparing base (44908ea) to head (7312075).

Files with missing lines	Patch %	Lines
github/repos_contents.go	83.33%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4153      +/-   ##
==========================================
+ Coverage   93.76%   93.77%   +0.01%     
==========================================
  Files         211      211              
  Lines       19701    19677      -24     
==========================================
- Hits        18472    18452      -20     
+ Misses       1031     1029       -2     
+ Partials      198      196       -2     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stevehipwell]

@gmlewis can we get this merged?

[gmlewis]

I'm quite concerned about this PR because it appears to me that the behavior of following redirects has been deleted and there are many unit tests that have also simply been deleted without comment or explanation. One of the great things about unit tests is that when major refactors are performed like this one, if the unit tests are left alone we can easily detect regressions. As it is in this PR, however, where a major refactor happens and unit tests are also heavily refactored and/or deleted, it is hard to tell what is actually happening.

Can this be broken down into 3 PRs?

1. Update the openapi_operations.yaml file - I'll do that myself momentarily.
2. Refactor the download methods without modifying unit tests
3. Refactor and/or delete unit tests

[gmlewis]

We typically like to update this file in PRs that only update the one file to keep all our PR clean and focused I can do that in a separate PR if it is needed for this one.

[stevehipwell]

@gmlewis let me take a look, but the main problem here is that the tests appear to be tightly coupled to the implementation with mocks designed to make the test pass rather than to mirror the actual API. I'll add the deleted tests back, but the mocks will need to be refactored to add the schema required download_url to the content payload.

On a slight tangent, shouldn't the mock payloads be validated against the schema?

[gmlewis]

On a slight tangent, shouldn't the mock payloads be validated against the schema?

Yes, they probably be should. I don't remember when GitHub v3 API docs started sharing schemas for endpoints, but it is possible that these were written prior to that.

I think my biggest concern is following redirects because I remember a bunch of issues devoted solely to this topic, and to my shock and disappointment, I don't see any of the unit tests actually testing out following redirects and I could have sworn that it took a good deal of effort to get those unit tests to pass at one point. :-(