diff --git a/src/Tgstation.Server.Host/Components/Instance.cs b/src/Tgstation.Server.Host/Components/Instance.cs index 83e4bca775..fbe5466555 100644 --- a/src/Tgstation.Server.Host/Components/Instance.cs +++ b/src/Tgstation.Server.Host/Components/Instance.cs @@ -1,5 +1,6 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Octokit; using Serilog.Context; using System; using System.Collections.Generic; @@ -13,6 +14,9 @@ using Tgstation.Server.Host.Components.Deployment; using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Components.Watchdog; +using Tgstation.Server.Host.Configuration; +using Tgstation.Server.Host.Core; +using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; @@ -60,6 +64,11 @@ namespace Tgstation.Server.Host.Components /// readonly IEventConsumer eventConsumer; + /// + /// The for the . + /// + readonly IGitHubClientFactory gitHubClientFactory; + /// /// The for the /// @@ -70,6 +79,11 @@ namespace Tgstation.Server.Host.Components /// readonly Api.Models.Instance metadata; + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// for and . /// @@ -98,7 +112,9 @@ namespace Tgstation.Server.Host.Components /// The value of /// The value of /// The value of + /// The value of . /// The value of + /// The value of . public Instance( Api.Models.Instance metadata, IRepositoryManager repositoryManager, @@ -111,7 +127,9 @@ namespace Tgstation.Server.Host.Components IDmbFactory dmbFactory, IJobManager jobManager, IEventConsumer eventConsumer, - ILogger logger) + IGitHubClientFactory gitHubClientFactory, + ILogger logger, + GeneralConfiguration generalConfiguration) { this.metadata = metadata ?? throw new ArgumentNullException(nameof(metadata)); RepositoryManager = repositoryManager ?? throw new ArgumentNullException(nameof(repositoryManager)); @@ -123,7 +141,9 @@ namespace Tgstation.Server.Host.Components this.dmbFactory = dmbFactory ?? throw new ArgumentNullException(nameof(dmbFactory)); this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); + this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); timerLock = new object(); } @@ -230,7 +250,7 @@ namespace Tgstation.Server.Host.Components .Include(x => x.ActiveTestMerges).ThenInclude(x => x.TestMerge) .FirstOrDefaultAsync(jobCancellationToken); - async Task UpdateRevInfo(string currentHead, bool onOrigin) + async Task UpdateRevInfo(string currentHead, bool onOrigin, IEnumerable updatedTestMerges) { if (currentRevInfo == null) currentRevInfo = await LoadRevInfo().ConfigureAwait(false); @@ -253,7 +273,8 @@ namespace Tgstation.Server.Host.Components Instance = attachedInstance }; if (!onOrigin) - currentRevInfo.ActiveTestMerges = new List(oldRevInfo.ActiveTestMerges); + currentRevInfo.ActiveTestMerges = new List( + updatedTestMerges ?? oldRevInfo.ActiveTestMerges); databaseContext.Instances.Attach(attachedInstance); databaseContext.RevisionInformations.Add(currentRevInfo); @@ -261,8 +282,9 @@ namespace Tgstation.Server.Host.Components } // take appropriate auto update actions - bool shouldSyncTracked; - if (repositorySettings.AutoUpdatesKeepTestMerges.Value) + bool shouldSyncTracked = false; + bool preserveTestMerges = repositorySettings.AutoUpdatesKeepTestMerges.Value; + if (preserveTestMerges) { logger.LogTrace("Preserving test merges..."); @@ -275,21 +297,29 @@ namespace Tgstation.Server.Host.Components currentRevInfo = await currentRevInfoTask.ConfigureAwait(false); + var updatedTestMerges = await RemoveMergedPullRequests( + repo, + repositorySettings, + currentRevInfo, + cancellationToken) + .ConfigureAwait(false); + var lastRevInfoWasOriginCommit = currentRevInfo == default || currentRevInfo.CommitSha == currentRevInfo.OriginCommitSha; var stillOnOrigin = result.Value && lastRevInfoWasOriginCommit; var currentHead = repo.Head; if (currentHead != startSha) { - await UpdateRevInfo(currentHead, stillOnOrigin).ConfigureAwait(false); + await UpdateRevInfo(currentHead, stillOnOrigin, updatedTestMerges).ConfigureAwait(false); shouldSyncTracked = stillOnOrigin; } else shouldSyncTracked = false; } - else + + if (!preserveTestMerges) { - logger.LogTrace("Not preserving test merges..."); + logger.LogTrace("Resetting to origin..."); await repo.ResetToOrigin(NextProgressReporter(), jobCancellationToken).ConfigureAwait(false); var currentHead = repo.Head; @@ -301,7 +331,7 @@ namespace Tgstation.Server.Host.Components .ConfigureAwait(false); if (currentHead != startSha && currentRevInfo == default) - await UpdateRevInfo(currentHead, true).ConfigureAwait(false); + await UpdateRevInfo(currentHead, true, null).ConfigureAwait(false); shouldSyncTracked = true; } @@ -312,7 +342,7 @@ namespace Tgstation.Server.Host.Components var pushedOrigin = await repo.Sychronize(repositorySettings.AccessUser, repositorySettings.AccessToken, repositorySettings.CommitterName, repositorySettings.CommitterEmail, NextProgressReporter(), shouldSyncTracked, jobCancellationToken).ConfigureAwait(false); var currentHead = repo.Head; if (currentHead != currentRevInfo.CommitSha) - await UpdateRevInfo(currentHead, pushedOrigin).ConfigureAwait(false); + await UpdateRevInfo(currentHead, pushedOrigin, null).ConfigureAwait(false); } repoHead = repo.Head; @@ -393,7 +423,69 @@ namespace Tgstation.Server.Host.Components logger.LogTrace("Leaving auto update loop..."); } - #pragma warning restore CA1502 +#pragma warning restore CA1502 + + /// + /// Get the updated list of s for an origin merge. + /// + /// The to use. + /// The . + /// The current . + /// The for the operation. + /// A resulting in the of s that should remain the new . + async Task> RemoveMergedPullRequests( + IRepository repository, + RepositorySettings repositorySettings, + RevisionInformation revisionInformation, + CancellationToken cancellationToken) + { + if (revisionInformation.ActiveTestMerges?.Any() != true) + { + logger.LogTrace("No test merges to remove."); + return Array.Empty(); + } + + var gitHubClient = repositorySettings.AccessToken != null + ? gitHubClientFactory.CreateClient(repositorySettings.AccessToken) + : (String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken) + ? gitHubClientFactory.CreateClient() + : gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken)); + + var tasks = revisionInformation + .ActiveTestMerges + .Select(x => gitHubClient + .PullRequest + .Get(repository.GitHubOwner, repository.GitHubRepoName, x.TestMerge.Number) + .WithToken(cancellationToken)); + try + { + await Task.WhenAll(tasks).ConfigureAwait(false); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + logger.LogWarning(ex, "Pull requests update check failed!"); + } + + var newList = revisionInformation.ActiveTestMerges.ToList(); + + async Task CheckRemovePR(Task task) + { + var pr = await task.ConfigureAwait(false); + if (!pr.Merged) + return; + + // We don't just assume, actually check the repo contains the merge commit. + if (await repository.ShaIsParent(pr.MergeCommitSha, cancellationToken).ConfigureAwait(false)) + newList.Remove( + newList.First( + potential => potential.TestMerge.Number == pr.Number)); + } + + foreach (var prTask in tasks) + await CheckRemovePR(prTask).ConfigureAwait(false); + + return newList; + } /// public Task InstanceRenamed(string newName, CancellationToken cancellationToken) diff --git a/src/Tgstation.Server.Host/Components/InstanceFactory.cs b/src/Tgstation.Server.Host/Components/InstanceFactory.cs index 916b6950f4..dc6243b294 100644 --- a/src/Tgstation.Server.Host/Components/InstanceFactory.cs +++ b/src/Tgstation.Server.Host/Components/InstanceFactory.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using System; using System.Threading; using System.Threading.Tasks; @@ -11,6 +12,7 @@ using Tgstation.Server.Host.Components.Interop.Bridge; using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Components.Session; using Tgstation.Server.Host.Components.Watchdog; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; using Tgstation.Server.Host.Database; using Tgstation.Server.Host.IO; @@ -123,6 +125,11 @@ namespace Tgstation.Server.Host.Components /// readonly IServerPortProvider serverPortProvider; + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// Construct an /// @@ -146,6 +153,7 @@ namespace Tgstation.Server.Host.Components /// The value of . /// The value of . /// The value of . + /// The containing the value of . public InstanceFactory( IIOManager ioManager, IDatabaseContextFactory databaseContextFactory, @@ -166,7 +174,8 @@ namespace Tgstation.Server.Host.Components IPlatformIdentifier platformIdentifier, ILibGit2RepositoryFactory repositoryFactory, ILibGit2Commands repositoryCommands, - IServerPortProvider serverPortProvider) + IServerPortProvider serverPortProvider, + IOptions generalConfigurationOptions) { this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); this.databaseContextFactory = databaseContextFactory ?? throw new ArgumentNullException(nameof(databaseContextFactory)); @@ -188,6 +197,7 @@ namespace Tgstation.Server.Host.Components this.repositoryFactory = repositoryFactory ?? throw new ArgumentNullException(nameof(repositoryFactory)); this.repositoryCommands = repositoryCommands ?? throw new ArgumentNullException(nameof(repositoryCommands)); this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider)); + generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); } /// @@ -287,7 +297,9 @@ namespace Tgstation.Server.Host.Components dmbFactory, jobManager, eventConsumer, - loggerFactory.CreateLogger()); + gitHubClientFactory, + loggerFactory.CreateLogger(), + generalConfiguration); return instance; } diff --git a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs index 44cb5b39db..7790315cc2 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Threading; using System.Threading.Tasks; using Tgstation.Server.Api.Models; @@ -132,5 +132,14 @@ namespace Tgstation.Server.Host.Components.Repository /// The for the operation /// A representing the running operation Task CopyTo(string path, CancellationToken cancellationToken); + + /// + /// Check if a given is a parent of the current . + /// + /// The SHA to check. + /// The for the operation. + /// A resulting in if is a parent of , otherwise. + /// This function is NOT reentrant. + Task ShaIsParent(string sha, CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 4db0962c02..1165e0f420 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -22,6 +22,16 @@ namespace Tgstation.Server.Host.Components.Repository /// public const string GitHubUrl = "://github.com/"; + /// + /// The default username for committers. + /// + public const string DefaultCommitterName = "tgstation-server"; + + /// + /// The default password for committers. + /// + public const string DefaultCommitterEmail = "tgstation-server@users.noreply.github.com"; + /// /// Template error message for when tracking of the most recent origin commit fails /// @@ -554,7 +564,7 @@ namespace Tgstation.Server.Host.Components.Repository trackedBranch = libGitRepo.Head.TrackedBranch; logger.LogDebug("Merge origin/{2}: <{0} ({1})>", committerName, committerEmail, trackedBranch.FriendlyName); - result = libGitRepo.Merge(trackedBranch, new Signature(new Identity(committerName, committerEmail), DateTimeOffset.Now), new MergeOptions + result = libGitRepo.Merge(trackedBranch, new Signature(committerName, committerEmail, DateTimeOffset.Now), new MergeOptions { CommitOnSuccess = true, FailOnConflict = true, @@ -708,5 +718,43 @@ namespace Tgstation.Server.Host.Components.Repository return true; return false; }, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Current); + + /// + public Task ShaIsParent(string sha, CancellationToken cancellationToken) => Task.Factory.StartNew(() => + { + var targetCommit = libGitRepo.Lookup(sha); + if(targetCommit == null) + { + logger.LogTrace("Commit {0} not found in repository", sha); + return false; + } + + cancellationToken.ThrowIfCancellationRequested(); + var startSha = Head; + var mergeResult = libGitRepo.Merge( + targetCommit, + new Signature( + DefaultCommitterName, + DefaultCommitterEmail, + DateTimeOffset.Now), + new MergeOptions + { + FastForwardStrategy = FastForwardStrategy.FastForwardOnly, + FailOnConflict = true + }); + + if (mergeResult.Status == MergeStatus.UpToDate) + return true; + + commands.Checkout( + libGitRepo, + new CheckoutOptions + { + CheckoutModifiers = CheckoutModifiers.Force + }, + startSha); + + return false; + }, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Current); } } diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 7483cecb69..243fe297db 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -126,8 +126,8 @@ namespace Tgstation.Server.Host.Controllers ChatBotLimit = initialSettings.ChatBotLimit ?? Models.Instance.DefaultChatBotLimit, RepositorySettings = new RepositorySettings { - CommitterEmail = "tgstation-server@users.noreply.github.com", - CommitterName = "tgstation-server", + CommitterEmail = Components.Repository.Repository.DefaultCommitterEmail, + CommitterName = Components.Repository.Repository.DefaultCommitterName, PushTestMergeCommits = false, ShowTestMergeCommitters = false, AutoUpdatesKeepTestMerges = false, diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index 127235d59d..3f96c59e2d 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -19,6 +19,8 @@ using System.Threading.Tasks; using Tgstation.Server.Api; using Tgstation.Server.Api.Models; using Tgstation.Server.Client; +using Tgstation.Server.Host.Components.Events; +using Tgstation.Server.Host.Components.Repository; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Database; using Tgstation.Server.Host.Database.Migrations; @@ -437,5 +439,30 @@ namespace Tgstation.Server.Tests Assert.AreEqual(String.Empty, (await process.GetErrorOutput(default)).Trim()); Assert.AreEqual("Hello World!", (await process.GetStandardOutput(default)).Trim()); } + + [TestMethod] + public async Task TestRepoParentLookup() + { + using var testingServer = new TestingServer(); + LibGit2Sharp.Repository.Clone("https://github.com/Cyberboss/test", testingServer.Directory); + var libGit2Repo = new LibGit2Sharp.Repository(testingServer.Directory); + using var repo = new Host.Components.Repository.Repository( + libGit2Repo, + new LibGit2Commands(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of>(), + () => { }); + + const string StartSha = "af4da8beb9f9b374b04a3cc4d65acca662e8cc1a"; + await repo.CheckoutObject(StartSha, progress => { }, default); + var result = await repo.ShaIsParent("2f8588a3ca0f6b027704a2a04381215619de3412", default); + Assert.IsTrue(result); + Assert.AreEqual(StartSha, repo.Head); + result = await repo.ShaIsParent("f636418bf47d238d33b0e4a34f0072b23a8aad0e", default); + Assert.IsFalse(result); ; + Assert.AreEqual(StartSha, repo.Head); + } } }