Handle shortened JSON file in dotnet openapi (#36171)

* Handle shortened JSON file in `dotnet openapi`
  - #35767
  - an existing JSON file must be truncated
* !fixup! Address nits in changed file
  - take VS suggestions
* Stop skipping `dotnet openapi` tests
  - .NET SDKs and Visual Studio's `msbuild` avoid #32686 already
* !fixup! Address nits in `dotnet openapi` test files
  - take VS suggestions
* Extend `dotnet openapi refresh` tests
  - include regression test for #35767

Author: dougbu

src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs

@@ -1,18 +1,13 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
-using System.Collections.Generic;
 using System.Diagnostics;
-using System.IO;
 using System.Linq;
 using System.Net;
 using System.Net.Http;
 using System.Reflection;
 using System.Security.Cryptography;
 using System.Text.Json;
-using System.Threading;
-using System.Threading.Tasks;
 using Microsoft.Build.Evaluation;
 using Microsoft.DotNet.Openapi.Tools;
 using Microsoft.DotNet.Openapi.Tools.Internal;
@@ -46,9 +41,9 @@ public BaseCommand(CommandLineApplication parent, string name, IHttpClientWrappe
 
             ProjectFileOption = Option("-p|--updateProject", "The project file update.", CommandOptionType.SingleValue);
 
-            if (Parent is Application)
+            if (Parent is Application application)
             {
-                WorkingDirectory = ((Application)Parent).WorkingDirectory;
+                WorkingDirectory = application.WorkingDirectory;
             }
             else
             {
@@ -89,10 +84,11 @@ private async Task<int> ExecuteAsync()
         private Application GetApplication()
         {
             var parent = Parent;
-            while(!(parent is Application))
+            while(parent is not Application)
             {
                 parent = parent.Parent;
             }
+
             return (Application)parent;
         }
 
@@ -126,7 +122,7 @@ internal FileInfo ResolveProjectFile(CommandOption projectOption)
             return new FileInfo(project);
         }
 
-        protected Project LoadProject(FileInfo projectFile)
+        protected static Project LoadProject(FileInfo projectFile)
         {
             var project = ProjectCollection.GlobalProjectCollection.LoadProject(
                 projectFile.FullName,
@@ -136,12 +132,12 @@ protected Project LoadProject(FileInfo projectFile)
             return project;
         }
 
-        internal bool IsProjectFile(string file)
+        internal static bool IsProjectFile(string file)
         {
             return File.Exists(Path.GetFullPath(file)) && file.EndsWith(".csproj", StringComparison.Ordinal);
         }
 
-        internal bool IsUrl(string file)
+        internal static bool IsUrl(string file)
         {
             return Uri.TryCreate(file, UriKind.Absolute, out var _) && file.StartsWith("http", StringComparison.Ordinal);
         }
@@ -167,7 +163,8 @@ internal async Task AddOpenAPIReference(
 
             if (sourceUrl != null)
             {
-                if (items.Any(i => string.Equals(i.GetMetadataValue(SourceUrlAttrName), sourceUrl)))
+                if (items.Any(
+                    i => string.Equals(i.GetMetadataValue(SourceUrlAttrName), sourceUrl, StringComparison.Ordinal)))
                 {
                     Warning.Write($"A reference to '{sourceUrl}' already exists in '{project.FullPath}'.");
                     return;
@@ -299,8 +296,8 @@ internal async Task<string> DownloadGivenOption(string url, CommandOption fileOp
         /// <param name="retryCount"></param>
         private static async Task<IHttpResponseMessageWrapper> RetryRequest(
             Func<Task<IHttpResponseMessageWrapper>> retryBlock,
-            CancellationToken cancellationToken = default,
-            int retryCount = 60)
+            int retryCount = 60,
+            CancellationToken cancellationToken = default)
         {
             for (var retry = 0; retry < retryCount; retry++)
             {
@@ -331,7 +328,7 @@ private static async Task<IHttpResponseMessageWrapper> RetryRequest(
                     {
                         if (exception is HttpRequestException || exception is WebException)
                         {
-                            await Task.Delay(1 * 1000); //Wait for a while before retry.
+                            await Task.Delay(1 * 1000, cancellationToken); // Wait for a while before retry.
                         }
                     }
                 }
@@ -340,7 +337,7 @@ private static async Task<IHttpResponseMessageWrapper> RetryRequest(
             throw new OperationCanceledException("Failed to connect, retry limit exceeded.");
         }
 
-        private string GetUniqueFileName(string directory, string fileName, string extension)
+        private static string GetUniqueFileName(string directory, string fileName, string extension)
         {
             var uniqueName = fileName;
 
@@ -366,7 +363,7 @@ private string GetUniqueFileName(string directory, string fileName, string exten
             return uniqueName + extension;
         }
 
-        private string GetFileNameFromResponse(IHttpResponseMessageWrapper response, string url)
+        private static string GetFileNameFromResponse(IHttpResponseMessageWrapper response, string url)
         {
             var contentDisposition = response.ContentDisposition();
             string result;
@@ -396,22 +393,12 @@ private string GetFileNameFromResponse(IHttpResponseMessageWrapper response, str
                 else
                 {
                     var parts = uri.Host.Split('.');
-
-                    // There's no segment, use the domain name.
-                    string domain;
-                    switch (parts.Length)
+                    var domain = parts.Length switch
                     {
-                        case 1:
-                        case 2:
-                            // It's localhost if 1, no www if 2
-                            domain = parts.First();
-                            break;
-                        case 3:
-                            domain = parts[1];
-                            break;
-                        default:
-                            throw new NotImplementedException("We don't handle the case that the Host has more than three segments");
-                    }
+                        1 or 2 => parts.First(), // It's localhost or somewhere in an Intranet if 1; no www if 2.
+                        3 => parts[1],           // Grab XYZ in www.XYZ.domain.com or similar.
+                        _ => throw new NotImplementedException("We don't handle the case that the Host has more than three segments"),
+                    };
 
                     result = domain + DefaultExtension;
                 }
@@ -420,7 +407,7 @@ private string GetFileNameFromResponse(IHttpResponseMessageWrapper response, str
             return result;
         }
 
-        internal CodeGenerator? GetCodeGenerator(CommandOption codeGeneratorOption)
+        internal static CodeGenerator? GetCodeGenerator(CommandOption codeGeneratorOption)
         {
             CodeGenerator? codeGenerator;
             if (codeGeneratorOption.HasValue())
@@ -435,7 +422,7 @@ private string GetFileNameFromResponse(IHttpResponseMessageWrapper response, str
             return codeGenerator;
         }
 
-        internal void ValidateCodeGenerator(CommandOption codeGeneratorOption)
+        internal static void ValidateCodeGenerator(CommandOption codeGeneratorOption)
         {
             if (codeGeneratorOption.HasValue())
             {
@@ -494,7 +481,7 @@ private async Task<IDictionary<string, string>> LoadPackageVersionsFromURLAsync(
 
         private static IDictionary<string, string> GetServicePackages(CodeGenerator? type)
         {
-            CodeGenerator generator = type ?? CodeGenerator.NSwagCSharp;
+            var generator = type ?? CodeGenerator.NSwagCSharp;
             var name = Enum.GetName(typeof(CodeGenerator), generator);
             var attributes = typeof(Program).Assembly.GetCustomAttributes<OpenApiDependencyAttribute>();
 
@@ -513,10 +500,8 @@ private static IDictionary<string, string> GetServicePackages(CodeGenerator? typ
 
         private static byte[] GetHash(Stream stream)
         {
-            using (var algorithm = SHA256.Create())
-            {
-                return algorithm.ComputeHash(stream);
-            }
+            using var algorithm = SHA256.Create();
+            return algorithm.ComputeHash(stream);
         }
 
         private async Task WriteToFileAsync(Stream content, string destinationPath, bool overwrite)
@@ -572,12 +557,13 @@ private async Task WriteToFileAsync(Stream content, string destinationPath, bool
 
                 // Create or overwrite the destination file.
                 reachedCopy = true;
-                using var fileStream = new FileStream(destinationPath, FileMode.OpenOrCreate, FileAccess.Write);
+                using var fileStream = new FileStream(destinationPath, FileMode.Create, FileAccess.Write);
                 fileStream.Seek(0, SeekOrigin.Begin);
                 if (content.CanSeek)
                 {
                     content.Seek(0, SeekOrigin.Begin);
                 }
+
                 await content.CopyToAsync(fileStream);
             }
             catch (Exception ex)

src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs

@@ -1,13 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.IO;
 using System.Text.RegularExpressions;
-using System.Threading.Tasks;
 using System.Xml;
-using Microsoft.AspNetCore.Testing;
 using Microsoft.DotNet.OpenApi.Tests;
-using Xunit;
 using Xunit.Abstractions;
 
 namespace Microsoft.DotNet.OpenApi.Add.Tests
@@ -16,18 +12,18 @@ public class OpenApiAddFileTests : OpenApiTestBase
     {
         public OpenApiAddFileTests(ITestOutputHelper output) : base(output) { }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_Empty_ShowsHelp()
         {
             var app = GetApplication();
-            var run = app.Execute(new string[] { });
+            var run = app.Execute(Array.Empty<string>());
 
             AssertNoErrors(run);
 
             Assert.Contains("Usage: openapi ", _output.ToString());
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_NoProjectExists()
         {
             var app = GetApplication();
@@ -38,7 +34,7 @@ public void OpenApi_NoProjectExists()
             Assert.Equal(1, run);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_ExplicitProject_Missing()
         {
             var app = GetApplication();
@@ -50,7 +46,7 @@ public void OpenApi_ExplicitProject_Missing()
             Assert.Equal(1, run);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_Add_Empty_ShowsHelp()
         {
             var app = GetApplication();
@@ -61,7 +57,7 @@ public void OpenApi_Add_Empty_ShowsHelp()
             Assert.Contains("Usage: openapi add", _output.ToString());
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_Add_File_Empty_ShowsHelp()
         {
             var app = GetApplication();
@@ -72,7 +68,7 @@ public void OpenApi_Add_File_Empty_ShowsHelp()
             Assert.Contains("Usage: openapi ", _output.ToString());
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public async Task OpenApi_Add_ReuseItemGroup()
         {
             var project = CreateBasicProject(withOpenApi: true);
@@ -101,7 +97,7 @@ public async Task OpenApi_Add_ReuseItemGroup()
             Assert.Same(openApiRefs[0].ParentNode, openApiRefs[1].ParentNode);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public void OpenApi_Add_File_EquivilentPaths()
         {
             var project = CreateBasicProject(withOpenApi: true);
@@ -126,7 +122,7 @@ public void OpenApi_Add_File_EquivilentPaths()
             Assert.Single(openApiRefs);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public async Task OpenApi_Add_NSwagTypeScript()
         {
             var project = CreateBasicProject(withOpenApi: true);
@@ -146,7 +142,7 @@ public async Task OpenApi_Add_NSwagTypeScript()
             Assert.Contains($"<OpenApiReference Include=\"{nswagJsonFile}\" CodeGenerator=\"NSwagTypeScript\" />", content);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public async Task OpenApi_Add_FromJson()
         {
             var project = CreateBasicProject(withOpenApi: true);
@@ -166,7 +162,7 @@ public async Task OpenApi_Add_FromJson()
             Assert.Contains($"<OpenApiReference Include=\"{nswagJsonFile}\"", content);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public async Task OpenApi_Add_File_UseProjectOption()
         {
             var project = CreateBasicProject(withOpenApi: true);
@@ -186,7 +182,7 @@ public async Task OpenApi_Add_File_UseProjectOption()
             Assert.Contains($"<OpenApiReference Include=\"{nswagJsonFIle}\"", content);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
+        [Fact]
         public async Task OpenApi_Add_MultipleTimes_OnlyOneReference()
         {
             var project = CreateBasicProject(withOpenApi: true);