Merge pull request #11303 from dependabot/dev/brettfo/nuget-packages-…

…config-parse

validate shape of `packages.config` and consolidate error handling

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Analyze.cs

@@ -26,6 +26,8 @@ public async Task FindsUpdatedPackageAndReturnsTheCorrectData()
             await RunAsync(path =>
                 [
                     "analyze",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -146,6 +148,8 @@ public async Task DotNetToolsJsonCanBeAnalyzed()
             await RunAsync(path =>
                 [
                     "analyze",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -235,6 +239,8 @@ public async Task GlobalJsonCanBeAnalyzed()
             await RunAsync(path =>
                 [
                     "analyze",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -345,7 +351,7 @@ private static async Task RunAsync(
                     {
                         if (args[i] == "--job-path")
                         {
-                            var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
+                            var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", args[i + 1]);
                             experimentsManager = experimentsResult.ExperimentsManager;
                         }
                     }

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Discover.cs

@@ -1,5 +1,6 @@
 using System.Text;
 using System.Text.Json;
+using System.Text.Json.Serialization;
 
 using NuGetUpdater.Core;
 using NuGetUpdater.Core.Discover;
@@ -25,6 +26,8 @@ public async Task PathWithSpaces(bool useDirectDiscovery)
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -83,6 +86,8 @@ public async Task WithSolution(bool useDirectDiscovery)
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -178,6 +183,8 @@ public async Task WithProject(bool useDirectDiscovery)
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -251,6 +258,8 @@ public async Task WithDirectory(bool useDirectDiscovery)
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -321,6 +330,8 @@ public async Task WithDuplicateDependenciesOfDifferentTypes()
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -398,6 +409,8 @@ public async Task JobFileParseErrorIsReported_InvalidJson()
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     jobFilePath,
                     "--repo-root",
@@ -412,8 +425,7 @@ await RunAsync(path =>
                 {
                     Path = "/",
                     Projects = [],
-                    ErrorType = ErrorType.Unknown,
-                    ErrorDetailsPattern = "JsonException",
+                    ErrorRegex = "Error deserializing job file contents",
                 }
             );
         }
@@ -445,6 +457,8 @@ await File.WriteAllTextAsync(jobFilePath, """
             await RunAsync(path =>
                 [
                     "discover",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     jobFilePath,
                     "--repo-root",
@@ -459,8 +473,7 @@ await RunAsync(path =>
                 {
                     Path = "/",
                     Projects = [],
-                    ErrorType = ErrorType.BadRequirement,
-                    ErrorDetailsPattern = "not a valid requirement",
+                    Error = new Core.Run.ApiModel.BadRequirement("not a valid requirement"),
                 }
             );
         }
@@ -497,7 +510,7 @@ private static async Task RunAsync(
                         switch (args[i])
                         {
                             case "--job-path":
-                                var experimentsResult = await ExperimentsManager.FromJobFileAsync(args[i + 1]);
+                                var experimentsResult = await ExperimentsManager.FromJobFileAsync("TEST-JOB-ID", args[i + 1]);
                                 experimentsManager = experimentsResult.ExperimentsManager;
                                 break;
                             case "--output":
@@ -520,11 +533,38 @@ private static async Task RunAsync(
 
                 resultPath ??= Path.Join(path, DiscoveryWorker.DiscoveryResultFileName);
                 var resultJson = await File.ReadAllTextAsync(resultPath);
-                var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, DiscoveryWorker.SerializerOptions);
+                var serializerOptions = new JsonSerializerOptions()
+                {
+                    Converters = { new TestJobErrorBaseConverter() }
+                };
+                foreach (var converter in DiscoveryWorker.SerializerOptions.Converters)
+                {
+                    serializerOptions.Converters.Add(converter);
+                }
+                var resultObject = JsonSerializer.Deserialize<WorkspaceDiscoveryResult>(resultJson, serializerOptions);
                 return resultObject!;
             });
 
             ValidateWorkspaceResult(expectedResult, actualResult, experimentsManager);
         }
+
+        private class TestJobErrorBaseConverter : JsonConverter<Core.Run.ApiModel.JobErrorBase>
+        {
+            public override Core.Run.ApiModel.JobErrorBase? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+            {
+                var dict = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(ref reader, options)!;
+                return dict["error-type"].GetString() switch
+                {
+                    "illformed_requirement" => new Core.Run.ApiModel.BadRequirement(dict["error-details"].GetProperty("message").GetString()!),
+                    "unknown_error" => new Core.Run.ApiModel.UnknownError(new Exception("Error deserializing job file contents"), "TEST-JOB-ID"),
+                    _ => throw new NotImplementedException($"Unknown error type: {dict["error-type"]}"),
+                };
+            }
+
+            public override void Write(Utf8JsonWriter writer, Core.Run.ApiModel.JobErrorBase value, JsonSerializerOptions options)
+            {
+                throw new NotImplementedException();
+            }
+        }
     }
 }

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli.Test/EntryPointTests.Update.cs

@@ -19,6 +19,8 @@ public async Task WithSolution()
             await Run(path =>
                 [
                     "update",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -122,6 +124,8 @@ public async Task WithProject()
             await Run(path =>
                 [
                     "update",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -202,6 +206,8 @@ public async Task WithDirsProjAndDirectoryBuildPropsThatIsOutOfDirectoryButStill
             await Run(path =>
                 [
                     "update",
+                    "--job-id",
+                    "TEST-JOB-ID",
                     "--job-path",
                     Path.Combine(path, "job.json"),
                     "--repo-root",
@@ -361,6 +367,8 @@ await File.WriteAllTextAsync(projectPath, """
             IEnumerable<string> executableArgs = [
                 executableName,
                 "update",
+                "--job-id",
+                "TEST-JOB-ID",
                 "--job-path",
                 Path.Combine(tempDir.DirectoryPath, "job.json"),
                 "--repo-root",

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/AnalyzeCommand.cs

@@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands;
 
 internal static class AnalyzeCommand
 {
+    internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
     internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
     internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root") { IsRequired = true };
     internal static readonly Option<FileInfo> DependencyFilePathOption = new("--dependency-file-path") { IsRequired = true };
@@ -17,6 +18,7 @@ internal static Command GetCommand(Action<int> setExitCode)
     {
         Command command = new("analyze", "Determines how to update a dependency based on the workspace discovery information.")
         {
+            JobIdOption,
             JobPathOption,
             RepoRootOption,
             DependencyFilePathOption,
@@ -26,13 +28,13 @@ internal static Command GetCommand(Action<int> setExitCode)
 
         command.TreatUnmatchedTokensAsErrors = true;
 
-        command.SetHandler(async (jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) =>
+        command.SetHandler(async (jobId, jobPath, repoRoot, discoveryPath, dependencyPath, analysisDirectory) =>
         {
             var logger = new ConsoleLogger();
-            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
-            var worker = new AnalyzeWorker(experimentsManager, logger);
+            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
+            var worker = new AnalyzeWorker(jobId, experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, discoveryPath.FullName, dependencyPath.FullName, analysisDirectory.FullName);
-        }, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption);
+        }, JobIdOption, JobPathOption, RepoRootOption, DiscoveryFilePathOption, DependencyFilePathOption, AnalysisFolderOption);
 
         return command;
     }

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/DiscoverCommand.cs

@@ -7,6 +7,7 @@ namespace NuGetUpdater.Cli.Commands;
 
 internal static class DiscoverCommand
 {
+    internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
     internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
     internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root") { IsRequired = true };
     internal static readonly Option<string> WorkspaceOption = new("--workspace") { IsRequired = true };
@@ -16,6 +17,7 @@ internal static Command GetCommand(Action<int> setExitCode)
     {
         Command command = new("discover", "Generates a report of the workspace dependencies and where they are located.")
         {
+            JobIdOption,
             JobPathOption,
             RepoRootOption,
             WorkspaceOption,
@@ -24,27 +26,26 @@ internal static Command GetCommand(Action<int> setExitCode)
 
         command.TreatUnmatchedTokensAsErrors = true;
 
-        command.SetHandler(async (jobPath, repoRoot, workspace, outputPath) =>
+        command.SetHandler(async (jobId, jobPath, repoRoot, workspace, outputPath) =>
         {
-            var (experimentsManager, errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
-            if (errorResult is not null)
+            var (experimentsManager, error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
+            if (error is not null)
             {
                 // to make testing easier, this should be a `WorkspaceDiscoveryResult` object
                 var discoveryErrorResult = new WorkspaceDiscoveryResult
                 {
+                    Error = error,
                     Path = workspace,
                     Projects = [],
-                    ErrorType = errorResult.ErrorType,
-                    ErrorDetails = errorResult.ErrorDetails,
                 };
                 await DiscoveryWorker.WriteResultsAsync(repoRoot.FullName, outputPath.FullName, discoveryErrorResult);
                 return;
             }
 
             var logger = new ConsoleLogger();
-            var worker = new DiscoveryWorker(experimentsManager, logger);
+            var worker = new DiscoveryWorker(jobId, experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, workspace, outputPath.FullName);
-        }, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption);
+        }, JobIdOption, JobPathOption, RepoRootOption, WorkspaceOption, OutputOption);
 
         return command;
     }

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/RunCommand.cs

@@ -33,11 +33,11 @@ internal static Command GetCommand(Action<int> setExitCode)
         command.SetHandler(async (jobPath, repoContentsPath, apiUrl, jobId, outputPath, baseCommitSha) =>
         {
             var apiHandler = new HttpApiHandler(apiUrl.ToString(), jobId);
-            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
+            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
             var logger = new ConsoleLogger();
-            var discoverWorker = new DiscoveryWorker(experimentsManager, logger);
-            var analyzeWorker = new AnalyzeWorker(experimentsManager, logger);
-            var updateWorker = new UpdaterWorker(experimentsManager, logger);
+            var discoverWorker = new DiscoveryWorker(jobId, experimentsManager, logger);
+            var analyzeWorker = new AnalyzeWorker(jobId, experimentsManager, logger);
+            var updateWorker = new UpdaterWorker(jobId, experimentsManager, logger);
             var worker = new RunWorker(jobId, apiHandler, discoverWorker, analyzeWorker, updateWorker, logger);
             await worker.RunAsync(jobPath, repoContentsPath, baseCommitSha, outputPath);
         }, JobPathOption, RepoContentsPathOption, ApiUrlOption, JobIdOption, OutputPathOption, BaseCommitShaOption);

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Cli/Commands/UpdateCommand.cs

@@ -6,6 +6,7 @@ namespace NuGetUpdater.Cli.Commands;
 
 internal static class UpdateCommand
 {
+    internal static readonly Option<string> JobIdOption = new("--job-id") { IsRequired = true };
     internal static readonly Option<FileInfo> JobPathOption = new("--job-path") { IsRequired = true };
     internal static readonly Option<DirectoryInfo> RepoRootOption = new("--repo-root", () => new DirectoryInfo(Environment.CurrentDirectory)) { IsRequired = false };
     internal static readonly Option<FileInfo> SolutionOrProjectFileOption = new("--solution-or-project") { IsRequired = true };
@@ -19,6 +20,7 @@ internal static Command GetCommand(Action<int> setExitCode)
     {
         Command command = new("update", "Applies the changes from an analysis report to update a dependency.")
         {
+            JobIdOption,
             JobPathOption,
             RepoRootOption,
             SolutionOrProjectFileOption,
@@ -30,15 +32,25 @@ internal static Command GetCommand(Action<int> setExitCode)
         };
 
         command.TreatUnmatchedTokensAsErrors = true;
-
-        command.SetHandler(async (jobPath, repoRoot, solutionOrProjectFile, dependencyName, newVersion, previousVersion, isTransitive, resultOutputPath) =>
+        command.SetHandler(async (context) =>
         {
-            var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName);
+            // since we have more than 8 arguments, we have to pull them out manually
+            var jobId = context.ParseResult.GetValueForOption(JobIdOption)!;
+            var jobPath = context.ParseResult.GetValueForOption(JobPathOption)!;
+            var repoRoot = context.ParseResult.GetValueForOption(RepoRootOption)!;
+            var solutionOrProjectFile = context.ParseResult.GetValueForOption(SolutionOrProjectFileOption)!;
+            var dependencyName = context.ParseResult.GetValueForOption(DependencyNameOption)!;
+            var newVersion = context.ParseResult.GetValueForOption(NewVersionOption)!;
+            var previousVersion = context.ParseResult.GetValueForOption(PreviousVersionOption)!;
+            var isTransitive = context.ParseResult.GetValueForOption(IsTransitiveOption);
+            var resultOutputPath = context.ParseResult.GetValueForOption(ResultOutputPathOption);
+
+            var (experimentsManager, _error) = await ExperimentsManager.FromJobFileAsync(jobId, jobPath.FullName);
             var logger = new ConsoleLogger();
-            var worker = new UpdaterWorker(experimentsManager, logger);
+            var worker = new UpdaterWorker(jobId, experimentsManager, logger);
             await worker.RunAsync(repoRoot.FullName, solutionOrProjectFile.FullName, dependencyName, previousVersion, newVersion, isTransitive, resultOutputPath);
             setExitCode(0);
-        }, JobPathOption, RepoRootOption, SolutionOrProjectFileOption, DependencyNameOption, NewVersionOption, PreviousVersionOption, IsTransitiveOption, ResultOutputPathOption);
+        });
 
         return command;
     }