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

client.Repository.Content.GetAllContents for root not currently possible. #800

Closed
DavidStrickland0 opened this issue May 11, 2015 · 9 comments · Fixed by #1064
Closed

client.Repository.Content.GetAllContents for root not currently possible. #800

DavidStrickland0 opened this issue May 11, 2015 · 9 comments · Fixed by #1064
Labels
Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented

Comments

@DavidStrickland0
Copy link

DavidStrickland0 commented May 11, 2015

Task<IReadOnlyList> GetAllContents(string owner, string name, string path);
Will currently only work with subdirectories ie:
client.Repository.Content.GetAllContents("octokit", "octokit.net", "Octokit") returns 18 items.
However
client.Repository.Content.GetAllContents("octokit", "octokit.net", "") Throws a ArgumentException("String cannot be empty", name)

For a quick fixed I hacked GetAllContents as follows locally but a better option is probably needed.

public async Task<IReadOnlyList<RepositoryContent>> GetAllContents(string owner, string name, string path)
        {
            Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
            Ensure.ArgumentNotNullOrEmptyString(name, "name");
            //Ensure.ArgumentNotNullOrEmptyString(path, "path");
            System.Uri url;
            if (string.IsNullOrEmpty(path))
            {
                url = ApiUrls.RepositoryContent(owner, name);
            }
            else
            {
                url = ApiUrls.RepositoryContent(owner, name, path);
            }
            return await ApiConnection.GetAll<RepositoryContent>(url);
        }
@SplitInfinity
Copy link

I am able to get the contents of a repository's root directory by passing in "/" as the path, but I am unsure if this is due to changes made after this issue was opened.

@ostruk
Copy link

ostruk commented Nov 3, 2015

👍

"/" only works if you're retrieving master branch. If you're retrieving some other branch it defaults to master branch, because the generated url is actually invalid: "contents//?ref=branch-name". Remove the check for empty string when specifying path.

@naveensrinivasan
Copy link

You could get all the contents of the branch using the overload client.Repository.Content.GetAllContents which takes the reference as the last parameter.

I am able to get all the contents of the branch and master including the sub-directories. Here is a sample.

async Task Main(string[] args)
{
    var owner = "octokit";
    var reponame = "octokit.net";

    GitHubClient client = new GitHubClient(new Octokit.ProductHeaderValue("Octokit.samples"));
    client.Credentials = new Credentials(Util.GetPassword("github"));
    var branchcontents = await client.Repository.Content.GetAllContents(owner,reponame,"/Octokit.Reactive/Properties","codeformatter");
    var mastercontents = await client.Repository.Content.GetAllContents(owner,reponame,"/Octokit.Reactive/Properties");
    branchcontents .Dump();
    mastercontents.Dump();
    branchcontents = await client.Repository.Content.GetAllContents(owner,reponame,"/","codeformatter");
    mastercontents = await client.Repository.Content.GetAllContents(owner,reponame,"/");

    branchcontents .Dump();
    mastercontents.Dump();
}

@ostruk
Copy link

ostruk commented Nov 3, 2015

I have 7 files on my master branch, and 6 on my custom branch ("ostruk-branch" does not have "added.js").

When I call

var all = await client.Repository.Content.GetAllContents("UW-demo", "multiJs", "/", "ostruk-branch");

I get 7 files, same as if I called without "ostruk-branch". When I inspect the code, I can see it attempts to make a call to

https://api.github.com/repos/UW-demo/multiJs/contents//?ref=ostruk-branch

where you can see that all files have "?ref=master" by them, indicating they are being pulled from the master branch. If I change the url to

https://api.github.com/repos/UW-demo/multiJs/contents/?ref=ostruk-branch

, which is equivalent to passing "'" for path argument, then response is correct.

I'm using 0.16.0.

Thanks!

@naveensrinivasan
Copy link

@ostruk Yes you are right. It is a bug.

naveensrinivasan pushed a commit to naveensrinivasan/octokit.net that referenced this issue Nov 4, 2015
… not currently possible. octokit#800

The client.Repository.Content.GetAllContents does not allow null in the
`path` argument. With the branch the root was always returning the
`master` instead of the reference passed in the argument.
naveensrinivasan pushed a commit to naveensrinivasan/octokit.net that referenced this issue Nov 4, 2015
…ntly possible. octokit#800

The code client.Repository.Content.GetAllContents  didn't allow getting
the root directory contents when there was a branch in the `ref`
parameter.  This fix would disallow the null check for the path to get
by this.
naveensrinivasan pushed a commit to naveensrinivasan/octokit.net that referenced this issue Nov 4, 2015
…ly possible. octokit#800

The client.Repository.Content.GetAllContents does not allow getting root
folders for Branches. This fix would allow the users to pass null/empty
strings for the path which will address the issue.
shiftkey pushed a commit that referenced this issue Jan 25, 2016
…ly possible. #800

The client.Repository.Content.GetAllContents does not allow getting root
folders for Branches. This fix would allow the users to pass null/empty
strings for the path which will address the issue.
@jamesqo
Copy link

jamesqo commented Jun 8, 2017

Can we reopen this issue? The fix was accidentally undone during #1348 and I'm running into it again.

@ryangribble
Copy link
Contributor

Whoops!

Need to dig into this to see why there isn't an integration test that failed

@ryangribble ryangribble reopened this Jun 9, 2017
@ryangribble
Copy link
Contributor

@jamesqo just looking at this one... are you referring to the fact that GetAllContents(string owner, string name, string path) is enforcing path to not be null or empty?

If that's the case can't you just call GetAllContents(string owner, string name) overload instead?

@nickfloyd nickfloyd added the bug label Apr 25, 2022
@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
@github-actions
Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Jul 24, 2023
@github-actions github-actions bot closed this as completed Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants