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

Caching GraphQL queries #2181

Merged
merged 20 commits into from Apr 4, 2019

Conversation

@grokys
Copy link
Contributor

commented Jan 16, 2019

This PR adds caching to GraphQL queries. It introduces a new IGraphQLClient which is used instead of (and wraps) Octokit.GraphQL's IConnection.

When a query is submitted, this class hashes the query and uses this as a cache key. Queries are cached in the filesystem using a slightly modified version of https://github.com/acarteas/FileCache.

The main complication here is that if you have say 3 pages worth of the PR list cached then pressing "Refresh" needs to clear all pages of the PR list. We use the region name to identify such queries and when refresh is pressed, call FileCache.ClearRegion. This method isn't present in the mainline FileCache which is why we ship a slightly modified version of it.

Depends on octokit/octokit.graphql.net#186

grokys added some commits Jan 7, 2019

Implement refreshing PR list and details.
Also brought `FileCache` into our source instead of importing its package because needed to add the `ClearRegion` method.
@StanleyGoldman
Copy link
Contributor

left a comment

I don't mean to do this. This is a test.

Sorry

StanleyGoldman added some commits Feb 27, 2019

Merge branch 'master' into feature/graphql-caching
# Conflicts:
#	submodules/octokit.graphql.net
@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

As per conversations had on slack graphql caching will be a double edged sword. We will need to review all of our usages of the graphql library to make sure caching will not result in any user experience bugs.

@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

Functionality to get a page of a list of pull requests

  • Handled by: Caching for default 8 hours and clear on refresh
    ICompiledQuery<Page<PullRequestListItemModel>> query;
    if (address.IsGitHubDotCom())
    {
    if (readPullRequests == null)
    {
    readPullRequests = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .PullRequests(
    first: 100,
    after: Var(nameof(after)),
    orderBy: new IssueOrder { Direction = OrderDirection.Desc, Field = IssueOrderField.CreatedAt },
    states: Var(nameof(states)))
    .Select(page => new Page<PullRequestListItemModel>
    {
    EndCursor = page.PageInfo.EndCursor,
    HasNextPage = page.PageInfo.HasNextPage,
    TotalCount = page.TotalCount,
    Items = page.Nodes.Select(pr => new ListItemAdapter
    {
    Id = pr.Id.Value,
    LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit =>
    new LastCommitSummaryAdapter
    {
    CheckSuites = commit.Commit.CheckSuites(null, null, null, null, null).AllPages(10)
    .Select(suite => new CheckSuiteSummaryModel
    {
    CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10)
    .Select(run => new CheckRunSummaryModel
    {
    Conclusion = run.Conclusion.FromGraphQl(),
    Status = run.Status.FromGraphQl()
    }).ToList(),
    }).ToList(),
    Statuses = commit.Commit.Status
    .Select(context =>
    context.Contexts.Select(statusContext => new StatusSummaryModel
    {
    State = statusContext.State.FromGraphQl(),
    }).ToList()
    ).SingleOrDefault()
    }).ToList().FirstOrDefault(),
    Author = new ActorModel
    {
    Login = pr.Author.Login,
    AvatarUrl = pr.Author.AvatarUrl(null),
    },
    CommentCount = pr.Comments(0, null, null, null).TotalCount,
    Number = pr.Number,
    Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter
    {
    Body = review.Body,
    CommentCount = review.Comments(null, null, null, null).TotalCount,
    }).ToList(),
    State = pr.State.FromGraphQl(),
    Title = pr.Title,
    UpdatedAt = pr.UpdatedAt,
    }).ToList(),
    }).Compile();
    }
    query = readPullRequests;
    }
    else
    {
    if (readPullRequestsEnterprise == null)
    {
    readPullRequestsEnterprise = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .PullRequests(
    first: 100,
    after: Var(nameof(after)),
    orderBy: new IssueOrder { Direction = OrderDirection.Desc, Field = IssueOrderField.CreatedAt },
    states: Var(nameof(states)))
    .Select(page => new Page<PullRequestListItemModel>
    {
    EndCursor = page.PageInfo.EndCursor,
    HasNextPage = page.PageInfo.HasNextPage,
    TotalCount = page.TotalCount,
    Items = page.Nodes.Select(pr => new ListItemAdapter
    {
    Id = pr.Id.Value,
    LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit =>
    new LastCommitSummaryAdapter
    {
    Statuses = commit.Commit.Status.Select(context =>
    context == null
    ? null
    : context.Contexts
    .Select(statusContext => new StatusSummaryModel
    {
    State = statusContext.State.FromGraphQl()
    }).ToList()
    ).SingleOrDefault()
    }).ToList().FirstOrDefault(),
    Author = new ActorModel
    {
    Login = pr.Author.Login,
    AvatarUrl = pr.Author.AvatarUrl(null),
    },
    CommentCount = pr.Comments(0, null, null, null).TotalCount,
    Number = pr.Number,
    Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter
    {
    Body = review.Body,
    CommentCount = review.Comments(null, null, null, null).TotalCount,
    }).ToList(),
    State = pr.State.FromGraphQl(),
    Title = pr.Title,
    UpdatedAt = pr.UpdatedAt,
    }).ToList(),
    }).Compile();
    }
    query = readPullRequestsEnterprise;
    }
    var graphql = await graphqlFactory.CreateConnection(address);
    var vars = new Dictionary<string, object>
    {
    { nameof(owner), owner },
    { nameof(name), name },
    { nameof(after), after },
    { nameof(states), states.Select(x => (Octokit.GraphQL.Model.PullRequestState)x).ToList() },
    };
    var region = owner + '/' + name + "/pr-list";
    var result = await graphql.Run(query, vars, regionName: region);

Functionality to get the pull request detail

  • Handled by: Caching for default 8 hours and clear on item refresh
    if (readPullRequest == null)
    {
    readPullRequest = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .PullRequest(number: Var(nameof(number)))
    .Select(pr => new PullRequestDetailModel
    {
    Id = pr.Id.Value,
    Number = pr.Number,
    Author = new ActorModel
    {
    Login = pr.Author.Login,
    AvatarUrl = pr.Author.AvatarUrl(null),
    },
    Title = pr.Title,
    Body = pr.Body,
    BaseRefSha = pr.BaseRefOid,
    BaseRefName = pr.BaseRefName,
    BaseRepositoryOwner = pr.Repository.Owner.Login,
    HeadRefName = pr.HeadRefName,
    HeadRefSha = pr.HeadRefOid,
    HeadRepositoryOwner = pr.HeadRepositoryOwner != null ? pr.HeadRepositoryOwner.Login : null,
    State = pr.State.FromGraphQl(),
    UpdatedAt = pr.UpdatedAt,
    CommentCount = pr.Comments(0, null, null, null).TotalCount,
    Comments = pr.Comments(null, null, null, null).AllPages().Select(comment => new CommentModel
    {
    Id = comment.Id.Value,
    Author = new ActorModel
    {
    Login = comment.Author.Login,
    AvatarUrl = comment.Author.AvatarUrl(null),
    },
    Body = comment.Body,
    CreatedAt = comment.CreatedAt,
    DatabaseId = comment.DatabaseId.Value,
    Url = comment.Url,
    }).ToList(),
    Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new PullRequestReviewModel
    {
    Id = review.Id.Value,
    Body = review.Body,
    CommitId = review.Commit.Oid,
    State = review.State.FromGraphQl(),
    SubmittedAt = review.SubmittedAt,
    Author = new ActorModel
    {
    Login = review.Author.Login,
    AvatarUrl = review.Author.AvatarUrl(null),
    },
    Comments = review.Comments(null, null, null, null).AllPages().Select(comment => new CommentAdapter
    {
    Id = comment.Id.Value,
    PullRequestId = comment.PullRequest.Number,
    DatabaseId = comment.DatabaseId.Value,
    Author = new ActorModel
    {
    Login = comment.Author.Login,
    AvatarUrl = comment.Author.AvatarUrl(null),
    },
    Body = comment.Body,
    Path = comment.Path,
    CommitSha = comment.Commit.Oid,
    DiffHunk = comment.DiffHunk,
    Position = comment.Position,
    OriginalPosition = comment.OriginalPosition,
    OriginalCommitId = comment.OriginalCommit.Oid,
    ReplyTo = comment.ReplyTo != null ? comment.ReplyTo.Id.Value : null,
    CreatedAt = comment.CreatedAt,
    Url = comment.Url,
    }).ToList(),
    }).ToList(),
    Timeline = pr.Timeline(null, null, null, null, null).AllPages().Select(item => item.Switch<object>(when =>
    when.Commit(commit => new CommitModel
    {
    AbbreviatedOid = commit.AbbreviatedOid,
    // TODO: commit.Author.User can be null
    Author = new ActorModel
    {
    Login = commit.Author.User.Login,
    AvatarUrl = commit.Author.User.AvatarUrl(null),
    },
    MessageHeadline = commit.MessageHeadline,
    Oid = commit.Oid,
    }).IssueComment(comment => new CommentModel
    {
    Author = new ActorModel
    {
    Login = comment.Author.Login,
    AvatarUrl = comment.Author.AvatarUrl(null),
    },
    Body = comment.Body,
    CreatedAt = comment.CreatedAt,
    DatabaseId = comment.DatabaseId.Value,
    Id = comment.Id.Value,
    Url = comment.Url,
    }))).ToList()
    }).Compile();
    }
    var vars = new Dictionary<string, object>
    {
    { nameof(owner), owner },
    { nameof(name), name },
    { nameof(number), number },
    };
    var connection = await graphqlFactory.CreateConnection(address);
    var result = await connection.Run(readPullRequest, vars, refresh: refresh);

Functionality to get the last commit of a pull request for statuses

  • Handled by: Caching for default 8 hours and clear on item refresh
    async Task<LastCommitAdapter> GetPullRequestLastCommitAdapter(HostAddress address, string owner, string name, int number, bool refresh)
    {
    ICompiledQuery<IEnumerable<LastCommitAdapter>> query;
    if (address.IsGitHubDotCom())
    {
    if (readCommitStatuses == null)
    {
    readCommitStatuses = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .PullRequest(number: Var(nameof(number))).Commits(last: 1).Nodes.Select(
    commit => new LastCommitAdapter
    {
    HeadSha = commit.Commit.Oid,
    CheckSuites = commit.Commit.CheckSuites(null, null, null, null, null).AllPages(10)
    .Select(suite => new CheckSuiteModel
    {
    CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10)
    .Select(run => new CheckRunModel
    {
    Id = run.Id.Value,
    Conclusion = run.Conclusion.FromGraphQl(),
    Status = run.Status.FromGraphQl(),
    Name = run.Name,
    DetailsUrl = run.Permalink,
    Summary = run.Summary,
    Text = run.Text,
    Annotations = run.Annotations(null, null, null, null).AllPages()
    .Select(annotation => new CheckRunAnnotationModel
    {
    Title = annotation.Title,
    Message = annotation.Message,
    Path = annotation.Path,
    AnnotationLevel = annotation.AnnotationLevel.Value.FromGraphQl(),
    StartLine = annotation.Location.Start.Line,
    EndLine = annotation.Location.End.Line,
    }).ToList()
    }).ToList(),
    ApplicationName = suite.App != null ? suite.App.Name : "Private App"
    }).ToList(),
    Statuses = commit.Commit.Status
    .Select(context =>
    context.Contexts.Select(statusContext => new StatusModel
    {
    State = statusContext.State.FromGraphQl(),
    Context = statusContext.Context,
    TargetUrl = statusContext.TargetUrl,
    Description = statusContext.Description
    }).ToList()
    ).SingleOrDefault()
    }
    ).Compile();
    }
    query = readCommitStatuses;
    }
    else
    {
    if (readCommitStatusesEnterprise == null)
    {
    readCommitStatusesEnterprise = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .PullRequest(number: Var(nameof(number))).Commits(last: 1).Nodes.Select(
    commit => new LastCommitAdapter
    {
    Statuses = commit.Commit.Status == null ? null : commit.Commit.Status
    .Select(context => context == null
    ? null
    : context.Contexts
    .Select(statusContext => new StatusModel
    {
    State = statusContext.State.FromGraphQl(),
    Context = statusContext.Context,
    TargetUrl = statusContext.TargetUrl,
    Description = statusContext.Description,
    }).ToList()
    ).SingleOrDefault()
    }
    ).Compile();
    }
    query = readCommitStatusesEnterprise;
    }
    var vars = new Dictionary<string, object>
    {
    { nameof(owner), owner },
    { nameof(name), name },
    { nameof(number), number },
    };
    var connection = await graphqlFactory.CreateConnection(address);
    var result = await connection.Run(query, vars, refresh);
    return result.First();
    }

Functionality to get a list of viewer repositories for cloning

  • Handled by: Caching for default 8 hours and clear on refresh
    public async Task<ViewerRepositoriesModel> ReadViewerRepositories(HostAddress address)
    {
    if (readViewerRepositories == null)
    {
    var order = new RepositoryOrder
    {
    Field = RepositoryOrderField.PushedAt,
    Direction = OrderDirection.Desc
    };
    var repositorySelection = new Fragment<Repository, RepositoryListItemModel>(
    "repository",
    repo => new RepositoryListItemModel
    {
    IsFork = repo.IsFork,
    IsPrivate = repo.IsPrivate,
    Name = repo.Name,
    Owner = repo.Owner.Login,
    Url = new Uri(repo.Url),
    });
    readViewerRepositories = new Query()
    .Viewer
    .Select(viewer => new ViewerRepositoriesModel
    {
    Owner = viewer.Login,
    Repositories = viewer.Repositories(null, null, null, null, null, null, null, order, null, null)
    .AllPages()
    .Select(repositorySelection).ToList(),
    ContributedToRepositories = viewer.RepositoriesContributedTo(100, null, null, null, null, null, null, order, null)
    .Nodes
    .Select(repositorySelection).ToList(),
    Organizations = viewer.Organizations(null, null, null, null).AllPages().Select(org => new
    {
    org.Login,
    Repositories = org.Repositories(100, null, null, null, null, null, null, order, null, null)
    .Nodes
    .Select(repositorySelection).ToList()
    }).ToDictionary(x => x.Login, x => (IReadOnlyList<RepositoryListItemModel>)x.Repositories),
    }).Compile();
    }
    var graphql = await graphqlFactory.CreateConnection(address).ConfigureAwait(false);
    var result = await graphql.Run(readViewerRepositories).ConfigureAwait(false);
    return result;
    }

Functionality to get a pull request node id

  • Handled by: Caching for default 8 hours
    public async Task<string> GetGraphQLPullRequestId(
    LocalRepositoryModel localRepository,
    string repositoryOwner,
    int number)
    {
    var address = HostAddress.Create(localRepository.CloneUrl.Host);
    var graphql = await graphqlFactory.CreateConnection(address);
    var query = new Query()
    .Repository(owner: repositoryOwner, name: localRepository.Name)
    .PullRequest(number)
    .Select(x => x.Id);
    return (await graphql.Run(query)).Value;
    }

Functionality to get assignable users

  • Handled by: Caching for 1 hour
    public async Task<Page<ActorModel>> ReadAssignableUsers(
    HostAddress address,
    string owner,
    string name,
    string after)
    {
    if (readAssignableUsers == null)
    {
    readAssignableUsers = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .AssignableUsers(first: 100, after: Var(nameof(after)))
    .Select(connection => new Page<ActorModel>
    {
    EndCursor = connection.PageInfo.EndCursor,
    HasNextPage = connection.PageInfo.HasNextPage,
    TotalCount = connection.TotalCount,
    Items = connection.Nodes.Select(user => new ActorModel
    {
    AvatarUrl = user.AvatarUrl(30),
    Login = user.Login,
    }).ToList(),
    }).Compile();
    }
    var graphql = await graphqlFactory.CreateConnection(address);
    var vars = new Dictionary<string, object>
    {
    { nameof(owner), owner },
    { nameof(name), name },
    { nameof(after), after },
    };
    return await graphql.Run(readAssignableUsers, vars);
    }

Functionality to get the viewer details

  • Handled by: Caching for 10 minutes
    public virtual async Task<ActorModel> ReadViewer(HostAddress address)
    {
    if (readViewer == null)
    {
    readViewer = new Query()
    .Viewer
    .Select(x => new ActorModel
    {
    Login = x.Login,
    AvatarUrl = x.AvatarUrl(null),
    }).Compile();
    }
    var connection = await graphqlFactory.CreateConnection(address);
    return await connection.Run(readViewer);
    }

Functionality to get a parent repo if one exists

  • Handled by: Caching for default 8 hours
    public async Task<(string owner, string name)?> FindParent(HostAddress address, string owner, string name)
    {
    Guard.ArgumentNotNull(address, nameof(address));
    Guard.ArgumentNotEmptyString(owner, nameof(owner));
    Guard.ArgumentNotEmptyString(name, nameof(name));
    if (readParentOwnerLogin == null)
    {
    readParentOwnerLogin = new Query()
    .Repository(owner: Var(nameof(owner)), name: Var(nameof(name)))
    .Select(r => r.Parent != null ? Tuple.Create(r.Parent.Owner.Login, r.Parent.Name) : null)
    .Compile();
    }
    var vars = new Dictionary<string, object>
    {
    { nameof(owner), owner },
    { nameof(name), name },
    };
    var graphql = await graphqlFactory.CreateConnection(address);
    var result = await graphql.Run(readParentOwnerLogin, vars);
    return result != null ? (result.Item1, result.Item2) : ((string, string)?)null;
    }
@StanleyGoldman
Copy link
Contributor

left a comment

The refresh flag should be chained down to this call

var lastCommitModel = await log.TimeAsync(nameof(GetPullRequestLastCommitAdapter),
() => GetPullRequestLastCommitAdapter(address, owner, name, number));

Fixed

@StanleyGoldman
Copy link
Contributor

left a comment

Minor nitpick

Show resolved Hide resolved src/GitHub.App/Services/PullRequestService.cs
@@ -293,6 +294,14 @@ public class PullRequestService : IssueishService, IPullRequestService, IStaticR
return result;
}

public async Task ClearPullRequestsCache(HostAddress address, string owner, string name)
{
var region = owner + '/' + name + "/pr-list";

This comment has been minimized.

Copy link
@StanleyGoldman

StanleyGoldman Mar 1, 2019

Contributor

With that.

StanleyGoldman added some commits Mar 8, 2019

@StanleyGoldman StanleyGoldman referenced this pull request Mar 26, 2019

Open

[wip] Adding Caching to the clone repo dialog #2301

0 of 1 task complete

@StanleyGoldman StanleyGoldman changed the title WIP: GraphQL caching. GraphQL caching. Mar 27, 2019

StanleyGoldman added some commits Mar 27, 2019

Merge remote-tracking branch 'origin/master' into feature/graphql-cac…
…hing

# Conflicts:
#	src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

I think it's good, but i'm probably too close now to evaluate

StanleyGoldman added some commits Mar 27, 2019

@jcansdale
Copy link
Collaborator

left a comment

Looks good to me. Just a question about removing a Visual Studio threading dependency.

Show resolved Hide resolved src/GitHub.Api/GraphQLClient.cs Outdated
@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

While I was prepping to present at the Visual Studio 2019 launch, my head had a thought about how the caching would affect the fork function and what routine is that using...

StanleyGoldman added some commits Apr 4, 2019

@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

GraphQL is not used to fork repos

@jcansdale
Copy link
Collaborator

left a comment

Let's do this!

@jcansdale jcansdale merged commit 794ae47 into master Apr 4, 2019

3 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
github.VisualStudio Build #20190404.11 succeeded
Details

@jcansdale jcansdale deleted the feature/graphql-caching branch Apr 4, 2019

@StanleyGoldman StanleyGoldman changed the title GraphQL caching. Caching GraphQL queries Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.