From 9a5bf6afe2469717274582d7af0ad4e4f88453c1 Mon Sep 17 00:00:00 2001 From: Doug Bunting <6431421+dougbu@users.noreply.github.com> Date: Fri, 3 Sep 2021 17:57:34 -0700 Subject: [PATCH 1/5] Handle shortened JSON file in `dotnet openapi` - #35767 - an existing JSON file must be truncated --- src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs b/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs index 25e48d17990f..a3333d98957b 100644 --- a/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs +++ b/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs @@ -572,7 +572,7 @@ 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) { From 47bf0fa8f57eea7610a55fb7845a8996ba49f7a4 Mon Sep 17 00:00:00 2001 From: Doug Bunting <6431421+dougbu@users.noreply.github.com> Date: Fri, 3 Sep 2021 21:00:40 -0700 Subject: [PATCH 2/5] !fixup! Address nits in changed file - take VS suggestions --- .../src/Commands/BaseCommand.cs | 64 ++++++++----------- 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs b/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs index a3333d98957b..fea939677f63 100644 --- a/src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs +++ b/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 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 DownloadGivenOption(string url, CommandOption fileOp /// private static async Task RetryRequest( Func> 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 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 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> LoadPackageVersionsFromURLAsync( private static IDictionary 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(); @@ -513,10 +500,8 @@ private static IDictionary 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) @@ -578,6 +563,7 @@ private async Task WriteToFileAsync(Stream content, string destinationPath, bool { content.Seek(0, SeekOrigin.Begin); } + await content.CopyToAsync(fileStream); } catch (Exception ex) From 3efb9271dea46d834b841059fc6c311d0ef57f02 Mon Sep 17 00:00:00 2001 From: Doug Bunting <6431421+dougbu@users.noreply.github.com> Date: Fri, 3 Sep 2021 19:27:03 -0700 Subject: [PATCH 3/5] Stop skipping `dotnet openapi` - .NET SDKs and Visual Studio's `msbuild` avoid #32686 --- .../test/OpenApiAddFileTests.cs | 26 +++++++---------- .../test/OpenApiAddURLTests.cs | 29 ++++++++----------- .../test/OpenApiRefreshTests.cs | 7 +---- .../test/OpenApiRemoveTests.cs | 10 ++----- 4 files changed, 27 insertions(+), 45 deletions(-) diff --git a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs index 4898225e3144..23ca88e31514 100644 --- a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs +++ b/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,7 +12,7 @@ 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(); @@ -27,7 +23,7 @@ public void OpenApi_Empty_ShowsHelp() 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($"", 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($" Date: Fri, 3 Sep 2021 21:07:33 -0700 Subject: [PATCH 4/5] !fixup! Address nits in `dotnet openapi` test files - take VS suggestions --- .../test/OpenApiAddFileTests.cs | 2 +- .../test/OpenApiTestBase.cs | 29 ++++++------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs index 23ca88e31514..30a147cd0ac4 100644 --- a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs +++ b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs @@ -16,7 +16,7 @@ public OpenApiAddFileTests(ITestOutputHelper output) : base(output) { } public void OpenApi_Empty_ShowsHelp() { var app = GetApplication(); - var run = app.Execute(new string[] { }); + var run = app.Execute(Array.Empty()); AssertNoErrors(run); diff --git a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiTestBase.cs b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiTestBase.cs index 8aaefa51b93f..bc7409f2cd73 100644 --- a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiTestBase.cs +++ b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiTestBase.cs @@ -1,17 +1,12 @@ // 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.IO; using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Text; -using System.Threading.Tasks; using Microsoft.DotNet.Openapi.Tools; using Microsoft.Extensions.Tools.Internal; -using Xunit; using Xunit.Abstractions; namespace Microsoft.DotNet.OpenApi.Tests @@ -92,7 +87,7 @@ internal Application GetApplication(bool realHttp = false) _tempDir.Root, wrapper, _output, _error); } - private IDictionary> DownloadMock() + private static IDictionary> DownloadMock() { var noExtension = new ContentDispositionHeaderValue("attachment"); noExtension.Parameters.Add(new NameValueHeaderValue("filename", "filename")); @@ -113,7 +108,7 @@ private IDictionary> Downlo protected void AssertNoErrors(int appExitCode) { - Assert.True(string.IsNullOrEmpty(_error.ToString()), $"Threw error: {_error.ToString()}"); + Assert.True(string.IsNullOrEmpty(_error.ToString()), $"Threw error: {_error}"); Assert.Equal(0, appExitCode); } @@ -124,7 +119,7 @@ public void Dispose() } } - public class TestHttpClientWrapper : IHttpClientWrapper + public sealed class TestHttpClientWrapper : IHttpClientWrapper { private readonly IDictionary> _results; @@ -143,7 +138,7 @@ public Task GetResponseAsync(string url) MemoryStream stream = null; if(result != null) { - byte[] byteArray = Encoding.ASCII.GetBytes(result.Item1); + var byteArray = Encoding.ASCII.GetBytes(result.Item1); stream = new MemoryStream(byteArray); } @@ -151,7 +146,7 @@ public Task GetResponseAsync(string url) } } - public class TestHttpResponseMessageWrapper : IHttpResponseMessageWrapper + public sealed class TestHttpResponseMessageWrapper : IHttpResponseMessageWrapper { public Task Stream { get; } @@ -159,17 +154,11 @@ public class TestHttpResponseMessageWrapper : IHttpResponseMessageWrapper public bool IsSuccessCode() { - switch(StatusCode) + return StatusCode switch { - case HttpStatusCode.OK: - case HttpStatusCode.Created: - case HttpStatusCode.NoContent: - case HttpStatusCode.Accepted: - return true; - case HttpStatusCode.NotFound: - default: - return false; - } + HttpStatusCode.OK or HttpStatusCode.Created or HttpStatusCode.NoContent or HttpStatusCode.Accepted => true, + _ => false, + }; } private readonly ContentDispositionHeaderValue _contentDisposition; From 33e855bfc3d1577cc1f889c2c617b8df3303bb54 Mon Sep 17 00:00:00 2001 From: Doug Bunting <6431421+dougbu@users.noreply.github.com> Date: Fri, 3 Sep 2021 21:08:36 -0700 Subject: [PATCH 5/5] Extend `dotnet openapi refresh` tests - include regression test for #35767 --- .../test/OpenApiRefreshTests.cs | 65 +++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiRefreshTests.cs b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiRefreshTests.cs index 06cb8e9d8d08..4a6faf6c3974 100644 --- a/src/Tools/Microsoft.dotnet-openapi/test/OpenApiRefreshTests.cs +++ b/src/Tools/Microsoft.dotnet-openapi/test/OpenApiRefreshTests.cs @@ -15,19 +15,19 @@ public async Task OpenApi_Refresh_Basic() { CreateBasicProject(withOpenApi: false); + // Add to the project. Ignore initial filename.json content. var app = GetApplication(); var run = app.Execute(new[] { "add", "url", FakeOpenApiUrl }); AssertNoErrors(run); + // File will grow after the refresh. var expectedJsonPath = Path.Combine(_tempDir.Root, "filename.json"); - var json = await File.ReadAllTextAsync(expectedJsonPath); - json += "trash"; - await File.WriteAllTextAsync(expectedJsonPath, json); + await File.WriteAllTextAsync(expectedJsonPath, "trash"); var firstWriteTime = File.GetLastWriteTime(expectedJsonPath); - Thread.Sleep(TimeSpan.FromSeconds(1)); + await Task.Delay(TimeSpan.FromSeconds(1)); app = GetApplication(); run = app.Execute(new[] { "refresh", FakeOpenApiUrl }); @@ -36,6 +36,63 @@ public async Task OpenApi_Refresh_Basic() var secondWriteTime = File.GetLastWriteTime(expectedJsonPath); Assert.True(firstWriteTime < secondWriteTime, $"File wasn't updated! {firstWriteTime} {secondWriteTime}"); + Assert.Equal(Content, await File.ReadAllTextAsync(expectedJsonPath), ignoreLineEndingDifferences: true); + } + + // Regression test for #35767 scenario. + [Fact] + public async Task OpenApi_Refresh_MuchShorterFile() + { + CreateBasicProject(withOpenApi: false); + + // Add to the project. Ignore initial filename.json content. + var app = GetApplication(); + var run = app.Execute(new[] { "add", "url", FakeOpenApiUrl }); + + AssertNoErrors(run); + + // File will shrink after the refresh. + var expectedJsonPath = Path.Combine(_tempDir.Root, "filename.json"); + await File.WriteAllTextAsync(expectedJsonPath, PackageUrlContent); + + var firstWriteTime = File.GetLastWriteTime(expectedJsonPath); + + await Task.Delay(TimeSpan.FromSeconds(1)); + + app = GetApplication(); + run = app.Execute(new[] { "refresh", FakeOpenApiUrl }); + + AssertNoErrors(run); + + var secondWriteTime = File.GetLastWriteTime(expectedJsonPath); + Assert.True(firstWriteTime < secondWriteTime, $"File wasn't updated! {firstWriteTime} {secondWriteTime}"); + Assert.Equal(Content, await File.ReadAllTextAsync(expectedJsonPath), ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task OpenApi_Refresh_UnchangedFile() + { + CreateBasicProject(withOpenApi: false); + + // Add to the project and write the filename.json file. + var app = GetApplication(); + var run = app.Execute(new[] { "add", "url", FakeOpenApiUrl }); + + AssertNoErrors(run); + + var expectedJsonPath = Path.Combine(_tempDir.Root, "filename.json"); + var firstWriteTime = File.GetLastWriteTime(expectedJsonPath); + + await Task.Delay(TimeSpan.FromSeconds(1)); + + app = GetApplication(); + run = app.Execute(new[] { "refresh", FakeOpenApiUrl }); + + AssertNoErrors(run); + + var secondWriteTime = File.GetLastWriteTime(expectedJsonPath); + Assert.Equal(firstWriteTime, secondWriteTime); + Assert.Equal(Content, await File.ReadAllTextAsync(expectedJsonPath)); } } }