From a396c89322143f417824197b57ff872cf73f81a2 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Mon, 28 Sep 2020 10:31:49 +0800 Subject: [PATCH] HHH-14229 Fix unexpected foreign key creation before this commit, foreign key is created even ConstraintMode.NO_CONSTRAINT present on the @ManyToOne side --- .../cfg/annotations/CollectionBinder.java | 36 ++++--- .../OneToManyBidirectionalForeignKeyTest.java | 94 +++++++++++++++++++ 2 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/foreignkeys/disabled/OneToManyBidirectionalForeignKeyTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java index 554c13cfc0..75e3d85a01 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java @@ -47,6 +47,7 @@ import org.hibernate.annotations.LazyGroup; import org.hibernate.annotations.Loader; import org.hibernate.annotations.ManyToAny; import org.hibernate.annotations.OnDelete; +import org.hibernate.annotations.OnDeleteAction; import org.hibernate.annotations.OptimisticLock; import org.hibernate.annotations.OrderBy; import org.hibernate.annotations.Parameter; @@ -101,6 +102,7 @@ import org.hibernate.mapping.KeyValue; import org.hibernate.mapping.ManyToOne; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; +import org.hibernate.mapping.Selectable; import org.hibernate.mapping.SimpleValue; import org.hibernate.mapping.Table; @@ -1210,14 +1212,24 @@ public abstract class CollectionBinder { key.setForeignKeyDefinition( StringHelper.nullIfEmpty( fkOverride.foreignKeyDefinition() ) ); } else { - final JoinColumn joinColumnAnn = property.getAnnotation( JoinColumn.class ); - if ( joinColumnAnn != null ) { - if ( joinColumnAnn.foreignKey().value() == ConstraintMode.NO_CONSTRAINT ) { - key.setForeignKeyName( "none" ); - } - else { - key.setForeignKeyName( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().name() ) ); - key.setForeignKeyDefinition( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().foreignKeyDefinition() ) ); + final OneToMany oneToManyAnn = property.getAnnotation( OneToMany.class ); + final OnDelete onDeleteAnn = property.getAnnotation( OnDelete.class ); + if ( oneToManyAnn != null && !oneToManyAnn.mappedBy().isEmpty() + && ( onDeleteAnn == null || onDeleteAnn.action() != OnDeleteAction.CASCADE ) ) { + // foreign key should be up to @ManyToOne side + // @OnDelete generate "on delete cascade" foreign key + key.setForeignKeyName( "none" ); + } + else { + final JoinColumn joinColumnAnn = property.getAnnotation( JoinColumn.class ); + if ( joinColumnAnn != null ) { + if ( joinColumnAnn.foreignKey().value() == ConstraintMode.NO_CONSTRAINT ) { + key.setForeignKeyName( "none" ); + } + else { + key.setForeignKeyName( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().name() ) ); + key.setForeignKeyDefinition( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().foreignKeyDefinition() ) ); + } } } } @@ -1679,13 +1691,13 @@ public abstract class CollectionBinder { final String mappedBy = columns[0].getMappedBy(); if ( StringHelper.isNotEmpty( mappedBy ) ) { final Property property = referencedEntity.getRecursiveProperty( mappedBy ); - Iterator mappedByColumns; + Iterator mappedByColumns; if ( property.getValue() instanceof Collection ) { mappedByColumns = ( (Collection) property.getValue() ).getKey().getColumnIterator(); } else { //find the appropriate reference key, can be in a join - Iterator joinsIt = referencedEntity.getJoinIterator(); + Iterator joinsIt = referencedEntity.getJoinIterator(); KeyValue key = null; while ( joinsIt.hasNext() ) { Join join = (Join) joinsIt.next(); @@ -1694,7 +1706,9 @@ public abstract class CollectionBinder { break; } } - if ( key == null ) key = property.getPersistentClass().getIdentifier(); + if ( key == null ) { + key = property.getPersistentClass().getIdentifier(); + } mappedByColumns = key.getColumnIterator(); } while ( mappedByColumns.hasNext() ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/foreignkeys/disabled/OneToManyBidirectionalForeignKeyTest.java b/hibernate-core/src/test/java/org/hibernate/test/foreignkeys/disabled/OneToManyBidirectionalForeignKeyTest.java new file mode 100644 index 0000000000..38cd379156 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/foreignkeys/disabled/OneToManyBidirectionalForeignKeyTest.java @@ -0,0 +1,94 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.foreignkeys.disabled; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.stream.StreamSupport; + +import javax.persistence.CascadeType; +import javax.persistence.ConstraintMode; +import javax.persistence.Entity; +import javax.persistence.ForeignKey; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.hibernate.annotations.OnDelete; +import org.hibernate.annotations.OnDeleteAction; +import org.hibernate.boot.Metadata; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.mapping.Table; +import org.hibernate.testing.TestForIssue; +import org.junit.Test; + +/** + * {@inheritDoc} + * + * @author Yanming Zhou + */ +@TestForIssue(jiraKey = "HHH-14229") +public class OneToManyBidirectionalForeignKeyTest { + + private static final String TABLE_NAME_PLAIN = "plain"; + private static final String TABLE_NAME_WITH_ON_DELETE = "cascade_delete"; + + @Test + public void testForeignKeyShouldNotBeCreated() { + Metadata metadata = new MetadataSources(new StandardServiceRegistryBuilder().build()) + .addAnnotatedClass(PlainTreeEntity.class).addAnnotatedClass(TreeEntityWithOnDelete.class) + .buildMetadata(); + assertTrue(findTable(metadata, TABLE_NAME_PLAIN).getForeignKeys().isEmpty()); + assertFalse(findTable(metadata, TABLE_NAME_WITH_ON_DELETE).getForeignKeys().isEmpty()); + } + + private static Table findTable(Metadata metadata, String tableName) { + return StreamSupport.stream(metadata.getDatabase().getNamespaces().spliterator(), false) + .flatMap(namespace -> namespace.getTables().stream()).filter(t -> t.getName().equals(tableName)) + .findFirst().orElse(null); + } + + @Entity + @javax.persistence.Table(name = TABLE_NAME_PLAIN) + public static class PlainTreeEntity { + + @Id + private Long id; + + @ManyToOne + @JoinColumn(foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)) + private PlainTreeEntity parent; + + @OneToMany(mappedBy = "parent") + // workaround + // @org.hibernate.annotations.ForeignKey(name = "none") + private Collection children = new ArrayList<>(0); + } + + @Entity + @javax.persistence.Table(name = TABLE_NAME_WITH_ON_DELETE) + public static class TreeEntityWithOnDelete { + + @Id + private Long id; + + @ManyToOne + @JoinColumn(foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)) + private TreeEntityWithOnDelete parent; + + @OneToMany(mappedBy = "parent", cascade = CascadeType.REMOVE) + @OnDelete(action = OnDeleteAction.CASCADE) + private Collection children = new ArrayList<>(0); + + } + +}