From 730fa8167b17ff66f96555921d8b1a2e4b6c0e92 Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 24 Oct 2019 01:07:31 +1000 Subject: [PATCH 1/6] Update Octopus.Server.Extensibility version --- source/Server.Tests/Server.Tests.csproj | 2 +- source/Server.Tests/WorkItemLinkMapperScenarios.cs | 3 ++- source/Server/Server.csproj | 2 +- source/Server/WorkItems/WorkItemLinkMapper.cs | 5 +++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/source/Server.Tests/Server.Tests.csproj b/source/Server.Tests/Server.Tests.csproj index 80c9de0..0ca46d2 100644 --- a/source/Server.Tests/Server.Tests.csproj +++ b/source/Server.Tests/Server.Tests.csproj @@ -13,7 +13,7 @@ - + diff --git a/source/Server.Tests/WorkItemLinkMapperScenarios.cs b/source/Server.Tests/WorkItemLinkMapperScenarios.cs index a05cdbe..4a0a383 100644 --- a/source/Server.Tests/WorkItemLinkMapperScenarios.cs +++ b/source/Server.Tests/WorkItemLinkMapperScenarios.cs @@ -1,6 +1,7 @@ using System; using NSubstitute; using NUnit.Framework; +using Octopus.Diagnostics; using Octopus.Server.Extensibility.Extensions; using Octopus.Server.Extensibility.HostServices.Model.BuildInformation; using Octopus.Server.Extensibility.HostServices.Model.IssueTrackers; @@ -29,7 +30,7 @@ public void WhenDisabledReturnsNull() var links = CreateWorkItemLinkMapper(false).Map(new OctopusBuildInformation { BuildUrl = "http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24" - }); + }, Substitute.For()); Assert.IsTrue(links.Succeeded); Assert.IsNull(links.Value); } diff --git a/source/Server/Server.csproj b/source/Server/Server.csproj index 2a08f2a..3824317 100644 --- a/source/Server/Server.csproj +++ b/source/Server/Server.csproj @@ -16,7 +16,7 @@ - + diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index 5cee8a0..4b6e1f4 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -1,4 +1,5 @@ -using Octopus.Server.Extensibility.Extensions; +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.Extensions; using Octopus.Server.Extensibility.Extensions.WorkItems; using Octopus.Server.Extensibility.HostServices.Model.BuildInformation; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients; @@ -21,7 +22,7 @@ public WorkItemLinkMapper(IAzureDevOpsConfigurationStore store, IAdoApiClient cl public string CommentParser => AzureDevOpsConfigurationStore.CommentParser; public bool IsEnabled => store.GetIsEnabled(); - public SuccessOrErrorResult Map(OctopusBuildInformation buildInformation) + public SuccessOrErrorResult Map(OctopusBuildInformation buildInformation, ILogWithContext log) { // For ADO, we should ignore anything that wasn't built by ADO because we get work items from the build if (!IsEnabled From 1581a959630bf70c3df4fc87ab8146d3b0e63f99 Mon Sep 17 00:00:00 2001 From: David Young Date: Thu, 24 Oct 2019 10:37:54 +1000 Subject: [PATCH 2/6] Pass packageId, version to issue trackers for clearer log messages --- source/Server.Tests/Server.Tests.csproj | 2 +- source/Server.Tests/WorkItemLinkMapperScenarios.cs | 3 ++- source/Server/Server.csproj | 2 +- source/Server/WorkItems/WorkItemLinkMapper.cs | 3 ++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/source/Server.Tests/Server.Tests.csproj b/source/Server.Tests/Server.Tests.csproj index 0ca46d2..f50adc2 100644 --- a/source/Server.Tests/Server.Tests.csproj +++ b/source/Server.Tests/Server.Tests.csproj @@ -13,7 +13,7 @@ - + diff --git a/source/Server.Tests/WorkItemLinkMapperScenarios.cs b/source/Server.Tests/WorkItemLinkMapperScenarios.cs index 4a0a383..a72d2a8 100644 --- a/source/Server.Tests/WorkItemLinkMapperScenarios.cs +++ b/source/Server.Tests/WorkItemLinkMapperScenarios.cs @@ -9,6 +9,7 @@ using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems; using Octopus.Server.Extensibility.Resources.IssueTrackers; +using Octopus.Versioning.Semver; namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Tests { @@ -27,7 +28,7 @@ private WorkItemLinkMapper CreateWorkItemLinkMapper(bool enabled) [Test] public void WhenDisabledReturnsNull() { - var links = CreateWorkItemLinkMapper(false).Map(new OctopusBuildInformation + var links = CreateWorkItemLinkMapper(false).Map("Deployable", new SemanticVersion("1.0"), new OctopusBuildInformation { BuildUrl = "http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24" }, Substitute.For()); diff --git a/source/Server/Server.csproj b/source/Server/Server.csproj index 3824317..1fa1f29 100644 --- a/source/Server/Server.csproj +++ b/source/Server/Server.csproj @@ -16,7 +16,7 @@ - + diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index 4b6e1f4..94a7f8c 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -5,6 +5,7 @@ using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; using Octopus.Server.Extensibility.Resources.IssueTrackers; +using Octopus.Versioning; namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems { @@ -22,7 +23,7 @@ public WorkItemLinkMapper(IAzureDevOpsConfigurationStore store, IAdoApiClient cl public string CommentParser => AzureDevOpsConfigurationStore.CommentParser; public bool IsEnabled => store.GetIsEnabled(); - public SuccessOrErrorResult Map(OctopusBuildInformation buildInformation, ILogWithContext log) + public SuccessOrErrorResult Map(string packageId, IVersion version, OctopusBuildInformation buildInformation, ILogWithContext log) { // For ADO, we should ignore anything that wasn't built by ADO because we get work items from the build if (!IsEnabled From 292520065860e9b681960b6987539f68075463fc Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 25 Oct 2019 17:16:02 +1000 Subject: [PATCH 3/6] Log reasons for aborting, catch & return parsing errors --- source/Server/WorkItems/WorkItemLinkMapper.cs | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index 94a7f8c..e0dde72 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -1,4 +1,5 @@ -using Octopus.Diagnostics; +using System; +using Octopus.Diagnostics; using Octopus.Server.Extensibility.Extensions; using Octopus.Server.Extensibility.Extensions.WorkItems; using Octopus.Server.Extensibility.HostServices.Model.BuildInformation; @@ -25,13 +26,42 @@ public WorkItemLinkMapper(IAzureDevOpsConfigurationStore store, IAdoApiClient cl public SuccessOrErrorResult Map(string packageId, IVersion version, OctopusBuildInformation buildInformation, ILogWithContext log) { - // For ADO, we should ignore anything that wasn't built by ADO because we get work items from the build - if (!IsEnabled - || buildInformation?.BuildEnvironment != "Azure DevOps" - || string.IsNullOrWhiteSpace(buildInformation?.BuildUrl)) + if (!IsEnabled) + { + log.Verbose("Azure DevOps Issue Tracker is disabled in Settings."); return null; + } - return client.GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); + if (buildInformation == null) + { + log.Info($"No build information was found for package {packageId} {version}. To incorporate build information, and enable support for work" + + $" items and release notes generation, consider adding a Push Build Information step to your build process."); + return null; + } + + if (buildInformation.BuildEnvironment != "Azure DevOps") + { + // We are only interested in build URLs from Azure DevOps, because get use its build APIs to get associated work items + log.Verbose($"The build environment for package {packageId} {version} was '{buildInformation.BuildEnvironment}' rather than 'Azure DevOps'," + + $" so the build URL will not be checked for Azure DevOps work item associations."); + return null; + } + + if (string.IsNullOrWhiteSpace(buildInformation.BuildUrl)) + { + log.Info($"No build URL was found in the build information for package {packageId} {version}, so it will not be checked for Azure" + + $" DevOps work item associations."); + return null; + } + + try + { + return client.GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); + } + catch (Exception ex) + { + return SuccessOrErrorResult.Failure(ex.Message); + } } } } \ No newline at end of file From 651829acecf0857aa3f6952c1b794df2ea4a83f5 Mon Sep 17 00:00:00 2001 From: David Young Date: Fri, 25 Oct 2019 17:16:40 +1000 Subject: [PATCH 4/6] Log Azure DevOps client requests --- .../WorkItemLinkMapperScenarios.cs | 8 +++--- .../Server/AdoClients/AdoApiClientFactory.cs | 28 +++++++++++++++++++ source/Server/AdoClients/HttpJsonClient.cs | 9 ++++++ .../AzureDevOpsIssueTrackerExtension.cs | 4 +++ source/Server/WorkItems/WorkItemLinkMapper.cs | 8 +++--- 5 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 source/Server/AdoClients/AdoApiClientFactory.cs diff --git a/source/Server.Tests/WorkItemLinkMapperScenarios.cs b/source/Server.Tests/WorkItemLinkMapperScenarios.cs index a72d2a8..418a643 100644 --- a/source/Server.Tests/WorkItemLinkMapperScenarios.cs +++ b/source/Server.Tests/WorkItemLinkMapperScenarios.cs @@ -2,13 +2,10 @@ using NSubstitute; using NUnit.Framework; using Octopus.Diagnostics; -using Octopus.Server.Extensibility.Extensions; using Octopus.Server.Extensibility.HostServices.Model.BuildInformation; -using Octopus.Server.Extensibility.HostServices.Model.IssueTrackers; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems; -using Octopus.Server.Extensibility.Resources.IssueTrackers; using Octopus.Versioning.Semver; namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Tests @@ -22,7 +19,9 @@ private WorkItemLinkMapper CreateWorkItemLinkMapper(bool enabled) config.GetIsEnabled().Returns(enabled); var adoApiClient = Substitute.For(); adoApiClient.GetBuildWorkItemLinks(null).ReturnsForAnyArgs(ci => throw new InvalidOperationException()); - return new WorkItemLinkMapper(config, adoApiClient); + var clientFactory = Substitute.For(); + clientFactory.CreateWithLog(null).ReturnsForAnyArgs(ci => adoApiClient); + return new WorkItemLinkMapper(config, clientFactory); } [Test] @@ -30,6 +29,7 @@ public void WhenDisabledReturnsNull() { var links = CreateWorkItemLinkMapper(false).Map("Deployable", new SemanticVersion("1.0"), new OctopusBuildInformation { + BuildEnvironment = "Azure DevOps", BuildUrl = "http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24" }, Substitute.For()); Assert.IsTrue(links.Succeeded); diff --git a/source/Server/AdoClients/AdoApiClientFactory.cs b/source/Server/AdoClients/AdoApiClientFactory.cs new file mode 100644 index 0000000..87f54ba --- /dev/null +++ b/source/Server/AdoClients/AdoApiClientFactory.cs @@ -0,0 +1,28 @@ +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; +using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems; + +namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients +{ + public interface IAdoApiClientFactory + { + IAdoApiClient CreateWithLog(ILogWithContext log); + } + + public class AdoApiClientFactory : IAdoApiClientFactory + { + private readonly IAzureDevOpsConfigurationStore store; + private readonly HtmlConvert htmlConvert; + + public AdoApiClientFactory(IAzureDevOpsConfigurationStore store, HtmlConvert htmlConvert) + { + this.store = store; + this.htmlConvert = htmlConvert; + } + + public IAdoApiClient CreateWithLog(ILogWithContext log) + { + return new AdoApiClient(store, new HttpJsonClient(log), htmlConvert); + } + } +} \ No newline at end of file diff --git a/source/Server/AdoClients/HttpJsonClient.cs b/source/Server/AdoClients/HttpJsonClient.cs index 824e753..e571881 100644 --- a/source/Server/AdoClients/HttpJsonClient.cs +++ b/source/Server/AdoClients/HttpJsonClient.cs @@ -4,6 +4,7 @@ using System.Net.Http.Headers; using System.Text; using Newtonsoft.Json.Linq; +using Octopus.Diagnostics; namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients { @@ -51,8 +52,14 @@ public interface IHttpJsonClient : IDisposable public sealed class HttpJsonClient : IHttpJsonClient { + private readonly ILogWithContext log; private readonly HttpClient httpClient = new HttpClient(); + public HttpJsonClient(ILogWithContext log) + { + this.log = log; + } + public (HttpJsonClientStatus status, JObject jObject) Get(string url, string basicPassword = null) { var request = new HttpRequestMessage(HttpMethod.Get, url); @@ -63,6 +70,8 @@ public sealed class HttpJsonClient : IHttpJsonClient Convert.ToBase64String(Encoding.UTF8.GetBytes(":" + basicPassword))); } + log.Trace($"Azure DevOps client request: GET {url} ({(string.IsNullOrEmpty(basicPassword) ? "unauthenticated" : "authenticated")})"); + HttpResponseMessage response; try { diff --git a/source/Server/AzureDevOpsIssueTrackerExtension.cs b/source/Server/AzureDevOpsIssueTrackerExtension.cs index b7f324a..7feb249 100644 --- a/source/Server/AzureDevOpsIssueTrackerExtension.cs +++ b/source/Server/AzureDevOpsIssueTrackerExtension.cs @@ -50,6 +50,10 @@ public void Load(ContainerBuilder builder) .As() .InstancePerLifetimeScope(); + builder.RegisterType() + .As() + .InstancePerLifetimeScope(); + builder.RegisterType() .As() .InstancePerDependency(); diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index e0dde72..9e98d3d 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -13,12 +13,12 @@ namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems public class WorkItemLinkMapper : IWorkItemLinkMapper { private readonly IAzureDevOpsConfigurationStore store; - private readonly IAdoApiClient client; + private readonly IAdoApiClientFactory clientFactory; - public WorkItemLinkMapper(IAzureDevOpsConfigurationStore store, IAdoApiClient client) + public WorkItemLinkMapper(IAzureDevOpsConfigurationStore store, IAdoApiClientFactory clientFactory) { this.store = store; - this.client = client; + this.clientFactory = clientFactory; } public string CommentParser => AzureDevOpsConfigurationStore.CommentParser; @@ -56,7 +56,7 @@ public SuccessOrErrorResult Map(string packageId, IVersion versi try { - return client.GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); + return clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); } catch (Exception ex) { From e89601160598f8ed4babbea172a77e5034dd5e6b Mon Sep 17 00:00:00 2001 From: David Young Date: Mon, 28 Oct 2019 00:40:29 +1000 Subject: [PATCH 5/6] Log a hint to trace when no work items are found --- source/Server.Tests/AdoApiClientScenarios.cs | 20 +++++++++---------- source/Server/AdoClients/AdoApiClient.cs | 16 ++++++++++++--- .../Server/AdoClients/AdoApiClientFactory.cs | 2 +- source/Server/WorkItems/WorkItemLinkMapper.cs | 12 +++++------ 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/source/Server.Tests/AdoApiClientScenarios.cs b/source/Server.Tests/AdoApiClientScenarios.cs index 44c8e6b..dd5c6c0 100644 --- a/source/Server.Tests/AdoApiClientScenarios.cs +++ b/source/Server.Tests/AdoApiClientScenarios.cs @@ -13,7 +13,8 @@ namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Tests [TestFixture] public class AdoApiClientScenarios { - private static readonly HtmlConvert HtmlConvert = new HtmlConvert(Substitute.For()); + private static readonly ILogWithContext Log = Substitute.For(); + private static readonly HtmlConvert HtmlConvert = new HtmlConvert(Log); private static IAzureDevOpsConfigurationStore CreateSubstituteStore() { @@ -36,7 +37,7 @@ public void ClientCanRequestAndParseWorkItemsRefsAndLinks() .Returns((HttpStatusCode.OK, JObject.Parse(@"{""id"":2,""fields"":{""System.CommentCount"":0,""System.Title"": ""README has no useful content""}}"))); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24")); Assert.IsTrue(workItemLinks.Succeeded); @@ -46,7 +47,6 @@ public void ClientCanRequestAndParseWorkItemsRefsAndLinks() Assert.AreEqual("README has no useful content", workItemLink.Description); } - [Test] public void SourceGetsSet() { @@ -59,7 +59,7 @@ public void SourceGetsSet() .Returns((HttpStatusCode.OK, JObject.Parse(@"{""id"":2,""fields"":{""System.CommentCount"":0,""System.Title"": ""README has no useful content""}}"))); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24")); Assert.IsTrue(workItemLinks.Succeeded); @@ -82,7 +82,7 @@ public void ClientCanRequestAndParseWorkItemsWithReleaseNotes() .Returns((HttpStatusCode.OK, JObject.Parse(@"{""totalCount"":3,""count"":3,""comments"":[{""text"":""= Changelog = N/A""}," + @"{""text"":""
= Changelog = README riddle now has an answer!
""},{""text"":""See also related issue.""}]}"))); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=28")); Assert.IsTrue(workItemLinks.Succeeded); @@ -111,7 +111,7 @@ public void ClientReportsFailuresAndReturnsPartialResults() httpJsonClient.Get("http://redstoneblock/DefaultCollection/Deployable/_apis/wit/workitems/6/comments?api-version=5.0-preview.2", "rumor") .Returns((HttpStatusCode.InternalServerError, null)); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=29")); Assert.IsFalse(workItemLinks.Succeeded); @@ -139,12 +139,12 @@ public void PersonalAccessTokenIsOnlySentToItsOrigin() }); // Request to other host should not include password - new AdoApiClient(store, httpJsonClient, HtmlConvert) + new AdoApiClient(store, httpJsonClient, HtmlConvert, Log) .GetBuildWorkItemsRefs(AdoBuildUrls.ParseBrowserUrl("http://someotherhost/DefaultCollection/Deployable/_build/results?buildId=24")); Assert.IsNull(passwordSent); // Request to origin should include password - new AdoApiClient(store, httpJsonClient, HtmlConvert) + new AdoApiClient(store, httpJsonClient, HtmlConvert, Log) .GetBuildWorkItemsRefs(AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=24")); Assert.AreEqual("rumor", passwordSent); } @@ -158,7 +158,7 @@ public void AcceptsDeletedBuildAsPermanentEmptySet() .Returns((HttpStatusCode.NotFound, JObject.Parse(@"{""$id"":""1"",""message"":""The requested build 7 could not be found."",""errorCode"":0,""eventId"":3000}"))); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=7")); Assert.IsTrue(workItemLinks.Succeeded); @@ -177,7 +177,7 @@ public void AcceptsDeletedWorkItemAsPermanentMissingTitle() .Returns((HttpStatusCode.NotFound, JObject.Parse(@"{""$id"":""1"",""message"":""TF401232: Work item 999 does not exist."",""errorCode"":0,""eventId"":3200}"))); - var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert).GetBuildWorkItemLinks( + var workItemLinks = new AdoApiClient(store, httpJsonClient, HtmlConvert, Log).GetBuildWorkItemLinks( AdoBuildUrls.ParseBrowserUrl("http://redstoneblock/DefaultCollection/Deployable/_build/results?buildId=8")); Assert.IsTrue(workItemLinks.Succeeded); diff --git a/source/Server/AdoClients/AdoApiClient.cs b/source/Server/AdoClients/AdoApiClient.cs index 181d271..93a0990 100644 --- a/source/Server/AdoClients/AdoApiClient.cs +++ b/source/Server/AdoClients/AdoApiClient.cs @@ -3,6 +3,7 @@ using System.Net; using System.Text.RegularExpressions; using Newtonsoft.Json.Linq; +using Octopus.Diagnostics; using Octopus.Server.Extensibility.Extensions; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.WorkItems; @@ -12,7 +13,7 @@ namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients { public interface IAdoApiClient { - SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls); + SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildNumber = null); } public class AdoApiClient : IAdoApiClient @@ -22,12 +23,14 @@ public class AdoApiClient : IAdoApiClient private readonly IAzureDevOpsConfigurationStore store; private readonly IHttpJsonClient client; private readonly HtmlConvert htmlConvert; + private readonly ILogWithContext log; - public AdoApiClient(IAzureDevOpsConfigurationStore store, IHttpJsonClient client, HtmlConvert htmlConvert) + public AdoApiClient(IAzureDevOpsConfigurationStore store, IHttpJsonClient client, HtmlConvert htmlConvert, ILogWithContext log) { this.store = store; this.client = client; this.htmlConvert = htmlConvert; + this.log = log; } internal string GetPersonalAccessToken(AdoUrl adoUrl) @@ -184,7 +187,7 @@ public SuccessOrErrorResult GetWorkItemLink(AdoProjectUrls adoProj return SuccessOrErrorResult.Conditional(workItemLink, workItem, releaseNote); } - public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls) + public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildNumber = null) { var workItemsRefs = GetBuildWorkItemsRefs(adoBuildUrls); if (!workItemsRefs.Succeeded) @@ -192,6 +195,13 @@ public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls a return SuccessOrErrorResult.Failure(workItemsRefs); } + if (!workItemsRefs.Value.Any()) + { + log.Trace($"No associated work items were found in Azure DevOps for build '{buildNumber}'."); + log.Trace("Work items can be linked when editing an open pull request, or by editing a work item and linking a build / commit / pull request," + + " or by mentioning a work item in a commit message. For more information, see https://g.octopushq.com/AzureDevOpsIssueTracker"); + } + var workItemLinks = workItemsRefs.Value .Select(w => GetWorkItemLink(adoBuildUrls, w.id)) .ToArray(); diff --git a/source/Server/AdoClients/AdoApiClientFactory.cs b/source/Server/AdoClients/AdoApiClientFactory.cs index 87f54ba..96fee83 100644 --- a/source/Server/AdoClients/AdoApiClientFactory.cs +++ b/source/Server/AdoClients/AdoApiClientFactory.cs @@ -22,7 +22,7 @@ public AdoApiClientFactory(IAzureDevOpsConfigurationStore store, HtmlConvert htm public IAdoApiClient CreateWithLog(ILogWithContext log) { - return new AdoApiClient(store, new HttpJsonClient(log), htmlConvert); + return new AdoApiClient(store, new HttpJsonClient(log), htmlConvert, log); } } } \ No newline at end of file diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index 9e98d3d..172c35c 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -28,22 +28,22 @@ public SuccessOrErrorResult Map(string packageId, IVersion versi { if (!IsEnabled) { - log.Verbose("Azure DevOps Issue Tracker is disabled in Settings."); + log.Trace("Azure DevOps Issue Tracker is disabled in Settings."); return null; } if (buildInformation == null) { - log.Info($"No build information was found for package {packageId} {version}. To incorporate build information, and enable support for work" - + $" items and release notes generation, consider adding a Push Build Information step to your build process."); + log.Verbose($"No build information was found for package {packageId} {version}. To incorporate build information, and enable support for work" + + $" items and release notes generation, consider adding a Push Build Information step to your build process."); return null; } if (buildInformation.BuildEnvironment != "Azure DevOps") { // We are only interested in build URLs from Azure DevOps, because get use its build APIs to get associated work items - log.Verbose($"The build environment for package {packageId} {version} was '{buildInformation.BuildEnvironment}' rather than 'Azure DevOps'," - + $" so the build URL will not be checked for Azure DevOps work item associations."); + log.Trace($"The build environment for package {packageId} {version} was '{buildInformation.BuildEnvironment}' rather than 'Azure DevOps'," + + $" so the build URL will not be checked for Azure DevOps work item associations."); return null; } @@ -56,7 +56,7 @@ public SuccessOrErrorResult Map(string packageId, IVersion versi try { - return clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); + return clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl), buildInformation.BuildNumber); } catch (Exception ex) { From 19761b27f30411c65bfdcacc52e9ea7aabca9725 Mon Sep 17 00:00:00 2001 From: David Young Date: Mon, 28 Oct 2019 01:21:29 +1000 Subject: [PATCH 6/6] Improve build description and add summary link --- source/Server/AdoClients/AdoApiClient.cs | 6 +++--- source/Server/AdoClients/AdoUrl.cs | 8 +++++++- source/Server/WorkItems/WorkItemLinkMapper.cs | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/source/Server/AdoClients/AdoApiClient.cs b/source/Server/AdoClients/AdoApiClient.cs index 93a0990..682455f 100644 --- a/source/Server/AdoClients/AdoApiClient.cs +++ b/source/Server/AdoClients/AdoApiClient.cs @@ -13,7 +13,7 @@ namespace Octopus.Server.Extensibility.IssueTracker.AzureDevOps.AdoClients { public interface IAdoApiClient { - SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildNumber = null); + SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildDescription = null); } public class AdoApiClient : IAdoApiClient @@ -187,7 +187,7 @@ public SuccessOrErrorResult GetWorkItemLink(AdoProjectUrls adoProj return SuccessOrErrorResult.Conditional(workItemLink, workItem, releaseNote); } - public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildNumber = null) + public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls adoBuildUrls, string buildDescription = null) { var workItemsRefs = GetBuildWorkItemsRefs(adoBuildUrls); if (!workItemsRefs.Succeeded) @@ -197,7 +197,7 @@ public SuccessOrErrorResult GetBuildWorkItemLinks(AdoBuildUrls a if (!workItemsRefs.Value.Any()) { - log.Trace($"No associated work items were found in Azure DevOps for build '{buildNumber}'."); + log.Trace($"No associated work items were found in Azure DevOps for {buildDescription}: {adoBuildUrls.BuildSummaryUrl}"); log.Trace("Work items can be linked when editing an open pull request, or by editing a work item and linking a build / commit / pull request," + " or by mentioning a work item in a commit message. For more information, see https://g.octopushq.com/AzureDevOpsIssueTracker"); } diff --git a/source/Server/AdoClients/AdoUrl.cs b/source/Server/AdoClients/AdoUrl.cs index 3dee791..8f82059 100644 --- a/source/Server/AdoClients/AdoUrl.cs +++ b/source/Server/AdoClients/AdoUrl.cs @@ -16,6 +16,7 @@ public class AdoProjectUrls : AdoUrl public class AdoBuildUrls : AdoProjectUrls { + public string BuildSummaryUrl { get; set; } public int BuildId { get; set; } public static AdoBuildUrls ParseBrowserUrl(string browserUrl) @@ -31,11 +32,16 @@ ArgumentException ParseError(Exception innerException = null) throw ParseError(); } + var browserUri = new Uri(browserUrl, UriKind.Absolute); + var summaryQuery = browserUri.ParseQueryString(); + summaryQuery["view"] = "results"; + return new AdoBuildUrls { OrganizationUrl = prefixMatch.Groups[2].Value, ProjectUrl = prefixMatch.Groups[1].Value, - BuildId = int.Parse(new Uri(browserUrl, UriKind.Absolute).ParseQueryString()["buildId"]) + BuildSummaryUrl = new UriBuilder(browserUri) {Query = summaryQuery.ToString()}.ToString(), + BuildId = int.Parse(browserUri.ParseQueryString()["buildId"]) }; } catch (Exception ex) diff --git a/source/Server/WorkItems/WorkItemLinkMapper.cs b/source/Server/WorkItems/WorkItemLinkMapper.cs index 172c35c..30f70f9 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -56,7 +56,7 @@ public SuccessOrErrorResult Map(string packageId, IVersion versi try { - return clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl), buildInformation.BuildNumber); + return clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl), $"{packageId} build {buildInformation.BuildNumber}"); } catch (Exception ex) {