Skip to content

Commit e9f2407

Browse files
NassimAMGHARNassimAMGHAR
committed
Implement N+1 problem detection for JPA entities with enhanced reporting
1 parent 4fc4197 commit e9f2407

File tree

5 files changed

+68
-66
lines changed

5 files changed

+68
-66
lines changed

‎src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue {
3030
public List<Author> smellGetAllAuthors() {
3131
List<Author> authors = authorRepository.findAll();
3232
for (Author author : authors) {
33-
List<Book> books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur
33+
List<Book> books = author.getBooks(); // // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }}
3434
}
3535
return authors;
3636
}

‎src/main/java/org/greencodeinitiative/creedengo/java/JavaCheckRegistrar.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public class JavaCheckRegistrar implements CheckRegistrar {
5050
FreeResourcesOfAutoCloseableInterface.class,
5151
AvoidMultipleIfElseStatement.class,
5252
UseOptionalOrElseGetVsOrElse.class,
53-
MakeNonReassignedVariablesConstants.class
53+
MakeNonReassignedVariablesConstants.class,
54+
AvoidNPlusOneProblemInJPAEntitiesCheck.class
5455
);
5556

5657
/**

‎src/main/java/org/greencodeinitiative/creedengo/java/checks/AvoidNPlusOneProblemInJPAEntitiesCheck.java‎

Lines changed: 62 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3,103 +3,103 @@
33
import org.sonar.check.Rule;
44
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
55
import org.sonar.plugins.java.api.semantic.MethodMatchers;
6+
import org.sonar.plugins.java.api.semantic.Symbol;
7+
import org.sonar.plugins.java.api.semantic.Symbol.VariableSymbol;
68
import org.sonar.plugins.java.api.tree.*;
79
import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
810

9-
import java.util.Arrays;
10-
import java.util.List;
11+
import java.util.*;
1112

1213
@Rule(key = "GRC3")
1314
@DeprecatedRuleKey(repositoryKey = "ecocode-java", ruleKey = "GRC3")
1415
@DeprecatedRuleKey(repositoryKey = "greencodeinitiative-java", ruleKey = "GRC3")
1516
public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor {
16-
protected static final String RULE_MESSAGE = "Avoid N+1 with nested JPA Entities";
1717

18-
private static final String BASE_STREAM = "java.util.stream.BaseStream";
18+
protected static final String RULE_MESSAGE = " Evitez le N+1 : utilisez un fetch join ou une récupération eager. ";
19+
1920
private static final String SPRING_REPOSITORY = "org.springframework.data.repository.Repository";
2021

21-
private static final MethodMatchers SPRING_REPOSITORY_METHOD =
22-
MethodMatchers
23-
.create()
22+
private static final MethodMatchers SPRING_REPOSITORY_METHOD_FIND_ALL =
23+
MethodMatchers.create()
2424
.ofSubTypes(SPRING_REPOSITORY)
25-
.anyName()
26-
.withAnyParameters()
27-
.build();
28-
29-
private static final MethodMatchers STREAM_FOREACH_METHOD =
30-
MethodMatchers
31-
.create()
32-
.ofSubTypes(BASE_STREAM)
33-
.names("forEach", "forEachOrdered", "map", "peek")
25+
.names("findAll")
3426
.withAnyParameters()
3527
.build();
3628

37-
private final AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor visitorInFile = new AvoidNPlusOneProblemInJPAEntitiesCheck.AvoidSpringRepositoryCallInLoopCheckVisitor();
38-
private final AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor streamVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.StreamVisitor();
39-
40-
private final AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor ancestorMethodVisitor = new AvoidNPlusOneProblemInJPAEntitiesCheck.AncestorMethodVisitor();
29+
private final Map<Symbol, Tree> repositoryFindAllVars = new HashMap<>();
4130

4231
@Override
4332
public List<Tree.Kind> nodesToVisit() {
44-
return Arrays.asList(
45-
Tree.Kind.FOR_EACH_STATEMENT, // loop
46-
Tree.Kind.FOR_STATEMENT, // loop
47-
Tree.Kind.WHILE_STATEMENT, // loop
48-
Tree.Kind.DO_STATEMENT, // loop
49-
Tree.Kind.METHOD_INVOCATION // stream
50-
);
33+
return Arrays.asList(Tree.Kind.VARIABLE, Tree.Kind.METHOD_INVOCATION, Tree.Kind.FOR_EACH_STATEMENT);
5134
}
5235

5336
@Override
5437
public void visitNode(Tree tree) {
55-
if (tree.is(Tree.Kind.METHOD_INVOCATION)) { // stream process
56-
MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree;
57-
if (STREAM_FOREACH_METHOD.matches(methodInvocationTree)) {
58-
tree.accept(streamVisitor);
38+
if (tree.is(Tree.Kind.VARIABLE)) {
39+
VariableTree variableTree = (VariableTree) tree;
40+
ExpressionTree initializer = variableTree.initializer();
41+
if (initializer != null && initializer.is(Tree.Kind.METHOD_INVOCATION)) {
42+
MethodInvocationTree methodInvocation = (MethodInvocationTree) initializer;
43+
if (SPRING_REPOSITORY_METHOD_FIND_ALL.matches(methodInvocation)) {
44+
VariableSymbol symbol = (VariableSymbol) variableTree.symbol();
45+
repositoryFindAllVars.put(symbol, tree);
46+
}
5947
}
60-
} else { // loop process
61-
tree.accept(visitorInFile);
6248
}
63-
}
6449

65-
private class AvoidSpringRepositoryCallInLoopCheckVisitor extends BaseTreeVisitor {
66-
@Override
67-
public void visitMethodInvocation(MethodInvocationTree tree) {
68-
if (SPRING_REPOSITORY_METHOD.matches(tree)) {
69-
reportIssue(tree, RULE_MESSAGE);
70-
} else {
71-
super.visitMethodInvocation(tree);
50+
// Cas d'un foreach sur une variable issue de findAll()
51+
if (tree.is(Tree.Kind.FOR_EACH_STATEMENT)) {
52+
ForEachStatement forEach = (ForEachStatement) tree;
53+
ExpressionTree iterable = forEach.expression();
54+
if (iterable.is(Tree.Kind.IDENTIFIER)) {
55+
Symbol symbol = ((IdentifierTree) iterable).symbol();
56+
if (repositoryFindAllVars.containsKey(symbol)) {
57+
// On marque la variable d'itération comme issue d'un findAll()
58+
VariableSymbol loopVar = (VariableSymbol) forEach.variable().symbol();
59+
repositoryFindAllVars.put(loopVar, tree);
60+
}
7261
}
7362
}
7463

75-
}
76-
77-
private class StreamVisitor extends BaseTreeVisitor {
64+
// Détection d'un appel de getter sur une variable issue de findAll()
65+
if (tree.is(Tree.Kind.METHOD_INVOCATION)) {
66+
MethodInvocationTree methodInvocation = (MethodInvocationTree) tree;
7867

79-
@Override
80-
public void visitLambdaExpression(LambdaExpressionTree tree) {
81-
tree.accept(ancestorMethodVisitor);
82-
}
68+
// Check if the call is something like post.getAuthor() or post.getAuthor().getName()
69+
ExpressionTree select = methodInvocation.methodSelect();
70+
if (select.is(Tree.Kind.MEMBER_SELECT)) {
71+
MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) select;
72+
ExpressionTree root = memberSelect.expression();
8373

84-
}
74+
if (root.is(Tree.Kind.IDENTIFIER)) {
75+
Symbol symbol = ((IdentifierTree) root).symbol();
76+
if (repositoryFindAllVars.containsKey(symbol) && isGetter(memberSelect.identifier().name())) {
77+
reportIssue(methodInvocation, RULE_MESSAGE);
78+
}
79+
}
8580

86-
private class AncestorMethodVisitor extends BaseTreeVisitor {
87-
88-
@Override
89-
public void visitMethodInvocation(MethodInvocationTree tree) {
90-
// if the method is a spring repository method, report an issue
91-
if (SPRING_REPOSITORY_METHOD.matches(tree)) {
92-
reportIssue(tree, RULE_MESSAGE);
93-
} else { // else, check if the method is a method invocation and check recursively
94-
if (tree.methodSelect().is(Tree.Kind.MEMBER_SELECT)) {
95-
MemberSelectExpressionTree memberSelectTree = (MemberSelectExpressionTree) tree.methodSelect();
96-
if ( memberSelectTree.expression().is(Tree.Kind.METHOD_INVOCATION)) {
97-
MethodInvocationTree methodInvocationTree = (MethodInvocationTree) memberSelectTree.expression();
98-
methodInvocationTree.accept(ancestorMethodVisitor);
81+
// Handle nested getter chains (e.g., post.getAuthor().getName())
82+
if (root.is(Tree.Kind.METHOD_INVOCATION)) {
83+
MethodInvocationTree rootInvocation = (MethodInvocationTree) root;
84+
ExpressionTree deeperSelect = rootInvocation.methodSelect();
85+
if (deeperSelect.is(Tree.Kind.MEMBER_SELECT)) {
86+
MemberSelectExpressionTree deeperMemberSelect = (MemberSelectExpressionTree) deeperSelect;
87+
ExpressionTree deeperRoot = deeperMemberSelect.expression();
88+
89+
if (deeperRoot.is(Tree.Kind.IDENTIFIER)) {
90+
Symbol rootSymbol = ((IdentifierTree) deeperRoot).symbol();
91+
if (repositoryFindAllVars.containsKey(rootSymbol) && isGetter(deeperMemberSelect.identifier().name())) {
92+
reportIssue(methodInvocation, RULE_MESSAGE);
93+
}
94+
}
9995
}
10096
}
10197
}
10298
}
99+
}
103100

101+
// Méthode utilitaire pour détecter un getter
102+
private boolean isGetter(String methodName) {
103+
return methodName.startsWith("get") && methodName.length() > 3 && Character.isUpperCase(methodName.charAt(3));
104104
}
105105
}

‎src/main/resources/org/greencodeinitiative/creedengo/java/creedengo_way_profile.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"GCI78",
1919
"GCI79",
2020
"GCI82",
21-
"GCI94"
21+
"GCI94",
22+
"GRC3"
2223
]
2324
}

‎src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class AvoidNPlusOneProblemInJPAEntitiesCheckIssue {
3030
public List<Author> smellGetAllAuthors() {
3131
List<Author> authors = authorRepository.findAll();
3232
for (Author author : authors) {
33-
List<Book> books = author.getBooks(); // Cela peut déclencher une requête SQL pour chaque auteur
33+
List<Book> books = author.getBooks(); // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }}
3434
}
3535
return authors;
3636
}

0 commit comments

Comments
 (0)