diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json index a9b3acaf13a..a0a3f51e87b 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json @@ -1,6 +1,6 @@ { "ruleKey": "S6813", "hasTruePositives": true, - "falseNegatives": 66, + "falseNegatives": 69, "falsePositives": 0 } \ No newline at end of file diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8909.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8909.json new file mode 100644 index 00000000000..f5fab362944 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8909.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8909", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} \ No newline at end of file diff --git a/java-checks-test-sources/default/src/main/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java new file mode 100644 index 00000000000..a060560e5be --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java @@ -0,0 +1,175 @@ +package checks.quarkus; + +import io.quarkus.cache.CacheKeyGenerator; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.Dependent; +import jakarta.enterprise.context.RequestScoped; +import jakarta.inject.Inject; +import java.lang.reflect.Method; + +class NoncompliantBasic implements CacheKeyGenerator { // Noncompliant {{Make this class a CDI bean by adding a scope annotation, or add a public no-args constructor.}} +// ^^^^^^^^^^^^^^^^^ + private final ConfigService configService; + + public NoncompliantBasic(ConfigService configService) { + this.configService = configService; + } + + @Override + public Object generate(Method method, Object... methodParams) { + return configService.getPrefix() + methodParams[0]; + } +} + +class NoncompliantMultipleDependencies implements CacheKeyGenerator { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + private final DatabaseService dbService; + private final CacheConfig config; + + public NoncompliantMultipleDependencies(DatabaseService dbService, CacheConfig config) { + this.dbService = dbService; + this.config = config; + } + + @Override + public Object generate(Method method, Object... methodParams) { + return dbService.format(methodParams[0]); + } +} + +class NoncompliantMultipleConstructors implements CacheKeyGenerator { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + private final String prefix; + + public NoncompliantMultipleConstructors(String prefix) { + this.prefix = prefix; + } + + public NoncompliantMultipleConstructors(String prefix, int timeout) { + this.prefix = prefix; + } + + @Override + public Object generate(Method method, Object... methodParams) { + return prefix + methodParams[0]; + } +} + +class NoncompliantPrivateConstructor implements CacheKeyGenerator { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + private NoncompliantPrivateConstructor() {} + + @Override + public Object generate(Method method, Object... methodParams) { + return methodParams[0]; + } +} + +@ApplicationScoped +class CompliantApplicationScoped implements CacheKeyGenerator { + @Inject + ConfigService configService; + + @Override + public Object generate(Method method, Object... methodParams) { + String prefix = configService.getPrefix(); + return prefix + methodParams[0]; + } +} + +@Dependent +class CompliantDependent implements CacheKeyGenerator { + @Inject + ConfigService configService; + + @Override + public Object generate(Method method, Object... methodParams) { + return configService.getPrefix() + methodParams[0]; + } +} + +@RequestScoped +class CompliantRequestScoped implements CacheKeyGenerator { + @Inject + ConfigService configService; + + @Override + public Object generate(Method method, Object... methodParams) { + return configService.getPrefix() + methodParams[0]; + } +} + +class NoncompliantPackagePrivateImplicitConstructor implements CacheKeyGenerator { // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + @Override + public Object generate(Method method, Object... methodParams) { + return methodParams[0]; + } +} + +class CompliantExplicitNoArgsConstructor implements CacheKeyGenerator { + public CompliantExplicitNoArgsConstructor() {} + + @Override + public Object generate(Method method, Object... methodParams) { + return methodParams[0]; + } +} + +class CompliantMultipleConstructorsWithNoArgs implements CacheKeyGenerator { + private final String prefix; + + public CompliantMultipleConstructorsWithNoArgs() { + this.prefix = "default"; + } + + public CompliantMultipleConstructorsWithNoArgs(String prefix) { + this.prefix = prefix; + } + + @Override + public Object generate(Method method, Object... methodParams) { + return prefix + methodParams[0]; + } +} + +@jakarta.enterprise.context.ApplicationScoped +class CompliantFullyQualifiedJakartaAnnotation implements CacheKeyGenerator { + @Override + public Object generate(Method method, Object... methodParams) { + return methodParams[0]; + } +} + +@javax.enterprise.context.ApplicationScoped +class CompliantFullyQualifiedJavaxAnnotation implements CacheKeyGenerator { + @Override + public Object generate(Method method, Object... methodParams) { + return methodParams[0]; + } +} + +abstract class CompliantAbstractClass implements CacheKeyGenerator { +} + +interface CompliantInterface extends CacheKeyGenerator { +} + +class NotACacheKeyGenerator { + public NotACacheKeyGenerator(String param) {} +} + +class ConfigService { + public String getPrefix() { + return "prefix"; + } +} + +class DatabaseService { + public String format(Object obj) { + return obj.toString(); + } +} + +class CacheConfig { +} diff --git a/java-checks-test-sources/default/src/main/java/io/quarkus/cache/CacheKeyGenerator.java b/java-checks-test-sources/default/src/main/java/io/quarkus/cache/CacheKeyGenerator.java new file mode 100644 index 00000000000..ff29f8fedfa --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/io/quarkus/cache/CacheKeyGenerator.java @@ -0,0 +1,10 @@ +package io.quarkus.cache; + +import java.lang.reflect.Method; + +/** + * Mock-up interface for Quarkus CacheKeyGenerator for testing purposes + */ +public interface CacheKeyGenerator { + Object generate(Method method, Object... methodParams); +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheck.java b/java-checks/src/main/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheck.java new file mode 100644 index 00000000000..a3a518039e0 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheck.java @@ -0,0 +1,111 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.quarkus; + +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8909") +public class CacheKeyGeneratorInstantiableCheck extends IssuableSubscriptionVisitor { + + private static final String CACHE_KEY_GENERATOR = "io.quarkus.cache.CacheKeyGenerator"; + + private static final List CDI_SCOPE_ANNOTATIONS = List.of( + "jakarta.enterprise.context.ApplicationScoped", + "jakarta.enterprise.context.Dependent", + "jakarta.enterprise.context.RequestScoped", + "jakarta.enterprise.context.SessionScoped", + "jakarta.enterprise.context.ConversationScoped", + "javax.enterprise.context.ApplicationScoped", + "javax.enterprise.context.Dependent", + "javax.enterprise.context.RequestScoped", + "javax.enterprise.context.SessionScoped", + "javax.enterprise.context.ConversationScoped" + ); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + if (!isApplicableClass(classTree) || hasCdiScopeAnnotation(classTree) || hasPublicNoArgsConstructor(classTree)) { + return; + } + reportIssue(Objects.requireNonNull(classTree.simpleName()), + "Make this class a CDI bean by adding a scope annotation, or add a public no-args constructor."); + } + + private static boolean isApplicableClass(ClassTree classTree) { + return !isAnonymous(classTree) + && !classTree.symbol().isAbstract() + && !classTree.symbol().isInterface() + && implementsCacheKeyGenerator(classTree); + } + + private static boolean isAnonymous(ClassTree classTree) { + return classTree.simpleName() == null; + } + + private static boolean implementsCacheKeyGenerator(ClassTree classTree) { + return classTree.symbol().type().isSubtypeOf(CACHE_KEY_GENERATOR); + } + + private static boolean hasCdiScopeAnnotation(ClassTree classTree) { + return CDI_SCOPE_ANNOTATIONS.stream().anyMatch(annotation -> + classTree.symbol().metadata().isAnnotatedWith(annotation) + || classTree.modifiers().annotations().stream().anyMatch(tree -> matchesAnnotation(tree, annotation))); + } + + private static boolean matchesAnnotation(AnnotationTree annotationTree, String fullyQualifiedName) { + String simpleName = fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf('.') + 1); + Tree annotationType = annotationTree.annotationType(); + return switch (annotationType.kind()) { + case IDENTIFIER -> ((IdentifierTree) annotationType).name().equals(simpleName); + case MEMBER_SELECT -> { + MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) annotationType; + yield memberSelect.identifier().name().equals(simpleName) || memberSelect.expression().symbolType().is(fullyQualifiedName); + } + default -> false; + }; + } + + private static boolean hasPublicNoArgsConstructor(ClassTree classTree) { + Collection constructors = classTree.symbol().lookupSymbols(""); + return constructors.stream() + .map(Symbol.MethodSymbol.class::cast) + .filter(CacheKeyGeneratorInstantiableCheck::isNoArgConstructor) + .findFirst() + .map(Symbol::isPublic) + .orElse(classTree.symbol().isPublic()); + } + + private static boolean isNoArgConstructor(Symbol.MethodSymbol constructor) { + return constructor.parameterTypes().isEmpty(); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckTest.java new file mode 100644 index 00000000000..a4f0dba2f57 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/quarkus/CacheKeyGeneratorInstantiableCheckTest.java @@ -0,0 +1,42 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.quarkus; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class CacheKeyGeneratorInstantiableCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java")) + .withCheck(new CacheKeyGeneratorInstantiableCheck()) + .verifyIssues(); + } + + @Test + void testWithoutSemantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/quarkus/CacheKeyGeneratorInstantiableCheckSample.java")) + .withCheck(new CacheKeyGeneratorInstantiableCheck()) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.html new file mode 100644 index 00000000000..789705e1d01 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.html @@ -0,0 +1,82 @@ +

This rule raises an issue when a class implements the cache key generator interface but does not provide a mechanism for the caching framework to +instantiate it, either through a default constructor or through registration with the dependency injection container.

+

In Java, this specifically refers to the CacheKeyGenerator interface and CDI (Contexts and Dependency Injection) bean annotations.

+

Why is this an issue?

+

When implementing a custom cache key generator interface in the framework, the caching subsystem must be able to create instances of your +implementation class. The framework supports two instantiation mechanisms:

+
    +
  • Managed component injection: If your class is registered as a managed component in the dependency injection container + (annotated with scope management annotations that mark it as application-scoped, request-scoped, dependent-scoped, etc.), the framework will use the + container to create and manage instances.
  • +
  • Public no-args constructor: If your class is not a managed component, the framework will create new instances using reflection + by calling a public no-args constructor.
  • +
+

When a cache key generator implementation lacks both of these, the framework cannot instantiate it. This prevents the cache from generating keys +properly.

+

The problem typically manifests when developers try to pass configuration or dependencies through constructor parameters without providing an +alternative instantiation path. While the intention may be to configure the key generator, this approach breaks the framework’s ability to create +instances.

+

What happens at runtime

+

When the cache attempts to use your key generator, the framework will try to instantiate it. Without a valid instantiation mechanism:

+
    +
  • The framework cannot create an instance of your generator
  • +
  • A runtime exception will be thrown when cache operations occur
  • +
  • Cache annotations that reference your generator will fail
  • +
+

This failure occurs during actual cache operations, not at application startup, making it harder to detect during development.

+

What is the potential impact?

+

When custom cache key generator implementations cannot be instantiated, the application will fail at runtime when cache operations attempt to use +the key generator. This breaks caching functionality for any methods that reference the problematic generator.

+

Users will experience:

+
    +
  • Failed cache lookups and invalidations
  • +
  • Broken cache-related functionality, potentially affecting application performance if caching is critical
  • +
  • Possible application crashes if exceptions are not properly handled
  • +
  • Degraded performance if the caching layer becomes unavailable
  • +
  • Errors that only appear during actual usage, making them harder to detect during development
  • +
+

Since the failure happens at runtime rather than build time, it may not be caught during development and could reach production environments.

+

How to fix it in Quarkus

+

Convert your CacheKeyGenerator implementation into a CDI bean by adding a scope annotation like @ApplicationScoped or +@Dependent. This allows you to inject dependencies normally while letting Quarkus manage the instance lifecycle.

+

Code examples

+

Noncompliant code example

+
+public class CustomKeyGen implements CacheKeyGenerator {
+    private final ConfigService configService;
+
+    public CustomKeyGen(ConfigService configService) { // Noncompliant
+        this.configService = configService;
+    }
+
+    @Override
+    public Object generate(Method method, Object... methodParams) {
+        String prefix = configService.getPrefix();
+        return new CompositeCacheKey(prefix, methodParams[0]);
+    }
+}
+
+

Compliant solution

+
+@ApplicationScoped
+public class CustomKeyGen implements CacheKeyGenerator {
+    @Inject
+    ConfigService configService;
+
+    @Override
+    public Object generate(Method method, Object... methodParams) {
+        String prefix = configService.getPrefix();
+        return new CompositeCacheKey(prefix, methodParams[0]);
+    }
+}
+
+

Resources

+ +

Related rules

+
    +
  • {rule:java:S2060} - Classes should not be coupled to their subclasses through constructors or static methods
  • +
+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.json new file mode 100644 index 00000000000..6293081c0ca --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8909.json @@ -0,0 +1,25 @@ +{ + "title": "\"CacheKeyGenerator\" implementations should be instantiable by the framework", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "quarkus", + "injection", + "pitfall" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-8909", + "sqKey": "S8909", + "scope": "Main", + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH" + }, + "attribute": "COMPLETE" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 7fb0e749028..8bd063b6d83 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -534,6 +534,7 @@ "S8715", "S8745", "S8786", + "S8909", "S8911", "S8924" ]