Skip to content

Commit 2a3a20a

Browse files
MpdreamzMpdreamz
authored
Remove Fetch from CrossLinkResolver, enforce eager fetching of crosslinks (#1784)
* Remove `Fetch` from CrossLinkResolver, enforce eager fetching of crosslinks. This removes a hidden requirement on `DocumentationSet` that its resolver is not usable until `DocumentationGenerator.GenerateAll()` has been called. We now enforce `DocumentationSet` receives a `ICrossLinkResolver` that is ready to resolve crosslinks. * Found more cases of unhandled navigation types * Remove new caching behavior in DocSetConfigurationCrossLinkFetcher
1 parent 0e0215d commit 2a3a20a

33 files changed

+241
-307
lines changed

‎docs-builder.sln.DotSettings‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
2+
<s:Boolean x:Key="/Default/UserDictionary/Words/=crosslink/@EntryIndexedValue">True</s:Boolean>
23
<s:Boolean x:Key="/Default/UserDictionary/Words/=docset/@EntryIndexedValue">True</s:Boolean>
34
<s:Boolean x:Key="/Default/UserDictionary/Words/=frontmatter/@EntryIndexedValue">True</s:Boolean>
45
<s:Boolean x:Key="/Default/UserDictionary/Words/=linenos/@EntryIndexedValue">True</s:Boolean>

‎src/Elastic.Documentation.Configuration/Builder/TableOfContentsConfiguration.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ file is null && crossLink is null && folder is null && toc is null &&
233233

234234
if (crossLink is not null)
235235
{
236-
return [new CrossLinkReference(this, crossLink, title, hiddenFile, children ?? [])];
236+
if (Uri.TryCreate(crossLink, UriKind.Absolute, out var crossUri) && CrossLinkValidator.IsCrossLink(crossUri))
237+
return [new CrossLinkReference(this, crossUri, title, hiddenFile, children ?? [])];
238+
else
239+
reader.EmitError($"Cross-link '{crossLink}' is not a valid absolute URI format", tocEntry);
237240
}
238241

239242
if (folder is not null)

‎src/Elastic.Documentation.Configuration/TableOfContents/ITocItem.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public interface ITocItem
1414
public record FileReference(ITableOfContentsScope TableOfContentsScope, string RelativePath, bool Hidden, IReadOnlyCollection<ITocItem> Children)
1515
: ITocItem;
1616

17-
public record CrossLinkReference(ITableOfContentsScope TableOfContentsScope, string CrossLinkUri, string? Title, bool Hidden, IReadOnlyCollection<ITocItem> Children)
17+
public record CrossLinkReference(ITableOfContentsScope TableOfContentsScope, Uri CrossLinkUri, string? Title, bool Hidden, IReadOnlyCollection<ITocItem> Children)
1818
: ITocItem;
1919

2020
public record FolderReference(ITableOfContentsScope TableOfContentsScope, string RelativePath, IReadOnlyCollection<ITocItem> Children)

‎src/Elastic.Markdown/DocumentationGenerator.cs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class DocumentationGenerator
4848

4949
public DocumentationSet DocumentationSet { get; }
5050
public BuildContext Context { get; }
51-
public ICrossLinkResolver Resolver { get; }
51+
public ICrossLinkResolver CrossLinkResolver { get; }
5252
public IMarkdownStringRenderer MarkdownStringRenderer => HtmlWriter;
5353

5454
public DocumentationGenerator(
@@ -70,7 +70,7 @@ public DocumentationGenerator(
7070

7171
DocumentationSet = docSet;
7272
Context = docSet.Context;
73-
Resolver = docSet.LinkResolver;
73+
CrossLinkResolver = docSet.CrossLinkResolver;
7474
HtmlWriter = new HtmlWriter(DocumentationSet, _writeFileSystem, new DescriptionGenerator(), navigationHtmlWriter, legacyUrlMapper,
7575
positionalNavigation);
7676
_documentationFileExporter =
@@ -120,9 +120,6 @@ public async Task<GenerationResult> GenerateAll(Cancel ctx)
120120
if (CompilationNotNeeded(generationState, out var offendingFiles, out var outputSeenChanges))
121121
return result;
122122

123-
_logger.LogInformation($"Fetching external links");
124-
_ = await Resolver.FetchLinks(ctx);
125-
126123
await ResolveDirectoryTree(ctx);
127124

128125
await ProcessDocumentationFiles(offendingFiles, outputSeenChanges, ctx);

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

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using Elastic.Documentation.Configuration;
1111
using Elastic.Documentation.Configuration.Builder;
1212
using Elastic.Documentation.Configuration.TableOfContents;
13-
using Elastic.Documentation.LinkIndex;
1413
using Elastic.Documentation.Links;
1514
using Elastic.Documentation.Site.Navigation;
1615
using Elastic.Markdown.Extensions;
@@ -28,6 +27,7 @@ public interface INavigationLookups
2827
IReadOnlyCollection<ITocItem> TableOfContents { get; }
2928
IReadOnlyCollection<IDocsBuilderExtension> EnabledExtensions { get; }
3029
FrozenDictionary<string, DocumentationFile[]> FilesGroupedByFolder { get; }
30+
ICrossLinkResolver CrossLinkResolver { get; }
3131
}
3232

3333
public interface IPositionalNavigation
@@ -94,10 +94,12 @@ public record NavigationLookups : INavigationLookups
9494
public required IReadOnlyCollection<ITocItem> TableOfContents { get; init; }
9595
public required IReadOnlyCollection<IDocsBuilderExtension> EnabledExtensions { get; init; }
9696
public required FrozenDictionary<string, DocumentationFile[]> FilesGroupedByFolder { get; init; }
97+
public required ICrossLinkResolver CrossLinkResolver { get; init; }
9798
}
9899

99100
public class DocumentationSet : INavigationLookups, IPositionalNavigation
100101
{
102+
private readonly ILogger<DocumentationSet> _logger;
101103
public BuildContext Context { get; }
102104
public string Name { get; }
103105
public IFileInfo OutputStateFile { get; }
@@ -112,7 +114,7 @@ public class DocumentationSet : INavigationLookups, IPositionalNavigation
112114

113115
public MarkdownParser MarkdownParser { get; }
114116

115-
public ICrossLinkResolver LinkResolver { get; }
117+
public ICrossLinkResolver CrossLinkResolver { get; }
116118

117119
public TableOfContentsTree Tree { get; }
118120

@@ -135,23 +137,23 @@ public class DocumentationSet : INavigationLookups, IPositionalNavigation
135137
public DocumentationSet(
136138
BuildContext context,
137139
ILoggerFactory logFactory,
138-
ICrossLinkResolver? linkResolver = null,
140+
ICrossLinkResolver linkResolver,
139141
TableOfContentsTreeCollector? treeCollector = null
140142
)
141143
{
144+
_logger = logFactory.CreateLogger<DocumentationSet>();
142145
Context = context;
143146
Source = ContentSourceMoniker.Create(context.Git.RepositoryName, null);
144147
SourceDirectory = context.DocumentationSourceDirectory;
145148
OutputDirectory = context.OutputDirectory;
146-
LinkResolver =
147-
linkResolver ?? new CrossLinkResolver(new ConfigurationCrossLinkFetcher(logFactory, context.Configuration, Aws3LinkIndexReader.CreateAnonymous()));
149+
CrossLinkResolver = linkResolver;
148150
Configuration = context.Configuration;
149151
EnabledExtensions = InstantiateExtensions();
150152
treeCollector ??= new TableOfContentsTreeCollector();
151153

152154
var resolver = new ParserResolvers
153155
{
154-
CrossLinkResolver = LinkResolver,
156+
CrossLinkResolver = CrossLinkResolver,
155157
DocumentationFileLookup = DocumentationFileLookup
156158
};
157159
MarkdownParser = new MarkdownParser(context, resolver);
@@ -184,7 +186,8 @@ public DocumentationSet(
184186
FlatMappedFiles = FlatMappedFiles,
185187
TableOfContents = Configuration.TableOfContents,
186188
EnabledExtensions = EnabledExtensions,
187-
FilesGroupedByFolder = FilesGroupedByFolder
189+
FilesGroupedByFolder = FilesGroupedByFolder,
190+
CrossLinkResolver = CrossLinkResolver
188191
};
189192

190193
Tree = new TableOfContentsTree(Source, Context, lookups, treeCollector, ref fileIndex);
@@ -232,7 +235,7 @@ private void UpdateNavigationIndex(IReadOnlyCollection<INavigationItem> navigati
232235
UpdateNavigationIndex(documentationGroup.NavigationItems, ref navigationIndex);
233236
break;
234237
default:
235-
Context.EmitError(Context.ConfigurationPath, $"Unhandled navigation item type: {item.GetType()}");
238+
Context.EmitError(Context.ConfigurationPath, $"{nameof(DocumentationSet)}.{nameof(UpdateNavigationIndex)}: Unhandled navigation item type: {item.GetType()}");
236239
break;
237240
}
238241
}
@@ -374,26 +377,7 @@ void ValidateExists(string from, string to, IReadOnlyDictionary<string, string?>
374377
return FlatMappedFiles.GetValueOrDefault(relativePath);
375378
}
376379

377-
public async Task ResolveDirectoryTree(Cancel ctx)
378-
{
379-
await Tree.Resolve(ctx);
380-
381-
// Validate cross-repo links in navigation
382-
try
383-
{
384-
await NavigationCrossLinkValidator.ValidateNavigationCrossLinksAsync(
385-
Tree,
386-
LinkResolver,
387-
(msg) => Context.EmitError(Context.ConfigurationPath, msg),
388-
ctx
389-
);
390-
}
391-
catch (Exception e)
392-
{
393-
// Log the error but don't fail the build
394-
Context.EmitError(Context.ConfigurationPath, $"Error validating cross-links in navigation: {e.Message}");
395-
}
396-
}
380+
public async Task ResolveDirectoryTree(Cancel ctx) => await Tree.Resolve(ctx);
397381

398382
private DocumentationFile CreateMarkDownFile(IFileInfo file, BuildContext context)
399383
{
@@ -476,6 +460,7 @@ public RepositoryLinks CreateLinkReference()
476460

477461
public void ClearOutputDirectory()
478462
{
463+
_logger.LogInformation("Clearing output directory {OutputDirectory}", OutputDirectory.Name);
479464
if (OutputDirectory.Exists)
480465
OutputDirectory.Delete(true);
481466
OutputDirectory.Create();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,13 @@ public string Url
121121
{
122122
if (_url is not null)
123123
return _url;
124-
if (_set.LinkResolver.UriResolver is IsolatedBuildEnvironmentUriResolver)
124+
if (_set.CrossLinkResolver.UriResolver is IsolatedBuildEnvironmentUriResolver)
125125
{
126126
_url = DefaultUrlPath;
127127
return _url;
128128
}
129129
var crossLink = new Uri(CrossLink);
130-
var uri = _set.LinkResolver.UriResolver.Resolve(crossLink, DefaultUrlPathSuffix);
130+
var uri = _set.CrossLinkResolver.UriResolver.Resolve(crossLink, DefaultUrlPathSuffix);
131131
_url = uri.AbsolutePath;
132132
return _url;
133133

‎src/Elastic.Markdown/IO/Navigation/CrossLinkNavigationItem.cs‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@ namespace Elastic.Markdown.IO.Navigation;
1111
[DebuggerDisplay("CrossLink: {Url}")]
1212
public record CrossLinkNavigationItem : ILeafNavigationItem<INavigationModel>
1313
{
14-
// Override Url accessor to use ResolvedUrl if available
15-
string INavigationItem.Url => ResolvedUrl ?? Url;
16-
public CrossLinkNavigationItem(string url, string title, DocumentationGroup group, bool hidden = false)
14+
public CrossLinkNavigationItem(Uri crossLinkUri, Uri resolvedUrl, string title, DocumentationGroup group, bool hidden = false)
1715
{
18-
_url = url;
16+
CrossLink = crossLinkUri;
17+
Url = resolvedUrl.ToString();
1918
NavigationTitle = title;
2019
Parent = group;
2120
NavigationRoot = group.NavigationRoot;
@@ -24,14 +23,10 @@ public CrossLinkNavigationItem(string url, string title, DocumentationGroup grou
2423

2524
public INodeNavigationItem<INavigationModel, INavigationItem>? Parent { get; set; }
2625
public IRootNavigationItem<INavigationModel, INavigationItem> NavigationRoot { get; }
27-
// Original URL from the cross-link
28-
private readonly string _url;
2926

30-
// Store resolved URL for rendering
31-
public string? ResolvedUrl { get; set; }
32-
33-
// Implement the INavigationItem.Url property to use ResolvedUrl if available
34-
public string Url => ResolvedUrl ?? _url; public string NavigationTitle { get; }
27+
public Uri CrossLink { get; }
28+
public string Url { get; }
29+
public string NavigationTitle { get; }
3530
public int NavigationIndex { get; set; }
3631
public bool Hidden { get; }
3732
public bool IsCrossLink => true; // This is always a cross-link

‎src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using Elastic.Documentation.Configuration;
88
using Elastic.Documentation.Configuration.TableOfContents;
99
using Elastic.Documentation.Extensions;
10-
using Elastic.Documentation.Links;
1110
using Elastic.Documentation.Site.Navigation;
1211

1312
namespace Elastic.Markdown.IO.Navigation;
@@ -124,13 +123,6 @@ void AddToNavigationItems(INavigationItem item, ref int fileIndex)
124123
{
125124
if (tocItem is CrossLinkReference crossLink)
126125
{
127-
// Validate crosslink URI and title
128-
if (!CrossLinkValidator.IsValidCrossLink(crossLink.CrossLinkUri, out var errorMessage))
129-
{
130-
context.EmitError(context.ConfigurationPath, errorMessage!);
131-
continue;
132-
}
133-
134126
// Validate that cross-link has a title
135127
if (string.IsNullOrWhiteSpace(crossLink.Title))
136128
{
@@ -139,9 +131,13 @@ void AddToNavigationItems(INavigationItem item, ref int fileIndex)
139131
continue;
140132
}
141133

134+
if (!lookups.CrossLinkResolver.TryResolve(msg => context.EmitError(context.ConfigurationPath, msg), crossLink.CrossLinkUri, out var resolvedUrl))
135+
continue; // the crosslink resolver will emit an error already
136+
142137
// Create a special navigation item for cross-repository links
143-
var crossLinkItem = new CrossLinkNavigationItem(crossLink.CrossLinkUri, crossLink.Title, this, crossLink.Hidden);
138+
var crossLinkItem = new CrossLinkNavigationItem(crossLink.CrossLinkUri, resolvedUrl, crossLink.Title, this, crossLink.Hidden);
144139
AddToNavigationItems(crossLinkItem, ref fileIndex);
140+
145141
}
146142
else if (tocItem is FileReference file)
147143
{

‎src/Elastic.Markdown/IO/Navigation/NavigationCrossLinkValidator.cs‎

Lines changed: 0 additions & 86 deletions
This file was deleted.

0 commit comments

Comments
 (0)