diff --git a/ApiReview.Server.Logic/ApiReview.Server.Logic.csproj b/ApiReview.Server.Logic/ApiReview.Server.Logic.csproj index c73d053..297b137 100644 --- a/ApiReview.Server.Logic/ApiReview.Server.Logic.csproj +++ b/ApiReview.Server.Logic/ApiReview.Server.Logic.csproj @@ -8,6 +8,7 @@ + diff --git a/ApiReview.Server.Logic/GitHubClientFactory.cs b/ApiReview.Server.Logic/GitHubClientFactory.cs index f865cb5..453dd34 100644 --- a/ApiReview.Server.Logic/GitHubClientFactory.cs +++ b/ApiReview.Server.Logic/GitHubClientFactory.cs @@ -1,7 +1,7 @@ -using System; -using System.Threading.Tasks; +using Octokit; -using Octokit; +using QConnection = Octokit.GraphQL.Connection; +using QProductHeaderValue = Octokit.GraphQL.ProductHeaderValue; namespace ApiReview.Server.Logic { @@ -10,36 +10,20 @@ internal static class GitHubClientFactory public static GitHubClient Create() { var key = ApiKeyStore.GetApiKey(); - return Create(key); - } - - public static GitHubClient Create(string apiKey) - { var productInformation = new ProductHeaderValue("APIReviewList"); var client = new GitHubClient(productInformation) { - Credentials = new Credentials(apiKey) + Credentials = new Credentials(key) }; return client; } - public static async Task IsValidKeyAsync(string apiKey) + public static QConnection CreateGraph() { - try - { - var client = Create(apiKey); - var request = new IssueRequest - { - Since = DateTimeOffset.Now - }; - await client.Issue.GetAllForCurrent(request); - } - catch (Exception) - { - return false; - } - - return true; + var key = ApiKeyStore.GetApiKey(); + var productInformation = new QProductHeaderValue("APIReviewList"); + var connection = new QConnection(productInformation, key); + return connection; } } } diff --git a/ApiReview.Server.Logic/GitHubManager.cs b/ApiReview.Server.Logic/GitHubManager.cs index 3e95b25..aa19d1b 100644 --- a/ApiReview.Server.Logic/GitHubManager.cs +++ b/ApiReview.Server.Logic/GitHubManager.cs @@ -1,12 +1,18 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.InteropServices; using System.Text.Json; using System.Threading.Tasks; using ApiReview.Shared; using Octokit; +using Octokit.GraphQL; +using Octokit.GraphQL.Model; + +using Issue = Octokit.Issue; +using static Octokit.GraphQL.Variable; namespace ApiReview.Server.Logic { @@ -53,12 +59,12 @@ public Task> GetFeedbackAsync(DateTimeOffset st private static async Task> GetFeedbackAsync(OrgAndRepo[] repos, DateTimeOffset start, DateTimeOffset end) { - static string GetApiStatus(Issue issue) + static string GetApiStatus(FeedbackIssue issue) { var isReadyForReview = issue.Labels.Any(l => l.Name == "api-ready-for-review"); var isApproved = issue.Labels.Any(l => l.Name == "api-approved"); var needsWork = issue.Labels.Any(l => l.Name == "api-needs-work"); - var isRejected = isReadyForReview && issue.State.Value == ItemState.Closed; + var isRejected = isReadyForReview && issue.State == IssueState.Open; var isApi = isApproved || needsWork || isRejected; @@ -74,47 +80,46 @@ static string GetApiStatus(Issue issue) return "Needs Work"; } - static bool WasEverReadyForReview(Issue issue, IEnumerable events) + static bool WasEverReadyForReview(FeedbackIssue issue) { if (issue.Labels.Any(l => l.Name == "api-ready-for-review" || l.Name == "api-approved")) return true; - foreach (var eventInfo in events) + foreach (var timelineItem in issue.TimelineItems) { - if (eventInfo.Label?.Name == "api-ready-for-review" || - eventInfo.Label?.Name == "api-approved") - return true; + if (timelineItem is ApiTimelineLabel labelItem) + { + if (labelItem.Name == "api-ready-for-review" || + labelItem.Name == "api-approved") + return true; + } } return false; } - static bool IsApiEvent(EventInfo eventInfo) + static bool IsApiEvent(ApiTimelineItem item) { - // We need to work around unsupported enum values: - // - https://github.com/octokit/octokit.net/issues/2023 - // - https://github.com/octokit/octokit.net/issues/2025 - // - // which will cause Value to throw an exception. - - switch (eventInfo.Event.StringValue) + if (item is ApiTimelineLabel l) { - case "labeled": - if (eventInfo.Label.Name == "api-approved" || eventInfo.Label.Name == "api-needs-work") - return true; - break; - case "closed": + if (l.Name == "api-approved" || l.Name == "api-needs-work") return true; } + else if (item is ApiTimelineClosure c) + { + return true; + } return false; } - static IEnumerable GetApiEvents(IEnumerable events, DateTimeOffset start, DateTimeOffset end) + static IEnumerable GetApiEvents(FeedbackIssue issue, DateTimeOffset start, DateTimeOffset end) { - foreach (var eventGroup in events.Where(e => start <= e.CreatedAt && e.CreatedAt <= end && IsApiEvent(e)) - .GroupBy(e => e.CreatedAt.Date)) + foreach (var eventGroup in issue.TimelineItems + .Where(e => e != null) + .Where(e => start <= e.CreatedAt && e.CreatedAt <= end && IsApiEvent(e)) + .GroupBy(e => e.CreatedAt.Date)) { var latest = eventGroup.OrderBy(e => e.CreatedAt).Last(); yield return latest; @@ -143,60 +148,133 @@ static IEnumerable GetApiEvents(IEnumerable events, DateTi return (null, body); } - var github = GitHubClientFactory.Create(); - var results = new List(); - - foreach (var (owner, repo) in repos) + static ApiReviewIssue CreateIssue(FeedbackIssue issue) { - var request = new RepositoryIssueRequest + var result = new ApiReviewIssue { - Filter = IssueFilter.All, - State = ItemStateFilter.All, - Since = start + Owner = issue.Owner, + Repo = issue.Repo, + Author = issue.Author, + CreatedAt = issue.CreateAt, + Labels = issue.Labels.ToArray(), + //Milestone = issue.Milestone ?? "(None)", + Title = GitHubIssueHelpers.FixTitle(issue.Title), + Url = issue.Url, + Id = issue.Number }; + return result; + } - var issues = await github.Issue.GetAllForRepository(owner, repo, request); + var filter = new IssueFilters() + { + Assignee = "*", + Milestone = "*", + Since = start, + }; + var query = new Query() + .Repository(Var("repo"), Var("owner")) + .Issues(filterBy: filter) + .AllPages() + .Select(i => new FeedbackIssue + { + Owner = i.Repository.Owner.Login, + Repo = i.Repository.Name, + Number = i.Number, + Title = i.Title, + CreateAt = i.CreatedAt, + Author = i.Author.Login, + State = i.State, + //Milestone = i.Milestone.Title, + Url = i.Url, + Labels = i.Labels(null, null, null, null, null) + .AllPages() + .Select(l => new ApiReviewLabel + { + Name = l.Name, + BackgroundColor = l.Color + }).ToList(), + TimelineItems = i + .TimelineItems(null, null, null, null, null, start, null) + .AllPages() + .Select(tl => tl == null ? null : tl.Switch(when => + when.IssueComment(ic => new ApiTimelineComment + { + Id = ic.Id.Value, + Body = ic.Body, + Url = ic.Url, + Actor = ic.Author.Login, + CreatedAt = ic.CreatedAt + }).LabeledEvent(l => new ApiTimelineLabel + { + Name = l.Label.Name, + Actor = l.Actor.Login, + CreatedAt = l.CreatedAt + }).ClosedEvent(c => new ApiTimelineClosure + { + Actor = c.Actor.Login, + CreatedAt = c.CreatedAt + }))).ToList() + }).Compile(); - foreach (var issue in issues) + var connection = GitHubClientFactory.CreateGraph(); + var vars = new Dictionary(); + + var issues = new List(); + + foreach (var ownerAndRepo in repos) + { + vars["owner"] = ownerAndRepo.OrgName; + vars["repo"] = ownerAndRepo.RepoName; + try { - var status = GetApiStatus(issue); - if (status == null) - continue; + var current = await connection.Run(query, vars); + issues.AddRange(current); + } + catch (Exception ex) + { + throw; + } + } - var events = await github.Issue.Events.GetAllForIssue(owner, repo, issue.Number); + var results = new List(); + + foreach (var issue in issues) + { + var status = GetApiStatus(issue); + if (status == null) + continue; - if (!WasEverReadyForReview(issue, events)) - continue; + if (!WasEverReadyForReview(issue)) + continue; - foreach (var apiEvent in GetApiEvents(events, start, end)) + foreach (var apiEvent in GetApiEvents(issue, start, end)) + { + var title = GitHubIssueHelpers.FixTitle(issue.Title); + var feedbackDateTime = apiEvent.CreatedAt; + var comments = issue.TimelineItems.OfType(); + var eventComment = comments.Where(c => c.Actor == apiEvent.Actor) + .Select(c => (comment: c, within: Math.Abs((c.CreatedAt - feedbackDateTime).TotalSeconds))) + .Where(c => c.within <= TimeSpan.FromMinutes(15).TotalSeconds) + .OrderBy(c => c.within) + .Select(c => c.comment) + .FirstOrDefault(); + var feedbackId = eventComment?.Id; + var feedbackUrl = eventComment?.Url ?? issue.Url; + var (videoUrl, feedbackMarkdown) = ParseFeedback(eventComment?.Body); + + var apiReviewIssue = CreateIssue(issue); + + var feedback = new ApiReviewFeedback { - var title = GitHubIssueHelpers.FixTitle(issue.Title); - var feedbackDateTime = apiEvent.CreatedAt; - var comments = await github.Issue.Comment.GetAllForIssue(owner, repo, issue.Number); - var eventComment = comments.Where(c => c.User.Login == apiEvent.Actor.Login) - .Select(c => (comment: c, within: Math.Abs((c.CreatedAt - feedbackDateTime).TotalSeconds))) - .Where(c => c.within <= TimeSpan.FromMinutes(15).TotalSeconds) - .OrderBy(c => c.within) - .Select(c => c.comment) - .FirstOrDefault(); - var feedbackId = eventComment?.Id; - var feedbackUrl = eventComment?.HtmlUrl ?? issue.HtmlUrl; - var (videoUrl, feedbackMarkdown) = ParseFeedback(eventComment?.Body); - - var apiReviewIssue = CreateIssue(owner, repo, issue); - - var feedback = new ApiReviewFeedback - { - Issue = apiReviewIssue, - FeedbackId = feedbackId, - FeedbackDateTime = feedbackDateTime, - FeedbackUrl = feedbackUrl, - FeedbackStatus = status, - FeedbackMarkdown = feedbackMarkdown, - VideoUrl = videoUrl - }; - results.Add(feedback); - } + Issue = apiReviewIssue, + FeedbackId = feedbackId, + FeedbackDateTime = feedbackDateTime, + FeedbackUrl = feedbackUrl, + FeedbackStatus = status, + FeedbackMarkdown = feedbackMarkdown, + VideoUrl = videoUrl + }; + results.Add(feedback); } } @@ -250,5 +328,48 @@ private static ApiReviewIssue CreateIssue(string owner, string repo, Issue issue }; return result; } + + private sealed class FeedbackIssue + { + public string Owner { get; set; } + public string Repo { get; set; } + public int Number { get; set; } + public DateTimeOffset CreateAt { get; set; } + public string Author { get; set; } + public string Title { get; set; } + public IssueState State { get; set; } + public string Milestone { get; set; } + public string Url { get; set; } + public List Labels { get; set; } + public List TimelineItems { get; set; } + + public override string ToString() + { + return $"{Owner}/{Repo}#{Number}: {Title}"; + } + } + + private abstract class ApiTimelineItem + { + public string Actor { get; set; } + public DateTimeOffset CreatedAt { get; set; } + } + + private sealed class ApiTimelineComment : ApiTimelineItem + { + public string Id { get; set; } + public string Body { get; set; } + public string Url { get; set; } + } + + private sealed class ApiTimelineLabel : ApiTimelineItem + { + public string Name { get; set; } + public ApiReviewLabel Label { get; set; } + } + + private sealed class ApiTimelineClosure : ApiTimelineItem + { + } } } diff --git a/ApiReview.Server.Logic/NoteSharingService.cs b/ApiReview.Server.Logic/NoteSharingService.cs index ae82b25..e2c9cb9 100644 --- a/ApiReview.Server.Logic/NoteSharingService.cs +++ b/ApiReview.Server.Logic/NoteSharingService.cs @@ -1,4 +1,5 @@ -using System.IO; +using System; +using System.IO; using System.Linq; using System.Threading.Tasks; @@ -71,7 +72,8 @@ private static async Task UpdateCommentsAsync(ApiReviewSummary summary) if (feedback.VideoUrl == null && feedback.FeedbackId != null) { var updatedMarkdown = $"[Video]({item.VideoTimeCodeUrl})\n\n{feedback.FeedbackMarkdown}"; - await github.Issue.Comment.Update(feedback.Issue.Owner, feedback.Issue.Repo, feedback.FeedbackId.Value, updatedMarkdown); + var commentId = Convert.ToInt32(feedback.FeedbackId); + await github.Issue.Comment.Update(feedback.Issue.Owner, feedback.Issue.Repo, commentId, updatedMarkdown); } } } diff --git a/ApiReview.Shared/ApiReviewFeedback.cs b/ApiReview.Shared/ApiReviewFeedback.cs index a2b4655..406f8f8 100644 --- a/ApiReview.Shared/ApiReviewFeedback.cs +++ b/ApiReview.Shared/ApiReviewFeedback.cs @@ -6,7 +6,7 @@ public sealed class ApiReviewFeedback { public ApiReviewIssue Issue { get; set; } public DateTimeOffset FeedbackDateTime { get; set; } - public int? FeedbackId { get; set; } + public string FeedbackId { get; set; } public string FeedbackUrl { get; set; } public string FeedbackStatus { get; set; } public string FeedbackMarkdown { get; set; } diff --git a/README.md b/README.md index b1eb1d1..ccd8900 100644 --- a/README.md +++ b/README.md @@ -4,56 +4,7 @@ This site allows us to browse the backlog and share notes. ## Missing -* We should consider using GraphQL to speed up retreival of issues * Auth * Sharing notes * Extract lookup of GitHub/YouTube keys * Use a bot as the GitHub user - -## GraphQL - -This query gets us the data from GitHub: - -```text -{ - viewer { - login - } - repository(name: "runtime", owner: "dotnet") { - issues(filterBy: {since: "2020-06-25T16:54:00Z"}, first: 100) { - nodes { - number - timelineItems(first: 250, since: "2020-06-25T16:54:00Z") { - nodes { - ... on IssueComment { - id - bodyText - url - createdAt - } - ... on LabeledEvent { - id - label { - name - } - createdAt - } - } - } - labels(first: 100) { - nodes { - color - name - } - } - createdAt - author { - login - } - state - title - } - } - } -} -```