From f4e337057d8e1138a284a01dbfbe6fb813a61d94 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 10 Jul 2025 11:55:33 +0200 Subject: [PATCH 1/3] Remove Uri sources from navigation rendering. This PR removes the need to keep track of NavigationSource on markdown files to render the navigation. Previous to this we needed the keep this around to track the 'real' parent of a section once it may have been reshuffled by `navigation.yml`. We now more diligently build the global navigation more truthfully and can relay on walking the current navigation items `.Parent` to find the proper root to render. This includes scrubbing/skipping phantoms from the navigation. Another step to refactoring our navigation acyclic graph as the single source of truth. --- src/Elastic.ApiExplorer/OpenApiGenerator.cs | 2 +- .../Navigation/INavigationHtmlWriter.cs | 3 +- .../IsolatedBuildNavigationHtmlWriter.cs | 5 +- src/Elastic.Markdown/HtmlWriter.cs | 4 +- src/Elastic.Markdown/IO/MarkdownFile.cs | 3 - .../IO/Navigation/DocumentationGroup.cs | 3 +- .../IO/Navigation/TableOfContentsTree.cs | 2 +- .../TableOfContentsTreeCollector.cs | 6 +- src/tooling/docs-assembler/AssembleSources.cs | 27 ++--- .../Building/PublishEnvironmentUriResolver.cs | 4 +- .../Links/NavigationPrefixChecker.cs | 2 +- .../Navigation/GlobalNavigation.cs | 109 ++++++++++++++---- .../Navigation/GlobalNavigationHtmlWriter.cs | 78 ++++--------- .../GlobalNavigationPathProvider.cs | 4 +- .../GlobalNavigationTests.cs | 18 +-- 15 files changed, 146 insertions(+), 124 deletions(-) diff --git a/src/Elastic.ApiExplorer/OpenApiGenerator.cs b/src/Elastic.ApiExplorer/OpenApiGenerator.cs index cb08b4cde..ec68fc819 100644 --- a/src/Elastic.ApiExplorer/OpenApiGenerator.cs +++ b/src/Elastic.ApiExplorer/OpenApiGenerator.cs @@ -288,7 +288,7 @@ private async Task Render(INavigationItem current, T page, ApiRend if (!outputFile.Directory!.Exists) outputFile.Directory.Create(); - var navigationRenderResult = await navigationRenderer.RenderNavigation(current.NavigationRoot, new Uri("http://ignored.example"), INavigationHtmlWriter.AllLevels, ctx); + var navigationRenderResult = await navigationRenderer.RenderNavigation(current.NavigationRoot, INavigationHtmlWriter.AllLevels, ctx); renderContext = renderContext with { CurrentNavigation = current, diff --git a/src/Elastic.Documentation.Site/Navigation/INavigationHtmlWriter.cs b/src/Elastic.Documentation.Site/Navigation/INavigationHtmlWriter.cs index cf86b765c..3f598bcac 100644 --- a/src/Elastic.Documentation.Site/Navigation/INavigationHtmlWriter.cs +++ b/src/Elastic.Documentation.Site/Navigation/INavigationHtmlWriter.cs @@ -10,8 +10,7 @@ public interface INavigationHtmlWriter { const int AllLevels = -1; - Task RenderNavigation(IRootNavigationItem currentRootNavigation, Uri navigationSource, - int maxLevel, Cancel ctx = default); + Task RenderNavigation(IRootNavigationItem currentRootNavigation, int maxLevel, Cancel ctx = default); async Task Render(NavigationViewModel model, Cancel ctx) { diff --git a/src/Elastic.Documentation.Site/Navigation/IsolatedBuildNavigationHtmlWriter.cs b/src/Elastic.Documentation.Site/Navigation/IsolatedBuildNavigationHtmlWriter.cs index e0c3c5d29..198405d18 100644 --- a/src/Elastic.Documentation.Site/Navigation/IsolatedBuildNavigationHtmlWriter.cs +++ b/src/Elastic.Documentation.Site/Navigation/IsolatedBuildNavigationHtmlWriter.cs @@ -13,8 +13,9 @@ public class IsolatedBuildNavigationHtmlWriter(BuildContext context, IRootNaviga { private readonly ConcurrentDictionary<(string, int), string> _renderedNavigationCache = []; - public async Task RenderNavigation(IRootNavigationItem currentRootNavigation, - Uri navigationSource, int maxLevel, Cancel ctx = default) + public async Task RenderNavigation( + IRootNavigationItem currentRootNavigation, int maxLevel, Cancel ctx = default + ) { var navigation = context.Configuration.Features.PrimaryNavEnabled || currentRootNavigation.IsUsingNavigationDropdown ? currentRootNavigation diff --git a/src/Elastic.Markdown/HtmlWriter.cs b/src/Elastic.Markdown/HtmlWriter.cs index b53e57a5a..7a21f0481 100644 --- a/src/Elastic.Markdown/HtmlWriter.cs +++ b/src/Elastic.Markdown/HtmlWriter.cs @@ -56,8 +56,8 @@ private async Task RenderLayout(MarkdownFile markdown, MarkdownDoc var html = MarkdownFile.CreateHtml(document); await DocumentationSet.Tree.Resolve(ctx); - var fullNavigationRenderResult = await NavigationHtmlWriter.RenderNavigation(markdown.NavigationRoot, markdown.NavigationSource, INavigationHtmlWriter.AllLevels, ctx); - var miniNavigationRenderResult = await NavigationHtmlWriter.RenderNavigation(markdown.NavigationRoot, markdown.NavigationSource, 1, ctx); + var fullNavigationRenderResult = await NavigationHtmlWriter.RenderNavigation(markdown.NavigationRoot, INavigationHtmlWriter.AllLevels, ctx); + var miniNavigationRenderResult = await NavigationHtmlWriter.RenderNavigation(markdown.NavigationRoot, 1, ctx); var navigationHtmlRenderResult = DocumentationSet.Context.Configuration.Features.LazyLoadNavigation ? miniNavigationRenderResult diff --git a/src/Elastic.Markdown/IO/MarkdownFile.cs b/src/Elastic.Markdown/IO/MarkdownFile.cs index 592d69832..6e1c21eec 100644 --- a/src/Elastic.Markdown/IO/MarkdownFile.cs +++ b/src/Elastic.Markdown/IO/MarkdownFile.cs @@ -57,7 +57,6 @@ DocumentationSet set ScopeDirectory = build.Configuration.ScopeDirectory; NavigationRoot = set.Tree; - NavigationSource = set.Source; } public bool PartOfNavigation { get; set; } @@ -66,8 +65,6 @@ DocumentationSet set public IRootNavigationItem NavigationRoot { get; set; } - public Uri NavigationSource { get; set; } - private IDiagnosticsCollector Collector { get; } public string? UrlPathPrefix { get; } diff --git a/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs b/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs index 31009a135..fd7554c85 100644 --- a/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs +++ b/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs @@ -11,7 +11,7 @@ namespace Elastic.Markdown.IO.Navigation; -[DebuggerDisplay("Group >{Depth} {FolderName} ({NavigationItems.Count} items)")] +[DebuggerDisplay("Toc: {Depth} {NavigationSource} > ({NavigationItems.Count} items)")] public class DocumentationGroup : INodeNavigationItem { private readonly TableOfContentsTreeCollector _treeCollector; @@ -138,7 +138,6 @@ void AddToNavigationItems(INavigationItem item, ref int fileIndex) // TODO these have to be refactor to be pure navigational properties md.ScopeDirectory = file.TableOfContentsScope.ScopeDirectory; md.NavigationRoot = rootNavigationItem; - md.NavigationSource = NavigationSource; foreach (var extension in lookups.EnabledExtensions) extension.Visit(d, tocItem); diff --git a/src/Elastic.Markdown/IO/Navigation/TableOfContentsTree.cs b/src/Elastic.Markdown/IO/Navigation/TableOfContentsTree.cs index f2e92e695..d9caeb648 100644 --- a/src/Elastic.Markdown/IO/Navigation/TableOfContentsTree.cs +++ b/src/Elastic.Markdown/IO/Navigation/TableOfContentsTree.cs @@ -8,7 +8,7 @@ namespace Elastic.Markdown.IO.Navigation; -[DebuggerDisplay("Toc >{Depth} {FolderName} ({NavigationItems.Count} items)")] +[DebuggerDisplay("Toc: {Depth} {NavigationSource} > ({NavigationItems.Count} items)")] public class TableOfContentsTree : DocumentationGroup, IRootNavigationItem { public Uri Source { get; } diff --git a/src/Elastic.Markdown/IO/Navigation/TableOfContentsTreeCollector.cs b/src/Elastic.Markdown/IO/Navigation/TableOfContentsTreeCollector.cs index 47d8e8547..103a91a93 100644 --- a/src/Elastic.Markdown/IO/Navigation/TableOfContentsTreeCollector.cs +++ b/src/Elastic.Markdown/IO/Navigation/TableOfContentsTreeCollector.cs @@ -11,11 +11,7 @@ public class TableOfContentsTreeCollector { private Dictionary NestedTableOfContentsTrees { get; } = []; - public void Collect(Uri source, TableOfContentsTree tree) => - NestedTableOfContentsTrees[source] = tree; - - public void Collect(TocReference tocReference, TableOfContentsTree tree) => - NestedTableOfContentsTrees[tocReference.Source] = tree; + public void Collect(Uri source, TableOfContentsTree tree) => NestedTableOfContentsTrees[source] = tree; public bool TryGetTableOfContentsTree(Uri source, [NotNullWhen(true)] out TableOfContentsTree? tree) => NestedTableOfContentsTrees.TryGetValue(source, out tree); diff --git a/src/tooling/docs-assembler/AssembleSources.cs b/src/tooling/docs-assembler/AssembleSources.cs index 1bdcf6396..9ffe0664d 100644 --- a/src/tooling/docs-assembler/AssembleSources.cs +++ b/src/tooling/docs-assembler/AssembleSources.cs @@ -20,7 +20,8 @@ using YamlDotNet.RepresentationModel; namespace Documentation.Assembler; -public record TocTopLevelMapping + +public record NavigationTocMapping { public required Uri Source { get; init; } public required string SourcePathPrefix { get; init; } @@ -30,7 +31,7 @@ public record TocTopLevelMapping public record TocConfigurationMapping { - public required TocTopLevelMapping TopLevel { get; init; } + public required NavigationTocMapping TopLevel { get; init; } public required ConfigurationFile RepositoryConfigurationFile { get; init; } public required TableOfContentsConfiguration TableOfContentsConfiguration { get; init; } } @@ -40,7 +41,7 @@ public class AssembleSources public AssembleContext AssembleContext { get; } public FrozenDictionary AssembleSets { get; } - public FrozenDictionary TocTopLevelMappings { get; } + public FrozenDictionary NavigationTocMappings { get; } public FrozenDictionary> HistoryMappings { get; } @@ -61,11 +62,11 @@ public static async Task AssembleAsync(ILoggerFactory logger, A private AssembleSources(ILoggerFactory logger, AssembleContext assembleContext, Checkout[] checkouts, VersionsConfiguration versionsConfiguration) { AssembleContext = assembleContext; - TocTopLevelMappings = GetConfiguredSources(assembleContext); + NavigationTocMappings = GetTocMappings(assembleContext); HistoryMappings = GetHistoryMapping(assembleContext); var linkIndexProvider = Aws3LinkIndexReader.CreateAnonymous(); var crossLinkFetcher = new AssemblerCrossLinkFetcher(logger, assembleContext.Configuration, assembleContext.Environment, linkIndexProvider); - UriResolver = new PublishEnvironmentUriResolver(TocTopLevelMappings, assembleContext.Environment); + UriResolver = new PublishEnvironmentUriResolver(NavigationTocMappings, assembleContext.Environment); var crossLinkResolver = new CrossLinkResolver(crossLinkFetcher, UriResolver); AssembleSets = checkouts .Where(c => c.Repository is { Skip: false }) @@ -73,7 +74,7 @@ private AssembleSources(ILoggerFactory logger, AssembleContext assembleContext, .ToDictionary(s => s.Checkout.Repository.Name, s => s) .ToFrozenDictionary(); - TocConfigurationMapping = TocTopLevelMappings + TocConfigurationMapping = NavigationTocMappings .Select(kv => { var repo = kv.Value.Source.Scheme; @@ -148,11 +149,11 @@ static void ReadHistoryMappings(IDictionary> } - public static FrozenDictionary GetConfiguredSources(AssembleContext context) + public static FrozenDictionary GetTocMappings(AssembleContext context) { - var dictionary = new Dictionary(); + var dictionary = new Dictionary(); var reader = new YamlStreamReader(context.NavigationPath, context.Collector); - var entries = new List>(); + var entries = new List>(); foreach (var entry in reader.Read()) { switch (entry.Key) @@ -167,7 +168,7 @@ public static FrozenDictionary GetConfiguredSources(Ass return dictionary.ToFrozenDictionary(); static void ReadTocBlocks( - List> entries, + List> entries, YamlStreamReader reader, KeyValuePair entry, string? parent, @@ -196,7 +197,7 @@ static void ReadTocBlocks( } } static void ReadBlock( - List> entries, + List> entries, YamlStreamReader reader, YamlMappingNode tocEntry, string? parent, @@ -259,14 +260,14 @@ static void ReadBlock( topLevelSource ??= sourceUri; parentSource ??= sourceUri; - var tocTopLevelMapping = new TocTopLevelMapping + var tocTopLevelMapping = new NavigationTocMapping { Source = sourceUri, SourcePathPrefix = pathPrefix, TopLevelSource = topLevelSource, ParentSource = parentSource }; - entries.Add(new KeyValuePair(sourceUri, tocTopLevelMapping)); + entries.Add(new KeyValuePair(sourceUri, tocTopLevelMapping)); foreach (var entry in tocEntry.Children) { diff --git a/src/tooling/docs-assembler/Building/PublishEnvironmentUriResolver.cs b/src/tooling/docs-assembler/Building/PublishEnvironmentUriResolver.cs index ad2ee16c3..528e3480a 100644 --- a/src/tooling/docs-assembler/Building/PublishEnvironmentUriResolver.cs +++ b/src/tooling/docs-assembler/Building/PublishEnvironmentUriResolver.cs @@ -11,14 +11,14 @@ namespace Documentation.Assembler.Building; public class PublishEnvironmentUriResolver : IUriEnvironmentResolver { - private readonly FrozenDictionary _topLevelMappings; + private readonly FrozenDictionary _topLevelMappings; private Uri BaseUri { get; } private PublishEnvironment PublishEnvironment { get; } private IReadOnlyList TableOfContentsPrefixes { get; } - public PublishEnvironmentUriResolver(FrozenDictionary topLevelMappings, PublishEnvironment environment) + public PublishEnvironmentUriResolver(FrozenDictionary topLevelMappings, PublishEnvironment environment) { _topLevelMappings = topLevelMappings; PublishEnvironment = environment; diff --git a/src/tooling/docs-assembler/Links/NavigationPrefixChecker.cs b/src/tooling/docs-assembler/Links/NavigationPrefixChecker.cs index 399b6c0b1..f66128d0f 100644 --- a/src/tooling/docs-assembler/Links/NavigationPrefixChecker.cs +++ b/src/tooling/docs-assembler/Links/NavigationPrefixChecker.cs @@ -53,7 +53,7 @@ public NavigationPrefixChecker(ILoggerFactory logger, AssembleContext context) _logger = logger.CreateLogger(); _loggerFactory = logger; - var tocTopLevelMappings = AssembleSources.GetConfiguredSources(context); + var tocTopLevelMappings = AssembleSources.GetTocMappings(context); _uriResolver = new PublishEnvironmentUriResolver(tocTopLevelMappings, context.Environment); } diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs index 14b921ff0..209e14159 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information using System.Collections.Frozen; +using System.Collections.Immutable; +using Elastic.Documentation.Configuration.Assembler; using Elastic.Documentation.Configuration.TableOfContents; using Elastic.Documentation.Site.Navigation; using Elastic.Markdown.IO; @@ -25,14 +27,30 @@ public record GlobalNavigation : IPositionalNavigation public FrozenDictionary NavigationIndexedByOrder { get; } +#pragma warning disable IDE0052 + private ImmutableHashSet Phantoms { get; } +#pragma warning restore IDE0052 + + private TableOfContentsTree RootContentTree { get; } + public GlobalNavigation(AssembleSources assembleSources, GlobalNavigationFile navigationFile) { _assembleSources = assembleSources; _navigationFile = navigationFile; - NavigationItems = BuildNavigation(navigationFile.TableOfContents.Concat(navigationFile.Phantoms).ToArray(), 0); + + // the root files of `docs-content://` are special they contain several special pages such as 404, archive, versions etc. + // we inject them forcefully here + var source = new Uri($"{NarrativeRepository.RepositoryName}://"); + RootContentTree = assembleSources.TreeCollector.TryGetTableOfContentsTree(source, out var docsContentTree) + ? docsContentTree + : throw new Exception($"Could not locate: {source} as root of global navigation."); + Phantoms = [.. navigationFile.Phantoms.Select(p => p.Source)]; + NavigationItems = BuildNavigation(navigationFile.TableOfContents, 0); + var navigationIndex = 0; var allNavigationItems = new HashSet(); - UpdateNavigationIndex(allNavigationItems, NavigationItems, null, ref navigationIndex); + UpdateParent(allNavigationItems, NavigationItems, null); + UpdateNavigationIndex(NavigationItems, ref navigationIndex); TopLevelItems = NavigationItems.OfType().Where(t => !t.Hidden).ToList(); NavigationLookup = TopLevelItems.ToDictionary(kv => kv.Source, kv => kv); @@ -42,13 +60,13 @@ public GlobalNavigation(AssembleSources assembleSources, GlobalNavigationFile na .SelectMany(DocumentationSet.Pairs) .ToDictionary(kv => kv.Item1, kv => kv.Item2) .ToFrozenDictionary(); + } - private void UpdateNavigationIndex( + private void UpdateParent( HashSet allNavigationItems, IReadOnlyCollection navigationItems, - INodeNavigationItem? parent, - ref int navigationIndex + INodeNavigationItem? parent ) { foreach (var item in navigationItems) @@ -56,19 +74,15 @@ ref int navigationIndex switch (item) { case FileNavigationItem fileNavigationItem: - var fileIndex = Interlocked.Increment(ref navigationIndex); - fileNavigationItem.NavigationIndex = fileIndex; if (parent is not null) fileNavigationItem.Parent = parent; _ = allNavigationItems.Add(fileNavigationItem); break; case DocumentationGroup documentationGroup: - var groupIndex = Interlocked.Increment(ref navigationIndex); - documentationGroup.NavigationIndex = groupIndex; if (parent is not null) documentationGroup.Parent = parent; _ = allNavigationItems.Add(documentationGroup); - UpdateNavigationIndex(allNavigationItems, documentationGroup.NavigationItems, documentationGroup, ref navigationIndex); + UpdateParent(allNavigationItems, documentationGroup.NavigationItems, documentationGroup); break; default: _navigationFile.EmitError($"Unhandled navigation item type: {item.GetType()}"); @@ -77,29 +91,57 @@ ref int navigationIndex } } - private IReadOnlyCollection BuildNavigation(IReadOnlyCollection node, int depth) + + private void UpdateNavigationIndex(IReadOnlyCollection navigationItems, ref int navigationIndex) + { + foreach (var item in navigationItems) + { + switch (item) + { + case FileNavigationItem fileNavigationItem: + var fileIndex = Interlocked.Increment(ref navigationIndex); + fileNavigationItem.NavigationIndex = fileIndex; + break; + case DocumentationGroup documentationGroup: + var groupIndex = Interlocked.Increment(ref navigationIndex); + documentationGroup.NavigationIndex = groupIndex; + UpdateNavigationIndex(documentationGroup.NavigationItems, ref navigationIndex); + break; + default: + _navigationFile.EmitError($"Unhandled navigation item type: {item.GetType()}"); + break; + } + } + } + + private IReadOnlyCollection BuildNavigation(IReadOnlyCollection references, int depth) { var list = new List(); - foreach (var toc in node) + foreach (var tocReference in references) { - if (!_assembleSources.TreeCollector.TryGetTableOfContentsTree(toc.Source, out var tree)) + if (!_assembleSources.TreeCollector.TryGetTableOfContentsTree(tocReference.Source, out var tree)) { - _navigationFile.EmitError($"{toc.Source} does not define a toc.yml or docset.yml file"); + _navigationFile.EmitError($"{tocReference.Source} does not define a toc.yml or docset.yml file"); continue; } - var navigationItem = tree; - var tocChildren = toc.Children.OfType().ToArray(); + var tocChildren = tocReference.Children.OfType().ToArray(); var tocNavigationItems = BuildNavigation(tocChildren, depth + 1); - var allNavigationItems = + if (depth == 0 && tree.Parent != RootContentTree) + { + tree.Parent = RootContentTree; + tree.Index.NavigationRoot = tree; + } + + var configuredNavigationItems = depth == 0 ? tocNavigationItems.Concat(tree.NavigationItems) : tree.NavigationItems.Concat(tocNavigationItems); var cleanNavigationItems = new List(); var seenSources = new HashSet(); - foreach (var item in allNavigationItems) + foreach (var item in configuredNavigationItems) { if (item is not TableOfContentsTree tocNav) { @@ -110,22 +152,43 @@ private IReadOnlyCollection BuildNavigation(IReadOnlyCollection if (seenSources.Contains(tocNav.Source)) continue; - if (!_assembleSources.TocTopLevelMappings.TryGetValue(tocNav.Source, out var mapping)) + if (Phantoms.Contains(tree.NavigationSource)) + continue; + + // toc is not part of `navigation.yml` + if (!_assembleSources.NavigationTocMappings.TryGetValue(tocNav.Source, out var mapping)) continue; + // this TOC was moved in navigation.yml to a new parent and should not be part of the current navigation items if (mapping.ParentSource != tree.Source) continue; _ = seenSources.Add(tocNav.Source); cleanNavigationItems.Add(item); - item.Parent = navigationItem; + item.Parent = tree; } tree.NavigationItems = cleanNavigationItems.ToArray(); - list.Add(navigationItem); + list.Add(tree); - if (toc.IsPhantom) - navigationItem.Hidden = true; + if (tocReference.IsPhantom) + tree.Hidden = true; + } + + if (depth != 0) + return list.ToArray().AsReadOnly(); + + // the root files of `docs-content://` are special they contain several special pages such as 404, archive, versions etc. + // we inject them forcefully here + if (!RootContentTree.NavigationItems.OfType().Any()) + _navigationFile.EmitError($"Could not inject root file navigation items from: {RootContentTree.Source}."); + else + { + var filesAtRoot = RootContentTree.NavigationItems.OfType().ToArray(); + list.AddRange(filesAtRoot); + // ensure index exist as a single item rather than injecting the whole tree (which already exists in the returned list) + var index = new FileNavigationItem(RootContentTree.Index, RootContentTree, RootContentTree.Hidden); + list.Add(index); } return list.ToArray().AsReadOnly(); diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs index 277636107..670590334 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs @@ -11,87 +11,53 @@ namespace Documentation.Assembler.Navigation; +#pragma warning disable CS9113 // Parameter is unread. public class GlobalNavigationHtmlWriter( GlobalNavigationFile navigationFile, AssembleContext assembleContext, GlobalNavigation globalNavigation, AssembleSources assembleSources ) : INavigationHtmlWriter +#pragma warning restore CS9113 // Parameter is unread. { - private readonly ConcurrentDictionary<(Uri, int), string> _renderedNavigationCache = []; + private readonly ConcurrentDictionary<(string, int), string> _renderedNavigationCache = []; private ImmutableHashSet Phantoms { get; } = [.. navigationFile.Phantoms.Select(p => p.Source)]; - private bool TryGetNavigationRoot( - Uri navigationSource, - [NotNullWhen(true)] out TableOfContentsTree? navigationRoot, - [NotNullWhen(true)] out Uri? navigationRootSource - ) + public async Task RenderNavigation(IRootNavigationItem currentRootNavigation, int maxLevel, Cancel ctx = default) { - navigationRoot = null; - navigationRootSource = null; - if (!assembleSources.TocTopLevelMappings.TryGetValue(navigationSource, out var topLevelMapping)) + await Task.CompletedTask; + INodeNavigationItem lastParentBeforeRoot = currentRootNavigation; + INodeNavigationItem parent = currentRootNavigation; + while (parent.Parent is not null) { - if (assembleSources.TreeCollector.TryGetTableOfContentsTree(navigationSource, out navigationRoot)) - { - if (navigationRoot.Parent is DocumentationGroup parent && - assembleSources.TocTopLevelMappings.TryGetValue(parent.NavigationSource, out var topLevelMapping2)) - { - navigationRootSource = topLevelMapping2.TopLevelSource; - return true; - } - - assembleContext.Collector.EmitError(assembleContext.NavigationPath.FullName, $"The following toc: {navigationSource} is not declared in navigation.yml and it's parent failed to yield a navigation root."); - return false; - } - assembleContext.Collector.EmitError(assembleContext.NavigationPath.FullName, $"The following toc: {navigationSource} is not declared in navigation.yml"); - return false; + lastParentBeforeRoot = parent; + parent = parent.Parent; } - - if (!assembleSources.TreeCollector.TryGetTableOfContentsTree(topLevelMapping.TopLevelSource, out navigationRoot)) - { - assembleContext.Collector.EmitError(assembleContext.NavigationPath.FullName, $"None of the assemble sources define a toc for: {topLevelMapping.TopLevelSource}"); - return false; - } - navigationRootSource = topLevelMapping.TopLevelSource; - return true; - } - - public async Task RenderNavigation(IRootNavigationItem currentRootNavigation, - Uri navigationSource, int maxLevel, Cancel ctx = default) - { - if (Phantoms.Contains(navigationSource) - || !TryGetNavigationRoot(navigationSource, out var navigationRoot, out var navigationRootSource) - || Phantoms.Contains(navigationRootSource) - ) - return NavigationRenderResult.Empty; - - var navigationId = ShortId.Create($"{(navigationRootSource, maxLevel).GetHashCode()}"); - - if (_renderedNavigationCache.TryGetValue((navigationRootSource, maxLevel), out var value)) + if (_renderedNavigationCache.TryGetValue((lastParentBeforeRoot.Id, maxLevel), out var html)) { return new NavigationRenderResult { - Html = value, - Id = navigationId + Html = html, + Id = lastParentBeforeRoot.Id }; } - if (navigationRootSource == new Uri("docs-content:///")) + Console.WriteLine($"Rendering navigation for {lastParentBeforeRoot.NavigationTitle} ({lastParentBeforeRoot.Id})"); + if (lastParentBeforeRoot.NavigationTitle == "Cloud release notes") { - _renderedNavigationCache[(navigationRootSource, maxLevel)] = string.Empty; - return NavigationRenderResult.Empty; } - Console.WriteLine($"Rendering navigation for {navigationRootSource}"); + if (lastParentBeforeRoot is not DocumentationGroup group) + return NavigationRenderResult.Empty; - var model = CreateNavigationModel(navigationRoot, maxLevel); - value = await ((INavigationHtmlWriter)this).Render(model, ctx); - _renderedNavigationCache[(navigationRootSource, maxLevel)] = value; + var model = CreateNavigationModel(group, maxLevel); + html = await ((INavigationHtmlWriter)this).Render(model, ctx); + _renderedNavigationCache[(lastParentBeforeRoot.Id, maxLevel)] = html; return new NavigationRenderResult { - Html = value, - Id = navigationId + Html = html, + Id = lastParentBeforeRoot.Id }; } diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigationPathProvider.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigationPathProvider.cs index 04f156d64..b9c10a978 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigationPathProvider.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigationPathProvider.cs @@ -25,7 +25,7 @@ public GlobalNavigationPathProvider(GlobalNavigationFile navigationFile, Assembl _assembleSources = assembleSources; _context = context; - TableOfContentsPrefixes = [..assembleSources.TocTopLevelMappings + TableOfContentsPrefixes = [..assembleSources.NavigationTocMappings .Values .Select(p => { @@ -91,7 +91,7 @@ public GlobalNavigationPathProvider(GlobalNavigationFile navigationFile, Assembl break; } - if (match is null || !_assembleSources.TocTopLevelMappings.TryGetValue(match, out var toc)) + if (match is null || !_assembleSources.NavigationTocMappings.TryGetValue(match, out var toc)) { if (relativePath.StartsWith("raw-migrated-files/", StringComparison.Ordinal)) return null; diff --git a/tests/docs-assembler.Tests/src/docs-assembler.Tests/GlobalNavigationTests.cs b/tests/docs-assembler.Tests/src/docs-assembler.Tests/GlobalNavigationTests.cs index 6ba63df13..23bedfdb9 100644 --- a/tests/docs-assembler.Tests/src/docs-assembler.Tests/GlobalNavigationTests.cs +++ b/tests/docs-assembler.Tests/src/docs-assembler.Tests/GlobalNavigationTests.cs @@ -106,7 +106,7 @@ public async Task PathProvider() var navigationFile = new GlobalNavigationFile(Context, assembleSources); var pathProvider = new GlobalNavigationPathProvider(navigationFile, assembleSources, Context); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(new Uri("detection-rules://")); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(new Uri("detection-rules://")); pathProvider.TableOfContentsPrefixes.Should().Contain("detection-rules://"); } @@ -122,12 +122,12 @@ public async Task ParsesReferences() var clients = new Uri("docs-content://reference/elasticsearch-clients/"); var assembleSources = await Setup(); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(sut); - assembleSources.TocTopLevelMappings[sut].TopLevelSource.Should().Be(expectedRoot); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(expectedRoot); - assembleSources.TocTopLevelMappings[sut].ParentSource.Should().Be(expectedParent); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(sut); + assembleSources.NavigationTocMappings[sut].TopLevelSource.Should().Be(expectedRoot); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(expectedRoot); + assembleSources.NavigationTocMappings[sut].ParentSource.Should().Be(expectedParent); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(new Uri("detection-rules://")); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(new Uri("detection-rules://")); var navigationFile = new GlobalNavigationFile(Context, assembleSources); var referenceToc = navigationFile.TableOfContents.FirstOrDefault(t => t.Source == expectedRoot); @@ -193,9 +193,9 @@ public async Task ParsesGlobalNavigation() var kibanaExtendMoniker = new Uri("kibana://extend/"); var assembleSources = await Setup(); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(kibanaExtendMoniker); - assembleSources.TocTopLevelMappings[kibanaExtendMoniker].TopLevelSource.Should().Be(expectedRoot); - assembleSources.TocTopLevelMappings.Should().NotBeEmpty().And.ContainKey(new Uri("docs-content://reference/apm/")); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(kibanaExtendMoniker); + assembleSources.NavigationTocMappings[kibanaExtendMoniker].TopLevelSource.Should().Be(expectedRoot); + assembleSources.NavigationTocMappings.Should().NotBeEmpty().And.ContainKey(new Uri("docs-content://reference/apm/")); var uri = new Uri("integration-docs://reference/"); assembleSources.TreeCollector.Should().NotBeNull(); From 6a9b6c4aa2447fb6ced86da6fdf65c50fe9b2779 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 10 Jul 2025 12:57:30 +0200 Subject: [PATCH 2/3] remove temporary pragma's while refactoring --- .../docs-assembler/Cli/RepositoryCommands.cs | 2 +- .../docs-assembler/Navigation/GlobalNavigation.cs | 2 -- .../Navigation/GlobalNavigationHtmlWriter.cs | 15 +-------------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/tooling/docs-assembler/Cli/RepositoryCommands.cs b/src/tooling/docs-assembler/Cli/RepositoryCommands.cs index b2b514cb9..df389ba54 100644 --- a/src/tooling/docs-assembler/Cli/RepositoryCommands.cs +++ b/src/tooling/docs-assembler/Cli/RepositoryCommands.cs @@ -130,7 +130,7 @@ public async Task BuildAll( var navigation = new GlobalNavigation(assembleSources, navigationFile); var pathProvider = new GlobalNavigationPathProvider(navigationFile, assembleSources, assembleContext); - var htmlWriter = new GlobalNavigationHtmlWriter(navigationFile, assembleContext, navigation, assembleSources); + var htmlWriter = new GlobalNavigationHtmlWriter(navigation); var legacyPageChecker = new LegacyPageChecker(); var historyMapper = new PageLegacyUrlMapper(legacyPageChecker, assembleSources.HistoryMappings); diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs index 209e14159..f028616cd 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigation.cs @@ -27,9 +27,7 @@ public record GlobalNavigation : IPositionalNavigation public FrozenDictionary NavigationIndexedByOrder { get; } -#pragma warning disable IDE0052 private ImmutableHashSet Phantoms { get; } -#pragma warning restore IDE0052 private TableOfContentsTree RootContentTree { get; } diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs index 670590334..958fc64f8 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs @@ -3,30 +3,17 @@ // See the LICENSE file in the project root for more information using System.Collections.Concurrent; -using System.Collections.Immutable; -using System.Diagnostics.CodeAnalysis; -using Elastic.Documentation.Extensions; using Elastic.Documentation.Site.Navigation; using Elastic.Markdown.IO.Navigation; namespace Documentation.Assembler.Navigation; -#pragma warning disable CS9113 // Parameter is unread. -public class GlobalNavigationHtmlWriter( - GlobalNavigationFile navigationFile, - AssembleContext assembleContext, - GlobalNavigation globalNavigation, - AssembleSources assembleSources -) : INavigationHtmlWriter -#pragma warning restore CS9113 // Parameter is unread. +public class GlobalNavigationHtmlWriter(GlobalNavigation globalNavigation) : INavigationHtmlWriter { private readonly ConcurrentDictionary<(string, int), string> _renderedNavigationCache = []; - private ImmutableHashSet Phantoms { get; } = [.. navigationFile.Phantoms.Select(p => p.Source)]; - public async Task RenderNavigation(IRootNavigationItem currentRootNavigation, int maxLevel, Cancel ctx = default) { - await Task.CompletedTask; INodeNavigationItem lastParentBeforeRoot = currentRootNavigation; INodeNavigationItem parent = currentRootNavigation; while (parent.Parent is not null) From c68005ee3eb9e59acb9892e3c52297f6c3608c3d Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 10 Jul 2025 13:00:42 +0200 Subject: [PATCH 3/3] use logger not Console.WriteLine --- src/tooling/docs-assembler/Cli/RepositoryCommands.cs | 2 +- .../Navigation/GlobalNavigationHtmlWriter.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tooling/docs-assembler/Cli/RepositoryCommands.cs b/src/tooling/docs-assembler/Cli/RepositoryCommands.cs index df389ba54..a94e734d2 100644 --- a/src/tooling/docs-assembler/Cli/RepositoryCommands.cs +++ b/src/tooling/docs-assembler/Cli/RepositoryCommands.cs @@ -130,7 +130,7 @@ public async Task BuildAll( var navigation = new GlobalNavigation(assembleSources, navigationFile); var pathProvider = new GlobalNavigationPathProvider(navigationFile, assembleSources, assembleContext); - var htmlWriter = new GlobalNavigationHtmlWriter(navigation); + var htmlWriter = new GlobalNavigationHtmlWriter(logger, navigation); var legacyPageChecker = new LegacyPageChecker(); var historyMapper = new PageLegacyUrlMapper(legacyPageChecker, assembleSources.HistoryMappings); diff --git a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs index 958fc64f8..9a65dd4ad 100644 --- a/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs +++ b/src/tooling/docs-assembler/Navigation/GlobalNavigationHtmlWriter.cs @@ -5,11 +5,14 @@ using System.Collections.Concurrent; using Elastic.Documentation.Site.Navigation; using Elastic.Markdown.IO.Navigation; +using Microsoft.Extensions.Logging; namespace Documentation.Assembler.Navigation; -public class GlobalNavigationHtmlWriter(GlobalNavigation globalNavigation) : INavigationHtmlWriter +public class GlobalNavigationHtmlWriter(ILoggerFactory logFactory, GlobalNavigation globalNavigation) : INavigationHtmlWriter { + private readonly ILogger _logger = logFactory.CreateLogger(); + private readonly ConcurrentDictionary<(string, int), string> _renderedNavigationCache = []; public async Task RenderNavigation(IRootNavigationItem currentRootNavigation, int maxLevel, Cancel ctx = default) @@ -30,10 +33,7 @@ public async Task RenderNavigation(IRootNavigationItem