From bb81bb370f9ff734bc55405bac13bd54639320d7 Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:07:01 -0400 Subject: [PATCH 01/23] Update Codesnippet dependencies (#8078) --- .../codesnippet-maven-plugin/README.md | 2 +- .../codesnippet-maven-plugin/pom.xml | 48 ++++++------- .../implementation/SnippetReplacer.java | 72 +++++++++---------- .../implementation/SnippetReplacerTests.java | 10 +-- 4 files changed, 66 insertions(+), 66 deletions(-) diff --git a/packages/java-packages/codesnippet-maven-plugin/README.md b/packages/java-packages/codesnippet-maven-plugin/README.md index b6e5e902078..d0b1e99b714 100644 --- a/packages/java-packages/codesnippet-maven-plugin/README.md +++ b/packages/java-packages/codesnippet-maven-plugin/README.md @@ -12,7 +12,7 @@ First, reference the plugin in your maven project's `pom.xml` file. com.azure.tools codesnippet-maven-plugin - 1.0.0-beta.8 + 1.0.0-beta.9 **/src/samples/java/**/*.java ${project.basedir}/src/samples/java diff --git a/packages/java-packages/codesnippet-maven-plugin/pom.xml b/packages/java-packages/codesnippet-maven-plugin/pom.xml index b107e4cabd6..6e49532e1c5 100644 --- a/packages/java-packages/codesnippet-maven-plugin/pom.xml +++ b/packages/java-packages/codesnippet-maven-plugin/pom.xml @@ -6,7 +6,7 @@ com.azure.tools codesnippet-maven-plugin - 1.0.0-beta.8 + 1.0.0-beta.9 maven-plugin Codesnippet Maven Plugin @@ -52,7 +52,7 @@ UTF-8 8 8 - 3.6.3 + 3.9.6 @@ -83,32 +83,32 @@ org.apache.maven.plugin-tools maven-plugin-annotations - 3.6.1 + 3.12.0 provided - junit - junit - 4.13.2 + org.junit.jupiter + junit-jupiter-api + 5.10.2 test - org.apache.maven.plugin-testing - maven-plugin-testing-harness - 3.3.0 + org.junit.jupiter + junit-jupiter-engine + 5.10.2 test org.openjdk.jmh jmh-core - 1.34 + 1.37 test org.openjdk.jmh jmh-generator-annprocess - 1.34 + 1.37 test @@ -123,27 +123,27 @@ maven-clean-plugin - 3.1.0 + 3.3.2 maven-compiler-plugin - 3.8.0 + 3.13.0 maven-surefire-plugin - 2.22.1 + 3.2.5 maven-install-plugin - 2.5.2 + 3.1.1 maven-deploy-plugin - 2.8.2 + 3.1.1 maven-invoker-plugin - 3.1.0 + 3.6.1 @@ -151,7 +151,7 @@ org.apache.maven.plugins maven-plugin-plugin - 3.6.0 + 3.12.0 true @@ -174,7 +174,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.3.1 + 3.6.3 1.8 false @@ -185,7 +185,7 @@ false true - true + false all true @@ -202,14 +202,14 @@ org.apache.maven.plugins maven-jar-plugin - 3.1.2 + 3.3.0 org.apache.maven.plugins maven-antrun-plugin - 1.8 + 3.1.0 copy @@ -230,7 +230,7 @@ org.apache.maven.plugins maven-source-plugin - 3.0.1 + 3.3.1 attach-sources @@ -247,7 +247,7 @@ org.apache.maven.plugins maven-resources-plugin - 3.0.2 + 3.3.1 ${basedir}/target/test-classes diff --git a/packages/java-packages/codesnippet-maven-plugin/src/main/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacer.java b/packages/java-packages/codesnippet-maven-plugin/src/main/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacer.java index ab12f0470a3..fea0935e1c2 100644 --- a/packages/java-packages/codesnippet-maven-plugin/src/main/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacer.java +++ b/packages/java-packages/codesnippet-maven-plugin/src/main/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacer.java @@ -14,7 +14,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.BitSet; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -26,11 +26,11 @@ public final class SnippetReplacer { private static final String JAVADOC_PRE_FENCE = "
";
     private static final String JAVADOC_POST_FENCE = "
"; - static final BitSet VALID_SNIPPET_ID_CHARACTER; + static final boolean[] VALID_SNIPPET_ID_CHARACTER; static final String[] JAVADOC_CODESNIPPET_REPLACEMENTS; static { - JAVADOC_CODESNIPPET_REPLACEMENTS = new String[256]; + JAVADOC_CODESNIPPET_REPLACEMENTS = new String[128]; JAVADOC_CODESNIPPET_REPLACEMENTS['&'] = "&"; JAVADOC_CODESNIPPET_REPLACEMENTS['\"'] = """; JAVADOC_CODESNIPPET_REPLACEMENTS['>'] = ">"; @@ -43,15 +43,15 @@ public final class SnippetReplacer { JAVADOC_CODESNIPPET_REPLACEMENTS['/'] = "/"; JAVADOC_CODESNIPPET_REPLACEMENTS['\\'] = "\"; - VALID_SNIPPET_ID_CHARACTER = new BitSet(256); - VALID_SNIPPET_ID_CHARACTER.set('a', 'z' + 1); - VALID_SNIPPET_ID_CHARACTER.set('A', 'Z' + 1); - VALID_SNIPPET_ID_CHARACTER.set('0', '9' + 1); - VALID_SNIPPET_ID_CHARACTER.set('.'); - VALID_SNIPPET_ID_CHARACTER.set('#'); - VALID_SNIPPET_ID_CHARACTER.set('\\'); - VALID_SNIPPET_ID_CHARACTER.set('-'); - VALID_SNIPPET_ID_CHARACTER.set('_'); + VALID_SNIPPET_ID_CHARACTER = new boolean[128]; + Arrays.fill(VALID_SNIPPET_ID_CHARACTER, 'a', 'z' + 1, true); + Arrays.fill(VALID_SNIPPET_ID_CHARACTER, 'A', 'Z' + 1, true); + Arrays.fill(VALID_SNIPPET_ID_CHARACTER, '0', '9' + 1, true); + VALID_SNIPPET_ID_CHARACTER['.'] = true; + VALID_SNIPPET_ID_CHARACTER['#'] = true; + VALID_SNIPPET_ID_CHARACTER['\\'] = true; + VALID_SNIPPET_ID_CHARACTER['-'] = true; + VALID_SNIPPET_ID_CHARACTER['_'] = true; } /** @@ -262,15 +262,14 @@ private static List updateSnippets(Path file, String linePrefix = prefixFunction(end, additionalLinePrefix); int longestSnippetLine = 0; - StringBuilder snippetIndentationBuilder = new StringBuilder(); - for (int i = 0; i < snippetTagIndentation; i++) { - snippetIndentationBuilder.append(" "); - } - String snippetIndentation = snippetIndentationBuilder.toString(); + byte[] whitespace = new byte[snippetTagIndentation]; + Arrays.fill(whitespace, (byte) ' '); + String snippetIndentation = new String(whitespace); + for (String snippet : respaceLines(newSnippets.getContent())) { longestSnippetLine = Math.max(longestSnippetLine, snippet.length()); String modifiedSnippet = applyReplacements(snippet, replacements); - modifiedSnippets.add(modifiedSnippet.length() == 0 + modifiedSnippets.add(modifiedSnippet.isEmpty() ? stripTrailingWhitespace(linePrefix) + lineSep : snippetIndentation + linePrefix + modifiedSnippet + lineSep); } @@ -279,7 +278,7 @@ private static List updateSnippets(Path file, updateErrors.add(new CodesnippetLengthError(currentSnippetId, file, longestSnippetLine)); } - if (preFence != null && preFence.length() > 0) { + if (preFence != null && !preFence.isEmpty()) { modifiedLines.add(linePrefix); modifiedLines.add(preFence); modifiedLines.add(lineSep); @@ -287,7 +286,7 @@ private static List updateSnippets(Path file, modifiedLines.addAll(modifiedSnippets); - if (postFence != null && postFence.length() > 0) { + if (postFence != null && !postFence.isEmpty()) { modifiedLines.add(linePrefix); modifiedLines.add(postFence); modifiedLines.add(lineSep); @@ -365,15 +364,14 @@ private static List verifySnippets(Path file, String linePrefix = prefixFunction(end, additionalLinePrefix); int longestSnippetLine = 0; - StringBuilder snippetIndentationBuilder = new StringBuilder(); - for (int i = 0; i < snippetTagIndentation; i++) { - snippetIndentationBuilder.append(" "); - } - String snippetIndentation = snippetIndentationBuilder.toString(); + byte[] whitespace = new byte[snippetTagIndentation]; + Arrays.fill(whitespace, (byte) ' '); + String snippetIndentation = new String(whitespace); + for (String snippet : respaceLines(newSnippets.getContent())) { longestSnippetLine = Math.max(longestSnippetLine, snippet.length()); String modifiedSnippet = applyReplacements(snippet, replacements); - modifiedSnippets.add(modifiedSnippet.length() == 0 + modifiedSnippets.add(modifiedSnippet.isEmpty() ? stripTrailingWhitespace(linePrefix) + lineSep : snippetIndentation + linePrefix + modifiedSnippet + lineSep); } @@ -391,7 +389,7 @@ private static List verifySnippets(Path file, } } else { if (inSnippet) { - if (preFence.length() > 0 && postFence.length() > 0) { + if (!preFence.isEmpty() && !postFence.isEmpty()) { if (!line.contains(preFence) && !line.contains(postFence)) { currentSnippetSet.add(line + lineSep); } @@ -523,11 +521,13 @@ private static List respaceLines(List snippetText) { for (String snippetLine : snippetText) { // only look at non-whitespace only strings for the min indent - if (snippetLine.trim().length() == 0) { + int leadingWhitespace = nextNonWhitespace(snippetLine, 0); + if (leadingWhitespace == -1) { + // -1 indicates the string is all whitespace continue; } - minWhitespace = Math.min(minWhitespace, leadingWhitespaceCount(snippetLine)); + minWhitespace = Math.min(minWhitespace, leadingWhitespace); if (minWhitespace == 0) { break; } @@ -568,7 +568,7 @@ static String applyReplacements(String snippet, String[] replacements) { for (int i = 0; i < snippetLength; i++) { char c = snippet.charAt(i); - if (c >= 256) { + if (c >= 128) { continue; } @@ -653,7 +653,7 @@ private static int leadingWhitespaceCount(String str) { } private static int nextNonWhitespace(String str, int offset) { - if (str == null || str.length() == 0 || str.length() - 1 == offset) { + if (str == null || str.isEmpty() || str.length() - 1 == offset) { return -1; } @@ -670,7 +670,7 @@ private static int nextNonWhitespace(String str, int offset) { } private static String stripTrailingWhitespace(String str) { - if (str == null || str.length() == 0) { + if (str == null || str.isEmpty()) { return str; } @@ -700,7 +700,7 @@ private static String getSnippetId(String str, int offset) { int end = start; for (; end < strLength; end++) { char c = str.charAt(end); - if (!VALID_SNIPPET_ID_CHARACTER.get(c)) { + if (c >= 128 || !VALID_SNIPPET_ID_CHARACTER[c]) { if (!Character.isWhitespace(c)) { return null; } else { @@ -713,7 +713,7 @@ private static String getSnippetId(String str, int offset) { } private static SnippetInfo getSnippetDefinition(String str, boolean beginDefinition) { - if (str == null || str.length() == 0) { + if (str == null || str.isEmpty()) { return null; } @@ -741,7 +741,7 @@ private static SnippetInfo getSnippetDefinition(String str, boolean beginDefinit } private static SnippetInfo getSourceCall(String str, boolean beginSourceCall) { - if (str == null || str.length() == 0) { + if (str == null || str.isEmpty()) { return null; } @@ -783,7 +783,7 @@ private static SnippetInfo getSourceCall(String str, boolean beginSourceCall) { } private static SnippetInfo getReadmeCall(String str, boolean beginReadmeCall) { - if (str == null || str.length() == 0) { + if (str == null || str.isEmpty()) { return null; } diff --git a/packages/java-packages/codesnippet-maven-plugin/src/test/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacerTests.java b/packages/java-packages/codesnippet-maven-plugin/src/test/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacerTests.java index 9620f8c1102..ab43f2ab16e 100644 --- a/packages/java-packages/codesnippet-maven-plugin/src/test/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacerTests.java +++ b/packages/java-packages/codesnippet-maven-plugin/src/test/java/com/azure/tools/codesnippetplugin/implementation/SnippetReplacerTests.java @@ -3,7 +3,7 @@ package com.azure.tools.codesnippetplugin.implementation; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.io.File; import java.io.IOException; @@ -16,10 +16,10 @@ import java.util.List; import java.util.Map; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class SnippetReplacerTests { private Path getPathToResource(String fileName) { From dc052c104fa4d6e76ea0344a03d613ea3efbf14f Mon Sep 17 00:00:00 2001 From: Jose Alvarez Date: Mon, 15 Apr 2024 09:02:39 -0700 Subject: [PATCH 02/23] Removed `cleanup` flag from code snippet (#8088) --- doc/common/TypeSpec-Project-Scripts.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/common/TypeSpec-Project-Scripts.md b/doc/common/TypeSpec-Project-Scripts.md index de46a605c62..d6756ba1460 100644 --- a/doc/common/TypeSpec-Project-Scripts.md +++ b/doc/common/TypeSpec-Project-Scripts.md @@ -110,7 +110,6 @@ additionalDirectories: - specification/cognitiveservices/OpenAI.Authoring commit: 14f11cab735354c3e253045f7fbd2f1b9f90f7ca repo: Azure/azure-rest-api-specs -cleanup: false ``` ## TypeSpec-Project-Process.ps1 From f494b727c6a23fa21319d53f6bcc6e23029adfdc Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 15 Apr 2024 12:37:23 -0400 Subject: [PATCH 03/23] Add debug non-safe points to Java profiling (#8089) Add debug non-safe points to Java profiling --- tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Java.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Java.cs b/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Java.cs index bd91f4fca8a..724d101ba63 100644 --- a/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Java.cs +++ b/tools/perf-automation/Azure.Sdk.Tools.PerfAutomation/Java.cs @@ -130,7 +130,7 @@ public override async Task RunAsync( if (profile) { var profileOutputPath = Path.GetFullPath(Path.Combine(Util.GetProfileDirectory(WorkingDirectory), $"{testName}_{profileCount++}.jfr")); - profilingConfig = $"-XX:StartFlightRecording=filename={profileOutputPath},maxsize=1gb"; + profilingConfig = $"-XX:+UnlockDiagnosticVMOptions -XX:+DebugNonSafepoints -XX:StartFlightRecording=filename={profileOutputPath},maxsize=1gb"; // If Java 8 is the version of Java being used add '-XX:+UnlockCommercialFeatures' as that is required to run Java Flight Recording in Java 8. // Don't add '-XX:+UnlockCommercialFeatures' if it is any other version as this causes the JVM to crash on an unrecognized VM options. From f37db1a5713d5826db3403b09990badb9caf0227 Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 15 Apr 2024 12:38:02 -0400 Subject: [PATCH 04/23] Use azure-json in ApiView Generator (#8033) Use azure-json in ApiView Generator --- src/java/apiview-java-processor/pom.xml | 26 ++--- .../azure/tools/apiview/processor/Main.java | 86 ++++++++------ .../apiview/processor/analysers/Analyser.java | 5 +- .../processor/analysers/JavaASTAnalyser.java | 108 +++++++++++------- .../processor/analysers/XMLASTAnalyser.java | 14 +-- .../processor/analysers/util/ASTUtils.java | 53 ++++++++- .../processor/analysers/util/MiscUtils.java | 54 ++++----- .../general/ModuleInfoDiagnosticRule.java | 35 +++--- .../NoLocalesInJavadocUrlDiagnosticRule.java | 2 +- .../FluentSetterReturnTypeDiagnosticRule.java | 2 +- .../apiview/processor/model/APIListing.java | 89 +++++++++------ .../processor/model/ApiViewProperties.java | 46 +++++++- .../apiview/processor/model/ChildItem.java | 56 ++++++--- .../apiview/processor/model/Diagnostic.java | 66 +++++++++-- .../processor/model/DiagnosticKind.java | 17 ++- .../tools/apiview/processor/model/Flavor.java | 32 ++++-- .../processor/model/LanguageVariant.java | 13 ++- .../tools/apiview/processor/model/Tags.java | 39 ++++++- .../tools/apiview/processor/model/Token.java | 68 +++++++++-- .../apiview/processor/model/TokenKind.java | 22 +++- .../apiview/processor/model/TypeKind.java | 12 +- 21 files changed, 586 insertions(+), 259 deletions(-) diff --git a/src/java/apiview-java-processor/pom.xml b/src/java/apiview-java-processor/pom.xml index 9c8e17abe78..953b5f22319 100644 --- a/src/java/apiview-java-processor/pom.xml +++ b/src/java/apiview-java-processor/pom.xml @@ -18,15 +18,15 @@ - com.fasterxml.jackson.core - jackson-databind - 2.17.0 + com.azure + azure-json + 1.1.0 com.github.javaparser - javaparser-symbol-solver-core - 3.25.9 + javaparser-core + 3.25.10 @@ -50,14 +50,14 @@ org.mockito mockito-core - 4.0.0 + 4.11.0 test - - commons-lang - commons-lang - 2.6 - + + org.unbescape + unbescape + 1.1.6.RELEASE + @@ -66,7 +66,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.8.1 + 3.12.1 1.8 1.8 @@ -79,7 +79,7 @@ org.apache.maven.plugins maven-assembly-plugin - 3.1.1 + 3.3.0 jar-with-dependencies diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java index d5eedab17b7..28dc2dcd135 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java @@ -1,19 +1,22 @@ package com.azure.tools.apiview.processor; -import com.azure.tools.apiview.processor.analysers.JavaASTAnalyser; +import com.azure.json.JsonProviders; +import com.azure.json.JsonReader; +import com.azure.json.JsonWriter; import com.azure.tools.apiview.processor.analysers.Analyser; +import com.azure.tools.apiview.processor.analysers.JavaASTAnalyser; import com.azure.tools.apiview.processor.analysers.XMLASTAnalyser; -import com.azure.tools.apiview.processor.model.*; +import com.azure.tools.apiview.processor.model.APIListing; +import com.azure.tools.apiview.processor.model.ApiViewProperties; +import com.azure.tools.apiview.processor.model.Diagnostic; +import com.azure.tools.apiview.processor.model.DiagnosticKind; +import com.azure.tools.apiview.processor.model.LanguageVariant; +import com.azure.tools.apiview.processor.model.Token; import com.azure.tools.apiview.processor.model.maven.Pom; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.ObjectWriter; -import com.fasterxml.jackson.databind.exc.InvalidFormatException; import java.io.File; import java.io.FileInputStream; import java.io.IOException; -import java.net.URL; import java.nio.file.FileSystem; import java.nio.file.FileSystems; import java.nio.file.Files; @@ -26,17 +29,8 @@ import java.util.stream.Stream; import static com.azure.tools.apiview.processor.model.TokenKind.LINE_ID_MARKER; -import static com.fasterxml.jackson.databind.MapperFeature.AUTO_DETECT_CREATORS; -import static com.fasterxml.jackson.databind.MapperFeature.AUTO_DETECT_FIELDS; -import static com.fasterxml.jackson.databind.MapperFeature.AUTO_DETECT_GETTERS; -import static com.fasterxml.jackson.databind.MapperFeature.AUTO_DETECT_IS_GETTERS; public class Main { - private static final ObjectWriter WRITER = new ObjectMapper() - .disable(AUTO_DETECT_CREATORS, AUTO_DETECT_FIELDS, AUTO_DETECT_GETTERS, AUTO_DETECT_IS_GETTERS) - .setSerializationInclusion(JsonInclude.Include.NON_NULL) - .writerWithDefaultPrettyPrinter(); - // expected argument order: // [inputFiles] public static void main(String[] args) throws IOException { @@ -133,7 +127,9 @@ private static void processFile(final File inputFile, final File outputFile) thr } // Write out to the filesystem - WRITER.writeValue(outputFile, apiListing); + try (JsonWriter jsonWriter = JsonProviders.createWriter(Files.newBufferedWriter(outputFile.toPath()))) { + apiListing.toJson(jsonWriter); + } } private static void processJavaSourcesJar(File inputFile, APIListing apiListing) throws IOException { @@ -168,25 +164,7 @@ private static void processJavaSourcesJar(File inputFile, APIListing apiListing) // Read all files within the jar file so that we can create a list of files to analyse final List allFiles = new ArrayList<>(); try (FileSystem fs = FileSystems.newFileSystem(inputFile.toPath(), Main.class.getClassLoader())) { - - try { - // we eagerly load the apiview_properties.json file into an ApiViewProperties object, so that it can - // be used throughout the analysis process, as required - // the filename is [_]apiview_properties.json - String filename = (artifactId != null && !artifactId.isEmpty() ? (artifactId + "_") : "") + "apiview_properties.json"; - URL apiViewPropertiesFile = fs.getPath("/META-INF/" + filename).toUri().toURL(); - final ObjectMapper objectMapper = new ObjectMapper(); - ApiViewProperties properties = objectMapper.readValue(apiViewPropertiesFile, ApiViewProperties.class); - apiListing.setApiViewProperties(properties); - System.out.println(" Found apiview_properties.json file in jar file"); - System.out.println(" - Found " + properties.getCrossLanguageDefinitionIds().size() + " cross-language definition IDs"); - } catch (InvalidFormatException e) { - System.out.println(" ERROR: Unable to parse apiview_properties.json file in jar file"); - e.printStackTrace(); - } catch (Exception e) { - // this is fine, we just won't have any APIView properties to read in - System.out.println(" No apiview_properties.json file found in jar file - continuing..."); - } + tryParseApiViewProperties(fs, apiListing, artifactId); fs.getRootDirectories().forEach(root -> { try (Stream paths = Files.walk(root)) { @@ -205,6 +183,42 @@ private static void processJavaSourcesJar(File inputFile, APIListing apiListing) } } + /** + * Attempts to process the {@code apiview_properties.json} file in the jar file, if it exists. + *

+ * If the file was found and successfully parsed as {@link ApiViewProperties}, it is set on the {@link APIListing} + * object. + * + * @param fs the {@link FileSystem} representing the jar file + * @param apiListing the {@link APIListing} object to set the {@link ApiViewProperties} on + * @param artifactId the artifact ID of the jar file + */ + private static void tryParseApiViewProperties(FileSystem fs, APIListing apiListing, String artifactId) { + // the filename is [_]apiview_properties.json + String artifactName = (artifactId != null && !artifactId.isEmpty()) ? (artifactId + "_") : ""; + String filePath = "/META-INF/" + artifactName + "apiview_properties.json"; + Path apiviewPropertiesPath = fs.getPath(filePath); + if (!Files.exists(apiviewPropertiesPath)) { + System.out.println(" No apiview_properties.json file found in jar file - continuing..."); + return; + } + + try { + // we eagerly load the apiview_properties.json file into an ApiViewProperties object, so that it can + // be used throughout the analysis process, as required + try (JsonReader reader = JsonProviders.createReader(Files.readAllBytes(apiviewPropertiesPath))) { + ApiViewProperties properties = ApiViewProperties.fromJson(reader); + apiListing.setApiViewProperties(properties); + System.out.println(" Found apiview_properties.json file in jar file"); + System.out.println(" - Found " + properties.getCrossLanguageDefinitionIds().size() + + " cross-language definition IDs"); + } + } catch (IOException e) { + System.out.println(" ERROR: Unable to parse apiview_properties.json file in jar file - continuing..."); + e.printStackTrace(); + } + } + private static void processXmlFile(File inputFile, APIListing apiListing) { final ReviewProperties reviewProperties = new ReviewProperties(); diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/Analyser.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/Analyser.java index 903a726eb73..2c01c8b501b 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/Analyser.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/Analyser.java @@ -1,8 +1,7 @@ package com.azure.tools.apiview.processor.analysers; - import java.nio.file.Path; -import java.util.Arrays; +import java.util.Collections; import java.util.List; /** @@ -25,6 +24,6 @@ public interface Analyser { void analyse(List allFiles); default void analyse(Path file) { - analyse(Arrays.asList(file)); + analyse(Collections.singletonList(file)); } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java index 9ae94e720de..21bb153c477 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java @@ -11,8 +11,9 @@ import com.azure.tools.apiview.processor.model.TypeKind; import com.azure.tools.apiview.processor.model.maven.Dependency; import com.azure.tools.apiview.processor.model.maven.Pom; +import com.github.javaparser.JavaParser; +import com.github.javaparser.JavaParserAdapter; import com.github.javaparser.ParserConfiguration; -import com.github.javaparser.StaticJavaParser; import com.github.javaparser.TokenRange; import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.ImportDeclaration; @@ -48,27 +49,36 @@ import com.github.javaparser.ast.type.Type; import com.github.javaparser.ast.type.TypeParameter; import com.github.javaparser.ast.visitor.VoidVisitorAdapter; -import com.github.javaparser.printer.configuration.DefaultPrinterConfiguration; -import com.github.javaparser.symbolsolver.JavaSymbolSolver; -import com.github.javaparser.symbolsolver.resolution.typesolvers.CombinedTypeSolver; -import com.github.javaparser.symbolsolver.resolution.typesolvers.ReflectionTypeSolver; +import org.unbescape.html.HtmlEscape; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.commons.lang.StringEscapeUtils; - -import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.*; -import static com.azure.tools.apiview.processor.analysers.util.MiscUtils.*; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.attemptToFindJavadocComment; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.getNodeFullyQualifiedName; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.getPackageName; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.isInterfaceType; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.isPrivateOrPackagePrivate; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.isPublicOrProtected; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.isTypeAPublicAPI; +import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.makeId; +import static com.azure.tools.apiview.processor.analysers.util.MiscUtils.upperCase; import static com.azure.tools.apiview.processor.analysers.util.TokenModifier.INDENT; import static com.azure.tools.apiview.processor.analysers.util.TokenModifier.NEWLINE; import static com.azure.tools.apiview.processor.analysers.util.TokenModifier.NOTHING; @@ -78,6 +88,8 @@ import static com.azure.tools.apiview.processor.model.TokenKind.DEPRECATED_RANGE_START; import static com.azure.tools.apiview.processor.model.TokenKind.DOCUMENTATION_RANGE_END; import static com.azure.tools.apiview.processor.model.TokenKind.DOCUMENTATION_RANGE_START; +import static com.azure.tools.apiview.processor.model.TokenKind.EXTERNAL_LINK_END; +import static com.azure.tools.apiview.processor.model.TokenKind.EXTERNAL_LINK_START; import static com.azure.tools.apiview.processor.model.TokenKind.KEYWORD; import static com.azure.tools.apiview.processor.model.TokenKind.MEMBER_NAME; import static com.azure.tools.apiview.processor.model.TokenKind.NEW_LINE; @@ -87,8 +99,6 @@ import static com.azure.tools.apiview.processor.model.TokenKind.TEXT; import static com.azure.tools.apiview.processor.model.TokenKind.TYPE_NAME; import static com.azure.tools.apiview.processor.model.TokenKind.WHITESPACE; -import static com.azure.tools.apiview.processor.model.TokenKind.EXTERNAL_LINK_START; -import static com.azure.tools.apiview.processor.model.TokenKind.EXTERNAL_LINK_END; public class JavaASTAnalyser implements Analyser { public static final String MAVEN_KEY = "Maven"; @@ -112,7 +122,19 @@ public class JavaASTAnalyser implements Analyser { ANNOTATION_RULE_MAP.put("Metadata", new AnnotationRule().setShowProperties(true).setCondensed(true)); } - private static final Pattern SPLIT_NEWLINE = Pattern.compile(MiscUtils.LINEBREAK); + private static final JavaParserAdapter JAVA_8_PARSER; + private static final JavaParserAdapter JAVA_11_PARSER; + + static { + // Configure JavaParser to use type resolution + JAVA_8_PARSER = JavaParserAdapter.of(new JavaParser(new ParserConfiguration() + .setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_8) + .setDetectOriginalLineSeparator(false))); + + JAVA_11_PARSER = JavaParserAdapter.of(new JavaParser(new ParserConfiguration() + .setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_11) + .setDetectOriginalLineSeparator(false))); + } // This is the model that we build up as the AST of all files are analysed. The APIListing is then output as // JSON that can be understood by APIView. @@ -205,19 +227,11 @@ private Optional scanForTypes(Path path) { return Optional.of(new ScanClass(path, null)); } try { - // Set up a minimal type solver that only looks at the classes used to run this sample. - CombinedTypeSolver combinedTypeSolver = new CombinedTypeSolver(); - combinedTypeSolver.add(new ReflectionTypeSolver(false)); + CompilationUnit compilationUnit = path.endsWith("module-info.java") + ? JAVA_11_PARSER.parse(Files.newBufferedReader(path)) + : JAVA_8_PARSER.parse(Files.newBufferedReader(path)); - ParserConfiguration parserConfiguration = new ParserConfiguration() - .setStoreTokens(true) - .setSymbolResolver(new JavaSymbolSolver(combinedTypeSolver)) - .setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_11); - - // Configure JavaParser to use type resolution - StaticJavaParser.setConfiguration(parserConfiguration); - - CompilationUnit compilationUnit = StaticJavaParser.parse(path); + compilationUnit.setStorage(path, StandardCharsets.UTF_8); new ScanForClassTypeVisitor().visit(compilationUnit, null); if (path.endsWith("package-info.java")) { @@ -516,7 +530,7 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { addToken(new Token(KEYWORD, "to"), SPACE); for (int i = 0; i < names.size(); i++) { - addToken(new Token(TYPE_NAME, names.get(i).toString())); + addToken(new Token(TYPE_NAME, names.get(i).asString())); if (i < names.size() - 1) { addToken(new Token(PUNCTUATION, ","), SPACE); @@ -571,7 +585,7 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { NodeList names = d.getWith(); for (int i = 0; i < names.size(); i++) { - addToken(new Token(TYPE_NAME, names.get(i).toString())); + addToken(new Token(TYPE_NAME, names.get(i).asString())); if (i < names.size() - 1) { addToken(new Token(PUNCTUATION, ","), SPACE); @@ -827,7 +841,7 @@ private void tokeniseFields(boolean isInterfaceDeclaration, TypeDeclaration t final NodeList fieldModifiers = fieldDeclaration.getModifiers(); // public, protected, static, final for (final Modifier fieldModifier : fieldModifiers) { - addToken(new Token(KEYWORD, fieldModifier.toString())); + addToken(new Token(KEYWORD, fieldModifier.getKeyword().asString())); } // field type and name @@ -1163,7 +1177,7 @@ private void processAnnotationValueExpression(final Expression valueExpr, final private void getModifiers(NodeList modifiers) { for (final Modifier modifier : modifiers) { - addToken(new Token(KEYWORD, modifier.toString())); + addToken(new Token(KEYWORD, modifier.getKeyword().asString())); } } @@ -1251,7 +1265,7 @@ private void getThrowException(CallableDeclaration callableDeclaration) { addToken(new Token(KEYWORD, "throws"), SPACE); for (int i = 0, max = thrownExceptions.size(); i < max; i++) { - final String exceptionName = thrownExceptions.get(i).getElementType().toString(); + final String exceptionName = thrownExceptions.get(i).getElementType().asString(); final Token throwsToken = new Token(TYPE_NAME, exceptionName); // we look up the package name in case it is a custom type in the same library, @@ -1290,7 +1304,7 @@ private void getType(Object type) { private void getClassType(Type type) { if (type.isPrimitiveType()) { - addToken(new Token(TYPE_NAME, type.asPrimitiveType().toString())); + addToken(new Token(TYPE_NAME, type.asPrimitiveType().asString())); } else if (type.isVoidType()) { addToken(new Token(TYPE_NAME, "void")); } else if (type.isReferenceType()) { @@ -1417,7 +1431,7 @@ public void visit(CompilationUnit compilationUnit, Map arg) { compilationUnit.getImports().stream() .map(ImportDeclaration::getName) .forEach(name -> name.getQualifier().ifPresent(packageName -> - apiListing.addPackageTypeMapping(packageName.toString(), name.getIdentifier()))); + apiListing.addPackageTypeMapping(packageName.asString(), name.getIdentifier()))); } } @@ -1468,13 +1482,11 @@ private void visitJavaDoc(JavadocComment javadoc) { // The updated configuration from getDeclarationAsString removes the comment option and hence the toString // returns an empty string now. So, here we are using the toString overload that takes a PrintConfiguration // to get the old behavior. - String javaDocText = javadoc.toString(new DefaultPrinterConfiguration()); - Arrays.stream(SPLIT_NEWLINE.split(javaDocText)).forEach(line -> { + splitNewLine(javadoc.asString()).forEach(line -> { // we want to wrap our javadocs so that they are easier to read, so we wrap at 120 chars - final String wrappedString = MiscUtils.wrap(line, 120); - Arrays.stream(SPLIT_NEWLINE.split(wrappedString)).forEach(line2 -> { + MiscUtils.wrap(line, 120).forEach(line2 -> { if (line2.contains("&")) { - line2 = StringEscapeUtils.unescapeHtml(line2); + line2 = HtmlEscape.unescapeHtml(line2); } addToken(makeWhitespace()); @@ -1515,11 +1527,13 @@ private void unindent() { } private Token makeWhitespace() { - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < indent; i++) { - sb.append(" "); - } - return new Token(WHITESPACE, sb.toString()); + // Use a byte array with Arrays.fill with empty space (' ') character rather than StringBuilder as StringBuilder + // will check that it has sufficient size every time a new character is appended. We know ahead of time the size + // needed and can remove all those checks by removing usage of StringBuilder with this simpler pattern. + byte[] bytes = new byte[indent]; + Arrays.fill(bytes, (byte) ' '); + + return new Token(WHITESPACE, new String(bytes, StandardCharsets.UTF_8)); } private void addComment(String comment) { @@ -1559,4 +1573,12 @@ private void handleTokenModifier(TokenModifier modifier) { break; } } + + private static Stream splitNewLine(String input) { + if (input == null || input.isEmpty()) { + return Stream.empty(); + } + + return Stream.of(input.split("\n")); + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/XMLASTAnalyser.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/XMLASTAnalyser.java index 070e5d4cf13..ac33a082204 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/XMLASTAnalyser.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/XMLASTAnalyser.java @@ -8,11 +8,11 @@ import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; -import java.io.FileInputStream; -import java.io.FileNotFoundException; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; import java.util.List; +import java.util.Objects; import static com.azure.tools.apiview.processor.model.TokenKind.COMMENT; import static com.azure.tools.apiview.processor.model.TokenKind.KEYWORD; @@ -51,7 +51,7 @@ private void processFile(Path file) { XMLStreamReader reader = null; try { - reader = factory.createXMLStreamReader(new FileInputStream(file.toFile())); + reader = factory.createXMLStreamReader(Files.newBufferedReader(file)); while (reader.hasNext()) { final int eventType = reader.next(); @@ -126,7 +126,7 @@ private void processFile(Path file) { } } } - } catch (XMLStreamException | FileNotFoundException e) { + } catch (XMLStreamException | IOException e) { e.printStackTrace(); } finally { if (reader != null) { @@ -174,7 +174,7 @@ private void startElement(final XMLStreamReader reader) { if (attributeCount > 0) { for (int i = 0; i < attributeCount; i++) { final String prefix = reader.getAttributePrefix(i); - final String key = (prefix == "" ? "" : prefix + ":") + reader.getAttributeLocalName(i); + final String key = (Objects.equals(prefix, "") ? "" : prefix + ":") + reader.getAttributeLocalName(i); final String value = reader.getAttributeValue(i); addAttribute(key, value); @@ -256,4 +256,4 @@ private void addToken(Token token) { apiListing.getTokens().add(token); lastToken = token; } -} \ No newline at end of file +} diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/ASTUtils.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/ASTUtils.java index aea307a1fd4..5f979ed8c94 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/ASTUtils.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/ASTUtils.java @@ -25,14 +25,12 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.regex.Pattern; import java.util.stream.Stream; /** * Abstract syntax tree (AST) utility methods. */ public final class ASTUtils { - private static final Pattern MAKE_ID = Pattern.compile("\"| "); /** * Attempts to get the package name for a compilation unit. @@ -118,7 +116,9 @@ public static Stream getPublicOrProtectedConstructors(Co * @return All public API constructors contained in the type declaration. */ public static Stream getPublicOrProtectedConstructors(TypeDeclaration typeDeclaration) { - return typeDeclaration.getConstructors().stream() + return typeDeclaration.getMembers().stream() + .filter(member -> member instanceof ConstructorDeclaration) + .map(member -> (ConstructorDeclaration) member) .filter(type -> isPublicOrProtected(type.getAccessSpecifier())); } @@ -139,7 +139,9 @@ public static Stream getPublicOrProtectedMethods(CompilationU * @return All public API methods contained in the type declaration. */ public static Stream getPublicOrProtectedMethods(TypeDeclaration typeDeclaration) { - return typeDeclaration.getMethods().stream() + return typeDeclaration.getMembers().stream() + .filter(member -> member instanceof MethodDeclaration) + .map(member -> (MethodDeclaration) member) .filter(type -> isPublicOrProtected(type.getAccessSpecifier())); } @@ -160,7 +162,9 @@ public static Stream getPublicOrProtectedFields(CompilationUni * @return All public API fields contained in the type declaration. */ public static Stream getPublicOrProtectedFields(TypeDeclaration typeDeclaration) { - return typeDeclaration.getFields().stream() + return typeDeclaration.getMembers().stream() + .filter(member -> member instanceof FieldDeclaration) + .map(member -> (FieldDeclaration) member) .filter(type -> isPublicOrProtected(type.getAccessSpecifier())); } @@ -209,7 +213,44 @@ public static String makeId(EnumConstantDeclaration enumDeclaration) { } public static String makeId(String fullPath) { - return MAKE_ID.matcher(fullPath).replaceAll("-"); + // Previously, this used a regex to replace '"' and ' ' with '-', that wasn't necessary. The replacement pattern + // is simple and can be replaced with a simple loop. This is a performance optimization. + // + // The logic is that we iterate over the string, if no replacements are needed we return the original string. + // Otherwise, we create a new StringBuilder the size of the string being replaced, as the replacement size is + // the same as the search size, and we append the parts of the string that don't need to be replaced, and the + // replacement character for the parts that do. At the end of the loop, we append the last part of the string + // that doesn't need to be replaced and return the final string. + if (fullPath == null || fullPath.isEmpty()) { + return fullPath; + } + + StringBuilder sb = null; + int prevStart = 0; + + int length = fullPath.length(); + for (int i = 0; i < length; i++) { + char c = fullPath.charAt(i); + if (c == '"' || c == ' ') { + if (sb == null) { + sb = new StringBuilder(length); + } + + if (prevStart != i) { + sb.append(fullPath, prevStart, i); + } + + sb.append('-'); + prevStart = i + 1; + } + } + + if (sb == null) { + return fullPath; + } + + sb.append(fullPath, prevStart, length); + return sb.toString(); } public static String makeId(AnnotationExpr annotation, NodeWithAnnotations nodeWithAnnotations) { diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/MiscUtils.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/MiscUtils.java index 85ba4a865e2..6bdd018e527 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/MiscUtils.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/util/MiscUtils.java @@ -2,6 +2,8 @@ import com.azure.tools.apiview.processor.model.Token; +import java.util.ArrayList; +import java.util.List; import java.util.regex.Pattern; import static com.azure.tools.apiview.processor.model.TokenKind.TEXT; @@ -10,33 +12,17 @@ * Miscellaneous utility methods. */ public final class MiscUtils { + public static final Pattern URL_MATCH = Pattern.compile( + "https?://(www\\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}\\.[a-zA-Z0-9()]{1,6}\\b([-a-zA-Z0-9()@:%_+.~#?&/=]*)"); - public static final String LINEBREAK = "\n"; // or "\r\n"; - private static final Pattern QUOTED_LINEBREAK = Pattern.compile(Pattern.quote(LINEBREAK)); - - public static final Pattern URL_MATCH = Pattern.compile("https?:\\/\\/(www\\.)?[-a-zA-Z0-9@:%._\\+~#=]{1,256}\\.[a-zA-Z0-9()]{1,6}\\b([-a-zA-Z0-9()@:%_\\+.~#?&//=]*)"); - - public static String escapeHTML(final String s) { - final StringBuilder out = new StringBuilder(Math.max(16, s.length())); - for (int i = 0; i < s.length(); i++) { - final char c = s.charAt(i); - if (c > 127 || c == '"' || c == '\'' || c == '<' || c == '>' || c == '&') { - out.append("&#"); - out.append((int) c); - out.append(';'); - } else { - out.append(c); - } - } - return out.toString(); - } + public static List wrap(String string, int lineLength) { + List wrappedLines = new ArrayList<>(); - public static String wrap(final String string, final int lineLength) { - final StringBuilder b = new StringBuilder(); - for (final String line : QUOTED_LINEBREAK.split(string)) { - b.append(wrapLine(line, lineLength)); + for (String line : string.split("\n")) { + wrapLine(line, lineLength, wrappedLines); } - return b.toString(); + + return wrappedLines; } /** @@ -63,28 +49,30 @@ public static Token tokeniseKeyValue(String key, Object value, String prefix) { return new Token(TEXT, value == null ? "" : value.toString(), prefix + key + "-" + value); } - private static String wrapLine(final String line, final int lineLength) { - if (line.isEmpty()) return LINEBREAK; - if (line.length() <= lineLength) return line + LINEBREAK; + private static void wrapLine(String line, int lineLength, List collector) { + if (line.isEmpty()) { + return; + } + + if (line.length() <= lineLength) { + collector.add(line); + return; + } final String[] words = line.split(" "); - final StringBuilder allLines = new StringBuilder(); StringBuilder trimmedLine = new StringBuilder(); for (final String word : words) { if (trimmedLine.length() + 1 + word.length() > lineLength) { - allLines.append(trimmedLine).append(LINEBREAK); + collector.add(trimmedLine.toString()); trimmedLine = new StringBuilder(); } trimmedLine.append(word).append(" "); } if (trimmedLine.length() > 0) { - allLines.append(trimmedLine); + collector.add(trimmedLine.toString()); } - - allLines.append(LINEBREAK); - return allLines.toString(); } /** diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/ModuleInfoDiagnosticRule.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/ModuleInfoDiagnosticRule.java index 653b8effa90..d1e3b952ff8 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/ModuleInfoDiagnosticRule.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/ModuleInfoDiagnosticRule.java @@ -11,9 +11,7 @@ import java.util.Comparator; import java.util.HashSet; -import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import static com.azure.tools.apiview.processor.analysers.util.ASTUtils.makeId; @@ -41,27 +39,32 @@ public void scanFinal(APIListing listing) { .min(Comparator.comparingInt(String::length)) .orElse(""); - // Check for the presence of module-info - Optional moduleInfoToken = listing.getTokens().stream() - .filter(token -> token.getKind().equals(TokenKind.TYPE_NAME)) - .filter(token -> token.getDefinitionId() != null && token.getDefinitionId().equals(JavaASTAnalyser.MODULE_INFO_KEY)) - .findFirst(); + Token moduleInfoToken = null; + Set exportsPackages = new HashSet<>(); - // Collect all packages that are exported - Set exportsPackages = listing.getTokens().stream() - .filter(token -> token.getKind().equals(TokenKind.TYPE_NAME)) - .filter(token -> token.getDefinitionId() != null && token.getDefinitionId().startsWith("module-info" + - "-exports")) - .map(token -> token.getValue()) - .collect(Collectors.toSet()); + for (Token token : listing.getTokens()) { + if (!TokenKind.TYPE_NAME.equals(token.getKind()) || token.getDefinitionId() == null) { + continue; + } + + // Check for the presence of module-info + if (token.getDefinitionId().equals(JavaASTAnalyser.MODULE_INFO_KEY) && moduleInfoToken == null) { + moduleInfoToken = token; + } + + // Collect all packages that are exported + if (token.getDefinitionId().startsWith("module-info-exports")) { + exportsPackages.add(token.getValue()); + } + } - if (!moduleInfoToken.isPresent()) { + if (moduleInfoToken == null) { listing.addDiagnostic(new Diagnostic(DiagnosticKind.WARNING, makeId(basePackageName), "This module is missing module-info.java")); return; } - String moduleName = moduleInfoToken.get().getValue(); + String moduleName = moduleInfoToken.getValue(); if (moduleName != null) { // special casing azure-core as the base package doesn't have any classes and hence not included in the // list of packages diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/NoLocalesInJavadocUrlDiagnosticRule.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/NoLocalesInJavadocUrlDiagnosticRule.java index 1a2c0b380b5..4415b6458b8 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/NoLocalesInJavadocUrlDiagnosticRule.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/general/NoLocalesInJavadocUrlDiagnosticRule.java @@ -52,7 +52,7 @@ public void scanIndividual(final CompilationUnit cu, final APIListing listing) { void checkJavaDoc(NodeWithJavadoc n, String id, APIListing listing) { n.getJavadocComment().ifPresent(javadoc -> { - final String javadocString = javadoc.toString().toLowerCase(); + final String javadocString = javadoc.asString().toLowerCase(); Matcher matcher = LANGUAGE_PATTERN.matcher(javadocString); while (matcher.find()) { diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/utils/FluentSetterReturnTypeDiagnosticRule.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/utils/FluentSetterReturnTypeDiagnosticRule.java index cdf9950f49c..9884d57729c 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/utils/FluentSetterReturnTypeDiagnosticRule.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/diagnostics/rules/utils/FluentSetterReturnTypeDiagnosticRule.java @@ -34,7 +34,7 @@ private void processFluentType(final TypeDeclaration type, final APIListing l getPublicOrProtectedMethods(type) .filter(method -> method.getNameAsString().startsWith("set")) .forEach(method -> { - if (!method.getType().toString().equals(typeName)) { + if (!method.getType().asString().equals(typeName)) { listing.addDiagnostic(new Diagnostic( ERROR, makeId(method), diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/APIListing.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/APIListing.java index ca9606aa481..9840081d12b 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/APIListing.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/APIListing.java @@ -1,62 +1,39 @@ package com.azure.tools.apiview.processor.model; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonToken; +import com.azure.json.JsonWriter; import com.azure.tools.apiview.processor.model.maven.Pom; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonProperty; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; -public class APIListing { - @JsonProperty("Navigation") +public class APIListing implements JsonSerializable { private List navigation; - - @JsonIgnore private ChildItem rootNav; - - @JsonProperty("Name") private String name; - - @JsonProperty("Language") private String language; - - @JsonProperty("LanguageVariant") private LanguageVariant languageVariant; - - @JsonProperty("PackageName") private String packageName; - - @JsonProperty("PackageVersion") private String packageVersion; // This string is taken from here: // https://github.com/Azure/azure-sdk-tools/blob/main/src/dotnet/APIView/APIView/Languages/CodeFileBuilder.cs#L50 - @JsonProperty("VersionString") private final String versionString = "21"; - - @JsonProperty("Tokens") private List tokens; - - @JsonProperty("Diagnostics") private List diagnostics; - - @JsonIgnore private Map knownTypes; // a map of package names to a list of types within that package - @JsonIgnore private final Map> packageNamesToTypesMap; - - @JsonIgnore private final Map typeToPackageNameMap; - - @JsonIgnore private Pom mavenPom; - - @JsonIgnore private ApiViewProperties apiViewProperties; public APIListing() { @@ -132,7 +109,7 @@ public void setTokens(List tokens) { @Override public String toString() { - return "APIListing [rootNav = "+rootNav+", Name = "+ name +", Tokens = "+tokens+"]"; + return "APIListing [rootNav = " + rootNav + ", Name = " + name + ", Tokens = " + tokens + "]"; } /** @@ -170,4 +147,52 @@ public void setApiViewProperties(ApiViewProperties properties) { public ApiViewProperties getApiViewProperties() { return apiViewProperties; } -} \ No newline at end of file + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + return jsonWriter.writeStartObject() + .writeArrayField("Navigation", navigation, JsonWriter::writeJson) + .writeStringField("Name", name) + .writeStringField("Language", language) + .writeStringField("LanguageVariant", Objects.toString(languageVariant, null)) + .writeStringField("PackageName", packageName) + .writeStringField("PackageVersion", packageVersion) + .writeStringField("VersionString", versionString) + .writeArrayField("Tokens", tokens, JsonWriter::writeJson) + .writeArrayField("Diagnostics", diagnostics, JsonWriter::writeJson) + .writeEndObject(); + } + + public static APIListing fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + APIListing apiListing = new APIListing(); + + while (reader.nextToken() != JsonToken.END_OBJECT) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if ("Navigation".equals(fieldName)) { + apiListing.navigation = reader.readArray(ChildItem::fromJson); + } else if ("Name".equals(fieldName)) { + apiListing.name = reader.getString(); + } else if ("Language".equals(fieldName)) { + apiListing.language = reader.getString(); + } else if ("LanguageVariant".equals(fieldName)) { + apiListing.languageVariant = LanguageVariant.fromString(reader.getString()); + } else if ("PackageName".equals(fieldName)) { + apiListing.packageName = reader.getString(); + } else if ("PackageVersion".equals(fieldName)) { + apiListing.packageVersion = reader.getString(); + } else if ("Tokens".equals(fieldName)) { + apiListing.tokens = reader.readArray(Token::fromJson); + } else if ("Diagnostics".equals(fieldName)) { + apiListing.diagnostics = reader.readArray(Diagnostic::fromJson); + } else { + reader.skipChildren(); + } + } + + return apiListing; + }); + } +} diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ApiViewProperties.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ApiViewProperties.java index 2781c90adda..e7f4fef61ee 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ApiViewProperties.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ApiViewProperties.java @@ -1,7 +1,11 @@ package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonToken; +import com.azure.json.JsonWriter; +import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -11,14 +15,11 @@ * Sometimes libraries carry additional metadata with them that can make the output from APIView more useful. This * class is used to store that metadata, as it is deserialized from the /META-INF/apiview_properties.json file. */ -public class ApiViewProperties { - - @JsonProperty("flavor") +public class ApiViewProperties implements JsonSerializable { private Flavor flavor; // This is a map of model names and methods to their TypeSpec definition IDs. - @JsonProperty("CrossLanguageDefinitionId") - private final Map crossLanguageDefinitionIds = new HashMap<>(); + private Map crossLanguageDefinitionIds = new HashMap<>(); /** * Cross Languages Definition ID is used to map from a model name or a method name to a TypeSpec definition ID. This @@ -39,4 +40,37 @@ public Map getCrossLanguageDefinitionIds() { public Flavor getFlavor() { return flavor; } + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + jsonWriter.writeStartObject() + .writeMapField("CrossLanguageDefinitionId", crossLanguageDefinitionIds, JsonWriter::writeString); + + if (flavor != null) { + jsonWriter.writeStringField("flavor", flavor.getSerializationValue()); + } + + return jsonWriter.writeEndObject(); + } + + public static ApiViewProperties fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + ApiViewProperties properties = new ApiViewProperties(); + + while (reader.nextToken() != JsonToken.END_OBJECT) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if ("CrossLanguageDefinitionId".equals(fieldName)) { + properties.crossLanguageDefinitionIds = reader.readMap(JsonReader::getString); + } else if ("flavor".equals(fieldName)) { + properties.flavor = Flavor.fromSerializationValue(reader.getString()); + } else { + reader.skipChildren(); + } + } + + return properties; + }); + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ChildItem.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ChildItem.java index 37f882eb600..825413d7944 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ChildItem.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/ChildItem.java @@ -1,29 +1,24 @@ package com.azure.tools.apiview.processor.model; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonToken; +import com.azure.json.JsonWriter; import com.azure.tools.apiview.processor.analysers.JavaASTAnalyser; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonProperty; +import java.io.IOException; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.TreeSet; -public class ChildItem implements Comparable { - @JsonProperty("ChildItems") +public class ChildItem implements Comparable, JsonSerializable { private Set childItems; - - @JsonIgnore private Map packageNameToChildMap; - - @JsonProperty("NavigationId") private String navigationId; - - @JsonProperty("Text") private String text; - - @JsonProperty("Tags") private Tags tags; public ChildItem(final String text, TypeKind typeKind) { @@ -111,4 +106,39 @@ public boolean equals(final Object o) { public int hashCode() { return Objects.hash(text); } -} \ No newline at end of file + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + return jsonWriter.writeStartObject() + .writeArrayField("ChildItems", childItems, JsonWriter::writeJson) + .writeStringField("NavigationId", navigationId) + .writeStringField("Text", text) + .writeJsonField("Tags", tags) + .writeEndObject(); + } + + public static ChildItem fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + ChildItem childItem = new ChildItem(null, null, null); + + while (reader.nextToken() != JsonToken.END_OBJECT) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if ("ChildItems".equals(fieldName)) { + childItem.childItems = new LinkedHashSet<>(reader.readArray(ChildItem::fromJson)); + } else if ("NavigationId".equals(fieldName)) { + childItem.navigationId = reader.getString(); + } else if ("Text".equals(fieldName)) { + childItem.text = reader.getString(); + } else if ("Tags".equals(fieldName)) { + childItem.tags = Tags.fromJson(reader); + } else { + reader.skipChildren(); + } + } + + return childItem; + }); + } +} diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Diagnostic.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Diagnostic.java index 1efddb63e0b..aa40c5e84d7 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Diagnostic.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Diagnostic.java @@ -1,23 +1,18 @@ package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonWriter; -public class Diagnostic { +import java.io.IOException; + +public class Diagnostic implements JsonSerializable { private static int diagnosticIdCounter = 1; - @JsonProperty("DiagnosticId") private String diagnosticId; - - @JsonProperty("Text") private String text; - - @JsonProperty("HelpLinkUri") private String helpLinkUri; - - @JsonProperty("TargetId") private String targetId; - - @JsonProperty("Level") private DiagnosticKind level; public Diagnostic(DiagnosticKind level, String targetId, String text) { @@ -39,4 +34,53 @@ public String getText() { public String getHelpLinkUri() { return helpLinkUri; } + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + jsonWriter.writeStartObject() + .writeStringField("DiagnosticId", diagnosticId) + .writeStringField("Text", text) + .writeStringField("HelpLinkUri", helpLinkUri) + .writeStringField("TargetId", targetId); + + if (level != null) { + jsonWriter.writeIntField("Level", level.getLevel()); + } + + return jsonWriter.writeEndObject(); + } + + public static Diagnostic fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + String diagnosticId = null; + String text = null; + String helpLinkUri = null; + String targetId = null; + DiagnosticKind level = null; + + while (reader.nextToken() != null) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if ("DiagnosticId".equals(fieldName)) { + diagnosticId = reader.getString(); + } else if ("Text".equals(fieldName)) { + text = reader.getString(); + } else if ("HelpLinkUri".equals(fieldName)) { + helpLinkUri = reader.getString(); + } else if ("TargetId".equals(fieldName)) { + targetId = reader.getString(); + } else if ("Level".equals(fieldName)) { + level = DiagnosticKind.fromInt(reader.getInt()); + } else { + reader.skipChildren(); + } + } + + Diagnostic diagnostic = new Diagnostic(level, targetId, text, helpLinkUri); + diagnostic.diagnosticId = diagnosticId; + + return diagnostic; + }); + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/DiagnosticKind.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/DiagnosticKind.java index 5e34bcd39a9..f8afa6c61ac 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/DiagnosticKind.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/DiagnosticKind.java @@ -1,6 +1,5 @@ -package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonValue; +package com.azure.tools.apiview.processor.model; public enum DiagnosticKind { ERROR(3), // red @@ -13,7 +12,19 @@ public enum DiagnosticKind { this.level = level; } - @JsonValue + public static DiagnosticKind fromInt(int level) { + switch (level) { + case 3: + return ERROR; + case 2: + return WARNING; + case 1: + return INFO; + default: + return null; + } + } + public int getLevel() { return level; } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Flavor.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Flavor.java index 9563c4c5e8d..5bd8ebca95a 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Flavor.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Flavor.java @@ -1,27 +1,37 @@ package com.azure.tools.apiview.processor.model; import com.azure.tools.apiview.processor.model.maven.Dependency; -import com.fasterxml.jackson.annotation.JsonProperty; public enum Flavor { - @JsonProperty("azure") - AZURE("com.azure"), - - @JsonProperty("generic") - GENERIC("io.clientcore"), - - UNKNOWN(null); + AZURE("com.azure", "azure"), + GENERIC("io.clientcore", "generic"), + UNKNOWN(null, "unknown"); private final String packagePrefix; + private final String serializationValue; - private Flavor(String packagePrefix) { + Flavor(String packagePrefix, String serializationValue) { this.packagePrefix = packagePrefix; + this.serializationValue = serializationValue; } public String getPackagePrefix() { return packagePrefix; } + public String getSerializationValue() { + return serializationValue; + } + + public static Flavor fromSerializationValue(String serializationValue) { + for (Flavor flavor : values()) { + if (flavor.getSerializationValue().equals(serializationValue)) { + return flavor; + } + } + return UNKNOWN; + } + public static Flavor getFlavor(APIListing apiListing) { Flavor flavor = apiListing.getApiViewProperties().getFlavor(); if (flavor != null) { @@ -61,6 +71,4 @@ public static Flavor getFlavor(APIListing apiListing) { // see which count is greatest (and non-zero), and return that flavour. If equal, return unknown return azureCount > genericCount ? AZURE : genericCount > azureCount ? GENERIC : UNKNOWN; } - - -} \ No newline at end of file +} diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/LanguageVariant.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/LanguageVariant.java index 329e2e82716..65d077740f7 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/LanguageVariant.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/LanguageVariant.java @@ -1,6 +1,5 @@ -package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonValue; +package com.azure.tools.apiview.processor.model; public enum LanguageVariant { DEFAULT("Default"), @@ -14,8 +13,16 @@ public enum LanguageVariant { } @Override - @JsonValue public String toString() { return this.variantName; } + + public static LanguageVariant fromString(String name) { + for (LanguageVariant variant : LanguageVariant.values()) { + if (variant.toString().equals(name)) { + return variant; + } + } + return null; + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Tags.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Tags.java index a009a1e7faa..038d2534c23 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Tags.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Tags.java @@ -1,9 +1,13 @@ package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonToken; +import com.azure.json.JsonWriter; -public class Tags { - @JsonProperty("TypeKind") +import java.io.IOException; + +public class Tags implements JsonSerializable { private TypeKind typeKind; public Tags(TypeKind typeKind) { @@ -22,4 +26,33 @@ public void setTypeKind(TypeKind TypeKind) { public String toString() { return "Tags [typeKind = "+ typeKind +"]"; } + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + jsonWriter.writeStartObject(); + + if (typeKind != null) { + jsonWriter.writeStringField("TypeKind", typeKind.getName()); + } + + return jsonWriter.writeEndObject(); + } + + public static Tags fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + TypeKind typeKind = null; + + while (reader.nextToken() != JsonToken.END_OBJECT) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if (fieldName.equals("TypeKind")) { + typeKind = TypeKind.fromName(reader.getString()); + } else { + reader.skipChildren(); + } + } + return new Tags(typeKind); + }); + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Token.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Token.java index e51ca69db93..2d74e985b9d 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Token.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/Token.java @@ -1,21 +1,17 @@ + package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.azure.json.JsonReader; +import com.azure.json.JsonSerializable; +import com.azure.json.JsonWriter; -public class Token { - @JsonProperty("DefinitionId") - private String definitionId; +import java.io.IOException; - @JsonProperty("NavigateToId") +public class Token implements JsonSerializable { + private String definitionId; private String navigateToId; - - @JsonProperty("Kind") private TokenKind kind; - - @JsonProperty("Value") private String value; - - @JsonProperty("CrossLanguageDefinitionId") private String crossLanguageDefinitionId; public Token(final TokenKind kind) { @@ -80,4 +76,54 @@ public void setValue(String Value) { public String toString() { return "Token [definitionId = "+definitionId+", navigateToId = "+navigateToId+", kind = "+kind+", value = "+value+"]"; } + + @Override + public JsonWriter toJson(JsonWriter jsonWriter) throws IOException { + jsonWriter.writeStartObject() + .writeStringField("DefinitionId", definitionId) + .writeStringField("NavigateToId", navigateToId); + + if (kind != null) { + jsonWriter.writeIntField("Kind", kind.getId()); + } + + return jsonWriter.writeStringField("Value", value) + .writeStringField("CrossLanguageDefinitionId", crossLanguageDefinitionId) + .writeEndObject(); + } + + public static Token fromJson(JsonReader jsonReader) throws IOException { + return jsonReader.readObject(reader -> { + String definitionId = null; + String navigateToId = null; + TokenKind kind = null; + String value = null; + String crossLanguageDefinitionId = null; + + while (reader.nextToken() != null) { + String fieldName = reader.getFieldName(); + reader.nextToken(); + + if (fieldName.equals("DefinitionId")) { + definitionId = reader.getString(); + } else if (fieldName.equals("NavigateToId")) { + navigateToId = reader.getString(); + } else if (fieldName.equals("Kind")) { + kind = TokenKind.fromId(reader.getInt()); + } else if (fieldName.equals("Value")) { + value = reader.getString(); + } else if (fieldName.equals("CrossLanguageDefinitionId")) { + crossLanguageDefinitionId = reader.getString(); + } else { + reader.skipChildren(); + } + } + + Token token = new Token(kind, value, definitionId); + token.setNavigateToId(navigateToId); + token.setCrossLanguageDefinitionId(crossLanguageDefinitionId); + + return token; + }); + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TokenKind.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TokenKind.java index 686c7a2d88c..493aba9c39f 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TokenKind.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TokenKind.java @@ -1,6 +1,5 @@ -package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonValue; +package com.azure.tools.apiview.processor.model; public enum TokenKind { TEXT(0), @@ -36,14 +35,31 @@ public enum TokenKind { EXTERNAL_LINK_START(28), EXTERNAL_LINK_END(29); + private static final TokenKind[] VALUES; + + static { + VALUES = new TokenKind[30]; + for (TokenKind kind : TokenKind.values()) { + VALUES[kind.id] = kind; + } + } + private final int id; TokenKind(int id) { this.id = id; } - @JsonValue public int getId() { return id; } + + public static TokenKind fromId(int id) { + for (TokenKind kind : TokenKind.values()) { + if (kind.id == id) { + return kind; + } + } + return null; + } } diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TypeKind.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TypeKind.java index c26ee1b1412..a2921937310 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TypeKind.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/model/TypeKind.java @@ -1,7 +1,5 @@ package com.azure.tools.apiview.processor.model; -import com.fasterxml.jackson.annotation.JsonValue; - public enum TypeKind { ASSEMBLY("assembly"), // i.e. a Jar File NAMESPACE("namespace"), // i.e. a Java package @@ -20,7 +18,6 @@ public enum TypeKind { this.name = name; } - @JsonValue public String getName() { return name; } @@ -34,4 +31,13 @@ public static TypeKind fromClass(Class cls) { return CLASS; } } + + public static TypeKind fromName(String name) { + for (TypeKind typeKind : TypeKind.values()) { + if (typeKind.getName().equals(name)) { + return typeKind; + } + } + return null; + } } From 16404705d42ebfbbb55d9aa885e7456ad9cceba6 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Mon, 15 Apr 2024 10:50:49 -0700 Subject: [PATCH 05/23] Add service account login to abstract devops store base class (#8082) --- .../Commands/RotationCommandBase.cs | 2 +- .../AzureDevOpsStore.cs | 104 ++++++++++++++++++ .../ServiceAccountPersonalAccessTokenStore.cs | 74 +++---------- .../ServiceConnectionParameterStore.cs | 83 ++++++++------ 4 files changed, 171 insertions(+), 92 deletions(-) create mode 100644 tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/AzureDevOpsStore.cs diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs index f5eacac0c96..28e73cc965f 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs @@ -97,7 +97,7 @@ private static IDictionary> GetDef [KeyVaultCertificateStore.MappingKey] = KeyVaultCertificateStore.GetSecretStoreFactory(tokenCredential, logger), [ManualActionStore.MappingKey] = ManualActionStore.GetSecretStoreFactory(logger, new ConsoleValueProvider()), [ServiceAccountPersonalAccessTokenStore.MappingKey] = ServiceAccountPersonalAccessTokenStore.GetSecretStoreFactory(tokenCredential, new SecretProvider(logger), logger), - [ServiceConnectionParameterStore.MappingKey] = ServiceConnectionParameterStore.GetSecretStoreFactory(tokenCredential, logger), + [ServiceConnectionParameterStore.MappingKey] = ServiceConnectionParameterStore.GetSecretStoreFactory(tokenCredential, new SecretProvider(logger), logger), [AadApplicationSecretStore.MappingKey] = AadApplicationSecretStore.GetSecretStoreFactory(tokenCredential, logger), [AzureWebsiteStore.MappingKey] = AzureWebsiteStore.GetSecretStoreFactory(tokenCredential, logger), [AadApplicationSecretStore.MappingKey] = AadApplicationSecretStore.GetSecretStoreFactory(tokenCredential, logger), diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/AzureDevOpsStore.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/AzureDevOpsStore.cs new file mode 100644 index 00000000000..958e2dd7e03 --- /dev/null +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/AzureDevOpsStore.cs @@ -0,0 +1,104 @@ +using Azure.Core; +using Azure.Identity; +using Azure.Sdk.Tools.SecretRotation.Azure; +using Azure.Sdk.Tools.SecretRotation.Core; +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.Services.Client; +using Microsoft.VisualStudio.Services.Common; +using Microsoft.VisualStudio.Services.WebApi; +using AccessToken = Azure.Core.AccessToken; + +namespace Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps; + +public abstract class AzureDevOpsStore : SecretStore +{ + private const string AzureDevOpsApplicationId = "499b84ac-1321-427f-aa17-267ca6975798"; + private const string AzureCliApplicationId = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"; + + protected readonly ILogger logger; + protected readonly string organization; + private readonly string? serviceAccountTenantId; + private readonly string? serviceAccountName; + private readonly Uri? serviceAccountPasswordSecret; + private readonly TokenCredential tokenCredential; + private readonly ISecretProvider secretProvider; + + protected AzureDevOpsStore(ILogger logger, + string organization, + string? serviceAccountTenantId, + string? serviceAccountName, + Uri? serviceAccountPasswordSecret, + TokenCredential tokenCredential, + ISecretProvider secretProvider) + { + this.logger = logger; + this.organization = organization; + this.serviceAccountTenantId = serviceAccountTenantId; + this.serviceAccountName = serviceAccountName; + this.serviceAccountPasswordSecret = serviceAccountPasswordSecret; + this.tokenCredential = tokenCredential; + this.secretProvider = secretProvider; + } + + protected async Task GetConnectionAsync() + { + string[] scopes = { "499b84ac-1321-427f-aa17-267ca6975798/.default" }; + string? parentRequestId = null; + + VssCredentials vssCredentials = this.serviceAccountPasswordSecret != null + ? await GetServiceAccountCredentialsAsync() + : await GetTokenPrincipleCredentialsAsync(scopes, parentRequestId); + + VssConnection connection = new(new Uri($"https://vssps.dev.azure.com/{this.organization}"), vssCredentials); + + await connection.ConnectAsync(); + + return connection; + } + + private static async Task GetDevopsAadBearerTokenAsync(TokenCredential credential) + { + string[] scopes = { $"{AzureDevOpsApplicationId}/.default" }; + + var tokenRequestContext = new TokenRequestContext(scopes, parentRequestId: null); + + AccessToken authenticationResult = await credential.GetTokenAsync(tokenRequestContext, CancellationToken.None); + + return authenticationResult; + } + + private async Task GetTokenPrincipleCredentialsAsync(string[] scopes, string? parentRequestId) + { + TokenRequestContext tokenRequestContext = new (scopes, parentRequestId); + + AccessToken authenticationResult = await this.tokenCredential.GetTokenAsync( + tokenRequestContext, + CancellationToken.None); + + VssAadCredential credentials = new (new VssAadToken("Bearer", authenticationResult.Token)); + + return credentials; + } + + private async Task GetServiceAccountCredentialsAsync() + { + if (this.serviceAccountPasswordSecret == null) + { + throw new ArgumentNullException(nameof(serviceAccountPasswordSecret)); + } + + this.logger.LogDebug("Getting service account password from secret '{SecretName}'", this.serviceAccountPasswordSecret); + + string serviceAccountPassword = await this.secretProvider.GetSecretValueAsync(this.tokenCredential, this.serviceAccountPasswordSecret); + + this.logger.LogDebug("Getting token and devops client for 'dev.azure.com/{Organization}'", this.organization); + + UsernamePasswordCredential serviceAccountCredential = new (this.serviceAccountName, serviceAccountPassword, this.serviceAccountTenantId, AzureCliApplicationId); + + AccessToken token = await GetDevopsAadBearerTokenAsync(serviceAccountCredential); + + VssAadCredential credentials = new (new VssAadToken("Bearer", token.Token)); + + return credentials; + } +} diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceAccountPersonalAccessTokenStore.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceAccountPersonalAccessTokenStore.cs index ca5f0f49023..bbe3868b412 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceAccountPersonalAccessTokenStore.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceAccountPersonalAccessTokenStore.cs @@ -1,38 +1,27 @@ +using System.Net; using System.Text.Json; using System.Text.Json.Serialization; using Azure.Core; -using Azure.Identity; using Azure.Sdk.Tools.SecretRotation.Azure; using Azure.Sdk.Tools.SecretRotation.Configuration; using Azure.Sdk.Tools.SecretRotation.Core; using Microsoft.Extensions.Logging; -using Microsoft.VisualStudio.Services.Client; -using Microsoft.VisualStudio.Services.Common; using Microsoft.VisualStudio.Services.DelegatedAuthorization; using Microsoft.VisualStudio.Services.DelegatedAuthorization.Client; using Microsoft.VisualStudio.Services.WebApi; -using AccessToken = Azure.Core.AccessToken; namespace Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps; -public class ServiceAccountPersonalAccessTokenStore : SecretStore +public class ServiceAccountPersonalAccessTokenStore : AzureDevOpsStore { public const string MappingKey = "Service Account ADO PAT"; - private const string AzureDevOpsApplicationId = "499b84ac-1321-427f-aa17-267ca6975798"; - private const string AzureCliApplicationId = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"; - private readonly ILogger logger; - private readonly string organization; private readonly string patDisplayName; private readonly string scopes; - private readonly string serviceAccountTenantId; - private readonly string serviceAccountName; - private readonly Uri serviceAccountPasswordSecret; private readonly RevocationAction revocationAction; - private readonly TokenCredential tokenCredential; - private readonly ISecretProvider secretProvider; - private ServiceAccountPersonalAccessTokenStore(ILogger logger, + private ServiceAccountPersonalAccessTokenStore( + ILogger logger, string organization, string patDisplayName, string scopes, @@ -42,17 +31,18 @@ private ServiceAccountPersonalAccessTokenStore(ILogger logger, RevocationAction revocationAction, TokenCredential tokenCredential, ISecretProvider secretProvider) + : base( + logger, + organization, + serviceAccountTenantId, + serviceAccountName, + serviceAccountPasswordSecret, + tokenCredential, + secretProvider) { - this.logger = logger; - this.organization = organization; this.patDisplayName = patDisplayName; this.scopes = scopes; - this.serviceAccountTenantId = serviceAccountTenantId; - this.serviceAccountName = serviceAccountName; - this.serviceAccountPasswordSecret = serviceAccountPasswordSecret; this.revocationAction = revocationAction; - this.tokenCredential = tokenCredential; - this.secretProvider = secretProvider; } public override bool CanOriginate => true; @@ -131,7 +121,8 @@ public override async Task OriginateValueAsync(SecretState currentS PatTokenResult result = await client.CreatePatAsync(new PatTokenCreateRequest { AllOrgs = false, DisplayName = this.patDisplayName, Scope = this.scopes, ValidTo = expirationDate.UtcDateTime }); - if (result.PatTokenError != SessionTokenError.None) { + if (result.PatTokenError != SessionTokenError.None) + { throw new RotationException($"Unable to create PAT: {result.PatTokenError}"); } @@ -147,17 +138,6 @@ public override async Task OriginateValueAsync(SecretState currentS }; } - private static async Task GetDevopsBearerTokenAsync(TokenCredential credential) - { - string[] scopes = { $"{AzureDevOpsApplicationId}/.default" }; - - var tokenRequestContext = new TokenRequestContext(scopes, parentRequestId: null); - - AccessToken authenticationResult = await credential.GetTokenAsync(tokenRequestContext, CancellationToken.None); - - return authenticationResult; - } - public override Func? GetRevocationActionAsync(SecretState secretState, bool whatIf) { if (!secretState.Tags.TryGetValue("AdoPatAuthorizationId", out string? authorizationIdString) || @@ -204,32 +184,6 @@ private static async Task GetDevopsBearerTokenAsync(TokenCredential }; } - private static async Task GetVssCredentials(TokenCredential credential) - { - AccessToken token = await GetDevopsBearerTokenAsync(credential); - - return new VssAadCredential(new VssAadToken("Bearer", token.Token)); - } - - private async Task GetConnectionAsync() - { - this.logger.LogDebug("Getting service account password from secret '{SecretName}'", this.serviceAccountPasswordSecret); - - string serviceAccountPassword = await this.secretProvider.GetSecretValueAsync(this.tokenCredential, this.serviceAccountPasswordSecret); - - this.logger.LogDebug("Getting token and devops client for 'dev.azure.com/{Organization}'", this.organization); - - var serviceAccountCredential = new UsernamePasswordCredential(this.serviceAccountName, serviceAccountPassword, this.serviceAccountTenantId, AzureCliApplicationId); - - VssCredentials vssCredentials = await GetVssCredentials(serviceAccountCredential); - - var connection = new VssConnection(new Uri($"https://vssps.dev.azure.com/{this.organization}"), vssCredentials); - - await connection.ConnectAsync(); - - return connection; - } - private enum RevocationAction { [JsonPropertyName("none")] diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceConnectionParameterStore.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceConnectionParameterStore.cs index 995795a5708..722ba62f43d 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceConnectionParameterStore.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps/ServiceConnectionParameterStore.cs @@ -1,40 +1,52 @@ using System.Text.Json; using System.Text.Json.Serialization; using Azure.Core; +using Azure.Sdk.Tools.SecretRotation.Azure; using Azure.Sdk.Tools.SecretRotation.Configuration; using Azure.Sdk.Tools.SecretRotation.Core; using Microsoft.Extensions.Logging; -using Microsoft.VisualStudio.Services.Client; using Microsoft.VisualStudio.Services.ServiceEndpoints.WebApi; using Microsoft.VisualStudio.Services.WebApi; namespace Azure.Sdk.Tools.SecretRotation.Stores.AzureDevOps; -public class ServiceConnectionParameterStore : SecretStore +public class ServiceConnectionParameterStore : AzureDevOpsStore { public const string MappingKey = "ADO Service Connection Parameter"; - private readonly string accountName; private readonly string connectionId; - private readonly TokenCredential credential; - private readonly ILogger logger; private readonly string parameterName; private readonly string projectName; - public ServiceConnectionParameterStore(string accountName, string projectName, string connectionId, - string parameterName, TokenCredential credential, ILogger logger) + public ServiceConnectionParameterStore( + string organization, + string projectName, + string connectionId, + string parameterName, + string? serviceAccountTenantId, + string? serviceAccountName, + Uri? serviceAccountPasswordSecret, + TokenCredential tokenCredential, + ISecretProvider secretProvider, + ILogger logger) + : base( + logger, + organization, + serviceAccountTenantId, + serviceAccountName, + serviceAccountPasswordSecret, + tokenCredential, + secretProvider) { - this.accountName = accountName; this.projectName = projectName; this.connectionId = connectionId; this.parameterName = parameterName; - this.credential = credential; - this.logger = logger; } public override bool CanWrite => true; public static Func GetSecretStoreFactory(TokenCredential credential, + ISecretProvider secretProvider, ILogger logger) { return configuration => @@ -61,12 +73,31 @@ public static Func GetSecretStoreFactory(TokenC throw new Exception("Missing required parameter 'parameterName'"); } + var passwordSecretParsed = Uri.TryCreate(parameters.ServiceAccountPasswordSecret, UriKind.Absolute, out Uri? serviceAccountPasswordSecret); + + if (!string.IsNullOrEmpty(parameters.ServiceAccountName)) + { + if (string.IsNullOrEmpty(parameters.ServiceAccountPasswordSecret)) + { + throw new Exception("Missing required parameter 'serviceAccountPasswordSecret'"); + } + + if (!passwordSecretParsed) + { + throw new Exception("Unable to parse Uri from parameter 'serviceAccountPasswordSecret'"); + } + } + return new ServiceConnectionParameterStore( parameters.AccountName, parameters.ProjectName, parameters.ConnectionId, parameters.ParameterName, + parameters.ServiceAccountTenantId, + parameters.ServiceAccountName, + serviceAccountPasswordSecret, credential, + secretProvider, logger); }; } @@ -74,7 +105,7 @@ public static Func GetSecretStoreFactory(TokenC public override async Task WriteSecretAsync(SecretValue secretValue, SecretState currentState, DateTimeOffset? revokeAfterDate, bool whatIf) { - this.logger.LogDebug("Getting token and devops client for dev.azure.com/{AccountName}", this.accountName); + this.logger.LogDebug("Getting token and devops client for dev.azure.com/{Organization}", this.organization); VssConnection connection = await GetConnectionAsync(); var client = await connection.GetClientAsync(); @@ -106,26 +137,6 @@ public override async Task WriteSecretAsync(SecretValue secretValue, SecretState } } - private async Task GetConnectionAsync() - { - string[] scopes = { "499b84ac-1321-427f-aa17-267ca6975798/.default" }; - string? parentRequestId = null; - - var tokenRequestContext = new TokenRequestContext(scopes, parentRequestId); - - AccessToken authenticationResult = await this.credential.GetTokenAsync( - tokenRequestContext, - CancellationToken.None); - - var connection = new VssConnection( - new Uri($"https://dev.azure.com/{this.accountName}"), - new VssAadCredential(new VssAadToken("Bearer", authenticationResult.Token))); - - await connection.ConnectAsync(); - - return connection; - } - private class Parameters { [JsonPropertyName("accountName")] @@ -139,5 +150,15 @@ private class Parameters [JsonPropertyName("parameterName")] public string? ParameterName { get; set; } + + [JsonPropertyName("serviceAccountTenantId")] + public string? ServiceAccountTenantId { get; set; } + + [JsonPropertyName("serviceAccountName")] + public string? ServiceAccountName { get; set; } + + [JsonPropertyName("serviceAccountPasswordSecret")] + public string? ServiceAccountPasswordSecret { get; set; } + } } From 33f26e86dcc113da55b2f47d565f4357fa80b657 Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 15 Apr 2024 15:55:08 -0400 Subject: [PATCH 06/23] Downgrade Maven prerequisite to continue supporting older version of Maven (#8092) --- .../codesnippet-maven-plugin/CHANGELOG.md | 14 +++++++++++++- .../codesnippet-maven-plugin/README.md | 2 +- .../java-packages/codesnippet-maven-plugin/pom.xml | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/java-packages/codesnippet-maven-plugin/CHANGELOG.md b/packages/java-packages/codesnippet-maven-plugin/CHANGELOG.md index 58ee39af0dc..48078bb8830 100644 --- a/packages/java-packages/codesnippet-maven-plugin/CHANGELOG.md +++ b/packages/java-packages/codesnippet-maven-plugin/CHANGELOG.md @@ -1,5 +1,17 @@ # Release History +## 1.0.0-beta.10 (2024-04-15) + +### Bugs Fixed + +- Downgraded Maven prerequisite version to `3.6.3` to prevent compatibility issues with older versions of Maven. + +## 1.0.0-beta.9 (2024-04-15) + +### Other Changes + +- Updated Maven versions used in the plugin. + ## 1.0.0-beta.8 (2022-08-18) ### Features Added @@ -83,4 +95,4 @@ ### Features Added -- Initial release of `codesnippet-maven-plugin`. \ No newline at end of file +- Initial release of `codesnippet-maven-plugin`. diff --git a/packages/java-packages/codesnippet-maven-plugin/README.md b/packages/java-packages/codesnippet-maven-plugin/README.md index d0b1e99b714..2beb6a95e62 100644 --- a/packages/java-packages/codesnippet-maven-plugin/README.md +++ b/packages/java-packages/codesnippet-maven-plugin/README.md @@ -12,7 +12,7 @@ First, reference the plugin in your maven project's `pom.xml` file. com.azure.tools codesnippet-maven-plugin - 1.0.0-beta.9 + 1.0.0-beta.10 **/src/samples/java/**/*.java ${project.basedir}/src/samples/java diff --git a/packages/java-packages/codesnippet-maven-plugin/pom.xml b/packages/java-packages/codesnippet-maven-plugin/pom.xml index 6e49532e1c5..876c96fce0e 100644 --- a/packages/java-packages/codesnippet-maven-plugin/pom.xml +++ b/packages/java-packages/codesnippet-maven-plugin/pom.xml @@ -6,7 +6,7 @@ com.azure.tools codesnippet-maven-plugin - 1.0.0-beta.9 + 1.0.0-beta.10 maven-plugin Codesnippet Maven Plugin @@ -45,7 +45,7 @@ - ${maven.version} + 3.6.3 From 9ae20c3fa206a76ee8350d4508c35209701aee40 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Mon, 15 Apr 2024 13:39:49 -0700 Subject: [PATCH 07/23] Add warning state to rotation (#8030) * Add warning state to rotation * Use an implicit warning threshold of `rotationThreshold / 2` * Add unit test for implicit warning window --- .../Commands/StatusCommand.cs | 26 ++++---- .../PlanConfiguration.cs | 2 + .../RotationConfiguration.cs | 1 + .../RotationPlan.cs | 25 +++++-- .../RotationPlanStatus.cs | 4 +- .../RotationState.cs | 10 +++ .../CoreTests/RotationPlanTests.cs | 66 +++++++++++-------- .../Valid/random-string.json | 1 + .../secret-management/schema/1.0.0/plan.json | 6 +- 9 files changed, 89 insertions(+), 52 deletions(-) create mode 100644 tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs index 0f84b03744f..bafb33044b5 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs @@ -38,8 +38,7 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu builder.AppendLine($" Status:"); builder.AppendLine($" ExpirationDate: {status.ExpirationDate}"); - builder.AppendLine($" Expired: {status.Expired}"); - builder.AppendLine($" ThresholdExpired: {status.ThresholdExpired}"); + builder.AppendLine($" State: {status.State}"); builder.AppendLine($" RequiresRevocation: {status.RequiresRevocation}"); builder.AppendLine($" Exception: {status.Exception?.Message}"); @@ -50,23 +49,21 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu statuses.Add((plan, status)); } - var errored = statuses.Where(x => x.Status.Exception is not null).ToArray(); - var expired = statuses.Except(errored).Where(x => x.Status is { Expired: true }).ToArray(); - var expiring = statuses.Except(errored).Where(x => x.Status is { Expired: false, ThresholdExpired: true }).ToArray(); - var upToDate = statuses.Except(errored).Where(x => x.Status is { Expired: false, ThresholdExpired: false }).ToArray(); + var plansBuyState = statuses.GroupBy(x => x.Status.State) + .ToDictionary(x => x.Key, x => x.ToArray()); var statusBuilder = new StringBuilder(); - void AppendStatusSection(IList<(RotationPlan Plan, RotationPlanStatus Status)> sectionStatuses, string header) + void AppendStatusSection(RotationState state, string header) { - if (!sectionStatuses.Any()) + if (!plansBuyState.TryGetValue(RotationState.Expired, out var matchingPlans)) { return; } statusBuilder.AppendLine(); statusBuilder.AppendLine(header); - foreach ((RotationPlan plan, RotationPlanStatus status) in sectionStatuses) + foreach ((RotationPlan plan, RotationPlanStatus status) in matchingPlans) { foreach (string line in GetPlanStatusLine(plan, status).Split("\n")) { @@ -76,14 +73,15 @@ void AppendStatusSection(IList<(RotationPlan Plan, RotationPlanStatus Status)> s } } - AppendStatusSection(expired, "Expired:"); - AppendStatusSection(expiring, "Expiring:"); - AppendStatusSection(upToDate, "Up-to-date:"); - AppendStatusSection(errored, "Error reading plan status:"); + AppendStatusSection(RotationState.Expired, "Expired:"); + AppendStatusSection(RotationState.Warning, "Expiring:"); + AppendStatusSection(RotationState.Rotate, "Should Rotate:"); + AppendStatusSection(RotationState.UpToDate, "Up-to-date:"); + AppendStatusSection(RotationState.Error, "Error reading plan status:"); logger.LogInformation(statusBuilder.ToString()); - if (expired.Any()) + if (statuses.Any(x => x.Status.State is RotationState.Expired or RotationState.Warning)) { invocationContext.ExitCode = 1; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs index 4c355314629..e6027a75495 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/PlanConfiguration.cs @@ -24,6 +24,8 @@ public class PlanConfiguration public TimeSpan? RotationThreshold { get; set; } + public TimeSpan? WarningThreshold { get; set; } + public TimeSpan? RotationPeriod { get; set; } public TimeSpan? RevokeAfterPeriod { get; set; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs index f728d78c56f..802d4af47e4 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Configuration/RotationConfiguration.cs @@ -122,6 +122,7 @@ private RotationPlan ResolveRotationPlan(PlanConfiguration planConfiguration, IL primary!, secondaries, planConfiguration.RotationThreshold!.Value, + planConfiguration.WarningThreshold, planConfiguration.RotationPeriod!.Value, planConfiguration.RevokeAfterPeriod); diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs index 3e7e242ab22..2695525d447 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlan.cs @@ -16,6 +16,7 @@ public RotationPlan(ILogger logger, SecretStore primaryStore, IList secondaryStores, TimeSpan rotationThreshold, + TimeSpan? warningThreshold, TimeSpan rotationPeriod, TimeSpan? revokeAfterPeriod) { @@ -25,6 +26,7 @@ public RotationPlan(ILogger logger, OriginStore = originStore; PrimaryStore = primaryStore; RotationThreshold = rotationThreshold; + WarningThreshold = warningThreshold ?? rotationThreshold / 2; RotationPeriod = rotationPeriod; RevokeAfterPeriod = revokeAfterPeriod; SecondaryStores = new ReadOnlyCollection(secondaryStores); @@ -40,6 +42,8 @@ public RotationPlan(ILogger logger, public TimeSpan RotationThreshold { get; } + public TimeSpan WarningThreshold { get; } + public TimeSpan RotationPeriod { get; } public TimeSpan? RevokeAfterPeriod { get; } @@ -102,21 +106,29 @@ public async Task GetStatusAsync() SecretState[] allStates = secondaryStoreStates.Prepend(primaryStoreState).ToArray(); - DateTimeOffset thresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); + DateTimeOffset rotationThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(RotationThreshold); + + DateTimeOffset warningThresholdDate = this.timeProvider.GetCurrentDateTimeOffset().Add(WarningThreshold); - DateTimeOffset? minExpirationDate = allStates.Where(x => x.ExpirationDate.HasValue).Min(x => x.ExpirationDate); + DateTimeOffset? minExpirationDate = allStates + .Where(x => x.ExpirationDate.HasValue) + .Min(x => x.ExpirationDate); - bool anyExpired = minExpirationDate == null || minExpirationDate <= invocationTime; + var state = RotationState.UpToDate; - bool anyThresholdExpired = minExpirationDate <= thresholdDate; + if (minExpirationDate == null || minExpirationDate <= invocationTime) + state = RotationState.Expired; + else if (minExpirationDate <= warningThresholdDate) + state = RotationState.Warning; + else if (minExpirationDate <= rotationThresholdDate) + state = RotationState.Rotate; bool anyRequireRevocation = rotationArtifacts.Any(state => state.RevokeAfterDate <= invocationTime); var status = new RotationPlanStatus { ExpirationDate = minExpirationDate, - Expired = anyExpired, - ThresholdExpired = anyThresholdExpired, + State = state, RequiresRevocation = anyRequireRevocation, PrimaryStoreState = primaryStoreState, SecondaryStoreStates = secondaryStoreStates.ToArray() @@ -128,6 +140,7 @@ public async Task GetStatusAsync() { var status = new RotationPlanStatus { + State = RotationState.Error, Exception = ex }; diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs index cbf8a2eedd1..3f7f13c2e24 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationPlanStatus.cs @@ -4,9 +4,7 @@ namespace Azure.Sdk.Tools.SecretRotation.Core; public class RotationPlanStatus { - public bool Expired { get; set; } - - public bool ThresholdExpired { get; set; } + public RotationState State { get; set; } public bool RequiresRevocation { get; set; } diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs new file mode 100644 index 00000000000..333395f0c1a --- /dev/null +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationState.cs @@ -0,0 +1,10 @@ +namespace Azure.Sdk.Tools.SecretRotation.Core; + +public enum RotationState +{ + Error, + UpToDate, + Rotate, + Warning, + Expired, +} diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs index 95f432bce86..2f5eaa4339e 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/CoreTests/RotationPlanTests.cs @@ -19,34 +19,42 @@ public class RotationPlanTests /// Then the result's Expired property should return True /// [Theory] - [TestCase(24, 0, 30, true, true)] - [TestCase(24, -1, 30, true, true)] - [TestCase(24, -1, 23, true, true)] - [TestCase(24, 30, 0, true, true)] - [TestCase(24, 30, -1, true, true)] - [TestCase(24, 23, -1, true, true)] - [TestCase(24, 24, 30, false, true)] - [TestCase(24, 23, 30, false, true)] - [TestCase(24, 30, 24, false, true)] - [TestCase(24, 30, 23, false, true)] - [TestCase(24, 25, 25, false, false)] + [TestCase(24, 22, 0, 30, RotationState.Expired)] // primary at expiration + [TestCase(24, 22, -1, 30, RotationState.Expired)] // primary past expiration + [TestCase(24, 22, -1, 23, RotationState.Expired)] // primary past expiration, secondary past rotation + [TestCase(24, 22, 30, 0, RotationState.Expired)] // secondary at expiration + [TestCase(24, 22, 30, -1, RotationState.Expired)] // secondary past expiration + [TestCase(24, 22, 23, -1, RotationState.Expired)] // secondary past expiration, primary past rotation + [TestCase(24, 22, 24, 30, RotationState.Rotate)] // primary at rotation + [TestCase(24, 22, 23, 30, RotationState.Rotate)] // primary between rotate and warning + [TestCase(24, 22, 30, 24, RotationState.Rotate)] // secondary at rotate + [TestCase(24, 22, 30, 23, RotationState.Rotate)] // secondary between rotate and warning + [TestCase(24, 22, 22, 30, RotationState.Warning)] // primary at warning + [TestCase(24, 22, 10, 30, RotationState.Warning)] // primary past warning + [TestCase(24, 22, 30, 22, RotationState.Warning)] // secondary at warning + [TestCase(24, 22, 30, 10, RotationState.Warning)] // secondary past warning + [TestCase(24, null, 30, 10, RotationState.Warning)] // implicit 12h warning window + [TestCase(24, 22, 25, 25, RotationState.UpToDate)] public async Task GetStatusAsync_ExpectExpirationState( - int thresholdHours, + int rotateThresholdHours, + int? warningThresholdHours, int hoursUntilPrimaryExpires, int hoursUntilSecondaryExpires, - bool expectExpired, - bool expectThresholdExpired) + RotationState expectedState) { DateTimeOffset staticTestTime = DateTimeOffset.Parse("2020-06-01T12:00:00Z"); - TimeSpan threshold = TimeSpan.FromHours(thresholdHours); + TimeSpan rotateThreshold = TimeSpan.FromHours(rotateThresholdHours); + TimeSpan? warningThreshold = warningThresholdHours.HasValue ? TimeSpan.FromHours(warningThresholdHours.Value) : null; - var primaryState = - new SecretState { ExpirationDate = staticTestTime.AddHours(hoursUntilPrimaryExpires) }; // after threshold - var secondaryState = - new SecretState - { - ExpirationDate = staticTestTime.AddHours(hoursUntilSecondaryExpires) - }; // before threshold + var primaryState = new SecretState + { + ExpirationDate = staticTestTime.AddHours(hoursUntilPrimaryExpires) + }; + + var secondaryState = new SecretState + { + ExpirationDate = staticTestTime.AddHours(hoursUntilSecondaryExpires) + }; var rotationPlan = new RotationPlan( Mock.Of(), @@ -58,15 +66,15 @@ public async Task GetStatusAsync_ExpectExpirationState( { Mock.Of(x => x.CanRead && x.GetCurrentStateAsync() == Task.FromResult(secondaryState)) }, - threshold, + rotateThreshold, + warningThreshold, default, default); // Act RotationPlanStatus status = await rotationPlan.GetStatusAsync(); - Assert.AreEqual(expectExpired, status.Expired); - Assert.AreEqual(expectThresholdExpired, status.ThresholdExpired); + Assert.AreEqual(expectedState, status.State); } ///

@@ -102,6 +110,7 @@ public async Task GetStatusAsync_RequiresRevocation(int hoursUntilRevocation, bo Array.Empty(), default, default, + default, default); // Act @@ -157,9 +166,10 @@ public async Task RotatePlansAsync_RequiresRevocation_DoesRevocation() originStore, primaryStore, new[] { secondaryStore }, - TimeSpan.FromDays(1), - TimeSpan.FromDays(2), - default); + rotationThreshold: TimeSpan.FromDays(3), + warningThreshold: TimeSpan.FromDays(2), + rotationPeriod: TimeSpan.FromDays(2), + revokeAfterPeriod: default); // Act await rotationPlan.ExecuteAsync(true, false); diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json index f9dcacb4d04..2e8b7535158 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Tests/TestConfigurations/Valid/random-string.json @@ -1,6 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/azure/azure-sdk-tools/main/tools/secret-management/schema/1.0.0/plan.json", "rotationThreshold": "9.00:00:00", + "warningThreshold": "6.00:00:00", "rotationPeriod": "12.00:00:00", "tags": [ "random" ], "stores": [ diff --git a/tools/secret-management/schema/1.0.0/plan.json b/tools/secret-management/schema/1.0.0/plan.json index 6538c106126..58a459d93df 100644 --- a/tools/secret-management/schema/1.0.0/plan.json +++ b/tools/secret-management/schema/1.0.0/plan.json @@ -14,7 +14,11 @@ "type": "string" }, "rotationThreshold": { - "description": "Time span indicating when the secret should be rotated before it expires", + "description": "Time span before expiration indicating when the secret should be rotated", + "type": "string" + }, + "warningThreshold": { + "description": "Time span before expiration indicating when warnings should be issued", "type": "string" }, "revokeAfterPeriod": { From 507b9981d0651fec6d1f693a041992ca5ff8cd62 Mon Sep 17 00:00:00 2001 From: Wes Haggard Date: Mon, 15 Apr 2024 15:04:48 -0700 Subject: [PATCH 08/23] Switch away from SAS for upload link cache file (#8080) * Switch away from SAS for upload link cache file * Update githubio-linkcheck.yml * Update githubio-linkcheck.yml --- eng/pipelines/githubio-linkcheck.yml | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/eng/pipelines/githubio-linkcheck.yml b/eng/pipelines/githubio-linkcheck.yml index a19d4aaca24..08303ed0a9b 100644 --- a/eng/pipelines/githubio-linkcheck.yml +++ b/eng/pipelines/githubio-linkcheck.yml @@ -91,7 +91,7 @@ jobs: -inputCacheFile "$(cachefile)" -outputCacheFile "$(cachefile)" -devOpsLogging: $true - + - task: PowerShell@2 displayName: 'tools link check' condition: succeededOrFailed() @@ -111,7 +111,14 @@ jobs: condition: succeededOrFailed() displayName: Upload verified links - - pwsh: | - azcopy copy '$(cachefile)' 'https://azuresdkartifacts.blob.core.windows.net/verify-links-cache$(azuresdkartifacts-verify-links-cache-sas)' - condition: succeededOrFailed() - displayName: Upload cache file to blob container + - task: AzurePowerShell@5 + displayName: 'Upload cache file to blob container' + inputs: + azureSubscription: 'Azure SDK Artifacts' + ScriptType: 'InlineScript' + azurePowerShellVersion: LatestVersion + pwsh: true + Inline: | + azcopy copy '$(cachefile)' 'https://azuresdkartifacts.blob.core.windows.net/verify-links-cache' + env: + AZCOPY_AUTO_LOGIN_TYPE: 'PSCRED' From 5e4401db5cd4747ec6c6dc1084291f961fbaebca Mon Sep 17 00:00:00 2001 From: Wes Haggard Date: Mon, 15 Apr 2024 16:10:27 -0700 Subject: [PATCH 09/23] Use WIF to connect storage container (#8076) Moving away from SAS tokens for connecting to storage so switching to using a Workload Identity Federation connection to the container to download the needed files. --- .../pipelines/templates/steps/policheck.yml | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/eng/common/pipelines/templates/steps/policheck.yml b/eng/common/pipelines/templates/steps/policheck.yml index 5ef30187e56..199af797295 100644 --- a/eng/common/pipelines/templates/steps/policheck.yml +++ b/eng/common/pipelines/templates/steps/policheck.yml @@ -2,14 +2,20 @@ parameters: ExclusionDataBaseFileName: '' TargetDirectory: '' PublishAnalysisLogs: false - PoliCheckBlobSAS: "$(azuresdk-policheck-blob-SAS)" ExclusionFilePath: "$(Build.SourcesDirectory)/eng/guardian-tools/policheck/PolicheckExclusions.xml" steps: - - pwsh: | - azcopy copy "https://azuresdkartifacts.blob.core.windows.net/policheck/${{ parameters.ExclusionDataBaseFileName }}.mdb?${{ parameters.PoliCheckBlobSAS }}" ` - "$(Build.BinariesDirectory)" - displayName: 'Download PoliCheck Exclusion Database' + - task: AzurePowerShell@5 + displayName: 'Download Policheck Exclusion Database' + inputs: + azureSubscription: 'Azure SDK Artifacts' + ScriptType: 'InlineScript' + azurePowerShellVersion: LatestVersion + pwsh: true + Inline: | + azcopy copy "https://azuresdkartifacts.blob.core.windows.net/policheck/${{ parameters.ExclusionDataBaseFileName }}.mdb" "$(Build.BinariesDirectory)" + env: + AZCOPY_AUTO_LOGIN_TYPE: 'PSCRED' - task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2 displayName: 'Run PoliCheck' @@ -33,4 +39,4 @@ steps: - ${{ if eq(parameters.PublishAnalysisLogs, 'true') }}: - task: securedevelopmentteam.vss-secure-development-tools.build-task-publishsecurityanalysislogs.PublishSecurityAnalysisLogs@3 - displayName: 'Publish Security Analysis Logs' \ No newline at end of file + displayName: 'Publish Security Analysis Logs' From 2bd8966e45186a3380d46da9aa9f645f2aa4dcab Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Mon, 15 Apr 2024 16:37:47 -0700 Subject: [PATCH 10/23] [Pylint] Don't allow typing under TYPE_CHECKING (#8061) * typing check * # * readme * typing_extensions * fix offset * bump version * 4.0 --- .../CHANGELOG.md | 4 ++ .../azure-pylint-guidelines-checker/README.md | 1 + .../pylint_guidelines_checker.py | 38 ++++++++++- .../azure-pylint-guidelines-checker/setup.py | 2 +- .../tests/test_pylint_custom_plugins.py | 65 +++++++++++++++++++ 5 files changed, 108 insertions(+), 2 deletions(-) diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md b/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md index 4bbcc326151..523606fb676 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md @@ -1,5 +1,9 @@ # Release History +## 0.4.0 (2024-04-15) + +- Checker to enforce no importing typing under TYPE_CHECKING block. + ## 0.3.1 (2023-1-16) - Docstring bug fix where paramtype was being considered for params diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md b/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md index 39ae46f3fae..e2d5042c9ab 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/README.md @@ -91,3 +91,4 @@ do-not-import-legacy-six | pylint:disable=do-not-import-legacy-six | Do no-legacy-azure-core-http-response-import | pylint:disable=no-legacy-azure-core-http-response-import | Do not import HttpResponse from azure.core.pipeline.transport outside of Azure Core. You can import HttpResponse from azure.core.rest instead. | [link](https://github.com/Azure/azure-sdk-for-python/issues/30785) | docstring-keyword-should-match-keyword-only | pylint:disable=docstring-keyword-should-match-keyword-only | Docstring keyword arguments and keyword-only method arguments should match. | [link](https://azure.github.io/azure-sdk/python_documentation.html#docstrings) | docstring-type-do-not-use-class | pylint:disable=docstring-type-do-not-use-class | Docstring type is formatted incorrectly. Do not use `:class` in docstring type. | [link](https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html) | +no-typing-import-in-type-check | pylint:disable=no-typing-import-in-type-check | Do not import typing under TYPE_CHECKING. | No Link. | \ No newline at end of file diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py index 2d4c8897706..3bba4819fb0 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py @@ -4,7 +4,7 @@ # ------------------------------------ """ -Pylint custom checkers for SDK guidelines: C4717 - C4758 +Pylint custom checkers for SDK guidelines: C4717 - C4760 """ import logging @@ -2708,6 +2708,41 @@ def visit_importfrom(self, node): ) + +class NoImportTypingFromTypeCheck(BaseChecker): + + """Rule to check that we aren't importing typing under TYPE_CHECKING.""" + + name = "no-typing-import-in-type-check" + priority = -1 + msgs = { + "C4760": ( + "Do not import from typing inside of TYPE_CHECKING.", + "no-typing-import-in-type-check", + "Do not import from typing inside of TYPE_CHECKING. You can import from typing outside of TYPE_CHECKING.", + ), + } + + def visit_importfrom(self, node): + """Check that we aren't importing from typing under if TYPE_CHECKING.""" + if isinstance(node.parent, astroid.If) and (node.modname == "typing" or node.modname == "typing_extensions"): + self.add_message( + msgid=f"no-typing-import-in-type-check", + node=node, + confidence=None, + ) + + def visit_import(self, node): + """Check that we aren't importing from typing under if TYPE_CHECKING.""" + if isinstance(node.parent, astroid.If): + for name, _ in node.names: + if name == "typing" or name == "typing_extensions": + self.add_message( + msgid=f"no-typing-import-in-type-check", + node=node, + confidence=None, + ) + # if a linter is registered in this function then it will be checked with pylint def register(linter): linter.register_checker(ClientsDoNotUseStaticMethods(linter)) @@ -2739,6 +2774,7 @@ def register(linter): linter.register_checker(ClientMethodsHaveTracingDecorators(linter)) linter.register_checker(DoNotImportLegacySix(linter)) linter.register_checker(NoLegacyAzureCoreHttpResponseImport(linter)) + linter.register_checker(NoImportTypingFromTypeCheck(linter)) # disabled by default, use pylint --enable=check-docstrings if you want to use it linter.register_checker(CheckDocstringParameters(linter)) diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py index ee0f0f4f2d6..5e8cb4aaea7 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py @@ -6,7 +6,7 @@ setup( name="azure-pylint-guidelines-checker", - version="0.3.1", + version="0.4.0", url="http://github.com/Azure/azure-sdk-for-python", license="MIT License", description="A pylint plugin which enforces azure sdk guidelines.", diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py index c8cd909795d..d2a2f21261d 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -4746,3 +4746,68 @@ def test_allowed_imports(self): importfrom_node.root().name = "azure.core" with self.assertNoMessages(): self.checker.visit_importfrom(importfrom_node) + + +class TestCheckNoTypingUnderTypeChecking(pylint.testutils.CheckerTestCase): + """Test that we are blocking disallowed imports and allowing allowed imports.""" + + CHECKER_CLASS = checker.NoImportTypingFromTypeCheck + + def test_disallowed_import_from(self): + """Check that illegal imports raise warnings""" + import_node = astroid.extract_node( + """ + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from typing import Any #@ + """ + ) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="no-typing-import-in-type-check", + line=5, + node=import_node, + col_offset=4, + end_line=5, + end_col_offset=26, + ) + ): + self.checker.visit_importfrom(import_node) + + def test_disallowed_import_from_extensions(self): + """Check that illegal imports raise warnings""" + import_node = astroid.extract_node( + """ + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + import typing_extensions #@ + """ + ) + with self.assertAddsMessages( + pylint.testutils.MessageTest( + msg_id="no-typing-import-in-type-check", + line=5, + node=import_node, + col_offset=4, + end_line=5, + end_col_offset=28, + ) + ): + self.checker.visit_import(import_node) + + def test_allowed_imports(self): + """Check that allowed imports don't raise warnings.""" + # import not in the blocked list. + importfrom_node = astroid.extract_node( + """ + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + + from math import PI + """ + ) + with self.assertNoMessages(): + self.checker.visit_importfrom(importfrom_node) From daa30b9f597d03b07dacaf80832edc258c27f12d Mon Sep 17 00:00:00 2001 From: Praven Kuttappan <55455725+praveenkuttappan@users.noreply.github.com> Date: Mon, 15 Apr 2024 20:07:46 -0400 Subject: [PATCH 11/23] Authenticate requests from APIView to DevOps using managed Identity (#8086) * Authenticate requests from APIView to DevOps using managed Identity --- .../APIView/APIViewWeb/APIViewWeb.csproj | 8 +- .../Controllers/PullRequestController.cs | 7 +- .../Repositories/DevopsArtifactRepository.cs | 175 ++++++++---------- 3 files changed, 87 insertions(+), 103 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/APIViewWeb.csproj b/src/dotnet/APIView/APIViewWeb/APIViewWeb.csproj index 81f69826d59..9957e6ba219 100644 --- a/src/dotnet/APIView/APIViewWeb/APIViewWeb.csproj +++ b/src/dotnet/APIView/APIViewWeb/APIViewWeb.csproj @@ -26,7 +26,7 @@ - + @@ -45,12 +45,14 @@ - + + all runtime; build; native; contentfiles; analyzers; buildtransitive - + + diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/PullRequestController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/PullRequestController.cs index a52883fb139..0b5d9dea013 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/PullRequestController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/PullRequestController.cs @@ -63,7 +63,8 @@ public async Task DetectApiChanges( string codeFile = null, string baselineCodeFile = null, bool commentOnPR = true, - string language = null) + string language = null, + string project = "internal") { if (!ValidateInputParams()) { @@ -80,7 +81,7 @@ public async Task DetectApiChanges( repoName: repoName, packageName: packageName, prNumber: pullRequestNumber, hostName: this.Request.Host.ToUriComponent(), codeFileName: codeFile, baselineCodeFileName: baselineCodeFile, - commentOnPR: commentOnPR, language: language); + commentOnPR: commentOnPR, language: language, project: project); return !string.IsNullOrEmpty(reviewUrl) ? StatusCode(statusCode: StatusCodes.Status201Created, reviewUrl) : StatusCode(statusCode: StatusCodes.Status208AlreadyReported); } @@ -102,7 +103,7 @@ private async Task DetectAPIChanges(string buildId, string baselineCodeFileName = null, bool commentOnPR = true, string language = null, - string project = "public") + string project = "internal") { language = LanguageServiceHelpers.MapLanguageAlias(language: language); var requestTelemetry = new RequestTelemetry { Name = "Detecting API changes for PR: " + prNumber }; diff --git a/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs b/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs index fbaf660f9b2..012c1a90091 100644 --- a/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs +++ b/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs @@ -1,130 +1,111 @@ -using Microsoft.ApplicationInsights.Extensibility; -using Microsoft.ApplicationInsights; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Caching.Memory; +using Azure.Core; +using Azure.Identity; +using Microsoft.ApplicationInsights; using Microsoft.Extensions.Configuration; -using Microsoft.TeamFoundation.Build.WebApi; -using Microsoft.TeamFoundation.Core.WebApi; -using Microsoft.VisualStudio.Services.Common; -using Microsoft.VisualStudio.Services.WebApi; -using Newtonsoft.Json; -using Octokit; +using Microsoft.TeamFoundation.Build.WebApi; +using Microsoft.TeamFoundation.Core.WebApi; +using Microsoft.VisualStudio.Services.Client; +using Microsoft.VisualStudio.Services.WebApi; +using Newtonsoft.Json; +using Polly; using System; using System.Collections.Generic; using System.IO; using System.Linq; -using System.Net; using System.Net.Http; using System.Net.Http.Headers; -using System.Text.Json; +using System.Threading; using System.Threading.Tasks; namespace APIViewWeb.Repositories { public class DevopsArtifactRepository : IDevopsArtifactRepository { - private readonly HttpClient _devopsClient; private readonly IConfiguration _configuration; - private readonly string _devopsAccessToken; private readonly string _hostUrl; private readonly TelemetryClient _telemetryClient; public DevopsArtifactRepository(IConfiguration configuration, TelemetryClient telemetryClient) { _configuration = configuration; - _devopsAccessToken = Convert.ToBase64String(System.Text.Encoding.ASCII.GetBytes(string.Format("{0}:{1}", "", _configuration["Azure-Devops-PAT"]))); _hostUrl = _configuration["APIVIew-Host-Url"]; _telemetryClient = telemetryClient; - - _devopsClient = new HttpClient(); - _devopsClient.DefaultRequestHeaders.Accept.Clear(); - _devopsClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); - _devopsClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", _devopsAccessToken); } public async Task DownloadPackageArtifact(string repoName, string buildId, string artifactName, string filePath, string project, string format= "file") { - var downloadUrl = await GetDownloadArtifactUrl(repoName, buildId, artifactName, project); - if (!string.IsNullOrEmpty(downloadUrl)) + var downloadUrl = await getDownloadArtifactUrl(buildId, artifactName, project); + if (string.IsNullOrEmpty(downloadUrl)) + { + throw new Exception(string.Format("Failed to get download url for artifact {0} in build {1} in project {2}", artifactName, buildId, project)); + } + + if(!string.IsNullOrEmpty(filePath)) { - if(!string.IsNullOrEmpty(filePath)) + if (!filePath.StartsWith("/")) { - if (!filePath.StartsWith("/")) - { - filePath = "/" + filePath; - } - downloadUrl = downloadUrl.Split("?")[0] + "?format=" + format + "&subPath=" + filePath; + filePath = "/" + filePath; } - - var downloadResp = await GetFromDevopsAsync(downloadUrl); - downloadResp.EnsureSuccessStatusCode(); - return await downloadResp.Content.ReadAsStreamAsync(); + downloadUrl = downloadUrl.Split("?")[0] + "?format=" + format + "&subPath=" + filePath; } - return null; + + HttpResponseMessage downloadResp = await GetFromDevopsAsync(downloadUrl); + downloadResp.EnsureSuccessStatusCode(); + return await downloadResp.Content.ReadAsStreamAsync(); } - private async Task GetFromDevopsAsync(string request) + private async Task getDownloadArtifactUrl(string buildId, string artifactName, string project) { - var downloadResp = await _devopsClient.GetAsync(request); - - - if (!downloadResp.IsSuccessStatusCode) - { - var retryAfter = downloadResp.Headers.GetValues("Retry-After"); - var rateLimitResource = downloadResp.Headers.GetValues("X-RateLimit-Resource"); - var rateLimitDelay = downloadResp.Headers.GetValues("X-RateLimit-Delay"); - var rateLimitLimit = downloadResp.Headers .GetValues("X-RateLimit-Limit"); - var rateLimitRemaining = downloadResp.Headers.GetValues("X-RateLimit-Remaining"); - var rateLimitReset = downloadResp.Headers.GetValues("X-RateLimit-Reset"); - - var traceMessage = $"request: {request} failed with statusCode: {downloadResp.StatusCode}," + - $"retryAfter: {retryAfter.FirstOrDefault()}, rateLimitResource: {rateLimitResource.FirstOrDefault()}, rateLimitDelay: {rateLimitDelay.FirstOrDefault()}," + - $"rateLimitLimit: {rateLimitLimit.FirstOrDefault()}, rateLimitRemaining: {rateLimitRemaining.FirstOrDefault()}, rateLimitReset: {rateLimitReset.FirstOrDefault()}"; - - _telemetryClient.TrackTrace(traceMessage); - } + var connection = await CreateVssConnection(); + var buildClient = connection.GetClient(); + var artifact = await buildClient.GetArtifactAsync(project, int.Parse(buildId), artifactName); + return artifact?.Resource?.DownloadUrl; + } - int count = 0; - int[] waitTimes = new int[] { 0, 1, 2, 4, 8, 16, 32, 64, 128, 256 }; - while ((downloadResp.StatusCode == HttpStatusCode.TooManyRequests || downloadResp.StatusCode == HttpStatusCode.BadRequest) && count < waitTimes.Length) - { - _telemetryClient.TrackTrace($"Download request from devops artifact is either throttled or flaky, waiting {waitTimes[count]} seconds before retrying, Retry count: {count}"); - await Task.Delay(TimeSpan.FromSeconds(waitTimes[count])); - downloadResp = await _devopsClient.GetAsync(request); - count++; - } - return downloadResp; + private async Task CreateVssConnection() + { + var accessToken = await getAccessToken(); + var token = new VssAadToken("Bearer", accessToken); + return new VssConnection(new Uri("https://dev.azure.com/azure-sdk/"), new VssAadCredential(token)); } - private async Task GetDownloadArtifactUrl(string repoName, string buildId, string artifactName, string project) + private async Task getAccessToken() { - var artifactGetReq = GetArtifactRestAPIForRepo(repoName).Replace("{buildId}", buildId).Replace("{artifactName}", artifactName).Replace("{project}", project); - var response = await GetFromDevopsAsync(artifactGetReq); - response.EnsureSuccessStatusCode(); - var buildResource = JsonDocument.Parse(await response.Content.ReadAsStringAsync()); - if (buildResource == null) - { - return null; - } - return buildResource.RootElement.GetProperty("resource").GetProperty("downloadUrl").GetString(); + // APIView deployed instances uses managed identity to authenticate requests to Azure DevOps. + // For local testing, VS will use developer credentials to create token + var credential = new DefaultAzureCredential(); + var tokenRequestContext = new TokenRequestContext(VssAadSettings.DefaultScopes); + var token = await credential.GetTokenAsync(tokenRequestContext, CancellationToken.None); + return token.Token; } - private string GetArtifactRestAPIForRepo(string repoName) + private async Task GetFromDevopsAsync(string request) { - var downloadArtifactRestApi = _configuration["download-artifact-rest-api-for-" + repoName]; - if (downloadArtifactRestApi == null) + var httpClient = new HttpClient(); + var accessToken = await getAccessToken(); + httpClient.DefaultRequestHeaders.Accept.Clear(); + httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", accessToken); + var maxRetryAttempts = 10; + var pauseBetweenFailures = TimeSpan.FromSeconds(2); + + var retryPolicy = Policy + .Handle() + .WaitAndRetryAsync(maxRetryAttempts, i => pauseBetweenFailures); + + HttpResponseMessage downloadResp = null; + await retryPolicy.ExecuteAsync(async () => { - downloadArtifactRestApi = _configuration["download-artifact-rest-api"]; - } - return downloadArtifactRestApi; + downloadResp = await httpClient.GetAsync(request); + }); + return downloadResp; } public async Task RunPipeline(string pipelineName, string reviewDetails, string originalStorageUrl) { //Create dictionary of all required parametes to run tools - generate--apireview pipeline in azure devops var reviewDetailsDict = new Dictionary { { "Reviews", reviewDetails }, { "APIViewUrl", _hostUrl }, { "StorageContainerUrl", originalStorageUrl } }; - var devOpsCreds = new VssBasicCredential("nobody", _configuration["Azure-Devops-PAT"]); - var devOpsConnection = new VssConnection(new Uri($"https://dev.azure.com/azure-sdk/"), devOpsCreds); + var devOpsConnection = await CreateVssConnection(); string projectName = _configuration["Azure-Devops-internal-project"] ?? "internal"; BuildHttpClient buildClient = await devOpsConnection.GetClientAsync(); @@ -132,29 +113,29 @@ public async Task RunPipeline(string pipelineName, string reviewDetails, string string envName = _configuration["apiview-deployment-environment"]; string updatedPipelineName = string.IsNullOrEmpty(envName) ? pipelineName : $"{pipelineName}-{envName}"; int definitionId = await GetPipelineId(updatedPipelineName, buildClient, projectName); - if (definitionId == 0) - { - throw new Exception(string.Format("Azure Devops pipeline is not found with name {0}. Please recheck and ensure pipeline exists with this name", updatedPipelineName)); + if (definitionId == 0) + { + throw new Exception(string.Format("Azure Devops pipeline is not found with name {0}. Please recheck and ensure pipeline exists with this name", updatedPipelineName)); } - var definition = await buildClient.GetDefinitionAsync(projectName, definitionId); + var definition = await buildClient.GetDefinitionAsync(projectName, definitionId); var project = await projectClient.GetProject(projectName); - await buildClient.QueueBuildAsync(new Build() - { - Definition = definition, - Project = project, - Parameters = JsonConvert.SerializeObject(reviewDetailsDict) + await buildClient.QueueBuildAsync(new Build() + { + Definition = definition, + Project = project, + Parameters = JsonConvert.SerializeObject(reviewDetailsDict) }); } - private async Task GetPipelineId(string pipelineName, BuildHttpClient client, string projectName) - { - var pipelines = await client.GetFullDefinitionsAsync2(project: projectName); - if (pipelines != null) - { - return pipelines.Single(p => p.Name == pipelineName).Id; - } - return 0; + private async Task GetPipelineId(string pipelineName, BuildHttpClient client, string projectName) + { + var pipelines = await client.GetFullDefinitionsAsync2(project: projectName); + if (pipelines != null) + { + return pipelines.Single(p => p.Name == pipelineName).Id; + } + return 0; } } } From 963a492d624a12c9ea21446a9fd0dfe45eee0961 Mon Sep 17 00:00:00 2001 From: Konrad Jamrozik Date: Mon, 15 Apr 2024 22:06:13 -0700 Subject: [PATCH 12/23] Update metadata_customized_docs.json: add spec dir sturcture, api versions and branches, breaking changes deep dive; remove avocado as it is obsolete (#8029) * Update metadata_customized_docs.json: add dir sturcture, api versions and branches, breaking changes deep dive; remove avocado as it is obsolete * Update metadata_customized_docs.json * Update metadata_customized_docs.json --- .../Embeddings/settings/metadata_customized_docs.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/sdk-ai-bots/Embeddings/settings/metadata_customized_docs.json b/tools/sdk-ai-bots/Embeddings/settings/metadata_customized_docs.json index 4a3cd8cd39a..68c80fc52f1 100644 --- a/tools/sdk-ai-bots/Embeddings/settings/metadata_customized_docs.json +++ b/tools/sdk-ai-bots/Embeddings/settings/metadata_customized_docs.json @@ -47,10 +47,6 @@ "title": "AutoRest Extensions for OpenAPI 2.0", "url": "https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md" }, - "avocado-readme.md": { - "title": "Avocado Readme", - "url": "https://github.com/Azure/avocado/blob/main/README.md" - }, "control_plane_template.md": { "title": "Control plane pull request template", "url": "https://github.com/Azure/azure-rest-api-specs/blob/main/.github/PULL_REQUEST_TEMPLATE/control_plane_template.md" @@ -58,5 +54,9 @@ "data_plane_template.md": { "title": "Data plane pull request template", "url": "https://github.com/Azure/azure-rest-api-specs/blob/main/.github/PULL_REQUEST_TEMPLATE/data_plane_template.md" + }, + "directory-structure.md": { + "title": "azure-rest-api-specs repos directory structure", + "url": "https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/directory-structure.md" } -} \ No newline at end of file +} From de61b0ca1affcca9a8ca550a00276523785c7b54 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Tue, 16 Apr 2024 10:43:59 -0700 Subject: [PATCH 13/23] Use subset of Go module identity for package name (#7868) * Use subset of Go module identity for package name --- .../templates/steps/sdk-testgen-set-env.yml | 2 +- src/dotnet/APIView/apiview.yml | 2 +- src/go/cmd/api_view_test.go | 8 ++++ src/go/cmd/module.go | 41 +++++++++---------- src/go/go.mod | 8 ++-- src/go/go.sum | 18 ++++---- 6 files changed, 42 insertions(+), 37 deletions(-) diff --git a/eng/pipelines/templates/steps/sdk-testgen-set-env.yml b/eng/pipelines/templates/steps/sdk-testgen-set-env.yml index e0bee427b27..e43bb7621ca 100644 --- a/eng/pipelines/templates/steps/sdk-testgen-set-env.yml +++ b/eng/pipelines/templates/steps/sdk-testgen-set-env.yml @@ -6,7 +6,7 @@ steps: displayName: "Install Node.js" - task: GoTool@0 inputs: - version: '1.16.7' + version: '1.22.1' displayName: "Install Go" - script: | npm install -g "@microsoft/rush" diff --git a/src/dotnet/APIView/apiview.yml b/src/dotnet/APIView/apiview.yml index 520bb4cf694..57c3e608a90 100644 --- a/src/dotnet/APIView/apiview.yml +++ b/src/dotnet/APIView/apiview.yml @@ -38,7 +38,7 @@ variables: BuildConfiguration: 'Release' TypeScriptGeneratorDirectory: 'tools/apiview/parsers/js-api-parser' GoParserPackagePath: 'src/go' - GoVersion: '1.18' + GoVersion: '1.22.1' NugetSecurityAnalysisWarningLevel: 'none' AzuriteConnectionString: "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1" CosmosEmulatorConnectionString: "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==" diff --git a/src/go/cmd/api_view_test.go b/src/go/cmd/api_view_test.go index 27523e29981..622945a9c9f 100644 --- a/src/go/cmd/api_view_test.go +++ b/src/go/cmd/api_view_test.go @@ -290,3 +290,11 @@ func TestVars(t *testing.T) { require.EqualValues(t, 2, countSomeChoice) require.True(t, hasHTTPClient) } + +func Test_getPackageNameFromModPath(t *testing.T) { + require.EqualValues(t, "foo", getPackageNameFromModPath("foo")) + require.EqualValues(t, "foo", getPackageNameFromModPath("foo/v2")) + require.EqualValues(t, "sdk/foo", getPackageNameFromModPath("github.com/Azure/azure-sdk-for-go/sdk/foo")) + require.EqualValues(t, "sdk/foo/bar", getPackageNameFromModPath("github.com/Azure/azure-sdk-for-go/sdk/foo/bar")) + require.EqualValues(t, "sdk/foo/bar", getPackageNameFromModPath("github.com/Azure/azure-sdk-for-go/sdk/foo/bar/v5")) +} diff --git a/src/go/cmd/module.go b/src/go/cmd/module.go index 8c7b1e4d43e..7d25b01bfef 100644 --- a/src/go/cmd/module.go +++ b/src/go/cmd/module.go @@ -39,6 +39,23 @@ type Module struct { packages map[string]*Pkg } +var majorVerSuffix = regexp.MustCompile(`/v\d+$`) + +// fetches the value for Module.PackageName from the full module path +func getPackageNameFromModPath(modPath string) string { + // for official SDKs, use a subset of the full module path + // e.g. github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/appcomplianceautomation/armappcomplianceautomation + // becomes sdk/resourcemanager/appcomplianceautomation/armappcomplianceautomation + if suffix, ok := strings.CutPrefix(modPath, "github.com/Azure/azure-sdk-for-go/"); ok { + modPath = suffix + } + // now strip off any major version suffix + if loc := majorVerSuffix.FindStringIndex(modPath); loc != nil { + modPath = modPath[:loc[0]] + } + return modPath +} + // NewModule indexes an Azure SDK module's ASTs func NewModule(dir string) (*Module, error) { mf, err := parseModFile(dir) @@ -48,30 +65,12 @@ func NewModule(dir string) (*Module, error) { // sdkRoot is the path on disk to the sdk folder e.g. /home/user/me/azure-sdk-for-go/sdk. // Used to find definitions of types imported from other Azure SDK modules. sdkRoot := "" - packageName := "" - if before, after, found := strings.Cut(dir, fmt.Sprintf("%s%c", sdkDirName, filepath.Separator)); found { + if before, _, found := strings.Cut(dir, fmt.Sprintf("%s%c", sdkDirName, filepath.Separator)); found { sdkRoot = filepath.Join(before, sdkDirName) - if filepath.Base(after) == "internal" { - packageName = after - } else { - packageName = filepath.Base(after) - } - fmt.Printf("Package Name: %s\n", packageName) } - //Package name can still be empty when generating API review using uploaded zip folder of a specific package in which case parent directory will not be sdk - if packageName == "" { - modulePath := mf.Module.Mod.Path - packageName = path.Base(modulePath) - fmt.Printf("Module path: %s\n", modulePath) - // Set relative path as package name for internal package to avoid collision - if packageName == "internal" { - if _, after, found := strings.Cut(modulePath, fmt.Sprintf("/%s/", sdkDirName)); found { - packageName = after - } - } - fmt.Printf("Package Name: %s\n", packageName) - } + packageName := getPackageNameFromModPath(mf.Module.Mod.Path) + fmt.Printf("Package Name: %s\n", packageName) m := Module{Name: filepath.Base(dir), PackageName: packageName, packages: map[string]*Pkg{}} baseImportPath := path.Dir(mf.Module.Mod.Path) + "/" diff --git a/src/go/go.mod b/src/go/go.mod index a8d4d79c410..440558e388f 100644 --- a/src/go/go.mod +++ b/src/go/go.mod @@ -4,13 +4,13 @@ go 1.18 require ( github.com/spf13/cobra v1.8.0 - github.com/stretchr/testify v1.7.0 - golang.org/x/exp v0.0.0-20240119083558-1b970713d09a - golang.org/x/mod v0.14.0 + github.com/stretchr/testify v1.9.0 + golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 + golang.org/x/mod v0.16.0 ) require ( - github.com/davecgh/go-spew v1.1.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/src/go/go.sum b/src/go/go.sum index 097db132ca6..a3eb35d4abc 100644 --- a/src/go/go.sum +++ b/src/go/go.sum @@ -1,6 +1,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -10,15 +10,13 @@ github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA= -golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= -golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= -golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 h1:LfspQV/FYTatPTr/3HzIcmiUFH7PGP+OQ6mgDYo3yuQ= +golang.org/x/exp v0.0.0-20240222234643-814bf88cf225/go.mod h1:CxmFvTBINI24O/j8iY7H1xHzx2i4OsyguNBmN/uPtqc= +golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic= +golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 8ac3c6b9afed1d303f71fb0c8fc51150803c64ef Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Tue, 16 Apr 2024 11:10:07 -0700 Subject: [PATCH 14/23] Improve status and list commands (#8102) --- .../Commands/ListCommand.cs | 19 ++- .../Commands/StatusCommand.cs | 110 ++++++++---------- .../SimplerConsoleFormatter.cs | 4 +- .../RotationException.cs | 2 +- .../TaskExtensions.cs | 51 ++++++++ 5 files changed, 117 insertions(+), 69 deletions(-) create mode 100644 tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/TaskExtensions.cs diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/ListCommand.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/ListCommand.cs index 6d6a5c3b72d..11bd7f67261 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/ListCommand.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/ListCommand.cs @@ -1,7 +1,6 @@ using System.CommandLine.Invocation; -using System.Text.Json; +using System.Text; using Azure.Sdk.Tools.SecretRotation.Configuration; -using Azure.Sdk.Tools.SecretRotation.Core; namespace Azure.Sdk.Tools.SecretManagement.Cli.Commands; @@ -16,7 +15,21 @@ protected override Task HandleCommandAsync(ILogger logger, RotationConfiguration { foreach (PlanConfiguration plan in rotationConfiguration.PlanConfigurations) { - Console.WriteLine($"name: {plan.Name} - tags: {string.Join(", ", plan.Tags)}"); + logger.LogInformation(plan.Name); + + if (logger.IsEnabled(LogLevel.Debug)) + { + var builder = new StringBuilder(); + + builder.AppendLine($" Tags: {string.Join(", ", plan.Tags)}"); + builder.AppendLine($" Rotation Period: {plan.RotationPeriod}"); + builder.AppendLine($" Rotation Threshold: {plan.RotationThreshold}"); + builder.AppendLine($" Warning Threshold: {plan.WarningThreshold}"); + builder.AppendLine($" Revoke After Period: {plan.RevokeAfterPeriod}"); + builder.AppendLine($" Store Types: {string.Join(", ", plan.StoreConfigurations.Select(s => s.Type).Distinct() )}"); + + logger.LogDebug(builder.ToString()); + } } return Task.CompletedTask; diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs index bafb33044b5..38e27fec7a6 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/StatusCommand.cs @@ -1,8 +1,5 @@ using System.CommandLine.Invocation; -using System.Reflection; using System.Text; -using System.Text.Json; -using System.Text.Json.Serialization; using Azure.Sdk.Tools.SecretRotation.Configuration; using Azure.Sdk.Tools.SecretRotation.Core; @@ -18,68 +15,73 @@ protected override async Task HandleCommandAsync(ILogger logger, RotationConfigu InvocationContext invocationContext) { var timeProvider = new TimeProvider(); - IEnumerable plans = rotationConfiguration.GetAllRotationPlans(logger, timeProvider); + RotationPlan[] plans = rotationConfiguration.GetAllRotationPlans(logger, timeProvider).ToArray(); - List<(RotationPlan Plan, RotationPlanStatus Status)> statuses = new(); + logger.LogInformation($"Getting status for {plans.Length} plans"); - foreach (RotationPlan plan in plans) - { - logger.LogInformation($"Getting status for plan '{plan.Name}'"); - RotationPlanStatus status = await plan.GetStatusAsync(); - - if (logger.IsEnabled(LogLevel.Debug)) - { - var builder = new StringBuilder(); - - builder.AppendLine($" Plan:"); - builder.AppendLine($" RotationPeriod: {plan.RotationPeriod}"); - builder.AppendLine($" RotationThreshold: {plan.RotationThreshold}"); - builder.AppendLine($" RevokeAfterPeriod: {plan.RevokeAfterPeriod}"); - - builder.AppendLine($" Status:"); - builder.AppendLine($" ExpirationDate: {status.ExpirationDate}"); - builder.AppendLine($" State: {status.State}"); - builder.AppendLine($" RequiresRevocation: {status.RequiresRevocation}"); - builder.AppendLine($" Exception: {status.Exception?.Message}"); - - logger.LogDebug(builder.ToString()); - } + (RotationPlan Plan, RotationPlanStatus Status)[] statuses = await plans + .Select(async plan => { + logger.LogDebug($"Getting status for plan '{plan.Name}'."); + return (plan, await plan.GetStatusAsync()); + }) + .LimitConcurrencyAsync(10); - statuses.Add((plan, status)); - } - var plansBuyState = statuses.GroupBy(x => x.Status.State) .ToDictionary(x => x.Key, x => x.ToArray()); - var statusBuilder = new StringBuilder(); - void AppendStatusSection(RotationState state, string header) + void LogStatusSection(RotationState state, string header) { - if (!plansBuyState.TryGetValue(RotationState.Expired, out var matchingPlans)) + if (!plansBuyState.TryGetValue(state, out var matchingPlans)) { return; } - statusBuilder.AppendLine(); - statusBuilder.AppendLine(header); + logger.LogInformation($"\n{header}"); + foreach ((RotationPlan plan, RotationPlanStatus status) in matchingPlans) { - foreach (string line in GetPlanStatusLine(plan, status).Split("\n")) + var builder = new StringBuilder(); + var debugBuilder = new StringBuilder(); + + builder.Append($" {plan.Name} - "); + DateTimeOffset? expirationDate = status.ExpirationDate; + if (expirationDate.HasValue) + { + builder.AppendLine($"{expirationDate} ({FormatTimeSpan(expirationDate.Value.Subtract(DateTimeOffset.UtcNow))})"); + } + else { - statusBuilder.Append(" "); - statusBuilder.AppendLine(line); + builder.AppendLine("no expiration date"); } + + debugBuilder.AppendLine($" Plan:"); + debugBuilder.AppendLine($" Rotation Period: {plan.RotationPeriod}"); + debugBuilder.AppendLine($" Rotation Threshold: {plan.RotationThreshold}"); + debugBuilder.AppendLine($" Warning Threshold: {plan.WarningThreshold}"); + debugBuilder.AppendLine($" Revoke After Period: {plan.RevokeAfterPeriod}"); + debugBuilder.AppendLine($" Status:"); + debugBuilder.AppendLine($" Expiration Date: {status.ExpirationDate}"); + debugBuilder.AppendLine($" State: {status.State}"); + debugBuilder.AppendLine($" Requires Revocation: {status.RequiresRevocation}"); + + if (status.Exception != null) + { + builder.AppendLine($" Exception:"); + builder.AppendLine($" {status.Exception.Message}"); + } + + logger.LogInformation(builder.ToString()); + logger.LogDebug(debugBuilder.ToString()); } } - AppendStatusSection(RotationState.Expired, "Expired:"); - AppendStatusSection(RotationState.Warning, "Expiring:"); - AppendStatusSection(RotationState.Rotate, "Should Rotate:"); - AppendStatusSection(RotationState.UpToDate, "Up-to-date:"); - AppendStatusSection(RotationState.Error, "Error reading plan status:"); - - logger.LogInformation(statusBuilder.ToString()); + LogStatusSection(RotationState.Expired, "Expired:"); + LogStatusSection(RotationState.Warning, "Expiring:"); + LogStatusSection(RotationState.Rotate, "Should Rotate:"); + LogStatusSection(RotationState.UpToDate, "Up-to-date:"); + LogStatusSection(RotationState.Error, "Error reading plan status:"); if (statuses.Any(x => x.Status.State is RotationState.Expired or RotationState.Warning)) { @@ -87,24 +89,6 @@ void AppendStatusSection(RotationState state, string header) } } - private static string GetPlanStatusLine(RotationPlan plan, RotationPlanStatus status) - { - if (status.Exception != null) - { - return $"{plan.Name}:\n {status.Exception.Message}"; - } - - DateTimeOffset? expirationDate = status.ExpirationDate; - - DateTimeOffset now = DateTimeOffset.UtcNow; - - string expiration = expirationDate.HasValue - ? $"{FormatTimeSpan(expirationDate.Value.Subtract(now))}" - : "No expiration date"; - - return $"{plan.Name} - {expiration} / ({FormatTimeSpan(plan.RotationPeriod)} @ {FormatTimeSpan(plan.RotationThreshold)})"; - } - private static string FormatTimeSpan(TimeSpan timeSpan) { if (timeSpan == TimeSpan.Zero) diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/SimplerConsoleFormatter.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/SimplerConsoleFormatter.cs index e495c510bca..194d0bba9e5 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/SimplerConsoleFormatter.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/SimplerConsoleFormatter.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Logging.Console; using Microsoft.Extensions.Options; @@ -83,7 +83,7 @@ private DateTimeOffset GetCurrentDateTime() return logLevel switch { LogLevel.Trace => "trce: ", - LogLevel.Debug => "dbug: ", + LogLevel.Debug => null, LogLevel.Information => null, LogLevel.Warning => "warn: ", LogLevel.Error => "fail: ", diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationException.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationException.cs index 001ae8cffed..eb60e747bef 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationException.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/RotationException.cs @@ -1,4 +1,4 @@ -namespace Azure.Sdk.Tools.SecretRotation.Core; +namespace Azure.Sdk.Tools.SecretRotation.Core; public class RotationException : Exception { diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/TaskExtensions.cs b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/TaskExtensions.cs new file mode 100644 index 00000000000..b3793a7fec8 --- /dev/null +++ b/tools/secret-management/Azure.Sdk.Tools.SecretRotation.Core/TaskExtensions.cs @@ -0,0 +1,51 @@ +namespace Azure.Sdk.Tools.SecretRotation.Core; + +public static class TaskExtensions +{ + public static async Task LimitConcurrencyAsync(this IEnumerable> tasks, int concurrencyLimit = 1, CancellationToken cancellationToken = default) + { + if (concurrencyLimit == int.MaxValue) + { + return await Task.WhenAll(tasks); + } + + var results = new List(); + + if (concurrencyLimit == 1) + { + foreach (var task in tasks) + { + results.Add(await task); + } + + return results.ToArray(); + } + + var pending = new List>(); + + foreach (var task in tasks) + { + pending.Add(task); + + if (cancellationToken.IsCancellationRequested) + { + break; + } + + if (pending.Count < concurrencyLimit) continue; + + var completed = await Task.WhenAny(pending); + pending.Remove(completed); + results.Add(await completed); + } + + results.AddRange(await Task.WhenAll(pending)); + + return results.ToArray(); + } + + public static Task LimitConcurrencyAsync(this IEnumerable source, Func> taskFactory, int concurrencyLimit = 1, CancellationToken cancellationToken = default) + { + return LimitConcurrencyAsync(source.Select(taskFactory), concurrencyLimit, cancellationToken); + } +} From 791250a633f928df25ba873ca9cc5a1fea4ce8f1 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Tue, 16 Apr 2024 13:03:21 -0700 Subject: [PATCH 15/23] Dispose of the logger factory to flush logs on exit (#8103) --- .../Commands/RotationCommandBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs index 28e73cc965f..539cc834242 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs @@ -66,7 +66,7 @@ private async Task ParseAndHandleCommandAsync(InvocationContext invocationContex LogLevel logLevel = verbose ? LogLevel.Trace : LogLevel.Information; - ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder + using ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder .AddConsoleFormatter() .AddConsole(options => options.FormatterName = SimplerConsoleFormatter.FormatterName) .SetMinimumLevel(logLevel)); From 5814d1ff0ecfdf5934fdffba8086caa2a0897530 Mon Sep 17 00:00:00 2001 From: Patrick Hallisey Date: Tue, 16 Apr 2024 14:15:57 -0700 Subject: [PATCH 16/23] Allow for azcli or azpwsh creds (#8106) * Allow for azcli or azpwsh creds * Try powershell creds before cli --- .../Commands/RotationCommandBase.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs index 539cc834242..6e692092e4b 100644 --- a/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs +++ b/tools/secret-management/Azure.Sdk.Tools.SecretManagement.Cli/Commands/RotationCommandBase.cs @@ -1,5 +1,6 @@ using System.CommandLine; using System.CommandLine.Invocation; +using Azure.Core; using Azure.Identity; using Azure.Sdk.Tools.SecretRotation.Azure; using Azure.Sdk.Tools.SecretRotation.Configuration; @@ -76,7 +77,7 @@ private async Task ParseAndHandleCommandAsync(InvocationContext invocationContex logger.LogDebug("Parsing configuration"); // TODO: Pass a logger to the token so it can verbose log getting tokens. - var tokenCredential = new AzureCliCredential(); + TokenCredential tokenCredential = new ChainedTokenCredential(new AzurePowerShellCredential(), new AzureCliCredential()); IDictionary> secretStoreFactories = GetDefaultSecretStoreFactories(tokenCredential, logger); @@ -88,7 +89,7 @@ private async Task ParseAndHandleCommandAsync(InvocationContext invocationContex } private static IDictionary> GetDefaultSecretStoreFactories( - AzureCliCredential tokenCredential, ILogger logger) + TokenCredential tokenCredential, ILogger logger) { return new Dictionary> { From 803923116b60e34f17dfc1e75da1a89939e37b23 Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Tue, 16 Apr 2024 17:32:46 -0400 Subject: [PATCH 17/23] Fix missing spaces between keywords (#8107) --- .../processor/analysers/JavaASTAnalyser.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java index 21bb153c477..89dc7924787 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java @@ -530,7 +530,7 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { addToken(new Token(KEYWORD, "to"), SPACE); for (int i = 0; i < names.size(); i++) { - addToken(new Token(TYPE_NAME, names.get(i).asString())); + addToken(new Token(TYPE_NAME, names.get(i).toString())); if (i < names.size() - 1) { addToken(new Token(PUNCTUATION, ","), SPACE); @@ -585,7 +585,7 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { NodeList names = d.getWith(); for (int i = 0; i < names.size(); i++) { - addToken(new Token(TYPE_NAME, names.get(i).asString())); + addToken(new Token(TYPE_NAME, names.get(i).toString())); if (i < names.size() - 1) { addToken(new Token(PUNCTUATION, ","), SPACE); @@ -841,7 +841,7 @@ private void tokeniseFields(boolean isInterfaceDeclaration, TypeDeclaration t final NodeList fieldModifiers = fieldDeclaration.getModifiers(); // public, protected, static, final for (final Modifier fieldModifier : fieldModifiers) { - addToken(new Token(KEYWORD, fieldModifier.getKeyword().asString())); + addToken(new Token(KEYWORD, fieldModifier.toString())); } // field type and name @@ -1177,7 +1177,7 @@ private void processAnnotationValueExpression(final Expression valueExpr, final private void getModifiers(NodeList modifiers) { for (final Modifier modifier : modifiers) { - addToken(new Token(KEYWORD, modifier.getKeyword().asString())); + addToken(new Token(KEYWORD, modifier.toString())); } } @@ -1265,7 +1265,7 @@ private void getThrowException(CallableDeclaration callableDeclaration) { addToken(new Token(KEYWORD, "throws"), SPACE); for (int i = 0, max = thrownExceptions.size(); i < max; i++) { - final String exceptionName = thrownExceptions.get(i).getElementType().asString(); + final String exceptionName = thrownExceptions.get(i).getElementType().toString(); final Token throwsToken = new Token(TYPE_NAME, exceptionName); // we look up the package name in case it is a custom type in the same library, @@ -1304,7 +1304,7 @@ private void getType(Object type) { private void getClassType(Type type) { if (type.isPrimitiveType()) { - addToken(new Token(TYPE_NAME, type.asPrimitiveType().asString())); + addToken(new Token(TYPE_NAME, type.asPrimitiveType().toString())); } else if (type.isVoidType()) { addToken(new Token(TYPE_NAME, "void")); } else if (type.isReferenceType()) { @@ -1431,7 +1431,7 @@ public void visit(CompilationUnit compilationUnit, Map arg) { compilationUnit.getImports().stream() .map(ImportDeclaration::getName) .forEach(name -> name.getQualifier().ifPresent(packageName -> - apiListing.addPackageTypeMapping(packageName.asString(), name.getIdentifier()))); + apiListing.addPackageTypeMapping(packageName.toString(), name.getIdentifier()))); } } From 2fa41b4b965ac5e8d061bf0249cdc7805806183a Mon Sep 17 00:00:00 2001 From: Praven Kuttappan <55455725+praveenkuttappan@users.noreply.github.com> Date: Tue, 16 Apr 2024 23:25:07 -0400 Subject: [PATCH 18/23] Show review link in logs and set DevOps project name in request (#8110) * Show review link in logs and set DevOps project name based on the pipeline run --- eng/common/pipelines/templates/steps/detect-api-changes.yml | 1 + eng/common/scripts/Create-APIReview.ps1 | 2 ++ eng/common/scripts/Detect-Api-Changes.ps1 | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/eng/common/pipelines/templates/steps/detect-api-changes.yml b/eng/common/pipelines/templates/steps/detect-api-changes.yml index 5210ebbe4b6..f39a88eaa3a 100644 --- a/eng/common/pipelines/templates/steps/detect-api-changes.yml +++ b/eng/common/pipelines/templates/steps/detect-api-changes.yml @@ -22,6 +22,7 @@ steps: -RepoFullName $(Build.Repository.Name) -APIViewUri $(ApiChangeDetectRequestUrl) -ArtifactName ${{ parameters.ArtifactName }} + -DevopsProject $(System.TeamProject) pwsh: true displayName: Detect API changes condition: and(succeededOrFailed(), eq(variables['Build.Reason'],'PullRequest')) diff --git a/eng/common/scripts/Create-APIReview.ps1 b/eng/common/scripts/Create-APIReview.ps1 index a3f22a2678d..c5c5e51f31b 100644 --- a/eng/common/scripts/Create-APIReview.ps1 +++ b/eng/common/scripts/Create-APIReview.ps1 @@ -78,6 +78,7 @@ function Upload-SourceArtifact($filePath, $apiLabel, $releaseStatus, $packageVer try { $Response = Invoke-WebRequest -Method 'POST' -Uri $uri -Body $multipartContent -Headers $headers + Write-Host "API review: $($Response.Content)" $StatusCode = $Response.StatusCode } catch @@ -114,6 +115,7 @@ function Upload-ReviewTokenFile($packageName, $apiLabel, $releaseStatus, $review try { $Response = Invoke-WebRequest -Method 'GET' -Uri $uri -Headers $headers + Write-Host "API review: $($Response.Content)" $StatusCode = $Response.StatusCode } catch diff --git a/eng/common/scripts/Detect-Api-Changes.ps1 b/eng/common/scripts/Detect-Api-Changes.ps1 index 572ef43e1cf..52234650233 100644 --- a/eng/common/scripts/Detect-Api-Changes.ps1 +++ b/eng/common/scripts/Detect-Api-Changes.ps1 @@ -15,7 +15,8 @@ Param ( [string] $APIViewUri, [string] $RepoFullName = "", [string] $ArtifactName = "packages", - [string] $TargetBranch = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/") + [string] $TargetBranch = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/"), + [string] $DevopsProject = "internal" ) . (Join-Path $PSScriptRoot common.ps1) @@ -37,6 +38,7 @@ function Submit-Request($filePath, $packageName) $query.Add('pullRequestNumber', $PullRequestNumber) $query.Add('packageName', $packageName) $query.Add('language', $LanguageShort) + $query.Add('project', $DevopsProject) $reviewFileFullName = Join-Path -Path $ArtifactPath $packageName $reviewFileName if (Test-Path $reviewFileFullName) { @@ -87,6 +89,7 @@ function Log-Input-Params() Write-Host "Language: $($Language)" Write-Host "Commit SHA: $($CommitSha)" Write-Host "Repo Name: $($RepoFullName)" + Write-Host "Project: $($DevopsProject)" } Log-Input-Params From 6529d09f8e6706a186b6a1f94b6165b0a77ef233 Mon Sep 17 00:00:00 2001 From: Praven Kuttappan <55455725+praveenkuttappan@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:53:01 -0400 Subject: [PATCH 19/23] Add polly retry to get artifact URL from DevOps (#8112) * Add polly retry to get artifact URL from DevOps --- .../Repositories/DevopsArtifactRepository.cs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs b/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs index 012c1a90091..bc0ce11921b 100644 --- a/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs +++ b/src/dotnet/APIView/APIViewWeb/Repositories/DevopsArtifactRepository.cs @@ -56,10 +56,25 @@ public async Task DownloadPackageArtifact(string repoName, string buildI private async Task getDownloadArtifactUrl(string buildId, string artifactName, string project) { + var pauseBetweenFailures = TimeSpan.FromSeconds(2); + var retryPolicy = Policy + .Handle() + .WaitAndRetryAsync(5, i => pauseBetweenFailures); + var connection = await CreateVssConnection(); var buildClient = connection.GetClient(); - var artifact = await buildClient.GetArtifactAsync(project, int.Parse(buildId), artifactName); - return artifact?.Resource?.DownloadUrl; + string url = null; + await retryPolicy.ExecuteAsync(async () => + { + var artifact = await buildClient.GetArtifactAsync(project, int.Parse(buildId), artifactName); + url = artifact?.Resource?.DownloadUrl; + }); + + if (string.IsNullOrEmpty(url)) + { + throw new Exception(string.Format("Failed to get download url for artifact {0} in build {1} in project {2}", artifactName, buildId, project)); + } + return url; } private async Task CreateVssConnection() From e7fb87c04d14aaa3b59c8a6f25b731138903f6a4 Mon Sep 17 00:00:00 2001 From: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com> Date: Wed, 17 Apr 2024 15:11:10 -0400 Subject: [PATCH 20/23] Add indentation handling method in Java ApiView tool (#8113) Add indentation handling method in Java ApiView tool --- .../processor/analysers/JavaASTAnalyser.java | 521 +++++++++--------- 1 file changed, 255 insertions(+), 266 deletions(-) diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java index 89dc7924787..130d60d88b4 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/analysers/JavaASTAnalyser.java @@ -107,6 +107,8 @@ public class JavaASTAnalyser implements Analyser { private static final boolean SHOW_JAVADOC = true; private static final Map ANNOTATION_RULE_MAP; + private static final JavaParserAdapter JAVA_8_PARSER; + private static final JavaParserAdapter JAVA_11_PARSER; static { /* For some annotations, we want to customise how they are displayed. Sometimes, we don't show them in any @@ -120,12 +122,7 @@ public class JavaASTAnalyser implements Analyser { // we always want @Metadata annotations to be fully expanded, but in a condensed form ANNOTATION_RULE_MAP.put("Metadata", new AnnotationRule().setShowProperties(true).setCondensed(true)); - } - private static final JavaParserAdapter JAVA_8_PARSER; - private static final JavaParserAdapter JAVA_11_PARSER; - - static { // Configure JavaParser to use type resolution JAVA_8_PARSER = JavaParserAdapter.of(new JavaParser(new ParserConfiguration() .setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_8) @@ -144,7 +141,7 @@ public class JavaASTAnalyser implements Analyser { private final Diagnostics diagnostic; - private int indent = 0; + private int indentLevel = 0; public JavaASTAnalyser(APIListing apiListing) { this.apiListing = apiListing; @@ -271,13 +268,9 @@ private void processPackage(String packageName, List scanClasses) { addToken(packageToken, SPACE); addToken(new Token(PUNCTUATION, "{"), NEWLINE); - indent(); - - scanClasses.stream() + indentedBlock(() -> scanClasses.stream() .sorted(Comparator.comparing(s -> s.primaryTypeName)) - .forEach(this::processSingleFile); - - unindent(); + .forEach(this::processSingleFile)); addToken(new Token(PUNCTUATION, "}"), NEWLINE); } @@ -302,96 +295,90 @@ private void tokeniseMavenPom(Pom mavenPom) { addToken(makeWhitespace()); addToken(new Token(KEYWORD, "maven", MAVEN_KEY), SPACE); addToken(new Token(PUNCTUATION, "{"), NEWLINE); - indent(); - addToken(new Token(SKIP_DIFF_START)); - // parent - String gavStr = mavenPom.getParent().getGroupId() + ":" + mavenPom.getParent().getArtifactId() + ":" + mavenPom.getParent().getVersion(); - tokeniseKeyValue("parent", gavStr, ""); - - // properties - gavStr = mavenPom.getGroupId() + ":" + mavenPom.getArtifactId() + ":" + mavenPom.getVersion(); - tokeniseKeyValue("properties", gavStr, ""); + indentedBlock(() -> { + addToken(new Token(SKIP_DIFF_START)); + // parent + String gavStr = mavenPom.getParent().getGroupId() + ":" + mavenPom.getParent().getArtifactId() + ":" + + mavenPom.getParent().getVersion(); + tokeniseKeyValue("parent", gavStr, ""); + + // properties + gavStr = mavenPom.getGroupId() + ":" + mavenPom.getArtifactId() + ":" + mavenPom.getVersion(); + tokeniseKeyValue("properties", gavStr, ""); + + // configuration + boolean showJacoco = mavenPom.getJacocoMinLineCoverage() != null + && mavenPom.getJacocoMinBranchCoverage() != null; + boolean showCheckStyle = mavenPom.getCheckstyleExcludes() != null && !mavenPom.getCheckstyleExcludes() + .isEmpty(); + + if (showJacoco || showCheckStyle) { + addToken(INDENT, new Token(KEYWORD, "configuration"), SPACE); + addToken(new Token(PUNCTUATION, "{"), NEWLINE); - // configuration - boolean showJacoco = mavenPom.getJacocoMinLineCoverage() != null && - mavenPom.getJacocoMinBranchCoverage() != null; - boolean showCheckStyle = mavenPom.getCheckstyleExcludes() != null && - !mavenPom.getCheckstyleExcludes().isEmpty(); + indentedBlock(() -> { + if (showCheckStyle) { + tokeniseKeyValue("checkstyle-excludes", mavenPom.getCheckstyleExcludes(), ""); + } + if (showJacoco) { + addToken(INDENT, new Token(KEYWORD, "jacoco"), SPACE); + addToken(new Token(PUNCTUATION, "{"), NEWLINE); + indentedBlock(() -> { + tokeniseKeyValue("min-line-coverage", mavenPom.getJacocoMinLineCoverage(), "jacoco"); + tokeniseKeyValue("min-branch-coverage", mavenPom.getJacocoMinBranchCoverage(), "jacoco"); + }); + addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); + } + }); - if (showJacoco || showCheckStyle) { - addToken(INDENT, new Token(KEYWORD, "configuration"), SPACE); - addToken(new Token(PUNCTUATION, "{"), NEWLINE); - indent(); - if (showCheckStyle) { - tokeniseKeyValue("checkstyle-excludes", mavenPom.getCheckstyleExcludes(), ""); - } - if (showJacoco) { - addToken(INDENT, new Token(KEYWORD, "jacoco"), SPACE); - addToken(new Token(PUNCTUATION, "{"), NEWLINE); - indent(); - tokeniseKeyValue("min-line-coverage", mavenPom.getJacocoMinLineCoverage(), "jacoco"); - tokeniseKeyValue("min-branch-coverage", mavenPom.getJacocoMinBranchCoverage(), "jacoco"); - unindent(); addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); } - unindent(); - addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); - } - // Maven name - tokeniseKeyValue("name", mavenPom.getName(), ""); + // Maven name + tokeniseKeyValue("name", mavenPom.getName(), ""); - // Maven description - tokeniseKeyValue("description", mavenPom.getDescription(), ""); + // Maven description + tokeniseKeyValue("description", mavenPom.getDescription(), ""); - // dependencies - addToken(INDENT, new Token(KEYWORD, "dependencies"), SPACE); - addToken(new Token(PUNCTUATION, "{"), NEWLINE); + // dependencies + addToken(INDENT, new Token(KEYWORD, "dependencies"), SPACE); + addToken(new Token(PUNCTUATION, "{"), NEWLINE); - indent(); - mavenPom.getDependencies().stream().collect(Collectors.groupingBy(Dependency::getScope)).forEach((k, v) -> { - if ("test".equals(k)) { - // we don't care to present test scope dependencies - return; - } - String scope = k.isEmpty() ? "compile" : k; + indentedBlock(() -> { + mavenPom.getDependencies() + .stream() + .collect(Collectors.groupingBy(Dependency::getScope)) + .forEach((k, v) -> { + if ("test".equals(k)) { + // we don't care to present test scope dependencies + return; + } + String scope = k.isEmpty() ? "compile" : k; - addToken(makeWhitespace()); - addToken(new Token(COMMENT, "// " + scope + " scope"), NEWLINE); + addToken(makeWhitespace()); + addToken(new Token(COMMENT, "// " + scope + " scope"), NEWLINE); - for (int i = 0; i < v.size(); i++) { - Dependency d = v.get(i); - addToken(makeWhitespace()); - String gav = d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion(); - addToken(new Token(TEXT, gav, gav)); + for (int i = 0; i < v.size(); i++) { + Dependency d = v.get(i); + addToken(makeWhitespace()); + String gav = d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion(); + addToken(new Token(TEXT, gav, gav)); - if (i < v.size() - 1) { - addNewLine(); - } - } + if (i < v.size() - 1) { + addNewLine(); + } + } + + addNewLine(); + }); + }); - addNewLine(); + addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); + addToken(new Token(SKIP_DIFF_END)); + // close maven }); - unindent(); - addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); - addToken(new Token(SKIP_DIFF_END)); - - // allowed dependencies (in maven-enforcer) -// if (!mavenPom.getAllowedDependencies().isEmpty()) { -// addToken(INDENT, new Token(KEYWORD, "allowed-dependencies"), SPACE); -// addToken(new Token(PUNCTUATION, "{"), NEWLINE); -// indent(); -// mavenPom.getAllowedDependencies().stream().forEach(value -> { -// addToken(INDENT, new Token(TEXT, value, value), NEWLINE); -// }); -// unindent(); -// addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); -// } - - // close maven - unindent(); addToken(INDENT, new Token(PUNCTUATION, "}"), NEWLINE); } @@ -505,9 +492,7 @@ private void visitClassOrInterfaceOrEnumDeclaration(TypeDeclaration typeDecla if (typeDeclaration.getMembers().isEmpty()) { // we have an empty interface declaration, it is probably a marker interface and we will leave a // comment to that effect - indent(); - addComment("// This interface is does not declare any API."); - unindent(); + indentedBlock(() -> addComment("// This interface is does not declare any API.")); } } @@ -540,62 +525,66 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { }; moduleDeclaration.getDirectives().forEach(moduleDirective -> { - indent(); - addToken(makeWhitespace()); + indentedBlock(() -> { + addToken(makeWhitespace()); - moduleDirective.ifModuleRequiresStmt(d -> { - addToken(new Token(KEYWORD, "requires"), SPACE); + moduleDirective.ifModuleRequiresStmt(d -> { + addToken(new Token(KEYWORD, "requires"), SPACE); - if (d.hasModifier(Modifier.Keyword.STATIC)) { - addToken(new Token(KEYWORD, "static"), SPACE); - } + if (d.hasModifier(Modifier.Keyword.STATIC)) { + addToken(new Token(KEYWORD, "static"), SPACE); + } - if (d.isTransitive()) { - addToken(new Token(KEYWORD, "transitive"), SPACE); - } + if (d.isTransitive()) { + addToken(new Token(KEYWORD, "transitive"), SPACE); + } - addToken(new Token(TYPE_NAME, d.getNameAsString(), makeId(MODULE_INFO_KEY + "-requires-" + d.getNameAsString()))); - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - }); + addToken(new Token(TYPE_NAME, d.getNameAsString(), + makeId(MODULE_INFO_KEY + "-requires-" + d.getNameAsString()))); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + }); - moduleDirective.ifModuleExportsStmt(d -> { - addToken(new Token(KEYWORD, "exports"), SPACE); - addToken(new Token(TYPE_NAME, d.getNameAsString(), makeId(MODULE_INFO_KEY + "-exports-" + d.getNameAsString()))); - conditionalExportsToOrOpensToConsumer.accept(d.getModuleNames()); - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - }); + moduleDirective.ifModuleExportsStmt(d -> { + addToken(new Token(KEYWORD, "exports"), SPACE); + addToken(new Token(TYPE_NAME, d.getNameAsString(), + makeId(MODULE_INFO_KEY + "-exports-" + d.getNameAsString()))); + conditionalExportsToOrOpensToConsumer.accept(d.getModuleNames()); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + }); - moduleDirective.ifModuleOpensStmt(d -> { - addToken(new Token(KEYWORD, "opens"), SPACE); - addToken(new Token(TYPE_NAME, d.getNameAsString(), makeId(MODULE_INFO_KEY + "-opens-" + d.getNameAsString()))); - conditionalExportsToOrOpensToConsumer.accept(d.getModuleNames()); - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - }); + moduleDirective.ifModuleOpensStmt(d -> { + addToken(new Token(KEYWORD, "opens"), SPACE); + addToken(new Token(TYPE_NAME, d.getNameAsString(), + makeId(MODULE_INFO_KEY + "-opens-" + d.getNameAsString()))); + conditionalExportsToOrOpensToConsumer.accept(d.getModuleNames()); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + }); - moduleDirective.ifModuleUsesStmt(d -> { - addToken(new Token(KEYWORD, "uses"), SPACE); - addToken(new Token(TYPE_NAME, d.getNameAsString(), makeId(MODULE_INFO_KEY + "-uses-" + d.getNameAsString()))); - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - }); + moduleDirective.ifModuleUsesStmt(d -> { + addToken(new Token(KEYWORD, "uses"), SPACE); + addToken(new Token(TYPE_NAME, d.getNameAsString(), + makeId(MODULE_INFO_KEY + "-uses-" + d.getNameAsString()))); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + }); - moduleDirective.ifModuleProvidesStmt(d -> { - addToken(new Token(KEYWORD, "provides"), SPACE); - addToken(new Token(TYPE_NAME, d.getNameAsString(), makeId(MODULE_INFO_KEY + "-provides-" + d.getNameAsString())), SPACE); - addToken(new Token(KEYWORD, "with"), SPACE); + moduleDirective.ifModuleProvidesStmt(d -> { + addToken(new Token(KEYWORD, "provides"), SPACE); + addToken(new Token(TYPE_NAME, d.getNameAsString(), + makeId(MODULE_INFO_KEY + "-provides-" + d.getNameAsString())), SPACE); + addToken(new Token(KEYWORD, "with"), SPACE); - NodeList names = d.getWith(); - for (int i = 0; i < names.size(); i++) { - addToken(new Token(TYPE_NAME, names.get(i).toString())); + NodeList names = d.getWith(); + for (int i = 0; i < names.size(); i++) { + addToken(new Token(TYPE_NAME, names.get(i).toString())); - if (i < names.size() - 1) { - addToken(new Token(PUNCTUATION, ","), SPACE); + if (i < names.size() - 1) { + addToken(new Token(PUNCTUATION, ","), SPACE); + } } - } - addToken(new Token(PUNCTUATION, ";"), NEWLINE); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + }); }); - - unindent(); }); // close module @@ -605,49 +594,48 @@ private void visitModuleDeclaration(ModuleDeclaration moduleDeclaration) { private void getEnumEntries(EnumDeclaration enumDeclaration) { final NodeList enumConstantDeclarations = enumDeclaration.getEntries(); int size = enumConstantDeclarations.size(); - indent(); - AtomicInteger counter = new AtomicInteger(); + indentedBlock(() -> { + AtomicInteger counter = new AtomicInteger(); - enumConstantDeclarations.forEach(enumConstantDeclaration -> { - visitJavaDoc(enumConstantDeclaration); + enumConstantDeclarations.forEach(enumConstantDeclaration -> { + visitJavaDoc(enumConstantDeclaration); - addToken(makeWhitespace()); + addToken(makeWhitespace()); - // annotations - getAnnotations(enumConstantDeclaration, false, false); + // annotations + getAnnotations(enumConstantDeclaration, false, false); - // create a unique id for enum constants by using the fully-qualified constant name - // (package, enum name, and enum constant name) - final String name = enumConstantDeclaration.getNameAsString(); - final String definitionId = makeId(enumConstantDeclaration); - final boolean isDeprecated = enumConstantDeclaration.isAnnotationPresent("Deprecated"); + // create a unique id for enum constants by using the fully-qualified constant name + // (package, enum name, and enum constant name) + final String name = enumConstantDeclaration.getNameAsString(); + final String definitionId = makeId(enumConstantDeclaration); + final boolean isDeprecated = enumConstantDeclaration.isAnnotationPresent("Deprecated"); - if (isDeprecated) { - addToken(new Token(DEPRECATED_RANGE_START)); - } + if (isDeprecated) { + addToken(new Token(DEPRECATED_RANGE_START)); + } - addToken(new Token(MEMBER_NAME, name, definitionId)); + addToken(new Token(MEMBER_NAME, name, definitionId)); - if (isDeprecated) { - addToken(new Token(DEPRECATED_RANGE_END)); - } + if (isDeprecated) { + addToken(new Token(DEPRECATED_RANGE_END)); + } - enumConstantDeclaration.getArguments().forEach(expression -> { - addToken(new Token(PUNCTUATION, "(")); - addToken(new Token(TEXT, expression.toString())); - addToken(new Token(PUNCTUATION, ")")); - }); + enumConstantDeclaration.getArguments().forEach(expression -> { + addToken(new Token(PUNCTUATION, "(")); + addToken(new Token(TEXT, expression.toString())); + addToken(new Token(PUNCTUATION, ")")); + }); - if (counter.getAndIncrement() < size - 1) { - addToken(new Token(PUNCTUATION, ",")); - } else { - addToken(new Token(PUNCTUATION, ";")); - } - addNewLine(); + if (counter.getAndIncrement() < size - 1) { + addToken(new Token(PUNCTUATION, ",")); + } else { + addToken(new Token(PUNCTUATION, ";")); + } + addNewLine(); + }); }); - - unindent(); } private void getTypeDeclaration(TypeDeclaration typeDeclaration) { @@ -781,113 +769,118 @@ private void checkForCrossLanguageDefinitionId(Token typeNameToken, NodeWithSimp } private void tokeniseAnnotationMember(AnnotationDeclaration annotationDeclaration) { - indent(); - // Member methods in the annotation declaration - NodeList> annotationDeclarationMembers = annotationDeclaration.getMembers(); - for (BodyDeclaration bodyDeclaration : annotationDeclarationMembers) { - Optional annotationMemberDeclarationOptional = bodyDeclaration.toAnnotationMemberDeclaration(); - if (!annotationMemberDeclarationOptional.isPresent()) { - continue; - } - final AnnotationMemberDeclaration annotationMemberDeclaration = annotationMemberDeclarationOptional.get(); + indentedBlock(() -> { + // Member methods in the annotation declaration + NodeList> annotationDeclarationMembers = annotationDeclaration.getMembers(); + for (BodyDeclaration bodyDeclaration : annotationDeclarationMembers) { + Optional annotationMemberDeclarationOptional + = bodyDeclaration.toAnnotationMemberDeclaration(); + if (!annotationMemberDeclarationOptional.isPresent()) { + continue; + } + final AnnotationMemberDeclaration annotationMemberDeclaration + = annotationMemberDeclarationOptional.get(); - addToken(makeWhitespace()); - getClassType(annotationMemberDeclaration.getType()); - addToken(new Token(WHITESPACE, " ")); + addToken(makeWhitespace()); + getClassType(annotationMemberDeclaration.getType()); + addToken(new Token(WHITESPACE, " ")); - final String name = annotationMemberDeclaration.getNameAsString(); - final String definitionId = makeId(annotationDeclaration.getFullyQualifiedName().get() + "." + name); + final String name = annotationMemberDeclaration.getNameAsString(); + final String definitionId = makeId( + annotationDeclaration.getFullyQualifiedName().get() + "." + name); - addToken(new Token(MEMBER_NAME, name, definitionId)); - addToken(new Token(PUNCTUATION, "(")); - addToken(new Token(PUNCTUATION, ")")); + addToken(new Token(MEMBER_NAME, name, definitionId)); + addToken(new Token(PUNCTUATION, "(")); + addToken(new Token(PUNCTUATION, ")")); - // default value - final Optional defaultValueOptional = annotationMemberDeclaration.getDefaultValue(); - if (defaultValueOptional.isPresent()) { - addToken(SPACE, new Token(KEYWORD, "default"), SPACE); + // default value + final Optional defaultValueOptional = annotationMemberDeclaration.getDefaultValue(); + if (defaultValueOptional.isPresent()) { + addToken(SPACE, new Token(KEYWORD, "default"), SPACE); - final Expression defaultValueExpr = defaultValueOptional.get(); - final String value = defaultValueExpr.toString(); - addToken(new Token(KEYWORD, value)); - } + final Expression defaultValueExpr = defaultValueOptional.get(); + final String value = defaultValueExpr.toString(); + addToken(new Token(KEYWORD, value)); + } - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - } - unindent(); + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + } + }); } private void tokeniseFields(boolean isInterfaceDeclaration, TypeDeclaration typeDeclaration) { final List fieldDeclarations = typeDeclaration.getFields(); final String fullPathName = typeDeclaration.getFullyQualifiedName().get(); - indent(); - for (FieldDeclaration fieldDeclaration : fieldDeclarations) { - // By default , interface has public abstract methods if there is no access specifier declared - if (isInterfaceDeclaration) { - // no-op - we take all methods in the method - } else if (isPrivateOrPackagePrivate(fieldDeclaration.getAccessSpecifier())) { - // Skip if not public API - continue; - } + indentedBlock(() -> { + for (FieldDeclaration fieldDeclaration : fieldDeclarations) { + // By default , interface has public abstract methods if there is no access specifier declared + if (isInterfaceDeclaration) { + // no-op - we take all methods in the method + } else if (isPrivateOrPackagePrivate(fieldDeclaration.getAccessSpecifier())) { + // Skip if not public API + continue; + } - visitJavaDoc(fieldDeclaration); + visitJavaDoc(fieldDeclaration); - addToken(makeWhitespace()); + addToken(makeWhitespace()); - // Add annotation for field declaration - getAnnotations(fieldDeclaration, false, false); + // Add annotation for field declaration + getAnnotations(fieldDeclaration, false, false); - final NodeList fieldModifiers = fieldDeclaration.getModifiers(); - // public, protected, static, final - for (final Modifier fieldModifier : fieldModifiers) { - addToken(new Token(KEYWORD, fieldModifier.toString())); - } + final NodeList fieldModifiers = fieldDeclaration.getModifiers(); + // public, protected, static, final + for (final Modifier fieldModifier : fieldModifiers) { + addToken(new Token(KEYWORD, fieldModifier.toString())); + } - // field type and name - final NodeList variableDeclarators = fieldDeclaration.getVariables(); + // field type and name + final NodeList variableDeclarators = fieldDeclaration.getVariables(); - if (variableDeclarators.size() > 1) { - getType(fieldDeclaration); + if (variableDeclarators.size() > 1) { + getType(fieldDeclaration); - for (VariableDeclarator variableDeclarator : variableDeclarators) { + for (VariableDeclarator variableDeclarator : variableDeclarators) { + final String name = variableDeclarator.getNameAsString(); + final String definitionId = makeId(fullPathName + "." + variableDeclarator.getName()); + addToken(new Token(MEMBER_NAME, name, definitionId)); + addToken(new Token(PUNCTUATION, ","), SPACE); + } + apiListing.getTokens().remove(apiListing.getTokens().size() - 1); + apiListing.getTokens().remove(apiListing.getTokens().size() - 1); + } else if (variableDeclarators.size() == 1) { + getType(fieldDeclaration); + final VariableDeclarator variableDeclarator = variableDeclarators.get(0); final String name = variableDeclarator.getNameAsString(); final String definitionId = makeId(fullPathName + "." + variableDeclarator.getName()); addToken(new Token(MEMBER_NAME, name, definitionId)); - addToken(new Token(PUNCTUATION, ","), SPACE); - } - apiListing.getTokens().remove(apiListing.getTokens().size() - 1); - apiListing.getTokens().remove(apiListing.getTokens().size() - 1); - } else if (variableDeclarators.size() == 1) { - getType(fieldDeclaration); - final VariableDeclarator variableDeclarator = variableDeclarators.get(0); - final String name = variableDeclarator.getNameAsString(); - final String definitionId = makeId(fullPathName + "." + variableDeclarator.getName()); - addToken(new Token(MEMBER_NAME, name, definitionId)); - final Optional variableDeclaratorOption = variableDeclarator.getInitializer(); - if (variableDeclaratorOption.isPresent()) { - Expression e = variableDeclaratorOption.get(); - if (e.isObjectCreationExpr() && e.asObjectCreationExpr().getAnonymousClassBody().isPresent()) { - // no-op because we don't want to include all of the anonymous inner class details - } else { - addToken(SPACE, new Token(PUNCTUATION, "="), SPACE); - addToken(new Token(TEXT, variableDeclaratorOption.get().toString())); + final Optional variableDeclaratorOption = variableDeclarator.getInitializer(); + if (variableDeclaratorOption.isPresent()) { + Expression e = variableDeclaratorOption.get(); + if (e.isObjectCreationExpr() && e.asObjectCreationExpr() + .getAnonymousClassBody() + .isPresent()) { + // no-op because we don't want to include all of the anonymous inner class details + } else { + addToken(SPACE, new Token(PUNCTUATION, "="), SPACE); + addToken(new Token(TEXT, variableDeclaratorOption.get().toString())); + } } } - } - // close the variable declaration - addToken(new Token(PUNCTUATION, ";"), NEWLINE); - } - unindent(); + // close the variable declaration + addToken(new Token(PUNCTUATION, ";"), NEWLINE); + } + }); } private void tokeniseConstructorsOrMethods(final TypeDeclaration typeDeclaration, final boolean isInterfaceDeclaration, final boolean isConstructor, final List> callableDeclarations) { - indent(); + indentedBlock(() -> { if (isConstructor) { // determining if we are looking at a set of constructors that are all private, indicating that the @@ -900,12 +893,10 @@ private void tokeniseConstructorsOrMethods(final TypeDeclaration typeDeclarat if (isAllPrivateOrPackagePrivate) { if (typeDeclaration.isEnumDeclaration()) { - unindent(); return; } addComment("// This class does not have any public constructors, and is not able to be instantiated using 'new'."); - unindent(); return; } } @@ -981,8 +972,7 @@ private void tokeniseConstructorsOrMethods(final TypeDeclaration typeDeclarat addNewLine(); }); }); - - unindent(); + }); } private int sortMethods(CallableDeclaration c1, CallableDeclaration c2) { @@ -1044,9 +1034,8 @@ private int sortMethods(CallableDeclaration c1, CallableDeclaration c2) { private void tokeniseInnerClasses(Stream> innerTypes) { innerTypes.forEach(innerType -> { if (innerType.isEnumDeclaration() || innerType.isClassOrInterfaceDeclaration()) { - indent(); - new ClassOrInterfaceVisitor(parentNav).visitClassOrInterfaceOrEnumDeclaration(innerType); - unindent(); + indentedBlock(() -> new ClassOrInterfaceVisitor(parentNav) + .visitClassOrInterfaceOrEnumDeclaration(innerType)); } }); } @@ -1092,7 +1081,7 @@ private void getAnnotations(final NodeWithAnnotations nodeWithAnnotations, private List renderAnnotation(AnnotationRendererModel m) { final AnnotationExpr a = m.getAnnotation(); List tokens = new ArrayList<>(); - tokens.add(new Token(TYPE_NAME, "@" + a.getNameAsString(), makeId(a, m.getAnnotationParent()))); + tokens.add(new Token(TYPE_NAME, "@" + a.getNameAsString())); if (m.isShowProperties()) { if (a instanceof NormalAnnotationExpr) { tokens.add(new Token(PUNCTUATION, "(")); @@ -1404,16 +1393,14 @@ private void getTypeDFS(Node node) { } private void addDefaultConstructor(TypeDeclaration typeDeclaration) { - indent(); - - addToken(INDENT, new Token(KEYWORD, "public"), SPACE); - final String name = typeDeclaration.getNameAsString(); - final String definitionId = makeId(typeDeclaration.getNameAsString()); - addToken(new Token(MEMBER_NAME, name, definitionId)); - addToken(new Token(PUNCTUATION, "(")); - addToken(new Token(PUNCTUATION, ")"), NEWLINE); - - unindent(); + indentedBlock(() -> { + addToken(INDENT, new Token(KEYWORD, "public"), SPACE); + final String name = typeDeclaration.getNameAsString(); + final String definitionId = makeId(typeDeclaration.getNameAsString()); + addToken(new Token(MEMBER_NAME, name, definitionId)); + addToken(new Token(PUNCTUATION, "(")); + addToken(new Token(PUNCTUATION, ")"), NEWLINE); + }); } } @@ -1482,7 +1469,7 @@ private void visitJavaDoc(JavadocComment javadoc) { // The updated configuration from getDeclarationAsString removes the comment option and hence the toString // returns an empty string now. So, here we are using the toString overload that takes a PrintConfiguration // to get the old behavior. - splitNewLine(javadoc.asString()).forEach(line -> { + splitNewLine(javadoc.toString()).forEach(line -> { // we want to wrap our javadocs so that they are easier to read, so we wrap at 120 chars MiscUtils.wrap(line, 120).forEach(line2 -> { if (line2.contains("&")) { @@ -1518,22 +1505,24 @@ private void visitJavaDoc(JavadocComment javadoc) { addToken(new Token(DOCUMENTATION_RANGE_END)); } - private void indent() { - indent += 4; + private void indentedBlock(Runnable indentedSection) { + indentLevel++; + indentedSection.run(); + indentLevel--; } - private void unindent() { - indent = Math.max(indent - 4, 0); + private Token makeWhitespace() { + return new Token(WHITESPACE, makeWhitespace(indentLevel)); } - private Token makeWhitespace() { + private static String makeWhitespace(int indentLevel) { // Use a byte array with Arrays.fill with empty space (' ') character rather than StringBuilder as StringBuilder // will check that it has sufficient size every time a new character is appended. We know ahead of time the size // needed and can remove all those checks by removing usage of StringBuilder with this simpler pattern. - byte[] bytes = new byte[indent]; + byte[] bytes = new byte[indentLevel * 4]; Arrays.fill(bytes, (byte) ' '); - return new Token(WHITESPACE, new String(bytes, StandardCharsets.UTF_8)); + return new String(bytes, StandardCharsets.UTF_8); } private void addComment(String comment) { From 93fbd668368cac7bdd58d4c86287284541847574 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Wed, 17 Apr 2024 12:30:20 -0700 Subject: [PATCH 21/23] [Pylint] bug fix typing (#8104) * recognize else * check string of the if * update * pass * Update tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md --- .../CHANGELOG.md | 4 +++ .../pylint_guidelines_checker.py | 31 ++++++++++++------- .../azure-pylint-guidelines-checker/setup.py | 2 +- .../tests/test_pylint_custom_plugins.py | 19 ++++++++++++ 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md b/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md index 523606fb676..65beb3a3bca 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/CHANGELOG.md @@ -1,5 +1,9 @@ # Release History +## 0.4.1 (2024-04-17) + +- Bug fix for typing under TYPE_CHECKING block. + ## 0.4.0 (2024-04-15) - Checker to enforce no importing typing under TYPE_CHECKING block. diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py index 3bba4819fb0..f79af8f887a 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py @@ -2725,23 +2725,30 @@ class NoImportTypingFromTypeCheck(BaseChecker): def visit_importfrom(self, node): """Check that we aren't importing from typing under if TYPE_CHECKING.""" - if isinstance(node.parent, astroid.If) and (node.modname == "typing" or node.modname == "typing_extensions"): - self.add_message( - msgid=f"no-typing-import-in-type-check", - node=node, - confidence=None, - ) - - def visit_import(self, node): - """Check that we aren't importing from typing under if TYPE_CHECKING.""" - if isinstance(node.parent, astroid.If): - for name, _ in node.names: - if name == "typing" or name == "typing_extensions": + try: + if isinstance(node.parent, astroid.If) and "TYPE_CHECKING" in node.parent.as_string(): + if node.modname == "typing" or node.modname == "typing_extensions": self.add_message( msgid=f"no-typing-import-in-type-check", node=node, confidence=None, ) + except: + pass + + def visit_import(self, node): + """Check that we aren't importing from typing under if TYPE_CHECKING.""" + try: + if isinstance(node.parent, astroid.If) and "TYPE_CHECKING" in node.parent.as_string(): + for name, _ in node.names: + if name == "typing" or name == "typing_extensions": + self.add_message( + msgid=f"no-typing-import-in-type-check", + node=node, + confidence=None, + ) + except: + pass # if a linter is registered in this function then it will be checked with pylint def register(linter): diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py index 5e8cb4aaea7..12db0fc7bdb 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/setup.py @@ -6,7 +6,7 @@ setup( name="azure-pylint-guidelines-checker", - version="0.4.0", + version="0.4.1", url="http://github.com/Azure/azure-sdk-for-python", license="MIT License", description="A pylint plugin which enforces azure sdk guidelines.", diff --git a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py index d2a2f21261d..dfdca1775bc 100644 --- a/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py +++ b/tools/pylint-extensions/azure-pylint-guidelines-checker/tests/test_pylint_custom_plugins.py @@ -4811,3 +4811,22 @@ def test_allowed_imports(self): ) with self.assertNoMessages(): self.checker.visit_importfrom(importfrom_node) + + def test_allowed_import_else(self): + """Check that illegal imports raise warnings""" + ima, imb, imc, imd = astroid.extract_node( + """ + if sys.version_info >= (3, 9): + from collections.abc import MutableMapping + else: + from typing import MutableMapping #@ + import typing #@ + import typing_extensions #@ + from typing_extensions import Protocol #@ + """ + ) + with self.assertNoMessages(): + self.checker.visit_importfrom(ima) + self.checker.visit_import(imb) + self.checker.visit_import(imc) + self.checker.visit_importfrom(imd) \ No newline at end of file From 5bac009a6ea1a439e6412e1825818ef8012f0b09 Mon Sep 17 00:00:00 2001 From: Jonathan Giles Date: Wed, 17 Apr 2024 12:58:27 -0700 Subject: [PATCH 22/23] Introducing a test framework for Java APIView processor (#8115) Introducing a test framework for Java APIView processor to compare output for published libraries against their known-good representations, to help avoid regrettable regressions. There are no test outputs in place yet - they will be added after the token refactoring is done. --- .../azure/tools/apiview/processor/Main.java | 59 +++++--- .../tools/apiview/TestExpectedOutputs.java | 142 ++++++++++++++++++ 2 files changed, 179 insertions(+), 22 deletions(-) create mode 100644 src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/TestExpectedOutputs.java diff --git a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java index 28dc2dcd135..422e5988e83 100644 --- a/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java +++ b/src/java/apiview-java-processor/src/main/java/com/azure/tools/apiview/processor/Main.java @@ -22,6 +22,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.Enumeration; import java.util.List; import java.util.jar.JarEntry; @@ -33,7 +34,7 @@ public class Main { // expected argument order: // [inputFiles] - public static void main(String[] args) throws IOException { + public static void main(String[] args) { if (args.length != 2) { System.out.println("Expected argument order: [comma-separated sources jarFiles] , e.g. /path/to/jarfile.jar ./temp/"); System.exit(-1); @@ -43,28 +44,35 @@ public static void main(String[] args) throws IOException { final String[] jarFilesArray = jarFiles.split(","); final File outputDir = new File(args[1]); - if (!outputDir.exists()) { - if (!outputDir.mkdirs()) { - System.out.printf("Failed to create output directory %s%n", outputDir); - } - } System.out.println("Running with following configuration:"); System.out.printf(" Output directory: '%s'%n", outputDir); - for (final String jarFile : jarFilesArray) { - System.out.printf(" Processing input .jar file: '%s'%n", jarFile); + Arrays.stream(jarFilesArray).forEach(jarFile -> run(new File(jarFile), outputDir)); + } - final File file = new File(jarFile); - if (!file.exists()) { - System.out.printf("Cannot find file '%s'%n", file); + /** + * Runs APIView parser and returns the output file path. + */ + public static File run(File jarFile, File outputDir) { + System.out.printf(" Processing input .jar file: '%s'%n", jarFile); + + if (!jarFile.exists()) { + System.out.printf("Cannot find file '%s'%n", jarFile); + System.exit(-1); + } + + if (!outputDir.exists()) { + if (!outputDir.mkdirs()) { + System.out.printf("Failed to create output directory %s%n", outputDir); System.exit(-1); } - - final String jsonFileName = file.getName().substring(0, file.getName().length() - 4) + ".json"; - final File outputFile = new File(outputDir, jsonFileName); - processFile(file, outputFile); } + + final String jsonFileName = jarFile.getName().substring(0, jarFile.getName().length() - 4) + ".json"; + final File outputFile = new File(outputDir, jsonFileName); + processFile(jarFile, outputFile); + return outputFile; } private static ReviewProperties getReviewProperties(File inputFile) { @@ -104,7 +112,7 @@ private static ReviewProperties getReviewProperties(File inputFile) { return reviewProperties; } - private static void processFile(final File inputFile, final File outputFile) throws IOException { + private static void processFile(final File inputFile, final File outputFile) { final APIListing apiListing = new APIListing(); // empty tokens list that we will fill as we process each class file @@ -126,13 +134,22 @@ private static void processFile(final File inputFile, final File outputFile) thr "that was submitted to APIView was named " + inputFile.getName())); } - // Write out to the filesystem - try (JsonWriter jsonWriter = JsonProviders.createWriter(Files.newBufferedWriter(outputFile.toPath()))) { - apiListing.toJson(jsonWriter); + try { + // Write out to the filesystem, make the file if it doesn't exist + if (!outputFile.exists()) { + if (!outputFile.createNewFile()) { + System.out.printf("Failed to create output file %s%n", outputFile); + } + } + try (JsonWriter jsonWriter = JsonProviders.createWriter(Files.newBufferedWriter(outputFile.toPath()))) { + apiListing.toJson(jsonWriter); + } + } catch (IOException e) { + e.printStackTrace(); } } - private static void processJavaSourcesJar(File inputFile, APIListing apiListing) throws IOException { + private static void processJavaSourcesJar(File inputFile, APIListing apiListing) { final ReviewProperties reviewProperties = getReviewProperties(inputFile); final String groupId = reviewProperties.getMavenPom().getGroupId(); @@ -260,8 +277,6 @@ private static void processXmlFile(File inputFile, APIListing apiListing) { } catch (IOException e) { e.printStackTrace(); } - - } private static class ReviewProperties { diff --git a/src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/TestExpectedOutputs.java b/src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/TestExpectedOutputs.java new file mode 100644 index 00000000000..b44d6926016 --- /dev/null +++ b/src/java/apiview-java-processor/src/test/java/com/azure/tools/apiview/TestExpectedOutputs.java @@ -0,0 +1,142 @@ +package com.azure.tools.apiview; + +import com.azure.tools.apiview.processor.Main; +import org.junit.jupiter.api.*; +import org.junit.jupiter.params.*; +import org.junit.jupiter.params.provider.*; + +import java.net.URL; +import java.nio.file.*; +import java.io.*; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; + +public class TestExpectedOutputs { + // This is a flag to add a '-DELETE' suffix to the generated output file if the expected output file is missing. + // By default we do, but if we want to regenerate a bunch of expected outputs, we can see this to false to save + // having to rename. + private static final boolean ADD_DELETE_PREFIX_FOR_MISSING_EXPECTED_OUTPUTS = true; + + private static final String root = "src/test/resources/"; + private static final File tempDir = new File(root + "temp"); + + private static Stream provideFileNames() { + // This is a stream of local files or URLs to download the file from (or a combination of both). For each value, + // if it starts with http, it'll try downloading the file into a temp location. If it sees no http, it'll look + // for the file in the inputs directory. To prevent clogging up the repository with source jars, it is + // preferable to download the source jars from known-good places. + return Stream.of( + "" +// "https://repo1.maven.org/maven2/com/azure/azure-core/1.48.0/azure-core-1.48.0-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-communication-chat/1.5.0/azure-communication-chat-1.5.0-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-security-keyvault-keys/4.8.2/azure-security-keyvault-keys-4.8.2-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-data-appconfiguration/1.6.0/azure-data-appconfiguration-1.6.0-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-messaging-eventhubs/5.18.2/azure-messaging-eventhubs-5.18.2-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-messaging-servicebus/7.15.2/azure-messaging-servicebus-7.15.2-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-identity/1.12.0/azure-identity-1.12.0-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-storage-blob/12.25.3/azure-storage-blob-12.25.3-sources.jar", +// "https://repo1.maven.org/maven2/com/azure/azure-cosmos/4.57.0/azure-cosmos-4.57.0-sources.jar" + ); + } + + @BeforeAll + public static void setup() { + tempDir.mkdirs(); + } + + @ParameterizedTest + @MethodSource("provideFileNames") + public void testGeneratedJson(String filenameOrUrl) { + if (filenameOrUrl.isEmpty()) { + return; + } + + String filename; + Path inputFile; + + if (filenameOrUrl.startsWith("http")) { + // download the file if it isn't local... + filename = filenameOrUrl.substring(filenameOrUrl.lastIndexOf('/') + 1); + + // download the file and save it to the temp directory + String destinationFile = tempDir + "/" + filename; + downloadFile(filenameOrUrl, destinationFile); + + // strip off the file extension + filename = filename.substring(0, filename.lastIndexOf('.')); + inputFile = Paths.get(destinationFile); + } else { + // the file is local, so we can go straight to it + filename = filenameOrUrl; + inputFile = Paths.get(root + "inputs/" + filename + ".jar"); + } + + final Path expectedOutputFile = Paths.get(root + "expected-outputs/" + filename + ".json"); + + // Run the processor, receiving the name of the file that was generated + final Path actualOutputFile = Main.run(inputFile.toFile(), tempDir).toPath(); + + if (expectedOutputFile.toFile().exists()) { + try { + // Compare the temporary file to the expected-outputs file + assertArrayEquals(Files.readAllBytes(expectedOutputFile), Files.readAllBytes(actualOutputFile)); + } catch (IOException e) { + fail("Failed to compare the actual output file with the expected output file.", e); + } + } else { + // the test was never going to pass as we don't have an expected file to compare against. + try { + final String outputFile = root + "expected-outputs/" + filename + (ADD_DELETE_PREFIX_FOR_MISSING_EXPECTED_OUTPUTS ? "-DELETE" : "") + ".json"; + final Path suggestedOutputFile = Paths.get(outputFile); + + if (!suggestedOutputFile.toFile().getParentFile().exists()) { + suggestedOutputFile.toFile().getParentFile().mkdirs(); + } + Files.move(actualOutputFile, suggestedOutputFile, StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + // unable to move file + e.printStackTrace(); + } + if (ADD_DELETE_PREFIX_FOR_MISSING_EXPECTED_OUTPUTS) { + fail("Could not find expected output file for test. The output from the sources.jar was added into the " + + "/expected-outputs directory, but with a '-DELETE' filename suffix. Please review and rename if it " + + "should be included in the test suite."); + } else { + fail("Could not find expected output file for test. The output from the sources.jar was added into the " + + "/expected-outputs directory without any -DELETE suffix. It will be used next time the test is run!"); + + } + } + } + + @AfterAll + public static void tearDown() throws IOException { + // Delete the temporary file + if (tempDir.exists()) { + // delete everything in tempDir and then delete the directory + Files.walk(tempDir.toPath()) + .map(Path::toFile) + .forEach(File::delete); + + try { + Files.delete(tempDir.toPath()); + } catch (NoSuchFileException e) { + // ignore + } + } + } + + private static void downloadFile(String urlStr, String destinationFile) { + File file = new File(destinationFile); + if (file.exists()) { + return; + } + try (InputStream in = new URL(urlStr).openStream()) { + Files.copy(in, file.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + fail("Failed to download and / or copy the given URL to the given destination { url: " + + urlStr + ", destination: " + destinationFile + " }"); + } + } +} \ No newline at end of file From 93565cb9f86ef0308d33f044ba850b33c8edc023 Mon Sep 17 00:00:00 2001 From: Scott Beddall <45376673+scbedd@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:03:05 -0700 Subject: [PATCH 23/23] Universal Output wasn't actually universal (#8072) * change the addition of consoles so that on universal output, stuff really doesn't make it to stderr * finally fix the weirdness with the loglevel --- .../CommandOptions/OptionsGenerator.cs | 3 ++- tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/CommandOptions/OptionsGenerator.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/CommandOptions/OptionsGenerator.cs index 35f48b6cda7..8f0ad59bb8b 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/CommandOptions/OptionsGenerator.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/CommandOptions/OptionsGenerator.cs @@ -50,7 +50,7 @@ public static RootCommand GenerateCommandLineOptions(Func dumpOption.AddAlias("-d"); var universalOption = new Option( - name: "--universalOutput", + name: "--universal", description: "Flag; Redirect all logs to stdout, including what would normally be showing up on stderr.", getDefaultValue: () => false); universalOption.AddAlias("-u"); @@ -83,6 +83,7 @@ public static RootCommand GenerateCommandLineOptions(Func var startCommand = new Command("start", "Start the TestProxy."); startCommand.AddOption(insecureOption); startCommand.AddOption(dumpOption); + startCommand.AddOption(universalOption); startCommand.AddArgument(collectedArgs); startCommand.SetHandler(async (startOpts) => await callback(startOpts), diff --git a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs index 90d868513bf..b4285640fec 100644 --- a/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs +++ b/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Startup.cs @@ -147,10 +147,14 @@ private static void StartServer(StartOptions startOptions) { loggingBuilder.ClearProviders(); loggingBuilder.AddConfiguration(hostBuilder.Configuration.GetSection("Logging")); - loggingBuilder.AddConsole(options => + if (!startOptions.UniversalOutput) { - options.LogToStandardErrorThreshold = startOptions.UniversalOutput ? LogLevel.None : LogLevel.Error; - }).AddSimpleConsole(options => + loggingBuilder.AddConsole(options => + { + options.LogToStandardErrorThreshold = LogLevel.Error; + }); + } + loggingBuilder.AddSimpleConsole(options => { options.TimestampFormat = "[HH:mm:ss] "; });