Skip to content

Commit

Permalink
HHH-15060 - Associations with @NotFound should always be left joined …
Browse files Browse the repository at this point in the history
…when de-referenced in HQL/Criteria

- `@NotFound` no longer exports a physical foreign-key
- tests showing bugs and inconsistencies wrt `@NotFound` handling
- added `FetchNotFoundException`
- force association to EAGER
  • Loading branch information
sebersole committed Feb 23, 2022
1 parent a297a62 commit 6d1a47e
Show file tree
Hide file tree
Showing 25 changed files with 700 additions and 399 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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;

import java.util.Locale;
import javax.persistence.EntityNotFoundException;

/**
* Exception for {@link org.hibernate.annotations.NotFoundAction#EXCEPTION}
*
* @see org.hibernate.annotations.NotFound
*
* @author Steve Ebersole
*/
public class FetchNotFoundException extends EntityNotFoundException {
private final String entityName;
private final Object identifier;

public FetchNotFoundException(String entityName, Object identifier) {
super(
String.format(
Locale.ROOT,
"Entity `%s` with identifier value `%s` does not exist",
entityName,
identifier
)
);
this.entityName = entityName;
this.identifier = identifier;
}

public String getEntityName() {
return entityName;
}

public Object getIdentifier() {
return identifier;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,27 @@ public enum FetchMode {
/**
* Use a secondary select for each individual entity, collection, or join load.
*/
SELECT,
SELECT( org.hibernate.FetchMode.SELECT ),
/**
* Use an outer join to load the related entities, collections or joins.
*/
JOIN,
JOIN( org.hibernate.FetchMode.JOIN ),
/**
* Available for collections only.  When accessing a non-initialized collection, this fetch mode will trigger loading all elements of all collections of the same role for all owners associated with the persistence context using a single secondary select.
* Available for collections only.
*
* When accessing a non-initialized collection, this fetch mode will trigger
* loading all elements of all collections of the same role for all owners
* associated with the persistence context using a single secondary select.
*/
SUBSELECT
SUBSELECT( org.hibernate.FetchMode.SELECT );

private final org.hibernate.FetchMode hibernateFetchMode;

FetchMode(org.hibernate.FetchMode hibernateFetchMode) {
this.hibernateFetchMode = hibernateFetchMode;
}

public org.hibernate.FetchMode getHibernateFetchMode() {
return hibernateFetchMode;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,36 @@
*/
package org.hibernate.annotations;

import org.hibernate.FetchNotFoundException;

/**
* Possible actions when an associated entity is not found in the database. Often seen with "legacy" foreign-key
* schemes which do not use {@code NULL} to indicate a missing reference, instead using a "magic value".
* Possible actions when the database contains a non-null fk with no
* matching target. This also implies that there are no physical
* foreign-key constraints on the database.
*
* As an example, consider a typical Customer/Order model. These actions apply
* when a non-null `orders.customer_fk` value does not have a corresponding value
* in `customers.id`.
*
* Generally this will occur in 2 scenarios:<ul>
* <li>the associated data has been deleted</li>
* <li>the model uses special "magic" values to indicate null</li>
* </ul>
*
* @author Steve Ebersole
* @author Emmanuel Bernard
*/
public enum NotFoundAction {
/**
* Raise an exception when an element is not found (default and recommended).
* Throw an exception when the association is not found (default and recommended).
*
* @see FetchNotFoundException
*/
EXCEPTION,

/**
* Ignore the element when not found in database.
* Ignore the association when not found in database. Effectively treats the
* association as null, despite the non-null foreign-key value.
*/
IGNORE
}
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,8 @@ private static void processElementAnnotations(

Cascade hibernateCascade = property.getAnnotation( Cascade.class );
NotFound notFound = property.getAnnotation( NotFound.class );
boolean ignoreNotFound = notFound != null && notFound.action().equals( NotFoundAction.IGNORE );
NotFoundAction notFoundAction = notFound == null ? null : notFound.action();
boolean ignoreNotFound = notFoundAction == NotFoundAction.IGNORE;
matchIgnoreNotFoundWithFetchType(propertyHolder.getEntityName(), property.getName(), ignoreNotFound, ann.fetch());
OnDelete onDeleteAnn = property.getAnnotation( OnDelete.class );
boolean onDeleteCascade = onDeleteAnn != null && OnDeleteAction.CASCADE.equals( onDeleteAnn.action() );
Expand All @@ -1821,7 +1822,7 @@ private static void processElementAnnotations(
getCascadeStrategy( ann.cascade(), hibernateCascade, false, forcePersist ),
joinColumns,
!mandatory,
ignoreNotFound,
notFoundAction,
onDeleteCascade,
ToOneBinder.getTargetEntity( inferredData, context ),
propertyHolder,
Expand Down Expand Up @@ -1851,7 +1852,8 @@ else if ( property.isAnnotationPresent( OneToOne.class ) ) {
boolean trueOneToOne = hasPkjc;
Cascade hibernateCascade = property.getAnnotation( Cascade.class );
NotFound notFound = property.getAnnotation( NotFound.class );
boolean ignoreNotFound = notFound != null && notFound.action().equals( NotFoundAction.IGNORE );
NotFoundAction notFoundAction = notFound == null ? null : notFound.action();
boolean ignoreNotFound = notFoundAction == NotFoundAction.IGNORE;
// MapsId means the columns belong to the pk;
// A @MapsId association (obviously) must be non-null when the entity is first persisted.
// If a @MapsId association is not mapped with @NotFound(IGNORE), then the association
Expand All @@ -1878,7 +1880,8 @@ else if ( property.isAnnotationPresent( OneToOne.class ) ) {
joinColumns,
!mandatory,
getFetchMode( ann.fetch() ),
ignoreNotFound, onDeleteCascade,
notFoundAction,
onDeleteCascade,
ToOneBinder.getTargetEntity( inferredData, context ),
propertyHolder,
inferredData,
Expand Down Expand Up @@ -3040,7 +3043,7 @@ private static void bindManyToOne(
String cascadeStrategy,
Ejb3JoinColumn[] columns,
boolean optional,
boolean ignoreNotFound,
NotFoundAction notFoundAction,
boolean cascadeOnDelete,
XClass targetEntity,
PropertyHolder propertyHolder,
Expand All @@ -3060,7 +3063,7 @@ private static void bindManyToOne(
final XProperty property = inferredData.getProperty();
defineFetchingStrategy( value, property );
//value.setFetchMode( fetchMode );
value.setIgnoreNotFound( ignoreNotFound );
value.setNotFoundAction( notFoundAction );
value.setCascadeDeleteEnabled( cascadeOnDelete );
//value.setLazy( fetchMode != FetchMode.JOIN );
if ( !optional ) {
Expand Down Expand Up @@ -3166,6 +3169,8 @@ protected static void defineFetchingStrategy(ToOne toOne, XProperty property) {
Fetch fetch = property.getAnnotation( Fetch.class );
ManyToOne manyToOne = property.getAnnotation( ManyToOne.class );
OneToOne oneToOne = property.getAnnotation( OneToOne.class );
NotFound notFound = property.getAnnotation( NotFound.class );

FetchType fetchType;
if ( manyToOne != null ) {
fetchType = manyToOne.fetch();
Expand All @@ -3178,7 +3183,12 @@ else if ( oneToOne != null ) {
"Define fetch strategy on a property not annotated with @OneToMany nor @OneToOne"
);
}
if ( lazy != null ) {

if ( notFound != null ) {
toOne.setLazy( false );
toOne.setUnwrapProxy( true );
}
else if ( lazy != null ) {
toOne.setLazy( !( lazy.value() == LazyToOneOption.FALSE ) );
toOne.setUnwrapProxy( ( lazy.value() == LazyToOneOption.NO_PROXY ) );
}
Expand All @@ -3187,6 +3197,7 @@ else if ( oneToOne != null ) {
toOne.setUnwrapProxy( fetchType != FetchType.LAZY );
toOne.setUnwrapProxyImplicit( true );
}

if ( fetch != null ) {
if ( fetch.value() == org.hibernate.annotations.FetchMode.JOIN ) {
toOne.setFetchMode( FetchMode.JOIN );
Expand All @@ -3213,7 +3224,7 @@ private static void bindOneToOne(
Ejb3JoinColumn[] joinColumns,
boolean optional,
FetchMode fetchMode,
boolean ignoreNotFound,
NotFoundAction notFoundAction,
boolean cascadeOnDelete,
XClass targetEntity,
PropertyHolder propertyHolder,
Expand Down Expand Up @@ -3267,7 +3278,7 @@ private static void bindOneToOne(
propertyHolder,
inferredData,
targetEntity,
ignoreNotFound,
notFoundAction,
cascadeOnDelete,
optional,
cascadeStrategy,
Expand All @@ -3287,7 +3298,7 @@ private static void bindOneToOne(
else {
//has a FK on the table
bindManyToOne(
cascadeStrategy, joinColumns, optional, ignoreNotFound, cascadeOnDelete,
cascadeStrategy, joinColumns, optional, notFoundAction, cascadeOnDelete,
targetEntity,
propertyHolder, inferredData, true, isIdentifierMapper, inSecondPass,
propertyBinder, context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.hibernate.mapping.Collection;
import org.hibernate.mapping.IndexedCollection;
import org.hibernate.mapping.OneToMany;
import org.hibernate.mapping.PersistentClass;
import org.hibernate.mapping.Selectable;
import org.hibernate.mapping.Value;

Expand Down Expand Up @@ -68,8 +69,7 @@ public void doSecondPass(java.util.Map persistentClasses)
}
}

abstract public void secondPass(java.util.Map persistentClasses, java.util.Map inheritedMetas)
throws MappingException;
abstract public void secondPass(java.util.Map persistentClasses, java.util.Map inheritedMetas);

private static String columns(Value val) {
StringBuilder columns = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@

import java.util.Iterator;
import java.util.Map;

import javax.persistence.JoinColumn;
import javax.persistence.JoinColumns;

import org.hibernate.AnnotationException;
import org.hibernate.FetchMode;
import org.hibernate.MappingException;
import org.hibernate.annotations.LazyGroup;
import org.hibernate.annotations.NotFoundAction;
import org.hibernate.annotations.common.reflection.XClass;
import org.hibernate.boot.spi.MetadataBuildingContext;
import org.hibernate.cfg.annotations.PropertyBinder;
Expand All @@ -41,7 +40,7 @@ public class OneToOneSecondPass implements SecondPass {
private String ownerEntity;
private String ownerProperty;
private PropertyHolder propertyHolder;
private boolean ignoreNotFound;
private NotFoundAction notFoundAction;
private PropertyData inferredData;
private XClass targetEntity;
private boolean cascadeOnDelete;
Expand All @@ -57,7 +56,7 @@ public OneToOneSecondPass(
PropertyHolder propertyHolder,
PropertyData inferredData,
XClass targetEntity,
boolean ignoreNotFound,
NotFoundAction notFoundAction,
boolean cascadeOnDelete,
boolean optional,
String cascadeStrategy,
Expand All @@ -68,7 +67,7 @@ public OneToOneSecondPass(
this.mappedBy = mappedBy;
this.propertyHolder = propertyHolder;
this.buildingContext = buildingContext;
this.ignoreNotFound = ignoreNotFound;
this.notFoundAction = notFoundAction;
this.inferredData = inferredData;
this.targetEntity = targetEntity;
this.cascadeOnDelete = cascadeOnDelete;
Expand Down Expand Up @@ -191,7 +190,7 @@ else if ( otherSideProperty.getValue() instanceof ManyToOne ) {
);
ManyToOne manyToOne = new ManyToOne( buildingContext, mappedByJoin.getTable() );
//FIXME use ignore not found here
manyToOne.setIgnoreNotFound( ignoreNotFound );
manyToOne.setNotFoundAction( notFoundAction );
manyToOne.setCascadeDeleteEnabled( value.isCascadeDeleteEnabled() );
manyToOne.setFetchMode( value.getFetchMode() );
manyToOne.setLazy( value.isLazy() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ public void doSecondPass(java.util.Map persistentClasses) throws MappingExceptio
/*
* HbmMetadataSourceProcessorImpl does this only when property-ref != null, but IMO, it makes sense event if it is null
*/
if ( !manyToOne.isIgnoreNotFound() ) manyToOne.createPropertyRefConstraints( persistentClasses );
if ( manyToOne.getNotFoundAction() == null ) {
manyToOne.createPropertyRefConstraints( persistentClasses );
}
}
else if ( value instanceof OneToOne ) {
value.createForeignKey();
Expand Down
Loading

0 comments on commit 6d1a47e

Please sign in to comment.