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.Tests/Server.Tests.csproj b/source/Server.Tests/Server.Tests.csproj index 80c9de0..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 a05cdbe..418a643 100644 --- a/source/Server.Tests/WorkItemLinkMapperScenarios.cs +++ b/source/Server.Tests/WorkItemLinkMapperScenarios.cs @@ -1,13 +1,12 @@ using System; using NSubstitute; using NUnit.Framework; -using Octopus.Server.Extensibility.Extensions; +using Octopus.Diagnostics; 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 { @@ -20,16 +19,19 @@ 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] public void WhenDisabledReturnsNull() { - var links = CreateWorkItemLinkMapper(false).Map(new OctopusBuildInformation + 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); Assert.IsNull(links.Value); } diff --git a/source/Server/AdoClients/AdoApiClient.cs b/source/Server/AdoClients/AdoApiClient.cs index 181d271..682455f 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 buildDescription = 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 buildDescription = 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 {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"); + } + 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 new file mode 100644 index 0000000..96fee83 --- /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, log); + } + } +} \ No newline at end of file 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/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/Server.csproj b/source/Server/Server.csproj index 2a08f2a..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 5cee8a0..30f70f9 100644 --- a/source/Server/WorkItems/WorkItemLinkMapper.cs +++ b/source/Server/WorkItems/WorkItemLinkMapper.cs @@ -1,35 +1,67 @@ -using Octopus.Server.Extensibility.Extensions; +using System; +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; using Octopus.Server.Extensibility.IssueTracker.AzureDevOps.Configuration; using Octopus.Server.Extensibility.Resources.IssueTrackers; +using Octopus.Versioning; 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; public bool IsEnabled => store.GetIsEnabled(); - public SuccessOrErrorResult Map(OctopusBuildInformation buildInformation) + 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.Trace("Azure DevOps Issue Tracker is disabled in Settings."); return null; + } - return client.GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl)); + if (buildInformation == null) + { + 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.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; + } + + 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 clientFactory.CreateWithLog(log).GetBuildWorkItemLinks(AdoBuildUrls.ParseBrowserUrl(buildInformation.BuildUrl), $"{packageId} build {buildInformation.BuildNumber}"); + } + catch (Exception ex) + { + return SuccessOrErrorResult.Failure(ex.Message); + } } } } \ No newline at end of file