HHH-10844 Resolve columnDefinition to appropriate sql-type for audit mappings

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/AuditMetadataGenerator.java

@@ -110,7 +110,7 @@ public final class AuditMetadataGenerator {
 		this.auditStrategy = auditStrategy;
 		this.revisionInfoRelationMapping = revisionInfoRelationMapping;
 
-		this.basicMetadataGenerator = new BasicMetadataGenerator();
+		this.basicMetadataGenerator = new BasicMetadataGenerator( this );
 		this.componentMetadataGenerator = new ComponentMetadataGenerator( this );
 		this.idMetadataGenerator = new IdMetadataGenerator( this );
 		this.toOneRelationMetadataGenerator = new ToOneRelationMetadataGenerator( this );
@@ -460,7 +460,7 @@ public final class AuditMetadataGenerator {
 			}
 
 			final Element joinKey = joinElement.addElement( "key" );
-			MetadataTools.addColumns( joinKey, join.getKey().getColumnIterator() );
+			MetadataTools.addColumns( joinKey, join.getKey().getColumnIterator(), metadata );
 			MetadataTools.addColumn( joinKey, verEntCfg.getRevisionFieldName(), null, null, null, null, null, null );
 		}
 	}
@@ -509,7 +509,7 @@ public final class AuditMetadataGenerator {
 		if ( pc.getDiscriminator() != null ) {
 			final Element discriminatorElement = classMapping.addElement( "discriminator" );
 			// Database column or SQL formula allowed to distinguish entity types
-			MetadataTools.addColumnsOrFormulas( discriminatorElement, pc.getDiscriminator().getColumnIterator() );
+			MetadataTools.addColumnsOrFormulas( discriminatorElement, pc.getDiscriminator().getColumnIterator(), metadata );
 			discriminatorElement.addAttribute( "type", pc.getDiscriminator().getType().getName() );
 		}
 
@@ -633,7 +633,7 @@ public final class AuditMetadataGenerator {
 
 				// Adding the "key" element with all id columns...
 				final Element keyMapping = mappingData.getFirst().addElement( "key" );
-				MetadataTools.addColumns( keyMapping, pc.getTable().getPrimaryKey().columnIterator() );
+				MetadataTools.addColumns( keyMapping, pc.getTable().getPrimaryKey().columnIterator(), metadata );
 
 				// ... and the revision number column, read from the revision info relation mapping.
 				keyMapping.add( (Element) cloneAndSetupRevisionInfoRelationMapping().element( "column" ).clone() );

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/BasicMetadataGenerator.java

@@ -8,6 +8,7 @@ package org.hibernate.envers.configuration.internal.metadata;
 
 import java.util.Properties;
 
+import org.hibernate.boot.Metadata;
 import org.hibernate.envers.configuration.internal.metadata.reader.PropertyAuditingData;
 import org.hibernate.envers.internal.entities.PropertyData;
 import org.hibernate.envers.internal.entities.mapper.SimpleMapperBuilder;
@@ -28,6 +29,12 @@ import org.dom4j.Element;
  */
 public final class BasicMetadataGenerator {
 
+	private final Metadata metadata;
+
+	public BasicMetadataGenerator(AuditMetadataGenerator mainGenerator) {
+		this.metadata = mainGenerator.getMetadata();
+	}
+
 	boolean addBasic(
 			Element parent,
 			PropertyAuditingData propertyAuditingData,
@@ -109,7 +116,7 @@ public final class BasicMetadataGenerator {
 				key
 		);
 
-		MetadataTools.addColumns( propMapping, value.getColumnIterator() );
+		MetadataTools.addColumns( propMapping, value.getColumnIterator(), metadata );
 
 		return propMapping;
 	}

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/IdMetadataGenerator.java

@@ -313,7 +313,7 @@ public final class IdMetadataGenerator {
 		// schema and the base table schema when a @ManyToOne is present in an identifier.
 		manyToOneElement.addAttribute( "foreign-key", "none" );
 
-		MetadataTools.addColumns( manyToOneElement, value.getColumnIterator() );
+		MetadataTools.addColumns( manyToOneElement, value.getColumnIterator(), mainGenerator.getMetadata() );
 
 		return true;
 	}

hibernate-envers/src/main/java/org/hibernate/envers/configuration/internal/metadata/MetadataTools.java

@@ -9,6 +9,10 @@ package org.hibernate.envers.configuration.internal.metadata;
 import java.util.Iterator;
 import javax.persistence.JoinColumn;
 
+import org.hibernate.boot.Metadata;
+import org.hibernate.dialect.Dialect;
+import org.hibernate.engine.spi.Mapping;
+import org.hibernate.envers.internal.EnversMessageLogger;
 import org.hibernate.envers.internal.tools.StringTools;
 import org.hibernate.mapping.Column;
 import org.hibernate.mapping.Formula;
@@ -18,12 +22,20 @@ import org.dom4j.Attribute;
 import org.dom4j.Document;
 import org.dom4j.Element;
 
+import org.jboss.logging.Logger;
+
 /**
  * @author Adam Warski (adam at warski dot org)
  * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
  * @author Michal Skowronek (mskowr at o2 dot pl)
  */
 public final class MetadataTools {
+
+	private static final EnversMessageLogger LOG = Logger.getMessageLogger(
+			EnversMessageLogger.class,
+			MetadataTools.class.getName()
+	);
+
 	private MetadataTools() {
 	}
 
@@ -285,38 +297,68 @@ public final class MetadataTools {
 		return joinMapping;
 	}
 
-	public static void addColumns(Element anyMapping, Iterator selectables) {
+	public static void addColumns(Element anyMapping, Iterator<?> selectables, Metadata metadata) {
+		addColumns( anyMapping, selectables, metadata, metadata.getDatabase().getDialect() );
+	}
+
+	public static void addColumns(Element anyMapping, Iterator<?> selectables, Mapping mapping, Dialect dialect) {
 		while ( selectables.hasNext() ) {
 			final Selectable selectable = (Selectable) selectables.next();
 			if ( selectable.isFormula() ) {
 				throw new FormulaNotSupportedException();
 			}
-			addColumn( anyMapping, (Column) selectable );
+			addColumn( anyMapping, (Column) selectable, mapping, dialect );
 		}
 	}
 
 	/**
-	 * Adds <code>column</code> element with the following attributes (unless empty): <code>name</code>,
+	 * Adds {@code column} element with the following attributes (unless empty):
-	 * <code>length</code>, <code>scale</code>, <code>precision</code>, <code>sql-type</code>, <code>read</code>
+	 * <ul>
-	 * and <code>write</code>.
+	 *     <li>name</li>>
+	 *     <li>length</li>
+	 *     <li>scale</li>
+	 *     <li>precision</li>
+	 *     <li>sql-type</li>
+	 *     <li>read</li>
+	 *     <li>write</li>
 	 *
-	 * @param anyMapping Parent element.
+	 * </ul>
-	 * @param column Column descriptor.
+	 *
+	 * @param anyMapping parent element
+	 * @param column column descriptor
+	 * @param mapping the metadata mapping
+	 * @param dialect the dialect
 	 */
-	public static void addColumn(Element anyMapping, Column column) {
+	public static void addColumn(Element anyMapping, Column column, Mapping mapping, Dialect dialect) {
 		addColumn(
 				anyMapping,
 				column.getName(),
 				column.getLength(),
 				column.getScale(),
 				column.getPrecision(),
-				column.getSqlType(),
+				resolveSqlType( column, mapping, dialect ),
 				column.getCustomRead(),
 				column.getCustomWrite(),
 				column.isQuoted()
 		);
 	}
 
+	private static String resolveSqlType(Column column, Mapping mapping, Dialect dialect) {
+		String columnDefinition = column.getSqlType();
+		if ( !StringTools.isEmpty( columnDefinition ) ) {
+			final int sqlTypeCode = column.getSqlTypeCode( mapping );
+			final String sqlType = dialect.getTypeName( sqlTypeCode, column.getLength(), column.getPrecision(), column.getScale() );
+			LOG.infof(
+					"Column [%s] uses a column-definition of [%s], resolved sql-type as [%s].",
+					column.getName(),
+					columnDefinition,
+					sqlType
+			);
+			columnDefinition = sqlType;
+		}
+		return columnDefinition;
+	}
+
 	@SuppressWarnings({"unchecked"})
 	private static void changeNamesInColumnElement(Element element, ColumnNameIterator columnNameIterator) {
 		final Iterator<Element> properties = element.elementIterator();
@@ -392,12 +434,13 @@ public final class MetadataTools {
 	 * @param element Parent element.
 	 * @param columnIterator Iterator pointing at {@link org.hibernate.mapping.Column} and/or
 	 * {@link org.hibernate.mapping.Formula} objects.
+	 * @param metadata The boot-time entity model metadata
 	 */
-	public static void addColumnsOrFormulas(Element element, Iterator columnIterator) {
+	public static void addColumnsOrFormulas(Element element, Iterator columnIterator, Metadata metadata) {
 		while ( columnIterator.hasNext() ) {
 			final Object o = columnIterator.next();
 			if ( o instanceof Column ) {
-				addColumn( element, (Column) o );
+				addColumn( element, (Column) o, metadata, metadata.getDatabase().getDialect() );
 			}
 			else if ( o instanceof Formula ) {
 				addFormula( element, (Formula) o );

hibernate-envers/src/test/java/org/hibernate/envers/test/integration/basic/BasicTypeColumnDefinitionTest.java

@@ -0,0 +1,156 @@
+/*
+ * 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.envers.test.integration.basic;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+import org.hibernate.annotations.Generated;
+import org.hibernate.annotations.GenerationTime;
+import org.hibernate.dialect.H2Dialect;
+import org.hibernate.envers.Audited;
+import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase;
+import org.hibernate.envers.test.Priority;
+import org.hibernate.mapping.Table;
+import org.junit.Test;
+
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
+
+import static org.hibernate.boot.model.naming.Identifier.toIdentifier;
+import static org.hibernate.mapping.Column.DEFAULT_LENGTH;
+import static org.hibernate.mapping.Column.DEFAULT_PRECISION;
+import static org.hibernate.mapping.Column.DEFAULT_SCALE;
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test verifies that resolving a column mapping's {@code sql-type} for HBM XML is performed
+ * correctly such that when a column supplies a {@code columnDefinition}, Envers properly builds
+ * its schema based on the right type rather than directly using the column definition as-is.
+ *
+ * The following illustrate some examples of expected transformations:
+ *
+ * <li>{@code @Column(columnDefinition = "varchar(10) not null")} => {@code sql-type = "varchar(255)"}</li>
+ * <li>{@code @Column(length = 10, columnDefinition = "varchar(10) not null")} => {@code sql-type = "varchar(10)"}</li>
+ * <li>{@code @Column(columnDefinition = "integer not null auto_increment")} => {@code sql-type = "integer"}</li>
+ *
+ * It is important to point out that resolving the sql-types length/precision/scale is all based on the
+ * values supplied as part of the {@link Column} annotation itself and not what is in the definition text.
+ *
+ * @author Chris Cranford
+ */
+@TestForIssue(jiraKey = "HHH-10844")
+@RequiresDialect(value = H2Dialect.class)
+public class BasicTypeColumnDefinitionTest extends BaseEnversJPAFunctionalTestCase {
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] { BasicTypeContainer.class };
+	}
+
+	@Test
+	@Priority(10)
+	public void testMetadataBindings() {
+		final Table auditTable = metadata().getEntityBinding( BasicTypeContainer.class.getName() + "_AUD" ).getTable();
+
+		final org.hibernate.mapping.Column caseNumber = auditTable.getColumn( toIdentifier( "caseNumber" ) );
+		assertEquals( "integer", caseNumber.getSqlType() );
+		assertEquals( DEFAULT_LENGTH, caseNumber.getLength() );
+		assertEquals( DEFAULT_PRECISION, caseNumber.getPrecision() );
+		assertEquals( DEFAULT_SCALE, caseNumber.getScale() );
+
+		final org.hibernate.mapping.Column colDef = auditTable.getColumn( toIdentifier( "columnWithDefinition" ) );
+		assertEquals( "varchar(10)", colDef.getSqlType() );
+		assertEquals( 10, colDef.getLength() );
+		assertEquals( DEFAULT_PRECISION, colDef.getPrecision() );
+		assertEquals( DEFAULT_SCALE, colDef.getScale() );
+	}
+
+	@Test
+	@Priority(10)
+	public void initData() {
+		final BasicTypeContainer detachedEntity = doInJPA( this::entityManagerFactory, entityManager -> {
+			final BasicTypeContainer entity = new BasicTypeContainer();
+			entity.setData( "test" );
+			entity.setColumnWithDefinition( "1234567890" );
+			entityManager.persist( entity );
+			return entity;
+		} );
+
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			final BasicTypeContainer entity = entityManager.find( BasicTypeContainer.class, detachedEntity.getId() );
+			entity.setData( "test2" );
+			entityManager.merge( entity );
+		} );
+	}
+
+	@Test
+	public void testRevisionHistory() {
+		assertEquals( 2, getAuditReader().getRevisions( BasicTypeContainer.class, 1 ).size() );
+
+		final BasicTypeContainer rev1 = getAuditReader().find( BasicTypeContainer.class, 1, 1 );
+		assertEquals( "test", rev1.getData() );
+		assertEquals( "1234567890", rev1.getColumnWithDefinition() );
+		assertEquals( Integer.valueOf( 1 ), rev1.getCaseNumber() );
+
+		final BasicTypeContainer rev2 = getAuditReader().find( BasicTypeContainer.class, 1, 2 );
+		assertEquals( "test2", rev2.getData() );
+		assertEquals( "1234567890", rev2.getColumnWithDefinition() );
+		assertEquals( Integer.valueOf( 1 ), rev2.getCaseNumber() );
+	}
+
+	@Entity(name = "BasicTypeContainer")
+	@Audited
+	public static class BasicTypeContainer {
+		@Id
+		@GeneratedValue
+		private Integer id;
+
+		@Generated(GenerationTime.INSERT)
+		@Column(name = "caseNumber", columnDefinition = "integer not null auto_increment")
+		private Integer caseNumber;
+
+		@Column(name = "columnWithDefinition", length = 10, nullable = false, columnDefinition = "varchar(10) not null")
+		private String columnWithDefinition;
+
+		private String data;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public Integer getCaseNumber() {
+			return caseNumber;
+		}
+
+		public void setCaseNumber(Integer caseNumber) {
+			this.caseNumber = caseNumber;
+		}
+
+		public String getColumnWithDefinition() {
+			return columnWithDefinition;
+		}
+
+		public void setColumnWithDefinition(String columnWithDefinition) {
+			this.columnWithDefinition = columnWithDefinition;
+		}
+
+		public String getData() {
+			return data;
+		}
+
+		public void setData(String data) {
+			this.data = data;
+		}
+	}
+}