octokit / octokit.graphql.net

A GitHub GraphQL client library for .NET
MIT License
143 stars 50 forks source link

Consider AutoPaging Optionally Returning Edges #197

Open jamisonhyatt opened 5 years ago

jamisonhyatt commented 5 years ago

A lot of times there is important information on the Edge (E.g., LanguageEdge has size in bytes, CollaboratorEdge has RepositoryPermisson) and the implementation of autopaging only returns the underlying node. Unless I'm missing something, this means anytime you need information from the edge you are required to fall back to manual paging.

It looks like AllPages could be modified to optionally return an IQueryableList of Edges rather than Nodes. Is that modification something that you would be in favor of? Was this considered and discarded?

terrajobst commented 4 years ago

Totally agree! It seems GraphQL has much better performance (compared to Octokit) but things like this make it pretty hard to migrate 😢

jeffhandley commented 2 years ago

I just hit this scenario too, also on the RepositoryCollaboratorEdge scenario. I was trying find the collaborator edge associated with a pull request's author. Here was my workaround:

var collaboratorQuery = new Query()
    .Repository(Var("repo"), Var("owner"))
    .Collaborators(query: Var("author"))
    .Select(c => new
    {
        c.PageInfo.HasNextPage,
        c.PageInfo.EndCursor,
        Users = c.Edges.Select(e => new
        {
            e.Node.Login,
            e.Permission
        }).ToList()
    })
    .Compile();

while (true)
{
    string? afterCursor = null;

    var collaborators = await connection.Run(collaboratorQuery, new Dictionary<string, object?>
    {
        { "owner", owner },
        { "repo", repo },
        { "author", data.PullRequest.Author },
        { "after_cursor", afterCursor }
    });

    var author = collaborators.Users.SingleOrDefault(user => string.Equals(user.Login, data.PullRequest.Author, StringComparison.InvariantCultureIgnoreCase));

    if (author is not null)
    {
        Console.WriteLine($"Collaborator:");
        Console.WriteLine($"  Login:      {author.Login}");
        Console.WriteLine($"  Permission: {author.Permission}");
        break;
    }

    if (collaborators.HasNextPage)
    {
        afterCursor = collaborators.EndCursor;
    }
    else
    {
        Console.WriteLine($"Author '{data.PullRequest.Author}' is not a collaborator in this repository.");
        break;
    }
}
jMarkP commented 1 year ago

I'm also comping up against this. Would be keen to pick this up if that's OK (I see it's 'up for grabs')?

In my head the solution I was going to suggest was to add an optional argument to AllPages to specify whether to paginate over edges or nodes (.AllPages(paginationTarget: PaginationTarget.Edges)?) and then in the QueryBuilder.CreateNodeQuery (when the AllPages expression gets converted into a .Nodes().Cast<>().... expression tree - https://github.com/octokit/octokit.graphql.net/blob/main/Octokit.GraphQL.Core/Core/Builders/QueryBuilder.cs#L1069) to examine that argument and emit an Edges() call instead.