From 9fc22a49be07b1699e6b70cbe0a38d83dd50682d Mon Sep 17 00:00:00 2001 From: Lukasz Antoniak Date: Thu, 18 Apr 2013 11:10:08 +0200 Subject: [PATCH] HHH-7214 - Validation of duplicated discriminator values --- .../entity/SingleTableEntityPersister.java | 26 ++++-- .../DuplicatedDiscriminatorValueTest.java | 91 +++++++++++++++++++ 2 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java index e66822ed56..d5f62787c6 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java @@ -410,7 +410,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { subclassClosure = new String[subclassSpan]; subclassClosure[0] = getEntityName(); if ( persistentClass.isPolymorphic() ) { - subclassesByDiscriminatorValue.put( discriminatorValue, getEntityName() ); + addSubclassByDiscriminatorValue( discriminatorValue, getEntityName() ); } // SUBCLASSES @@ -421,15 +421,15 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { Subclass sc = (Subclass) iter.next(); subclassClosure[k++] = sc.getEntityName(); if ( sc.isDiscriminatorValueNull() ) { - subclassesByDiscriminatorValue.put( NULL_DISCRIMINATOR, sc.getEntityName() ); + addSubclassByDiscriminatorValue( NULL_DISCRIMINATOR, sc.getEntityName() ); } else if ( sc.isDiscriminatorValueNotNull() ) { - subclassesByDiscriminatorValue.put( NOT_NULL_DISCRIMINATOR, sc.getEntityName() ); + addSubclassByDiscriminatorValue( NOT_NULL_DISCRIMINATOR, sc.getEntityName() ); } else { try { DiscriminatorType dtype = (DiscriminatorType) discriminatorType; - subclassesByDiscriminatorValue.put( + addSubclassByDiscriminatorValue( dtype.stringToObject( sc.getDiscriminatorValue() ), sc.getEntityName() ); @@ -452,6 +452,16 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { } + private void addSubclassByDiscriminatorValue(Object discriminatorValue, String entityName) { + String mappedEntityName = (String) subclassesByDiscriminatorValue.put( discriminatorValue, entityName ); + if ( mappedEntityName != null ) { + throw new MappingException( + "Entities [" + entityName + "] and [" + mappedEntityName + + "] are mapped with the same discriminator value '" + discriminatorValue + "'." + ); + } + } + public SingleTableEntityPersister( final EntityBinding entityBinding, final EntityRegionAccessStrategy cacheAccessStrategy, @@ -674,7 +684,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { subclassClosure = new String[subclassSpan]; subclassClosure[0] = getEntityName(); if ( entityBinding.isPolymorphic() ) { - subclassesByDiscriminatorValue.put( discriminatorValue, getEntityName() ); + addSubclassByDiscriminatorValue( discriminatorValue, getEntityName() ); } // SUBCLASSES @@ -683,15 +693,15 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { for ( EntityBinding subEntityBinding : entityBinding.getPostOrderSubEntityBindingClosure() ) { subclassClosure[k++] = subEntityBinding.getEntity().getName(); if ( subEntityBinding.isDiscriminatorMatchValueNull() ) { - subclassesByDiscriminatorValue.put( NULL_DISCRIMINATOR, subEntityBinding.getEntity().getName() ); + addSubclassByDiscriminatorValue( NULL_DISCRIMINATOR, subEntityBinding.getEntity().getName() ); } else if ( subEntityBinding.isDiscriminatorMatchValueNotNull() ) { - subclassesByDiscriminatorValue.put( NOT_NULL_DISCRIMINATOR, subEntityBinding.getEntity().getName() ); + addSubclassByDiscriminatorValue( NOT_NULL_DISCRIMINATOR, subEntityBinding.getEntity().getName() ); } else { try { DiscriminatorType dtype = (DiscriminatorType) discriminatorType; - subclassesByDiscriminatorValue.put( + addSubclassByDiscriminatorValue( dtype.stringToObject( subEntityBinding.getDiscriminatorMatchValue() ), subEntityBinding.getEntity().getName() ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java new file mode 100644 index 0000000000..fc68f75034 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java @@ -0,0 +1,91 @@ +package org.hibernate.test.annotations.inheritance.singletable; + +import javax.persistence.DiscriminatorColumn; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.junit.Assert; +import org.junit.Test; + +import org.hibernate.MappingException; +import org.hibernate.SessionFactory; +import org.hibernate.cfg.Configuration; +import org.hibernate.service.ServiceRegistry; +import org.hibernate.testing.ServiceRegistryBuilder; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseUnitTestCase; + +/** + * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com) + */ +@TestForIssue( jiraKey = "HHH-7214" ) +public class DuplicatedDiscriminatorValueTest extends BaseUnitTestCase { + private static final String DISCRIMINATOR_VALUE = "D"; + + @Test + public void testDuplicatedDiscriminatorValueSameHierarchy() { + try { + tryBuildingSessionFactory( Building.class, Building1.class, Building2.class ); + Assert.fail( MappingException.class.getName() + " expected when two subclasses are mapped with the same discriminator value." ); + } + catch ( MappingException e ) { + final String errorMsg = e.getCause().getMessage(); + // Check if error message contains descriptive information. + Assert.assertTrue( errorMsg.contains( Building1.class.getName() ) ); + Assert.assertTrue( errorMsg.contains( Building2.class.getName() ) ); + Assert.assertTrue( errorMsg.contains( "discriminator value '" + DISCRIMINATOR_VALUE + "'." ) ); + } + } + + @Test + public void testDuplicatedDiscriminatorValueDifferentHierarchy() { + tryBuildingSessionFactory( Building.class, Building1.class, Furniture.class, Chair.class ); + } + + private void tryBuildingSessionFactory(Class... annotatedClasses) { + Configuration cfg = new Configuration(); + for ( Class annotatedClass : annotatedClasses ) { + cfg.addAnnotatedClass( annotatedClass ); + } + ServiceRegistry serviceRegistry = null; + SessionFactory sessionFactory = null; + try { + serviceRegistry = ServiceRegistryBuilder.buildServiceRegistry( cfg.getProperties() ); + sessionFactory = cfg.buildSessionFactory( serviceRegistry ); + } + finally { + if ( sessionFactory != null ) { + sessionFactory.close(); + } + if ( serviceRegistry != null ) { + ServiceRegistryBuilder.destroy( serviceRegistry ); + } + } + } + + @Entity + @DiscriminatorValue(DISCRIMINATOR_VALUE) // Duplicated discriminator value in single hierarchy. + private static class Building1 extends Building { + } + + @Entity + @DiscriminatorValue(DISCRIMINATOR_VALUE) // Duplicated discriminator value in single hierarchy. + private static class Building2 extends Building { + } + + @Entity + @DiscriminatorColumn(name = "entity_type") + @DiscriminatorValue("F") + private static class Furniture { + @Id + @GeneratedValue + private Integer id; + } + + @Entity + @DiscriminatorValue(DISCRIMINATOR_VALUE) // Duplicated discriminator value in different hierarchy. + private static class Chair extends Furniture { + } +}