From 6f10b579ea9f2a439bd247168bf17feb6bf7b345 Mon Sep 17 00:00:00 2001 From: Jin Seop Kim Date: Tue, 27 Jan 2026 13:48:04 -0500 Subject: [PATCH] fix: allow overriding source directory per class in GitHub links Currently, the doclet assumes the source directory name matches the artifact ID when generating GitHub links. This leads to broken links for libraries where they differ, such as Firestore (artifact: `google-cloud-firestore`, directory: `google-cloud-firestore-admin` for `FirestoreAdminClient`). This change adds an optional `library_path_overrides` map to `RepoMetadata` and updates `ClassBuilder` to check this map using the simple class name before falling back to the artifact ID. Fixes: b/442875200 --- .../com/google/docfx/doclet/RepoMetadata.java | 13 +++++ .../com/microsoft/build/ClassBuilder.java | 15 +++--- .../google/docfx/doclet/RepoMetadataTest.java | 29 +++++++++++ .../com/microsoft/build/ClassBuilderTest.java | 48 +++++++++++++++++++ 4 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 third_party/docfx-doclet-143274/src/test/java/com/google/docfx/doclet/RepoMetadataTest.java diff --git a/third_party/docfx-doclet-143274/src/main/java/com/google/docfx/doclet/RepoMetadata.java b/third_party/docfx-doclet-143274/src/main/java/com/google/docfx/doclet/RepoMetadata.java index a6138546..b232498d 100644 --- a/third_party/docfx-doclet-143274/src/main/java/com/google/docfx/doclet/RepoMetadata.java +++ b/third_party/docfx-doclet-143274/src/main/java/com/google/docfx/doclet/RepoMetadata.java @@ -8,6 +8,8 @@ import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collections; +import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -49,6 +51,9 @@ public class RepoMetadata { @SerializedName("recommended_package") private String recommendedPackage; + @SerializedName("library_path_overrides") + private Map libraryPathOverrides; + private String artifactId; public String getNamePretty() { @@ -135,6 +140,14 @@ public void setApiId(String apiId) { this.apiId = apiId; } + public Map getLibraryPathOverrides() { + return this.libraryPathOverrides != null ? this.libraryPathOverrides : Collections.emptyMap(); + } + + public void setLibraryPathOverrides(Map libraryPathOverrides) { + this.libraryPathOverrides = libraryPathOverrides; + } + // artifactId is parsed from distributionName public String getArtifactId() { String substrings[] = this.distributionName.split(":"); diff --git a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/ClassBuilder.java b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/ClassBuilder.java index e1e93209..08123e4a 100644 --- a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/ClassBuilder.java +++ b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/ClassBuilder.java @@ -248,13 +248,16 @@ private void addClientClassInfo( private String createClientOverviewTable(TypeElement classElement, RepoMetadata repoMetadata) { String clientURI = classLookup.extractUid(classElement).replaceAll("\\.", "/"); + String className = classElement.getSimpleName().toString(); + + // Check overrides map first, otherwise default to artifactId + String directory = + repoMetadata + .getLibraryPathOverrides() + .getOrDefault(className, repoMetadata.getArtifactId()); + String githubSourceLink = - repoMetadata.getGithubLink() - + "/" - + repoMetadata.getArtifactId() - + "/src/main/java/" - + clientURI - + ".java"; + repoMetadata.getGithubLink() + "/" + directory + "/src/main/java/" + clientURI + ".java"; StringBuilder tableBuilder = new StringBuilder(); tableBuilder .append("") diff --git a/third_party/docfx-doclet-143274/src/test/java/com/google/docfx/doclet/RepoMetadataTest.java b/third_party/docfx-doclet-143274/src/test/java/com/google/docfx/doclet/RepoMetadataTest.java new file mode 100644 index 00000000..3b7d3317 --- /dev/null +++ b/third_party/docfx-doclet-143274/src/test/java/com/google/docfx/doclet/RepoMetadataTest.java @@ -0,0 +1,29 @@ +package com.google.docfx.doclet; + +import static org.junit.Assert.assertEquals; + +import com.google.gson.Gson; +import org.junit.Test; + +public class RepoMetadataTest { + + @Test + public void testParseWithLibraryPathOverrides() { + String json = + "{ " + + "\"distribution_name\": \"com.google.cloud:google-cloud-firestore\", " + + "\"library_path_overrides\": { " + + " \"FirestoreAdminClient\": \"google-cloud-firestore-admin\" " + + "}, " + + "\"repo\": \"googleapis/java-firestore\" " + + "}"; + + RepoMetadata metadata = new Gson().fromJson(json, RepoMetadata.class); + + assertEquals("google-cloud-firestore", metadata.getArtifactId()); + // Verify the map is populated correctly + assertEquals( + "google-cloud-firestore-admin", + metadata.getLibraryPathOverrides().get("FirestoreAdminClient")); + } +} diff --git a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/ClassBuilderTest.java b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/ClassBuilderTest.java index d3945f64..a09a5371 100644 --- a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/ClassBuilderTest.java +++ b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/ClassBuilderTest.java @@ -16,8 +16,10 @@ package com.microsoft.build; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; +import com.google.docfx.doclet.RepoMetadata; import com.google.testing.compile.CompilationRule; import com.microsoft.lookup.ClassItemsLookup; import com.microsoft.lookup.ClassLookup; @@ -28,6 +30,9 @@ import com.sun.source.util.DocTrees; import java.io.File; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import javax.lang.model.element.Name; // Required for mocking getSimpleName() import javax.lang.model.element.TypeElement; import javax.lang.model.util.Elements; import jdk.javadoc.doclet.DocletEnvironment; @@ -89,4 +94,47 @@ public void addConstructorsInfo() { Collection constructorItems = container.getItems(); assertEquals("Container should contain 2 constructor items", constructorItems.size(), 2); } + + @Test + public void createClientOverviewTable_usesLibraryPathOverride() { + // 1. Setup Mock RepoMetadata + RepoMetadata repoMetadata = new RepoMetadata(); + repoMetadata.setRepo("googleapis/java-firestore"); + repoMetadata.setDistributionName("com.google.cloud:google-cloud-firestore:1.0.0"); + + Map overrides = new HashMap<>(); + overrides.put("FirestoreAdminClient", "google-cloud-firestore-admin"); + repoMetadata.setLibraryPathOverrides(overrides); + + // 2. Mock ClassLookup and Element + ClassLookup classLookup = Mockito.mock(ClassLookup.class); + TypeElement classElement = Mockito.mock(TypeElement.class); + + Name simpleName = Mockito.mock(Name.class); + when(simpleName.toString()).thenReturn("FirestoreAdminClient"); + when(classElement.getSimpleName()).thenReturn(simpleName); + + when(classLookup.extractUid(classElement)) + .thenReturn("com.google.cloud.firestore.v1.FirestoreAdminClient"); + + // 3. Test + ClassBuilder builder = new ClassBuilder(null, classLookup, null, null, null, null); + + try { + java.lang.reflect.Method method = + ClassBuilder.class.getDeclaredMethod( + "createClientOverviewTable", TypeElement.class, RepoMetadata.class); + method.setAccessible(true); + String html = (String) method.invoke(builder, classElement, repoMetadata); + + // 4. Verify link contains "google-cloud-firestore-admin" and the double slash "//" + assertTrue( + "Link should use the override directory", + html.contains( + "googleapis/java-firestore/tree/main//google-cloud-firestore-admin/src/main/java")); + + } catch (Exception e) { + throw new RuntimeException(e); + } + } }