diff --git a/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs b/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs index 61a3f73d0a..df0da1312b 100644 --- a/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs +++ b/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs @@ -14,5 +14,10 @@ namespace Tgstation.Server.Api.Models /// Remote provider is GitHub.com /// GitHub, + + /// + /// Remote provider is GitLab.com + /// + GitLab, } } diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index cbc81333fd..62b62aa652 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -495,7 +495,7 @@ namespace Tgstation.Server.Host.Components.Deployment } /// - #pragma warning disable CA1506 + #pragma warning disable CA1506, CA1508 public async Task DeploymentProcess( Models.Job job, IDatabaseContextFactory databaseContextFactory, @@ -714,7 +714,7 @@ namespace Tgstation.Server.Host.Components.Deployment deploying = false; } } - #pragma warning restore CA1506 + #pragma warning restore CA1506, CA1508 /// /// Calculate the average length of a deployment using a given . diff --git a/src/Tgstation.Server.Host/Components/InstanceFactory.cs b/src/Tgstation.Server.Host/Components/InstanceFactory.cs index c2f1587ca4..12e308ccb0 100644 --- a/src/Tgstation.Server.Host/Components/InstanceFactory.cs +++ b/src/Tgstation.Server.Host/Components/InstanceFactory.cs @@ -132,6 +132,11 @@ namespace Tgstation.Server.Host.Components /// readonly IFileTransferTicketProvider fileTransferService; + /// + /// The for the . + /// + readonly IGitRemoteFeaturesFactory gitRemoteFeaturesFactory; + /// /// The for the . /// @@ -161,6 +166,7 @@ namespace Tgstation.Server.Host.Components /// The value of . /// The value of . /// The value of . + /// The value of . /// The containing the value of . public InstanceFactory( IIOManager ioManager, @@ -184,6 +190,7 @@ namespace Tgstation.Server.Host.Components ILibGit2Commands repositoryCommands, IServerPortProvider serverPortProvider, IFileTransferTicketProvider fileTransferService, + IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, IOptions generalConfigurationOptions) { this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); @@ -207,6 +214,7 @@ namespace Tgstation.Server.Host.Components this.repositoryCommands = repositoryCommands ?? throw new ArgumentNullException(nameof(repositoryCommands)); this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider)); this.fileTransferService = fileTransferService ?? throw new ArgumentNullException(nameof(fileTransferService)); + this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); } @@ -239,6 +247,7 @@ namespace Tgstation.Server.Host.Components repositoryCommands, repoIoManager, eventConsumer, + gitRemoteFeaturesFactory, loggerFactory.CreateLogger(), loggerFactory.CreateLogger()); try diff --git a/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs index 868ac3f789..3bdc876709 100644 --- a/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs @@ -1,3 +1,6 @@ +using System; +using System.Threading; +using System.Threading.Tasks; using Tgstation.Server.Api.Models; namespace Tgstation.Server.Host.Components.Repository @@ -8,7 +11,10 @@ namespace Tgstation.Server.Host.Components.Repository sealed class DefaultGitRemoteFeatures : IGitRemoteFeatures { /// - public string TestMergeRefSpecFormatter => null; + public string TestMergeRefSpecFormatter => throw new NotSupportedException(); + + /// + public string TestMergeLocalBranchNameFormatter => throw new NotSupportedException(); /// public RemoteGitProvider? RemoteGitProvider => Api.Models.RemoteGitProvider.Unknown; @@ -18,5 +24,11 @@ namespace Tgstation.Server.Host.Components.Repository /// public string RemoteRepositoryName => null; + + /// + public Task GetTestMerge( + TestMergeParameters parameters, + Api.Models.Internal.RepositorySettings repositorySettings, + CancellationToken cancellationToken) => throw new NotSupportedException(); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs index b7cac504da..1878171196 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs @@ -1,36 +1,117 @@ +using Microsoft.Extensions.Logging; +using Octokit; using System; +using System.Threading; +using System.Threading.Tasks; using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; +using Tgstation.Server.Host.Core; +using Tgstation.Server.Host.Extensions; namespace Tgstation.Server.Host.Components.Repository { /// /// GitHub . /// - sealed class GitHubRemoteFeatures : IGitRemoteFeatures + sealed class GitHubRemoteFeatures : GitRemoteFeaturesBase { /// - public string TestMergeRefSpecFormatter => "pull/{0}/head:{1}"; + public override string TestMergeRefSpecFormatter => "pull/{0}/head:{1}"; /// - public RemoteGitProvider? RemoteGitProvider => Api.Models.RemoteGitProvider.GitHub; + public override string TestMergeLocalBranchNameFormatter => "pull/{0}/headrefs/heads/{1}"; /// - public string RemoteRepositoryOwner { get; } + public override RemoteGitProvider? RemoteGitProvider => Api.Models.RemoteGitProvider.GitHub; /// - public string RemoteRepositoryName { get; } + public override string RemoteRepositoryOwner { get; } + + /// + public override string RemoteRepositoryName { get; } + + /// + /// The for the . + /// + readonly IGitHubClientFactory gitHubClientFactory; /// /// Initializes a new instance of the . /// + /// The value of . + /// The for the . /// The remote repository . - public GitHubRemoteFeatures(Uri remoteUrl) + public GitHubRemoteFeatures(IGitHubClientFactory gitHubClientFactory, ILogger logger, Uri remoteUrl) + : base(logger, remoteUrl) { + this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); + if (remoteUrl == null) throw new ArgumentNullException(nameof(remoteUrl)); RemoteRepositoryOwner = remoteUrl.Segments[1].TrimEnd('/'); RemoteRepositoryName = remoteUrl.Segments[2].TrimEnd('/'); } + + /// + protected override async Task GetTestMergeImpl( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken) + { + var gitHubClient = repositorySettings.AccessToken != null + ? gitHubClientFactory.CreateClient(repositorySettings.AccessToken) + : gitHubClientFactory.CreateClient(); + + PullRequest pr = null; + ApiException exception = null; + string errorMessage = null; + try + { + pr = await gitHubClient + .PullRequest + .Get(RemoteRepositoryOwner, RemoteRepositoryName, parameters.Number) + .WithToken(cancellationToken) + .ConfigureAwait(false); + } + catch (RateLimitExceededException ex) + { + // you look at your anonymous access and sigh + errorMessage = "GITHUB API ERROR: RATE LIMITED"; + exception = ex; + } + catch (AuthorizationException ex) + { + errorMessage = "GITHUB API ERROR: BAD CREDENTIALS"; + exception = ex; + } + catch (NotFoundException ex) + { + // you look at your shithub and sigh + errorMessage = "GITHUB API ERROR: PULL REQUEST NOT FOUND"; + exception = ex; + } + + if (exception != null) + Logger.LogWarning(exception, "Error retrieving pull request metadata!"); + + var revisionToUse = parameters.PullRequestRevision == null + || pr?.Head.Sha.StartsWith(parameters.PullRequestRevision, StringComparison.OrdinalIgnoreCase) == true + ? pr?.Head.Sha + : parameters.PullRequestRevision; + + var testMerge = new Models.TestMerge + { + Author = pr?.User.Login ?? errorMessage, + BodyAtMerge = pr?.Body ?? errorMessage ?? String.Empty, + TitleAtMerge = pr?.Title ?? errorMessage ?? String.Empty, + Comment = parameters.Comment, + Number = parameters.Number, + PullRequestRevision = revisionToUse, + Url = pr?.HtmlUrl ?? errorMessage + }; + + return testMerge; + } } } diff --git a/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs new file mode 100644 index 0000000000..aa5b932fd5 --- /dev/null +++ b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs @@ -0,0 +1,97 @@ +using GitLabApiClient; +using Microsoft.Extensions.Logging; +using System; +using System.Threading; +using System.Threading.Tasks; +using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; +using Tgstation.Server.Host.Extensions; + +namespace Tgstation.Server.Host.Components.Repository +{ + /// + /// GitLab . + /// + sealed class GitLabRemoteFeatures : GitRemoteFeaturesBase + { + /// + public override string TestMergeRefSpecFormatter => "merge-requests/{0}/head:{1}"; + + /// + public override string TestMergeLocalBranchNameFormatter => "merge-requests/{0}/headrefs/heads/{1}"; + + /// + public override RemoteGitProvider? RemoteGitProvider => Api.Models.RemoteGitProvider.GitLab; + + /// + public override string RemoteRepositoryOwner { get; } + + /// + public override string RemoteRepositoryName { get; } + + /// + /// Initializes a new instance of the . + /// + /// The for the . + /// The remote repository . + public GitLabRemoteFeatures(ILogger logger, Uri remoteUrl) + : base(logger, remoteUrl) + { + RemoteRepositoryOwner = remoteUrl.Segments[1].TrimEnd('/'); + RemoteRepositoryName = remoteUrl.Segments[2].TrimEnd('/'); + } + + /// + protected override async Task GetTestMergeImpl( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken) + { + const string GitLabUrl = "https://gitlab.com"; + + var client = repositorySettings.AccessToken != null + ? new GitLabClient(GitLabUrl, repositorySettings.AccessToken) + : new GitLabClient(GitLabUrl); + + try + { + var mr = await client + .MergeRequests + .GetAsync($"{RemoteRepositoryOwner}/{RemoteRepositoryName}", parameters.Number) + .WithToken(cancellationToken) + .ConfigureAwait(false); + + var revisionToUse = parameters.PullRequestRevision == null + || mr.Sha.StartsWith(parameters.PullRequestRevision, StringComparison.OrdinalIgnoreCase) + ? mr.Sha + : parameters.PullRequestRevision; + + return new Models.TestMerge + { + Author = mr.Author.Username, + BodyAtMerge = mr.Description, + TitleAtMerge = mr.Title, + Comment = parameters.Comment, + Number = parameters.Number, + PullRequestRevision = mr.Sha, + Url = mr.WebUrl + }; + } + catch (Exception ex) + { + Logger.LogWarning(ex, "Error retrieving merge request metadata!"); + + return new Models.TestMerge + { + Author = ex.Message, + BodyAtMerge = ex.Message, + TitleAtMerge = ex.Message, + Comment = parameters.Comment, + Number = parameters.Number, + PullRequestRevision = parameters.PullRequestRevision, + Url = ex.Message + }; + } + } + } +} diff --git a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs new file mode 100644 index 0000000000..92835dd7ad --- /dev/null +++ b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs @@ -0,0 +1,95 @@ +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; + +namespace Tgstation.Server.Host.Components.Repository +{ + /// + /// Base for implementing . + /// + abstract class GitRemoteFeaturesBase : IGitRemoteFeatures + { + /// + public abstract string TestMergeRefSpecFormatter { get; } + + /// + public abstract string TestMergeLocalBranchNameFormatter { get; } + + /// + public abstract RemoteGitProvider? RemoteGitProvider { get; } + + /// + public abstract string RemoteRepositoryOwner { get; } + + /// + public abstract string RemoteRepositoryName { get; } + + /// + /// The for the . + /// + protected ILogger Logger { get; } + + /// + /// Cache of created s. + /// + readonly Dictionary cachedLookups; + + /// + /// Initializes a new instance of the . + /// + /// The value of . + /// The remote repository . + public GitRemoteFeaturesBase(ILogger logger, Uri remoteUrl) + { + Logger = logger ?? throw new ArgumentNullException(nameof(logger)); + if (remoteUrl == null) + throw new ArgumentNullException(nameof(remoteUrl)); + + cachedLookups = new Dictionary(); + } + + /// + public async Task GetTestMerge( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken) + { + if (parameters == null) + throw new ArgumentNullException(nameof(parameters)); + if (repositorySettings == null) + throw new ArgumentNullException(nameof(repositorySettings)); + + Models.TestMerge result; + lock (cachedLookups) + if (cachedLookups.TryGetValue(parameters, out result)) + Logger.LogTrace("Using cache for test merge #{0}", parameters.Number); + + if (result == null) + { + Logger.LogTrace("Retrieving metadata for test merge #{0}...", parameters.Number); + result = await GetTestMergeImpl(parameters, repositorySettings, cancellationToken).ConfigureAwait(false); + lock (cachedLookups) + if (!cachedLookups.TryAdd(parameters, result)) + Logger.LogError("Race condition on adding test merge #{0}!", parameters.Number); + } + + return result; + } + + /// + /// Implementation of + /// + /// The . + /// The . + /// The for the operation. + /// A resulting in the of the . + protected abstract Task GetTestMergeImpl( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken); + } +} diff --git a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs new file mode 100644 index 0000000000..1c061adc6e --- /dev/null +++ b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs @@ -0,0 +1,80 @@ +using Microsoft.Extensions.Logging; +using System; +using Tgstation.Server.Host.Core; + +namespace Tgstation.Server.Host.Components.Repository +{ + /// + sealed class GitRemoteFeaturesFactory : IGitRemoteFeaturesFactory + { + /// + /// The for the . + /// + readonly IGitHubClientFactory gitHubClientFactory; + + /// + /// The for the . + /// + readonly ILoggerFactory loggerFactory; + + /// + /// The for the . + /// + readonly ILogger logger; + + /// + /// Initializes a new instance of the . + /// + /// The value of . + /// The value of . + /// The value of . + public GitRemoteFeaturesFactory( + IGitHubClientFactory gitHubClientFactory, + ILoggerFactory loggerFactory, + ILogger logger) + { + this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); + this.loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); + this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + /// + public IGitRemoteFeatures CreateGitRemoteFeatures(IRepository repository) + { + if (repository == null) + throw new ArgumentNullException(nameof(repository)); + + var primaryRemote = repository.Origin; + try + { + var primaryRemoteUrl = new Uri(primaryRemote); + + switch (primaryRemoteUrl.Host.ToUpperInvariant()) + { + case "GITHUB.COM": + case "WWW.GITHUB.COM": + case "GIT.GITHUB.COM": + return new GitHubRemoteFeatures( + gitHubClientFactory, + loggerFactory.CreateLogger(), + primaryRemoteUrl); + case "GITLAB.COM": + case "WWW.GITLAB.COM": + case "GIT.GITLAB.COM": + return new GitLabRemoteFeatures( + loggerFactory.CreateLogger(), + primaryRemoteUrl); + default: + logger.LogTrace("Unknown git remote: {0}", primaryRemoteUrl); + break; + } + } + catch (Exception ex) + { + logger.LogWarning(ex, "Error parsing remote git provider."); + } + + return new DefaultGitRemoteFeatures(); + } + } +} diff --git a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteAdditionalInformation.cs b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteAdditionalInformation.cs new file mode 100644 index 0000000000..7b4de75b0e --- /dev/null +++ b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteAdditionalInformation.cs @@ -0,0 +1,25 @@ +using System.Threading; +using System.Threading.Tasks; +using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; + +namespace Tgstation.Server.Host.Components.Repository +{ + /// + /// Additional information from git remotes. + /// + public interface IGitRemoteAdditionalInformation : IGitRemoteInformation + { + /// + /// Retrieve the representation of given test merge . + /// + /// The . + /// The . + /// The for the operation. + /// A resulting in the of the . + Task GetTestMerge( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken); + } +} diff --git a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs index d26e9cd562..7dc7502ba7 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs @@ -1,15 +1,18 @@ -using Tgstation.Server.Api.Models.Internal; - namespace Tgstation.Server.Host.Components.Repository { /// /// Provides features for remote git services /// - interface IGitRemoteFeatures : IGitRemoteInformation + interface IGitRemoteFeatures : IGitRemoteAdditionalInformation { /// /// Gets a formatter string which creates the remote refspec for fetching the HEAD of passed in pull request number. /// string TestMergeRefSpecFormatter { get; } + + /// + /// Get + /// + string TestMergeLocalBranchNameFormatter { get; } } } diff --git a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs new file mode 100644 index 0000000000..31085a16d2 --- /dev/null +++ b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs @@ -0,0 +1,15 @@ +namespace Tgstation.Server.Host.Components.Repository +{ + /// + /// Factory for creating . + /// + interface IGitRemoteFeaturesFactory + { + /// + /// Create the for a given . + /// + /// The to create for. + /// A new instance. + IGitRemoteFeatures CreateGitRemoteFeatures(IRepository repository); + } +} diff --git a/src/Tgstation.Server.Host/Components/Repository/ILibGit2RepositoryFactory.cs b/src/Tgstation.Server.Host/Components/Repository/ILibGit2RepositoryFactory.cs index 79e20a64fb..086ca8d93e 100644 --- a/src/Tgstation.Server.Host/Components/Repository/ILibGit2RepositoryFactory.cs +++ b/src/Tgstation.Server.Host/Components/Repository/ILibGit2RepositoryFactory.cs @@ -21,8 +21,8 @@ namespace Tgstation.Server.Host.Components.Repository /// /// The full path to the . /// The for the operation. - /// A resulting in a containing the loaded and the associated . - Task> CreateFromPath(string path, CancellationToken cancellationToken); + /// A resulting in the loaded . + Task CreateFromPath(string path, CancellationToken cancellationToken); /// /// Clone a remote . diff --git a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs index 41dd80e15e..be9d6b0b0d 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs @@ -2,14 +2,13 @@ using System; using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; -using Tgstation.Server.Api.Models.Internal; namespace Tgstation.Server.Host.Components.Repository { /// /// Represents an on-disk git repository /// - public interface IRepository : IGitRemoteInformation, IDisposable + public interface IRepository : IGitRemoteAdditionalInformation, IDisposable { /// /// If tracks an upstream branch diff --git a/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs b/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs index bd1f7de84c..137d6be054 100644 --- a/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs +++ b/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs @@ -2,7 +2,6 @@ using LibGit2Sharp; using LibGit2Sharp.Handlers; using Microsoft.Extensions.Logging; using System; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; @@ -36,7 +35,7 @@ namespace Tgstation.Server.Host.Components.Repository } /// - public async Task> CreateFromPath(string path, CancellationToken cancellationToken) + public async Task CreateFromPath(string path, CancellationToken cancellationToken) { if (path == null) throw new ArgumentNullException(nameof(path)); @@ -52,42 +51,7 @@ namespace Tgstation.Server.Host.Components.Repository TaskScheduler.Current) .ConfigureAwait(false); - try - { - var remoteFeatures = CreateGitRemoteFeatures(repo); - return Tuple.Create(repo, remoteFeatures); - } - catch - { - repo.Dispose(); - throw; - } - } - - IGitRemoteFeatures CreateGitRemoteFeatures(LibGit2Sharp.IRepository repo) - { - var primaryRemote = repo.Network.Remotes.First(); - var primaryRemoteUrl = new Uri(primaryRemote.Url); - - try - { - switch (primaryRemoteUrl.Host.ToUpperInvariant()) - { - case "GITHUB.COM": - case "WWW.GITHUB.COM": - case "GIT.GITHUB.COM": - return new GitHubRemoteFeatures(primaryRemoteUrl); - default: - logger.LogTrace("Unknown git remote: {0}", primaryRemoteUrl); - break; - } - } - catch (Exception ex) - { - logger.LogWarning(ex, "Error parsing remote git provider."); - } - - return new DefaultGitRemoteFeatures(); + return repo; } /// diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 6c752c0cf6..3d83a36dbb 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; @@ -123,7 +124,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The value of /// The value of /// The value of - /// The value of . + /// The to provide the value of . /// The value of /// The value if public Repository( @@ -132,7 +133,7 @@ namespace Tgstation.Server.Host.Components.Repository IIOManager ioMananger, IEventConsumer eventConsumer, ICredentialsProvider credentialsProvider, - IGitRemoteFeatures gitRemoteFeatures, + IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, ILogger logger, Action onDispose) { @@ -141,9 +142,13 @@ namespace Tgstation.Server.Host.Components.Repository this.ioMananger = ioMananger ?? throw new ArgumentNullException(nameof(ioMananger)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); this.credentialsProvider = credentialsProvider ?? throw new ArgumentNullException(nameof(credentialsProvider)); - this.gitRemoteFeatures = gitRemoteFeatures ?? throw new ArgumentNullException(nameof(gitRemoteFeatures)); + if (gitRemoteFeaturesFactory == null) + throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); + this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose)); + + gitRemoteFeatures = gitRemoteFeaturesFactory.CreateGitRemoteFeatures(this); } /// @@ -299,7 +304,7 @@ namespace Tgstation.Server.Host.Components.Repository testMergeParameters.Comment ?? String.Empty); var prBranchName = String.Format(CultureInfo.InvariantCulture, "pr-{0}", testMergeParameters.Number); - var localBranchName = String.Format(CultureInfo.InvariantCulture, "pull/{0}/headrefs/heads/{1}", testMergeParameters.Number, prBranchName); + var localBranchName = String.Format(CultureInfo.InvariantCulture, gitRemoteFeatures.TestMergeLocalBranchNameFormatter, testMergeParameters.Number, prBranchName); var refSpec = String.Format(CultureInfo.InvariantCulture, gitRemoteFeatures.TestMergeRefSpecFormatter, testMergeParameters.Number, prBranchName); var refSpecList = new List { refSpec }; @@ -774,5 +779,14 @@ namespace Tgstation.Server.Host.Components.Repository return false; }, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); + + /// + public Task GetTestMerge( + TestMergeParameters parameters, + RepositorySettings repositorySettings, + CancellationToken cancellationToken) => gitRemoteFeatures.GetTestMerge( + parameters, + repositorySettings, + cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs index c82b56e22a..6e025f8b6f 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs @@ -40,6 +40,11 @@ namespace Tgstation.Server.Host.Components.Repository /// readonly IEventConsumer eventConsumer; + /// + /// The for the + /// + readonly IGitRemoteFeaturesFactory gitRemoteFeaturesFactory; + /// /// The created s /// @@ -62,6 +67,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The value of . /// The value of /// The value of + /// The value of . /// The value of /// The value of public RepositoryManager( @@ -69,6 +75,7 @@ namespace Tgstation.Server.Host.Components.Repository ILibGit2Commands commands, IIOManager ioManager, IEventConsumer eventConsumer, + IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, ILogger repositoryLogger, ILogger logger) { @@ -76,6 +83,7 @@ namespace Tgstation.Server.Host.Components.Repository this.commands = commands ?? throw new ArgumentNullException(nameof(commands)); this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); + this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); this.repositoryLogger = repositoryLogger ?? throw new ArgumentNullException(nameof(repositoryLogger)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); semaphore = new SemaphoreSlim(1); @@ -188,32 +196,21 @@ namespace Tgstation.Server.Host.Components.Repository { try { - var repoTuple = await repositoryFactory.CreateFromPath(ioManager.ResolvePath(), cancellationToken).ConfigureAwait(false); + var libGit2Repo = await repositoryFactory.CreateFromPath(ioManager.ResolvePath(), cancellationToken).ConfigureAwait(false); - try - { - var libGit2Repo = repoTuple.Item1; - var gitRemoteFeatures = repoTuple.Item2; - - return new Repository( - libGit2Repo, - commands, - ioManager, - eventConsumer, - repositoryFactory, - gitRemoteFeatures, - repositoryLogger, - () => - { - logger.LogTrace("Releasing semaphore due to Repository disposal..."); - semaphore.Release(); - }); - } - catch - { - repoTuple.Item1.Dispose(); - throw; - } + return new Repository( + libGit2Repo, + commands, + ioManager, + eventConsumer, + repositoryFactory, + gitRemoteFeaturesFactory, + repositoryLogger, + () => + { + logger.LogTrace("Releasing semaphore due to Repository disposal..."); + semaphore.Release(); + }); } catch { diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 1b77f22bf6..c83393aa98 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -17,7 +17,6 @@ using Tgstation.Server.Host.Components; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; using Tgstation.Server.Host.Database; -using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.Security; @@ -644,7 +643,6 @@ namespace Tgstation.Server.Host.Controllers } // test merging - Dictionary prMap = null; if (newTestMerges) { if (repo.RemoteGitProvider == RemoteGitProvider.Unknown) @@ -666,10 +664,6 @@ namespace Tgstation.Server.Host.Controllers bool needToApplyRemainingPrs = true; if (lastRevisionInfo.OriginCommitSha == lastRevisionInfo.CommitSha) { - // In order for this to work though we need the shas of all the commits - if (model.NewTestMerges.Any(x => x.PullRequestRevision == null)) - prMap = new Dictionary(); - bool cantSearch = false; foreach (var I in model.NewTestMerges) { @@ -681,11 +675,10 @@ namespace Tgstation.Server.Host.Controllers try { // retrieve the latest sha - var pr = await gitHubClient.PullRequest.Get(repoOwner, repoName, I.Number) - .WithToken(ct) - .ConfigureAwait(false); - prMap.Add(I.Number, pr); - I.PullRequestRevision = pr.Head.Sha; + var pr = await repo.GetTestMerge(I, currentModel, ct).ConfigureAwait(false); + + // we want to take the earliest truth possible to prevent RCEs, if this fails AddTestMerge will set it + I.PullRequestRevision = pr.PullRequestRevision; } catch { @@ -798,47 +791,10 @@ namespace Tgstation.Server.Host.Controllers { foreach (var I in model.NewTestMerges) { - Octokit.PullRequest pr = null; - string errorMessage = null; - if (lastRevisionInfo.ActiveTestMerges.Any(x => x.TestMerge.Number == I.Number)) throw new JobException(ErrorCode.RepoDuplicateTestMerge); - Exception exception = null; - try - { - // load from cache if possible - if (prMap == null || !prMap.TryGetValue(I.Number, out pr)) - pr = await gitHubClient - .PullRequest - .Get(repoOwner, repoName, I.Number) - .WithToken(ct) - .ConfigureAwait(false); - } - catch (Octokit.RateLimitExceededException ex) - { - // you look at your anonymous access and sigh - errorMessage = "REMOTE API ERROR: RATE LIMITED"; - exception = ex; - } - catch (Octokit.AuthorizationException ex) - { - errorMessage = "REMOTE API ERROR: BAD CREDENTIALS"; - exception = ex; - } - catch (Octokit.NotFoundException ex) - { - // you look at your shithub and sigh - errorMessage = "REMOTE API ERROR: PULL REQUEST NOT FOUND"; - exception = ex; - } - - if (exception != null) - Logger.LogWarning(exception, "Error retrieving pull request metadata!"); - - // we want to take the earliest truth possible to prevent RCEs, if this fails AddTestMerge will set it - if (I.PullRequestRevision == null && pr != null) - I.PullRequestRevision = pr.Head.Sha; + var fullTestMergeTask = repo.GetTestMerge(I, currentModel, ct); var mergeResult = await repo.AddTestMerge( I, @@ -849,28 +805,38 @@ namespace Tgstation.Server.Host.Controllers NextProgressReporter(), ct).ConfigureAwait(false); - if (!mergeResult.HasValue) + if (mergeResult == null) throw new JobException( ErrorCode.RepoTestMergeConflict, new JobException( $"Merge of PR #{I.Number} at {I.PullRequestRevision.Substring(0, 7)} conflicted!")); - ++doneSteps; + Models.TestMerge fullTestMerge; + try + { + fullTestMerge = await fullTestMergeTask.ConfigureAwait(false); + } + catch (Exception ex) + { + Logger.LogWarning("Error retrieving metadata for test merge #{0}!", I.Number); + + fullTestMerge = new Models.TestMerge + { + Author = ex.Message, + BodyAtMerge = ex.Message, + MergedAt = DateTimeOffset.Now, + TitleAtMerge = ex.Message, + Comment = I.Comment, + Number = I.Number, + PullRequestRevision = I.PullRequestRevision, + Url = ex.Message + }; + } // MergedBy will be set later - var tm = new Models.TestMerge - { - Author = pr?.User.Login ?? errorMessage, - BodyAtMerge = pr?.Body ?? errorMessage ?? String.Empty, - MergedAt = DateTimeOffset.Now, - TitleAtMerge = pr?.Title ?? errorMessage ?? String.Empty, - Comment = I.Comment, - Number = I.Number, - PullRequestRevision = I.PullRequestRevision, - Url = pr?.HtmlUrl ?? errorMessage - }; + ++doneSteps; - await UpdateRevInfo(tm).ConfigureAwait(false); + await UpdateRevInfo(fullTestMerge).ConfigureAwait(false); } } } diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index cf80be7bfd..071380e041 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -306,24 +306,23 @@ namespace Tgstation.Server.Host.Core services.AddSingleton(); } - // configure misc services + // configure component/misc services services.AddScoped(); services.AddTransient, LimitedFileStreamResultExecutor>(); - services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(x => x.GetRequiredService()); - services.AddSingleton(x => x.GetRequiredService()); - - // configure component services + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(x => x.GetRequiredService()); + services.AddSingleton(x => x.GetRequiredService()); // configure root services services.AddSingleton(); diff --git a/src/Tgstation.Server.Host/Models/Job.cs b/src/Tgstation.Server.Host/Models/Job.cs index 134993f14c..d2ba919450 100644 --- a/src/Tgstation.Server.Host/Models/Job.cs +++ b/src/Tgstation.Server.Host/Models/Job.cs @@ -1,9 +1,11 @@ -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; namespace Tgstation.Server.Host.Models { /// + #pragma warning disable CA1724 // naming conflict with gitlab package public sealed class Job : Api.Models.Internal.Job + #pragma warning restore CA1724 { /// /// See diff --git a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj index f8d5296d9d..f2de7240e3 100644 --- a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj +++ b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj @@ -65,6 +65,7 @@ + diff --git a/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryFactory.cs b/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryFactory.cs index 3fcb973507..43b4c66124 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryFactory.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Repository/TestRepositoryFactory.cs @@ -20,8 +20,7 @@ namespace Tgstation.Server.Host.Components.Repository.Tests string path, ILibGit2RepositoryFactory repositoryFactory = null) => (await (repositoryFactory ?? CreateFactory()) - .CreateFromPath(path, default)) - .Item1; + .CreateFromPath(path, default)); [TestMethod] public void TestConstructionThrows() => Assert.ThrowsException(() => new LibGit2RepositoryFactory(null)); diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index 9aa2f223a2..d0d50482eb 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -480,7 +480,7 @@ namespace Tgstation.Server.Tests Mock.Of(), Mock.Of(), Mock.Of(), - Mock.Of(), + Mock.Of(), Mock.Of>(), () => { });