From 108bb38bd9325baaa15b57637b3ddc7460c2f87f Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Fri, 17 Jun 2022 15:37:54 +0000 Subject: [PATCH 1/3] feat: Prefer local package files first --- .../java/com/microsoft/build/BuilderUtil.java | 83 +++++++++++++++---- .../main/java/com/microsoft/build/Lookup.java | 8 +- .../com/microsoft/build/BuilderUtilTest.java | 36 +++++--- ...icrosoft.samples.google.SpeechSettings.yml | 2 +- 4 files changed, 102 insertions(+), 27 deletions(-) diff --git a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java index 46214575..d6ea3675 100644 --- a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java +++ b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java @@ -37,7 +37,15 @@ final class BuilderUtil { private static final Pattern XREF_LINK_RESOLVE_PATTERN = Pattern.compile("(?\\w+)\\#(?\\w+)(\\((?.*)\\))?"); public final static String[] LANGS = {"java"}; - static String populateUidValues(String text, LookupContext lookupContext) { + /** + * Replaces all the references the link in the linkContent with the UID + * + * @param text Text that potentially contains a link reference + * @param packageName MetaFileItem's packageName + * @param lookupContext Lookup Context + * @return Text with a link reference to the full qualified link + */ + static String populateUidValues(String text, String packageName, LookupContext lookupContext) { if (StringUtils.isBlank(text)) { return text; } @@ -51,7 +59,7 @@ static String populateUidValues(String text, LookupContext lookupContext) { } String linkContent = linkContentMatcher.group(); - String uid = resolveUidFromLinkContent(linkContent, lookupContext); + String uid = resolveUidFromLinkContent(linkContent, packageName, lookupContext); String updatedLink = linkContentMatcher.replaceAll(uid); text = StringUtils.replace(text, link, updatedLink); } @@ -59,13 +67,21 @@ static String populateUidValues(String text, LookupContext lookupContext) { } /** + * * The linkContent could be in following format - * #memeber - * Class#member - * Class#method() - * Class#method(params) + * 1. #member + * 2. Class#member + * 3. Class#method() + * 4. Class#method(params) + * 5. Package.Class# +{Any combination of above} + * All Possibilities listed: https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#link + * + * @param linkContent Text of the link + * @param packageName MetaFileItem's packageName + * @param lookupContext LookupContext + * @return Fully qualified link url */ - static String resolveUidFromLinkContent(String linkContent, LookupContext lookupContext) { + static String resolveUidFromLinkContent(String linkContent, String packageName, LookupContext lookupContext) { if (StringUtils.isBlank(linkContent)) { return ""; } @@ -74,19 +90,42 @@ static String resolveUidFromLinkContent(String linkContent, LookupContext lookup // complete class name for class internal link if (linkContent.startsWith("#")) { + // Can't use packageName because it is missing the ClassName String firstKey = lookupContext.getOwnerUid(); linkContent = firstKey + linkContent; } - // fuzzy resolve, target for items from project external references String fuzzyResolvedUid = resolveUidFromReference(linkContent, lookupContext); // exact resolve in lookupContext linkContent = linkContent.replace("#", "."); - String exactResolveUid = resolveUidByLookup(linkContent, lookupContext); + String qualifiedLink = getFullyQualifiedLinkUrl(linkContent, packageName); + + // First, prefer references inside local package + String exactResolveUid = resolveUidByLookup(qualifiedLink, lookupContext); + if (exactResolveUid.isEmpty()) { + // Resolve without local package context + exactResolveUid = resolveUidByLookup(linkContent, lookupContext); + } + // Resolve with fuzzyResolve / external references return exactResolveUid.isEmpty() ? fuzzyResolvedUid : exactResolveUid; } + /** + * Logic to ensure that resulting link is in the format Package.Class.Method(Params...) + * + * @param linkContent String of the Class#Method + * @param packageName Package Name that should go in front of the link + * @return Fully qualified link url + */ + private static String getFullyQualifiedLinkUrl(String linkContent, String packageName) { + // If packageName does not exist/error'd or is already at the beginning of the link + if (packageName == null || linkContent.indexOf(packageName) == 0) { + return linkContent; + } + return String.format("%s.%s", packageName, linkContent); + } + static List splitUidWithGenericsIntoClassNames(String uid) { uid = RegExUtils.removeAll(uid, "[>]+$"); return Arrays.asList(StringUtils.split(uid, "<")); @@ -120,6 +159,23 @@ else if (uid != "") return specList; } + /** + * Populate the links for references based on a UID. Looker generates mappings for local context + * (file specific) and global context (all the UID references). Searching is done first in the + * local context and then in the global context if local context is not found. + * + * ex. {@link Lookup } would search all the references to find which Lookup file to use + * It would parse the text to search ("Lookup") + * + * Since packages (v1 and v1beta) may contain the same generated java file names, there may be some + * conflicts between which link it should be. + * + * The Looker#consumer() function guarantees that the UID (+ other combinations) will be put into the LookupContext. + * As long as the link that we extract contains Package#Method, it will match in either local or global context + * + * @param packageMetadataFiles Package specific metadata files + * @param classMetadataFiles Class specific metadata files + */ static void populateUidValues(List packageMetadataFiles, List classMetadataFiles) { Lookup lookup = new Lookup(packageMetadataFiles, classMetadataFiles); @@ -127,20 +183,19 @@ static void populateUidValues(List packageMetadataFiles, List { Optional.ofNullable(syntax.getParameters()).ifPresent( methodParams -> methodParams.forEach( - param -> { - param.setDescription(populateUidValues(param.getDescription(), lookupContext)); - }) + param -> param.setDescription(populateUidValues(param.getDescription(), packageName, lookupContext))) ); Optional.ofNullable(syntax.getReturnValue()).ifPresent(returnValue -> returnValue.setReturnDescription( - populateUidValues(syntax.getReturnValue().getReturnDescription(), lookupContext) + populateUidValues(syntax.getReturnValue().getReturnDescription(), packageName, lookupContext) ) ); } diff --git a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/Lookup.java b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/Lookup.java index 77781040..b8ebb071 100644 --- a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/Lookup.java +++ b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/Lookup.java @@ -12,14 +12,18 @@ public class Lookup { - private Map globalLookup = new HashMap<>(); - private Map> localLookupByFileName = new HashMap<>(); + private static final int INITIAL_GLOBAL_CAPACITY = 250000; + + private final Map globalLookup; + private final Map> localLookupByFileName; private final String UID_PACKAGE_NAME_REGEXP = "^.*?\\.(?=[A-Z].*)"; private final String PARAM_PACKAGE_NAME_REGEXP = "(?<=[\\( ]).*?(?=[A-Z].*)"; private final String METHOD_PARAMS_REGEXP = "\\s[^\\s]+?(?=[,)])"; public Lookup(List packageMetadataFiles, List classMetadataFiles) { + this.globalLookup = new HashMap<>(INITIAL_GLOBAL_CAPACITY); + this.localLookupByFileName = new HashMap<>(); consume(packageMetadataFiles); consume(classMetadataFiles); } diff --git a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java index 03bdda31..484f0a56 100644 --- a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java +++ b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java @@ -32,7 +32,7 @@ public class BuilderUtilTest { @Test - public void populateUidValues() { + public void testPopulateUidValues() { MetadataFile classMetadataFile = new MetadataFile("output", "name"); MetadataFileItem ownerClassItem = buildMetadataFileItem("a.b.OwnerClass", "Not important summary value"); @@ -41,15 +41,21 @@ public void populateUidValues() { populateSyntax(item1, "SomeClass#someMethod(String param)"); MetadataFileItem item2 = buildMetadataFileItem("UID known class", "SomeClass#someMethod(String param)"); MetadataFileItem item3 = buildMetadataFileItem("UID method only", "#someMethod2(String p1, String p2)"); + item3.setPackageName("a.b"); classMetadataFile.getItems().addAll(Arrays.asList(ownerClassItem, item1, item2, item3)); MetadataFileItem reference1 = new MetadataFileItem("a.b.SomeClass.someMethod(String param)"); + reference1.setPackageName("a.b"); reference1.setNameWithType("SomeClass.someMethod(String param)"); MetadataFileItem reference2 = new MetadataFileItem("a.b.OwnerClass.someMethod2(String p1, String p2)"); + reference2.setPackageName("a.b"); reference2.setNameWithType("OwnerClass.someMethod2(String p1, String p2)"); - classMetadataFile.getReferences().addAll(Arrays.asList(reference1, reference2)); + MetadataFileItem reference3 = new MetadataFileItem("c.d.OwnerClass.someMethod2(String p1, String p2)"); + reference3.setPackageName("c.d"); + reference3.setNameWithType("OwnerClass.someMethod2(String p1, String p2)"); + classMetadataFile.getReferences().addAll(Arrays.asList(reference1, reference2, reference3)); - BuilderUtil.populateUidValues(Collections.emptyList(), Arrays.asList(classMetadataFile)); + BuilderUtil.populateUidValues(Collections.emptyList(), List.of(classMetadataFile)); assertEquals("Wrong summary for unknown class", item1.getSummary(), "Bla bla UnknownClass bla"); @@ -79,21 +85,31 @@ private void populateSyntax(MetadataFileItem item, String value) { } @Test - public void determineUidByLinkContent() { + public void testDetermineUidByLinkContent() { + // Map similar to what is created in Lookup#consume() Map lookup = new HashMap<>() {{ put("SomeClass", "a.b.c.SomeClass"); put("SomeClass.someMethod()", "a.b.c.SomeClass.someMethod()"); put("SomeClass.someMethod(String param)", "a.b.c.SomeClass.someMethod(String param)"); + put("a.b.c.SomeClass", "a.b.c.SomeClass"); + put("a.b.c.SomeClass.someMethod()", "a.b.c.SomeClass.someMethod()"); + put("a.b.c.SomeClass.someMethod(String param)", "a.b.c.SomeClass.someMethod(String param)"); + put("SomeClass", "d.e.f.SomeClass"); + put("d.e.f.SomeClass", "d.e.f.SomeClass"); }}; + String packageName = "a.b.c"; + LookupContext lookupContext = new LookupContext(lookup, lookup); - assertEquals("Wrong result for class", BuilderUtil. - resolveUidByLookup("SomeClass", lookupContext), "a.b.c.SomeClass"); - assertEquals("Wrong result for method", BuilderUtil. - resolveUidFromLinkContent("SomeClass#someMethod()", lookupContext), "a.b.c.SomeClass.someMethod()"); - assertEquals("Wrong result for method with param", BuilderUtil. - resolveUidFromLinkContent("SomeClass#someMethod(String param)", lookupContext), + assertEquals("Wrong result for class", + BuilderUtil.resolveUidFromLinkContent("SomeClass", packageName, lookupContext), "a.b.c.SomeClass"); + assertEquals("Wrong result for method", + BuilderUtil.resolveUidFromLinkContent("SomeClass#someMethod()", packageName, lookupContext), "a.b.c.SomeClass.someMethod()"); + assertEquals("Wrong result for method with param", + BuilderUtil.resolveUidFromLinkContent("SomeClass#someMethod(String param)", packageName, lookupContext), "a.b.c.SomeClass.someMethod(String param)"); + assertEquals("Wrong result for class with duplicate className", + BuilderUtil.resolveUidFromLinkContent("SomeClass", packageName, lookupContext), "a.b.c.SomeClass"); assertEquals("Wrong result for unknown class", BuilderUtil. resolveUidByLookup("UnknownClass", lookupContext), ""); diff --git a/third_party/docfx-doclet-143274/src/test/resources/expected-generated-files/com.microsoft.samples.google.SpeechSettings.yml b/third_party/docfx-doclet-143274/src/test/resources/expected-generated-files/com.microsoft.samples.google.SpeechSettings.yml index faffd422..c4c0c8ce 100644 --- a/third_party/docfx-doclet-143274/src/test/resources/expected-generated-files/com.microsoft.samples.google.SpeechSettings.yml +++ b/third_party/docfx-doclet-143274/src/test/resources/expected-generated-files/com.microsoft.samples.google.SpeechSettings.yml @@ -28,7 +28,7 @@ items: fullName: "com.microsoft.samples.google.SpeechSettings" type: "Class" package: "com.microsoft.samples.google" - summary: "Settings class to configure an instance of SpeechClient.\n\n

The default instance has everything set to sensible defaults:\n\n

    \n
  • The default service address (speech.googleapis.com) and default port (443) are used.\n
  • Credentials are acquired automatically through Application Default Credentials.\n
  • Retries are configured for idempotent methods but not for non-idempotent methods.\n
\n\n

The builder of this class is recursive, so contained classes are themselves builders. When\n build() is called, the tree of builders is called to create the complete settings object.\n\n

For example, to set the total timeout of recognize to 30 seconds:\n\n 

\n SpeechSettings.Builder speechSettingsBuilder = SpeechSettings.newBuilder();\n speechSettingsBuilder\n     .recognizeSettings()\n     .setRetrySettings(\n         speechSettingsBuilder\n             .recognizeSettings()\n             .getRetrySettings()\n             .toBuilder()\n             .setTotalTimeout(Duration.ofSeconds(30))\n             .build());\n SpeechSettings speechSettings = speechSettingsBuilder.build();\n
" + summary: "Settings class to configure an instance of SpeechClient.\n\n

The default instance has everything set to sensible defaults:\n\n

    \n
  • The default service address (speech.googleapis.com) and default port (443) are used.\n
  • Credentials are acquired automatically through Application Default Credentials.\n
  • Retries are configured for idempotent methods but not for non-idempotent methods.\n
\n\n

The builder of this class is recursive, so contained classes are themselves builders. When\n build() is called, the tree of builders is called to create the complete settings object.\n\n

For example, to set the total timeout of recognize to 30 seconds:\n\n 

\n SpeechSettings.Builder speechSettingsBuilder = SpeechSettings.newBuilder();\n speechSettingsBuilder\n     .recognizeSettings()\n     .setRetrySettings(\n         speechSettingsBuilder\n             .recognizeSettings()\n             .getRetrySettings()\n             .toBuilder()\n             .setTotalTimeout(Duration.ofSeconds(30))\n             .build());\n SpeechSettings speechSettings = speechSettingsBuilder.build();\n
" syntax: content: "public class SpeechSettings extends ClientSettings" inheritance: From ddca6a66535518db9fbb7fa6660709f786b15f05 Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Fri, 17 Jun 2022 17:03:07 +0000 Subject: [PATCH 2/3] feat: Test use different package --- third_party/docfx-doclet-143274/pom.xml | 2 +- .../src/test/java/com/microsoft/build/BuilderUtilTest.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/third_party/docfx-doclet-143274/pom.xml b/third_party/docfx-doclet-143274/pom.xml index 9e53477f..57c9d96d 100644 --- a/third_party/docfx-doclet-143274/pom.xml +++ b/third_party/docfx-doclet-143274/pom.xml @@ -6,7 +6,7 @@ com.microsoft docfx-doclet - 1.0-SNAPSHOT + 1.7.0 UTF-8 diff --git a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java index 484f0a56..aafa969f 100644 --- a/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java +++ b/third_party/docfx-doclet-143274/src/test/java/com/microsoft/build/BuilderUtilTest.java @@ -28,6 +28,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; public class BuilderUtilTest { @@ -94,11 +95,13 @@ public void testDetermineUidByLinkContent() { put("a.b.c.SomeClass", "a.b.c.SomeClass"); put("a.b.c.SomeClass.someMethod()", "a.b.c.SomeClass.someMethod()"); put("a.b.c.SomeClass.someMethod(String param)", "a.b.c.SomeClass.someMethod(String param)"); + // Duplicate entry to simulate same ClassName in a different package put("SomeClass", "d.e.f.SomeClass"); put("d.e.f.SomeClass", "d.e.f.SomeClass"); }}; String packageName = "a.b.c"; + String otherPackageName = "d.e.f"; LookupContext lookupContext = new LookupContext(lookup, lookup); assertEquals("Wrong result for class", @@ -109,7 +112,7 @@ public void testDetermineUidByLinkContent() { BuilderUtil.resolveUidFromLinkContent("SomeClass#someMethod(String param)", packageName, lookupContext), "a.b.c.SomeClass.someMethod(String param)"); assertEquals("Wrong result for class with duplicate className", - BuilderUtil.resolveUidFromLinkContent("SomeClass", packageName, lookupContext), "a.b.c.SomeClass"); + BuilderUtil.resolveUidFromLinkContent("SomeClass", otherPackageName, lookupContext), "d.e.f.SomeClass"); assertEquals("Wrong result for unknown class", BuilderUtil. resolveUidByLookup("UnknownClass", lookupContext), ""); From 791204d309aa192b542f743ee22d87bff04ae2bd Mon Sep 17 00:00:00 2001 From: Lawrence Qiu Date: Fri, 17 Jun 2022 17:06:38 +0000 Subject: [PATCH 3/3] feat: Revert back to original version --- third_party/docfx-doclet-143274/pom.xml | 2 +- .../src/main/java/com/microsoft/build/BuilderUtil.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/third_party/docfx-doclet-143274/pom.xml b/third_party/docfx-doclet-143274/pom.xml index 57c9d96d..9e53477f 100644 --- a/third_party/docfx-doclet-143274/pom.xml +++ b/third_party/docfx-doclet-143274/pom.xml @@ -6,7 +6,7 @@ com.microsoft docfx-doclet - 1.7.0 + 1.0-SNAPSHOT UTF-8 diff --git a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java index d6ea3675..85878606 100644 --- a/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java +++ b/third_party/docfx-doclet-143274/src/main/java/com/microsoft/build/BuilderUtil.java @@ -104,7 +104,7 @@ static String resolveUidFromLinkContent(String linkContent, String packageName, // First, prefer references inside local package String exactResolveUid = resolveUidByLookup(qualifiedLink, lookupContext); if (exactResolveUid.isEmpty()) { - // Resolve without local package context + // Resolve with original linkContent exactResolveUid = resolveUidByLookup(linkContent, lookupContext); } // Resolve with fuzzyResolve / external references