From b3fa8266985da91cb65313a110ef8820b746177c Mon Sep 17 00:00:00 2001 From: Teo Voinea <58236992+tevoinea@users.noreply.github.com> Date: Wed, 28 Jun 2023 13:24:22 -0400 Subject: [PATCH] Cleanup (#3241) --- .../onefuzzlib/notifications/Ado.cs | 59 +++++++++++-------- .../onefuzzlib/notifications/GithubIssues.cs | 51 ++++++++-------- .../notifications/JinjaTemplateAdapter.cs | 6 +- .../notifications/NotificationsBase.cs | 4 +- 4 files changed, 62 insertions(+), 58 deletions(-) diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index 836c15f638..4a8cb0a5aa 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -24,7 +24,15 @@ public async Async.Task NotifyAdo(AdoTemplate config, Contain var report = (Report)reportable; - (string, string)[] notificationInfo = { ("notification_id", notificationId.ToString()), ("job_id", report.JobId.ToString()), ("task_id", report.TaskId.ToString()), ("ado_project", config.Project), ("ado_url", config.BaseUrl.ToString()), ("container", container.String), ("filename", filename) }; + var notificationInfo = new List<(string, string)> { + ("notification_id", notificationId.ToString()), + ("job_id", report.JobId.ToString()), + ("task_id", report.TaskId.ToString()), + ("ado_project", config.Project), + ("ado_url", config.BaseUrl.ToString()), + ("container", container.String), + ("filename", filename) + }; var adoEventType = "AdoNotify"; _logTracer.AddTags(notificationInfo); @@ -130,7 +138,7 @@ public sealed class AdoConnector { public static async Async.Task AdoConnectorCreator(IOnefuzzContext context, Container container, string filename, AdoTemplate config, Report report, ILogger logTracer, Renderer? renderer = null) { renderer ??= await Renderer.ConstructRenderer(context, container, filename, report, logTracer); var instanceUrl = context.Creds.GetInstanceUrl(); - var project = await renderer.Render(config.Project, instanceUrl); + var project = renderer.Render(config.Project, instanceUrl); var authToken = await context.SecretsOperations.GetSecretValue(config.AuthToken.Secret); var client = GetAdoClient(config.BaseUrl, authToken!); @@ -147,23 +155,23 @@ public AdoConnector(AdoTemplate config, Renderer renderer, string project, WorkI _logTracer = logTracer; } - public async Async.Task Render(string template) { + public string Render(string template) { try { - return await _renderer.Render(template, _instanceUrl, strictRendering: true); + return _renderer.Render(template, _instanceUrl, strictRendering: true); } catch { _logTracer.LogWarning("Failed to render template in strict mode. Falling back to relaxed mode. {Template} ", template); - return await _renderer.Render(template, _instanceUrl, strictRendering: false); + return _renderer.Render(template, _instanceUrl, strictRendering: false); } } - public async IAsyncEnumerable ExistingWorkItems((string, string)[] notificationInfo) { + public async IAsyncEnumerable ExistingWorkItems(IList<(string, string)> notificationInfo) { var filters = new Dictionary(); foreach (var key in _config.UniqueFields) { var filter = string.Empty; if (string.Equals("System.TeamProject", key)) { - filter = await Render(_config.Project); + filter = Render(_config.Project); } else if (_config.AdoFields.TryGetValue(key, out var field)) { - filter = await Render(field); + filter = Render(field); } else { _logTracer.AddTags(notificationInfo); _logTracer.LogError("Failed to check for existing work items using the UniqueField Key: {Key}. Value is not present in config field AdoFields.", key); @@ -241,17 +249,17 @@ public async IAsyncEnumerable ExistingWorkItems((string, string)[] not } /// true if the state of the item was modified - public async Async.Task UpdateExisting(WorkItem item, (string, string)[] notificationInfo) { + public async Async.Task UpdateExisting(WorkItem item, IList<(string, string)> notificationInfo) { _logTracer.AddTags(notificationInfo); _logTracer.AddTag("ItemId", item.Id.HasValue ? item.Id.Value.ToString() : ""); - if (await MatchesUnlessCase(item)) { + if (MatchesUnlessCase(item)) { _logTracer.LogMetric("WorkItemMatchedUnlessCase", 1); return false; } if (_config.OnDuplicate.Comment != null) { - var comment = await Render(_config.OnDuplicate.Comment); + var comment = Render(_config.OnDuplicate.Comment); _ = await _client.AddCommentAsync( new CommentCreate() { Text = comment @@ -272,7 +280,7 @@ public async Async.Task UpdateExisting(WorkItem item, (string, string)[] n } foreach (var field in _config.OnDuplicate.AdoFields) { - var fieldValue = await Render(_config.OnDuplicate.AdoFields[field.Key]); + var fieldValue = Render(_config.OnDuplicate.AdoFields[field.Key]); document.Add(new JsonPatchOperation() { Operation = VisualStudio.Services.WebApi.Patch.Operation.Replace, Path = $"/fields/{field.Key}", @@ -305,23 +313,22 @@ public async Async.Task UpdateExisting(WorkItem item, (string, string)[] n return stateUpdated; } - private async Async.Task MatchesUnlessCase(WorkItem workItem) => + private bool MatchesUnlessCase(WorkItem workItem) => _config.OnDuplicate.Unless != null && - await _config.OnDuplicate.Unless.ToAsyncEnumerable() + _config.OnDuplicate.Unless // Any condition from the list may match - .AnyAwaitAsync(async condition => - await condition.ToAsyncEnumerable() - // All fields within the condition must match - .AllAwaitAsync(async kvp => - workItem.Fields.TryGetValue(kvp.Key, out var value) && - string.Equals(await Render(kvp.Value), value, StringComparison.OrdinalIgnoreCase))); + .Any(condition => condition + // All fields within the condition must match + .All(kvp => + workItem.Fields.TryGetValue(kvp.Key, out var value) && + string.Equals(Render(kvp.Value), value, StringComparison.OrdinalIgnoreCase))); private async Async.Task CreateNew() { - var (taskType, document) = await RenderNew(); + var (taskType, document) = RenderNew(); var entry = await _client.CreateWorkItemAsync(document, _project, taskType); if (_config.Comment != null) { - var comment = await Render(_config.Comment); + var comment = Render(_config.Comment); _ = await _client.AddCommentAsync( new CommentCreate() { Text = comment, @@ -332,8 +339,8 @@ private async Async.Task CreateNew() { return entry; } - private async Async.Task<(string, JsonPatchDocument)> RenderNew() { - var taskType = await Render(_config.Type); + private (string, JsonPatchDocument) RenderNew() { + var taskType = Render(_config.Type); var document = new JsonPatchDocument(); if (!_config.AdoFields.ContainsKey("System.Tags")) { document.Add(new JsonPatchOperation() { @@ -344,7 +351,7 @@ private async Async.Task CreateNew() { } foreach (var field in _config.AdoFields.Keys) { - var value = await Render(_config.AdoFields[field]); + var value = Render(_config.AdoFields[field]); if (string.Equals(field, "System.Tags")) { value += ";Onefuzz"; @@ -360,7 +367,7 @@ private async Async.Task CreateNew() { return (taskType, document); } - public async Async.Task Process((string, string)[] notificationInfo) { + public async Async.Task Process(IList<(string, string)> notificationInfo) { var updated = false; WorkItem? oldestWorkItem = null; await foreach (var workItem in ExistingWorkItems(notificationInfo)) { diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs index 6d4e6e04a7..d6bb689681 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs @@ -65,7 +65,7 @@ private static GitHubClient GetGitHubClient(string user, string pat) { private async Async.Task Process(GithubIssuesTemplate config, Container container, string filename, Report report) { var renderer = await Renderer.ConstructRenderer(_context, container, filename, report, _logTracer); - var handler = await GithubConnnector.GithubConnnectorCreator(config, container, filename, renderer, _context.Creds.GetInstanceUrl(), _context, _logTracer); + var handler = await GithubConnnector.GithubConnnectorCreator(config, renderer, _context.Creds.GetInstanceUrl(), _context, _logTracer); await handler.Process(); } sealed class GithubConnnector { @@ -75,12 +75,9 @@ sealed class GithubConnnector { private readonly Uri _instanceUrl; private readonly ILogger _logTracer; - public static async Async.Task GithubConnnectorCreator(GithubIssuesTemplate config, Container container, string filename, Renderer renderer, Uri instanceUrl, IOnefuzzContext context, ILogger logTracer) { + public static async Async.Task GithubConnnectorCreator(GithubIssuesTemplate config, Renderer renderer, Uri instanceUrl, IOnefuzzContext context, ILogger logTracer) { var auth = await context.SecretsOperations.GetSecretValue(config.Auth.Secret); - if (auth == null) { - throw new Exception($"Failed to retrieve the auth info for {config}"); - } - return new GithubConnnector(config, renderer, instanceUrl, auth, logTracer); + return new GithubConnnector(config, renderer, instanceUrl, auth!, logTracer); } public GithubConnnector(GithubIssuesTemplate config, Renderer renderer, Uri instanceUrl, GithubAuth auth, ILogger logTracer) { @@ -100,32 +97,32 @@ public async Async.Task Process() { } } - private async Async.Task Render(string field) { + private string Render(string field) { try { - return await _renderer.Render(field, _instanceUrl, strictRendering: true); + return _renderer.Render(field, _instanceUrl, strictRendering: true); } catch { _logTracer.LogWarning("Failed to render field in strict mode. Falling back to relaxed mode. {Field}", field); - return await _renderer.Render(field, _instanceUrl, strictRendering: false); + return _renderer.Render(field, _instanceUrl, strictRendering: false); } } private async Async.Task> Existing() { var query = new List() { "is:issue", - await Render(_config.UniqueSearch.str), + Render(_config.UniqueSearch.str), $"repo:{_config.Organization}/{_config.Repository}" }; if (_config.UniqueSearch.Author != null) { - query.Add($"author:{await Render(_config.UniqueSearch.Author)}"); + query.Add($"author:{Render(_config.UniqueSearch.Author)}"); } if (_config.UniqueSearch.State != null) { query.Add($"state:{_config.UniqueSearch.State}"); } - var title = await Render(_config.Title); - var body = await Render(_config.Body); + var title = Render(_config.Title); + var body = Render(_config.Body); var issues = new List(); var t = await _gh.Search.SearchIssues(new SearchIssuesRequest(string.Join(' ', query))); foreach (var issue in t.Items) { @@ -150,12 +147,12 @@ await Render(_config.UniqueSearch.str), private async Async.Task Update(Issue issue) { _logTracer.LogInformation("updating issue: {Issue}", issue); if (_config.OnDuplicate.Comment != null) { - _ = await _gh.Issue.Comment.Create(issue.Repository.Id, issue.Number, await Render(_config.OnDuplicate.Comment)); + _ = await _gh.Issue.Comment.Create(issue.Repository.Id, issue.Number, Render(_config.OnDuplicate.Comment)); } if (_config.OnDuplicate.Labels.Any()) { - var labels = await _config.OnDuplicate.Labels.ToAsyncEnumerable() - .SelectAwait(async label => await Render(label)) - .ToArrayAsync(); + var labels = _config.OnDuplicate.Labels + .Select(Render) + .ToArray(); _ = await _gh.Issue.Labels.ReplaceAllForIssue(issue.Repository.Id, issue.Number, labels); } @@ -168,26 +165,26 @@ private async Async.Task Update(Issue issue) { private async Async.Task Create() { _logTracer.LogInformation("creating issue"); - var assignees = await _config.Assignees.ToAsyncEnumerable() - .SelectAwait(async assignee => await Render(assignee)) - .ToListAsync(); + var assignees = _config.Assignees + .Select(assignee => Render(assignee)) + .ToList(); - var labels = await _config.Labels.ToAsyncEnumerable() - .SelectAwait(async label => await Render(label)) - .ToHashSetAsync(); + var labels = _config.Labels + .Select(label => Render(label)) + .ToHashSet(); _ = labels.Add("OneFuzz"); - var newIssue = new NewIssue(await Render(_config.Title)) { - Body = await Render(_config.Body), + var newIssue = new NewIssue(Render(_config.Title)) { + Body = Render(_config.Body), }; labels.ToList().ForEach(label => newIssue.Labels.Add(label)); assignees.ForEach(assignee => newIssue.Assignees.Add(assignee)); _ = await _gh.Issue.Create( - await Render(_config.Organization), - await Render(_config.Repository), + Render(_config.Organization), + Render(_config.Repository), newIssue); } } diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs b/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs index 94c9f824d6..c72b9b650e 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs @@ -37,7 +37,7 @@ public static async Async.Task ValidateScribanTempla var (renderer, templateRenderContext) = await GenerateTemplateRenderContext(context, log, renderContext); - var renderedTemaplate = await renderer.Render(template, new Uri(instanceUrl), strictRendering: true); + var renderedTemaplate = renderer.Render(template, new Uri(instanceUrl), strictRendering: true); return new TemplateValidationResponse( renderedTemaplate, @@ -240,7 +240,7 @@ public static async Async.Task ValidateScribanTempla return (renderer, templateRenderContext); } - public async static Async.Task<(bool didModify, AdoTemplate template)> ConvertToScriban(AdoTemplate template, bool attemptRender = false, IOnefuzzContext? context = null, ILogger? log = null) { + public static async Async.Task<(bool didModify, AdoTemplate template)> ConvertToScriban(AdoTemplate template, bool attemptRender = false, IOnefuzzContext? context = null, ILogger? log = null) { if (attemptRender) { context = context.EnsureNotNull("Required to render"); log = log.EnsureNotNull("Required to render"); @@ -311,7 +311,7 @@ public static async Async.Task ValidateScribanTempla return (didModify, template); } - public async static Async.Task<(bool didModify, GithubIssuesTemplate template)> ConvertToScriban(GithubIssuesTemplate template, bool attemptRender = false, IOnefuzzContext? context = null, ILogger? log = null) { + public static async Async.Task<(bool didModify, GithubIssuesTemplate template)> ConvertToScriban(GithubIssuesTemplate template, bool attemptRender = false, IOnefuzzContext? context = null, ILogger? log = null) { if (attemptRender) { context = context.EnsureNotNull("Required to render"); log = log.EnsureNotNull("Required to render"); diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs b/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs index 44a3ecad28..a8ee96aa0b 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs @@ -117,7 +117,7 @@ public Renderer( // TODO: This function is fallible but the python // implementation doesn't have that so I'm trying to match it. // We should probably propagate any errors up - public async Async.Task Render(string templateString, Uri instanceUrl, bool strictRendering = false) { + public string Render(string templateString, Uri instanceUrl, bool strictRendering = false) { if (!_scribanOnly && JinjaTemplateAdapter.IsJinjaTemplate(templateString)) { templateString = JinjaTemplateAdapter.AdaptForScriban(templateString); } @@ -149,7 +149,7 @@ public async Async.Task Render(string templateString, Uri instanceUrl, b var template = Template.Parse(templateString); if (template != null) { - return await template.RenderAsync(context); + return template.Render(context); } return string.Empty; }