Skip to content

Handle shortened JSON file in dotnet openapi #36171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 10, 2021
Merged

Handle shortened JSON file in dotnet openapi #36171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9a5bf6a
Handle shortened JSON file in `dotnet openapi`
dougbu Sep 4, 2021
47bf0fa
!fixup! Address nits in changed file
dougbu Sep 4, 2021
3efb927
Stop skipping `dotnet openapi`
dougbu Sep 4, 2021
bac9c02
!fixup! Address nits in `dotnet openapi` test files
dougbu Sep 4, 2021
33e855b
Extend `dotnet openapi refresh` tests
dougbu Sep 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 26 additions & 40 deletions src/Tools/Microsoft.dotnet-openapi/src/Commands/BaseCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit odd. The method is only ever called w/ a single argument.

{
for (var retry = 0; retry < retryCount; retry++)
{
Expand Down Expand Up @@ -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.
}
}
}
Expand All @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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())
Expand All @@ -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())
{
Expand Down Expand Up @@ -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>();

Expand All @@ -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)
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most important parts of the PR are this and changes in src/Tools/Microsoft.dotnet-openapi/test/OpenApiRefreshTests.cs

fileStream.Seek(0, SeekOrigin.Begin);
if (content.CanSeek)
{
content.Seek(0, SeekOrigin.Begin);
}

await content.CopyToAsync(fileStream);
}
catch (Exception ex)
Expand Down
28 changes: 12 additions & 16 deletions src/Tools/Microsoft.dotnet-openapi/test/OpenApiAddFileTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -16,18 +12,18 @@ public class OpenApiAddFileTests : OpenApiTestBase
{
public OpenApiAddFileTests(ITestOutputHelper output) : base(output) { }

[Fact(Skip = "https://github.com/dotnet/aspnetcore/issues/32686")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will close this issue if tests run smoothly on CI. They're fine in VS locally 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good news…

[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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
Loading