diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java index c3564122088..44245d1a390 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java @@ -20,11 +20,14 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Base64; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; import org.sonar.api.batch.fs.InputFile; import org.sonar.java.reporting.AnalyzerMessage; import org.sonar.java.utils.PackageUtils; @@ -64,8 +67,10 @@ public class BeanDefinitionGatherer extends SpringContextModelGatherer { private static final String BEAN_SEPARATOR = "\n"; private static final String FIELD_SEPARATOR = "|"; private static final String DEP_SEPARATOR = ","; + private static final String DEP_KEY_VALUE_SEPARATOR = ":"; private static final String PRIMARY_ANNOTATION = "org.springframework.context.annotation.Primary"; + private static final String VALUE_ATTRIBUTE = "value"; private final List collectedBeans = new ArrayList<>(); @@ -79,7 +84,7 @@ private record BeanData( InputFile inputFile, AnalyzerMessage.TextSpan textSpan, boolean isPrimary, - List dependingBeans) { + Map dependingBeans) { } @Override @@ -107,20 +112,16 @@ public void visitNode(Tree tree) { if (SpringUtils.STEREOTYPE_ANNOTATIONS.stream().anyMatch(meta::isAnnotatedWith)) { String beanName = extractBeanName(meta) .orElseGet(() -> defaultBeanName(classTree.simpleName().name())); - List deps = collectAutowiredDependencies(classTree); + Map deps = collectAutowiredDependencies(classTree); // Class-level bean (stereotype annotations) - collectedBeans.add(new BeanData( + var beanData = new BeanData( beanName, fqn, pkg, context.getInputFile(), AnalyzerMessage.textSpanFor(classTree.simpleName()), meta.isAnnotatedWith(PRIMARY_ANNOTATION), - deps)); - beansCollectedAtFileLevel.add(new BeanData( - beanName, fqn, pkg, - context.getInputFile(), - AnalyzerMessage.textSpanFor(classTree.simpleName()), - meta.isAnnotatedWith(PRIMARY_ANNOTATION), - deps)); + deps); + collectedBeans.add(beanData); + beansCollectedAtFileLevel.add(beanData); // @Bean methods — only if class is a configuration/component class for (MethodTree method : SpringUtils.getBeanMethods(classTree)) { @@ -155,7 +156,10 @@ private static void writeToCache(JavaFileScannerContext context, List } private static String serializeBean(BeanData bean) { - var deps = String.join(DEP_SEPARATOR, bean.dependingBeans()); + var deps = bean.dependingBeans().entrySet().stream() + .map(e -> Base64.getEncoder().encodeToString(e.getKey().getBytes(StandardCharsets.UTF_8)) + + DEP_KEY_VALUE_SEPARATOR + e.getValue()) + .collect(Collectors.joining(DEP_SEPARATOR)); var span = bean.textSpan(); var encodedName = Base64.getEncoder().encodeToString(bean.beanName().getBytes(StandardCharsets.UTF_8)); return String.join(FIELD_SEPARATOR, @@ -225,7 +229,14 @@ private static BeanData deserializeBean(String line, InputFile inputFile) { Integer.parseInt(spanParts[2]), Integer.parseInt(spanParts[3])); boolean isPrimary = Boolean.parseBoolean(fields[4]); - List deps = fields[5].isEmpty() ? List.of() : List.of(fields[5].split(DEP_SEPARATOR)); + Map deps = new LinkedHashMap<>(); + if (!fields[5].isEmpty()) { + for (String entry : fields[5].split(DEP_SEPARATOR)) { + int idx = entry.indexOf(DEP_KEY_VALUE_SEPARATOR); + String key = new String(Base64.getDecoder().decode(entry.substring(0, idx)), StandardCharsets.UTF_8); + deps.put(key, entry.substring(idx + 1)); + } + } return new BeanData(beanName, type, beanPackage, inputFile, textSpan, isPrimary, deps); } @@ -234,7 +245,7 @@ private static Optional extractBeanName(SymbolMetadata meta) { List attrs = meta.valuesForAnnotation(annotation); if (attrs != null) { Optional name = attrs.stream() - .filter(v -> "value".equals(v.name()) || "name".equals(v.name())) + .filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name())) .map(v -> (String) v.value()) .filter(s -> !s.isBlank()) .findFirst(); @@ -255,7 +266,7 @@ private void collectBeanMethod(MethodTree method, String pkg) { List attrs = beanMeta.valuesForAnnotation(SpringUtils.BEAN_ANNOTATION); String beanName = Optional.ofNullable(attrs) .flatMap(list -> list.stream() - .filter(v -> "value".equals(v.name()) || "name".equals(v.name())) + .filter(v -> VALUE_ATTRIBUTE.equals(v.name()) || "name".equals(v.name())) .map(v -> { Object val = v.value(); if (val instanceof Object[] arr && arr.length > 0) { @@ -271,42 +282,61 @@ private void collectBeanMethod(MethodTree method, String pkg) { ? method.returnType().symbolType().fullyQualifiedName() : ""; - List paramDeps = method.parameters().stream() - .map(p -> p.symbol().type().fullyQualifiedName()) - .toList(); + Map paramDeps = parameterDependencies(method); - collectedBeans.add(new BeanData( - beanName, returnTypeFqn, pkg, - context.getInputFile(), - AnalyzerMessage.textSpanFor(method.simpleName()), - beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION), - paramDeps)); - beansCollectedAtFileLevel.add(new BeanData( + var beanData = new BeanData( beanName, returnTypeFqn, pkg, context.getInputFile(), AnalyzerMessage.textSpanFor(method.simpleName()), beanMeta.isAnnotatedWith(PRIMARY_ANNOTATION), - paramDeps)); + paramDeps); + collectedBeans.add(beanData); + beansCollectedAtFileLevel.add(beanData); } - private static List collectAutowiredDependencies(ClassTree classTree) { - List deps = new ArrayList<>(); + private static Map collectAutowiredDependencies(ClassTree classTree) { + Map deps = new LinkedHashMap<>(); for (Tree member : classTree.members()) { - if (member.is(Tree.Kind.VARIABLE)) { - VariableTree field = (VariableTree) member; + if (member instanceof VariableTree field) { if (field.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { - deps.add(field.symbol().type().fullyQualifiedName()); + String typeFqn = field.symbol().type().fullyQualifiedName(); + deps.put(dependencyKey(field.simpleName().name(), extractQualifier(field.symbol().metadata())), typeFqn); } } else if (member.is(Tree.Kind.CONSTRUCTOR, Tree.Kind.METHOD)) { MethodTree method = (MethodTree) member; if (method.symbol().metadata().isAnnotatedWith(SpringUtils.AUTOWIRED_ANNOTATION)) { - method.parameters().stream() - .map(p -> p.symbol().type().fullyQualifiedName()) - .forEach(deps::add); + deps.putAll(parameterDependencies(method)); } } } return deps; } + private static Map parameterDependencies(MethodTree method) { + Map deps = new LinkedHashMap<>(); + for (var p : method.parameters()) { + String typeFqn = p.symbol().type().fullyQualifiedName(); + deps.put(dependencyKey(p.simpleName().name(), extractQualifier(p.symbol().metadata())), typeFqn); + } + return deps; + } + + private static String dependencyKey(String fieldOrParamName, @Nullable String qualifier) { + return qualifier != null ? qualifier : fieldOrParamName; + } + + @Nullable + private static String extractQualifier(SymbolMetadata metadata) { + List attrs = metadata.valuesForAnnotation(SpringUtils.QUALIFIER_ANNOTATION); + if (attrs == null) { + return null; + } + return attrs.stream() + .filter(v -> VALUE_ATTRIBUTE.equals(v.name())) + .map(v -> (String) v.value()) + .filter(s -> !s.isBlank()) + .findFirst() + .orElse(null); + } + } diff --git a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java index 3cca0218e72..a857a50ca9f 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java +++ b/java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionHolder.java @@ -16,8 +16,8 @@ */ package org.sonar.java.model.springcontext; -import java.util.ArrayList; -import java.util.List; +import java.util.LinkedHashMap; +import java.util.Map; import javax.annotation.Nullable; /** @@ -51,8 +51,12 @@ public class BeanDefinitionHolder { /** Source location where the bean definition appears. */ private final BeanLocation location; - /** Names of other beans this bean depends on. */ - private List dependingBeans; + /** + * Dependencies this bean requires. + * Key: the {@code @Qualifier} value if present, otherwise the field or parameter name at the injection point. + * Value: the fully-qualified type name. + */ + private Map dependingBeans; /** Comma-separated Spring profile expressions under which this bean is active, or {@code null} if unconditional. */ @Nullable @@ -68,8 +72,8 @@ private BeanDefinitionHolder(String type, String module, String beanPackage, Bea this.location = location; } - private void setDependingBeans(List beansList) { - this.dependingBeans = beansList; + private void setDependingBeans(Map beans) { + this.dependingBeans = beans; } private void setProfiles(@Nullable String profiles) { @@ -96,7 +100,7 @@ public BeanLocation getLocation() { return location; } - public List getDependingBeans() { + public Map getDependingBeans() { return dependingBeans; } @@ -114,7 +118,7 @@ public static class Builder { private final String module; private final String beanPackage; private final BeanLocation location; - private List dependingBeans = new ArrayList<>(); + private Map dependingBeans = new LinkedHashMap<>(); @Nullable private String profiles; private boolean isPrimary = false; @@ -126,8 +130,8 @@ public Builder(String type, String module, String beanPackage, BeanLocation loca this.location = location; } - public Builder dependingBeans(List beansList) { - this.dependingBeans = beansList; + public Builder dependingBeans(Map beans) { + this.dependingBeans = beans; return this; } @@ -143,7 +147,7 @@ public Builder primary() { public BeanDefinitionHolder build() { BeanDefinitionHolder holder = new BeanDefinitionHolder(type, module, beanPackage, location); - holder.setDependingBeans(List.copyOf(dependingBeans)); + holder.setDependingBeans(Map.copyOf(dependingBeans)); holder.setProfiles(profiles); if (isPrimary) { holder.setPrimary(); diff --git a/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java b/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java index f1dc9d323be..9673af815dd 100644 --- a/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java +++ b/java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java @@ -33,6 +33,7 @@ public final class SpringUtils { public static final String REPOSITORY_ANNOTATION = "org.springframework.stereotype.Repository"; public static final String SERVICE_ANNOTATION = "org.springframework.stereotype.Service"; public static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired"; + public static final String QUALIFIER_ANNOTATION = "org.springframework.beans.factory.annotation.Qualifier"; public static final String VALUE_ANNOTATION = "org.springframework.beans.factory.annotation.Value"; public static final String TRANSACTIONAL_ANNOTATION = "org.springframework.transaction.annotation.Transactional"; public static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean"; diff --git a/java-frontend/src/test/files/springcontext/BlankQualifierDependency.java b/java-frontend/src/test/files/springcontext/BlankQualifierDependency.java new file mode 100644 index 00000000000..7048b6d2d70 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/BlankQualifierDependency.java @@ -0,0 +1,14 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.stereotype.Component; + +@Component +class BlankQualifierDependency { + + @Autowired + @Qualifier("") + private ApplicationContext applicationContext; +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java new file mode 100644 index 00000000000..6e982ea0368 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedBeanMethodDependencies.java @@ -0,0 +1,18 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; + +@Configuration +class QualifiedBeanMethodDependencies { + + @Bean + Object myBean( + @Qualifier("primaryContext") ApplicationContext applicationContext, + Environment environment) { + return new Object(); + } +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java new file mode 100644 index 00000000000..b15ab0dd188 --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedConstructorDependencies.java @@ -0,0 +1,22 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class QualifiedConstructorDependencies { + + private final ApplicationContext applicationContext; + private final Environment environment; + + @Autowired + QualifiedConstructorDependencies( + @Qualifier("primaryContext") ApplicationContext applicationContext, + Environment environment) { + this.applicationContext = applicationContext; + this.environment = environment; + } +} diff --git a/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java b/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java new file mode 100644 index 00000000000..4a5973814fe --- /dev/null +++ b/java-frontend/src/test/files/springcontext/QualifiedFieldDependencies.java @@ -0,0 +1,18 @@ +package checks.spring.context; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +@Component +class QualifiedFieldDependencies { + + @Autowired + @Qualifier("primaryContext") + private ApplicationContext applicationContext; + + @Autowired + private Environment environment; +} diff --git a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java index 30a7bbad110..4224b00bb41 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/springcontext/BeanDefinitionGathererTest.java @@ -36,7 +36,6 @@ import org.sonar.plugins.java.api.caching.CacheContext; import org.sonar.plugins.java.api.caching.JavaReadCache; import org.sonar.plugins.java.api.caching.JavaWriteCache; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.ArgumentMatchers.any; @@ -191,7 +190,7 @@ void dependencies_collected_as_depending_beans(String filePath, String expectedB var beans = model.getBeanDefinitionRegistry().getByName(expectedBeanName); assertThat(beans).hasSize(1); - assertThat(beans.get(0).getDependingBeans()) + assertThat(beans.get(0).getDependingBeans().values()) .containsExactlyInAnyOrder( "org.springframework.context.ApplicationContext", "org.springframework.core.env.Environment" @@ -206,6 +205,37 @@ static Stream dependencyCollectionArguments() { ); } + // ---- @Qualifier handling -------------------------------------------------- + + @ParameterizedTest(name = "{0}") + @MethodSource("qualifiedDependencyArguments") + void qualifier_is_captured_on_qualified_dependency(String filePath, String expectedBeanName) { + scan(filePath); + + var beans = model.getBeanDefinitionRegistry().getByName(expectedBeanName); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans().keySet()) + .containsExactlyInAnyOrder("primaryContext", "environment"); + } + + static Stream qualifiedDependencyArguments() { + return Stream.of( + Arguments.of("src/test/files/springcontext/QualifiedFieldDependencies.java", "qualifiedFieldDependencies"), + Arguments.of("src/test/files/springcontext/QualifiedConstructorDependencies.java", "qualifiedConstructorDependencies"), + Arguments.of("src/test/files/springcontext/QualifiedBeanMethodDependencies.java", "myBean") + ); + } + + @Test + void unqualified_dependency_uses_default_spring_bean_name_as_key() { + scan("src/test/files/springcontext/AutowiredDependencies.java"); + + var beans = model.getBeanDefinitionRegistry().getByName("autowiredDependencies"); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans().keySet()) + .containsExactlyInAnyOrder("applicationContext", "environment"); + } + // ---- Bean location -------------------------------------------------------- @Test @@ -347,6 +377,67 @@ void scanWithoutParsing_returns_false_on_corrupted_cache_entry() { assertThat(gatherer.scanWithoutParsing(context)).isFalse(); } + @Test + void leaveFile_writes_dependencies_with_qualifiers_to_cache() { + WriteCache writeCache = mock(WriteCache.class); + SensorContextTester ctx = SensorContextTester.create(new File("")); + ctx.setCacheEnabled(true); + ctx.setNextCache(writeCache); + + scan(ctx, "src/test/files/springcontext/QualifiedFieldDependencies.java"); + + var dataCaptor = ArgumentCaptor.forClass(byte[].class); + verify(writeCache).write(anyString(), dataCaptor.capture()); + String serialized = new String(dataCaptor.getValue(), StandardCharsets.UTF_8); + + String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); + String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); + assertThat(serialized) + .contains(encodedPrimaryContext + ":org.springframework.context.ApplicationContext") + .contains(encodedEnvironment + ":org.springframework.core.env.Environment"); + } + + @Test + void scanWithoutParsing_restores_dependencies_with_and_without_qualifier_from_cache() { + InputFile inputFile = TestUtils.inputFile(new File("src/test/files/springcontext/QualifiedFieldDependencies.java")); + String cacheKey = "java:spring:bean-definitions:" + inputFile.key(); + String encodedName = Base64.getEncoder().encodeToString("qualifiedFieldDependencies".getBytes(StandardCharsets.UTF_8)); + String encodedPrimaryContext = Base64.getEncoder().encodeToString("primaryContext".getBytes(StandardCharsets.UTF_8)); + String encodedEnvironment = Base64.getEncoder().encodeToString("environment".getBytes(StandardCharsets.UTF_8)); + String serialized = encodedName + "|checks.spring.context.QualifiedFieldDependencies|checks.spring.context|10:6:10:30|false|" + + encodedPrimaryContext + ":org.springframework.context.ApplicationContext" + + "," + encodedEnvironment + ":org.springframework.core.env.Environment"; + + JavaReadCache readCache = mock(JavaReadCache.class); + when(readCache.readBytes(cacheKey)).thenReturn(serialized.getBytes(StandardCharsets.UTF_8)); + CacheContext cacheContext = mockCacheContext(readCache, mock(JavaWriteCache.class)); + + InputFileScannerContext context = mock(InputFileScannerContext.class); + when(context.getInputFile()).thenReturn(inputFile); + when(context.getCacheContext()).thenReturn(cacheContext); + + assertThat(gatherer.scanWithoutParsing(context)).isTrue(); + + ModuleScannerContext moduleScannerContext = mock(ModuleScannerContext.class); + when(moduleScannerContext.getModuleKey()).thenReturn(""); + gatherer.gatherSpringContextData(moduleScannerContext, model); + + var beans = model.getBeanDefinitionRegistry().getByName("qualifiedFieldDependencies"); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans().keySet()) + .containsExactlyInAnyOrder("primaryContext", "environment"); + } + + @Test + void blank_qualifier_value_is_treated_as_no_qualifier() { + scan("src/test/files/springcontext/BlankQualifierDependency.java"); + + var beans = model.getBeanDefinitionRegistry().getByName("blankQualifierDependency"); + assertThat(beans).hasSize(1); + assertThat(beans.get(0).getDependingBeans()) + .containsOnlyKeys("applicationContext"); + } + private static CacheContext mockCacheContext(JavaReadCache readCache, JavaWriteCache writeCache) { CacheContext cacheContext = mock(CacheContext.class); when(cacheContext.isCacheEnabled()).thenReturn(true);