Skip to content

Commit e4a535f

Browse files
brettfobrettfo
authored
Merge pull request #14475 from dependabot/dev/brettfo/nuget-group-application
honor update-types in grouped/ungrouped updater
2 parents f294b40 + 6d2c9b6 commit e4a535f

3 files changed

Lines changed: 215 additions & 9 deletions

File tree

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Run/UpdateHandlers/GroupUpdateAllVersionsHandlerTests.cs‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,4 +1370,150 @@ await TestAsync(
13701370
]
13711371
);
13721372
}
1373+
1374+
[Fact]
1375+
public async Task UngroupedPullRequestCanBeCreatedIfGroupAppliesToNonMatchedTypes()
1376+
{
1377+
// group only applies to minor and patch updates, but a major update is requested and gets generated separately
1378+
await TestAsync(
1379+
job: new Job()
1380+
{
1381+
Source = CreateJobSource("/src"),
1382+
DependencyGroups = [new()
1383+
{
1384+
Name = "test-group",
1385+
Rules = new()
1386+
{
1387+
["patterns"] = new[] { "Some.Dependency" },
1388+
["update-types"] = new[] { "minor", "patch" }
1389+
},
1390+
}]
1391+
},
1392+
files: [
1393+
("src/project.csproj", "initial contents"),
1394+
],
1395+
discoveryWorker: TestDiscoveryWorker.FromResults(
1396+
("/src", new WorkspaceDiscoveryResult()
1397+
{
1398+
Path = "/src",
1399+
Projects = [
1400+
new()
1401+
{
1402+
FilePath = "project.csproj",
1403+
Dependencies = [
1404+
new("Some.Dependency", "1.0.0", DependencyType.PackageReference, TargetFrameworks: ["net9.0"]),
1405+
],
1406+
ImportedFiles = [],
1407+
AdditionalFiles = [],
1408+
}
1409+
],
1410+
})
1411+
),
1412+
analyzeWorker: new TestAnalyzeWorker(input =>
1413+
{
1414+
var repoRoot = input.Item1;
1415+
var discovery = input.Item2;
1416+
var dependencyInfo = input.Item3;
1417+
var newVersion = dependencyInfo.Name switch
1418+
{
1419+
"Some.Dependency" => "2.0.0",
1420+
_ => throw new NotImplementedException($"Test didn't expect to update dependency {dependencyInfo.Name}"),
1421+
};
1422+
return Task.FromResult(new AnalysisResult()
1423+
{
1424+
CanUpdate = true,
1425+
UpdatedVersion = newVersion,
1426+
UpdatedDependencies = [],
1427+
});
1428+
}),
1429+
updaterWorker: new TestUpdaterWorker(async input =>
1430+
{
1431+
var repoRoot = input.Item1;
1432+
var workspacePath = input.Item2;
1433+
var dependencyName = input.Item3;
1434+
var previousVersion = input.Item4;
1435+
var newVersion = input.Item5;
1436+
var isTransitive = input.Item6;
1437+
1438+
await File.WriteAllTextAsync(Path.Join(repoRoot, workspacePath), "updated contents");
1439+
1440+
return new UpdateOperationResult()
1441+
{
1442+
UpdateOperations = [new DirectUpdate() { DependencyName = dependencyName, NewVersion = NuGetVersion.Parse(newVersion), UpdatedFiles = [workspacePath] }],
1443+
};
1444+
}),
1445+
expectedUpdateHandler: GroupUpdateAllVersionsHandler.Instance,
1446+
expectedApiMessages: [
1447+
new IncrementMetric()
1448+
{
1449+
Metric = "updater.started",
1450+
Tags = new()
1451+
{
1452+
["operation"] = "group_update_all_versions",
1453+
}
1454+
},
1455+
// discovery from group updater
1456+
new UpdatedDependencyList()
1457+
{
1458+
Dependencies = [
1459+
new()
1460+
{
1461+
Name = "Some.Dependency",
1462+
Version = "1.0.0",
1463+
Requirements = [
1464+
new() { Requirement = "1.0.0", File = "/src/project.csproj", Groups = ["dependencies"] },
1465+
],
1466+
},
1467+
],
1468+
DependencyFiles = ["/src/project.csproj"],
1469+
},
1470+
// discovery from ungrouped updater
1471+
new UpdatedDependencyList()
1472+
{
1473+
Dependencies = [
1474+
new()
1475+
{
1476+
Name = "Some.Dependency",
1477+
Version = "1.0.0",
1478+
Requirements = [
1479+
new() { Requirement = "1.0.0", File = "/src/project.csproj", Groups = ["dependencies"] },
1480+
],
1481+
},
1482+
],
1483+
DependencyFiles = ["/src/project.csproj"],
1484+
},
1485+
new CreatePullRequest()
1486+
{
1487+
Dependencies = [
1488+
new()
1489+
{
1490+
Name = "Some.Dependency",
1491+
Version = "2.0.0",
1492+
Requirements = [
1493+
new() { Requirement = "2.0.0", File = "/src/project.csproj", Groups = ["dependencies"], Source = new() { SourceUrl = null } },
1494+
],
1495+
PreviousVersion = "1.0.0",
1496+
PreviousRequirements = [
1497+
new() { Requirement = "1.0.0", File = "/src/project.csproj", Groups = ["dependencies"] },
1498+
],
1499+
},
1500+
],
1501+
UpdatedDependencyFiles = [
1502+
new()
1503+
{
1504+
Directory = "/src",
1505+
Name = "project.csproj",
1506+
Content = "updated contents",
1507+
},
1508+
],
1509+
BaseCommitSha = "TEST-COMMIT-SHA",
1510+
CommitMessage = EndToEndTests.TestPullRequestCommitMessage,
1511+
PrTitle = EndToEndTests.TestPullRequestTitle,
1512+
PrBody = EndToEndTests.TestPullRequestBody,
1513+
DependencyGroup = null,
1514+
},
1515+
new MarkAsProcessed("TEST-COMMIT-SHA"),
1516+
]
1517+
);
1518+
}
13731519
}

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/ApiModel/DependencyGroup.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
using System.IO.Enumeration;
33
using System.Text.Json;
44

5+
using NuGet.Versioning;
6+
57
namespace NuGetUpdater.Core.Run.ApiModel;
68

79
public record DependencyGroup
@@ -40,6 +42,32 @@ public bool IsMatch(string dependencyName)
4042
return isMatch;
4143
}
4244

45+
public bool IsAllowedByVersion(NuGetVersion oldVersion, NuGetVersion newVersion)
46+
{
47+
var isMajorBump = newVersion.Major > oldVersion.Major;
48+
var isMinorBump = newVersion.Major == oldVersion.Major && newVersion.Minor > oldVersion.Minor;
49+
var isPatchBump = newVersion.Major == oldVersion.Major && newVersion.Minor == oldVersion.Minor && newVersion.Patch > oldVersion.Patch;
50+
51+
var allowedUpdateTypes = new HashSet<GroupUpdateType>(UpdateTypes);
52+
53+
if (isMajorBump && allowedUpdateTypes.Contains(GroupUpdateType.Major))
54+
{
55+
return true;
56+
}
57+
58+
if (isMinorBump && allowedUpdateTypes.Contains(GroupUpdateType.Minor))
59+
{
60+
return true;
61+
}
62+
63+
if (isPatchBump && allowedUpdateTypes.Contains(GroupUpdateType.Patch))
64+
{
65+
return true;
66+
}
67+
68+
return false;
69+
}
70+
4371
public static GroupMatcher FromRules(Dictionary<string, object> rules)
4472
{
4573
var patterns = GetStringArray(rules, "patterns", ["*"]); // default to matching everything unless explicitly excluded

‎nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Run/UpdateHandlers/GroupUpdateAllVersionsHandler.cs‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
using System.Collections.Immutable;
22

3+
using NuGet.Versioning;
4+
5+
using NuGetUpdater.Core.Analyze;
36
using NuGetUpdater.Core.Discover;
47
using NuGetUpdater.Core.Run.ApiModel;
58
using NuGetUpdater.Core.Updater;
@@ -106,6 +109,13 @@ private async Task RunGroupedDependencyUpdates(Job job, DirectoryInfo originalRe
106109
continue;
107110
}
108111

112+
var isUpdateAllowed = groupMatcher.IsAllowedByVersion(NuGetVersion.Parse(dependency.Version!), NuGetVersion.Parse(analysisResult.UpdatedVersion));
113+
if (!isUpdateAllowed)
114+
{
115+
logger.Info($"Dependency {dependency.Name} skipped for group {group.Name} because update type was not allowed.");
116+
continue;
117+
}
118+
109119
var projectDiscovery = discoveryResult.GetProjectDiscoveryFromPath(projectPath);
110120
var updaterResult = await updaterWorker.RunAsync(repoContentsPath.FullName, projectPath, dependency.Name, dependency.Version!, analysisResult.UpdatedVersion, dependency.IsTransitive);
111121
if (updaterResult.Error is not null)
@@ -204,15 +214,6 @@ private async Task RunUngroupedDependencyUpdates(Job job, DirectoryInfo original
204214
continue;
205215
}
206216

207-
var matchingGroups = job.DependencyGroups
208-
.Where(group => group.GetGroupMatcher().IsMatch(dependency.Name))
209-
.ToImmutableArray();
210-
if (matchingGroups.Length > 0)
211-
{
212-
logger.Info($"Dependency {dependency.Name} skipped for ungrouped updates because it's a member of the following groups: {string.Join(", ", matchingGroups.Select(group => group.Name))}");
213-
continue;
214-
}
215-
216217
var dependencyInfo = RunWorker.GetDependencyInfo(job, dependency, groupMatchers: [], allowCooldown: true);
217218
var analysisResult = await analyzeWorker.RunAsync(repoContentsPath.FullName, discoveryResult, dependencyInfo);
218219
if (analysisResult.Error is not null)
@@ -228,6 +229,12 @@ private async Task RunUngroupedDependencyUpdates(Job job, DirectoryInfo original
228229
continue;
229230
}
230231

232+
var isSkipped = IsUngroupedDependencySkipped(dependency, analysisResult, job.DependencyGroups, logger);
233+
if (isSkipped)
234+
{
235+
continue;
236+
}
237+
231238
var projectDiscovery = discoveryResult.GetProjectDiscoveryFromPath(projectPath);
232239
var updaterResult = await updaterWorker.RunAsync(repoContentsPath.FullName, projectPath, dependency.Name, dependency.Version!, analysisResult.UpdatedVersion, dependency.IsTransitive);
233240
if (updaterResult.Error is not null)
@@ -293,4 +300,29 @@ await apiHandler.CreatePullRequest(new CreatePullRequest()
293300
.ToImmutableArray();
294301
return updateOperationsToPerformByDependency;
295302
}
303+
304+
internal static bool IsUngroupedDependencySkipped(Dependency dependency, AnalysisResult dependencyAnalysis, ImmutableArray<DependencyGroup> dependencyGroups, ILogger logger)
305+
{
306+
var matcherGroups = dependencyGroups
307+
.Select(group => (group.Name, Matcher: group.GetGroupMatcher()))
308+
.Where(pair => pair.Matcher.IsMatch(dependency.Name))
309+
.ToImmutableArray();
310+
if (matcherGroups.Length > 0)
311+
{
312+
// update matches a group by name
313+
// if any group allows the proposed version range, then it's not allowed in an ungrouped update
314+
var oldVersion = NuGetVersion.Parse(dependency.Version!);
315+
var newVersion = NuGetVersion.Parse(dependencyAnalysis.UpdatedVersion);
316+
var matcherGroupsAllowingVersionRange = matcherGroups
317+
.Where(pair => pair.Matcher.IsAllowedByVersion(oldVersion, newVersion))
318+
.ToImmutableArray();
319+
if (matcherGroupsAllowingVersionRange.Length > 0)
320+
{
321+
logger.Info($"Dependency {dependency.Name} skipped for ungrouped updates because it's a member of the following groups: {string.Join(", ", matcherGroupsAllowingVersionRange.Select(pair => pair.Name))}");
322+
return true;
323+
}
324+
}
325+
326+
return false;
327+
}
296328
}

0 commit comments

Comments
 (0)