Repository contents apis don't handle the root tree properly

[Cyberboss]

Take a look at this tree:

https://github.com/tgstation/tgstation/tree/d728a08b4288b8445b14444d2e5e330b735f4d17

https://api.github.com/repos/tgstation/tgstation/contents/?ref=d728a08b4288b8445b14444d2e5e330b735f4d17

27 entries right?

However, if you retrieve it with octokit.net...

For some odd reason, suits.dmi will NOT be retrieved.

Note that this is running off commit a1d3c7a
Master latest commit: 3345f76

[Cyberboss]

Stepping into this, I've found that the URL that gets GET'd in the end is https://api.github.com/repositories/3234987/contents//?ref=d728a08b4288b8445b14444d2e5e330b735f4d17

Notice the double slash? In the end this ends up redirecting back to https://api.github.com/repositories/3234987/contents giving me the slightly different results

[Cyberboss]

Ok, so changing dir from "/" to "" and stepping over this line:

octokit.net/Octokit/Clients/RepositoryContentsClient.cs

Line 126 in a1d3c7a

Ensure.ArgumentNotNullOrEmptyString(path, nameof(path));

(Same line on master)
Allowed the request to run as expected

Solution would be to one or both of the following:

a) Properly handle "/" passed as a path to the repo content apis
b) Allow empty path strings as valid parameters

[ryangribble]

Hey @Cyberboss

This is ringing a bell... need to look through old issues to see if I can find it.

[ryangribble]

OK yeah, so that answer is that an overload already exists that has no path parameter, and does the root dir.

octokit.net/Octokit/Clients/RepositoryContentsClient.cs

Lines 154 to 163 in ef1994d

	/// <summary>
	/// Returns the contents of the root directory in a repository.
	/// </summary>
	/// <remarks>
	/// If given a path to a single file, this method returns a collection containing only that file.
	/// See the <a href="https://developer.github.com/v3/repos/contents/#get-contents">API documentation</a> for more information.
	/// </remarks>
	/// <param name="repositoryId">The Id of the repository</param>
	/// <param name="reference">The name of the commit/branch/tag. Default: the repository’s default branch (usually master)</param>
	public Task<IReadOnlyList<RepositoryContent>> GetAllContentsByRef(long repositoryId, string reference)

Previously an issue and PR #1688 & #1689 were raised to "fix" this, but were closed when they realised there was an overload that allowed this already. As I commented there though, I would be happy to allow empty string on the path overloads as well, if you do want to get a PR in.

[lethall]

Just ran into this as well. Either use the overload that doesn't take the path, or substitute a dot (.) for the slash (/) .

In fact, the API supports a named parameter (path) so it can be explicitly stated rather than using the position (?path=/&ref=) so the best of all worlds would be to make this patch:

From a05dcfcaea1c2f978dbd62e67f1e5e5bd6030426 Mon Sep 17 00:00:00 2001
From: Lee Hall <lhall@javafoundry.com>
Date: Thu, 13 Dec 2018 17:47:28 -0500
Subject: [PATCH] Bug 1837 - use path parameter

---
 Octokit/Helpers/ApiUrls.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs
index 1b77364a..b2662f9f 100644
--- a/Octokit/Helpers/ApiUrls.cs
+++ b/Octokit/Helpers/ApiUrls.cs
@@ -3328,7 +3328,7 @@ public static Uri RepositoryContent(long repositoryId, string path)
         /// <returns>The <see cref="Uri"/> for getting the contents of the specified repository and path</returns>
         public static Uri RepositoryContent(long repositoryId, string path, string reference)
         {
-            return "repositories/{0}/contents/{1}?ref={2}".FormatUri(repositoryId, path, reference);
+            return "repositories/{0}/contents?path={1}&ref={2}".FormatUri(repositoryId, path, reference);
         }

         /// <summary>
--
2.17.0

[shiftkey]

I believe this was related to RepositoryContentsClient.GetAllContentsByRef but there weren't any code samples provided to confirm this.

I had a failing test submitted in #1956 that covered this API, so I took that to fix the issue in #2105 which shipped in v0.42.0.

I believe this has been resolved but please let me know if you still see it, and the code sample used to reproduce the problem on our end.