From a4d98419158802ea8db71bc79020c5367192dca4 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Mon, 27 Nov 2017 15:29:52 -0600 Subject: [PATCH] challenge --- .../hibernate/engine/internal/Cascade.java | 14 ++- .../engine/internal/ForeignKeys.java | 4 + .../engine/internal/Nullability.java | 43 ++++++- .../internal/DefaultDeleteEventListener.java | 2 +- .../jpa/compliance/joincolumn/Company.java | 68 ++++++++++ .../compliance/joincolumn/JoinColumnTest.java | 116 ++++++++++++++++++ .../jpa/compliance/joincolumn/Location.java | 61 +++++++++ .../src/test/resources/log4j.properties | 1 + .../test/jpa/compliance/joincolumn/orm.xml | 31 +++++ 9 files changed, 331 insertions(+), 9 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Company.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/JoinColumnTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Location.java create mode 100644 hibernate-core/src/test/resources/org/hibernate/test/jpa/compliance/joincolumn/orm.xml diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java index 10d069c671..3e1d0ddd20 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/Cascade.java @@ -17,6 +17,7 @@ import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterc import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.engine.spi.CascadeStyle; import org.hibernate.engine.spi.CascadingAction; +import org.hibernate.engine.spi.CascadingActions; import org.hibernate.engine.spi.CollectionEntry; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.Status; @@ -72,9 +73,12 @@ public final class Cascade { * which is specific to each CascadingAction type */ public static void cascade( - final CascadingAction action, final CascadePoint cascadePoint, - final EventSource eventSource, final EntityPersister persister, final Object parent, final Object anything) - throws HibernateException { + final CascadingAction action, + final CascadePoint cascadePoint, + final EventSource eventSource, + final EntityPersister persister, + final Object parent, + final Object anything) throws HibernateException { if ( persister.hasCascades() || action.requiresNoCascadeChecking() ) { // performance opt final boolean traceEnabled = LOG.isTraceEnabled(); @@ -138,6 +142,10 @@ public final class Cascade { ); } else { + if ( action == CascadingActions.DELETE ) { + + } + if ( action.requiresNoCascadeChecking() ) { action.noCascade( eventSource, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java index e98f9e7c7f..fd90c95ba9 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java @@ -14,6 +14,7 @@ import org.hibernate.TransientObjectException; import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.engine.spi.Status; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; @@ -159,6 +160,9 @@ public final class ForeignKeys { if ( entityEntry == null ) { return isTransient( entityName, object, null, session ); } + else if ( isDelete && ( entityEntry.getStatus() == Status.DELETED || entityEntry.getStatus() == Status.GONE ) ) { + return false; + } else { return entityEntry.isNullifiable( isEarlyInsert, session ); } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/Nullability.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/Nullability.java index 5cbd5a4e1d..7fec9be63e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/Nullability.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/Nullability.java @@ -11,11 +11,14 @@ import java.util.Iterator; import org.hibernate.HibernateException; import org.hibernate.PropertyValueException; import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; +import org.hibernate.engine.spi.CascadeStyle; import org.hibernate.engine.spi.CascadingActions; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.engine.spi.Status; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.type.CollectionType; import org.hibernate.type.CompositeType; +import org.hibernate.type.EntityType; import org.hibernate.type.Type; /** @@ -49,7 +52,21 @@ public final class Nullability { public void checkNullability( final Object[] values, final EntityPersister persister, - final boolean isUpdate) throws HibernateException { + final boolean isUpdate) { + checkNullability( values, persister, isUpdate ? NullabilityCheckType.UPDATE : NullabilityCheckType.CREATE ); + } + + public enum NullabilityCheckType { + CREATE, + UPDATE, + DELETE + } + + public void checkNullability( + final Object[] values, + final EntityPersister persister, + final NullabilityCheckType checkType) { + /* * Typically when Bean Validation is on, we don't want to validate null values * at the Hibernate Core level. Hence the checkNullability setting. @@ -60,7 +77,7 @@ public final class Nullability { * Check for any level one nullability breaks * Look at non null components to * recursively check next level of nullability breaks - * Look at Collections contraining component to + * Look at Collections containing components to * recursively check next level of nullability breaks * * @@ -74,9 +91,9 @@ public final class Nullability { */ final boolean[] nullability = persister.getPropertyNullability(); - final boolean[] checkability = isUpdate ? - persister.getPropertyUpdateability() : - persister.getPropertyInsertability(); + final boolean[] checkability = checkType == NullabilityCheckType.CREATE + ? persister.getPropertyInsertability() + : persister.getPropertyUpdateability(); final Type[] propertyTypes = persister.getPropertyTypes(); for ( int i = 0; i < values.length; i++ ) { @@ -85,6 +102,22 @@ public final class Nullability { final Object value = values[i]; if ( !nullability[i] && value == null ) { + // generally speaking this is an exception as we will throw below + // but first, we check for a special condition in which: + // 1) we are processing a delete + // 2) the property represents the "other side" of this association + // 3) the other side is currently in the "being deleted" status + // 4) the property is defined to cascade deletes from the other side to + // this association property + // + // in such a case we allow this to continue + if ( checkType == NullabilityCheckType.DELETE ) { + if ( propertyTypes[i].isEntityType() ) { + final EntityType associationType = (EntityType) propertyTypes[i]; + + } + } + //check basic level one nullablilty throw new PropertyValueException( "not-null property references a null or transient value", diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java index a8b1665a8c..b634771ec1 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java @@ -258,7 +258,7 @@ public class DefaultDeleteEventListener implements DeleteEventListener { new ForeignKeys.Nullifier( entity, true, false, session ) .nullifyTransientReferences( entityEntry.getDeletedState(), propTypes ); - new Nullability( session ).checkNullability( entityEntry.getDeletedState(), persister, true ); + new Nullability( session ).checkNullability( entityEntry.getDeletedState(), persister, Nullability.NullabilityCheckType.DELETE ); persistenceContext.getNullifiableEntityKeys().add( key ); if ( isOrphanRemovalBeforeUpdates ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Company.java b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Company.java new file mode 100644 index 0000000000..f44195ab5d --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Company.java @@ -0,0 +1,68 @@ +/* + * 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.test.jpa.compliance.joincolumn; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; + +/** + * @author Steve Ebersole + */ +@Entity +public class Company { + private Integer id; + private String name; + private Set locations; + + public Company() { + } + + public Company(Integer id, String name) { + this.id = id; + this.name = name; + } + + public void addLocation(Location location) { + if ( locations == null ) { + locations = new HashSet<>(); + } + + locations.add( location ); + location.setCompany( this ); + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @OneToMany(cascade = CascadeType.PERSIST, mappedBy = "company") + public Set getLocations() { + return locations == null ? Collections.emptySet() : locations; + } + + public void setLocations(Set locations) { + this.locations = locations; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/JoinColumnTest.java b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/JoinColumnTest.java new file mode 100644 index 0000000000..549ec517a0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/JoinColumnTest.java @@ -0,0 +1,116 @@ +/* + * 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.test.jpa.compliance.joincolumn; + +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.dialect.H2Dialect; +import org.hibernate.engine.spi.SessionFactoryImplementor; + +import org.hibernate.testing.FailureExpected; +import org.hibernate.testing.RequiresDialect; +import org.hibernate.testing.junit4.BaseUnitTestCase; +import org.junit.Test; + +import static org.hibernate.testing.transaction.TransactionUtil2.inTransaction; + +/** + * @author Steve Ebersole + */ +@RequiresDialect(H2Dialect.class ) +@FailureExpected( jiraKey = "tck-challenge" ) +public class JoinColumnTest extends BaseUnitTestCase { + + @Test + public void testIt() { + final StandardServiceRegistry ssr = new StandardServiceRegistryBuilder() + .applySetting( AvailableSettings.HBM2DDL_AUTO, "create-drop" ) + .build(); + + try { + + try (SessionFactoryImplementor sf = (SessionFactoryImplementor) new MetadataSources( ssr ) + .addAnnotatedClass( Company.class ) + .addAnnotatedClass( Location.class ) + .addResource( "org/hibernate/test/jpa/compliance/joincolumn/orm.xml" ) + .buildMetadata() + .buildSessionFactory()) { + try { + inTransaction( + sf, + session -> { + final Company acme = new Company( 1, "Acme Corp" ); + new Location( 1, "86-215", acme ); + new Location( 2, "20-759", acme ); + + session.persist( acme ); + } + ); + + inTransaction( + sf, + session -> { + final Company acme = session.get( Company.class, 1 ); + assert acme.getLocations().size() == 2; + + // this fails. however it is due to a number of bad assumptions + // in the TCK: + +// First the spec says: +// +// {quote} +// The relationship modeling annotation constrains the use of the cascade=REMOVE specification. The +// cascade=REMOVE specification should only be applied to associations that are specified as One- +// ToOne or OneToMany. Applications that apply cascade=REMOVE to other associations are not por- +// table. +// {quote} +// +// Here the test is applying cascade=REMOVE to a ManyToOne. +// +// Secondly, the spec says: +// +// {quote} +// The persistence provider runtime is permitted to perform synchronization to the database at other times +// as well when a transaction is active and the persistence context is joined to the transaction. +// {quote} +// +// +// In other words, the provider is actually legal to perform the database delete immediately. Since +// the TCK deletes the Company first, a provider is legally able to perform the database delete on +// Company immediately. However the TCK test as defined makes this impossible: +// 1) simply deleting Company won't work since Location rows still reference it. Locations +// would need to be deleted first (cascade the remove to the @OneToMany `locations` +// attribute), or +// 2) perform a SQL update, updating the COMP_ID columns in the Location table to be null +// so that deleting Company wont cause FK violations. But again this is made impossible +// by the TCK because it defines the column as non-nullable (and not even in the @JoinColumn +// btw which would be another basis for challenge). + session.remove( acme ); + for ( Location location : acme.getLocations() ) { + session.remove( location ); + } + } + ); + } + finally { + inTransaction( + sf, + session -> { + session.createQuery( "delete Location" ).executeUpdate(); + session.createQuery( "delete Company" ).executeUpdate(); + } + ); + } + } + } + finally { + StandardServiceRegistryBuilder.destroy( ssr ); + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Location.java b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Location.java new file mode 100644 index 0000000000..2fe24ea522 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/jpa/compliance/joincolumn/Location.java @@ -0,0 +1,61 @@ +/* + * 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.test.jpa.compliance.joincolumn; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; + +/** + * @author Steve Ebersole + */ +@Entity +public class Location { + private Integer id; + private String code; + private Company company; + + public Location() { + } + + public Location(Integer id, String code, Company company) { + this.id = id; + this.code = code; + this.company = company; + + company.addLocation( this ); + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getCode() { + return code; + } + + public void setCode(String code) { + this.code = code; + } + + @ManyToOne(cascade = CascadeType.REMOVE) + @JoinColumn(name = "comp_id", referencedColumnName = "id", nullable = false) + public Company getCompany() { + return company; + } + + public void setCompany(Company company) { + this.company = company; + } +} diff --git a/hibernate-core/src/test/resources/log4j.properties b/hibernate-core/src/test/resources/log4j.properties index eb96581a28..40feef756b 100644 --- a/hibernate-core/src/test/resources/log4j.properties +++ b/hibernate-core/src/test/resources/log4j.properties @@ -50,6 +50,7 @@ log4j.logger.org.hibernate.engine.internal.StatisticalLoggingSessionEventListene log4j.logger.org.hibernate.boot.model.source.internal.hbm.ModelBinder=debug log4j.logger.org.hibernate.type.descriptor.java.JavaTypeDescriptorRegistry=debug +log4j.logger.org.hibernate.engine.internal.Cascade=trace ### When entity copy merge functionality is enabled using: ### hibernate.event.merge.entity_copy_observer=log, the following will diff --git a/hibernate-core/src/test/resources/org/hibernate/test/jpa/compliance/joincolumn/orm.xml b/hibernate-core/src/test/resources/org/hibernate/test/jpa/compliance/joincolumn/orm.xml new file mode 100644 index 0000000000..b794b16a62 --- /dev/null +++ b/hibernate-core/src/test/resources/org/hibernate/test/jpa/compliance/joincolumn/orm.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + +