HHH-8217 constraint name fixes for metamodel

hibernate-core/src/main/java/org/hibernate/metamodel/internal/Binder.java

@@ -23,6 +23,8 @@
  */
 package org.hibernate.metamodel.internal;
 
+import static org.hibernate.engine.spi.SyntheticAttributeHelper.SYNTHETIC_COMPOSITE_ID_ATTRIBUTE_NAME;
+
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -36,8 +38,6 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 
-import org.jboss.logging.Logger;
-
 import org.hibernate.AssertionFailure;
 import org.hibernate.EntityMode;
 import org.hibernate.MultiTenancyStrategy;
@@ -46,6 +46,7 @@ import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.NotYetImplementedException;
 import org.hibernate.cfg.ObjectNameNormalizer;
+import org.hibernate.cfg.ObjectNameNormalizer.NamingStrategyHelper;
 import org.hibernate.engine.FetchStyle;
 import org.hibernate.engine.FetchTiming;
 import org.hibernate.engine.config.spi.ConfigurationService;
@@ -176,9 +177,7 @@ import org.hibernate.type.ComponentType;
 import org.hibernate.type.EntityType;
 import org.hibernate.type.ForeignKeyDirection;
 import org.hibernate.type.Type;
-
+import org.jboss.logging.Logger;
-import static org.hibernate.cfg.ObjectNameNormalizer.NamingStrategyHelper;
-import static org.hibernate.engine.spi.SyntheticAttributeHelper.SYNTHETIC_COMPOSITE_ID_ATTRIBUTE_NAME;
 
 /**
  * The common binder shared between annotations and {@code hbm.xml} processing.
@@ -2106,13 +2105,15 @@ public class Binder {
 				)
 		);
 		if ( elementSource.isUnique() ) {
-			UniqueKey uk = collectionTable.getOrCreateUniqueKey( StringHelper.randomFixedLengthHex(
+			UniqueKey uk = new UniqueKey();
-					UniqueKey.GENERATED_NAME_PREFIX ) );
 			for ( RelationalValueBinding relationalValueBinding : elementBinding.getRelationalValueBindings() ) {
 				if ( ! relationalValueBinding.isDerived() )  {
 					uk.addColumn( (Column) relationalValueBinding.getValue() );
 				}
 			}
+			uk.setTable( collectionTable );
+			uk.generateName();
+			collectionTable.addUniqueKey( uk );
 		}
 		if ( !elementBinding.getPluralAttributeBinding().getPluralAttributeKeyBinding().isInverse() &&
 				!elementBinding.hasDerivedValue() ) {
@@ -2764,14 +2765,19 @@ public class Binder {
 			final EntitySource entitySource) {
 		for ( final ConstraintSource constraintSource : entitySource.getConstraints() ) {
 			if ( UniqueConstraintSource.class.isInstance( constraintSource ) ) {
+				UniqueKey uk = new UniqueKey();
+				
 				final TableSpecification table = entityBinding.locateTable( constraintSource.getTableName() );
+				final List<String> columnNames = constraintSource.columnNames();
 				final String constraintName = StringHelper.isEmpty( constraintSource.name() )
-						? StringHelper.randomFixedLengthHex( UniqueKey.GENERATED_NAME_PREFIX )
+						? UniqueKey.generateName( table, columnNames.toArray( new String[columnNames.size()] ) )
 						: constraintSource.name();
-				final UniqueKey uniqueKey = table.getOrCreateUniqueKey( constraintName );
+				for ( final String columnName : columnNames ) {
-				for ( final String columnName : constraintSource.columnNames() ) {
+					uk.addColumn( table.locateOrCreateColumn( quotedIdentifier( columnName ) ) );
-					uniqueKey.addColumn( table.locateOrCreateColumn( quotedIdentifier( columnName ) ) );
 				}
+				uk.setTable( table );
+				uk.setName( constraintName );
+				table.addUniqueKey( uk );
 			}
 		}
 	}
@@ -2948,8 +2954,11 @@ public class Binder {
 		column.setComment( columnSource.getComment() );
 		
 		if (columnSource.isUnique()) {
-			table.getOrCreateUniqueKey( StringHelper.randomFixedLengthHex(
+			UniqueKey uk = new UniqueKey();
-					UniqueKey.GENERATED_NAME_PREFIX ) ).addColumn( column );
+			uk.addColumn( column );
+			uk.setTable( table );
+			uk.generateName();
+			table.addUniqueKey( uk );
 		}
 		return column;
 	}

hibernate-core/src/main/java/org/hibernate/metamodel/internal/ForeignKeyHelper.java

@@ -25,17 +25,17 @@ package org.hibernate.metamodel.internal;
 
 import java.util.List;
 
-import org.jboss.logging.Logger;
-
 import org.hibernate.AssertionFailure;
+import org.hibernate.annotations.common.util.StringHelper;
 import org.hibernate.internal.CoreMessageLogger;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.metamodel.spi.relational.Column;
 import org.hibernate.metamodel.spi.relational.ForeignKey;
 import org.hibernate.metamodel.spi.relational.TableSpecification;
+import org.jboss.logging.Logger;
 
 /**
  * @author Gail Badner
+ * @author Brett Meyer
  */
 public class ForeignKeyHelper {
 	private static final CoreMessageLogger log = Logger.getMessageLogger(
@@ -55,16 +55,14 @@ public class ForeignKeyHelper {
 			final List<Column> sourceColumns,
 			final TableSpecification targetTable,
 			final List<Column> targetColumns) {
-		ForeignKey foreignKey = null;
+		if ( StringHelper.isEmpty( foreignKeyName ) ) {
-		if ( foreignKeyName != null ) {
+			foreignKeyName = ForeignKey.generateName( sourceTable, targetTable, sourceColumns, targetColumns );
-			foreignKey = locateAndBindForeignKeyByName( foreignKeyName, sourceTable, sourceColumns, targetTable, targetColumns );
-		}
-		else {
-			foreignKeyName = StringHelper.randomFixedLengthHex( ForeignKey.GENERATED_NAME_PREFIX );
 		}
+		
+		ForeignKey foreignKey = locateAndBindForeignKeyByName( foreignKeyName, sourceTable, sourceColumns, targetTable, targetColumns );
 		if ( foreignKey == null ) {
 			foreignKey = locateForeignKeyByColumnMapping( sourceTable, sourceColumns, targetTable, targetColumns );
-			if ( foreignKey != null && foreignKeyName != null ) {
+			if ( foreignKey != null ) {
 				if ( foreignKey.getName() == null ) {
 					// the foreign key name has not be initialized; set it to foreignKeyName
 					foreignKey.setName( foreignKeyName );
@@ -163,7 +161,7 @@ public class ForeignKeyHelper {
 				// The located foreign key already has columns bound;
 				// Make sure they are the same columns.
 				if ( !foreignKey.getSourceColumns().equals( sourceColumns ) ||
-						foreignKey.getTargetColumns().equals( targetColumns ) ) {
+						!foreignKey.getTargetColumns().equals( targetColumns ) ) {
 					throw binder.bindingContext().makeMappingException(
 							String.format(
 									"Attempt to bind exisitng foreign key \"%s\" with different columns.",

hibernate-core/src/main/java/org/hibernate/metamodel/internal/UniqueKeyHelper.java

@@ -23,7 +23,7 @@ package org.hibernate.metamodel.internal;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.hibernate.internal.util.StringHelper;
+import org.hibernate.metamodel.spi.relational.AbstractConstraint;
 import org.hibernate.metamodel.spi.relational.Column;
 import org.hibernate.metamodel.spi.relational.TableSpecification;
 import org.hibernate.metamodel.spi.relational.UniqueKey;
@@ -51,8 +51,11 @@ public class UniqueKeyHelper {
 			uniqueKey = naturalIdUniqueKeys.get( table );
 		}
 		else {
-			uniqueKey = table.getOrCreateUniqueKey(
+			String keyName = "UK_" + AbstractConstraint.hashedName( table.getLogicalName().getText() + "_NaturalID" ); 
-					StringHelper.randomFixedLengthHex( UniqueKey.GENERATED_NAME_PREFIX ) );
+			uniqueKey = new UniqueKey();
+			uniqueKey.setTable( table );
+			uniqueKey.setName( keyName );
+			table.addUniqueKey( uniqueKey );
 			naturalIdUniqueKeys.put( table, uniqueKey );
 		}
 		uniqueKey.addColumn( column );

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/UniqueConstraintSourceImpl.java

@@ -52,7 +52,7 @@ class UniqueConstraintSourceImpl implements UniqueConstraintSource {
 	}
 
 	@Override
-	public Iterable<String> columnNames() {
+	public List<String> columnNames() {
 		return columnNames;
 	}
 

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/AbstractConstraint.java

@@ -23,12 +23,16 @@
  */
 package org.hibernate.metamodel.spi.relational;
 
+import java.math.BigInteger;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.hibernate.HibernateException;
 import org.hibernate.dialect.Dialect;
 
 /**
@@ -40,7 +44,7 @@ import org.hibernate.dialect.Dialect;
  * @author Gail Badner
  */
 public abstract class AbstractConstraint implements Constraint {
-	private final TableSpecification table;
+	private TableSpecification table;
 	private String name;
 	private final Map<Identifier, Column> columnMap = new LinkedHashMap<Identifier, Column>();
 
@@ -54,6 +58,10 @@ public abstract class AbstractConstraint implements Constraint {
 		return table;
 	}
 	
+	public void setTable( TableSpecification table ) {
+		this.table = table;
+	}
+
 	/**
 	 * Returns the constraint name, or null if the name has not been set.
 	 *
@@ -88,6 +96,33 @@ public abstract class AbstractConstraint implements Constraint {
 		this.name = name;
 	}
 
+	/**
+	 * Hash a constraint name using MD5. Convert the MD5 digest to base 35
+	 * (full alphanumeric), guaranteeing
+	 * that the length of the name will always be smaller than the 30
+	 * character identifier restriction enforced by a few dialects.
+	 * 
+	 * @param s
+	 *            The name to be hashed.
+	 * @return String The hased name.
+	 */
+	public static String hashedName(String s) {
+		try {
+			MessageDigest md = MessageDigest.getInstance( "MD5" );
+			md.reset();
+			md.update( s.getBytes() );
+			byte[] digest = md.digest();
+			BigInteger bigInt = new BigInteger( 1, digest );
+			// By converting to base 35 (full alphanumeric), we guarantee
+			// that the length of the name will always be smaller than the 30
+			// character identifier restriction enforced by a few dialects.
+			return bigInt.toString( 35 );
+		}
+		catch ( NoSuchAlgorithmException e ) {
+			throw new HibernateException( "Unable to generate a hashed Constraint name!", e );
+		}
+	}
+
 	protected int generateConstraintColumnListId() {
 		return table.generateColumnListId( getColumns() );
 	}

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/ForeignKey.java

@@ -24,15 +24,16 @@
 package org.hibernate.metamodel.spi.relational;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 
-import org.jboss.logging.Logger;
-
 import org.hibernate.AssertionFailure;
 import org.hibernate.MappingException;
 import org.hibernate.dialect.Dialect;
+import org.jboss.logging.Logger;
 
 /**
  * Models the notion of a foreign key.
@@ -42,11 +43,10 @@ import org.hibernate.dialect.Dialect;
  *
  * @author Gavin King
  * @author Steve Ebersole
+ * @author Brett Meyer
  */
 public class ForeignKey extends AbstractConstraint {
 	
-	public static final String GENERATED_NAME_PREFIX = "FK";
-	
     private static final Logger LOG = Logger.getLogger( ForeignKey.class );
 
 	private static final String ON_DELETE = " on delete ";
@@ -137,11 +137,6 @@ public class ForeignKey extends AbstractConstraint {
 		}
 	}
 
-	@Override
-	public String getExportIdentifier() {
-		return getSourceTable().getLoggableValueQualifier() + ".FK-" + getName();
-	}
-
 	public ReferentialAction getDeleteRule() {
 		return deleteRule;
 	}
@@ -258,4 +253,54 @@ public class ForeignKey extends AbstractConstraint {
 		}
 		return columnMappingList;
 	}
+	
+	/**
+	 * If a constraint is not explicitly named, this is called to generate
+	 * a unique hash using the source/target table and column names.
+	 * Static so the name can be generated prior to creating the Constraint.
+	 * They're cached, keyed by name, in multiple locations.
+	 * 
+	 * @param sourceTable
+	 * @param targetTable
+	 * @param sourceColumns
+	 * @param targetColumns
+	 * @return String The generated name
+	 */
+	public static String generateName(TableSpecification sourceTable, TableSpecification targetTable,
+			List<Column> sourceColumns, List<Column> targetColumns) {
+		// Use a concatenation that guarantees uniqueness, even if identical names
+		// exist between all table and column identifiers.
+
+		StringBuilder sb = new StringBuilder( "table`" + sourceTable.getLogicalName().getText() + "`" );
+		sb.append( "table`" + targetTable.getLogicalName().getText() + "`" );
+		appendColumns( sourceColumns, sb );
+		appendColumns( targetColumns, sb );
+
+		return "FK_" + hashedName( sb.toString() );
+	}
+
+	private static void appendColumns(List<Column> columns, StringBuilder sb) {
+		// Ensure a consistent ordering of columns, regardless of the order
+		// they were bound.
+		// Clone the list, as sometimes a set of order-dependent Column
+		// bindings are given.
+		Column[] alphabeticalColumns = columns.toArray( new Column[columns.size()] );
+		Arrays.sort( alphabeticalColumns, ColumnComparator.INSTANCE );
+		for ( Column column : alphabeticalColumns ) {
+			sb.append( "column`" + column.getColumnName().getText() + "`" );
+		}
+	}
+
+	private static class ColumnComparator implements Comparator<Column> {
+		public static ColumnComparator INSTANCE = new ColumnComparator();
+
+		public int compare(Column col1, Column col2) {
+			return col1.getColumnName().toString().compareTo( col2.getColumnName().toString() );
+		}
+	}
+	
+	@Override
+	public String getExportIdentifier() {
+		return getSourceTable().getLoggableValueQualifier() + ".FK-" + getName();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/InLineView.java

@@ -79,7 +79,7 @@ public class InLineView extends AbstractTableSpecification {
 	}
 
 	@Override
-	public UniqueKey getOrCreateUniqueKey(String name) {
+	public void addUniqueKey(UniqueKey uk) {
 		throw new UnsupportedOperationException( "Cannot create unique-key on inline view" );
 	}
 

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/Index.java

@@ -33,22 +33,10 @@ import org.hibernate.dialect.Dialect;
  */
 public class Index extends AbstractConstraint {
 	
-	public static final String GENERATED_NAME_PREFIX = "IDX";
-	
 	protected Index(Table table, String name) {
 		super( table, name );
 	}
 	
-	@Override
-	public String getExportIdentifier() {
-		StringBuilder sb = new StringBuilder( getTable().getLoggableValueQualifier());
-		sb.append( ".IDX" );
-		for ( Column column : getColumns() ) {
-			sb.append( '_' ).append( column.getColumnName().getText() );
-		}
-		return sb.toString();
-	}
-	
 	@Override
 	public String sqlConstraintStringInAlterTable(Dialect dialect) {
 		StringBuilder buf = new StringBuilder( " index (" );
@@ -64,4 +52,14 @@ public class Index extends AbstractConstraint{
 		}
 		return buf.append( ')' ).toString();
 	}
+	
+	@Override
+	public String getExportIdentifier() {
+		StringBuilder sb = new StringBuilder( getTable().getLoggableValueQualifier() );
+		sb.append( ".IDX" );
+		for ( Column column : getColumns() ) {
+			sb.append( '_' ).append( column.getColumnName().getText() );
+		}
+		return sb.toString();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/PrimaryKey.java

@@ -36,17 +36,10 @@ import org.hibernate.dialect.Dialect;
  */
 public class PrimaryKey extends AbstractConstraint {
 
-	public static final String GENERATED_NAME_PREFIX = "PK";
-
 	protected PrimaryKey(TableSpecification table) {
 		super( table, null );
 	}
 
-	@Override
-	public String getExportIdentifier() {
-		return getTable().getLoggableValueQualifier() + ".PK";
-	}
-
 	public String sqlConstraintStringInCreateTable(Dialect dialect) {
 		StringBuilder buf = new StringBuilder("primary key (");
 		boolean first = true;
@@ -79,4 +72,9 @@ public class PrimaryKey extends AbstractConstraint {
 		return buf.append(')').toString();
 	}
 	
+	@Override
+	public String getExportIdentifier() {
+		return getTable().getLoggableValueQualifier() + ".PK";
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/Table.java

@@ -164,19 +164,9 @@ public class Table extends AbstractTableSpecification implements Exportable {
 		return Collections.unmodifiableSet( uniqueKeys );
 	}
 
-	// TODO: The "get" part of this should probably go away -- all callers
-	// should now be creating only.
 	@Override
-	public UniqueKey getOrCreateUniqueKey(String name) {
+	public void addUniqueKey(UniqueKey uk) {
-		UniqueKey result = null;
+		uniqueKeys.add( uk );
-		if ( name != null ) {
-			result = locateConstraint( uniqueKeys, name );
-		}
-		if ( result == null ) {
-			result = new UniqueKey( this, name );
-			uniqueKeys.add( result );
-		}
-		return result;
 	}
 	
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/TableSpecification.java

@@ -140,7 +140,7 @@ public interface TableSpecification extends ValueContainer, Loggable {
 
 	public Iterable<UniqueKey> getUniqueKeys();
 
-	public UniqueKey getOrCreateUniqueKey(String name);
+	public void addUniqueKey(UniqueKey uk);
 	
 	public boolean hasUniqueKey(Column column);
 

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/UniqueKey.java

@@ -23,6 +23,9 @@
  */
 package org.hibernate.metamodel.spi.relational;
 
+import java.util.Arrays;
+import java.util.List;
+
 import org.hibernate.dialect.Dialect;
 
 /**
@@ -30,25 +33,18 @@ import org.hibernate.dialect.Dialect;
  *
  * @author Gavin King
  * @author Steve Ebersole
+ * @author Brett Meyer
  */
 public class UniqueKey extends AbstractConstraint {
 
-	public static final String GENERATED_NAME_PREFIX = "UK";
+	public UniqueKey() {
+		super( null, null );
+	}
 
 	protected UniqueKey(Table table, String name) {
 		super( table, name );
 	}
 
-	@Override
-	public String getExportIdentifier() {
-		StringBuilder sb = new StringBuilder( getTable().getLoggableValueQualifier() );
-		sb.append( ".UK" );
-		for ( Column column : getColumns() ) {
-			sb.append( '_' ).append( column.getColumnName().getText() );
-		}
-		return sb.toString();
-	}
-
 	@Override
 	public String[] sqlCreateStrings(Dialect dialect) {
 		return new String[] { dialect.getUniqueDelegate().getAlterTableToAddUniqueKeyCommand( this ) };
@@ -64,4 +60,65 @@ public class UniqueKey extends AbstractConstraint {
 		// not used
 		return "";
 	}
+	
+	/**
+	 * If a constraint is not explicitly named, this is called to generate
+	 * a unique hash using the table and column names.
+	 * Static so the name can be generated prior to creating the Constraint.
+	 * They're cached, keyed by name, in multiple locations.
+	 * 
+	 * @param table
+	 * @param columnNames
+	 * @return String The generated name
+	 */
+	public static String generateName(TableSpecification table, String... columnNames) {
+		// Use a concatenation that guarantees uniqueness, even if identical names
+		// exist between all table and column identifiers.
+
+		StringBuilder sb = new StringBuilder( "table`" + table.getLogicalName().getText() + "`" );
+
+		// Ensure a consistent ordering of columns, regardless of the order
+		// they were bound.
+		// Clone the list, as sometimes a set of order-dependent Column
+		// bindings are given.
+		String[] alphabeticalColumns = columnNames.clone();
+		Arrays.sort( alphabeticalColumns );
+		for ( String columnName : alphabeticalColumns ) {
+			sb.append( "column`" + columnName + "`" );
+		}
+		return "UK_" + hashedName( sb.toString() );
+	}
+
+	/**
+	 * Helper method for {@link #generateName(String, TableSpecification, String...)}.
+	 * 
+	 * @param table
+	 * @param columns
+	 * @return String The generated name
+	 */
+	public static String generateName(TableSpecification table, List<Column> columns) {
+		String[] columnNames = new String[columns.size()];
+		for ( int i = 0; i < columns.size(); i++ ) {
+			columnNames[i] = columns.get( i ).getColumnName().getText();
+		}
+		return generateName( table, columnNames );
+	}
+
+	/**
+	 * Helper method for {@link #generateName(String, TableSpecification, String...)}.
+	 * Generates and sets a name for this existing constraint.
+	 */
+	public void generateName() {
+		setName( generateName( getTable(), getColumns() ) );
+	}
+	
+	@Override
+	public String getExportIdentifier() {
+		StringBuilder sb = new StringBuilder( getTable().getLoggableValueQualifier() );
+		sb.append( ".UK" );
+		for ( Column column : getColumns() ) {
+			sb.append( '_' ).append( column.getColumnName().getText() );
+		}
+		return sb.toString();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/spi/source/ConstraintSource.java

@@ -23,6 +23,8 @@
  */
 package org.hibernate.metamodel.spi.source;
 
+import java.util.List;
+
 /**
  * Contract describing source of table constraints
  *
@@ -44,5 +46,5 @@ public interface ConstraintSource {
 	/**
 	 * @return returns the names of the column which are part of this constraint
 	 */
-	Iterable<String> columnNames();
+	public List<String> columnNames();
 }