Improve dependent beans calculation (#2310)

EgorkaKulikov · web-flow · commit c17b0a7d5d7b · 2023-06-26T15:13:41.000Z
diff --git a/utbot-core/src/main/kotlin/org/utbot/common/ClassLoaderUtil.kt b/utbot-core/src/main/kotlin/org/utbot/common/ClassLoaderUtil.kt
@@ -0,0 +1,14 @@
+package org.utbot.common
+
+import java.net.URLClassLoader
+
+/**
+ * Checks that the class given by its binary name is on classpath of this classloader.
+ *
+ * Note: if the specified class is on classpath, `true` is returned even when
+ * superclass (or implemented interfaces) aren't on the classpath.
+ */
+fun URLClassLoader.hasOnClasspath(classBinaryName: String): Boolean {
+    val classFqn = classBinaryName.replace('.', '/').plus(".class")
+    return this.findResource(classFqn) != null
+}
diff --git a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/CoverageApi.kt b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/CoverageApi.kt
@@ -3,19 +3,21 @@ package org.utbot.framework.plugin.api
 /**
  * Represents a covered bytecode instruction.
  *
- * @param className the fqn in internal form, i.e. com/rest/order/services/OrderService$InnerClass.
+ * @param internalName the fqn in internal form, i.e. com/rest/order/services/OrderService$InnerClass.
  * @param methodSignature the signature of the method.
  * @param lineNumber a number of the line in the source file.
  * @param id a unique identifier among all instructions in all classes.
  *
  * @see <a href="CONFLUENCE:Test+Minimization">Test minimization</a>
  */
 data class Instruction(
-    val className: String,
+    val internalName: String,
     val methodSignature: String,
     val lineNumber: Int,
     val id: Long
-)
+) {
+    val className: String get() = internalName.replace('/', '.')
+}
 
 /**
  * Represents coverage information. Some other
diff --git a/utbot-framework-test/src/test/kotlin/org/utbot/sarif/SarifReportTest.kt b/utbot-framework-test/src/test/kotlin/org/utbot/sarif/SarifReportTest.kt
@@ -383,6 +383,7 @@ class SarifReportTest {
 
     private fun mockCoverage(mockExecution: UtExecution, lineNumber: Int, className: String) {
         Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.lineNumber).thenReturn(1)
+        Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.internalName).thenReturn("Main")
         Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.className).thenReturn("Main")
         (mockExecution as? UtSymbolicExecution)?.let { mockSymbolicSteps(it, lineNumber, className) }
     }
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/plugin/api/TestCaseGenerator.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/plugin/api/TestCaseGenerator.kt
@@ -10,12 +10,8 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.yield
 import mu.KLogger
 import mu.KotlinLogging
-import org.utbot.common.measureTime
-import org.utbot.common.runBlockingWithCancellationPredicate
-import org.utbot.common.runIgnoringCancellationException
-import org.utbot.common.trace
+import org.utbot.common.*
 import org.utbot.common.PathUtil.toURL
-import org.utbot.common.tryLoadClass
 import org.utbot.engine.EngineController
 import org.utbot.engine.Mocker
 import org.utbot.engine.UtBotSymbolicEngine
@@ -81,6 +77,7 @@ open class TestCaseGenerator(
                                 UtExecutionInstrumentation,
                                 approach.config,
                                 applicationContext.beanDefinitions,
+                                buildDirs.map { it.toURL() }.toTypedArray(),
                                 )
                     }
                 is TypeReplacementApproach.DoNotReplace -> UtExecutionInstrumentation
@@ -385,23 +382,18 @@ open class TestCaseGenerator(
             val filteredCoveredInstructions =
                 coverage.coveredInstructions
                     .filter { instruction ->
-                        val instrClassFqn =
-                            instruction.className
-                                .also {
-                                    val isInstrClassOnClassPath =
-                                        isClassOnUserClasspathCache.getOrPut(it) {
-                                            buildDirsClassLoader.findResource(it.plus(".class")) != null
-                                        }
-                                    if (!isInstrClassOnClassPath) return@filter false
-                                }
-                                .replace('/', '.')
+                        val instrClassName = instruction.className
 
-                        // We do not want to filter out instructions that are in class under test
-                        if (instrClassFqn == classUnderTestId.name) return@filter true
+                        val isInstrClassOnClassPath =
+                            isClassOnUserClasspathCache.getOrPut(instrClassName) {
+                                buildDirsClassLoader.hasOnClasspath(instrClassName)
+                            }
 
-                        // We do not want to take instructions in classes
-                        // marked with annotations from [annotationsToIgnore]
-                        return@filter !hasAnnotations(instrClassFqn, annotationsToIgnoreCoverage)
+                        // We want to
+                        // - always keep instructions that are in class under test
+                        // - ignore instructions in classes marked with annotations from [annotationsToIgnore]
+                        return@filter instrClassName == classUnderTestId.name ||
+                                (isInstrClassOnClassPath && !hasAnnotations(instrClassName, annotationsToIgnoreCoverage))
                     }
                     .ifEmpty {
                         coverage.coveredInstructions
diff --git a/utbot-framework/src/main/kotlin/org/utbot/sarif/SarifReport.kt b/utbot-framework/src/main/kotlin/org/utbot/sarif/SarifReport.kt
@@ -360,14 +360,14 @@ class SarifReport(
             ?: return listOf()
 
         val executionTrace = coveredInstructions.groupBy { instruction ->
-            instruction.className to instruction.methodSignature // group by method
+            instruction.internalName to instruction.methodSignature // group by method
         }.map { (_, instructionsForOneMethod) ->
             instructionsForOneMethod.last() // we need only last to construct the stack trace
         }
 
         val sarifExecutionTrace = executionTrace.map { instruction ->
             resolveStackTraceElementByNames(
-                classFqn = instruction.className.replace('/', '.'),
+                classFqn = instruction.className,
                 methodName = instruction.methodSignature.substringBefore('('),
                 lineNumber = instruction.lineNumber
             )
@@ -508,7 +508,7 @@ class SarifReport(
         if (lastCoveredInstruction != null) {
             return Pair(
                 lastCoveredInstruction.lineNumber, // .lineNumber is one-based
-                lastCoveredInstruction.className.replace('/', '.')
+                lastCoveredInstruction.className
             )
         }
 
diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/execution/SpringUtExecutionInstrumentation.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/execution/SpringUtExecutionInstrumentation.kt
@@ -13,8 +13,9 @@ import org.utbot.instrumentation.instrumentation.execution.mock.SpringInstrument
 import org.utbot.instrumentation.process.HandlerClassesLoader
 import org.utbot.spring.api.context.ContextWrapper
 import org.utbot.spring.api.repositoryWrapper.RepositoryInteraction
+import java.net.URL
+import java.net.URLClassLoader
 import java.security.ProtectionDomain
-import java.util.IdentityHashMap
 
 /**
  * UtExecutionInstrumentation wrapper that is aware of Spring config and initialises Spring context
@@ -23,8 +24,11 @@ class SpringUtExecutionInstrumentation(
     private val delegateInstrumentation: UtExecutionInstrumentation,
     private val springConfig: String,
     private val beanDefinitions: List<BeanDefinitionData>,
+    private val buildDirs: Array<URL>,
 ) : Instrumentation<UtConcreteExecutionResult> by delegateInstrumentation {
+
     private lateinit var instrumentationContext: SpringInstrumentationContext
+    private lateinit var userSourcesClassLoader: URLClassLoader
 
     private val relatedBeansCache = mutableMapOf<Class<*>, Set<String>>()
 
@@ -47,6 +51,7 @@ class SpringUtExecutionInstrumentation(
         )
 
         instrumentationContext = SpringInstrumentationContext(springConfig)
+        userSourcesClassLoader = URLClassLoader(buildDirs, null)
         delegateInstrumentation.instrumentationContext = instrumentationContext
         delegateInstrumentation.init(pathsToUserClasses)
     }
@@ -60,7 +65,7 @@ class SpringUtExecutionInstrumentation(
         RepositoryInteraction.recordedInteractions.clear()
 
         val beanNamesToReset: Set<String> = getRelevantBeanNames(clazz)
-        val repositoryDefinitions = springContext.resolveRepositories(beanNamesToReset)
+        val repositoryDefinitions = springContext.resolveRepositories(beanNamesToReset, userSourcesClassLoader)
 
         beanNamesToReset.forEach { beanName -> springContext.resetBean(beanName) }
         val jdbcTemplate = getBean("jdbcTemplate")
@@ -83,7 +88,7 @@ class SpringUtExecutionInstrumentation(
     private fun getRelevantBeanNames(clazz: Class<*>): Set<String> = relatedBeansCache.getOrPut(clazz) {
         beanDefinitions
             .filter { it.beanTypeFqn == clazz.name }
-            .flatMap { springContext.getDependenciesForBean(it.beanName) }
+            .flatMap { springContext.getDependenciesForBean(it.beanName, userSourcesClassLoader) }
             .toSet()
             .also { logger.info { "Detected relevant beans for class ${clazz.name}: $it" } }
     }
@@ -92,7 +97,7 @@ class SpringUtExecutionInstrumentation(
 
     fun getRepositoryDescriptions(classId: ClassId): Set<SpringRepositoryId> {
         val relevantBeanNames = getRelevantBeanNames(classId.jClass)
-        val repositoryDescriptions = springContext.resolveRepositories(relevantBeanNames.toSet())
+        val repositoryDescriptions = springContext.resolveRepositories(relevantBeanNames.toSet(), userSourcesClassLoader)
         return repositoryDescriptions.map { repositoryDescription ->
             SpringRepositoryId(
                 repositoryDescription.beanName,
diff --git a/utbot-junit-contest/src/main/kotlin/org/utbot/contest/Statistics.kt b/utbot-junit-contest/src/main/kotlin/org/utbot/contest/Statistics.kt
@@ -237,7 +237,7 @@ data class CoverageStatistic(val covered: Int, val total: Int)
 private fun CoverageInstructionsSet?.getCoverageInfo(classNames: Set<String>): CoverageStatistic = this?.run {
     CoverageStatistic(
         coveredInstructions.filter {
-            instr -> classNames.contains(instr.className)
+            instr -> classNames.contains(instr.internalName)
         }.map { it.id }.distinct().size,
         totalInstructions.toInt()
     )
diff --git a/utbot-spring-commons-api/src/main/kotlin/org/utbot/spring/api/context/ContextWrapper.kt b/utbot-spring-commons-api/src/main/kotlin/org/utbot/spring/api/context/ContextWrapper.kt
@@ -1,15 +1,18 @@
 package org.utbot.spring.api.context
 
+import java.net.URLClassLoader
+
+//TODO: `userSourcesClassLoader` must not be passed as a method argument, requires refactoring
 interface ContextWrapper {
     val context: Any
 
     fun getBean(beanName: String): Any
 
-    fun getDependenciesForBean(beanName: String): Set<String>
+    fun getDependenciesForBean(beanName: String, userSourcesClassLoader: URLClassLoader): Set<String>
 
     fun resetBean(beanName: String): Any
 
-    fun resolveRepositories(beanNames: Set<String>): Set<RepositoryDescription>
+    fun resolveRepositories(beanNames: Set<String>, userSourcesClassLoader: URLClassLoader): Set<RepositoryDescription>
 }
 
 data class RepositoryDescription(
diff --git a/utbot-spring-commons-api/src/main/kotlin/org/utbot/spring/api/instantiator/InstantiationSettings.kt b/utbot-spring-commons-api/src/main/kotlin/org/utbot/spring/api/instantiator/InstantiationSettings.kt
@@ -3,4 +3,4 @@ package org.utbot.spring.api.instantiator
 class InstantiationSettings(
     val configurationClasses: Array<Class<*>>,
     val profileExpression: String?,
-)
+)
diff --git a/utbot-spring-commons/build.gradle.kts b/utbot-spring-commons/build.gradle.kts
@@ -15,6 +15,7 @@ java {
 
 dependencies {
     implementation(project(":utbot-spring-commons-api"))
+    implementation(project(":utbot-core"))
 
     // https://mvnrepository.com/artifact/org.springframework.boot/spring-boot
     compileOnly("org.springframework.boot:spring-boot:$springBootVersion")
diff --git a/utbot-spring-commons/src/main/kotlin/org/utbot/spring/context/SpringContextWrapper.kt b/utbot-spring-commons/src/main/kotlin/org/utbot/spring/context/SpringContextWrapper.kt
@@ -5,15 +5,15 @@ import com.jetbrains.rd.util.warn
 import org.springframework.beans.factory.support.BeanDefinitionRegistry
 import org.springframework.context.ConfigurableApplicationContext
 import org.springframework.data.repository.CrudRepository
+import org.utbot.common.hasOnClasspath
 import org.utbot.spring.api.context.ContextWrapper
 import org.utbot.spring.api.context.RepositoryDescription
+import java.net.URLClassLoader
 
 private val logger = getLogger<SpringContextWrapper>()
 
 class SpringContextWrapper(override val context: ConfigurableApplicationContext) : ContextWrapper {
 
-    private val springClassPrefix = "org.springframework"
-
     private val isCrudRepositoryOnClasspath = try {
         CrudRepository::class.java.name
         true
@@ -23,14 +23,15 @@ class SpringContextWrapper(override val context: ConfigurableApplicationContext)
 
     override fun getBean(beanName: String): Any = context.getBean(beanName)
 
-    override fun getDependenciesForBean(beanName: String): Set<String> {
+    override fun getDependenciesForBean(beanName: String, userSourcesClassLoader: URLClassLoader): Set<String> {
         val analyzedBeanNames = mutableSetOf<String>()
-        return getDependenciesForBeanInternal(beanName, analyzedBeanNames)
+        return getDependenciesForBeanInternal(beanName, analyzedBeanNames, userSourcesClassLoader)
     }
 
     private fun getDependenciesForBeanInternal(
         beanName: String,
         analyzedBeanNames: MutableSet<String>,
+        userSourcesClassLoader: URLClassLoader,
         ): Set<String> {
         if (beanName in analyzedBeanNames) {
             return emptySet()
@@ -40,10 +41,17 @@ class SpringContextWrapper(override val context: ConfigurableApplicationContext)
 
         val dependencyBeanNames = context.beanFactory
             .getDependenciesForBean(beanName)
-            .filter { it in context.beanDefinitionNames } // filters out inner beans
+            // this filtering is applied to avoid inner beans
+            .filter { it in context.beanDefinitionNames }
+            .filter { name ->
+                val clazz = getBean(name)::class.java
+                // here immediate hierarchy is enough because proxies are inherited directly
+                val immediateClazzHierarchy = clazz.interfaces + clazz.superclass + clazz
+                immediateClazzHierarchy.any { clazz -> userSourcesClassLoader.hasOnClasspath(clazz.name) }
+            }
             .toSet()
 
-        return setOf(beanName) + dependencyBeanNames.flatMap { getDependenciesForBean(it) }
+        return setOf(beanName) + dependencyBeanNames.flatMap { getDependenciesForBean(it, userSourcesClassLoader) }
     }
 
     override fun resetBean(beanName: String) {
@@ -54,7 +62,7 @@ class SpringContextWrapper(override val context: ConfigurableApplicationContext)
         beanDefinitionRegistry.registerBeanDefinition(beanName, beanDefinition)
     }
 
-    override fun resolveRepositories(beanNames: Set<String>): Set<RepositoryDescription> {
+    override fun resolveRepositories(beanNames: Set<String>, userSourcesClassLoader: URLClassLoader): Set<RepositoryDescription> {
         if (!isCrudRepositoryOnClasspath) return emptySet()
         val repositoryBeans = beanNames
             .map { beanName -> SimpleBeanDefinition(beanName, getBean(beanName)) }
@@ -67,7 +75,7 @@ class SpringContextWrapper(override val context: ConfigurableApplicationContext)
             val repositoryClass = repositoryBean.bean::class.java
             val repositoryClassName = repositoryClass
                 .interfaces
-                .filterNot { it.name.startsWith(springClassPrefix) }
+                .filter { clazz -> userSourcesClassLoader.hasOnClasspath(clazz.name) }
                 .filter { CrudRepository::class.java.isAssignableFrom(it) }
                 .map { it.name }
                 .firstOrNull() ?: CrudRepository::class.java.name

Original file line number	Diff line number	Diff line change
`@@ -383,6 +383,7 @@ class SarifReportTest {`
`383`	`383`
`384`	`384`	`private fun mockCoverage(mockExecution: UtExecution, lineNumber: Int, className: String) {`
`385`	`385`	Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.lineNumber).thenReturn(1)
	`386`	+ Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.internalName).thenReturn("Main")
`386`	`387`	Mockito.`when`(mockExecution.coverage?.coveredInstructions?.lastOrNull()?.className).thenReturn("Main")
`387`	`388`	`(mockExecution as? UtSymbolicExecution)?.let { mockSymbolicSteps(it, lineNumber, className) }`
`388`	`389`	`}`
Original file line number	Diff line number	Diff line change
`@@ -360,14 +360,14 @@ class SarifReport(`
`360`	`360`	`?: return listOf()`
`361`	`361`
`362`	`362`	`val executionTrace = coveredInstructions.groupBy { instruction ->`
`363`		`- instruction.className to instruction.methodSignature // group by method`
	`363`	`+ instruction.internalName to instruction.methodSignature // group by method`
`364`	`364`	`}.map { (_, instructionsForOneMethod) ->`
`365`	`365`	`instructionsForOneMethod.last() // we need only last to construct the stack trace`
`366`	`366`	`}`
`367`	`367`
`368`	`368`	`val sarifExecutionTrace = executionTrace.map { instruction ->`
`369`	`369`	`resolveStackTraceElementByNames(`
`370`		`- classFqn = instruction.className.replace('/', '.'),`
	`370`	`+ classFqn = instruction.className,`
`371`	`371`	`methodName = instruction.methodSignature.substringBefore('('),`
`372`	`372`	`lineNumber = instruction.lineNumber`
`373`	`373`	`)`
`@@ -508,7 +508,7 @@ class SarifReport(`
`508`	`508`	`if (lastCoveredInstruction != null) {`
`509`	`509`	`return Pair(`
`510`	`510`	`lastCoveredInstruction.lineNumber, // .lineNumber is one-based`
`511`		`- lastCoveredInstruction.className.replace('/', '.')`
	`511`	`+ lastCoveredInstruction.className`
`512`	`512`	`)`
`513`	`513`	`}`
`514`	`514`
Original file line number	Diff line number	Diff line change
`@@ -237,7 +237,7 @@ data class CoverageStatistic(val covered: Int, val total: Int)`
`237`	`237`	`private fun CoverageInstructionsSet?.getCoverageInfo(classNames: Set<String>): CoverageStatistic = this?.run {`
`238`	`238`	`CoverageStatistic(`
`239`	`239`	`coveredInstructions.filter {`
`240`		`- instr -> classNames.contains(instr.className)`
	`240`	`+ instr -> classNames.contains(instr.internalName)`
`241`	`241`	`}.map { it.id }.distinct().size,`
`242`	`242`	`totalInstructions.toInt()`
`243`	`243`	`)`