Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRJVM205 - Forcer l'utilisation du FetchType LAZY sur les collections… #155

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,7 @@
import java.util.Collections;
import java.util.List;

import fr.greencodeinitiative.java.checks.ArrayCopyCheck;
import fr.greencodeinitiative.java.checks.AvoidConcatenateStringsInLoop;
import fr.greencodeinitiative.java.checks.AvoidFullSQLRequest;
import fr.greencodeinitiative.java.checks.AvoidGettingSizeCollectionInLoop;
import fr.greencodeinitiative.java.checks.AvoidMultipleIfElseStatement;
import fr.greencodeinitiative.java.checks.AvoidRegexPatternNotStatic;
import fr.greencodeinitiative.java.checks.AvoidSQLRequestInLoop;
import fr.greencodeinitiative.java.checks.AvoidSetConstantInBatchUpdate;
import fr.greencodeinitiative.java.checks.AvoidSpringRepositoryCallInLoopCheck;
import fr.greencodeinitiative.java.checks.AvoidStatementForDMLQueries;
import fr.greencodeinitiative.java.checks.AvoidUsageOfStaticCollections;
import fr.greencodeinitiative.java.checks.AvoidUsingGlobalVariablesCheck;
import fr.greencodeinitiative.java.checks.FreeResourcesOfAutoCloseableInterface;
import fr.greencodeinitiative.java.checks.IncrementCheck;
import fr.greencodeinitiative.java.checks.InitializeBufferWithAppropriateSize;
import fr.greencodeinitiative.java.checks.NoFunctionCallWhenDeclaringForLoop;
import fr.greencodeinitiative.java.checks.OptimizeReadFileExceptions;
import fr.greencodeinitiative.java.checks.UnnecessarilyAssignValuesToVariables;
import fr.greencodeinitiative.java.checks.UseCorrectForLoop;
import fr.greencodeinitiative.java.checks.*;
import org.sonar.plugins.java.api.JavaCheck;

public final class RulesList {
Expand Down Expand Up @@ -77,7 +59,8 @@ public static List<Class<? extends JavaCheck>> getJavaChecks() {
AvoidUsingGlobalVariablesCheck.class,
AvoidSetConstantInBatchUpdate.class,
FreeResourcesOfAutoCloseableInterface.class,
AvoidMultipleIfElseStatement.class
AvoidMultipleIfElseStatement.class,
UseFetchTypeLazyRule.class
));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package fr.greencodeinitiative.java.checks;

import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.tree.*;

@Rule(
key = "EC_CRJVM205",
name = "Developpement",
description = UseFetchTypeLazyRule.MESSAGE_RULE,
priority = Priority.MINOR,
tags = {"bug"})
public class UseFetchTypeLazyRule extends BaseTreeVisitor implements JavaFileScanner {
protected static final String MESSAGE_RULE = "Avoid Using FetchType.EAGER instead of FetchType.LAZY on collections in JPA Entity";

private JavaFileScannerContext context;

@Override
public void scanFile(JavaFileScannerContext javaFileScannerContext) {
this.context = javaFileScannerContext;
// The call to the scan method on the root of the tree triggers the visit of the AST by this visitor
scan(context.getTree());
}

@Override
public void visitAnnotation(AnnotationTree annotationTree) {
String annotationName = ((IdentifierTree) annotationTree.annotationType()).name();
if (annotationName.equals("OneToMany")
|| annotationName.equals("ManyToMany")) {
boolean fetchExist = false;
ExpressionTree fetchTypeArg = null;

for (ExpressionTree argument : annotationTree.arguments()) {
if (argument.is(Tree.Kind.ASSIGNMENT)) {
AssignmentExpressionTree assignmentInvocation = (AssignmentExpressionTree) argument;
if (assignmentInvocation.variable().toString().equals("fetch")) {
fetchExist = true;
fetchTypeArg = assignmentInvocation.expression();
}
}
}

this.reportFetchTypeIssue(fetchExist,fetchTypeArg,annotationTree);
}
super.visitAnnotation(annotationTree);
}

private void reportFetchTypeIssue(boolean fetchExist, ExpressionTree fetchTypeArg,AnnotationTree annotationTree){
if (!fetchExist) {
context.reportIssue(this, annotationTree, "JPA annotation without FetchType detected");
} else if (fetchTypeArg != null) {
String fetchType = ((MemberSelectExpressionTree) fetchTypeArg).identifier().name();
if (!fetchType.strip().equals("LAZY")) {
context.reportIssue(this, annotationTree, MESSAGE_RULE);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<h1>Avoid using a FetchType other than FetchType.LAZY</h1>
<p> Using <code>FetchType.EAGER</code> can lead to performance issues and should be avoided. Always use <code>FetchType.LAZY</code> instead.</p>
<h2>Noncompliant Code Example</h2>
<pre>
@OneToMany(fetch = FetchType.EAGER, mappedBy = "otherEntity_id")
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
<pre>
@ManyToMany(fetch = FetchType.EAGER)
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
<pre>
@OneToMany
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
<pre>
@ManyToMany
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
<h2>Compliant Solution</h2>
<pre>
@OneToMany(fetch = FetchType.LAZY, mappedBy = "otherEntity_id")
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
<pre>
@ManyToMany(fetch = FetchType.LAZY, mappedBy = "otherEntity_id")
@Column(name = "otherEntity_id")
private Collection&lt;otherEntity&gt; otherEntities;
</pre>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"title": "Avoid using a FetchType other than FetchType.LAZY",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"jpa",
"fetch",
"lazy",
"eco-design",
"performance",
"memory",
"bug",
"ecocode"
],
"defaultSeverity": "Minor"
}
34 changes: 34 additions & 0 deletions java-plugin/src/test/files/UseFetchTypeLazyRule.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package fr.greencodeinitiative.java.checks;

import javax.persistence.OneToMany;
import javax.persistence.ManyToOne;
import java.util.*;

public class UseFetchTypeLazyRuleTest {

@OneToMany(mappedBy = "firstEntity") // Noncompliant
private Collection<SomeEntity> firstEntities;

@ManyToMany(mappedBy = "firstEntity1") // Noncompliant
private Collection<SomeEntity> firstEntities1;

@OneToMany // Noncompliant
private Collection<SomeEntity> secondEntities1;

@ManyToMany // Noncompliant
private Collection<SomeEntity> secondEntities2;

@OneToMany(mappedBy = "thirdEntity1", fetch= FetchType.EAGER) // Noncompliant
private Collection<SomeEntity> thirdEntities1;

@ManyToMany(mappedBy = "thirdEntity1", fetch= FetchType.EAGER) // Noncompliant
private Collection<SomeEntity> thirdEntities2;

@OneToMany(fetch = FetchType.LAZY) // Compliant
private Collection<SomeEntity> fourthEntities1;

@ManyToMany(fetch = FetchType.LAZY) // Compliant
private Collection<SomeEntity> fourthEntities2;

}

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void checkNumberRules() {
final JavaCheckRegistrar registrar = new JavaCheckRegistrar();
registrar.register(context);

assertThat(context.checkClasses()).hasSize(19);
assertThat(context.checkClasses()).hasSize(20);
assertThat(context.testCheckClasses()).isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package fr.greencodeinitiative.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;


class UseFetchTypeLazyRuleTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile("src/test/files/UseFetchTypeLazyRule.java")
.withCheck(new UseFetchTypeLazyRule())
.verifyIssues();
}
}