-
Notifications
You must be signed in to change notification settings - Fork 32
Fix image URLs #2193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+226
−4
Merged
Fix image URLs #2193
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
afdf5db
Fix image path definition
cotti 3d1e396
Only use CrossLink navigation data on assembler builds
cotti a013b09
Fix acquisition method
cotti ed25466
Apply suggestion from @Mpdreamz
cotti fe8e487
Merge branch 'main' into fix/image_paths
cotti 61a6839
Add image path tests
reakaleek 1171979
rename
reakaleek 945d578
Fix test on windows
reakaleek f5e628e
Merge remote-tracking branch 'refs/remotes/origin/main' into fix/imag…
cotti 68a4df2
Add UrlCombine() to be used in lieu of Path.Combine() for URL path re…
cotti 7250768
Lint suggestion
cotti 9cb0b79
Simplify path generation with UriBuilder
cotti b8dfe22
Remove Path.Combine()
cotti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
208 changes: 208 additions & 0 deletions
208
tests/Elastic.Markdown.Tests/Inline/ImagePathResolutionTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| // 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 System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.IO.Abstractions.TestingHelpers; | ||
| using System.Threading.Tasks; | ||
| using Elastic.Documentation.Configuration; | ||
| using Elastic.Documentation.Navigation; | ||
| using Elastic.Markdown.IO; | ||
| using Elastic.Markdown.Myst; | ||
| using Elastic.Markdown.Myst.InlineParsers; | ||
| using Elastic.Markdown.Tests; | ||
| using FluentAssertions; | ||
| using Xunit; | ||
|
|
||
| namespace Elastic.Markdown.Tests.Inline; | ||
|
|
||
| public class ImagePathResolutionTests(ITestOutputHelper output) | ||
| { | ||
| [Fact] | ||
| public async Task UpdateRelativeUrlUsesNavigationPathWhenAssemblerBuildEnabled() | ||
| { | ||
| const string relativeAssetPath = "images/pic.png"; | ||
| var nonAssemblerResult = await ResolveUrlForBuildMode(relativeAssetPath, assemblerBuild: false, pathPrefix: "this-is-not-relevant"); | ||
| var assemblerResult = await ResolveUrlForBuildMode(relativeAssetPath, assemblerBuild: true, pathPrefix: "platform"); | ||
|
|
||
| nonAssemblerResult.Should().Be("/docs/setup/images/pic.png"); | ||
| assemblerResult.Should().Be("/docs/platform/setup/images/pic.png"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task UpdateRelativeUrlWithoutPathPrefixKeepsGlobalPrefix() | ||
| { | ||
| var relativeAssetPath = "images/funny-image.png"; | ||
| var assemblerResult = await ResolveUrlForBuildMode(relativeAssetPath, assemblerBuild: true, pathPrefix: null); | ||
|
|
||
| assemblerResult.Should().Be("/docs/setup/images/funny-image.png"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task UpdateRelativeUrlAppliesCustomPathPrefix() | ||
| { | ||
| var relativeAssetPath = "images/image.png"; | ||
| var assemblerResult = await ResolveUrlForBuildMode(relativeAssetPath, assemblerBuild: true, pathPrefix: "custom"); | ||
|
|
||
| assemblerResult.Should().Be("/docs/custom/setup/images/image.png"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Resolves a relative asset URL the same way the assembler would for a single markdown file, using the provided navigation path prefix. | ||
| /// </summary> | ||
| private async Task<string> ResolveUrlForBuildMode(string relativeAssetPath, bool assemblerBuild, string? pathPrefix) | ||
| { | ||
| const string guideRelativePath = "setup/guide.md"; | ||
| var navigationUrl = BuildNavigationUrl(pathPrefix, guideRelativePath); | ||
| var files = new Dictionary<string, MockFileData> | ||
| { | ||
| ["docs/docset.yml"] = new( | ||
| $""" | ||
| project: test | ||
| toc: | ||
| - file: index.md | ||
| - file: {guideRelativePath} | ||
| """ | ||
| ), | ||
| ["docs/index.md"] = new("# Home"), | ||
| ["docs/" + guideRelativePath] = new( | ||
| $""" | ||
| # Guide | ||
|
|
||
|  | ||
| """ | ||
| ), | ||
| ["docs/setup/" + relativeAssetPath] = new([]) | ||
| }; | ||
|
|
||
| var fileSystem = new MockFileSystem(files, new MockFileSystemOptions | ||
| { | ||
| CurrentDirectory = Paths.WorkingDirectoryRoot.FullName | ||
| }); | ||
|
|
||
| var collector = new TestDiagnosticsCollector(output); | ||
| _ = collector.StartAsync(TestContext.Current.CancellationToken); | ||
|
|
||
| var configurationContext = TestHelpers.CreateConfigurationContext(fileSystem); | ||
| var buildContext = new BuildContext(collector, fileSystem, configurationContext) | ||
| { | ||
| UrlPathPrefix = "/docs", | ||
| AssemblerBuild = assemblerBuild | ||
| }; | ||
|
|
||
| var documentationSet = new DocumentationSet(buildContext, new TestLoggerFactory(output), new TestCrossLinkResolver()); | ||
|
|
||
| await documentationSet.ResolveDirectoryTree(TestContext.Current.CancellationToken); | ||
|
|
||
| // Normalize path for cross-platform compatibility (Windows uses backslashes) | ||
| var normalizedPath = guideRelativePath.Replace('/', Path.DirectorySeparatorChar); | ||
| if (documentationSet.TryFindDocumentByRelativePath(normalizedPath) is not MarkdownFile markdownFile) | ||
| throw new InvalidOperationException($"Failed to resolve markdown file for test. Tried path: {normalizedPath}"); | ||
|
|
||
| // For assembler builds DocumentationSetNavigation seeds MarkdownNavigationLookup with navigation items whose Url already | ||
| // includes the computed path_prefix. To exercise the same branch in isolation, inject a stub navigation entry with the | ||
| // expected Url (and minimal metadata for the surrounding API contract). | ||
| _ = documentationSet.MarkdownNavigationLookup.Remove(markdownFile); | ||
| documentationSet.MarkdownNavigationLookup.Add(markdownFile, new NavigationItemStub(navigationUrl)); | ||
| documentationSet.MarkdownNavigationLookup.TryGetValue(markdownFile, out var navigation).Should() | ||
| .BeTrue("navigation lookup should contain current page"); | ||
| navigation?.Url.Should().Be(navigationUrl); | ||
|
|
||
| var parserState = new ParserState(buildContext) | ||
| { | ||
| MarkdownSourcePath = markdownFile.SourceFile, | ||
| YamlFrontMatter = null, | ||
| CrossLinkResolver = documentationSet.CrossLinkResolver, | ||
| TryFindDocument = file => documentationSet.TryFindDocument(file), | ||
| TryFindDocumentByRelativePath = path => documentationSet.TryFindDocumentByRelativePath(path), | ||
| PositionalNavigation = documentationSet | ||
| }; | ||
|
|
||
| var context = new ParserContext(parserState); | ||
| context.TryFindDocument(context.MarkdownSourcePath).Should().BeSameAs(markdownFile); | ||
| context.Build.AssemblerBuild.Should().Be(assemblerBuild); | ||
|
|
||
| var resolved = DiagnosticLinkInlineParser.UpdateRelativeUrl(context, relativeAssetPath); | ||
|
|
||
| await collector.StopAsync(TestContext.Current.CancellationToken); | ||
|
|
||
| return resolved; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Helper that mirrors the assembler's path-prefix handling in <c>DocumentationSetNavigation</c>: | ||
| /// combines the relative <c>path_prefix</c> from navigation.yml with the markdown path (stripped of ".md") so our stub | ||
| /// navigation item carries the same Url the production code would have provided. | ||
| /// </summary> | ||
| private static string BuildNavigationUrl(string? pathPrefix, string docRelativePath) | ||
| { | ||
| var docPath = docRelativePath.Replace('\\', '/').Trim('/'); | ||
| if (docPath.EndsWith(".md", StringComparison.OrdinalIgnoreCase)) | ||
| docPath = docPath[..^3]; | ||
|
|
||
| var segments = new List<string>(); | ||
| if (!string.IsNullOrWhiteSpace(pathPrefix)) | ||
| segments.Add(pathPrefix.Trim('/')); | ||
| if (!string.IsNullOrWhiteSpace(docPath)) | ||
| segments.Add(docPath); | ||
|
|
||
| var combined = string.Join('/', segments); | ||
| return "/" + combined.Trim('/'); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Minimal navigation stub so UpdateRelativeUrl can rely on navigation metadata without constructing the full site navigation tree. | ||
| /// </summary> | ||
| private sealed class NavigationItemStub(string url) : INavigationItem | ||
| { | ||
| private sealed class NavigationModelStub : INavigationModel | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Simplified root navigation item to satisfy the IRootNavigationItem contract. | ||
| /// </summary> | ||
| private sealed class RootNavigationItemStub : IRootNavigationItem<INavigationModel, INavigationItem> | ||
| { | ||
| /// <summary> | ||
| /// Leaf implementation used by the root stub. Navigation requires both root and leaf nodes present. | ||
| /// </summary> | ||
| private sealed class LeafNavigationItemStub(RootNavigationItemStub root) : ILeafNavigationItem<INavigationModel> | ||
| { | ||
| public string Url => "/"; | ||
| public string NavigationTitle => "Root"; | ||
| public IRootNavigationItem<INavigationModel, INavigationItem> NavigationRoot { get; } = root; | ||
| public INodeNavigationItem<INavigationModel, INavigationItem>? Parent { get; set; } | ||
| public bool Hidden => false; | ||
| public int NavigationIndex { get; set; } | ||
| public INavigationModel Model { get; } = new NavigationModelStub(); | ||
| } | ||
|
|
||
| public RootNavigationItemStub() => Index = new LeafNavigationItemStub(this); | ||
|
|
||
| public string Url => "/"; | ||
| public string NavigationTitle => "Root"; | ||
| public IRootNavigationItem<INavigationModel, INavigationItem> NavigationRoot => this; | ||
| public INodeNavigationItem<INavigationModel, INavigationItem>? Parent { get; set; } | ||
| public bool Hidden => false; | ||
| public int NavigationIndex { get; set; } | ||
| public string Id => "root"; | ||
| public ILeafNavigationItem<INavigationModel> Index { get; } | ||
| public IReadOnlyCollection<INavigationItem> NavigationItems { get; private set; } = []; | ||
| public bool IsUsingNavigationDropdown => false; | ||
| public Uri Identifier => new("https://example.test/"); | ||
| public void SetNavigationItems(IReadOnlyCollection<INavigationItem> navigationItems) => NavigationItems = navigationItems; | ||
| } | ||
|
|
||
| private static readonly RootNavigationItemStub Root = new(); | ||
|
|
||
| public string Url { get; } = url; | ||
| public string NavigationTitle => "Stub"; | ||
| public IRootNavigationItem<INavigationModel, INavigationItem> NavigationRoot => Root; | ||
| public INodeNavigationItem<INavigationModel, INavigationItem>? Parent { get; set; } | ||
| public bool Hidden => false; | ||
| public int NavigationIndex { get; set; } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.