Skip to content

Fix API docs index canonical URLs#347

Merged
PrzemyslawKlys merged 1 commit intomainfrom
codex/api-docs-index-trailing-slash
May 7, 2026
Merged

Fix API docs index canonical URLs#347
PrzemyslawKlys merged 1 commit intomainfrom
codex/api-docs-index-trailing-slash

Conversation

@PrzemyslawKlys
Copy link
Copy Markdown
Member

Summary

  • canonicalize generated API docs landing pages as directory URLs
  • keep API docs index Open Graph/Twitter URLs aligned with sitemap folder routes
  • update focused social metadata coverage

Validation

  • dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj --filter "FullyQualifiedName~WebApiDocsSocialMetaTests" --no-restore
  • C:\Support\GitHub\PSPublishModule\PowerForge.Web.Cli\bin\Debug\net10.0\PowerForge.Web.Cli.exe pipeline --config .\pipeline.deploy.json --mode dev --only build-evotec-xyz,project-apidocs-evotec-xyz,sitemap-evotec-xyz,agent-ready-evotec-xyz,audit-evotec-xyz

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Code Review — PR #347: Fix API docs index canonical URLs

Overview

This PR ensures that API docs index/directory URLs always end with a trailing slash in canonical <link> tags and Open Graph og:url metadata. This is the correct behavior for directory URLs (aligning with how web servers typically resolve them and what sitemaps emit), and the fix is well-scoped.


What's Good

  • Correct separation of concerns: a dedicated NormalizeApiIndexRoute wraps NormalizeApiRoute rather than modifying the original function, which keeps non-index routes (type pages, manifests) unaffected.
  • Both call sites are updated consistently (lines 46 and 144).
  • The test assertions are updated to match the new behavior, and the test exercises the full generation pipeline.
  • Extension-aware logic correctly avoids appending / to file paths (e.g. index.html).

Issues & Suggestions

1. Bug: relative URLs with query strings produce malformed output

var path = normalized;
var queryIndex = path.IndexOfAny(new[] { '?', '#' });
if (queryIndex >= 0)
    path = path[..queryIndex];          // path = "/api"
if (Uri.TryCreate(normalized, UriKind.Absolute, out var absolute))
    path = absolute.AbsolutePath;

return string.IsNullOrWhiteSpace(Path.GetExtension(path))
    ? normalized + "/"                  // ⚠️ still appends to full normalized, not stripped path
    : normalized;

For a relative URL like /api?v=2:

  • Uri.TryCreate fails (not absolute), so path = "/api" — extension check is correct.
  • But the return appends / to normalized (which still contains ?v=2), yielding /api?v=2/ — a malformed URL.

The fix is to append to the stripped path or insert the slash before the query:

return string.IsNullOrWhiteSpace(Path.GetExtension(path))
    ? (queryIndex >= 0 ? normalized[..queryIndex] + "/" + normalized[queryIndex..] : normalized + "/")
    : normalized;

In practice options.BaseUrl likely never contains ?, so this may not manifest today — but it is a latent bug worth closing.

2. Dead check: string.IsNullOrWhiteSpace(normalized) is always false

NormalizeApiRoute never returns null or whitespace — it returns "/" for any null/empty input. The leading guard:

if (string.IsNullOrWhiteSpace(normalized) || normalized.EndsWith("/", StringComparison.Ordinal))
    return normalized;

The first condition is unreachable. You could simplify to just:

if (normalized.EndsWith("/", StringComparison.Ordinal))
    return normalized;

3. Minor: Path.GetExtension on a URL path

System.IO.Path.GetExtension is a file-system API and works correctly here for simple extension detection, but /api.v2 (a directory whose name contains a dot) would incorrectly be treated as having an extension .v2 and would not get the trailing slash. If versioned API bases like /api.v2 are ever used, this could be a silent regression. A URL-aware check (e.g., splitting on / and checking the last segment) would be more robust.

4. Minor: per-call char array allocation

var queryIndex = path.IndexOfAny(new[] { '?', '#' });

This allocates a new char[] on every invocation. A private static readonly char[] QueryOrFragment = { '?', '#' }; field avoids the allocation. Low-priority for non-hot-path code, but a clean habit.


Test Coverage

The existing integration test (Generate_Html_EmitsOpenGraphAndTwitterMeta_ForApiIndex) is updated and covers the happy path end-to-end. A few targeted unit tests for NormalizeApiIndexRoute directly would harden the edge cases without needing full pipeline setup:

Input Expected
null / "" "/"
/api /api/
/api/ /api/ (no double slash)
https://example.test/api https://example.test/api/
https://example.test/api/ https://example.test/api/
/api/index.html /api/index.html (no slash)
/api?v=2 /api/?v=2 (currently produces /api?v=2/)

Summary

The core intent is correct and the change is safe for the project's current URL patterns. The most actionable fix before merging is item #1 (malformed URL for query-string inputs), even if it's a low-probability scenario today. Items #2#4 are polish. Adding a small unit test table for the new function would make future refactors safer.

@PrzemyslawKlys PrzemyslawKlys merged commit d1ba209 into main May 7, 2026
6 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the codex/api-docs-index-trailing-slash branch May 7, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant