diff --git a/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java b/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java index 850e141c7..9912b1372 100644 --- a/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java +++ b/java-plugin/src/main/java/fr/greencodeinitiative/java/RulesList.java @@ -56,7 +56,8 @@ public static List> getJavaChecks() { AvoidUsingGlobalVariablesCheck.class, AvoidSetConstantInBatchUpdate.class, FreeResourcesOfAutoCloseableInterface.class, - AvoidMultipleIfElseStatement.class + AvoidMultipleIfElseStatement.class, + ForceUsingLazyFetchTypeInJPAEntity.class )); } diff --git a/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntity.java b/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntity.java new file mode 100644 index 000000000..76ee2462a --- /dev/null +++ b/java-plugin/src/main/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntity.java @@ -0,0 +1,97 @@ +package fr.greencodeinitiative.java.checks; + +import org.sonar.check.Priority; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.*; +import org.sonar.plugins.java.api.tree.Tree.Kind; + +import java.util.List; + +@Rule(key = "EC80", name = "Developpement", + description = ForceUsingLazyFetchTypeInJPAEntity.MESSAGERULE, + priority = Priority.MINOR, + tags = {"bug"}) +public class ForceUsingLazyFetchTypeInJPAEntity extends IssuableSubscriptionVisitor { + + protected static final String MESSAGERULE = "Force the use of LAZY FetchType"; + private static final String EAGER_KEYWORD = "EAGER"; + private static final String FETCH_KEYWORD = "fetch"; + private static final String ONE_TO_MANY = "OneToMany"; + private static final String MANY_TO_ONE = "ManyToOne"; + private static final String ONE_TO_ONE = "OneToOne"; + private static final String MANY_TO_MANY = "ManyToMany"; + + @Override + public List nodesToVisit() { + return List.of(Kind.VARIABLE); + } + + @Override + public void visitNode(Tree tree) { + VariableTree variableTree = (VariableTree) tree; + ModifiersTree modifierTrees = variableTree.modifiers(); + if (modifierTrees == null) { + return; + } + List annotations = modifierTrees.annotations(); + // get all annotations on the attribute + for (AnnotationTree annotationTree : annotations) { + if (!needToCheckExplicitAnnotation(annotationTree.annotationType().symbolType().name())) { + // no Explicit annotation (@OneToMany, @ManyToOne, @ManyToMany, @OneToOne) was found, + // and we don't need to investigate further + continue; + } + Arguments arguments = annotationTree.arguments(); + performsCheck(arguments, annotationTree, tree); + } + } + + /** + * perform the check for two case : + * fetch keyword is found -> parse the annotation argument and search for eager keyword + * fetch keyword was not found -> we use the default value of fetch type by join type : + * OneToMany: Lazy + * ManyToOne: Eager + * ManyToMany: Lazy + * OneToOne: Eager + * reporting the issues if necessary + */ + private void performsCheck(Arguments arguments, AnnotationTree annotationTree, Tree tree) { + boolean fetchFound = false; + for (ExpressionTree argument : arguments) { + AssignmentExpressionTree assignmentExpression = (AssignmentExpressionTree) argument; + IdentifierTree variable = (IdentifierTree) assignmentExpression.variable(); + + if (!FETCH_KEYWORD.equals(variable.name())) { + // no need to continue checking this argument + continue; + } + String fetchValue = ((MemberSelectExpressionTree) assignmentExpression.expression()).identifier().name(); + fetchFound = true; + if (EAGER_KEYWORD.equals(fetchValue)) { + reportIssue(tree, MESSAGERULE); + } + } + //- The default case of the ManyToOne and the OneToOne + // the fetch keyword is not explicit + if (fetchFound) { + return; + } + String symbolType = annotationTree.annotationType().symbolType().name(); + if ((MANY_TO_ONE.equals(symbolType) || ONE_TO_ONE.equals(symbolType))) { + reportIssue(tree, MESSAGERULE); + } + } + + /** + * @param symbolType the annotation type name : @OneToMany, @ManyToOne... + * @return true if searched annotation was found and checks need to be performed + */ + private boolean needToCheckExplicitAnnotation(String symbolType) { + return ONE_TO_MANY.equals(symbolType) + || MANY_TO_ONE.equals(symbolType) + || ONE_TO_ONE.equals(symbolType) + || MANY_TO_MANY.equals(symbolType); + } +} diff --git a/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.html b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.html new file mode 100644 index 000000000..9ab41bc22 --- /dev/null +++ b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.html @@ -0,0 +1,30 @@ +

Consider using LAZY mode on your FetchType for collections in JPA type entities. This will reduce the amount of data loaded into memory.

+ +

Non-compliant Code Example

+
+    @Entity
+    @Table(name = "purchaseOrder")
+    public class Order implements Serializable {
+       
+      @OneToMany(mappedBy = "order", fetch = FetchType.EAGER)
+      private Set items = new HashSet();
+       
+      ...
+       
+    }
+
+
+

Compliant Code Example

+
+    @Entity
+    @Table(name = "purchaseOrder")
+    public class Order implements Serializable {
+       
+      @OneToMany(mappedBy = "order", fetch = FetchType.LAZY)
+      private Set items = new HashSet();
+       
+      ...
+       
+    }
+
+
\ No newline at end of file diff --git a/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.json b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.json new file mode 100644 index 000000000..df71a8247 --- /dev/null +++ b/java-plugin/src/main/resources/fr/greencodeinitiative/l10n/java/rules/java/EC80.json @@ -0,0 +1,16 @@ +{ + "title": "Force the use of FetchType LAZY on collections in Entity JPA", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "eco-design", + "performance", + "bug", + "ecocode" + ], + "defaultSeverity": "Minor" + } \ No newline at end of file diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseAllInOne.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseAllInOne.java new file mode 100644 index 000000000..bacb397ec --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseAllInOne.java @@ -0,0 +1,26 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseAllInOne implements Serializable { + + @OneToMany(mappedBy = "myOrders", fetch = FetchType.EAGER) // Noncompliant {{Force the use of LAZY FetchType}} + private Set items = new HashSet(); + + @OneToMany(mappedBy = "myOrders", fetch = FetchType.LAZY) + private Set items = new HashSet(); + + @Column(name = "ORDER", length = 50, nullable = false, unique = false) + private Set items = new HashSet(); + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseCompliant.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseCompliant.java new file mode 100644 index 000000000..eace2fa77 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseCompliant.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseCompliant implements Serializable { + + @OneToMany(mappedBy = "myOrders", fetch = FetchType.LAZY) + private Set items = new HashSet(); + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultManyToOne.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultManyToOne.java new file mode 100644 index 000000000..27f22ccb0 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultManyToOne.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseFalse implements Serializable { + + @ManyToOne // Noncompliant {{Force the use of LAZY FetchType}} + private Order order; + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultOneToOne.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultOneToOne.java new file mode 100644 index 000000000..1c3cbe436 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseDefaultOneToOne.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseFalse implements Serializable { + + @OneToOne // Noncompliant {{Force the use of LAZY FetchType}} + private OrderInformation orderInformations; + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseFalseTest.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseFalseTest.java new file mode 100644 index 000000000..b2df99b91 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseFalseTest.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseFalse implements Serializable { + + @Column(name = "ORDER", length = 50, nullable = false, unique = false) + private Set items = new HashSet(); + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseManyToOne.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseManyToOne.java new file mode 100644 index 000000000..4635d7d76 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseManyToOne.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseFalse implements Serializable { + + @ManyToOne(fetch = FetchType.EAGER) // Noncompliant {{Force the use of LAZY FetchType}} + private Order order; + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseNonCompliant.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseNonCompliant.java new file mode 100644 index 000000000..6cb7aa618 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseNonCompliant.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseNonCompliant implements Serializable { + + @OneToMany(mappedBy = "myOrders", fetch = FetchType.EAGER) // Noncompliant {{Force the use of LAZY FetchType}} + private Set items = new HashSet(); + +} diff --git a/java-plugin/src/test/files/ForceLazyFetchTypeUseOneToOne.java b/java-plugin/src/test/files/ForceLazyFetchTypeUseOneToOne.java new file mode 100644 index 000000000..fb597aa07 --- /dev/null +++ b/java-plugin/src/test/files/ForceLazyFetchTypeUseOneToOne.java @@ -0,0 +1,20 @@ +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; +import javax.persistence.Transient; + +@Entity +@Table(name = "myTable") +public class ForceLazyFetchTypeUseFalse implements Serializable { + + @OneToOne(fetch = FetchType.EAGER) // Noncompliant {{Force the use of LAZY FetchType}} + private OrderInformation orderInformations; + +} diff --git a/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java b/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java index 29cc52ae2..a55efad11 100644 --- a/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java +++ b/java-plugin/src/test/java/fr/greencodeinitiative/java/JavaCheckRegistrarTest.java @@ -30,7 +30,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(); } diff --git a/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntityTest.java b/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntityTest.java new file mode 100644 index 000000000..7e3535aa1 --- /dev/null +++ b/java-plugin/src/test/java/fr/greencodeinitiative/java/checks/ForceUsingLazyFetchTypeInJPAEntityTest.java @@ -0,0 +1,74 @@ +package fr.greencodeinitiative.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +class ForceUsingLazyFetchTypeInJPAEntityTest { + + @Test + void checkNonCompliantTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseNonCompliant.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + } + + @Test + void checkCompliantTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseCompliant.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyNoIssues(); + + } + + @Test + void checkPositiveFalseTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseFalseTest.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyNoIssues(); + } + + @Test + void checkAllTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseAllInOne.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + } + + @Test + void checkDefaultOneToOneTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseDefaultOneToOne.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + } + + @Test + void checkDefaultManyToOneTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseDefaultManyToOne.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + } + + @Test + void checkManyToOneTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseManyToOne.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + } + + @Test + void checkOneToOneTests() { + CheckVerifier.newVerifier() + .onFile("src/test/files/ForceLazyFetchTypeUseOneToOne.java") + .withCheck(new ForceUsingLazyFetchTypeInJPAEntity()) + .verifyIssues(); + + } + +}