Skip to content

Commit

Permalink
HSEARCH-2457 Make sure ContextualExceptionBridgeHelper provides as mu…
Browse files Browse the repository at this point in the history 
…ch information as possible in exception messages

We must do with whatever info we already have in this class, and can't
add externally provided information, because the implemented interface,
ConversionContext, is SPI, and we don't want to break that.
  • Loading branch information
@yrodiere @gsmet
yrodiere authored and gsmet committed Nov 23, 2016
1 parent 242712f commit 73ca2ca
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 34 deletions.
60 changes: 42 additions & 18 deletions .../src/main/java/org/hibernate/search/bridge/util/impl/ContextualExceptionBridgeHelper.java
View file
Expand Up @@ -17,6 +17,7 @@
import org.hibernate.search.bridge.StringBridge;
import org.hibernate.search.bridge.TwoWayFieldBridge;
import org.hibernate.search.bridge.spi.ConversionContext;
import org.hibernate.search.util.StringHelper;

/**
* Wrap the exception with an exception provide contextual feedback.
Expand All @@ -37,7 +38,7 @@ public final class ContextualExceptionBridgeHelper implements ConversionContext
private TwoWayFieldBridge twoWayBridge;

//Reused helpers:
private final ArrayList<String> path = new ArrayList<String>( 5 ); //half of usual increment size as I don't expect much
private final ArrayList<String> propertyPath = new ArrayList<String>( 5 ); //half of usual increment size as I don't expect much
private final OneWayConversionContextImpl oneWayAdapter = new OneWayConversionContextImpl();
private final TwoWayConversionContextImpl twoWayAdapter = new TwoWayConversionContextImpl();
private final StringConversionContextImpl stringAdapter = new StringConversionContextImpl();
Expand All @@ -50,16 +51,16 @@ public ConversionContext setClass(Class<?> clazz) {

@Override
public ConversionContext pushProperty(String propertyName) {
path.add( propertyName );
propertyPath.add( propertyName );
return this;
}

@Override
public ConversionContext popProperty() {
if ( path.size() == 0 ) {
if ( propertyPath.size() == 0 ) {
throw new IllegalStateException( "Trying to pop a property from an empty conversion context" );
}
path.remove( path.size() - 1 );
propertyPath.remove( propertyPath.size() - 1 );
return this;
}

Expand All @@ -69,19 +70,42 @@ public ConversionContext pushIdentifierProperty() {
return this;
}

protected BridgeException buildBridgeException(Exception e, String method) {
StringBuilder error = new StringBuilder( "Exception while calling bridge#" ).append( method );
protected BridgeException buildBridgeException(Exception e, String method, String fieldName, Object bridge) {
StringBuilder errorMessage = new StringBuilder( "Exception while calling bridge#" ).append( method );
if ( clazz != null ) {
error.append( "\n\tclass: " ).append( clazz.getName() );
errorMessage.append( "\n\tentity class: " ).append( clazz.getName() );
}
if ( path.size() > 0 ) {
error.append( "\n\tpath: " );
for ( String pathNode : path ) {
error.append( pathNode ).append( "." );
if ( propertyPath.size() > 0 ) {
errorMessage.append( "\n\tentity property path: " );
for ( String pathNode : propertyPath ) {
errorMessage.append( pathNode ).append( "." );
}
error.deleteCharAt( error.length() - 1 );
errorMessage.deleteCharAt( errorMessage.length() - 1 );
}
throw new BridgeException( error.toString(), e );
if ( StringHelper.isNotEmpty( fieldName ) ) {
errorMessage.append( "\n\tdocument field name: " ).append( fieldName );
}

Throwable exceptionWhenBuildingBridgeString = null;
if ( bridge != null ) {
String bridgeAsString = null;
try {
bridgeAsString = bridge.toString();
}
catch (Exception e2) {
// Make sure we support failing toString() implementations (field bridges may be custom)
exceptionWhenBuildingBridgeString = e;
bridgeAsString = "<toString() on the fieldbridge thrown " + e.getClass() + ">";
}
errorMessage.append( "\n\tfield bridge: " ).append( bridgeAsString );
}

BridgeException exception = new BridgeException( errorMessage.toString(), e );
if ( exceptionWhenBuildingBridgeString != null ) {
exception.addSuppressed( exceptionWhenBuildingBridgeString );
}

throw exception;
}

@Override
Expand Down Expand Up @@ -110,7 +134,7 @@ public void set(String name, Object value, Document document, LuceneOptions luce
oneWayBridge.set( name, value, document, luceneOptions );
}
catch (RuntimeException e) {
throw buildBridgeException( e, "set" );
throw buildBridgeException( e, "set", name, oneWayBridge );
}
}
}
Expand All @@ -123,7 +147,7 @@ public Object get(String name, Document document) {
return twoWayBridge.get( name, document );
}
catch (RuntimeException e) {
throw buildBridgeException( e, "get" );
throw buildBridgeException( e, "get", name, twoWayBridge );
}
}

Expand All @@ -133,7 +157,7 @@ public String objectToString(Object object) {
return twoWayBridge.objectToString( object );
}
catch (RuntimeException e) {
throw buildBridgeException( e, "objectToString" );
throw buildBridgeException( e, "objectToString", null, twoWayBridge );
}
}

Expand All @@ -143,7 +167,7 @@ public void set(String name, Object value, Document document, LuceneOptions luce
twoWayBridge.set( name, value, document, luceneOptions );
}
catch (RuntimeException e) {
throw buildBridgeException( e, "set" );
throw buildBridgeException( e, "set", name, twoWayBridge );
}
}
}
Expand All @@ -156,7 +180,7 @@ public String objectToString(Object object) {
return stringBridge.objectToString( object );
}
catch (RuntimeException e) {
throw buildBridgeException( e, "objectToString" );
throw buildBridgeException( e, "objectToString", null, stringBridge );
}
}
}
Expand Down
45 changes: 29 additions & 16 deletions orm/src/test/java/org/hibernate/search/test/bridge/BridgeConversionErrorTest.java
View file
Expand Up @@ -51,8 +51,10 @@ public void testClassBridgeError() throws Exception {
catch (Exception e) {
Throwable cause = e.getCause();
assertTrue( cause instanceof BridgeException );
String expectedErrorMessage = "Exception while calling bridge#set\n" +
"\tclass: org.hibernate.search.test.bridge.BridgeConversionErrorTest$ClassBridged";
String expectedErrorMessage = "Exception while calling bridge#set"
+ "\n\tentity class: org.hibernate.search.test.bridge.BridgeConversionErrorTest$ClassBridged"
+ "\n\tdocument field name: test"
+ "\n\tfield bridge: <result of ExceptionThrowingBridge.toString()>";
assertEquals( "Wrong error message", expectedErrorMessage, cause.getMessage() );
}
}
Expand All @@ -70,9 +72,11 @@ public void testFieldBridgeError() throws Exception {
catch (Exception e) {
Throwable cause = e.getCause();
assertTrue( cause instanceof BridgeException );
String expectedErrorMessage = "Exception while calling bridge#set\n" +
"\tclass: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity\n" +
"\tpath: name";
String expectedErrorMessage = "Exception while calling bridge#set"
+ "\n\tentity class: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity"
+ "\n\tentity property path: name"
+ "\n\tdocument field name: overriddenFieldName"
+ "\n\tfield bridge: <result of ExceptionThrowingBridge.toString()>";
assertEquals( "Wrong error message", expectedErrorMessage, cause.getMessage() );
}
}
Expand All @@ -92,9 +96,11 @@ public void testEmbeddedBridgeError() throws Exception {
catch (Exception e) {
Throwable cause = e.getCause();
assertTrue( cause instanceof BridgeException );
String expectedErrorMessage = "Exception while calling bridge#set\n" +
"\tclass: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity\n" +
"\tpath: embedded.name";
String expectedErrorMessage = "Exception while calling bridge#set"
+ "\n\tentity class: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity"
+ "\n\tentity property path: embedded.name"
+ "\n\tdocument field name: overriddenEmbeddedPrefix.overriddenEmbeddedFieldName"
+ "\n\tfield bridge: <result of ExceptionThrowingBridge.toString()>";
assertEquals( "Wrong error message", expectedErrorMessage, cause.getMessage() );
}
}
Expand All @@ -115,9 +121,11 @@ public void testEmbeddedEmbeddedBridgeError() throws Exception {
catch (Exception e) {
Throwable cause = e.getCause();
assertTrue( cause instanceof BridgeException );
String expectedErrorMessage = "Exception while calling bridge#set\n" +
"\tclass: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity\n" +
"\tpath: embedded.embeddedEmbedded.name";
String expectedErrorMessage = "Exception while calling bridge#set"
+ "\n\tentity class: org.hibernate.search.test.bridge.BridgeConversionErrorTest$SimpleEntity"
+ "\n\tentity property path: embedded.embeddedEmbedded.name"
+ "\n\tdocument field name: overriddenEmbeddedPrefix.overriddenEmbeddedEmbeddedPrefix.overriddenEmbeddedEmbeddedFieldName"
+ "\n\tfield bridge: <result of ExceptionThrowingBridge.toString()>";
assertEquals( "Wrong error message", expectedErrorMessage, cause.getMessage() );
}
}
Expand All @@ -139,6 +147,11 @@ public void set(String name, Object value, Document document, LuceneOptions luce
throw new RuntimeException( "boom" );
}
}

@Override
public String toString() {
return "<result of ExceptionThrowingBridge.toString()>";
}
}

@Entity
Expand All @@ -159,11 +172,11 @@ public static class SimpleEntity {
@GeneratedValue
private long id;

@Field
@Field(name = "overriddenFieldName")
@org.hibernate.search.annotations.FieldBridge(impl = ExceptionThrowingBridge.class)
private String name;

@IndexedEmbedded
@IndexedEmbedded(prefix = "overriddenEmbeddedPrefix.")
@OneToOne(cascade = CascadeType.ALL)
private EmbeddedEntity embedded;

Expand All @@ -186,7 +199,7 @@ public static class EmbeddedEntity {
@GeneratedValue
private long id;

@Field
@Field(name = "overriddenEmbeddedFieldName")
@org.hibernate.search.annotations.FieldBridge(impl = ExceptionThrowingBridge.class)
private String name;

Expand All @@ -197,7 +210,7 @@ public EmbeddedEntity(String name) {
public EmbeddedEntity() {
}

@IndexedEmbedded
@IndexedEmbedded(prefix = "overriddenEmbeddedEmbeddedPrefix.")
@OneToOne(cascade = CascadeType.ALL)
private EmbeddedEmbeddedEntity embeddedEmbedded;

Expand All @@ -213,7 +226,7 @@ public static class EmbeddedEmbeddedEntity {
@GeneratedValue
private long id;

@Field
@Field(name = "overriddenEmbeddedEmbeddedFieldName")
@org.hibernate.search.annotations.FieldBridge(impl = ExceptionThrowingBridge.class)
private String name;

Expand Down

0 comments on commit 73ca2ca

Please sign in to comment.