Skip to content

Commit 1e6e115

Browse files
MiYanniMiYanni
authored
Workload manifest updater cleanup (#35859)
2 parents 48ff2fc + b402558 commit 1e6e115

File tree

12 files changed

+185
-289
lines changed

12 files changed

+185
-289
lines changed

‎src/Cli/dotnet/commands/dotnet-workload/WorkloadInfoHelper.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public WorkloadInfoHelper(
6666
public IWorkloadInstallationRecordRepository WorkloadRecordRepo { get; private init; }
6767
public IWorkloadResolver WorkloadResolver { get; private init; }
6868

69-
public IEnumerable<WorkloadId> InstalledSdkWorkloadIds =>
70-
WorkloadRecordRepo.GetInstalledWorkloads(_currentSdkFeatureBand);
69+
public IEnumerable<WorkloadId> InstalledSdkWorkloadIds => WorkloadRecordRepo.GetInstalledWorkloads(_currentSdkFeatureBand);
7170

7271
public InstalledWorkloadsCollection AddInstalledVsWorkloads(IEnumerable<WorkloadId> sdkWorkloadIds)
7372
{

‎src/Cli/dotnet/commands/dotnet-workload/install/IWorkloadManifestUpdater.cs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using Microsoft.Extensions.EnvironmentAbstractions;
55
using Microsoft.NET.Sdk.WorkloadManifestReader;
6+
using WorkloadCollection = System.Collections.Generic.Dictionary<Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadId, Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadDefinition>;
67

78
namespace Microsoft.DotNet.Workloads.Workload.Install
89
{
@@ -12,18 +13,16 @@ internal interface IWorkloadManifestUpdater
1213

1314
Task BackgroundUpdateAdvertisingManifestsWhenRequiredAsync();
1415

15-
IEnumerable<(
16-
ManifestVersionUpdate manifestUpdate,
17-
Dictionary<WorkloadId, WorkloadDefinition> Workloads
18-
)> CalculateManifestUpdates();
16+
IEnumerable<ManifestUpdateWithWorkloads> CalculateManifestUpdates();
1917

20-
IEnumerable<ManifestVersionUpdate>
21-
CalculateManifestRollbacks(string rollbackDefinitionFilePath);
18+
IEnumerable<ManifestVersionUpdate> CalculateManifestRollbacks(string rollbackDefinitionFilePath);
2219

2320
Task<IEnumerable<WorkloadDownload>> GetManifestPackageDownloadsAsync(bool includePreviews, SdkFeatureBand providedSdkFeatureBand, SdkFeatureBand installedSdkFeatureBand);
2421

2522
IEnumerable<WorkloadId> GetUpdatableWorkloadsToAdvertise(IEnumerable<WorkloadId> installedWorkloads);
2623

2724
void DeleteUpdatableWorkloadsFile();
2825
}
26+
27+
internal record ManifestUpdateWithWorkloads(ManifestVersionUpdate ManifestUpdate, WorkloadCollection Workloads);
2928
}

‎src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public WorkloadInstallCommand(
4040
_workloadResolver, Verbosity, _userProfileDir, VerifySignatures, PackageDownloader, _dotnetPath, TempDirectoryPath,
4141
_packageSourceLocation, RestoreActionConfiguration, elevationRequired: !_printDownloadLinkOnly && string.IsNullOrWhiteSpace(_downloadToCacheOption));
4242

43-
_workloadManifestUpdater = _workloadManifestUpdaterFromConstructor ?? new WorkloadManifestUpdater(Reporter, _workloadResolver, PackageDownloader, _userProfileDir, TempDirectoryPath,
43+
_workloadManifestUpdater = _workloadManifestUpdaterFromConstructor ?? new WorkloadManifestUpdater(Reporter, _workloadResolver, PackageDownloader, _userProfileDir,
4444
_workloadInstaller.GetWorkloadInstallationRecordRepository(), _workloadInstaller, _packageSourceLocation, displayManifestUpdates: Verbosity.IsDetailedOrDiagnostic());
4545

4646
ValidateWorkloadIdsInput();
@@ -162,7 +162,7 @@ public void InstallWorkloads(IEnumerable<WorkloadId> workloadIds, bool skipManif
162162
_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(includePreviews, offlineCache).Wait();
163163
manifestsToUpdate = useRollback ?
164164
_workloadManifestUpdater.CalculateManifestRollbacks(_fromRollbackDefinition) :
165-
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => m.manifestUpdate);
165+
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => m.ManifestUpdate);
166166
}
167167

168168
InstallWorkloadsWithInstallRecord(_workloadInstaller, workloadIds, _sdkFeatureBand, manifestsToUpdate, offlineCache, useRollback);

‎src/Cli/dotnet/commands/dotnet-workload/install/WorkloadManifestUpdater.cs

Lines changed: 100 additions & 177 deletions
Large diffs are not rendered by default.

‎src/Cli/dotnet/commands/dotnet-workload/list/WorkloadListCommand.cs

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.NET.Sdk.WorkloadManifestReader;
1313
using Microsoft.TemplateEngine.Cli.Commands;
1414
using InformationStrings = Microsoft.DotNet.Workloads.Workload.LocalizableStrings;
15+
using WorkloadCollection = System.Collections.Generic.Dictionary<Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadId, Microsoft.NET.Sdk.WorkloadManifestReader.WorkloadDefinition>;
1516

1617
namespace Microsoft.DotNet.Workloads.Workload.List
1718
{
@@ -54,7 +55,7 @@ public WorkloadListCommand(
5455
string userProfileDir1 = userProfileDir ?? CliFolderPathCalculator.DotnetUserProfileFolderPath;
5556

5657
_workloadManifestUpdater = workloadManifestUpdater ?? new WorkloadManifestUpdater(Reporter,
57-
_workloadListHelper.WorkloadResolver, PackageDownloader, userProfileDir1, TempDirectoryPath, _workloadListHelper.WorkloadRecordRepo, _workloadListHelper.Installer);
58+
_workloadListHelper.WorkloadResolver, PackageDownloader, userProfileDir1, _workloadListHelper.WorkloadRecordRepo, _workloadListHelper.Installer);
5859
}
5960

6061
public override int Execute()
@@ -65,9 +66,9 @@ public override int Execute()
6566
{
6667
_workloadListHelper.CheckTargetSdkVersionIsValid();
6768

68-
UpdateAvailableEntry[] updateAvailable = GetUpdateAvailable(installedList);
69-
ListOutput listOutput = new(installedList.Select(id => id.ToString()).ToArray(),
70-
updateAvailable);
69+
var updateAvailable = GetUpdateAvailable(installedList);
70+
var installed = installedList.Select(id => id.ToString()).ToArray();
71+
ListOutput listOutput = new(installed, updateAvailable.ToArray());
7172

7273
Reporter.WriteLine("==workloadListJsonOutputStart==");
7374
Reporter.WriteLine(
@@ -108,29 +109,23 @@ public override int Execute()
108109
return 0;
109110
}
110111

111-
internal UpdateAvailableEntry[] GetUpdateAvailable(IEnumerable<WorkloadId> installedList)
112+
internal IEnumerable<UpdateAvailableEntry> GetUpdateAvailable(IEnumerable<WorkloadId> installedList)
112113
{
113-
HashSet<WorkloadId> installedWorkloads = installedList.ToHashSet();
114114
_workloadManifestUpdater.UpdateAdvertisingManifestsAsync(_includePreviews).Wait();
115-
var manifestsToUpdate =
116-
_workloadManifestUpdater.CalculateManifestUpdates();
115+
var manifestsToUpdate = _workloadManifestUpdater.CalculateManifestUpdates();
117116

118-
List<UpdateAvailableEntry> updateList = new();
119-
foreach ((ManifestVersionUpdate manifestUpdate, Dictionary<WorkloadId, WorkloadDefinition> workloads) in manifestsToUpdate)
117+
foreach ((ManifestVersionUpdate manifestUpdate, WorkloadCollection workloads) in manifestsToUpdate)
120118
{
121-
foreach ((WorkloadId WorkloadId, WorkloadDefinition workloadDefinition) in
122-
workloads)
119+
foreach ((WorkloadId workloadId, WorkloadDefinition workloadDefinition) in workloads)
123120
{
124-
if (installedWorkloads.Contains(new WorkloadId(WorkloadId.ToString())))
121+
if (installedList.Contains(workloadId))
125122
{
126-
updateList.Add(new UpdateAvailableEntry(manifestUpdate.ExistingVersion.ToString(),
123+
yield return new UpdateAvailableEntry(manifestUpdate.ExistingVersion.ToString(),
127124
manifestUpdate.NewVersion.ToString(),
128-
workloadDefinition.Description, WorkloadId.ToString()));
125+
workloadDefinition.Description, workloadId.ToString());
129126
}
130127
}
131128
}
132-
133-
return updateList.ToArray();
134129
}
135130

136131
internal record ListOutput(string[] Installed, UpdateAvailableEntry[] UpdateAvailable);

‎src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public WorkloadUpdateCommand(
4242
_dotnetPath, TempDirectoryPath, packageSourceLocation: _packageSourceLocation, RestoreActionConfiguration,
4343
elevationRequired: !_printDownloadLinkOnly && !_printRollbackDefinitionOnly && string.IsNullOrWhiteSpace(_downloadToCacheOption));
4444

45-
_workloadManifestUpdater = _workloadManifestUpdaterFromConstructor ?? new WorkloadManifestUpdater(Reporter, _workloadResolver, PackageDownloader, _userProfileDir, TempDirectoryPath,
45+
_workloadManifestUpdater = _workloadManifestUpdaterFromConstructor ?? new WorkloadManifestUpdater(Reporter, _workloadResolver, PackageDownloader, _userProfileDir,
4646
_workloadInstaller.GetWorkloadInstallationRecordRepository(), _workloadInstaller, _packageSourceLocation, sdkFeatureBand: _sdkFeatureBand);
4747
}
4848

@@ -108,7 +108,7 @@ public void UpdateWorkloads(bool includePreviews = false, DirectoryPath? offline
108108

109109
var manifestsToUpdate = useRollback ?
110110
_workloadManifestUpdater.CalculateManifestRollbacks(_fromRollbackDefinition) :
111-
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => m.manifestUpdate);
111+
_workloadManifestUpdater.CalculateManifestUpdates().Select(m => m.ManifestUpdate);
112112

113113
UpdateWorkloadsWithInstallRecord(_sdkFeatureBand, manifestsToUpdate, useRollback, offlineCache);
114114

‎src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadId.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
namespace Microsoft.NET.Sdk.WorkloadManifestReader
55
{
66
/// <summary>
7-
/// Wraps a workload definition id string to help ensure consistency of behaviour/semantics.
7+
/// Wraps a workload definition id string to help ensure consistency of behavior/semantics.
88
/// Comparisons are case insensitive but ToString() will return the original string for display purposes.
99
/// </summary>
1010
public readonly struct WorkloadId : IComparable<WorkloadId>, IEquatable<WorkloadId>

‎src/Tests/dotnet-workload-install.Tests/GivenDotnetWorkloadInstall.cs

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,8 @@ public void GivenNoWorkloadsInstalledInfoOptionRemarksOnThat()
169169
// However, we can test a setup where no workloads are installed and --info is provided.
170170

171171
_reporter.Clear();
172-
var mockWorkloadIds = new WorkloadId[] { new WorkloadId("xamarin-android"), new WorkloadId("xamarin-android-build") };
173172
var testDirectory = _testAssetsManager.CreateTestDirectory().Path;
174173
var dotnetRoot = Path.Combine(testDirectory, "dotnet");
175-
var installer = new MockPackWorkloadInstaller();
176174
var workloadResolver = WorkloadResolver.CreateForTests(new MockManifestProvider(new[] { _manifestPath }), dotnetRoot);
177175
var parseResult = Parser.Instance.Parse(new string[] { "dotnet", "workload", "install", "xamarin-android"});
178176

@@ -207,18 +205,17 @@ public void GivenWorkloadInstallItCanUpdateInstalledManifests(bool userLocal, st
207205
Parser.Instance.Parse(new string[] {"dotnet", "workload", "install", "xamarin-android"});
208206
var featureBand = new SdkFeatureBand(sdkVersion);
209207
var manifestsToUpdate =
210-
new (ManifestVersionUpdate manifestUpdate, Dictionary<WorkloadId, WorkloadDefinition> Workloads)[]
208+
new ManifestUpdateWithWorkloads[]
211209
{
212-
(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), featureBand.ToString(), new ManifestVersion("2.0.0"), featureBand.ToString()),
213-
null),
210+
new(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), featureBand.ToString(), new ManifestVersion("2.0.0"), featureBand.ToString()), null),
214211
};
215212
(_, var installManager, var installer, _, _, _) =
216213
GetTestInstallers(parseResult, userLocal, sdkVersion, manifestUpdates: manifestsToUpdate, installedFeatureBand: sdkVersion);
217214

218215
installManager.InstallWorkloads(new List<WorkloadId>(), false); // Don't actually do any installs, just update manifests
219216

220-
installer.InstalledManifests[0].manifestUpdate.ManifestId.Should().Be(manifestsToUpdate[0].manifestUpdate.ManifestId);
221-
installer.InstalledManifests[0].manifestUpdate.NewVersion.Should().Be(manifestsToUpdate[0].manifestUpdate.NewVersion);
217+
installer.InstalledManifests[0].manifestUpdate.ManifestId.Should().Be(manifestsToUpdate[0].ManifestUpdate.ManifestId);
218+
installer.InstalledManifests[0].manifestUpdate.NewVersion.Should().Be(manifestsToUpdate[0].ManifestUpdate.NewVersion);
222219
installer.InstalledManifests[0].manifestUpdate.NewFeatureBand.Should().Be(new SdkFeatureBand(sdkVersion).ToString());
223220
installer.InstalledManifests[0].offlineCache.Should().Be(null);
224221
}
@@ -232,11 +229,9 @@ public void GivenWorkloadInstallFromCacheItInstallsCachedManifest(bool userLocal
232229
{
233230
var featureBand = new SdkFeatureBand(sdkVersion);
234231
var manifestsToUpdate =
235-
new (ManifestVersionUpdate manifestUpdate, Dictionary<WorkloadId, WorkloadDefinition>
236-
Workloads)[]
232+
new ManifestUpdateWithWorkloads[]
237233
{
238-
(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), featureBand.ToString(), new ManifestVersion("2.0.0"), featureBand.ToString()),
239-
null)
234+
new(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), featureBand.ToString(), new ManifestVersion("2.0.0"), featureBand.ToString()), null)
240235
};
241236
var cachePath = Path.Combine(_testAssetsManager.CreateTestDirectory(identifier: AppendForUserLocal("mockCache_", userLocal) + sdkVersion).Path,
242237
"mockCachePath");
@@ -249,8 +244,8 @@ public void GivenWorkloadInstallFromCacheItInstallsCachedManifest(bool userLocal
249244

250245
installManager.Execute();
251246

252-
installer.InstalledManifests[0].manifestUpdate.ManifestId.Should().Be(manifestsToUpdate[0].manifestUpdate.ManifestId);
253-
installer.InstalledManifests[0].manifestUpdate.NewVersion.Should().Be(manifestsToUpdate[0].manifestUpdate.NewVersion);
247+
installer.InstalledManifests[0].manifestUpdate.ManifestId.Should().Be(manifestsToUpdate[0].ManifestUpdate.ManifestId);
248+
installer.InstalledManifests[0].manifestUpdate.NewVersion.Should().Be(manifestsToUpdate[0].ManifestUpdate.NewVersion);
254249
installer.InstalledManifests[0].manifestUpdate.NewFeatureBand.Should().Be(new SdkFeatureBand(sdkVersion).ToString());
255250
installer.InstalledManifests[0].offlineCache.Should().Be(new DirectoryPath(cachePath));
256251
}
@@ -264,7 +259,7 @@ public void GivenWorkloadInstallItCanDownloadToOfflineCache(bool userLocal, stri
264259
{
265260
var cachePath = Path.Combine(_testAssetsManager.CreateTestDirectory(identifier: AppendForUserLocal("mockCache_", userLocal) + sdkVersion).Path, "mockCachePath");
266261
var parseResult = Parser.Instance.Parse(new string[] { "dotnet", "workload", "install", "xamarin-android", "--download-to-cache", cachePath });
267-
(_, var installManager, var installer, _, var manifestUpdater, var packageDownloader) = GetTestInstallers(parseResult, userLocal, sdkVersion, tempDirManifestPath: _manifestPath, installedFeatureBand: sdkVersion);
262+
(_, var installManager, _, _, var manifestUpdater, var packageDownloader) = GetTestInstallers(parseResult, userLocal, sdkVersion, tempDirManifestPath: _manifestPath, installedFeatureBand: sdkVersion);
268263

269264
installManager.Execute();
270265

@@ -457,7 +452,7 @@ static void CreateFile(string path)
457452
string sdkVersion,
458453
[CallerMemberName] string testName = "",
459454
string failingWorkload = null,
460-
IEnumerable<(ManifestVersionUpdate manifestUpdate, Dictionary<WorkloadId, WorkloadDefinition> Workloads)> manifestUpdates = null,
455+
IEnumerable<ManifestUpdateWithWorkloads> manifestUpdates = null,
461456
string tempDirManifestPath = null,
462457
string installedFeatureBand = null)
463458
{
@@ -472,7 +467,7 @@ static void CreateFile(string path)
472467
};
473468

474469
var nugetDownloader = new MockNuGetPackageDownloader(dotnetRoot);
475-
var manifestUpdater = new MockWorkloadManifestUpdater(manifestUpdates, tempDirManifestPath);
470+
var manifestUpdater = new MockWorkloadManifestUpdater(manifestUpdates);
476471
if (userLocal)
477472
{
478473
WorkloadFileBasedInstall.SetUserLocal(dotnetRoot, sdkVersion);
@@ -593,13 +588,11 @@ public void ShowManifestUpdatesWhenVerbosityIsDetailedOrDiagnostic(string verbos
593588
var parseResult =
594589
Parser.Instance.Parse(new string[] { "dotnet", "workload", "install", verbosityFlag, "xamarin-android" });
595590
var manifestsToUpdate =
596-
new (ManifestVersionUpdate manifestUpdate, Dictionary<WorkloadId, WorkloadDefinition>
597-
Workloads)[]
591+
new ManifestUpdateWithWorkloads[]
598592
{
599-
(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), sdkFeatureBand, new ManifestVersion("2.0.0"), sdkFeatureBand),
600-
null),
593+
new(new ManifestVersionUpdate(new ManifestId("mock-manifest"), new ManifestVersion("1.0.0"), sdkFeatureBand, new ManifestVersion("2.0.0"), sdkFeatureBand), null),
601594
};
602-
(_, var installManager, var installer, _, _, _) =
595+
(_, var installManager, _, _, _, _) =
603596
GetTestInstallers(parseResult, true, sdkFeatureBand, manifestUpdates: manifestsToUpdate);
604597

605598
installManager.InstallWorkloads(new List<WorkloadId>(), false); // Don't actually do any installs, just update manifests

0 commit comments

Comments
 (0)