diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8948.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8948.json new file mode 100644 index 00000000000..6c27a381816 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8948.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8948", + "hasTruePositives": false, + "falseNegatives": 8, + "falsePositives": 0 +} diff --git a/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSample.java new file mode 100644 index 00000000000..c8f26a3f520 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSample.java @@ -0,0 +1,61 @@ +package checks; + +import jakarta.persistence.Entity; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +import java.util.List; + +public class OneToManyMappingCheckSample { + + @Entity + class Author { + @OneToMany // Noncompliant {{Add "mappedBy" or "@JoinColumn" to this "@OneToMany" relationship.}} +// ^^^^^^^^^^ + private List books; + } + + @Entity + class Book { + @ManyToOne + private Author author; + } + + // Compliant: uses mappedBy + @Entity + class AuthorWithMappedBy { + @OneToMany(mappedBy = "author") + private List books; + } + + @Entity + class BookWithAuthor { + @ManyToOne + @JoinColumn(name = "author_id") + private AuthorWithMappedBy author; + } + + // Compliant: uses @JoinColumn on the @OneToMany field + @Entity + class AuthorWithJoinColumn { + @OneToMany + @JoinColumn(name = "author_id") + private List books; + } + + @Entity + class BookNoRef { + // No reference back to Author + } + + @Entity + class AuthorUnidirectional { + @OneToMany // Noncompliant + private List books; + } + + @Entity + class AnotherBook { + // No reference back + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSampleJavax.java b/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSampleJavax.java new file mode 100644 index 00000000000..6e444164fab --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/OneToManyMappingCheckSampleJavax.java @@ -0,0 +1,61 @@ +package checks; + +import java.util.List; +import javax.persistence.Entity; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +public class OneToManyMappingCheckSampleJavax { + + @Entity + class Author { + @OneToMany // Noncompliant {{Add "mappedBy" or "@JoinColumn" to this "@OneToMany" relationship.}} +// ^^^^^^^^^^ + private List books; + } + + @Entity + class Book { + @ManyToOne + private Author author; + } + + // Compliant: uses mappedBy + @Entity + class AuthorWithMappedBy { + @OneToMany(mappedBy = "author") + private List books; + } + + @Entity + class BookWithAuthor { + @ManyToOne + @JoinColumn(name = "author_id") + private AuthorWithMappedBy author; + } + + // Compliant: uses @JoinColumn on the @OneToMany field + @Entity + class AuthorWithJoinColumn { + @OneToMany + @JoinColumn(name = "author_id") + private List books; + } + + @Entity + class BookNoRef { + // No reference back to Author + } + + @Entity + class AuthorUnidirectional { + @OneToMany // Noncompliant + private List books; + } + + @Entity + class AnotherBook { + // No reference back + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/OneToManyMappingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/OneToManyMappingCheck.java new file mode 100644 index 00000000000..aa8e81682c8 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/OneToManyMappingCheck.java @@ -0,0 +1,74 @@ +/* + * 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; + +import java.util.List; +import java.util.Set; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; + +@Rule(key = "S8948") +public class OneToManyMappingCheck extends IssuableSubscriptionVisitor { + + private static final Set ONE_TO_MANY_ANNOTATIONS = Set.of( + "jakarta.persistence.OneToMany", + "javax.persistence.OneToMany"); + + private static final Set JOIN_COLUMN_ANNOTATIONS = Set.of( + "jakarta.persistence.JoinColumn", + "javax.persistence.JoinColumn"); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.VARIABLE); + } + + @Override + public void visitNode(Tree tree) { + var variable = (VariableTree) tree; + var annotations = variable.modifiers().annotations(); + + annotations.stream() + .filter(OneToManyMappingCheck::isOneToManyAnnotation) + .filter(annotation -> !hasMappedBy(annotation)) + .filter(annotation -> annotations.stream().noneMatch(OneToManyMappingCheck::isJoinColumnAnnotation)) + .forEach(annotation -> reportIssue(annotation, "Add \"mappedBy\" or \"@JoinColumn\" to this \"@OneToMany\" relationship.")); + } + + private static boolean isOneToManyAnnotation(AnnotationTree annotation) { + return ONE_TO_MANY_ANNOTATIONS.stream().anyMatch(annotation.annotationType().symbolType()::is); + } + + private static boolean isJoinColumnAnnotation(AnnotationTree annotation) { + return JOIN_COLUMN_ANNOTATIONS.stream().anyMatch(annotation.annotationType().symbolType()::is); + } + + private static boolean hasMappedBy(AnnotationTree annotation) { + return annotation.arguments().stream() + .filter(arg -> arg.is(Tree.Kind.ASSIGNMENT)) + .map(AssignmentExpressionTree.class::cast) + .map(AssignmentExpressionTree::variable) + .filter(v -> v.is(Tree.Kind.IDENTIFIER)) + .map(IdentifierTree.class::cast) + .anyMatch(id -> "mappedBy".equals(id.name())); + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/OneToManyMappingCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/OneToManyMappingCheckTest.java new file mode 100644 index 00000000000..5877f4f04d7 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/OneToManyMappingCheckTest.java @@ -0,0 +1,41 @@ +/* + * 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; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class OneToManyMappingCheckTest { + + @Test + void testJakarta() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/OneToManyMappingCheckSample.java")) + .withCheck(new OneToManyMappingCheck()) + .verifyIssues(); + } + + @Test + void testJavax() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/OneToManyMappingCheckSampleJavax.java")) + .withCheck(new OneToManyMappingCheck()) + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.html new file mode 100644 index 00000000000..e6e5be83456 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.html @@ -0,0 +1,106 @@ +

This is an issue when a one-to-many relationship in object-relational mapping is declared without either specifying which side owns the +bidirectional relationship or providing explicit foreign key column configuration, causing the ORM framework to create an unnecessary join table.

+

In JPA, this specifically refers to the @OneToMany annotation without a mappedBy attribute or @JoinColumn +annotation.

+

Why is this an issue?

+

When you define a one-to-many relationship in an object-relational mapping framework without specifying how the relationship should be mapped, the +persistence provider follows its specification default: it creates a separate join table to represent the association.

+

For example, if you have an Author entity with a collection of Book entities:

+
+Entity: Author
+  one-to-many relationship -> books: collection of Book
+
+

The ORM will create three tables: Author, Book, and Author_Book (the join table). The join table contains +two foreign key columns: one referencing Author and one referencing Book.

+

This default behavior is rarely what you want for true one-to-many relationships

+

In JPA, this occurs when you use the @OneToMany annotation without additional mapping configuration. For example:

+
+@Entity
+public class Author {
+    @OneToMany
+    private List<Book> books;
+}
+
+

What is the potential impact?

+

Using the default join table mapping for one-to-many relationships in object-relational mapping frameworks degrades application performance through +additional database tables, extra SQL statements, and more complex query plans. It also makes the database schema less intuitive and harder to +maintain, as the foreign key is not stored where developers would naturally expect it.

+

In Java, this specifically refers to the @OneToMany annotation in JPA (Java Persistence API) and Hibernate.

+

How to fix it in Jakarta EE

+

For bidirectional relationships (where the child entity has a reference back to the parent), use the mappedBy attribute on the +@OneToMany side to indicate that the relationship is owned by the child entity. This tells JPA to use the foreign key column on the child +table. The mappedBy value must match the field name in the child entity that references the parent.

+

For unidirectional relationships (where only the parent knows about the children), use @JoinColumn to specify that the foreign key +should be stored in the child table. The @JoinColumn annotation tells JPA to create or use a foreign key column in the child table +instead of creating a join table.

+

Code examples

+

Noncompliant code example

+
+@Entity
+public class Author {
+    @OneToMany // Noncompliant
+    private List<Book> books;
+}
+
+@Entity
+public class Book {
+    @ManyToOne
+    private Author author;
+}
+
+

Compliant solution

+
+@Entity
+public class Author {
+    @OneToMany(mappedBy = "author")
+    private List<Book> books;
+}
+
+@Entity
+public class Book {
+    @ManyToOne
+    @JoinColumn(name = "author_id")
+    private Author author;
+}
+
+

Noncompliant code example

+
+@Entity
+public class Author {
+    @OneToMany // Noncompliant
+    private List<Book> books;
+}
+
+@Entity
+public class Book {
+    // No reference back to Author
+}
+
+

Compliant solution

+
+@Entity
+public class Author {
+    @OneToMany
+    @JoinColumn(name = "author_id")
+    private List<Book> books;
+}
+
+@Entity
+public class Book {
+    // No reference back to Author
+    // JPA will add author_id column to Book table
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.json new file mode 100644 index 00000000000..6b3aa37774c --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8948.json @@ -0,0 +1,26 @@ +{ + "title": "\"@OneToMany\" relationships should use \"mappedBy\" or \"@JoinColumn\"", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "jpa", + "hibernate", + "performance", + "jakarta" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-8948", + "sqKey": "S8948", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "EFFICIENT" + } +} 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..dcaa686a0bb 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 @@ -535,6 +535,7 @@ "S8745", "S8786", "S8911", - "S8924" + "S8924", + "S8948" ] }