From c259e157b0286b7c36859710215eca2e0da4fd4a Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 11 Apr 2013 14:28:22 -0500 Subject: [PATCH] HHH-8167 - Adding @NotNull to a @ManyToOne association with @JoinColumnsOrFormulas logs a ClassCastException --- changelog.txt | 2 +- .../CopyIdentifierComponentSecondPass.java | 14 ++++- .../cfg/beanvalidation/TypeSafeActivator.java | 30 ++++++--- .../util/collections/JoinedIterator.java | 62 ++++++++----------- .../org/hibernate/mapping/Collection.java | 5 +- .../java/org/hibernate/mapping/Component.java | 2 +- .../java/org/hibernate/mapping/OneToMany.java | 2 +- .../org/hibernate/mapping/Selectable.java | 4 ++ .../org/hibernate/mapping/SimpleValue.java | 5 +- .../java/org/hibernate/mapping/Value.java | 2 +- .../test/formulajoin/AnnotatedDetail.java | 42 +++++++++++++ ...atedFormWithBeanValidationNotNullTest.java | 44 +++++++++++++ .../test/formulajoin/AnnotatedMaster.java | 55 ++++++++++++++++ .../test/formulajoin/FormulaJoinTest.java | 3 +- 14 files changed, 216 insertions(+), 56 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedDetail.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedFormWithBeanValidationNotNullTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedMaster.java diff --git a/changelog.txt b/changelog.txt index a2fc570847..e679bf4880 100644 --- a/changelog.txt +++ b/changelog.txt @@ -4890,7 +4890,7 @@ Changes in version 0.9.5 (8.2.2002) * fixed potential bug related to cacheing of compiled queries * major rewrite of code relating to O-R mappings * Session.copy() and Session.equals() as convenience for users -* fixed repeated invocations of hasNext() on iterator + iterators now always work with distinct SQL resultsets +* fixed repeated invocations of hasNext() on iterator + wrappedIterators now always work with distinct SQL resultsets * McKoi dialect was contributed by Gabe Hicks Changes in version 0.9.4 (29.1.2002) diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/CopyIdentifierComponentSecondPass.java b/hibernate-core/src/main/java/org/hibernate/cfg/CopyIdentifierComponentSecondPass.java index 0e63016139..9dad56567f 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/CopyIdentifierComponentSecondPass.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/CopyIdentifierComponentSecondPass.java @@ -26,6 +26,8 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import org.jboss.logging.Logger; + import org.hibernate.AnnotationException; import org.hibernate.AssertionFailure; import org.hibernate.MappingException; @@ -33,12 +35,15 @@ import org.hibernate.mapping.Column; import org.hibernate.mapping.Component; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; +import org.hibernate.mapping.Selectable; import org.hibernate.mapping.SimpleValue; /** * @author Emmanuel Bernard */ public class CopyIdentifierComponentSecondPass implements SecondPass { + private static final Logger log = Logger.getLogger( CopyIdentifierComponentSecondPass.class ); + private final String referencedEntityName; private final Component component; private final Mappings mappings; @@ -113,7 +118,7 @@ public class CopyIdentifierComponentSecondPass implements SecondPass { final SimpleValue referencedValue = (SimpleValue) referencedProperty.getValue(); value.setTypeName( referencedValue.getTypeName() ); value.setTypeParameters( referencedValue.getTypeParameters() ); - final Iterator columns = referencedValue.getColumnIterator(); + final Iterator columns = referencedValue.getColumnIterator(); if ( joinColumns[0].isNameDeferred() ) { joinColumns[0].copyReferencedStructureAndCreateDefaultJoinColumns( @@ -124,7 +129,12 @@ public class CopyIdentifierComponentSecondPass implements SecondPass { else { //FIXME take care of Formula while ( columns.hasNext() ) { - Column column = columns.next(); + final Selectable selectable = columns.next(); + if ( ! Column.class.isInstance( selectable ) ) { + log.debug( "Encountered formula definition; skipping" ); + continue; + } + final Column column = (Column) selectable; final Ejb3JoinColumn joinColumn; String logicalColumnName = null; if ( isExplicitReference ) { diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/TypeSafeActivator.java b/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/TypeSafeActivator.java index fb5733adc8..07c808e746 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/TypeSafeActivator.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/TypeSafeActivator.java @@ -60,6 +60,7 @@ import org.hibernate.mapping.Column; import org.hibernate.mapping.Component; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; +import org.hibernate.mapping.Selectable; import org.hibernate.mapping.SingleTableSubclass; /** @@ -156,6 +157,7 @@ class TypeSafeActivator { } applyRelationalConstraints( + factory, activationContext.getConfiguration().createMappings().getClasses().values(), properties, activationContext.getServiceRegistry().getService( JdbcServices.class ).getDialect() @@ -163,8 +165,11 @@ class TypeSafeActivator { } @SuppressWarnings( {"UnusedDeclaration"}) - public static void applyRelationalConstraints(Collection persistentClasses, Properties properties, Dialect dialect) { - ValidatorFactory factory = getValidatorFactory( properties ); + public static void applyRelationalConstraints( + ValidatorFactory factory, + Collection persistentClasses, + Properties properties, + Dialect dialect) { Class[] groupsArray = new GroupsPerOperation( properties ).get( GroupsPerOperation.Operation.DDL ); Set> groups = new HashSet>( Arrays.asList( groupsArray ) ); @@ -305,14 +310,23 @@ class TypeSafeActivator { private static boolean applyNotNull(Property property, ConstraintDescriptor descriptor) { boolean hasNotNull = false; if ( NotNull.class.equals( descriptor.getAnnotation().annotationType() ) ) { + // single table inheritance should not be forced to null due to shared state if ( !( property.getPersistentClass() instanceof SingleTableSubclass ) ) { - //single table should not be forced to null - if ( !property.isComposite() ) { //composite should not add not-null on all columns - @SuppressWarnings( "unchecked" ) - Iterator iter = property.getColumnIterator(); + //composite should not add not-null on all columns + if ( !property.isComposite() ) { + final Iterator iter = property.getColumnIterator(); while ( iter.hasNext() ) { - iter.next().setNullable( false ); - hasNotNull = true; + final Selectable selectable = iter.next(); + if ( Column.class.isInstance( selectable ) ) { + Column.class.cast( selectable ).setNullable( false ); + } + else { + LOG.debugf( + "@NotNull was applied to attribute [%s] which is defined (at least partially) " + + "by formula(s); formula portions will be skipped", + property.getName() + ); + } } } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/JoinedIterator.java b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/JoinedIterator.java index b7a3e02085..28a910d9b7 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/JoinedIterator.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/JoinedIterator.java @@ -24,46 +24,35 @@ */ package org.hibernate.internal.util.collections; +import java.util.Collections; import java.util.Iterator; import java.util.List; /** - * An JoinedIterator is an Iterator that wraps a number of Iterators. - * - * This class makes multiple iterators look like one to the caller. - * When any method from the Iterator interface is called, the JoinedIterator - * will delegate to a single underlying Iterator. The JoinedIterator will - * invoke the Iterators in sequence until all Iterators are exhausted. + * An Iterator implementation that wraps other Iterators, and presents them all as one + * continuous Iterator. When any method from Iterator is called, we delegate to each + * wrapped Iterator in turn until all wrapped Iterators are exhausted. * + * @author Gavine King + * @author Steve Ebersole */ -public class JoinedIterator implements Iterator { +public class JoinedIterator implements Iterator { + private Iterator[] wrappedIterators; - private static final Iterator[] ITERATORS = {}; - - // wrapped iterators - private Iterator[] iterators; - - // index of current iterator in the wrapped iterators array private int currentIteratorIndex; + private Iterator currentIterator; + private Iterator lastUsedIterator; - // the current iterator - private Iterator currentIterator; - - // the last used iterator - private Iterator lastUsedIterator; - - public JoinedIterator(List iterators) { - this( (Iterator[]) iterators.toArray(ITERATORS) ); + @SuppressWarnings("unchecked") + public JoinedIterator(List> wrappedIterators) { + this( wrappedIterators.toArray( new Iterator[ wrappedIterators.size() ]) ); } - public JoinedIterator(Iterator[] iterators) { - if( iterators==null ) - throw new NullPointerException("Unexpected NULL iterators argument"); - this.iterators = iterators; - } - - public JoinedIterator(Iterator first, Iterator second) { - this( new Iterator[] { first, second } ); + public JoinedIterator(Iterator... iteratorsToWrap) { + if( iteratorsToWrap == null ) { + throw new NullPointerException( "Iterators to join were null" ); + } + this.wrappedIterators = iteratorsToWrap; } public boolean hasNext() { @@ -71,7 +60,7 @@ public class JoinedIterator implements Iterator { return currentIterator.hasNext(); } - public Object next() { + public T next() { updateCurrentIterator(); return currentIterator.next(); } @@ -85,22 +74,21 @@ public class JoinedIterator implements Iterator { // call this before any Iterator method to make sure that the current Iterator // is not exhausted protected void updateCurrentIterator() { - - if (currentIterator == null) { - if( iterators.length==0 ) { - currentIterator = EmptyIterator.INSTANCE; + if ( currentIterator == null ) { + if( wrappedIterators.length == 0 ) { + currentIterator = Collections.emptyIterator(); } else { - currentIterator = iterators[0]; + currentIterator = wrappedIterators[0]; } // set last used iterator here, in case the user calls remove // before calling hasNext() or next() (although they shouldn't) lastUsedIterator = currentIterator; } - while (! currentIterator.hasNext() && currentIteratorIndex < iterators.length - 1) { + while (! currentIterator.hasNext() && currentIteratorIndex < wrappedIterators.length - 1) { currentIteratorIndex++; - currentIterator = iterators[currentIteratorIndex]; + currentIterator = wrappedIterators[currentIteratorIndex]; } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java b/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java index 49dd3a0bc4..ba1ec4e63d 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java @@ -23,6 +23,7 @@ */ package org.hibernate.mapping; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; @@ -365,8 +366,8 @@ public abstract class Collection implements Fetchable, Value, Filterable { } } - public Iterator getColumnIterator() { - return EmptyIterator.INSTANCE; + public Iterator getColumnIterator() { + return Collections.emptyIterator(); } public int getColumnSpan() { diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java index fd48b6156c..7a2bfe4ee6 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java @@ -106,7 +106,7 @@ public class Component extends SimpleValue implements MetaAttributable { } return n; } - public Iterator getColumnIterator() { + public Iterator getColumnIterator() { Iterator[] iters = new Iterator[ getPropertySpan() ]; Iterator iter = getPropertyIterator(); int i=0; diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java b/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java index 5abfefd081..18b45cded1 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java @@ -76,7 +76,7 @@ public class OneToMany implements Value { // no foreign key element of for a one-to-many } - public Iterator getColumnIterator() { + public Iterator getColumnIterator() { return associatedClass.getKey().getColumnIterator(); } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Selectable.java b/hibernate-core/src/main/java/org/hibernate/mapping/Selectable.java index 92a370e8f3..b79b1b89ea 100755 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Selectable.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Selectable.java @@ -22,9 +22,13 @@ * Boston, MA 02110-1301 USA */ package org.hibernate.mapping; + import org.hibernate.dialect.Dialect; import org.hibernate.dialect.function.SQLFunctionRegistry; +/** + * Models the commonality between a column and a formula (computed value). + */ public interface Selectable { public String getAlias(Dialect dialect); public String getAlias(Dialect dialect, Table table); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index f5fb84a81b..a9bbdb3619 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java @@ -75,7 +75,8 @@ public class SimpleValue implements KeyValue { private final Mappings mappings; - private final List columns = new ArrayList(); + private final List columns = new ArrayList(); + private String typeName; private Properties identifierGeneratorProperties; private String identifierGeneratorStrategy = DEFAULT_ID_GEN_STRATEGY; @@ -132,7 +133,7 @@ public class SimpleValue implements KeyValue { public int getColumnSpan() { return columns.size(); } - public Iterator getColumnIterator() { + public Iterator getColumnIterator() { return columns.iterator(); } public List getConstraintColumns() { diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Value.java b/hibernate-core/src/main/java/org/hibernate/mapping/Value.java index eaa460e64a..731216baf1 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Value.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Value.java @@ -41,7 +41,7 @@ import org.hibernate.type.Type; */ public interface Value extends Serializable { public int getColumnSpan(); - public Iterator getColumnIterator(); + public Iterator getColumnIterator(); public Type getType() throws MappingException; public FetchMode getFetchMode(); public Table getTable(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedDetail.java b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedDetail.java new file mode 100644 index 0000000000..b2d090cc3d --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedDetail.java @@ -0,0 +1,42 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * 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, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY 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 + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.formulajoin; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; + +/** + * @author Steve Ebersole + */ +@Entity +public class AnnotatedDetail { + @Id + private Integer id; + private String name; + + // because otherwise schema export would not know about it... + @Column( name = "detail_domain" ) + private String domain; +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedFormWithBeanValidationNotNullTest.java b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedFormWithBeanValidationNotNullTest.java new file mode 100644 index 0000000000..2bb218be18 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedFormWithBeanValidationNotNullTest.java @@ -0,0 +1,44 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * 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, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY 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 + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.formulajoin; + +import org.hibernate.cfg.Configuration; + +import org.junit.Test; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseUnitTestCase; + +/** + * @author Steve Ebersole + */ +public class AnnotatedFormWithBeanValidationNotNullTest extends BaseUnitTestCase { + @Test + @TestForIssue( jiraKey = "HHH-8167" ) + public void testAnnotatedFormWithBeanValidationNotNull() { + Configuration cfg = new Configuration(); + cfg.addAnnotatedClass( AnnotatedMaster.class ).addAnnotatedClass( AnnotatedDetail.class ); + cfg.buildSessionFactory(); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedMaster.java b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedMaster.java new file mode 100644 index 0000000000..17779287f9 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/AnnotatedMaster.java @@ -0,0 +1,55 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * 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, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY 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 + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.formulajoin; + +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.validation.constraints.NotNull; + +import org.hibernate.annotations.Fetch; +import org.hibernate.annotations.FetchMode; +import org.hibernate.annotations.JoinColumnOrFormula; +import org.hibernate.annotations.JoinColumnsOrFormulas; +import org.hibernate.annotations.JoinFormula; + +/** + * @author Steve Ebersole + */ +@Entity +public class AnnotatedMaster { + @Id + private Integer id; + private String name; + @ManyToOne(fetch= FetchType.EAGER, optional=false) + @JoinColumnsOrFormulas({ + @JoinColumnOrFormula(formula=@JoinFormula(value="my_domain_key'", referencedColumnName="detail_domain")), + @JoinColumnOrFormula(column=@JoinColumn(name="detail", referencedColumnName="id")) + }) + @Fetch(FetchMode.JOIN) + @NotNull + private AnnotatedDetail detail; +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/formulajoin/FormulaJoinTest.java b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/FormulaJoinTest.java index 6f95c2eb91..de3f0728ea 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/formulajoin/FormulaJoinTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/formulajoin/FormulaJoinTest.java @@ -30,6 +30,8 @@ import org.hibernate.Session; import org.hibernate.Transaction; import org.hibernate.dialect.PostgreSQL81Dialect; import org.hibernate.dialect.PostgreSQLDialect; + +import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; import static org.junit.Assert.assertEquals; @@ -111,6 +113,5 @@ public class FormulaJoinTest extends BaseCoreFunctionalTestCase { s.close(); } - }