Test merging cleanup

This commit is contained in:
Cyberboss
2018-08-10 14:17:10 -04:00
parent 449d930df0
commit a430d7b742
4 changed files with 77 additions and 49 deletions
@@ -10,10 +10,10 @@ namespace Tgstation.Server.Api.Models
/// <summary>
/// The number of the pull request
/// </summary>
public int Number { get; set; }
public int? Number { get; set; }
/// <summary>
/// The sha of the pull request revision to merge
/// The sha of the pull request revision to merge. If not specified, the latest commit shall be used (semi-unsafe)
/// </summary>
[Required]
public string PullRequestRevision { get; set; }
@@ -1,6 +1,7 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Tgstation.Server.Api.Models;
namespace Tgstation.Server.Host.Components.Repository
{
@@ -55,17 +56,15 @@ namespace Tgstation.Server.Host.Components.Repository
/// <summary>
/// Attempt to merge a GitHub pull request into HEAD
/// </summary>
/// <param name="pullRequestNumber">The pull request number on the remote repository</param>
/// <param name="targetCommit">The commit in the pull request to merge</param>
/// <param name="testMergeParameters">The <see cref="TestMergeParameters"/> of the pull request</param>
/// <param name="committerName">The name of the merge committer</param>
/// <param name="committerEmail">The e-mail of the merge committer</param>
/// <param name="commitMessage">The commit message</param>
/// <param name="username">The username to fetch from the origin repository</param>
/// <param name="password">The password to fetch from the origin repository</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation</param>
/// <param name="progressReporter">Optional function to report 0-100 progress of the clone</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in a <see cref="Nullable{T}"/> <see cref="bool"/> representing the merge result that is <see langword="true"/> after a fast forward or up to date, <see langword="false"/> on a merge, <see langword="null"/> on a conflict</returns>
Task<bool?> AddTestMerge(int pullRequestNumber, string targetCommit, string committerName, string committerEmail, string commitMessage, string username, string password, Action<int> progressReporter, CancellationToken cancellationToken);
Task<bool?> AddTestMerge(TestMergeParameters testMergeParameters, string committerName, string committerEmail, string username, string password, Action<int> progressReporter, CancellationToken cancellationToken);
/// <summary>
/// Fetch commits from the origin repository
@@ -5,6 +5,7 @@ using System.Globalization;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Tgstation.Server.Api.Models;
using Tgstation.Server.Host.IO;
namespace Tgstation.Server.Host.Components.Repository
@@ -121,17 +122,27 @@ namespace Tgstation.Server.Host.Components.Repository
}
/// <inheritdoc />
public async Task<bool?> AddTestMerge(int pullRequestNumber, string targetCommit, string committerName, string committerEmail, string commitMessage, string username, string password, Action<int> progressReporter, CancellationToken cancellationToken)
public async Task<bool?> AddTestMerge(TestMergeParameters testMergeParameters, string committerName, string committerEmail, string username, string password, Action<int> progressReporter, CancellationToken cancellationToken)
{
if (testMergeParameters == null)
throw new ArgumentNullException(nameof(testMergeParameters));
if (committerName == null)
throw new ArgumentNullException(nameof(committerName));
if (committerEmail == null)
throw new ArgumentNullException(nameof(committerEmail));
if (!IsGitHubRepository)
throw new InvalidOperationException("Test merging is only available on GitHub hosted origin repositories!");
var Refspec = new List<string>();
var prBranchName = String.Format(CultureInfo.InvariantCulture, "pr-{0}", pullRequestNumber);
var localBranchName = String.Format(CultureInfo.InvariantCulture, "pull/{0}/headrefs/heads/{1}", pullRequestNumber, prBranchName);
Refspec.Add(String.Format(CultureInfo.InvariantCulture, "pull/{0}/head:{1}", pullRequestNumber, prBranchName));
var logMessage = String.Format(CultureInfo.InvariantCulture, "Merge remote pull request #{0}", pullRequestNumber);
var commitMessage = String.Format(CultureInfo.InvariantCulture, "Test merge of pull request #{0}{1}{2}", testMergeParameters.Number.Value, testMergeParameters.Comment != null ? Environment.NewLine : String.Empty, 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 Refspec = new List<string> { String.Format(CultureInfo.InvariantCulture, "pull/{0}/head:{1}", testMergeParameters.Number, prBranchName) };
var logMessage = String.Format(CultureInfo.InvariantCulture, "Merge remote pull request #{0}", testMergeParameters.Number);
var originalCommit = repository.Head;
@@ -142,39 +153,47 @@ namespace Tgstation.Server.Host.Components.Repository
{
try
{
var remote = repository.Network.Remotes.First();
Commands.Fetch((LibGit2Sharp.Repository)repository, remote.Name, Refspec, new FetchOptions
try
{
Prune = true,
OnProgress = (a) => !cancellationToken.IsCancellationRequested,
OnTransferProgress = (a) =>
var remote = repository.Network.Remotes.First();
Commands.Fetch((LibGit2Sharp.Repository)repository, remote.Name, Refspec, new FetchOptions
{
var percentage = 100 * (((float)a.IndexedObjects + a.ReceivedObjects) / (a.TotalObjects * 2));
progressReporter?.Invoke((int)percentage);
return !cancellationToken.IsCancellationRequested;
},
OnUpdateTips = (a, b, c) => !cancellationToken.IsCancellationRequested,
CredentialsProvider = (a, b, c) => username != null ? (Credentials)new UsernamePasswordCredentials
{
Username = username,
Password = password
} : new DefaultCredentials()
}, logMessage);
}
catch (UserCancelledException)
{
Prune = true,
OnProgress = (a) => !cancellationToken.IsCancellationRequested,
OnTransferProgress = (a) =>
{
var percentage = 100 * (((float)a.IndexedObjects + a.ReceivedObjects) / (a.TotalObjects * 2));
progressReporter?.Invoke((int)percentage);
return !cancellationToken.IsCancellationRequested;
},
OnUpdateTips = (a, b, c) => !cancellationToken.IsCancellationRequested,
CredentialsProvider = (a, b, c) => username != null ? (Credentials)new UsernamePasswordCredentials
{
Username = username,
Password = password
} : new DefaultCredentials()
}, logMessage);
}
catch (UserCancelledException) { }
cancellationToken.ThrowIfCancellationRequested();
testMergeParameters.PullRequestRevision = repository.Lookup(testMergeParameters.PullRequestRevision ?? localBranchName).Sha;
cancellationToken.ThrowIfCancellationRequested();
result = repository.Merge(testMergeParameters.PullRequestRevision, sig, new MergeOptions
{
CommitOnSuccess = commitMessage == null,
FailOnConflict = true,
FastForwardStrategy = FastForwardStrategy.NoFastForward,
SkipReuc = true
});
}
cancellationToken.ThrowIfCancellationRequested();
result = repository.Merge(targetCommit, sig, new MergeOptions
finally
{
CommitOnSuccess = commitMessage == null,
FailOnConflict = true,
FastForwardStrategy = FastForwardStrategy.NoFastForward,
SkipReuc = true
});
repository.Branches.Remove(localBranchName);
}
cancellationToken.ThrowIfCancellationRequested();
@@ -189,7 +208,7 @@ namespace Tgstation.Server.Host.Components.Repository
if (result.Status == MergeStatus.Conflicts)
{
await eventConsumer.HandleEvent(EventType.RepoMergeConflict, new List<string> { originalCommit.Tip.Sha, targetCommit, originalCommit.FriendlyName ?? UnknownReference, prBranchName }, cancellationToken).ConfigureAwait(false);
await eventConsumer.HandleEvent(EventType.RepoMergeConflict, new List<string> { originalCommit.Tip.Sha, testMergeParameters.PullRequestRevision, originalCommit.FriendlyName ?? UnknownReference, prBranchName }, cancellationToken).ConfigureAwait(false);
return false;
}
@@ -243,6 +243,9 @@ namespace Tgstation.Server.Host.Controllers
if (model.Origin != null)
return BadRequest(new ErrorMessage { Message = "origin cannot be modified without deleting the repository!" });
if(model.NewTestMerges.Any(x => !x.Number.HasValue))
return BadRequest(new ErrorMessage { Message = "All new test merges must provide a number!" });
var newTestMerges = model.NewTestMerges != null && model.NewTestMerges.Count > 0;
var userRights = (RepositoryRights)AuthenticationContext.GetRight(RightsType.Repository);
if (newTestMerges && !userRights.HasFlag(RepositoryRights.MergePullRequest))
@@ -396,11 +399,13 @@ namespace Tgstation.Server.Host.Controllers
I.PullRequestRevision = I.PullRequestRevision?.ToLowerInvariant();
#pragma warning restore CA1308 // Normalize strings to uppercase
revInfoWereLookingFor = await databaseContext.RevisionInformations.Where(
x => x.OriginCommitSha == lastRevisionInfo.OriginCommitSha
&& x.ActiveTestMerges.Count == model.NewTestMerges.Count)
revInfoWereLookingFor = await databaseContext.RevisionInformations
.Where(x => x.OriginCommitSha == lastRevisionInfo.OriginCommitSha && x.ActiveTestMerges.Count == model.NewTestMerges.Count)
//split here cause this bit probably has to be done locally
.Where(x => x.ActiveTestMerges.Select(y => y.TestMerge).All(y => model.NewTestMerges.Any(z => y.Number == z.Number && y.PullRequestRevision.StartsWith(z.PullRequestRevision, StringComparison.Ordinal)))).FirstOrDefaultAsync(ct).ConfigureAwait(false);
.Where(x => x.ActiveTestMerges.Select(y => y.TestMerge)
.All(y => model.NewTestMerges.Any(z => y.Number == z.Number && y.PullRequestRevision.StartsWith(z.PullRequestRevision, StringComparison.Ordinal))))
.Include(x => x.ActiveTestMerges).ThenInclude(x => x.TestMerge)
.FirstOrDefaultAsync(ct).ConfigureAwait(false);
}
@@ -423,7 +428,7 @@ namespace Tgstation.Server.Host.Controllers
string errorMessage = null;
try
{
pr = await gitHubClient.PullRequest.Get(repoOwner, repoName, I.Number).ConfigureAwait(false);
pr = await gitHubClient.PullRequest.Get(repoOwner, repoName, I.Number.Value).ConfigureAwait(false);
}
catch (Octokit.RateLimitExceededException)
{
@@ -432,11 +437,15 @@ namespace Tgstation.Server.Host.Controllers
}
catch (Octokit.NotFoundException)
{
//you look at your shithub access and sigh
//you look at your shithub and sigh
errorMessage = "P.R.E. NOT FOUND";
}
var mergeResult = await repo.AddTestMerge(I.Number, I.PullRequestRevision, committerName, currentModel.CommitterEmail, String.Format(CultureInfo.InvariantCulture, "Test merge of pull request #{0}{1}{2}", I.Number, I.Comment != null ? Environment.NewLine : null, I.Comment), currentModel.AccessUser, currentModel.AccessToken, x => progressReporter((x + 100 * doneFetches) / numFetches), ct).ConfigureAwait(false);
//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 mergeResult = await repo.AddTestMerge(I, committerName, currentModel.CommitterEmail, currentModel.AccessUser, currentModel.AccessToken, x => progressReporter((x + 100 * doneFetches) / numFetches), ct).ConfigureAwait(false);
if (!mergeResult.HasValue) //conflict, we don't care, dd already knows
continue;
@@ -469,7 +478,8 @@ namespace Tgstation.Server.Host.Controllers
}
}
if (startSha != repo.Head)
//never synchronize with test merges
if (startSha != repo.Head && lastRevisionInfo.ActiveTestMerges.Count == 0)
{
await repo.Sychronize(currentModel.AccessUser, currentModel.AccessToken, false, ct).ConfigureAwait(false);
await UpdateRevInfo().ConfigureAwait(false);