Skip to content

[#1220] Preserve documentation versions in PDF links - #387

Merged
vharseko merged 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix/pdf-documentation-versions
Nov 6, 2026
Merged

vharseko merged 3 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix/pdf-documentation-versions

Conversation

@vharseko

@vharseko vharseko commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

Cross-guide PDF links currently use the unversioned documentation site, so a historical release can send readers to the current reference. Add explicit docVersion and independent componentDocVersions settings. Explicit Antora versions in xrefs that name a module take precedence; fragments, component landing pages, and current-version defaults are preserved.

Antora's explicit versionless token _ selects the unversioned site, while the invalid empty @ form is left unchanged. Null and empty component-map entries override the project version and select unversioned links. Configuration values remain literal published site paths.

docVersion is available through plugin configuration and the Maven property, including project properties inherited from a parent POM. It applies to this project's component even when the xref names that component explicitly. This deliberately keeps archived self-links in the PDF on the configured version; Antora resolves a component-qualified xref without an explicit version to the latest component version, so its HTML link can differ.

Version-only resource IDs such as xref:15.1@page.adoc[Older] remain unsupported by the PDF converter and are left unchanged. The README documents this limitation and the explicit-module form to use instead, including in shared partials.

Document the archive workflow: publish and verify the historical site paths, then rebuild the release PDF using a doc-maven-plugin release that supports these settings. Versions 3.2.2 and earlier do not support them; release POMs that pin the plugin, including OpenDJ 4.10.2 and 5.2.0, must select a supporting plugin version for the rebuild. Previously distributed PDFs require replacement. The README's explicit-version example uses the published OpenDJ 4.10 path.

Validation at this head:

  • mvn -o -Dmaven.repo.local=<isolated-cache> -pl commons/doc-maven-plugin -am test on JDK 17: 547 tests passed, with no failures, errors, or skips; includes 20 PDF converter tests and five Maven configuration tests. The build used an isolated writable Maven cache.
  • Configuration tests execute source preparation with POM versions, an empty POM map entry, Maven's docVersion user property, and an absent component map, asserting the resulting links. A separate test checks that the generated plugin descriptor declares componentDocVersions.
  • Earlier isolated mutation checks confirmed that removing @Parameter fails the descriptor test and removing the map initializer fails the absent-map test. The existing tests passed under both mutations.
  • Regression tests reproduced the invalid-empty-version and versionless-token defects before the parser fix.
  • git diff --check passes. Full PDF rendering and site builds were not rerun for this follow-up.

Earlier integration validation: all seven generated current/historical site URLs returned HTTP 200, with the expected fragments present. Those seven URL and fragment checks were not repeated for this follow-up.

The companion wiki section-link migration is published in wiki commit 897f541.

Fixes OpenIdentityPlatform/OpenAM#1220

@vharseko vharseko added bug documentation Documentation and Javadoc changes tests Test code changes java Pull requests that update java code labels Nov 6, 2026
@vharseko
vharseko requested a review from maximthomas November 6, 2026 10:31

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: The version selection is scoped the right way: a configured version never leaks to another component.

  • targetComponent.equals(component) ? docVersion : null (AsciidocToPdfMojo.java:264) keeps docVersion to this project's component, pinned by testOtherComponentDoesNotInheritProjectVersion.
  • containsKey (AsciidocToPdfMojo.java:262) is the right test for an empty entry: Maven 3.9.16 binds <openam/> as openam -> null, and that stays unversioned.
  • The 4-argument preparePdfSource / convertXrefsForPdf overloads delegate with null / emptyMap(), so existing callers keep the unversioned links.

issue (blocking): Antora's versionless token _ in an explicit resource ID becomes a literal /_/ path segment.

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:191, :252, :266

Antora (3.1.15, the site's pin) spells the versionless version _: parse-resource-id.js maps _@opendj::index.adoc to version "", and the HTML page links to /opendj/index.html. Here explicitVersion = "_" (:252) gives versionPrefix = "_/" (:266), so the PDF gets <site>/opendj/_/index.html. The site never publishes a _ directory: tag versions are <major>.<minor> (versions-from-tags.js) and the branch is unversioned. The link is a 404.

private static final Pattern RESOURCE_ID_PATTERN =
        Pattern.compile("(?:([^:/@$]+)@)?(?:([^:/@$]+):)?([^:/@$]*):([^:@$]+)");
// ...
explicitVersion = id.group(1);
// Antora spells the versionless component version "_"
if ("_".equals(explicitVersion)) {
    explicitVersion = "";
}

The Javadoc at :201-202 changes to match: "an explicit _ version selects the unversioned site". This rule covers explicit versions only. The docVersion / componentDocVersions values come from the POM, not from Antora syntax, so they keep the empty-value rule.


issue (blocking): The README and the test document xref:@opendj::index.adoc[Current] as the explicit unversioned target, and Antora rejects that resource ID.

commons/doc-maven-plugin/README.md:804-806, commons/doc-maven-plugin/src/test/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojoTest.java:87, :92

The same .adoc sources feed the antora goal. AntoraMojo.convertXrefsToAntora (AntoraMojo.java:150-162) only rewrites ../, ./ and /, so @opendj::index.adoc reaches Antora unchanged. Antora's RESOURCE_ID_RX needs a non-empty version before @, so parseResourceId returns undefined on 3.1.9 and 3.1.15. The HTML page renders an unresolved xref and logs "target of xref not found", while the PDF link works. An author who follows the README breaks the site page.

README :804-805:

An empty map entry or Antora's versionless token `_` (`xref:_@opendj::index.adoc[Current]`)
selects the unversioned site.

Test: at :87, change xref:@opendj::index.adoc[Current] to xref:_@opendj::index.adoc[Current]. The expected value at :92 stays the same. After the pattern change above, also pin the invalid form:

assertThat(AsciidocToPdfMojo.convertXrefsForPdf("xref:@opendj::index.adoc[Current]",
        SITE, "openam", "how-to", "15.2", Collections.emptyMap()))
        .isEqualTo("xref:@opendj::index.adoc[Current]");

suggestion (non-blocking): No test passes an empty or null componentDocVersions value, so the README's "empty map entry selects the unversioned site" is unpinned.

commons/doc-maven-plugin/src/test/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojoTest.java:70, :83-84, :113, :135

Maven binds <openam/>, <openam></openam> and a blank value as openam -> null. Two mutants survive all 16 tests: get(...) != null instead of containsKey at AsciidocToPdfMojo.java:262, and a fallback to docVersion for "". Both produce /openam/15.2/... for an entry that should be unversioned.

@Test
public void testEmptyComponentEntrySelectsUnversionedSite() {
    Map<String, String> versions = new HashMap<>();
    versions.put("openam", null); // what Maven binds for <openam/>
    versions.put("opendj", "");
    assertThat(AsciidocToPdfMojo.convertXrefsForPdf(
            "xref:reference:page.adoc[A] xref:opendj::index.adoc[B]",
            SITE, "openam", "how-to", "15.2", versions)).isEqualTo(
            "link:" + SITE + "/openam/reference/page.html[A] "
                    + "link:" + SITE + "/opendj/index.html[B]");
}

Pin: this case fails for both mutants.


issue (non-blocking): An explicit version on a page in the same module, xref:4.9@page.adoc[...], is not converted.

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:191, commons/doc-maven-plugin/README.md:798-799

Antora accepts this form (current component and module, version 4.9). RESOURCE_ID_PATTERN needs a : before the page, so the xref is left as it is, and the PDF gets an xref to a document that does not exist. This contradicts the README rule that an explicit Antora version takes precedence. The behaviour is the same as at the base, and no consumer source uses the form today.

Or: narrow the README rule to resource IDs that name a module, and add a test for xref:4.9@page.adoc[x] that shows the chosen behaviour.


suggestion (non-blocking): No test exercises the Maven wiring of docVersion and componentDocVersions. The execute test sets both fields by reflection.

commons/doc-maven-plugin/src/test/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojoTest.java:133-150

setParameter uses Field.setAccessible. If the docVersion property is renamed or @Parameter is removed from componentDocVersions, every test stays green while -DdocVersion=15.2 and the POM map are silently ignored. AntoraMojoTest:46-51 already drives a mojo from a test POM. Add a POM under src/test/resources/asciidoc-to-pdf/pom.xml, copied from antora/pom.xml, with <docVersion>15.2</docVersion> and <componentDocVersions><opendj>4.10</opendj><openam/></componentDocVersions>, and this test in an AbstractMojoTestCase subclass (not run by this review):

@Test
public void testPomConfigurationReachesVersionParameters() throws Exception {
    File pom = getTestFile("src/test/resources/asciidoc-to-pdf/pom.xml");
    AsciidocToPdfMojo mojo = (AsciidocToPdfMojo) lookupMojo("asciidoc-to-pdf", pom);
    configureMojo(mojo, "doc-maven-plugin", pom);
    assertThat(getVariableValueFromObject(mojo, "docVersion")).isEqualTo("15.2");
    @SuppressWarnings("unchecked")
    Map<String, String> versions = (Map<String, String>) getVariableValueFromObject(mojo, "componentDocVersions");
    assertThat(versions).containsEntry("opendj", "4.10").containsKey("openam");
}

Pin: renaming the property or dropping @Parameter makes this test fail.


nitpick (non-blocking): The new section comes after the README's license footer.

commons/doc-maven-plugin/README.md:762, :745-759

It follows the * * * rule, the CDDL notice and "Copyright 2012-2015 ForgeRock AS.", so it renders below the license block and nests under "# Alternative Branding & Common Content" (:677). Move it above :745, either as a # chapter or under "# ForgeRock Doc Maven Plugin Features".


nitpick (non-blocking): The README adds 57 lines but no "Portions copyright 2026 3A Systems, LLC." line.

commons/doc-maven-plugin/README.md:754-759

The footer's own CDDL notice asks for this line. No other README in the repo has one, so leaving it out follows the existing practice.

@vharseko

vharseko commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Addressed the review in 141d137ec.

  • Explicit _ in an Antora resource ID now selects the unversioned site. Normalization applies only to the explicit xref version; POM configuration values remain literal site paths.
  • The resource-ID pattern now requires a non-empty version before @. README and the precedence test use _@, and a regression test keeps invalid empty-version forms unchanged.
  • Added coverage for both null and empty component-map values, including an override for this project's component when docVersion is set. containsKey is preserved.
  • For version@page.adoc, chose the documented-limit alternative from the review: it remains unchanged, with a test and an explicit-module workaround in the README. Resolving an omitted module inside a shared partial requires the including page's context, which the file-by-file converter does not have.
  • Added a test POM and three configuration tests using AbstractMojoTestCase. They execute source preparation and assert the resulting links for POM versions, <openam/>, and Maven's docVersion user property. The user-property case removes the POM value first, so it exercises the property mapping separately.
  • Moved the README section above the existing footer as a top-level chapter.

Validation: mvn -o -pl commons/doc-maven-plugin -am test passes all 545 tests, including 20 converter tests and three configuration tests, with no failures, errors, or skips. git diff --check passes. Full PDF rendering, site builds, and the earlier HTTP/fragment integration checks were not rerun for this review round.

@vharseko
vharseko requested a review from maximthomas November 6, 2026 11:57
vharseko added a commit to OpenIdentityPlatform/OpenAM that referenced this pull request Nov 6, 2026
Remove the deferred wiki migration patch and its application README
after publishing the migration to the wiki. The migrated articles link
to the documentation site while retaining 413 incoming section
destinations, including duplicate-heading suffixes and the Russian
Yandex Cloud alias.

Validation: the wiki migration passes forward/reverse applicability
checks at `c20b4de45f8453d9fa538737cddf8fc019bf49f8`; all 408 unique
destinations on 54 published How-To pages were checked. The remaining
eight OpenAM documentation checker tests pass, and `git diff --check` is
clean.

Wiki migration: [published commit
`897f541`](https://github.com/OpenIdentityPlatform/OpenAM/wiki/_compare/c20b4de...897f541).

Related to #1220. The PDF converter follow-up is implemented in [commons
#387](OpenIdentityPlatform/commons#387), which
carries the closing reference for the issue.

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: The parser fix is narrow and its regression rows really pin it.

  • _ is normalised only for the explicit xref version (AsciidocToPdfMojo.java:256-258), so configured values stay literal site paths (testConfiguredVersionsAreLiteralSitePaths).
  • With AsciidocToPdfMojo.java reverted to ee42bdc and the head's tests, exactly testEmptyExplicitVersionIsNotAnAntoraResourceId and testExplicitVersionOverridesConfiguredComponentVersion go red (18/20 stay green); at the head the class is 20/20.
  • testMavenUserPropertySelectsHistoricalVersion resolves docVersion through the plugin descriptor, so it pins the property name.

issue (non-blocking): The configuration tests do not pin the @Parameter annotation on componentDocVersions.

commons/doc-maven-plugin/src/test/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojoConfigurationTest.java:52-89, commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:95-96

lookupConfiguredMojo (harness 3.3.0) applies the raw <plugin><configuration> and sets fields by name, without checking the descriptor; Maven 3.9.16 (DefaultLifecycleExecutionPlanCalculator.finalizeMojoConfiguration) keeps only the children the descriptor declares. With @Parameter removed, the generated plugin.xml has no componentDocVersions and all three configuration tests stay green (3/3), while a real build drops the POM map and every other component's links go unversioned. The description says these tests cover POM versions; for the map they do not.

public void testComponentDocVersionsIsADescriptorParameter() throws Exception {
    assertTrue(newMojoExecution("asciidoc-to-pdf").getMojoDescriptor()
            .getParameterMap().containsKey("componentDocVersions"));
}

Pin: the case reads the generated descriptor, so removing the annotation turns it red (MojoDescriptor.getParameterMap() exists in maven-plugin-api 3.9.16; the case itself was not compiled in this review).


suggestion (non-blocking): No test leaves componentDocVersions unconfigured, so its = Collections.emptyMap() default is unpinned.

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:96, :268

Every case supplies the map: the test POM always keeps <componentDocVersions>, the execute test sets it by reflection, the 4-argument overloads pass emptyMap(). With the initializer removed, the module's tests stay 23/23 green, but a POM without the element — every current consumer — gets a null field and an NPE at componentDocVersions.containsKey on the first xref without an explicit version.

public void testAbsentMapKeepsOtherComponentsUnversioned() throws Exception {
    MavenProject project = readProject();
    Xpp3Dom configuration = (Xpp3Dom) project.getBuildPlugins().get(0).getConfiguration();
    configuration.removeChild(configuration.getChild("componentDocVersions"));
    AsciidocToPdfMojo mojo = (AsciidocToPdfMojo) lookupConfiguredMojo(project, "asciidoc-to-pdf");
    FileUtils.writeStringToFile(new File(mojo.getAsciidocBuildSourceDirectory(), "how-to/page.adoc"),
            "xref:reference:page.adoc[A] xref:opendj::index.adoc[B]", StandardCharsets.UTF_8);
    mojo.execute();
    assertThat(new File(mojo.getPdfSourceDirectory(), "how-to/page.adoc")).hasContent(
            "link:https://doc.openidentityplatform.org/openam/15.2/reference/page.html[A] "
                    + "link:https://doc.openidentityplatform.org/opendj/index.html[B]");
}

Pin: without the initializer execute() throws the NPE and the case goes red.


issue (non-blocking): The archive workflow does not say the rebuild needs a doc-maven-plugin release that has docVersion.

commons/doc-maven-plugin/README.md:797-803

OpenDJ release tags pin the plugin through ${commons.version} (5.2.0: 3.2.2; 4.10.2: 2.4.1), and no released version up to 3.2.2 has the parameter. Maven ignores a -DdocVersion that no mojo reads, without a warning, so the rebuilt PDF keeps the unversioned links and replaces the archive unchanged. OpenAM 15.2.2, OpenIG 6.2.0 and OpenIDM 7.1.2 declare the plugin without a version, so there the workflow works once a release with this change exists.

The rebuild needs a doc-maven-plugin release that supports `docVersion` (the
first release after 3.2.2). Release tags that pin the plugin through
`${commons.version}`, such as OpenDJ 4.10.2 and 5.x, must raise that plugin
version for the rebuild; otherwise Maven ignores `-DdocVersion` without a
warning and the PDF keeps its unversioned links.

question (non-blocking): Should a component-qualified xref to this project's own component without a version follow docVersion?

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:268-270, commons/doc-maven-plugin/src/test/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojoTest.java:72-75

With docVersion=15.2, xref:openam:ROOT:index.adoc (or xref:openam:reference:x.adoc) in OpenAM becomes /openam/15.2/... in the PDF, as the test pins; Antora 3.1.15 resolves a component-qualified ID without a version to the latest version, so the site page built from the same 15.2 source links to /openam/.... No current OpenAM, OpenDJ, OpenIG or OpenIDM source uses this form, so it is Minor either way. If the PDF should match the site, apply docVersion only when the resource ID names no component (relative ../ xrefs keep it) and flip the test:

String explicitVersion = null;
boolean namedComponent = false;
// ... in the resource-ID branch, after targetComponent is set:
namedComponent = id.group(2) != null;
// ... at :270:
        : targetComponent.equals(component) && !namedComponent ? docVersion : null;

If archived self-links are intended, say in rule 3 (README.md:784) that the PDF deliberately differs from the site for this form.


suggestion (non-blocking): Normalise configured version values before they become a path segment.

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:272, :213

-DdocVersion=15.2/ gives /openam/15.2//reference/..., and -DdocVersion="15.2 " a path with a space; siteUrl gets its trailing slashes stripped at :213, version values nothing. POM element text already arrives trimmed (Maven 3.9.16 POM reader), so whitespace comes only from -D; slashes from any route.

String trimmed = version == null ? "" : version.trim().replaceAll("^/+|/+$", "");
String versionPrefix = trimmed.isEmpty() ? "" : trimmed + "/";

This is a no-op for explicit xref versions (RESOURCE_ID_PATTERN at :192 excludes /, XREF_PATTERN at :190 excludes whitespace) and for _, so testConfiguredVersionsAreLiteralSitePaths is unaffected (not run in this review).


suggestion (non-blocking): Say that a project property named docVersion also sets the parameter.

commons/doc-maven-plugin/src/main/java/org/openidentityplatform/doc/maven/AsciidocToPdfMojo.java:87, commons/doc-maven-plugin/README.md:762

Maven resolves ${docVersion} from user, system and then project properties, inherited ones included, so a <properties><docVersion> defined for another purpose now versions every own-component PDF link. None exists today in OpenAM, OpenDJ, OpenIG, OpenIDM, OpenICF or the JEE agents; the unprefixed name matches siteUrl and antoraComponent, so keeping it is reasonable.

`docVersion` is also available as the Maven property `-DdocVersion=15.2`; a
`docVersion` project property, including one inherited from a parent POM, sets it too.

nitpick (non-blocking): The precedence example names OpenDJ 4.9, which the site does not publish.

commons/doc-maven-plugin/README.md:782

The site publishes OpenDJ-4.10.2 as 4.10, and README.md:804-805 says not to select an unpublished version path; with 4.10 the example stays copyable.

   `xref:4.10@opendj:install-guide:index.adoc[Install]`.

@vharseko
vharseko force-pushed the fix/pdf-documentation-versions branch from 141d137 to 4dd0ff2 Compare November 6, 2026 12:58
@vharseko

vharseko commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Addressed the follow-up review in 4dd0ff22f.

  • Added testComponentDocVersionsIsADescriptorParameter, which reads the generated mojo descriptor and requires componentDocVersions to be declared. This covers the descriptor gap in the existing harness-based configuration tests.
  • Added testAbsentMapKeepsOtherComponentsUnversioned. It removes the map from the test POM, runs source preparation, and checks that this component uses docVersion while OpenDJ stays unversioned.
  • Documented that archive rebuilds require a plugin release supporting these settings. Versions 3.2.2 and earlier do not support them; release POMs that pin the plugin through ${commons.version}, including OpenDJ 4.10.2 and 5.2.0, need a supporting plugin version for the rebuild.
  • Kept archived self-links intentional: an explicitly named own-component target without a version still follows docVersion. The README now explains the deliberate difference from Antora's latest-version resolution. Explicit versions and component-map overrides retain their precedence.
  • Documented project and inherited docVersion properties and changed the precedence example to the published OpenDJ 4.10 path.
  • Configured version values remain literal published site paths. Whitespace/slash normalization is deferred from this change.

Validation after rebasing onto current origin/master: mvn -o -Dmaven.repo.local=<isolated-cache> -pl commons/doc-maven-plugin -am test on JDK 17 passed all 547 tests, including 20 converter tests and five configuration tests, with no failures, errors, or skips. git diff --check passes. Earlier isolated mutation checks confirmed that removing @Parameter fails the descriptor test and removing the map initializer fails the absent-map test; the existing tests passed under both mutations. Full PDF rendering, site builds, and the earlier seven-URL/fragment integration checks were not repeated for this follow-up.

@vharseko
vharseko requested a review from maximthomas November 6, 2026 12:59

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: This round answers the previous review's items in both the tests and the README.

  • testComponentDocVersionsIsADescriptorParameter asserts that the asciidoc-to-pdf descriptor lists componentDocVersions.
  • testAbsentMapKeepsOtherComponentsUnversioned removes <componentDocVersions> and asserts that xref:opendj::index.adoc stays at /opendj/index.html.
  • README.md:804-807 names the ${commons.version} pin in OpenDJ 4.10.2 and 5.2.0 and says that the rebuild must select a plugin release that supports these options.

@vharseko
vharseko merged commit 72901ab into OpenIdentityPlatform:master Nov 6, 2026
14 checks passed
@vharseko
vharseko deleted the fix/pdf-documentation-versions branch November 6, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug documentation Documentation and Javadoc changes java Pull requests that update java code tests Test code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs: preserve wiki section links and PDF documentation versions after How-To migration

2 participants