diff --git a/src/Elastic.ApiExplorer/Navigation/ApiNavigationBuilder.cs b/src/Elastic.ApiExplorer/Navigation/ApiNavigationBuilder.cs index 98b4dd08a3..ea662b333f 100644 --- a/src/Elastic.ApiExplorer/Navigation/ApiNavigationBuilder.cs +++ b/src/Elastic.ApiExplorer/Navigation/ApiNavigationBuilder.cs @@ -394,6 +394,7 @@ private string ResolveTagClassification(string? tag, XTagGroups? xTagGroups, Has return UnknownTagGroupName; } + // The raw operationId stays the grouping key: humanized titles of distinct ids (get-script, get_script) can collide. private static string? NullIfBlank(string? value) => string.IsNullOrWhiteSpace(value) ? null : value; private static List GetOrderedClassificationKeys(XTagGroups xTagGroups, HashSet presentClassifications) diff --git a/src/Elastic.ApiExplorer/Navigation/MarkdownNavigationTitleReader.cs b/src/Elastic.ApiExplorer/Navigation/MarkdownNavigationTitleReader.cs index 15094962d2..46def3ea33 100644 --- a/src/Elastic.ApiExplorer/Navigation/MarkdownNavigationTitleReader.cs +++ b/src/Elastic.ApiExplorer/Navigation/MarkdownNavigationTitleReader.cs @@ -5,6 +5,7 @@ using System.IO.Abstractions; using Elastic.ApiExplorer.Model; using Elastic.ApiExplorer.Operations; +using Elastic.Documentation.Extensions; using YamlDotNet.RepresentationModel; namespace Elastic.ApiExplorer.Navigation; @@ -57,12 +58,5 @@ public static string GetNavigationTitle(IFileSystem readFileSystem, IFileInfo ma } /// Converts kebab-case/snake_case file names to a title-cased navigation title. - internal static string TitleCaseFromFileName(string fileName) => - Path - .GetFileNameWithoutExtension(fileName) - .Replace('-', ' ') - .Replace('_', ' ') - .Split(' ', StringSplitOptions.RemoveEmptyEntries) - .Select(word => char.ToUpper(word[0]) + word[1..].ToLower()) - .Aggregate((current, next) => $"{current} {next}"); + internal static string TitleCaseFromFileName(string fileName) => Path.GetFileNameWithoutExtension(fileName).TitleCaseFromSlug(); } diff --git a/src/Elastic.ApiExplorer/Operations/OperationCommonMark.cs b/src/Elastic.ApiExplorer/Operations/OperationCommonMark.cs index cc3b4f071b..03cd57eb5b 100644 --- a/src/Elastic.ApiExplorer/Operations/OperationCommonMark.cs +++ b/src/Elastic.ApiExplorer/Operations/OperationCommonMark.cs @@ -17,7 +17,7 @@ public static string Write(ApiOperation apiOperation, OperationPageModel page, A var markdown = new StringBuilder(); var apiBaseUrl = context.CurrentNavigation.NavigationRoot.Url; var operation = apiOperation.Operation; - var title = OpenApiExtensionReader.GetSummary(operation) ?? apiOperation.ApiName; + var title = OpenApiExtensionReader.GetSummary(operation) ?? apiOperation.DisplayName; ApiCommonMark.Heading(markdown, 1, title); WriteBadges(markdown, operation, page); WriteServers(markdown, page); diff --git a/src/Elastic.ApiExplorer/Operations/OperationNavigationItem.cs b/src/Elastic.ApiExplorer/Operations/OperationNavigationItem.cs index 6b4f5e8b1d..20bf93de87 100644 --- a/src/Elastic.ApiExplorer/Operations/OperationNavigationItem.cs +++ b/src/Elastic.ApiExplorer/Operations/OperationNavigationItem.cs @@ -25,6 +25,12 @@ string ApiName // rather than the spec; the sidebar badge must agree with it. string IHttpMethodNavigationModel.HttpMethod => OperationEndpoint.Build(this, Operation.Description, []).Rows[0].Method; + /// The title shown for the operation; an operationId used as the grouping key is shown as readable words. + public string DisplayName => + !string.IsNullOrWhiteSpace(Operation.OperationId) && string.Equals(ApiName, Operation.OperationId, StringComparison.Ordinal) + ? ApiName.TitleCaseFromSlug() + : ApiName; + /// /// The operation's parameters and the path item's, which apply to every operation on the path unless the operation /// declares one of the same name and location itself. @@ -70,7 +76,7 @@ public OperationNavigationItem( { NavigationRoot = root; Model = apiOperation; - NavigationTitle = apiOperation.ApiName; + NavigationTitle = apiOperation.DisplayName; Parent = parent; Url = ApiUrlBuilder.OperationUrl( urlPathPrefix, diff --git a/src/Elastic.Documentation/Extensions/StringExtensions.cs b/src/Elastic.Documentation/Extensions/StringExtensions.cs index 5a5daf85c5..3a1628f37c 100644 --- a/src/Elastic.Documentation/Extensions/StringExtensions.cs +++ b/src/Elastic.Documentation/Extensions/StringExtensions.cs @@ -14,4 +14,15 @@ public static string OptionalWindowsReplace(this string relativePath) relativePath = relativePath.Replace('\\', '/'); return relativePath; } + + /// Converts a kebab-case/snake_case identifier to a title-cased string ("get-script" becomes "Get Script"). + public static string TitleCaseFromSlug(this string slug) => + string.Join( + ' ', + slug + .Replace('-', ' ') + .Replace('_', ' ') + .Split(' ', StringSplitOptions.RemoveEmptyEntries) + .Select(word => char.ToUpperInvariant(word[0]) + word[1..].ToLowerInvariant()) + ); } diff --git a/tests/Elastic.ApiExplorer.Tests/DashboardOpenApiNavigationTests.cs b/tests/Elastic.ApiExplorer.Tests/DashboardOpenApiNavigationTests.cs index dbb3dd1131..dbf1f1d7e7 100644 --- a/tests/Elastic.ApiExplorer.Tests/DashboardOpenApiNavigationTests.cs +++ b/tests/Elastic.ApiExplorer.Tests/DashboardOpenApiNavigationTests.cs @@ -45,7 +45,7 @@ public async Task CreateNavigation_SingleTagOpenApiSpec_HasSidebarItems() } [Test] - public async Task CreateNavigation_BlankSummary_TitlesOperationByOperationId() + public async Task CreateNavigation_BlankSummary_TitlesOperationByHumanizedOperationId() { var configurationContext = TestHelpers.CreateConfigurationContext(new FileSystem()); var context = new BuildContext( @@ -61,7 +61,30 @@ public async Task CreateNavigation_BlankSummary_TitlesOperationByOperationId() var navigation = generator.CreateNavigation("blank", doc); var titles = Flatten(navigation.NavigationItems).OfType().Select(o => o.NavigationTitle).ToArray(); - titles.Should().ContainSingle().Which.Should().Be("get-script"); + titles.Should().ContainSingle().Which.Should().Be("Get Script"); + } + + [Test] + public async Task CreateNavigation_OperationIdsWithTheSameTitle_StaySeparatePages() + { + var configurationContext = TestHelpers.CreateConfigurationContext(new FileSystem()); + var context = new BuildContext( + new DiagnosticsCollector([]), + DocumentationFileSystem.Resolve(Paths.WorkingDirectoryRoot.FullName), + configurationContext + ); + var fs = new FileSystem(); + var fi = fs.FileInfo.New(fs.Path.Combine(AppContext.BaseDirectory, "TestData", "colliding-operation-ids-openapi.json")); + var doc = await OpenApiReader.Instance.ReadAsync(fi); + + var generator = new OpenApiGenerator(NullLoggerFactory.Instance, context, NoopMarkdownStringRenderer.Instance); + var navigation = generator.CreateNavigation("colliding", doc); + + var operations = Flatten(navigation.NavigationItems).OfType().ToArray(); + operations.Should().HaveCount(2); + operations.Select(o => o.Url).Should().OnlyHaveUniqueItems(); + operations.Should().OnlyContain(o => o.Siblings.Count == 0); + operations.Select(o => o.NavigationTitle).Should().AllBe("Get Script"); } private static IEnumerable Flatten(IEnumerable items) diff --git a/tests/Elastic.ApiExplorer.Tests/TestData/colliding-operation-ids-openapi.json b/tests/Elastic.ApiExplorer.Tests/TestData/colliding-operation-ids-openapi.json new file mode 100644 index 0000000000..be596f3d29 --- /dev/null +++ b/tests/Elastic.ApiExplorer.Tests/TestData/colliding-operation-ids-openapi.json @@ -0,0 +1,22 @@ +{ + "openapi": "3.0.3", + "info": { "title": "Colliding ids", "version": "1.0.0" }, + "paths": { + "/api/first": { + "get": { + "operationId": "get-script", + "tags": ["connectors"], + "description": "Returns the first script.", + "responses": { "200": { "description": "OK" } } + } + }, + "/api/second": { + "get": { + "operationId": "get_script", + "tags": ["connectors"], + "description": "Returns the second script.", + "responses": { "200": { "description": "OK" } } + } + } + } +} diff --git a/tests/Elastic.Documentation.Tests/StringExtensionsTests.cs b/tests/Elastic.Documentation.Tests/StringExtensionsTests.cs new file mode 100644 index 0000000000..619f067877 --- /dev/null +++ b/tests/Elastic.Documentation.Tests/StringExtensionsTests.cs @@ -0,0 +1,19 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; +using Elastic.Documentation.Extensions; + +namespace Elastic.Documentation.Tests; + +public class StringExtensionsTests +{ + [Test] + [Arguments("get-actions-connector-oauth-callback-script", "Get Actions Connector Oauth Callback Script")] + [Arguments("async_search_delete", "Async Search Delete")] + [Arguments("GET-Script", "Get Script")] + [Arguments("-", "")] + public void TitleCaseFromSlug_KebabOrSnakeCase_ReturnsTitleCasedWords(string slug, string expected) => + slug.TitleCaseFromSlug().Should().Be(expected); +}