From 329e85b0b08a3d5179e25567c6c1fa0f42dd1f37 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 14 May 2015 11:52:11 -0500 Subject: [PATCH] HHH-9797 - Inaccurate warnings logged about duplicate joins (HHH000072) --- .../InFlightMetadataCollectorImpl.java | 28 ---- .../boot/spi/InFlightMetadataCollector.java | 19 ++- .../cfg/annotations/EntityBinder.java | 3 +- .../org/hibernate/test/join/Customer.java | 52 ++++-- .../org/hibernate/test/join/Employee.java | 63 +++++--- .../java/org/hibernate/test/join/Person.java | 151 ++++++++++-------- .../test/join/SecondaryTableTest.java | 51 ++++++ .../java/org/hibernate/test/join/User.java | 69 ++++++-- 8 files changed, 289 insertions(+), 147 deletions(-) create mode 100755 hibernate-core/src/test/java/org/hibernate/test/join/SecondaryTableTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java index 4c0bfeb44a..2e7d17961c 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java @@ -1371,34 +1371,6 @@ public class InFlightMetadataCollectorImpl implements InFlightMetadataCollector return xrefEntry == null ? null : xrefEntry.secondaryTableJoinMap; } - @Override - public void addJoins(PersistentClass persistentClass, Map joins) { - // this part about resolving the super-EntityTableXref is a best effort. But - // really annotation binding is the only thing calling this, and it does not - // directly use the EntityTableXref stuff, so its all good. - final EntityTableXrefImpl superEntityTableXref; - if ( persistentClass.getSuperclass() == null ) { - superEntityTableXref = null; - } - else { - superEntityTableXref = entityTableXrefMap.get( persistentClass.getSuperclass().getEntityName() ); - } - - final String primaryTableLogicalName = getLogicalTableName( persistentClass.getTable() ); - final EntityTableXrefImpl xrefEntry = new EntityTableXrefImpl( - getDatabase().toIdentifier( primaryTableLogicalName ), - persistentClass.getTable(), - superEntityTableXref - ); - - final EntityTableXrefImpl old = entityTableXrefMap.put( persistentClass.getEntityName(), xrefEntry ); - if ( old != null ) { - log.duplicateJoins( persistentClass.getEntityName() ); - } - - xrefEntry.secondaryTableJoinMap = joins; - } - private final class EntityTableXrefImpl implements EntityTableXref { private final Identifier primaryTableLogicalName; private final Table primaryTable; diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/InFlightMetadataCollector.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/InFlightMetadataCollector.java index 94e2161bed..e6a2d12cf9 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/InFlightMetadataCollector.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/InFlightMetadataCollector.java @@ -292,10 +292,10 @@ public interface InFlightMetadataCollector extends Mapping, MetadataImplementor NaturalIdUniqueKeyBinder locateNaturalIdUniqueKeyBinder(String entityName); void registerNaturalIdUniqueKeyBinder(String entityName, NaturalIdUniqueKeyBinder ukBinder); - public static interface DelayedPropertyReferenceHandler extends Serializable { - public void process(InFlightMetadataCollector metadataCollector); + interface DelayedPropertyReferenceHandler extends Serializable { + void process(InFlightMetadataCollector metadataCollector); } - public void addDelayedPropertyReferenceHandler(DelayedPropertyReferenceHandler handler); + void addDelayedPropertyReferenceHandler(DelayedPropertyReferenceHandler handler); void addPropertyReference(String entityName, String propertyName); void addUniquePropertyReference(String entityName, String propertyName); @@ -310,7 +310,7 @@ public interface InFlightMetadataCollector extends Mapping, MetadataImplementor void addJpaIndexHolders(Table table, List jpaIndexHolders); - public static interface EntityTableXref { + interface EntityTableXref { void addSecondaryTable(LocalMetadataBuildingContext buildingContext, Identifier logicalName, Join secondaryTableJoin); void addSecondaryTable(Identifier logicalName, Join secondaryTableJoin); Table resolveTable(Identifier tableName); @@ -318,7 +318,7 @@ public interface InFlightMetadataCollector extends Mapping, MetadataImplementor Join locateJoin(Identifier tableName); } - public static class DuplicateSecondaryTableException extends HibernateException { + class DuplicateSecondaryTableException extends HibernateException { private final Identifier tableName; public DuplicateSecondaryTableException(Identifier tableName) { @@ -333,8 +333,11 @@ public interface InFlightMetadataCollector extends Mapping, MetadataImplementor } } - public EntityTableXref getEntityTableXref(String entityName); - public EntityTableXref addEntityTableXref(String entityName, Identifier primaryTableLogicalName, Table primaryTable, EntityTableXref superEntityTableXref); - void addJoins(PersistentClass persistentClass, Map secondaryTables); + EntityTableXref getEntityTableXref(String entityName); + EntityTableXref addEntityTableXref( + String entityName, + Identifier primaryTableLogicalName, + Table primaryTable, + EntityTableXref superEntityTableXref); Map getJoins(String entityName); } diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java index 5a4daf850e..d7991be2f8 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java @@ -136,6 +136,8 @@ public class EntityBinder { private boolean lazy; private XClass proxyClass; private String where; + // todo : we should defer to InFlightMetadataCollector.EntityTableXref for secondary table tracking; + // atm we use both from here; HBM binding solely uses InFlightMetadataCollector.EntityTableXref private java.util.Map secondaryTables = new HashMap(); private java.util.Map secondaryTableJoins = new HashMap(); private String cacheConcurrentStrategy; @@ -698,7 +700,6 @@ public class EntityBinder { Join join = (Join) joins.next(); createPrimaryColumnsToSecondaryTable( uncastedColumn, propertyHolder, join ); } - context.getMetadataCollector().addJoins( persistentClass, secondaryTables ); } private void createPrimaryColumnsToSecondaryTable(Object uncastedColumn, PropertyHolder propertyHolder, Join join) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/join/Customer.java b/hibernate-core/src/test/java/org/hibernate/test/join/Customer.java index 5b91cfd37f..3ca31923e0 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/join/Customer.java +++ b/hibernate-core/src/test/java/org/hibernate/test/join/Customer.java @@ -1,35 +1,61 @@ -//$Id: Customer.java 4364 2004-08-17 12:10:32Z oneovthafew $ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2015, 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.join; +import javax.persistence.Column; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.SecondaryTable; /** * @author Gavin King + * @author Steve Ebersole */ +@Entity +@DiscriminatorValue( "C" ) +@SecondaryTable( name="customer" ) public class Customer extends Person { private Employee salesperson; private String comments; - /** - * @return Returns the salesperson. - */ + @ManyToOne + @JoinColumn( table = "customer" ) public Employee getSalesperson() { return salesperson; } - /** - * @param salesperson The salesperson to set. - */ + public void setSalesperson(Employee salesperson) { this.salesperson = salesperson; } - /** - * @return Returns the comments. - */ + + @Column( table = "customer" ) public String getComments() { return comments; } - /** - * @param comments The comments to set. - */ + public void setComments(String comments) { this.comments = comments; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/join/Employee.java b/hibernate-core/src/test/java/org/hibernate/test/join/Employee.java index 75e8c788c2..15adb1939b 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/join/Employee.java +++ b/hibernate-core/src/test/java/org/hibernate/test/join/Employee.java @@ -1,47 +1,72 @@ -//$Id: Employee.java 4364 2004-08-17 12:10:32Z oneovthafew $ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2015, 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.join; + import java.math.BigDecimal; +import javax.persistence.Column; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.SecondaryTable; /** * @author Gavin King + * @author Steve Ebersole */ +@Entity +@DiscriminatorValue( "E" ) +@SecondaryTable( name = "employee" ) public class Employee extends Person { private String title; private BigDecimal salary; private Employee manager; - /** - * @return Returns the title. - */ + + @Column( table = "employee", name = "`title`") public String getTitle() { return title; } - /** - * @param title The title to set. - */ + public void setTitle(String title) { this.title = title; } - /** - * @return Returns the manager. - */ + + @ManyToOne + @JoinColumn( table = "employee" ) public Employee getManager() { return manager; } - /** - * @param manager The manager to set. - */ + public void setManager(Employee manager) { this.manager = manager; } - /** - * @return Returns the salary. - */ + + @Column( table = "employee" ) public BigDecimal getSalary() { return salary; } - /** - * @param salary The salary to set. - */ + public void setSalary(BigDecimal salary) { this.salary = salary; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/join/Person.java b/hibernate-core/src/test/java/org/hibernate/test/join/Person.java index 6c69979f98..ca26ff5f61 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/join/Person.java +++ b/hibernate-core/src/test/java/org/hibernate/test/join/Person.java @@ -1,11 +1,54 @@ -//$Id: Person.java 7203 2005-06-19 02:01:05Z oneovthafew $ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2015, 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.join; +import javax.persistence.Column; +import javax.persistence.DiscriminatorColumn; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Inheritance; +import javax.persistence.InheritanceType; +import javax.persistence.JoinColumn; +import javax.persistence.PrimaryKeyJoinColumn; +import javax.persistence.SecondaryTable; +import javax.persistence.Transient; +import org.hibernate.annotations.ColumnTransformer; +import org.hibernate.annotations.Fetch; +import org.hibernate.annotations.GenericGenerator; /** * @author Gavin King + * @author Steve Ebersole */ +@Entity +@Inheritance(strategy = InheritanceType.SINGLE_TABLE) +@DiscriminatorColumn( name = "person_type" ) +@DiscriminatorValue( value = "P" ) +@SecondaryTable( name = "address", pkJoinColumns = @PrimaryKeyJoinColumn(name = "address_id") ) public class Person { private long id; private String name; @@ -14,91 +57,73 @@ public class Person { private String country; private double heightInches; private char sex; - - /** - * @return Returns the sex. - */ - public char getSex() { - return sex; - } - /** - * @param sex The sex to set. - */ - public void setSex(char sex) { - this.sex = sex; - } - /** - * @return Returns the id. - */ + + @Id + @GeneratedValue( generator = "increment" ) + @GenericGenerator( name = "increment", strategy = "increment" ) public long getId() { return id; } - /** - * @param id The id to set. - */ + public void setId(long id) { this.id = id; } - /** - * @return Returns the identity. - */ + + public char getSex() { + return sex; + } + + public void setSex(char sex) { + this.sex = sex; + } + public String getName() { return name; } - /** - * @param identity The identity to set. - */ + public void setName(String identity) { this.name = identity; } + + @Transient public String getSpecies() { return null; } - /** - * @return Returns the country. - */ - public String getCountry() { - return country; - } - /** - * @param country The country to set. - */ - public void setCountry(String country) { - this.country = country; - } - /** - * @return Returns the zip. - */ - public String getZip() { - return zip; - } - /** - * @param zip The zip to set. - */ - public void setZip(String zip) { - this.zip = zip; - } - /** - * @return the The height in inches. - */ + @Column( name = "height_centimeters" ) + @ColumnTransformer( read = "height_centimeters / 2.54E0", write = "? * 2.54E0" ) public double getHeightInches() { return heightInches; } - /** - * @param heightInches The height in inches. - */ + public void setHeightInches(double heightInches) { this.heightInches = heightInches; } - /** - * @param address The address to set. - */ - public void setAddress(String address) { - this.address = address; - } - + + @Column(table = "address") public String getAddress() { return address; } + + public void setAddress(String address) { + this.address = address; + } + + @Column(table = "address") + public String getCountry() { + return country; + } + + public void setCountry(String country) { + this.country = country; + } + + @Column(table = "address") + public String getZip() { + return zip; + } + + public void setZip(String zip) { + this.zip = zip; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/join/SecondaryTableTest.java b/hibernate-core/src/test/java/org/hibernate/test/join/SecondaryTableTest.java new file mode 100755 index 0000000000..d1f108b818 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/join/SecondaryTableTest.java @@ -0,0 +1,51 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2015, 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.join; + +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Test; + +/** + * Copy of the model used in JoinTest, but using annotations rather than hbm.xml to look + * for the duplicate joins warning + * + * @author Steve Ebersole + */ +public class SecondaryTableTest extends BaseCoreFunctionalTestCase { + @Override + public String[] getMappings() { + return null; + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] {Person.class, Employee.class, Customer.class, User.class}; + } + + @Test + public void testIt() { + // really we have nothing to tes + } +} + diff --git a/hibernate-core/src/test/java/org/hibernate/test/join/User.java b/hibernate-core/src/test/java/org/hibernate/test/join/User.java index 7ceb83c753..07006b4d3e 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/join/User.java +++ b/hibernate-core/src/test/java/org/hibernate/test/join/User.java @@ -1,37 +1,76 @@ -//$Id$ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2015, 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.join; +import javax.persistence.Column; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.SecondaryTable; +import javax.persistence.SecondaryTables; + +import org.hibernate.annotations.ColumnTransformer; /** * @author Mike Dillon + * @author Steve Ebersole */ +@Entity +@DiscriminatorValue( "U" ) +@SecondaryTables({ + @SecondaryTable(name = "t_user"), + @SecondaryTable(name = "t_silly") +}) public class User extends Person { private String login; - private String silly; private Double passwordExpiryDays; + private String silly; - /** - * @return Returns the login. - */ + @Column(table = "t_user", name = "u_login") public String getLogin() { return login; } - /** - * @param login The login to set. - */ + public void setLogin(String login) { this.login = login; } - /** - * @return The password expiry policy in days. - */ + + @Column(table = "t_user", name = "pwd_expiry_weeks") + @ColumnTransformer( read = "pwd_expiry_weeks * 7.0E0", write = "? / 7.0E0") public Double getPasswordExpiryDays() { return passwordExpiryDays; } - /** - * @param passwordExpiryDays The password expiry policy in days. - */ + public void setPasswordExpiryDays(Double passwordExpiryDays) { this.passwordExpiryDays = passwordExpiryDays; - } + } + + @Column(table = "t_silly") + public String getSilly() { + return silly; + } + + public void setSilly(String silly) { + this.silly = silly; + } }