-
Notifications
You must be signed in to change notification settings - Fork 352
DOC-6909 Teach the feed pipeline and version archiver the /content/ link form #3796
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Test script for version_archiver's link versioning. | ||
|
|
||
| When a version is archived, intra-product links must point at the frozen copy, | ||
| not at latest. The archiver originally rewrote only `relref`, so a section | ||
| migrated to plain Markdown links (DOC-6909) was silently left pointing at | ||
| latest -- wrong content in an archived version, with no error or warning. | ||
|
|
||
| These tests cover both notations, the guards they share, and the forms that must | ||
| NOT be touched. | ||
| """ | ||
|
|
||
| import os | ||
| import sys | ||
| import tempfile | ||
|
|
||
| # Add the build directory to the path | ||
| sys.path.insert(0, os.path.dirname(__file__)) | ||
|
|
||
| from version_archiver import VersionArchiver | ||
|
|
||
|
|
||
| def archive(product, version, page_relpath, content): | ||
| """Run the real version_relrefs() over one page in an isolated tree. | ||
|
|
||
| page_relpath is relative to the versioned directory, so nesting can be | ||
| realistic -- it matters for source-relative links, which resolve against the | ||
| page's own location. | ||
| """ | ||
| cwd = os.getcwd() | ||
| with tempfile.TemporaryDirectory() as tmp: | ||
| arch_cwd = tmp | ||
| os.chdir(arch_cwd) | ||
| try: | ||
| archiver = VersionArchiver(product, version) | ||
| page = os.path.join(archiver.new_directory, page_relpath) | ||
| os.makedirs(os.path.dirname(page), exist_ok=True) | ||
| with open(page, "w") as f: | ||
| f.write(content) | ||
| archiver.version_relrefs() | ||
| with open(page) as f: | ||
| return f.read() | ||
| finally: | ||
| os.chdir(cwd) | ||
|
|
||
|
|
||
| DEEP = os.path.join("databases", "configure", "page.md") | ||
|
|
||
|
|
||
| def test_relref_is_versioned(): | ||
| """The original behaviour: an intra-product relref gains the version.""" | ||
| out = archive("rs", "9.9", DEEP, | ||
| '[a]({{< relref "/operate/rs/databases/memory/eviction" >}})') | ||
| assert "/operate/rs/9.9/databases/memory/eviction" in out, out | ||
| print("✓ relref link is versioned") | ||
|
|
||
|
|
||
| def test_plain_content_link_is_versioned(): | ||
| """DOC-6909's repo-root-relative form must be versioned the same way.""" | ||
| out = archive("rs", "9.9", DEEP, | ||
| '[b](/content/operate/rs/databases/memory/eviction.md)') | ||
| assert "](/content/operate/rs/9.9/databases/memory/eviction.md)" in out, out | ||
| print("✓ plain /content/ link is versioned") | ||
|
|
||
|
|
||
| def test_plain_content_link_keeps_anchor(): | ||
| """An anchor must survive versioning.""" | ||
| out = archive("rs", "9.9", DEEP, | ||
| '[c](/content/operate/rs/databases/memory/eviction.md#policies)') | ||
| assert "/operate/rs/9.9/databases/memory/eviction.md#policies" in out, out | ||
| print("✓ anchor preserved when versioning a plain link") | ||
|
|
||
|
|
||
| def test_source_relative_link_is_left_alone(): | ||
| """Source-relative links need no rewriting and must not be touched. | ||
|
|
||
| The whole subtree is copied, so a link between two pages inside it already | ||
| resolves within the versioned directory. | ||
| """ | ||
| link = '[d](../memory/eviction.md)' | ||
| out = archive("rs", "9.9", DEEP, link) | ||
| assert out == link, out | ||
| # and confirm the claim: it resolves inside the frozen tree | ||
| page_dir = os.path.join("content", "operate", "rs", "9.9", | ||
| os.path.dirname(DEEP)) | ||
| resolved = os.path.normpath(os.path.join(page_dir, "../memory/eviction.md")) | ||
| assert resolved.startswith(os.path.join("content", "operate", "rs", "9.9")), resolved | ||
| print("✓ source-relative link untouched, and resolves inside the version") | ||
|
|
||
|
|
||
| def test_release_notes_are_exempt(): | ||
| """Release notes are deliberately not versioned, in either notation.""" | ||
| both = ('[e]({{< relref "/operate/rs/release-notes/rs-7-8" >}})\n' | ||
| '[f](/content/operate/rs/release-notes/rs-7-8.md)') | ||
| out = archive("rs", "9.9", DEEP, both) | ||
| assert out == both, out | ||
| print("✓ release-notes links exempt in both notations") | ||
|
|
||
|
|
||
| def test_already_versioned_is_idempotent(): | ||
| """Re-running must not double-version an already-versioned link.""" | ||
| both = ('[g]({{< relref "/operate/rs/9.9/databases/memory/eviction" >}})\n' | ||
| '[h](/content/operate/rs/9.9/databases/memory/eviction.md)') | ||
| out = archive("rs", "9.9", DEEP, both) | ||
| assert out == both, out | ||
| assert "9.9/9.9" not in out, out | ||
| print("✓ already-versioned links are left alone (idempotent)") | ||
|
|
||
|
|
||
| def test_other_product_and_external_urls_untouched(): | ||
| """Only the product being archived is rewritten, and external URLs are safe. | ||
|
|
||
| The GitHub blob URL is the important one: it contains the substring | ||
| '/content/operate/rs/', so the pattern must anchor on a link destination | ||
| ('](/content/...') rather than matching anywhere in the line. | ||
| """ | ||
| content = ('[i](/content/operate/kubernetes/deploy/quickstart.md)\n' | ||
| '[j](https://github.com/redis/docs/blob/main/content/operate/rs/x.md)\n' | ||
| '[k]({{< relref "/develop/data-types/hashes" >}})') | ||
| out = archive("rs", "9.9", DEEP, content) | ||
| assert out == content, out | ||
| print("✓ other products, external URLs and other sections untouched") | ||
|
|
||
|
|
||
| def test_other_products_use_their_own_prefix(): | ||
| """The pattern is parameterised, so non-'operate' products work too.""" | ||
| out = archive("redis-data-integration", "1.20", DEEP, | ||
| '[l](/content/integrate/redis-data-integration/reference/config.md)') | ||
| assert "/integrate/redis-data-integration/1.20/reference/config.md" in out, out | ||
| print("✓ redis-data-integration (integrate prefix) is versioned") | ||
|
|
||
|
|
||
| def main(): | ||
| tests = [ | ||
| test_relref_is_versioned, | ||
| test_plain_content_link_is_versioned, | ||
| test_plain_content_link_keeps_anchor, | ||
| test_source_relative_link_is_left_alone, | ||
| test_release_notes_are_exempt, | ||
| test_already_versioned_is_idempotent, | ||
| test_other_product_and_external_urls_untouched, | ||
| test_other_products_use_their_own_prefix, | ||
| ] | ||
| try: | ||
| for t in tests: | ||
| t() | ||
| print("\n✅ All tests passed!") | ||
| return 0 | ||
| except AssertionError as e: | ||
| print(f"\n❌ Test failed: {e}") | ||
| return 1 | ||
| except Exception as e: | ||
| print(f"\n❌ Unexpected error: {e}") | ||
| import traceback | ||
| traceback.print_exc() | ||
| return 1 | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| sys.exit(main()) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,30 +16,6 @@ | |
| {{- $content := .RawContent -}} | ||
| {{- $visited := .Visited | default (slice) -}} | ||
|
|
||
| {{- /* Drop HTML-comment blocks before anything else looks at the content. | ||
|
|
||
| Hugo is configured with unsafe = true, so a comment passes through to the rendered | ||
| HTML and is invisible to readers -- which is how authors park prose that should not | ||
| be published yet. Nothing downstream of here knew that, so the commented text was | ||
| reaching the JSON feed as real section content and the Markdown output as real body | ||
| text: we were publishing to AI consumers exactly what we withhold from readers. On | ||
| develop/clients/observability that was a whole "Tracing overview" section, kept | ||
| because the explanation is good but the clients do not support tracing yet. | ||
|
|
||
| The open delimiter must be at the start of a line. That is what separates an author's | ||
| block comment from a comment inside a code example -- a Maven snippet carries | ||
| "</version> <!-- Check for the latest version -->" mid-line, and stripping that would | ||
| damage the sample. Measured over the whole corpus, the line-anchored form removes 48 | ||
| comments totalling about 19,200 characters and never matches inside a fenced code | ||
| block, where the unanchored form would have hit 9. The residual is inline comments, | ||
| which are small and usually part of a code sample anyway. | ||
|
|
||
| Known limit: a block comment written at column 0 *inside* a fenced code block would | ||
| still be stripped. None exists today. Both the literal and entity-escaped forms are | ||
| matched, because RawContent arrives escaped in some contexts. */ -}} | ||
| {{- $content = $content | replaceRE "(?ms)^<!--.*?-->" "" -}} | ||
| {{- $content = $content | replaceRE "(?ms)^<!--.*?-->" "" -}} | ||
|
|
||
| {{- /* Expand embed-md shortcodes before other transforms so embedded content can be processed too. */ -}} | ||
| {{- $content = partial "markdown-embed-md.html" (dict "RawContent" $content "Page" .Page "Visited" $visited) -}} | ||
| {{- /* Split wide table-scrollable blocks for AI-facing Markdown output only. */ -}} | ||
|
|
@@ -60,6 +36,18 @@ | |
| {{- /* Pattern for literal: {{< relref "path" >}} - capture optional leading slash */ -}} | ||
| {{- $content = $content | replaceRE `\{\{<\s*relref\s+"/?([^"]+)"\s*>\}\}` "https://redis.io/docs/latest/$1" -}} | ||
|
|
||
| {{- /* Fix repo-root-relative Markdown links (`](/content/...)`), which replace relref | ||
| in sections migrated for DOC-6909. Without this they reach the AI-facing | ||
| Markdown/JSON output as raw source paths, which resolve nowhere on the | ||
| published site. Rewritten to the same absolute form as relref above. | ||
| Section/leaf-bundle forms go first so the index segment is dropped rather | ||
| than left in the URL; `/index.md` mirrors the render-link hook, which | ||
| strips it, even though no content currently uses that form. A trailing | ||
| `#anchor` is outside each match and so survives untouched. */ -}} | ||
| {{- $content = $content | replaceRE `\]\(/content/([^)#]*?)/_index\.md` "](https://redis.io/docs/latest/$1" -}} | ||
| {{- $content = $content | replaceRE `\]\(/content/([^)#]*?)/index\.md` "](https://redis.io/docs/latest/$1" -}} | ||
| {{- $content = $content | replaceRE `\]\(/content/([^)#]*?)\.md` "](https://redis.io/docs/latest/$1" -}} | ||
|
|
||
| {{- /* Fix images - handle both HTML-escaped entities and literal characters */ -}} | ||
| {{- /* Pattern for HTML-escaped: {{< image filename="path" [alt="..."] >}} */ -}} | ||
| {{- $content = $content | replaceRE "\\{\\{<\\s*image\\s+filename="/?([^&]+)"[^}]*>\\}\\}" "" -}} | ||
|
|
@@ -99,26 +87,8 @@ | |
| {{- $content = $content | replaceRE "'" "'" -}} | ||
| {{- $content = $content | replaceRE "+" "+" -}} | ||
|
|
||
| {{- /* Expand table-children shortcodes into Markdown tables. Runs after the unescape | ||
| above (it matches literal {{< ... >}}) and before the strip below, which would | ||
| otherwise discard the table -- taking the whole body of every release-notes | ||
| index page with it. */ -}} | ||
| {{- $content = partial "markdown-table-children.html" (dict "RawContent" $content "Page" .Page) -}} | ||
| {{- /* Unescape again, as Hugo re-escapes partial output. Must cover the same entities | ||
| as the block above, including +, or a "+" in a table cell (a version column | ||
| such as "6.0+", say) reaches the feed as a literal entity. */ -}} | ||
| {{- $content = $content | replaceRE """ "\"" -}} | ||
| {{- $content = $content | replaceRE """ "\"" -}} | ||
| {{- $content = $content | replaceRE "'" "'" -}} | ||
| {{- $content = $content | replaceRE "<" "<" -}} | ||
| {{- $content = $content | replaceRE ">" ">" -}} | ||
| {{- $content = $content | replaceRE "&" "&" -}} | ||
| {{- $content = $content | replaceRE "+" "+" -}} | ||
|
|
||
| {{- /* Remove remaining shortcodes AFTER unescape (content now has literal < and >) */ -}} | ||
| {{- /* Match non-greedily to the first ">}}": a [^>]* class is defeated by a ">" inside | ||
| an attribute value (e.g. columnNames="...<br/>...") and leaks the raw shortcode. */ -}} | ||
| {{- $content = $content | replaceRE `(?s)\{\{<\s*/?.*?>\}\}` "" -}} | ||
| {{- $content = $content | replaceRE `\{\{<\s*/?[^>]*>\}\}` "" -}} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Table-children expansion droppedHigh Severity The call to Reviewed by Cursor Bugbot for commit c87015c. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shortcode strip regex regressesHigh Severity The remaining-shortcode remover changed from a non-greedy match to the first Reviewed by Cursor Bugbot for commit c87015c. Configure here. |
||
| {{- $content = $content | replaceRE `\{\{%\s*/?[^%]*%\}\}` "" -}} | ||
|
|
||
| {{- return $content -}} | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTML comment stripping removed
High Severity
This commit drops the line-anchored HTML-comment strip from
process-markdown-content.html, so draft blocks authors park in<!-- -->now flow into the AI Markdown/JSON feed as published body text.toc-from-markdown.htmlstill strips those comments and documents that the two must stay aligned, so TOC and body also diverge.Reviewed by Cursor Bugbot for commit c87015c. Configure here.