GCI604 V.E.R.T Grc3 feature n+1 issue#108
GCI604 V.E.R.T Grc3 feature n+1 issue#108NassimAMGHAR wants to merge 9 commits intogreen-code-initiative:mainfrom
Conversation
NassimAMGHAR
changed the title
| public List<Author> smellGetAllAuthors() { | ||
| List<Author> authors = authorRepository.findAll(); | ||
| for (Author author : authors) { | ||
| List<Book> books = author.getBooks(); // // Noncompliant {{ Evitez le N+1 : utilisez un fetch join ou une récupération eager. }} |
There was a problem hiding this comment.
More precisely, it's the call to a method on getBooks() (for example, .size() or .get(5)) that triggers the initialization of the list, causing SQL queries to be executed — and that's where the problem occurs.
Calling getBooks() alone simply returns the proxy; it does not trigger any query by itself.
However, I believe that calling getBooks() is enough to justify raising the rule violation: why retrieve the proxy if nothing is done with it afterward?
That’s why we should ensure the rule’s description clearly explains why this is an issue.
✅ Agreed: no need to simulate a method call; just the getBooks() access is enough to be considered problematic.
✅ The comment should be written in English.
✅ Remove the double //.
✅ Add a proper rule description.
There was a problem hiding this comment.
Yes agree, changed the comment to match the specification
NassimAMGHAR
force-pushed
the
GRC3-feature-N+1-issue
branch
from
5d76fdd to
0b9e962
Compare
|
This PR has been automatically marked as stale because it has no activity for 30 days. |
| @Rule(key = "GCI604") | ||
| public class AvoidNPlusOneProblemInJPAEntitiesCheck extends IssuableSubscriptionVisitor { | ||
|
|
||
| protected static final String RULE_MESSAGE = " Detection of the \"N+1 problem\" on Spring Data JPA repositories "; |
There was a problem hiding this comment.
please delete de first space in the message
| } | ||
| } | ||
|
|
||
| // Cas d'un foreach sur une variable issue de findAll() |
There was a problem hiding this comment.
please us english in comments in all files
| if (tree.is(Tree.Kind.METHOD_INVOCATION)) { | ||
| MethodInvocationTree methodInvocation = (MethodInvocationTree) tree; | ||
|
|
||
| // Check if the call is something like post.getAuthor() or post.getAuthor().getName() |
There was a problem hiding this comment.
comments contains example and not the algorithms description
| CheckVerifier.newVerifier() | ||
| .onFile("src/test/files/AvoidNPlusOneProblemInJPAEntitiesCheckIssue.java") | ||
| .withCheck(new AvoidNPlusOneProblemInJPAEntitiesCheck()) | ||
| .withClassPath(FilesUtils.getClassPath("target/test-jars")) |
There was a problem hiding this comment.
why do you need this method call "withClassPath" ?
| } | ||
|
|
||
|
|
||
| public class Author { |
There was a problem hiding this comment.
I don't see JPA annotations to link Author and Book objects, like the usage of @entity, @id, @onetomany, ... like described here https://www.baeldung.com/spring-hibernate-n1-problem
sorry, but I don't understand how it works without JPA annotations ...
|
This PR has been automatically marked as stale because it has no activity for 30 days. |
3 participants
Adding a new rule to check GRC3 : N+1 problem when calling a nested Object in an entity that causes multiples repository calls ->
#36