HSEARCH-3487 Check big negative lower bounds for scaled fields

hibernate · May 23, 2019 · fa8e9a7 · fa8e9a7
1 parent b02a72f
commit fa8e9a7
Show file tree

Hide file tree

Showing 8 changed files with 153 additions and 10 deletions.
diff --git a/...sticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java b/...sticsearch/src/main/java/org/hibernate/search/backend/elasticsearch/logging/impl/Log.java
@@ -507,7 +507,7 @@ SearchException unexpectedElasticsearchVersion(ElasticsearchVersion configuredVe
 			+ " If the value is bridged, set '.asBigDecimal().decimalScale( int )' in the bind, else verify your mapping.")
 	SearchException nullDecimalScale(@Param EventContext eventContext);
 
-	@Message(id = ID_OFFSET_3 + 69, value = "The value '%1$s' is too large to be indexed.")
+	@Message(id = ID_OFFSET_3 + 69, value = "The value '%1$s' cannot be indexed because its absolute value is too large.")
 	SearchException scaledNumberTooLarge(Number value);
 
 	@Message(id = ID_OFFSET_3 + 70, value = "Positive decimal scale ['%1$s'] is not allowed for BigInteger fields, since a BigInteger value cannot have any decimal digits.")

diff --git a/...nate/search/backend/elasticsearch/types/codec/impl/ElasticsearchBigDecimalFieldCodec.java b/...nate/search/backend/elasticsearch/types/codec/impl/ElasticsearchBigDecimalFieldCodec.java
@@ -11,6 +11,7 @@
 
 import org.hibernate.search.backend.elasticsearch.gson.impl.JsonElementTypes;
 import org.hibernate.search.backend.elasticsearch.logging.impl.Log;
+import org.hibernate.search.engine.cfg.spi.NumberScaleConstants;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 import com.google.gson.JsonElement;
@@ -33,7 +34,7 @@ public JsonElement encode(BigDecimal value) {
 			return JsonNull.INSTANCE;
 		}
 
-		if ( value.multiply( scalingFactor ).compareTo( BigDecimal.valueOf( Long.MAX_VALUE ) ) > 0 ) {
+		if ( isTooLarge( value ) ) {
 			throw log.scaledNumberTooLarge( value );
 		}
 
@@ -62,4 +63,12 @@ public boolean isCompatibleWith(ElasticsearchFieldCodec<?> obj) {
 		// comparing only their numeric values, they can have different scales
 		return scalingFactor.compareTo( other.scalingFactor ) == 0;
 	}
+
+	private boolean isTooLarge(BigDecimal value) {
+		BigDecimal scaled = value.multiply( scalingFactor );
+		return (
+			scaled.compareTo( NumberScaleConstants.MIN_LONG_AS_BIGDECIMAL ) < 0 ||
+			scaled.compareTo( NumberScaleConstants.MAX_LONG_AS_BIGDECIMAL ) > 0
+		);
+	}
 }
diff --git a/...nate/search/backend/elasticsearch/types/codec/impl/ElasticsearchBigIntegerFieldCodec.java b/...nate/search/backend/elasticsearch/types/codec/impl/ElasticsearchBigIntegerFieldCodec.java
@@ -12,6 +12,7 @@
 
 import org.hibernate.search.backend.elasticsearch.gson.impl.JsonElementTypes;
 import org.hibernate.search.backend.elasticsearch.logging.impl.Log;
+import org.hibernate.search.engine.cfg.spi.NumberScaleConstants;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 import com.google.gson.JsonElement;
@@ -34,7 +35,7 @@ public JsonElement encode(BigInteger value) {
 			return JsonNull.INSTANCE;
 		}
 
-		if ( new BigDecimal( value ).multiply( scalingFactor ).compareTo( BigDecimal.valueOf( Long.MAX_VALUE ) ) > 0 ) {
+		if ( isTooLarge( value ) ) {
 			throw log.scaledNumberTooLarge( value );
 		}
 
@@ -63,4 +64,12 @@ public boolean isCompatibleWith(ElasticsearchFieldCodec<?> obj) {
 		// comparing only their numeric values, they can have different scales
 		return scalingFactor.compareTo( other.scalingFactor ) == 0;
 	}
+
+	public boolean isTooLarge(BigInteger value) {
+		BigDecimal scaled = new BigDecimal( value ).multiply( scalingFactor );
+		return (
+			scaled.compareTo( NumberScaleConstants.MIN_LONG_AS_BIGDECIMAL ) < 0 ||
+			scaled.compareTo( NumberScaleConstants.MAX_LONG_AS_BIGDECIMAL ) > 0
+		);
+	}
 }
diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/logging/impl/Log.java
@@ -490,7 +490,7 @@ SearchException conflictingIdentifierTypesForPredicate(ToDocumentIdentifierValue
 			+ " If the value is bridged, set '.asBigDecimal().decimalScale( int )' in the bind, else verify your mapping.")
 	SearchException nullDecimalScale(@Param EventContext eventContext);
 
-	@Message(id = ID_OFFSET_2 + 81, value = "The value '%1$s' is too large to be indexed.")
+	@Message(id = ID_OFFSET_2 + 81, value = "The value '%1$s' cannot be indexed because its absolute value is too large.")
 	SearchException scaledNumberTooLarge(Number value);
 
 	@Message(id = ID_OFFSET_2 + 82, value = "Positive decimal scale ['%1$s'] is not allowed for BigInteger fields, since a BigInteger value cannot have any decimal digits.")

diff --git a/...java/org/hibernate/search/backend/lucene/types/codec/impl/LuceneBigDecimalFieldCodec.java b/...java/org/hibernate/search/backend/lucene/types/codec/impl/LuceneBigDecimalFieldCodec.java
@@ -12,6 +12,7 @@
 
 import org.hibernate.search.backend.lucene.document.impl.LuceneDocumentBuilder;
 import org.hibernate.search.backend.lucene.logging.impl.Log;
+import org.hibernate.search.engine.cfg.spi.NumberScaleConstants;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 import org.apache.lucene.document.Document;
@@ -23,15 +24,19 @@ public final class LuceneBigDecimalFieldCodec extends AbstractLuceneNumericField
 	private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() );
 
 	private final int decimalScale;
+	private final BigDecimal minScaledValue;
+	private final BigDecimal maxScaledValue;
 
 	public LuceneBigDecimalFieldCodec(boolean projectable, boolean sortable, BigDecimal indexNullAsValue, int decimalScale) {
 		super( projectable, sortable, indexNullAsValue );
 		this.decimalScale = decimalScale;
+		this.minScaledValue = new BigDecimal( NumberScaleConstants.MIN_LONG_AS_BIGINTEGER, decimalScale );
+		this.maxScaledValue = new BigDecimal( NumberScaleConstants.MAX_LONG_AS_BIGINTEGER, decimalScale );
 	}
 
 	@Override
 	void validate(BigDecimal value) {
-		if ( value.compareTo( BigDecimal.valueOf( Long.MAX_VALUE, decimalScale ) ) > 0 ) {
+		if ( isTooLarge( value ) ) {
 			throw log.scaledNumberTooLarge( value );
 		}
 	}
@@ -81,4 +86,8 @@ private Long unscale(BigDecimal value) {
 		// See tck.DecimalScaleIT#roundingMode
 		return value.setScale( decimalScale, RoundingMode.HALF_UP ).unscaledValue().longValue();
 	}
+
+	private boolean isTooLarge(BigDecimal value) {
+		return ( value.compareTo( minScaledValue ) < 0 || value.compareTo( maxScaledValue ) > 0 );
+	}
 }
diff --git a/...java/org/hibernate/search/backend/lucene/types/codec/impl/LuceneBigIntegerFieldCodec.java b/...java/org/hibernate/search/backend/lucene/types/codec/impl/LuceneBigIntegerFieldCodec.java
@@ -13,6 +13,7 @@
 
 import org.hibernate.search.backend.lucene.document.impl.LuceneDocumentBuilder;
 import org.hibernate.search.backend.lucene.logging.impl.Log;
+import org.hibernate.search.engine.cfg.spi.NumberScaleConstants;
 import org.hibernate.search.util.common.logging.impl.LoggerFactory;
 
 import org.apache.lucene.document.Document;
@@ -24,15 +25,19 @@ public final class LuceneBigIntegerFieldCodec extends AbstractLuceneNumericField
 	private static final Log log = LoggerFactory.make( Log.class, MethodHandles.lookup() );
 
 	private final int decimalScale;
+	private final BigDecimal minScaledValue;
+	private final BigDecimal maxScaledValue;
 
 	public LuceneBigIntegerFieldCodec(boolean projectable, boolean sortable, BigInteger indexNullAsValue, int decimalScale) {
 		super( projectable, sortable, indexNullAsValue );
 		this.decimalScale = decimalScale;
+		this.minScaledValue = new BigDecimal( NumberScaleConstants.MIN_LONG_AS_BIGINTEGER, decimalScale );
+		this.maxScaledValue = new BigDecimal( NumberScaleConstants.MAX_LONG_AS_BIGINTEGER, decimalScale );
 	}
 
 	@Override
 	void validate(BigInteger value) {
-		if ( new BigDecimal( value ).compareTo( BigDecimal.valueOf( Long.MAX_VALUE, decimalScale ) ) > 0 ) {
+		if ( isTooLarge( value ) ) {
 			throw log.scaledNumberTooLarge( value );
 		}
 	}
@@ -82,4 +87,9 @@ private Long unscale(BigInteger value) {
 		// See tck.DecimalScaleIT#roundingMode
 		return new BigDecimal( value ).setScale( decimalScale, RoundingMode.HALF_UP ).unscaledValue().longValue();
 	}
+
+	private boolean isTooLarge(BigInteger value) {
+		BigDecimal decimal = new BigDecimal( value );
+		return ( decimal.compareTo( minScaledValue ) < 0 || decimal.compareTo( maxScaledValue ) > 0 );
+	}
 }
diff --git a/engine/src/main/java/org/hibernate/search/engine/cfg/spi/NumberScaleConstants.java b/engine/src/main/java/org/hibernate/search/engine/cfg/spi/NumberScaleConstants.java
@@ -0,0 +1,22 @@
+/*
+ * Hibernate Search, full-text search for your domain model
+ *
+ * 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.search.engine.cfg.spi;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+public class NumberScaleConstants {
+
+	public static final BigDecimal MIN_LONG_AS_BIGDECIMAL = BigDecimal.valueOf( Long.MIN_VALUE );
+	public static final BigDecimal MAX_LONG_AS_BIGDECIMAL = BigDecimal.valueOf( Long.MAX_VALUE );
+	public static final BigInteger MIN_LONG_AS_BIGINTEGER = BigInteger.valueOf( Long.MIN_VALUE );
+	public static final BigInteger MAX_LONG_AS_BIGINTEGER = BigInteger.valueOf( Long.MAX_VALUE );
+
+	private NumberScaleConstants() {
+		// Private constructor, do not use
+	}
+}
diff --git a/...in/java/org/hibernate/search/integrationtest/backend/tck/decimalscale/DecimalScaleIT.java b/...in/java/org/hibernate/search/integrationtest/backend/tck/decimalscale/DecimalScaleIT.java
@@ -359,7 +359,28 @@ public void decimalScale_tooLargeDecimal_scale0_bigDecimal() {
 		} )
 				.assertThrown()
 				.isInstanceOf( SearchException.class )
-				.hasMessageContaining( "is too large to be indexed." );
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
+	}
+
+	@Test
+	public void decimalScale_tooLargeDecimal_scale0_bigDecimal_lowerBound() {
+		setupHelper.withDefaultConfiguration()
+				.withIndex( INDEX_NAME,
+						ctx -> this.decimalScaleIndexMapping = new DecimalScaleIndexMapping( ctx.getSchemaElement(), 0 ),
+						indexManager -> this.indexManager = indexManager )
+				.setup();
+
+		// Provide a value that cannot be represented as a long
+		BigDecimal tooLargeDecimal = BigDecimal.valueOf( Long.MIN_VALUE ).multiply( BigDecimal.TEN );
+
+		SubTest.expectException( () -> {
+			IndexWorkPlan<? extends DocumentElement> workPlan = indexManager.createWorkPlan();
+			workPlan.add( referenceProvider( "1" ), doc -> doc.addValue( decimalScaleIndexMapping.scaled, tooLargeDecimal ) );
+			workPlan.execute().join();
+		} )
+				.assertThrown()
+				.isInstanceOf( SearchException.class )
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
 	}
 
 	@Test
@@ -380,7 +401,28 @@ public void decimalScale_tooLargeDecimal_scale0_bigInteger() {
 		} )
 				.assertThrown()
 				.isInstanceOf( SearchException.class )
-				.hasMessageContaining( "is too large to be indexed." );
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
+	}
+
+	@Test
+	public void decimalScale_tooLargeDecimal_scale0_bigInteger_lowerBound() {
+		setupHelper.withDefaultConfiguration()
+				.withIndex( INDEX_NAME,
+						ctx -> this.integerScaleIndexMapping = new IntegerScaleIndexMapping( ctx.getSchemaElement(), 0 ),
+						indexManager -> this.indexManager = indexManager )
+				.setup();
+
+		// Provide a value that cannot be represented as a long
+		BigInteger tooLargeInteger = BigInteger.valueOf( Long.MIN_VALUE ).multiply( BigInteger.TEN );
+
+		SubTest.expectException( () -> {
+			IndexWorkPlan<? extends DocumentElement> workPlan = indexManager.createWorkPlan();
+			workPlan.add( referenceProvider( "1" ), doc -> doc.addValue( integerScaleIndexMapping.scaled, tooLargeInteger ) );
+			workPlan.execute().join();
+		} )
+				.assertThrown()
+				.isInstanceOf( SearchException.class )
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
 	}
 
 	@Test
@@ -401,7 +443,28 @@ public void decimalScale_tooLargeDecimal_scale2_bigDecimal() {
 		} )
 				.assertThrown()
 				.isInstanceOf( SearchException.class )
-				.hasMessageContaining( "is too large to be indexed." );
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
+	}
+
+	@Test
+	public void decimalScale_tooLargeDecimal_scale2_bigDecimal_lowerBound() {
+		setupHelper.withDefaultConfiguration()
+				.withIndex( INDEX_NAME,
+						ctx -> this.decimalScaleIndexMapping = new DecimalScaleIndexMapping( ctx.getSchemaElement(), 2 ),
+						indexManager -> this.indexManager = indexManager )
+				.setup();
+
+		// Provide a value that if it were divided by 10, could not be represented as a long, because the scale of 2
+		BigDecimal tooLargeDecimal = BigDecimal.valueOf( Long.MIN_VALUE ).divide( BigDecimal.TEN );
+
+		SubTest.expectException( () -> {
+			IndexWorkPlan<? extends DocumentElement> workPlan = indexManager.createWorkPlan();
+			workPlan.add( referenceProvider( "1" ), doc -> doc.addValue( decimalScaleIndexMapping.scaled, tooLargeDecimal ) );
+			workPlan.execute().join();
+		} )
+				.assertThrown()
+				.isInstanceOf( SearchException.class )
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
 	}
 
 	@Test
@@ -422,7 +485,28 @@ public void decimalScale_tooLargeDecimal_scaleMinus2_bigInteger() {
 		} )
 				.assertThrown()
 				.isInstanceOf( SearchException.class )
-				.hasMessageContaining( "is too large to be indexed." );
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
+	}
+
+	@Test
+	public void decimalScale_tooLargeDecimal_scaleMinus2_bigInteger_lowerBound() {
+		setupHelper.withDefaultConfiguration()
+				.withIndex( INDEX_NAME,
+						ctx -> this.integerScaleIndexMapping = new IntegerScaleIndexMapping( ctx.getSchemaElement(), -2 ),
+						indexManager -> this.indexManager = indexManager )
+				.setup();
+
+		// Provide a value that if it were multiplied by 100, could not be represented as a long, because the scale of -2
+		BigInteger tooLargeInteger = BigInteger.valueOf( Long.MIN_VALUE ).multiply( new BigInteger( "1000" ) );
+
+		SubTest.expectException( () -> {
+			IndexWorkPlan<? extends DocumentElement> workPlan = indexManager.createWorkPlan();
+			workPlan.add( referenceProvider( "1" ), doc -> doc.addValue( integerScaleIndexMapping.scaled, tooLargeInteger ) );
+			workPlan.execute().join();
+		} )
+				.assertThrown()
+				.isInstanceOf( SearchException.class )
+				.hasMessageContaining( "cannot be indexed because its absolute value is too large." );
 	}
 
 	@Test