Improve pom detection algorithm (#4053)

arangatang · web-flow · commit a0301d7d00d4 · 2023-12-18T13:24:25.000-08:00
* Use kotlins walkTopDown instead of intellijs collectChildrenRecursively.

* Reuse validated build files instead of walking again.

---------

Co-authored-by: Leonardo Araneda Freccero &lt;araneda@amazon.com&gt;
diff --git a/.changes/next-release/bugfix-4c31acbd-6839-4b1f-a0ec-7e2c8402e6ff.json b/.changes/next-release/bugfix-4c31acbd-6839-4b1f-a0ec-7e2c8402e6ff.json
@@ -0,0 +1,4 @@
+{
+  "type" : "bugfix",
+  "description": "Improved recursion when validating projects for Q Code Transform."
+}
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/CodeModernizerManager.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/CodeModernizerManager.kt
@@ -18,7 +18,6 @@ import com.intellij.openapi.project.modules
 import com.intellij.openapi.projectRoots.JavaSdkVersion
 import com.intellij.openapi.roots.ProjectRootManager
 import com.intellij.openapi.util.Disposer
-import com.intellij.openapi.vfs.VfsUtil
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.wm.ToolWindowManager
 import kotlinx.coroutines.Job
@@ -44,6 +43,7 @@ import software.aws.toolkits.jetbrains.services.codemodernizer.model.CodeModerni
 import software.aws.toolkits.jetbrains.services.codemodernizer.model.CustomerSelection
 import software.aws.toolkits.jetbrains.services.codemodernizer.model.InvalidTelemetryReason
 import software.aws.toolkits.jetbrains.services.codemodernizer.model.JobId
+import software.aws.toolkits.jetbrains.services.codemodernizer.model.MAVEN_CONFIGURATION_FILE_NAME
 import software.aws.toolkits.jetbrains.services.codemodernizer.model.ValidationResult
 import software.aws.toolkits.jetbrains.services.codemodernizer.panels.managers.CodeModernizerBottomWindowPanelManager
 import software.aws.toolkits.jetbrains.services.codemodernizer.state.CodeModernizerSessionState
@@ -69,6 +69,7 @@ import software.aws.toolkits.telemetry.CodeTransformPreValidationError
 import software.aws.toolkits.telemetry.CodeTransformStartSrcComponents
 import software.aws.toolkits.telemetry.CodetransformTelemetry
 import software.aws.toolkits.telemetry.Result
+import java.io.File
 import java.time.Instant
 import java.util.concurrent.atomic.AtomicBoolean
 import javax.swing.Icon
@@ -88,7 +89,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
             Disposer.register(contentManager, it)
         }
     }
-    private val supportedBuildFileNames = listOf("pom.xml")
+    private val supportedBuildFileNames = listOf(MAVEN_CONFIGURATION_FILE_NAME)
     private val supportedJavaMappings = mapOf(
         JavaSdkVersion.JDK_1_8 to setOf(JavaSdkVersion.JDK_17),
         JavaSdkVersion.JDK_11 to setOf(JavaSdkVersion.JDK_17),
@@ -152,9 +153,9 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
                 )
             )
         }
-        val valid = getSupportedBuildFilesInProject().isNotEmpty()
-        return if (valid) {
-            ValidationResult(true)
+        val validatedBuildFiles = getSupportedBuildFilesInProject()
+        return if (validatedBuildFiles.isNotEmpty()) {
+            ValidationResult(true, validatedBuildFiles = validatedBuildFiles)
         } else {
             ValidationResult(
                 false,
@@ -208,7 +209,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
             val validationResult = validate(project)
             runInEdt {
                 if (validationResult.valid) {
-                    runModernize()
+                    runModernize(validationResult.validatedBuildFiles) ?: isModernizationInProgress.set(false)
                 } else {
                     warnUnsupportedProject(validationResult.invalidReason)
                     isModernizationInProgress.set(false)
@@ -245,9 +246,9 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
         }
     }
 
-    fun runModernize(): Job? {
+    fun runModernize(validatedBuildFiles: List<VirtualFile>): Job? {
         initStopParameters()
-        val customerSelection = getCustomerSelection() ?: return null
+        val customerSelection = getCustomerSelection(validatedBuildFiles) ?: return null
         CodetransformTelemetry.jobStartedCompleteFromPopupDialog(
             codeTransformJavaSourceVersionsAllowed = CodeTransformJavaSourceVersionsAllowed.from(customerSelection.sourceJavaVersion.name),
             codeTransformJavaTargetVersionsAllowed = CodeTransformJavaTargetVersionsAllowed.from(customerSelection.targetJavaVersion.name),
@@ -267,9 +268,9 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
         CodeModernizerSessionState.getInstance(project).currentJobStopTime = Instant.MIN
     }
 
-    fun getCustomerSelection(): CustomerSelection? = PreCodeTransformUserDialog(
+    fun getCustomerSelection(validatedBuildFiles: List<VirtualFile>): CustomerSelection? = PreCodeTransformUserDialog(
         project,
-        getSupportedBuildFilesInProject(),
+        validatedBuildFiles,
         supportedJavaMappings,
     ).create()
 
@@ -638,6 +639,29 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
         managerState.flags.putAll(state.flags)
     }
 
+    private fun findBuildFiles(sourceFolder: File): List<File> {
+        /**
+         * For every dir, check if any supported build files (pom.xml etc) exists.
+         * If found store it and stop further recursion.
+         */
+        val buildFiles = mutableListOf<File>()
+        sourceFolder.walkTopDown()
+            .maxDepth(5)
+            .onEnter { currentDir ->
+                supportedBuildFileNames.forEach {
+                    val maybeSupportedFile = currentDir.resolve(MAVEN_CONFIGURATION_FILE_NAME)
+                    if (maybeSupportedFile.exists()) {
+                        buildFiles.add(maybeSupportedFile)
+                        return@onEnter false
+                    }
+                }
+                return@onEnter true
+            }.forEach {
+                // noop, collects the sequence
+            }
+        return buildFiles
+    }
+
     private fun getSupportedBuildFilesInProject(): List<VirtualFile> {
         /**
          * Strategy:
@@ -648,15 +672,14 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo
         val probableProjectRoot = project.basePath?.toVirtualFile() // May point to only one intellij module (the first opened one)
         val probableContentRoots = projectRootManager.contentRoots.toMutableSet() // May not point to the topmost folder of modules
         probableContentRoots.add(probableProjectRoot) // dedupe
-        val detectedBuildFiles = probableContentRoots.filterNotNull().flatMap { root ->
-            VfsUtil.collectChildrenRecursively(root).filter { it.name in supportedBuildFileNames }
+        val topLevelRoots = filterOnlyParentFiles(probableContentRoots)
+        val detectedBuildFiles = topLevelRoots.flatMap { root ->
+            findBuildFiles(root.toNioPath().toFile()).mapNotNull { it.path.toVirtualFile() }
         }
 
-        val topLevelBuildFiles = filterOnlyParentFiles(detectedBuildFiles)
-
         val supportedModules = getSupportedModulesInProject().toSet()
         val validProjectJdk = project.getSupportedJavaMappingsForProject(supportedJavaMappings).isNotEmpty()
-        return topLevelBuildFiles.filter {
+        return detectedBuildFiles.filter {
             val moduleOfFile = runReadAction { projectRootManager.fileIndex.getModuleForFile(it) }
             return@filter (moduleOfFile in supportedModules) || (moduleOfFile == null && validProjectJdk)
         }
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/Utils.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/Utils.kt
@@ -252,7 +252,7 @@ suspend fun JobId.pollTransformationStatusAndPlan(
     return PollingResult(true, transformationResponse?.transformationJob(), state, transformationPlan)
 }
 
-fun filterOnlyParentFiles(filePaths: List<VirtualFile>): List<VirtualFile> {
+fun filterOnlyParentFiles(filePaths: Set<VirtualFile>): List<VirtualFile> {
     if (filePaths.isEmpty()) return listOf()
     // sorts it like:
     // foo
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/model/ValidationResult.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/codemodernizer/model/ValidationResult.kt
@@ -3,8 +3,11 @@
 
 package software.aws.toolkits.jetbrains.services.codemodernizer.model
 
+import com.intellij.openapi.vfs.VirtualFile
+
 data class ValidationResult(
     val valid: Boolean,
     val invalidReason: String? = null,
-    val invalidTelemetryReason: InvalidTelemetryReason = InvalidTelemetryReason()
+    val invalidTelemetryReason: InvalidTelemetryReason = InvalidTelemetryReason(),
+    val validatedBuildFiles: List<VirtualFile> = emptyList()
 )
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/CodeWhispererActionNodeTest.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/CodeWhispererActionNodeTest.kt
@@ -107,7 +107,7 @@ class CodeWhispererActionNodeTest {
     fun `runCodeModernizer`() {
         whenever(codeModernizerManager.validate(any())).thenReturn(ValidationResult(true))
         whenever(codeModernizerManager.getRunActionButtonIcon()).thenReturn(AllIcons.Actions.Execute)
-        whenever(codeModernizerManager.runModernize()).thenReturn(Job())
+        whenever(codeModernizerManager.runModernize(any())).thenReturn(Job())
         sut = CodeModernizerRunModernizeNode(project)
 
         sut.onDoubleClick(mock())
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/codemodernizer/CodeWhispererCodeModernizerTest.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/codemodernizer/CodeWhispererCodeModernizerTest.kt
@@ -48,12 +48,13 @@ class CodeWhispererCodeModernizerTest : CodeWhispererCodeModernizerTestBase() {
 
     @Test
     fun `test runModernize handles user cancelled dialog`() {
-        doReturn(null).whenever(codeModernizerManagerSpy).getCustomerSelection()
+        doReturn(null).whenever(codeModernizerManagerSpy).getCustomerSelection(any())
         runInEdtAndWait {
-            val job = codeModernizerManagerSpy.runModernize()
+            val buildFiles = listOf(emptyPomFile)
+            val job = codeModernizerManagerSpy.runModernize(buildFiles)
             assertNull(job)
-            verify(codeModernizerManagerSpy, times(1)).runModernize()
-            verify(codeModernizerManagerSpy, times(1)).getCustomerSelection()
+            verify(codeModernizerManagerSpy, times(1)).runModernize(buildFiles)
+            verify(codeModernizerManagerSpy, times(1)).getCustomerSelection(buildFiles)
             // verify execution stopped after user cancelled
             verify(codeModernizerManagerSpy, times(0)).createCodeModernizerSession(any(), any())
             verify(codeModernizerManagerSpy, times(0)).launchModernizationJob(any())
@@ -68,14 +69,15 @@ class CodeWhispererCodeModernizerTest : CodeWhispererCodeModernizerTestBase() {
         `when`(mockFile.name).thenReturn("pomx.xml")
         `when`(mockFile.path).thenReturn("/mocked/path/pom.xml")
         val selection = CustomerSelection(mockFile, JavaSdkVersion.JDK_1_8, JavaSdkVersion.JDK_17)
-        doReturn(selection).whenever(codeModernizerManagerSpy).getCustomerSelection()
+        doReturn(selection).whenever(codeModernizerManagerSpy).getCustomerSelection(any())
         doNothing().whenever(codeModernizerManagerSpy).postModernizationJob(any())
         doReturn(CodeModernizerStartJobResult.Started(jobId)).whenever(testSessionSpy).createModernizationJob()
         runInEdtAndWait {
-            val job = codeModernizerManagerSpy.runModernize()
+            val buildFiles = listOf(emptyPomFile)
+            val job = codeModernizerManagerSpy.runModernize(buildFiles)
             assertNotNull(job)
-            verify(codeModernizerManagerSpy, times(1)).runModernize()
-            verify(codeModernizerManagerSpy, times(1)).getCustomerSelection()
+            verify(codeModernizerManagerSpy, times(1)).runModernize(buildFiles)
+            verify(codeModernizerManagerSpy, times(1)).getCustomerSelection(buildFiles)
             verify(codeModernizerManagerSpy, times(1)).createCodeModernizerSession(any(), any())
             verify(codeModernizerManagerSpy, times(1)).launchModernizationJob(any())
         }
@@ -116,13 +118,13 @@ class CodeWhispererCodeModernizerTest : CodeWhispererCodeModernizerTestBase() {
     @Test
     fun `able to filter roots correctly`() {
         val tests = listOf(
-            listOf<String>() to listOf(),
-            listOf("foo/bar") to listOf("foo/bar"),
-            listOf("foo/bar", "foo/baz", "foo/bar/qux") to listOf("foo/bar", "foo/baz"),
-            listOf("foos", "foo/bar", "foo/baz", "foo/bar/qux") to listOf("foos", "foo/bar", "foo/baz"),
-            listOf("foo", "foo/bar", "foo/baz", "foo/bar/qux") to listOf("foo"),
+            setOf<String>() to listOf(),
+            setOf("foo/bar") to listOf("foo/bar"),
+            setOf("foo/bar", "foo/baz", "foo/bar/qux") to listOf("foo/bar", "foo/baz"),
+            setOf("foos", "foo/bar", "foo/baz", "foo/bar/qux") to listOf("foos", "foo/bar", "foo/baz"),
+            setOf("foo", "foo/bar", "foo/baz", "foo/bar/qux") to listOf("foo"),
         ).map { (input, expected) -> input.map { "$it/pom.xml" } to expected.map { "$it/pom.xml" } }
-        tests.map { (input, expected) -> Pair(input.map { LightVirtualFile(it) }, expected) }
+        tests.map { (input, expected) -> Pair(input.map { LightVirtualFile(it) }.toSet(), expected) }
             .forEach { (input, expected) ->
                 assertEquals(expected, filterOnlyParentFiles(input).map { it.name })
             }
@@ -131,10 +133,10 @@ class CodeWhispererCodeModernizerTest : CodeWhispererCodeModernizerTestBase() {
     @Test
     fun `able to filter roots correctly when multiple on same level`() {
         val tests = listOf(
-            listOf("foo/tmp0.txt", "foo/bar/tmp.txt", "foo/bar/tmp2.txt", "foo/bar/qux/tmp3.txt") to listOf("foo/tmp0.txt"),
-            listOf("foo/bar/tmp.txt", "foo/bar/tmp2.txt", "foo/bar/qux/tmp3.txt") to listOf("foo/bar/tmp.txt", "foo/bar/tmp2.txt"),
+            setOf("foo/tmp0.txt", "foo/bar/tmp.txt", "foo/bar/tmp2.txt", "foo/bar/qux/tmp3.txt") to listOf("foo/tmp0.txt"),
+            setOf("foo/bar/tmp.txt", "foo/bar/tmp2.txt", "foo/bar/qux/tmp3.txt") to listOf("foo/bar/tmp.txt", "foo/bar/tmp2.txt"),
         )
-        tests.map { (input, expected) -> Pair(input.map { LightVirtualFile(it) }, expected) }
+        tests.map { (input, expected) -> Pair(input.map { LightVirtualFile(it) }.toSet(), expected) }
             .forEach { (input, expected) ->
                 assertEquals(expected, filterOnlyParentFiles(input).map { it.name })
             }
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/codemodernizer/CodeWhispererCodeModernizerTestBase.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/codewhisperer/codemodernizer/CodeWhispererCodeModernizerTestBase.kt
@@ -89,6 +89,7 @@ open class CodeWhispererCodeModernizerTestBase(
     internal lateinit var testSessionStateSpy: CodeModernizerSessionState
     internal val diffResource = "diff.patch".toResourceFile()
     internal val examplePatchVirtualFile = LightVirtualFile("diff.patch", diffResource.readText())
+    internal val emptyPomFile = LightVirtualFile("pom.xml", "")
     internal val jobId = JobId("Test job id")
     internal val migrationStep = MigrationStep("Test migration step")
     internal lateinit var testCodeModernizerArtifact: CodeModernizerArtifact

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "type" : "bugfix",
 +  "description": "Improved recursion when validating projects for Q Code Transform."
 +}
Original file line number	Diff line number	Diff line change
`@@ -252,7 +252,7 @@ suspend fun JobId.pollTransformationStatusAndPlan(`
`252`	`252`	`return PollingResult(true, transformationResponse?.transformationJob(), state, transformationPlan)`
`253`	`253`	`}`
`254`	`254`
`255`		`-fun filterOnlyParentFiles(filePaths: List<VirtualFile>): List<VirtualFile> {`
	`255`	`+fun filterOnlyParentFiles(filePaths: Set<VirtualFile>): List<VirtualFile> {`
`256`	`256`	`if (filePaths.isEmpty()) return listOf()`
`257`	`257`	`// sorts it like:`
`258`	`258`	`// foo`