Skip to content

Commit

Permalink
HSEARCH-4720 Throw exceptions instead of logging warnings for failure…
Browse files Browse the repository at this point in the history
…s to configure association inverse path resolution
  • Loading branch information
marko-bekhta committed Sep 12, 2024
1 parent b9b1fe8 commit 0767e58
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 27 deletions.
5 changes: 4 additions & 1 deletion documentation/src/main/asciidoc/migration/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,7 @@ to reflect the changes in the `org.hibernate.search.mapper.pojo.standalone.loadi
The behavior of Hibernate Search {hibernateSearchVersion}
is, in general, backward-compatible with Hibernate Search {hibernateSearchPreviousStableVersionShort}.

The default mass indexer logging monitor updated the format of the logged messages to provide the information in a more condense form.
* The default mass indexer logging monitor updated the format of the logged messages to provide the information in a more condense form.
* In a few places related to the discovery of the inverse side of an association (in the ORM mapper)
that previously logged warnings, Hibernate Search now will throw exceptions instead.
This is related to https://hibernate.atlassian.net/browse/HSEARCH-4708[HSEARCH-4708].
Original file line number Diff line number Diff line change
Expand Up @@ -271,15 +271,14 @@ SearchException unknownClassForMappedEntityType(@FormatWith(ClassFormatter.class
+ " Valid Hibernate ORM names for mapped entities are: %2$s")
SearchException unknownHibernateOrmEntityNameForMappedEntityType(String invalidName, Collection<String> validNames);

@LogMessage(level = Logger.Level.WARN)
@Message(id = ID_OFFSET + 121,
value = "An unexpected failure occurred while resolving the representation of path '%1$s' in the entity state array,"
+ " which is necessary to configure resolution of association inverse side for reindexing."
+ " This may lead to incomplete reindexing and thus out-of-sync indexes."
+ " The exception is being ignored to preserve backwards compatibility with earlier versions of Hibernate Search."
+ " Cannot proceed further as this may lead to incomplete reindexing and thus out-of-sync indexes."
+ " Failure: %3$s"
+ " %2$s") // Context
void failedToResolveStateRepresentation(String path, @FormatWith(EventContextFormatter.class) EventContext context,
SearchException failedToResolveStateRepresentation(String path,
@FormatWith(EventContextFormatter.class) EventContext context,
String causeMessage,
@Cause Exception cause);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,11 @@ public void resolvedStateRepresentation(String wholePathStringRepresentation) {
tryResolveStateRepresentation( wholePathStringRepresentation );
}
catch (RuntimeException e) {
// TODO HSEARCH-4720 when we can afford breaking changes (in the next major), we should probably throw an exception
// instead of just logging a warning here?
log.failedToResolveStateRepresentation(
throw log.failedToResolveStateRepresentation(
wholePathStringRepresentation,
EventContexts.fromType( typeModel ).append( PojoEventContexts.fromPath( wholePath ) ),
e.getMessage(), e
);
disableStateRepresentation();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,19 +189,10 @@ private List<List<PojoImplicitReindexingAssociationInverseSideResolverNode<Objec
entityStateRepresentation.pathFromStateArrayElement(), inversePathByInverseType );
}
catch (RuntimeException e) {
// We're logging instead of re-throwing for backwards compatibility,
// as we don't want this feature to cause errors in existing applications.
// TODO HSEARCH-4720 when we can afford breaking changes (in the next major), we should probably throw an exception
// instead of just logging a warning here?
// Wrap the failure to append a message "please report this bug"
AssertionFailure assertionFailure = e instanceof AssertionFailure
? (AssertionFailure) e
: new AssertionFailure( e.getMessage(), e );
log.failedToCreateImplicitReindexingAssociationInverseSideResolverNode(
throw log.failedToCreateImplicitReindexingAssociationInverseSideResolverNode(
inversePathByInverseType,
EventContexts.fromType( typeModel ).append( PojoEventContexts.fromPath( path ) ),
assertionFailure.getMessage(), assertionFailure );
continue;
e.getMessage(), e );
}
List<PojoImplicitReindexingAssociationInverseSideResolverNode<Object>> nodesForOrdinal = result.get( ordinal );
if ( nodesForOrdinal == null ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
*/
package org.hibernate.search.mapper.pojo.automaticindexing.impl;

import java.lang.invoke.MethodHandles;
import java.util.Map;

import org.hibernate.search.mapper.pojo.logging.impl.Log;
import org.hibernate.search.mapper.pojo.model.path.impl.PojoPathOrdinalReference;
import org.hibernate.search.mapper.pojo.model.spi.PojoRawTypeIdentifier;
import org.hibernate.search.util.common.logging.impl.LoggerFactory;
import org.hibernate.search.util.common.spi.ToStringTreeAppender;

/**
Expand All @@ -17,6 +20,8 @@
public class PojoImplicitReindexingAssociationInverseSideResolverMarkingNode
extends PojoImplicitReindexingAssociationInverseSideResolverNode<Object> {

private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() );

private final Map<PojoRawTypeIdentifier<?>, PojoPathOrdinalReference> inverseSidePathOrdinalByType;

public PojoImplicitReindexingAssociationInverseSideResolverMarkingNode(
Expand Down Expand Up @@ -45,11 +50,21 @@ void resolveEntitiesToReindex(PojoReindexingAssociationInverseSideCollector coll
PojoRawTypeIdentifier<?> entityTypeIdentifier = context.detectContainingEntityType( entity );
PojoPathOrdinalReference inverseSidePathOrdinal = inverseSidePathOrdinalByType.get( entityTypeIdentifier );
if ( inverseSidePathOrdinal == null ) {
// This shouldn't happen, as this means we encountered an unexpected entity type
// as the target of the association.
// We're ignoring the problem instead of throwing an exception for backwards compatibility,
// as we don't want this feature to cause errors in existing applications.
// TODO HSEARCH-4720 when we can afford breaking changes (in the next major), we should probably throw an exception here?
// This can happen when we have inheritance hierarchy in play:
// Assume having an entity A extended by an indexed entity B. And A has an association to A,e.g.:
//
// class A {
// A a;
// }
// @Indexed class B extends A {
// }
//
// Now in this scenario we would care if an actual instance of `A a` is B. As that would be when `a` has to be re-indexed.
// This means that the inverseSidePathOrdinalByType would contain only the key for `B` type, but not for `A`.
// At runtime, when an actual instance of `A a` is B all good we find a inverse side in the map.
// Otherwise, if the `A a` is an instance of A then the map won't have a reference, as we do not care about such scenario, `A` is not indexed.
//
// And if that happens we just want to return fast without updating the association.
return;
}
collector.updateBecauseOfContainedAssociation( entityTypeIdentifier, entity, inverseSidePathOrdinal.ordinal );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,15 +744,14 @@ SearchException errorDiscoveringJandexIndex(URL codeSourceLocation, String cause
value = "Invalid ObjectPath encountered '%1$s': %2$s")
SearchException invalidObjectPath(ObjectPath path, String causeMessage, @Cause Exception cause);

@LogMessage(level = Logger.Level.WARN)
@Message(id = ID_OFFSET + 122,
value = "An unexpected failure occurred while configuring resolution of association inverse side for reindexing."
+ " This may lead to incomplete reindexing and thus out-of-sync indexes."
+ " The exception is being ignored to preserve backwards compatibility with earlier versions of Hibernate Search."
+ " Failure: %3$s"
+ " %2$s" // Context
+ " Association inverse side: %1$s.")
void failedToCreateImplicitReindexingAssociationInverseSideResolverNode(
SearchException failedToCreateImplicitReindexingAssociationInverseSideResolverNode(
Map<PojoRawTypeModel<?>, PojoModelPathValueNode> inversePathByInverseType,
@FormatWith(EventContextFormatter.class) EventContext context,
String causeMessage, @Cause Exception cause);
Expand Down

0 comments on commit 0767e58

Please sign in to comment.