HHH-8217 Make generated constraint names short and non-random

Conflicts:
	hibernate-core/src/main/java/org/hibernate/cfg/AnnotationBinder.java
	hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java
	hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java
	hibernate-core/src/main/java/org/hibernate/mapping/UniqueKey.java

hibernate-core/src/main/java/org/hibernate/cfg/AnnotationBinder.java

@@ -35,6 +35,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
+
 import javax.persistence.Basic;
 import javax.persistence.Cacheable;
 import javax.persistence.CollectionTable;
@@ -81,8 +82,6 @@ import javax.persistence.TableGenerator;
 import javax.persistence.UniqueConstraint;
 import javax.persistence.Version;
 
-import org.jboss.logging.Logger;
-
 import org.hibernate.AnnotationException;
 import org.hibernate.AssertionFailure;
 import org.hibernate.EntityMode;
@@ -155,9 +154,9 @@ import org.hibernate.id.SequenceHiLoGenerator;
 import org.hibernate.id.TableHiLoGenerator;
 import org.hibernate.id.enhanced.SequenceStyleGenerator;
 import org.hibernate.internal.CoreMessageLogger;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.Any;
 import org.hibernate.mapping.Component;
+import org.hibernate.mapping.Constraint;
 import org.hibernate.mapping.DependantValue;
 import org.hibernate.mapping.IdGenerator;
 import org.hibernate.mapping.Join;
@@ -171,6 +170,7 @@ import org.hibernate.mapping.SingleTableSubclass;
 import org.hibernate.mapping.Subclass;
 import org.hibernate.mapping.ToOne;
 import org.hibernate.mapping.UnionSubclass;
+import org.jboss.logging.Logger;
 
 /**
  * JSR 175 annotation binder which reads the annotations from classes, applies the
@@ -2093,16 +2093,22 @@ public final class AnnotationBinder {
 			}
 		}
 
+		// Natural ID columns must reside in one single UniqueKey within the Table.
+		// For now, simply ensure consistent naming.
+		// TODO: AFAIK, there really isn't a reason for these UKs to be created
+		// on the secondPass.  This whole area should go away...
 		NaturalId naturalIdAnn = property.getAnnotation( NaturalId.class );
 		if ( naturalIdAnn != null ) {
 			if ( joinColumns != null ) {
 				for ( Ejb3Column column : joinColumns ) {
-					column.addUniqueKey( column.getTable().getNaturalIdUniqueKeyName(), inSecondPass );
+					String keyName = "UK_" + Constraint.hashedName( column.getTable().getName() + "_NaturalID" );
+					column.addUniqueKey( keyName, inSecondPass );
 				}
 			}
 			else {
 				for ( Ejb3Column column : columns ) {
-					column.addUniqueKey( column.getTable().getNaturalIdUniqueKeyName(), inSecondPass );
+					String keyName = "UK_" + Constraint.hashedName( column.getTable().getName() + "_NaturalID" );
+					column.addUniqueKey( keyName, inSecondPass );
 				}
 			}
 		}

hibernate-core/src/main/java/org/hibernate/cfg/Configuration.java

@@ -113,6 +113,7 @@ import org.hibernate.internal.util.xml.XmlDocumentImpl;
 import org.hibernate.mapping.AuxiliaryDatabaseObject;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Column;
+import org.hibernate.mapping.Constraint;
 import org.hibernate.mapping.DenormalizedTable;
 import org.hibernate.mapping.FetchProfile;
 import org.hibernate.mapping.ForeignKey;
@@ -1391,10 +1392,7 @@ public class Configuration implements Serializable {
 			final Table table = tableListEntry.getKey();
 			final List<UniqueConstraintHolder> uniqueConstraints = tableListEntry.getValue();
 			for ( UniqueConstraintHolder holder : uniqueConstraints ) {
-				final String keyName = StringHelper.isEmpty( holder.getName() )
+				buildUniqueKeyFromColumnNames( table, holder.getName(), holder.getColumns() );
-						? StringHelper.randomFixedLengthHex("UK_")
-						: holder.getName();
-				buildUniqueKeyFromColumnNames( table, keyName, holder.getColumns() );
 			}
 		}
 		
@@ -1553,8 +1551,6 @@ public class Configuration implements Serializable {
 	}
 
 	private void buildUniqueKeyFromColumnNames(Table table, String keyName, String[] columnNames) {
-		keyName = normalizer.normalizeIdentifierQuoting( keyName );
-
 		int size = columnNames.length;
 		Column[] columns = new Column[size];
 		Set<Column> unbound = new HashSet<Column>();
@@ -1571,6 +1567,12 @@ public class Configuration implements Serializable {
 				unboundNoLogical.add( new Column( column ) );
 			}
 		}
+		
+		if ( StringHelper.isEmpty( keyName ) ) {
+			keyName = Constraint.generateName( "UK_", table, columns );
+		}
+		keyName = normalizer.normalizeIdentifierQuoting( keyName );
+		
 		UniqueKey uk = table.getOrCreateUniqueKey( keyName );
 		for ( Column column : columns ) {
 			if ( table.containsColumn( column ) ) {

hibernate-core/src/main/java/org/hibernate/cfg/HbmBinder.java

@@ -59,6 +59,7 @@ import org.hibernate.mapping.Bag;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.Column;
 import org.hibernate.mapping.Component;
+import org.hibernate.mapping.Constraint;
 import org.hibernate.mapping.DependantValue;
 import org.hibernate.mapping.FetchProfile;
 import org.hibernate.mapping.Fetchable;
@@ -2246,7 +2247,6 @@ public final class HbmBinder {
 			}
 			else if ( "natural-id".equals( name ) ) {
 				UniqueKey uk = new UniqueKey();
-				uk.setName(StringHelper.randomFixedLengthHex("UK_"));
 				uk.setTable(table);
 				//by default, natural-ids are "immutable" (constant)
 				boolean mutableId = "true".equals( subnode.attributeValue("mutable") );
@@ -2260,6 +2260,8 @@ public final class HbmBinder {
 						false,
 						true
 					);
+				uk.setName( Constraint.generateName( uk.generatedConstraintNamePrefix(),
+						table, uk.getColumns() ) );
 				table.addUniqueKey(uk);
 			}
 			else if ( "query".equals(name) ) {

hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java

@@ -30,7 +30,6 @@ import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Iterator;
 import java.util.StringTokenizer;
-import java.util.UUID;
 
 import org.hibernate.dialect.Dialect;
 import org.hibernate.internal.util.collections.ArrayHelper;
@@ -762,18 +761,4 @@ public final class StringHelper {
 	public static String[] toArrayElement(String s) {
 		return ( s == null || s.length() == 0 ) ? new String[0] : new String[] { s };
 	}
-
-	// Oracle restricts identifier lengths to 30.  Rather than tie this to
-	// Dialect, simply restrict randomly-generated constrain names across
-	// the board.
-	private static final int MAX_NAME_LENGTH = 30;
-	public static String randomFixedLengthHex(String prefix) {
-		int length = MAX_NAME_LENGTH - prefix.length();
-		String s = UUID.randomUUID().toString();
-		s = s.replace( "-", "" );
-		if (s.length() > length) {
-			s = s.substring( 0, length );
-		}
-		return prefix + s;
-	}
 }

hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java

@@ -23,10 +23,16 @@
  */
 package org.hibernate.mapping;
 import java.io.Serializable;
+import java.math.BigInteger;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 
+import org.hibernate.HibernateException;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.spi.Mapping;
 
@@ -34,11 +40,12 @@ import org.hibernate.engine.spi.Mapping;
  * A relational constraint.
  *
  * @author Gavin King
+ * @author Brett Meyer
  */
 public abstract class Constraint implements RelationalModel, Serializable {
 
 	private String name;
-	private final List columns = new ArrayList();
+	private final ArrayList columns = new ArrayList();
 	private Table table;
 
 	public String getName() {
@@ -49,6 +56,85 @@ public abstract class Constraint implements RelationalModel, Serializable {
 		this.name = name;
 	}
 	
+	/**
+	 * 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 prefix
+	 *            Appended to the beginning of the generated name
+	 * @param table
+	 * @param columns
+	 * @return String The generated name
+	 */
+	public static String generateName(String prefix, Table table, Column... columns) {
+		// Use a concatenation that guarantees uniqueness, even if identical names
+		// exist between all table and column identifiers.
+
+		StringBuilder sb = new StringBuilder( "table`" + table.getName() + "`" );
+
+		// 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.clone();
+		Arrays.sort( alphabeticalColumns, ColumnComparator.INSTANCE );
+		for ( Column column : alphabeticalColumns ) {
+			String columnName = column == null ? "" : column.getName();
+			sb.append( "column`" + columnName + "`" );
+		}
+		return prefix + hashedName( sb.toString() );
+	}
+
+	/**
+	 * Helper method for {@link #generateName(String, Table, Column...)}.
+	 * 
+	 * @param prefix
+	 *            Appended to the beginning of the generated name
+	 * @param table
+	 * @param columns
+	 * @return String The generated name
+	 */
+	public static String generateName(String prefix, Table table, List<Column> columns) {
+		return generateName( prefix, table, columns.toArray( new Column[columns.size()] ) );
+	}
+
+	/**
+	 * 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 );
+		}
+	}
+
+	private static class ColumnComparator implements Comparator<Column> {
+		public static ColumnComparator INSTANCE = new ColumnComparator();
+
+		public int compare(Column col1, Column col2) {
+			return col1.getName().compareTo( col2.getName() );
+		}
+	}
+
 	public Iterator getColumnIterator() {
 		return columns.iterator();
 	}
@@ -133,4 +219,10 @@ public abstract class Constraint implements RelationalModel, Serializable {
 	public String toString() {
 		return getClass().getName() + '(' + getTable().getName() + getColumns() + ") as " + name;
 	}
+	
+	/**
+	 * @return String The prefix to use in generated constraint names.  Examples:
+	 * "UK_", "FK_", and "PK_".
+	 */
+	public abstract String generatedConstraintNamePrefix();
 }

hibernate-core/src/main/java/org/hibernate/mapping/ForeignKey.java

@@ -180,4 +180,8 @@ public class ForeignKey extends Constraint {
 		}
 		
 	}
+	
+	public String generatedConstraintNamePrefix() {
+		return "FK_";
+	}
 }

hibernate-core/src/main/java/org/hibernate/mapping/PrimaryKey.java

@@ -53,4 +53,8 @@ public class PrimaryKey extends Constraint {
 		}
 		return buf.append(')').toString();
 	}
+	
+	public String generatedConstraintNamePrefix() {
+		return "PK_";
+	}
 }

hibernate-core/src/main/java/org/hibernate/mapping/Table.java

@@ -36,7 +36,6 @@ import org.hibernate.HibernateException;
 import org.hibernate.MappingException;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.spi.Mapping;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.tool.hbm2ddl.ColumnMetadata;
 import org.hibernate.tool.hbm2ddl.TableMetadata;
 
@@ -70,15 +69,6 @@ public class Table implements RelationalModel, Serializable {
 	private boolean hasDenormalizedTables = false;
 	private String comment;
 	
-	/**
-	 * Natural ID columns must reside in one single UniqueKey within the Table.
-	 * To prevent separate UniqueKeys from being created, this keeps track of
-	 * a sole name used for all of them.  It's necessary since
-	 * AnnotationBinder#processElementAnnotations (static) creates the
-	 * UniqueKeys on a second pass using randomly-generated names.
-	 */
-	private final String naturalIdUniqueKeyName = StringHelper.randomFixedLengthHex( "UK_" );
-
 	static class ForeignKeyKey implements Serializable {
 		String referencedClassName;
 		List columns;
@@ -431,8 +421,8 @@ public class Table implements RelationalModel, Serializable {
 				}
 
 				if ( column.isUnique() ) {
-					UniqueKey uk = getOrCreateUniqueKey( 
+					String keyName = Constraint.generateName( "UK_", this, column );
-							StringHelper.randomFixedLengthHex("UK_"));
+					UniqueKey uk = getOrCreateUniqueKey( keyName );
 					uk.addColumn( column );
 					alter.append( dialect.getUniqueDelegate()
 							.applyUniqueToColumn( column ) );
@@ -534,8 +524,8 @@ public class Table implements RelationalModel, Serializable {
 			}
 			
 			if ( col.isUnique() ) {
-				UniqueKey uk = getOrCreateUniqueKey( 
+				String keyName = Constraint.generateName( "UK_", this, col );
-						StringHelper.randomFixedLengthHex("UK_"));
+				UniqueKey uk = getOrCreateUniqueKey( keyName );
 				uk.addColumn( col );
 				buf.append( dialect.getUniqueDelegate()
 						.applyUniqueToColumn( col ) );
@@ -631,7 +621,7 @@ public class Table implements RelationalModel, Serializable {
 	}
 
 	public UniqueKey createUniqueKey(List keyColumns) {
-		String keyName = StringHelper.randomFixedLengthHex("UK_");
+		String keyName = Constraint.generateName( "UK_", this, keyColumns );
 		UniqueKey uk = getOrCreateUniqueKey( keyName );
 		uk.addColumns( keyColumns.iterator() );
 		return uk;
@@ -667,19 +657,22 @@ public class Table implements RelationalModel, Serializable {
 		ForeignKey fk = (ForeignKey) foreignKeys.get( key );
 		if ( fk == null ) {
 			fk = new ForeignKey();
-			if ( keyName != null ) {
-				fk.setName( keyName );
-			}
-			else {
-				fk.setName( StringHelper.randomFixedLengthHex("FK_") );
-			}
 			fk.setTable( this );
-			foreignKeys.put( key, fk );
 			fk.setReferencedEntityName( referencedEntityName );
 			fk.addColumns( keyColumns.iterator() );
 			if ( referencedColumns != null ) {
 				fk.addReferencedColumns( referencedColumns.iterator() );
 			}
+			
+			if ( keyName != null ) {
+				fk.setName( keyName );
+			}
+			else {
+				fk.setName( Constraint.generateName( fk.generatedConstraintNamePrefix(),
+						this, keyColumns ) );
+			}
+			
+			foreignKeys.put( key, fk );
 		}
 
 		if ( keyName != null ) {
@@ -827,10 +820,6 @@ public class Table implements RelationalModel, Serializable {
 		return checkConstraints.iterator();
 	}
 
-	public String getNaturalIdUniqueKeyName() {
-		return naturalIdUniqueKeyName;
-	}
-
 	public Iterator sqlCommentStrings(Dialect dialect, String defaultCatalog, String defaultSchema) {
 		List comments = new ArrayList();
 		if ( dialect.supportsCommentOn() ) {

hibernate-core/src/main/java/org/hibernate/mapping/UniqueKey.java

@@ -57,4 +57,7 @@ public class UniqueKey extends Constraint {
 				this, defaultCatalog, defaultSchema );
 	}
 	
+	public String generatedConstraintNamePrefix() {
+		return "UK_";
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/schemavalidation/SynonymValidationTest.java

@@ -0,0 +1,187 @@
+/* 
+ * Hibernate, Relational Persistence for Idiomatic Java
+ * 
+ * JBoss, Home of Professional Open Source
+ * Copyright 2013 Red Hat Inc. and/or its affiliates and other contributors
+ * as indicated by the @authors tag. All rights reserved.
+ * See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU Lesser General Public License, v. 2.1.
+ * This program is distributed in the hope that it will be useful, but WITHOUT A
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more details.
+ * You should have received a copy of the GNU Lesser General Public License,
+ * v.2.1 along with this distribution; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA  02110-1301, USA.
+ */
+package org.hibernate.test.schemavalidation;
+
+import java.io.Serializable;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.Table;
+
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.cfg.Configuration;
+import org.hibernate.dialect.Oracle9iDialect;
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+import org.hibernate.tool.hbm2ddl.SchemaValidator;
+import org.junit.Test;
+
+/**
+ * @author Brett Meyer
+ */
+@RequiresDialect( Oracle9iDialect.class )
+public class SynonymValidationTest extends BaseUnitTestCase {
+	
+	@Test
+	public void testSynonymValidation() {
+//		Session s = openSession();
+//		s.getTransaction().begin();
+//		s.createSQLQuery( "CREATE SYNONYM test_synonym FOR test_entity" ).executeUpdate();
+//		s.getTransaction().commit();
+//		s.close();
+		
+		Configuration cfg = new Configuration();
+//		cfg.addAnnotatedClass( TestEntityWithSynonym.class );
+		cfg.addAnnotatedClass( TestEntity.class );
+		cfg.setProperty( AvailableSettings.ENABLE_SYNONYMS, "true" );
+		cfg.setProperty( "hibernate.connection.includeSynonyms", "true" );
+		cfg.getProperties().put( "includeSynonyms", true );
+		
+//		SchemaValidator schemaValidator = new SchemaValidator( serviceRegistry(), cfg );
+		SchemaValidator schemaValidator = new SchemaValidator( cfg );
+		schemaValidator.validate();
+		
+//		s = openSession();
+//		s.getTransaction().begin();
+//		s.createSQLQuery( "DROP SYNONYM test_synonym FORCE" ).executeUpdate();
+//		s.getTransaction().commit();
+//		s.close();
+	}
+	
+//	protected Class<?>[] getAnnotatedClasses() {
+//		return new Class<?>[] { TestEntity.class };
+//	}
+	
+	@Entity
+	@Table(name = "TEST_SYN")
+	private static class TestEntity implements Serializable {
+
+	    private static final long serialVersionUID = 1L;
+	    
+	    @Id
+	    @GeneratedValue
+	    private Long id;
+	    
+	    @Column(nullable = false)
+	    private String key;
+	    private String value;
+
+	    public Long getId() {
+	        return id;
+	    }
+
+	    public void setId(Long id) {
+	        this.id = id;
+	    }
+
+	    public String getKey() {
+	        return key;
+	    }
+
+	    public void setKey(String key) {
+	        this.key = key;
+	    }
+
+	    public String getValue() {
+	        return value;
+	    }
+
+	    public void setValue(String value) {
+	        this.value = value;
+	    }
+	}
+	
+//	@Entity
+//	@Table(name = "test_entity")
+//	private static class TestEntity {
+//	    @Id
+//	    @GeneratedValue
+//	    private Long id;
+//	    
+//	    @Column(nullable = false)
+//	    private String key;
+//	    
+//	    private String value;
+//
+//	    public Long getId() {
+//	        return id;
+//	    }
+//
+//	    public void setId(Long id) {
+//	        this.id = id;
+//	    }
+//
+//	    public String getKey() {
+//	        return key;
+//	    }
+//
+//	    public void setKey(String key) {
+//	        this.key = key;
+//	    }
+//
+//	    public String getValue() {
+//	        return value;
+//	    }
+//
+//	    public void setValue(String value) {
+//	        this.value = value;
+//	    }
+//	}
+//	
+//	@Entity
+//	@Table(name = "test_synonym")
+//	private static class TestEntityWithSynonym {
+//	    @Id
+//	    @GeneratedValue
+//	    private Long id;
+//	    
+//	    @Column(nullable = false)
+//	    private String key;
+//	    
+//	    private String value;
+//
+//	    public Long getId() {
+//	        return id;
+//	    }
+//
+//	    public void setId(Long id) {
+//	        this.id = id;
+//	    }
+//
+//	    public String getKey() {
+//	        return key;
+//	    }
+//
+//	    public void setKey(String key) {
+//	        this.key = key;
+//	    }
+//
+//	    public String getValue() {
+//	        return value;
+//	    }
+//
+//	    public void setValue(String value) {
+//	        this.value = value;
+//	    }
+//	}
+}