Skip to content

Commit c08ac2d

Browse files
MpdreamzMpdreamz
committed
Normalize sitePrefix in SiteNavigation, update URL handling, and extend test coverage:
- Add `NormalizeSitePrefix` method to standardize `sitePrefix` with leading slash and no trailing slash. - Apply normalized `sitePrefix` to navigation URLs. - Improve `GetDescription` error messaging in `LlmsNavigationEnhancer`. - Update tests to validate `sitePrefix` normalization and navigation item URL adjustments.
1 parent ee11a02 commit c08ac2d

File tree

9 files changed

+178
-24
lines changed

9 files changed

+178
-24
lines changed

‎src/Elastic.Documentation.Navigation/Assembler/AssembledNavigation.cs‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,16 @@ public record SiteNavigationNoIndexFile(string NavigationTitle) : IDocumentation
1414

1515
public class SiteNavigation : IRootNavigationItem<IDocumentationFile, INavigationItem>
1616
{
17-
public SiteNavigation(
18-
SiteNavigationFile siteNavigationFile,
17+
private readonly string? _sitePrefix;
18+
19+
public SiteNavigation(SiteNavigationFile siteNavigationFile,
1920
IDocumentationContext context,
20-
IReadOnlyCollection<IDocumentationSetNavigation> documentationSetNavigations
21+
IReadOnlyCollection<IDocumentationSetNavigation> documentationSetNavigations,
22+
string? sitePrefix
2123
)
2224
{
25+
// Normalize sitePrefix to ensure it has a leading slash and no trailing slash
26+
_sitePrefix = NormalizeSitePrefix(sitePrefix);
2327
// Initialize root properties
2428
NavigationRoot = this;
2529
Parent = null;
@@ -72,7 +76,7 @@ IReadOnlyCollection<IDocumentationSetNavigation> documentationSetNavigations
7276
NavigationItems.OfType<INodeNavigationItem<INavigationModel, INavigationItem>>().ToList();
7377

7478
/// <inheritdoc />
75-
public string Url { get; set; } = "/";
79+
public string Url => string.IsNullOrEmpty(_sitePrefix) ? "/" : _sitePrefix;
7680

7781
/// <inheritdoc />
7882
public string NavigationTitle => Index.NavigationTitle;
@@ -107,6 +111,27 @@ IReadOnlyCollection<IDocumentationSetNavigation> documentationSetNavigations
107111
/// <inheritdoc />
108112
public IReadOnlyCollection<INavigationItem> NavigationItems { get; }
109113

114+
/// <summary>
115+
/// Normalizes the site prefix to ensure it has a leading slash and no trailing slash.
116+
/// Returns null for null or empty/whitespace input.
117+
/// </summary>
118+
private static string? NormalizeSitePrefix(string? sitePrefix)
119+
{
120+
if (string.IsNullOrWhiteSpace(sitePrefix))
121+
return null;
122+
123+
var normalized = sitePrefix.Trim();
124+
125+
// Ensure leading slash
126+
if (!normalized.StartsWith('/'))
127+
normalized = "/" + normalized;
128+
129+
// Remove trailing slash
130+
normalized = normalized.TrimEnd('/');
131+
132+
return normalized;
133+
}
134+
110135
private INavigationItem? CreateSiteTableOfContentsNavigation(
111136
SiteTableOfContentsRef tocRef,
112137
int index,
@@ -128,6 +153,14 @@ IDocumentationContext context
128153
pathPrefix += $"/{tocRef.Source.AbsolutePath}";
129154
}
130155

156+
// Normalize pathPrefix to remove leading/trailing slashes for consistent combination
157+
pathPrefix = pathPrefix.Trim('/');
158+
159+
// Combine with site prefix if present, otherwise ensure leading slash
160+
pathPrefix = !string.IsNullOrWhiteSpace(_sitePrefix)
161+
? $"{_sitePrefix}/{pathPrefix}"
162+
: "/" + pathPrefix;
163+
131164
// Look up the node in the collected nodes
132165
if (!_nodes.TryGetValue(tocRef.Source, out var node))
133166
{

‎src/Elastic.Documentation.Navigation/Isolated/DocumentationSetNavigation.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ int depth
493493
var finalTocNavigation = new TableOfContentsNavigation(
494494
tocDirectory,
495495
depth + 1,
496-
relativeTocPath, // Use relative path (last segment) for URL construction
496+
fullTocPath, // Use full path for identifier registration
497497
parent,
498498
prefixProvider,
499499
children,

‎src/Elastic.Markdown/DocumentationGenerator.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@ await Parallel.ForEachAsync(DocumentationSet.Files, ctx, async (file, token) =>
168168
}
169169

170170
if (processedFiles % 100 == 0)
171-
_logger.LogInformation("-> Processed {ProcessedFiles}/{TotalFileCount} files", processedFiles, totalFileCount);
171+
_logger.LogInformation(" {Name} -> Processed {ProcessedFiles}/{TotalFileCount} files", Context.Git.RepositoryName, processedFiles, totalFileCount);
172172
});
173-
_logger.LogInformation("-> Processed {ProcessedFileCount}/{TotalFileCount} files", processedFileCount, totalFileCount);
173+
_logger.LogInformation(" {Name} -> Processed {ProcessedFileCount}/{TotalFileCount} files", Context.Git.RepositoryName, processedFileCount, totalFileCount);
174174

175175
}
176176

‎src/Elastic.Markdown/IO/DocumentationSet.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,19 +120,20 @@ ICrossLinkResolver linkResolver
120120

121121
private IReadOnlyCollection<INavigationItem> CreateNavigationLookup(INavigationItem item)
122122
{
123+
// warning emit an error again
123124
switch (item)
124125
{
125126
case ILeafNavigationItem<MarkdownFile> markdownLeaf:
126127
var added = MarkdownNavigationLookup.TryAdd(markdownLeaf.Model, markdownLeaf);
127128
if (!added)
128-
Context.EmitError(Configuration.SourceFile, $"Duplicate navigation item {markdownLeaf.Model.CrossLink}");
129+
Context.EmitWarning(Configuration.SourceFile, $"Duplicate navigation item {markdownLeaf.Model.CrossLink}");
129130
return [markdownLeaf];
130131
case ILeafNavigationItem<INavigationModel> leaf:
131132
return [leaf];
132133
case INodeNavigationItem<MarkdownFile, INavigationItem> node:
133134
var addedNode = MarkdownNavigationLookup.TryAdd(node.Index.Model, node);
134135
if (!addedNode)
135-
Context.EmitError(Configuration.SourceFile, $"Duplicate navigation item {node.Index.Model.CrossLink}");
136+
Context.EmitWarning(Configuration.SourceFile, $"Duplicate navigation item {node.Index.Model.CrossLink}");
136137
var nodeItems = node.NavigationItems.SelectMany(CreateNavigationLookup);
137138
return nodeItems.Concat([node]).ToArray();
138139
case INodeNavigationItem<INavigationModel, INavigationItem> node:

‎src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ Cancel ctx
7373
var navigationFileInfo = configurationContext.ConfigurationFileProvider.NavigationFile;
7474
var siteNavigationFile = SiteNavigationFile.Deserialize(await fs.File.ReadAllTextAsync(navigationFileInfo.FullName, ctx));
7575
var documentationSets = assembleSources.AssembleSets.Values.Select(s => s.DocumentationSet.Navigation).ToArray();
76-
var navigation = new SiteNavigation(siteNavigationFile, assembleContext, documentationSets);
76+
var navigation = new SiteNavigation(siteNavigationFile, assembleContext, documentationSets, assembleContext.Environment.PathPrefix);
7777

7878
_logger.LogInformation("Validating navigation.yml does not contain colliding path prefixes");
7979
// this validates all path prefixes are unique, early exit if duplicates are detected

‎src/services/Elastic.Documentation.Assembler/Navigation/LlmsNavigationEnhancer.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text;
77
using Elastic.Documentation.Navigation;
88
using Elastic.Documentation.Navigation.Assembler;
9+
using Elastic.Documentation.Navigation.Isolated;
910
using Elastic.Markdown.IO;
1011
using Elastic.Markdown.Myst.Renderers.LlmMarkdown;
1112

@@ -89,11 +90,15 @@ private static IReadOnlyCollection<INavigationItem> GetFirstLevelChildren(INodeN
8990
INodeNavigationItem<MarkdownFile, INavigationItem> markdownNodeNavigation =>
9091
markdownNodeNavigation.Index.Model.YamlFrontMatter?.Description,
9192

93+
// we only know about MarkdownFiles for now
94+
ILeafNavigationItem<IDocumentationFile> => null,
95+
INodeNavigationItem<IDocumentationFile, INavigationItem> => null,
96+
9297
// API-related navigation items (these don't have markdown frontmatter)
9398
// Check by namespace to avoid direct assembly references
9499
{ } item when item.GetType().FullName?.StartsWith("Elastic.ApiExplorer.", StringComparison.Ordinal) == true => null,
95100

96101
// Throw exception for any unhandled navigation item types
97-
_ => throw new InvalidOperationException($"Unhandled navigation item type: {navigationItem.GetType().FullName}")
102+
_ => throw new InvalidOperationException($"{nameof(LlmsNavigationEnhancer)}.{nameof(GetDescription)}: Unhandled navigation item type: {navigationItem.GetType().FullName}")
98103
};
99104
}

‎tests/Navigation.Tests/Assembler/ComplexSiteNavigationTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void ComplexNavigationWithMultipleNestedTocsAppliesPathPrefixToRootUrls()
5858
var siteContext = SiteNavigationTestFixture.CreateContext(
5959
fileSystem, "/checkouts/current/observability", output);
6060

61-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
61+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
6262

6363
// Verify we have all expected top-level items
6464
siteNavigation.NavigationItems.Should().HaveCount(4);
@@ -134,7 +134,7 @@ public void DeeplyNestedNavigationMaintainsPathPrefixThroughoutHierarchy()
134134
var siteContext = SiteNavigationTestFixture.CreateContext(
135135
fileSystem, "/checkouts/current/platform", output);
136136

137-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
137+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
138138

139139
var platform = siteNavigation.NavigationItems.First() as INodeNavigationItem<INavigationModel, INavigationItem>;
140140
platform.Should().NotBeNull();
@@ -182,7 +182,7 @@ public void FileNavigationLeafUrlsReflectPathPrefixInDeeplyNestedStructures()
182182
var siteContext = SiteNavigationTestFixture.CreateContext(
183183
fileSystem, "/checkouts/current/platform", output);
184184

185-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
185+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
186186

187187
var platform = siteNavigation.NavigationItems.First() as INodeNavigationItem<INavigationModel, INavigationItem>;
188188
platform.Should().NotBeNull();
@@ -239,7 +239,7 @@ public void FolderNavigationWithinNestedTocsHasCorrectPathPrefix()
239239
var siteContext = SiteNavigationTestFixture.CreateContext(
240240
fileSystem, "/checkouts/current/platform", output);
241241

242-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
242+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
243243

244244
var platform = siteNavigation.NavigationItems.First() as INodeNavigationItem<INavigationModel, INavigationItem>;
245245
platform.Should().NotBeNull();

‎tests/Navigation.Tests/Assembler/SiteDocumentationSetsTests.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public void SiteNavigationIntegratesWithDocumentationSets()
9595
// Create site navigation context (using any repository's filesystem)
9696
var siteContext = SiteNavigationTestFixture.CreateContext(fileSystem, "/checkouts/current/observability", output);
9797

98-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
98+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
9999

100100
siteNavigation.Should().NotBeNull();
101101
siteNavigation.NavigationItems.Should().HaveCount(3);
@@ -138,7 +138,7 @@ public void SiteNavigationWithNestedTocs()
138138

139139
var siteContext = SiteNavigationTestFixture.CreateContext(fileSystem, "/checkouts/current/platform", output);
140140

141-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
141+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
142142

143143
siteNavigation.NavigationItems.Should().HaveCount(1);
144144

@@ -203,7 +203,7 @@ public void SiteNavigationWithAllRepositories()
203203
var siteContext = SiteNavigationTestFixture.CreateContext(
204204
fileSystem, "/checkouts/current/observability", output);
205205

206-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
206+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
207207

208208
siteNavigation.Should().NotBeNull();
209209
siteNavigation.NavigationItems.Should().HaveCount(5);
@@ -327,7 +327,7 @@ public void SiteNavigationAppliesPathPrefixToAllUrls()
327327
var documentationSets = new List<IDocumentationSetNavigation> { new DocumentationSetNavigation<IDocumentationFile>(observabilityDocset, observabilityContext, GenericDocumentationFileFactory.Instance) };
328328

329329
var siteContext = SiteNavigationTestFixture.CreateContext(fileSystem, "/checkouts/current/observability", output);
330-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
330+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
331331

332332
// Verify root URL has path prefix
333333
var root = siteNavigation.NavigationItems.First();
@@ -364,7 +364,7 @@ public void SiteNavigationWithNestedTocsAppliesCorrectPathPrefixes()
364364
var documentationSets = new List<IDocumentationSetNavigation> { new DocumentationSetNavigation<IDocumentationFile>(platformDocset, platformContext, GenericDocumentationFileFactory.Instance) };
365365

366366
var siteContext = SiteNavigationTestFixture.CreateContext(fileSystem, "/checkouts/current/platform", output);
367-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
367+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
368368

369369
var platform = siteNavigation.NavigationItems.First() as INodeNavigationItem<INavigationModel, INavigationItem>;
370370
platform.Should().NotBeNull();
@@ -395,14 +395,14 @@ public void SiteNavigationRequiresPathPrefix()
395395
var documentationSets = new List<IDocumentationSetNavigation> { new DocumentationSetNavigation<IDocumentationFile>(observabilityDocset, observabilityContext, GenericDocumentationFileFactory.Instance) };
396396

397397
var siteContext = SiteNavigationTestFixture.CreateContext(fileSystem, "/checkouts/current/observability", output);
398-
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets);
398+
var siteNavigation = new SiteNavigation(siteNavFile, siteContext, documentationSets, sitePrefix: null);
399399

400400
// navigation will still be build
401401
siteNavigation.NavigationItems.Should().NotBeEmpty();
402402

403403
var toc = siteNavigation.NavigationItems.First() as SiteTableOfContentsNavigation<IDocumentationFile>;
404404
toc.Should().NotBeNull();
405-
toc.PathPrefixProvider.PathPrefix.Should().Be("observability"); //constructed from toc URI as fallback
405+
toc.PathPrefixProvider.PathPrefix.Should().Be("/observability"); //constructed from toc URI as fallback, normalized with leading slash
406406
}
407407

408408
[Fact]

0 commit comments

Comments
 (0)