Merge pull request fabric8io#4849 from andreaTP/wip-race

[java-generator] Fix a race condition in the usage of JavaParser for large CRDs
shawkins · Feb 10, 2023 · c0cf3d0 · c0cf3d0
2 parents 0162d30 + cc90035
commit c0cf3d0
Show file tree

Hide file tree

Showing 10 changed files with 21 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 * Fix #4798: fix leader election release on cancel
 * Fix #4815: (java-generator) create target download directory if it doesn't exist
 * Fix #4818: [java-generator] Escape `*/` in generated JavaDocs
+* Fix #4723: [java-generator] Fix a race in the use of JavaParser hitting large CRDs
 
 #### Improvements
 * Fix #4747: migrate to SnakeYAML Engine

diff --git a/java-generator/cli/src/main/java/io/fabric8/java/generator/cli/GenerateJavaSources.java b/java-generator/cli/src/main/java/io/fabric8/java/generator/cli/GenerateJavaSources.java
@@ -75,7 +75,7 @@ public class GenerateJavaSources implements Runnable {
   String codeStructure = null;
 
   @Option(names = { "-skip-generated-annotations",
-      "--skip-generated-annotations" }, description = "Add extra lombok and sundrio annotation to the generated classes", required = false, hidden = true)
+      "--skip-generated-annotations" }, description = "Skip emitting the @javax.annotation.processing.Generated annotation on the generated sources", required = false, hidden = true)
   Boolean skipGeneratedAnnotations = null;
 
   @Option(names = { "-package-overrides",
@@ -87,15 +87,15 @@ public void run() {
     final Config.Prefix pSt = (prefixStrategy != null) ? Config.Prefix.valueOf(prefixStrategy) : null;
     final Config.Suffix sSt = (suffixStrategy != null) ? Config.Suffix.valueOf(suffixStrategy) : null;
     final Config.CodeStructure structure = (codeStructure != null) ? Config.CodeStructure.valueOf(codeStructure) : null;
-    final Boolean generatedAnnotations = (skipGeneratedAnnotations != null) ? skipGeneratedAnnotations : null;
+    final Boolean noGeneratedAnnotations = (skipGeneratedAnnotations != null) ? skipGeneratedAnnotations : false;
     final Config config = new Config(
         uppercaseEnum,
         pSt,
         sSt,
         alwaysPreserveUnkownFields,
         addExtraAnnotations,
         structure,
-        generatedAnnotations,
+        !noGeneratedAnnotations,
         packageOverrides);
 
     List<JavaGenerator> runners = new ArrayList<>();

diff --git a/java-generator/core/pom.xml b/java-generator/core/pom.xml
@@ -47,13 +47,6 @@
       <version>3.25.0</version>
     </dependency>
 
-    <dependency>
-      <groupId>io.sundr</groupId>
-      <artifactId>sundr-adapter-reflect</artifactId>
-      <version>${sundrio.version}</version>
-      <scope>compile</scope>
-    </dependency>
-
     <dependency>
       <groupId>io.sundr</groupId>
       <artifactId>builder-annotations</artifactId>

diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/CRGeneratorRunner.java b/java-generator/core/src/main/java/io/fabric8/java/generator/CRGeneratorRunner.java
@@ -98,7 +98,7 @@ public List<WritableCRCompilationUnit> generate(CustomResourceDefinition crd, St
 
       List<GeneratorResult.ClassResult> classResults = validateAndAggregate(crGenerator, specGenerator, statusGenerator);
 
-      writableCUs.add(new WritableCRCompilationUnit(classResults));
+      writableCUs.add(new WritableCRCompilationUnit(classResults, basePackageName));
     }
 
     return writableCUs;

diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/FileJavaGenerator.java b/java-generator/core/src/main/java/io/fabric8/java/generator/FileJavaGenerator.java
@@ -95,10 +95,9 @@ private void runOnSingleSource(File source, File basePath) {
                     CustomResourceDefinition crd = (CustomResourceDefinition) resource;
 
                     final String basePackage = groupToPackage(crd.getSpec().getGroup());
-                    List<WritableCRCompilationUnit> writables = crGeneratorRunner.generate(crd, basePackage);
 
-                    writables.parallelStream()
-                        .forEach(w -> w.writeAllJavaClasses(basePath, basePackage));
+                    crGeneratorRunner.generate(crd, basePackage).parallelStream()
+                        .forEach(w -> w.writeAllJavaClasses(basePath));
                   } else {
                     LOGGER.warn("Not generating nothing for resource of kind: {}", resource.getKind());
                   }

diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/WritableCRCompilationUnit.java b/java-generator/core/src/main/java/io/fabric8/java/generator/WritableCRCompilationUnit.java
@@ -34,16 +34,18 @@ public class WritableCRCompilationUnit {
   private static final Logger LOGGER = LoggerFactory.getLogger(WritableCRCompilationUnit.class);
 
   private final List<GeneratorResult.ClassResult> classResults;
+  private final String basePackage;
 
-  WritableCRCompilationUnit(List<GeneratorResult.ClassResult> classResults) {
+  WritableCRCompilationUnit(List<GeneratorResult.ClassResult> classResults, String basePackage) {
     this.classResults = classResults;
+    this.basePackage = basePackage;
   }
 
   public List<GeneratorResult.ClassResult> getClassResults() {
     return classResults;
   }
 
-  public void writeAllJavaClasses(File basePath, String basePackage) {
+  public void writeAllJavaClasses(File basePath) {
     try {
       createFolders(basePackage, basePath);
       for (GeneratorResult.ClassResult cr : this.classResults) {

diff --git a/...generator/core/src/main/java/io/fabric8/java/generator/nodes/AbstractJSONSchema2Pojo.java b/...generator/core/src/main/java/io/fabric8/java/generator/nodes/AbstractJSONSchema2Pojo.java
@@ -42,9 +42,11 @@ public abstract class AbstractJSONSchema2Pojo {
   static final String OBJECT_CRD_TYPE = "object";
   static final String ARRAY_CRD_TYPE = "array";
 
-  public static final AnnotationExpr GENERATED_ANNOTATION = new SingleMemberAnnotationExpr(
-      new Name("javax.annotation.processing.Generated"),
-      new StringLiteralExpr("io.fabric8.java.generator.CRGeneratorRunner"));
+  public static final AnnotationExpr newGeneratedAnnotation() {
+    return new SingleMemberAnnotationExpr(
+        new Name("javax.annotation.processing.Generated"),
+        new StringLiteralExpr("io.fabric8.java.generator.CRGeneratorRunner"));
+  }
 
   protected final String description;
   protected final Config config;

diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JCRObject.java b/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JCRObject.java
@@ -140,7 +140,7 @@ public GeneratorResult generateJava() {
     }
 
     if (config.isGeneratedAnnotations()) {
-      clz.addAnnotation(GENERATED_ANNOTATION);
+      clz.addAnnotation(newGeneratedAnnotation());
     }
     if (config.isObjectExtraAnnotations()) {
       addExtraAnnotations(clz);

diff --git a/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JObject.java b/java-generator/core/src/main/java/io/fabric8/java/generator/nodes/JObject.java
@@ -150,7 +150,7 @@ public GeneratorResult generateJava() {
                 "using = com.fasterxml.jackson.databind.JsonDeserializer.None.class")));
 
     if (config.isGeneratedAnnotations()) {
-      clz.addAnnotation(GENERATED_ANNOTATION);
+      clz.addAnnotation(newGeneratedAnnotation());
     }
     if (config.isObjectExtraAnnotations()) {
       addExtraAnnotations(clz);

diff --git a/java-generator/core/src/test/java/io/fabric8/java/generator/GeneratorTest.java b/java-generator/core/src/test/java/io/fabric8/java/generator/GeneratorTest.java
@@ -452,17 +452,18 @@ void testObjectWithAndWithoutGeneratedAnnotation() {
         null,
         Boolean.FALSE,
         null);
+    String generatedAnnotationName = AbstractJSONSchema2Pojo.newGeneratedAnnotation().getNameAsString();
 
     // Act
     GeneratorResult res1 = obj1.generateJava();
     GeneratorResult res2 = obj2.generateJava();
 
     // Assert
     Optional<ClassOrInterfaceDeclaration> clz1 = res1.getTopLevelClasses().get(0).getCompilationUnit().getClassByName("T");
-    assertTrue(clz1.get().getAnnotationByName(AbstractJSONSchema2Pojo.GENERATED_ANNOTATION.getNameAsString()).isPresent());
+    assertTrue(clz1.get().getAnnotationByName(generatedAnnotationName).isPresent());
 
     Optional<ClassOrInterfaceDeclaration> clz2 = res2.getTopLevelClasses().get(0).getCompilationUnit().getClassByName("T");
-    assertFalse(clz2.get().getAnnotationByName(AbstractJSONSchema2Pojo.GENERATED_ANNOTATION.getNameAsString()).isPresent());
+    assertFalse(clz2.get().getAnnotationByName(generatedAnnotationName).isPresent());
   }
 
   @Test