diff --git a/src/Refitter.Core/IdentifierUtils.cs b/src/Refitter.Core/IdentifierUtils.cs index a774f8db..40da096b 100644 --- a/src/Refitter.Core/IdentifierUtils.cs +++ b/src/Refitter.Core/IdentifierUtils.cs @@ -41,7 +41,10 @@ public static string Sanitize(this string value) { const char dash = '-'; - // @ can be used and still make valid methode names. but this should make most use cases safe + if (string.IsNullOrEmpty(value)) + return value; + + // @ can be used and still make valid method names. but this should make most use cases safe if ( (value.First() < 'A' || value.First() > 'Z') && (value.First() < 'a' || value.First() > 'z') && diff --git a/src/Refitter.Core/OperationNameGenerator.cs b/src/Refitter.Core/OperationNameGenerator.cs index b622afda..6e540d25 100644 --- a/src/Refitter.Core/OperationNameGenerator.cs +++ b/src/Refitter.Core/OperationNameGenerator.cs @@ -50,7 +50,7 @@ public OperationNameGenerator(OpenApiDocument document, RefitGeneratorSettings s } [ExcludeFromCodeCoverage] - public bool SupportsMultipleClients => throw new System.NotImplementedException(); + public bool SupportsMultipleClients => defaultGenerator.SupportsMultipleClients; [ExcludeFromCodeCoverage] public string GetClientName(OpenApiDocument document, string path, string httpMethod, OpenApiOperation operation) diff --git a/src/Refitter.Core/RefitGenerator.cs b/src/Refitter.Core/RefitGenerator.cs index 11687a4b..1ddd3a0d 100644 --- a/src/Refitter.Core/RefitGenerator.cs +++ b/src/Refitter.Core/RefitGenerator.cs @@ -114,6 +114,19 @@ public string Generate() var factory = new CSharpClientGeneratorFactory(settings, document); var generator = factory.Create(); var docGenerator = new XmlDocumentationGenerator(settings); + + // Create the interface generator before calling GenerateFile() so that + // OperationNameGenerator.CheckForDuplicateOperationIds() sees the original + // (pre-generation) operation IDs. GenerateFile() auto-populates operation IDs + // with globally unique names which would prevent the switch to the path segments + // generator, causing unnecessary numeric suffixes in ByTag mode. + IRefitInterfaceGenerator interfaceGenerator = settings.MultipleInterfaces switch + { + MultipleInterfaces.ByEndpoint => new RefitMultipleInterfaceGenerator(settings, document, generator, docGenerator), + MultipleInterfaces.ByTag => new RefitMultipleInterfaceByTagGenerator(settings, document, generator, docGenerator), + _ => new RefitInterfaceGenerator(settings, document, generator, docGenerator), + }; + var contracts = generator.GenerateFile(); contracts = SanitizeGeneratedContracts(contracts); @@ -126,13 +139,6 @@ public string Generate() (current, import) => current.Replace($"{import}.", string.Empty)); } - IRefitInterfaceGenerator interfaceGenerator = settings.MultipleInterfaces switch - { - MultipleInterfaces.ByEndpoint => new RefitMultipleInterfaceGenerator(settings, document, generator, docGenerator), - MultipleInterfaces.ByTag => new RefitMultipleInterfaceByTagGenerator(settings, document, generator, docGenerator), - _ => new RefitInterfaceGenerator(settings, document, generator, docGenerator), - }; - var refitInterfaces = GenerateClient(interfaceGenerator); var interfaceNames = refitInterfaces.Select(c => c.TypeName).ToArray(); var refitInterfacesCode = string.Join("", refitInterfaces.Select(c => c.Content)); @@ -160,9 +166,12 @@ public GeneratorOutput GenerateMultipleFiles() var factory = new CSharpClientGeneratorFactory(settings, document); var generator = factory.Create(); var docGenerator = new XmlDocumentationGenerator(settings); - var contracts = generator.GenerateFile(); - contracts = SanitizeGeneratedContracts(contracts); + // Create the interface generator before calling GenerateFile() so that + // OperationNameGenerator.CheckForDuplicateOperationIds() sees the original + // (pre-generation) operation IDs. GenerateFile() auto-populates operation IDs + // with globally unique names which would prevent the switch to the path segments + // generator, causing unnecessary numeric suffixes in ByTag mode. IRefitInterfaceGenerator interfaceGenerator = settings.MultipleInterfaces switch { MultipleInterfaces.ByEndpoint @@ -172,6 +181,9 @@ public GeneratorOutput GenerateMultipleFiles() _ => new RefitInterfaceGenerator(settings, document, generator, docGenerator), }; + var contracts = generator.GenerateFile(); + contracts = SanitizeGeneratedContracts(contracts); + var generatedFiles = new List(); var refitInterfaces = GenerateClient(interfaceGenerator); diff --git a/src/Refitter.Core/RefitInterfaceGenerator.cs b/src/Refitter.Core/RefitInterfaceGenerator.cs index 68cb74d0..3921dcc6 100644 --- a/src/Refitter.Core/RefitInterfaceGenerator.cs +++ b/src/Refitter.Core/RefitInterfaceGenerator.cs @@ -45,6 +45,7 @@ private string GenerateInterfaceBody(out string? dynamicQuerystringParameters) { var code = new StringBuilder(); var dynamicQuerystringParametersCodeBuilder = new StringBuilder(); + var knownIdentifiers = new HashSet(); foreach (var kv in document.Paths) { foreach (var operations in kv.Value) @@ -58,7 +59,8 @@ private string GenerateInterfaceBody(out string? dynamicQuerystringParameters) var returnType = GetTypeName(operation); var verb = operations.Key.CapitalizeFirstCharacter(); - var operationName = GenerateOperationName(kv.Key, verb, operation); + var operationName = IdentifierUtils.Counted(knownIdentifiers, GenerateOperationName(kv.Key, verb, operation)); + knownIdentifiers.Add(operationName); var dynamicQuerystringParameterType = operationName + "QueryParams"; var operationModel = generator.CreateOperationModel(operation); diff --git a/src/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cs b/src/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cs index b3b2afae..dce1e787 100644 --- a/src/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cs +++ b/src/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cs @@ -45,7 +45,7 @@ public override IEnumerable GenerateCode() var returnType = GetTypeName(operation); var verb = operations.Key.CapitalizeFirstCharacter(); - string interfaceName = null!; + string interfaceName; if (!interfacesByGroup.TryGetValue(kv.Key, out var sb)) { interfacesByGroup[kv.Key] = sb = new StringBuilder(); @@ -59,6 +59,10 @@ public override IEnumerable GenerateCode() interfacesNamesByGroup[kv.Key] = interfaceName; } + else + { + interfaceName = interfacesNamesByGroup[kv.Key]; + } var operationName = GetOperationName(interfaceName, op.PathItem.Key, operations.Key, operation); var dynamicQuerystringParameterType = operationName + "QueryParams"; diff --git a/src/Refitter.Tests/Examples/MultipleInterfacesByTagsWithSamePathSegmentTests.cs b/src/Refitter.Tests/Examples/MultipleInterfacesByTagsWithSamePathSegmentTests.cs new file mode 100644 index 00000000..c5e02174 --- /dev/null +++ b/src/Refitter.Tests/Examples/MultipleInterfacesByTagsWithSamePathSegmentTests.cs @@ -0,0 +1,125 @@ +using FluentAssertions; +using Refitter.Core; +using Refitter.Tests.Build; +using Refitter.Tests.TestUtilities; +using TUnit.Core; + +namespace Refitter.Tests.Examples; + +/// +/// Regression tests for https://github.com/christianhelle/refitter/issues/361 +/// When using ByTag, method names should not have numeric suffixes added for +/// cross-interface uniqueness - deduplication should be per-interface. +/// +public class MultipleInterfacesByTagsWithSamePathSegmentTests +{ + private const string OpenApiSpec = @" +openapi: '3.0.0' +info: + title: 'Test API' + version: '1.0' +paths: + /api/manage/2fa: + post: + tags: + - 'Manage' + responses: + '200': + description: 'ok' + /api/manage/info: + get: + tags: + - 'Manage' + responses: + '200': + description: 'ok' + post: + tags: + - 'Manage' + responses: + '200': + description: 'ok' + /api/account/info: + get: + tags: + - 'Account' + responses: + '200': + description: 'ok' + /api/user/info: + get: + tags: + - 'User' + responses: + '200': + description: 'ok' +"; + + [Test] + public async Task Can_Generate_Code() + { + string generatedCode = await GenerateCode(); + generatedCode.Should().NotBeNullOrWhiteSpace(); + } + + [Test] + public async Task Generates_Manage_Interface() + { + string generatedCode = await GenerateCode(); + generatedCode.Should().Contain("partial interface IManageApi"); + } + + [Test] + public async Task Generates_Account_Interface() + { + string generatedCode = await GenerateCode(); + generatedCode.Should().Contain("partial interface IAccountApi"); + } + + [Test] + public async Task Generates_User_Interface() + { + string generatedCode = await GenerateCode(); + generatedCode.Should().Contain("partial interface IUserApi"); + } + + [Test] + public async Task Account_Interface_Does_Not_Have_Numeric_Suffix() + { + string generatedCode = await GenerateCode(); + // IAccountApi's GET /api/account/info should not get a numeric suffix + // even though IManageApi also has a GET /api/manage/info operation + generatedCode.Should().NotContainAny("Info2(", "Info3(", "InfoGet2(", "InfoGet3(", "InfoGET2(", "InfoGET3("); + } + + [Test] + public async Task User_Interface_Does_Not_Have_Numeric_Suffix() + { + string generatedCode = await GenerateCode(); + // IUserApi's GET /api/user/info should not get a numeric suffix + generatedCode.Should().NotContainAny("Info2(", "Info3(", "InfoGet2(", "InfoGet3(", "InfoGET2(", "InfoGET3("); + } + + [Test] + public async Task Can_Build_Generated_Code() + { + string generatedCode = await GenerateCode(); + BuildHelper + .BuildCSharp(generatedCode) + .Should() + .BeTrue(); + } + + private static async Task GenerateCode() + { + var swaggerFile = await SwaggerFileHelper.CreateSwaggerFile(OpenApiSpec); + var settings = new RefitGeneratorSettings + { + OpenApiPath = swaggerFile, + MultipleInterfaces = MultipleInterfaces.ByTag, + }; + + var sut = await RefitGenerator.CreateAsync(settings); + return sut.Generate(); + } +}