HHH-14229 Fix unexpected foreign key creation

before this commit, foreign key is created even ConstraintMode.NO_CONSTRAINT present on the @ManyToOne side

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 );
+							final OneToMany oneToManyAnn = property.getAnnotation( OneToMany.class );
-							if ( joinColumnAnn != null ) {
+							final OnDelete onDeleteAnn = property.getAnnotation( OnDelete.class );
-								if ( joinColumnAnn.foreignKey().value() == ConstraintMode.NO_CONSTRAINT ) {
+							if ( oneToManyAnn != null && !oneToManyAnn.mappedBy().isEmpty()
-									key.setForeignKeyName( "none" );
+									&& ( onDeleteAnn == null || onDeleteAnn.action() != OnDeleteAction.CASCADE ) ) {
-								}
+								// foreign key should be up to @ManyToOne side
-								else {
+								// @OnDelete generate "on delete cascade" foreign key
-									key.setForeignKeyName( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().name() ) );
+								key.setForeignKeyName( "none" );
-									key.setForeignKeyDefinition( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().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() ) );
+									}
 								}
 							}
 						}
@@ -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<Selectable> 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<Join> 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() ) {

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 <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+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<PlainTreeEntity> 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<TreeEntityWithOnDelete> children = new ArrayList<>(0);
+
+	}
+
+}