Merge pull request #612 from FgForrest/611-sortindex-handles-bigdecim…

…al-in-wrong-way

fix(611): SortIndex handles BigDecimal in wrong way

evita_common/src/main/java/io/evitadb/utils/NumberUtils.java

@@ -6,7 +6,7 @@
  *             |  __/\ V /| | || (_| | |_| | |_) |
  *              \___| \_/ |_|\__\__,_|____/|____/
  *
- *   Copyright (c) 2023
+ *   Copyright (c) 2023-2024
  *
  *   Licensed under the Business Source License, Version 1.1 (the "License");
  *   you may not use this file except in compliance with the License.
@@ -207,4 +207,14 @@ public static int[] split(long number) {
 		};
 	}
 
+	/**
+	 * Normalizes BigDecimal value to the form which can be used as a reliable key in a map.
+	 * @param bigDecimal value to be normalized
+	 * @return normalized value
+	 */
+	@Nonnull
+	public static BigDecimal normalize(@Nonnull BigDecimal bigDecimal) {
+		return bigDecimal.stripTrailingZeros();
+	}
+
 }

evita_engine/src/main/java/io/evitadb/index/attribute/SortIndex.java

@@ -49,6 +49,7 @@
 import io.evitadb.store.model.StoragePart;
 import io.evitadb.store.spi.model.storageParts.index.SortIndexStoragePart;
 import io.evitadb.utils.ArrayUtils;
+import io.evitadb.utils.NumberUtils;
 import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 
@@ -58,12 +59,14 @@
 import java.io.Serial;
 import java.io.Serializable;
 import java.lang.reflect.Array;
+import java.math.BigDecimal;
 import java.text.Normalizer;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.UnaryOperator;
 
 import static io.evitadb.utils.Assert.isTrue;
@@ -218,14 +221,13 @@ private static Comparator createComparatorFor(@Nullable Locale locale, @Nonnull
 	private static <T> UnaryOperator<T> createNormalizerFor(@Nonnull ComparatorSource[] comparatorBase) {
 		@SuppressWarnings("unchecked")
 		final UnaryOperator<T>[] normalizers = new UnaryOperator[comparatorBase.length];
-		boolean atLeastOneStringFound = false;
+		boolean atLeastOneNormalizerFound = false;
 		for (int i = 0; i < comparatorBase.length; i++) {
-			if (String.class.isAssignableFrom(comparatorBase[i].type())) {
-				atLeastOneStringFound = true;
-				normalizers[i] = createStringNormalizer(comparatorBase[i]);
-			}
+			final Optional<UnaryOperator<T>> normalizer = createNormalizerFor(comparatorBase[i]);
+			normalizers[i] = normalizer.orElseGet(UnaryOperator::identity);
+			atLeastOneNormalizerFound = atLeastOneNormalizerFound || normalizer.isPresent();
 		}
-		return atLeastOneStringFound ?
+		return atLeastOneNormalizerFound ?
 			new ComparableArrayNormalizer<>(normalizers) : UnaryOperator.identity();
 	}
 
@@ -236,11 +238,13 @@ private static <T> UnaryOperator<T> createNormalizerFor(@Nonnull ComparatorSourc
 	 */
 	@SuppressWarnings("unchecked")
 	@Nonnull
-	private static <T> UnaryOperator<T> createStringNormalizer(@Nonnull ComparatorSource comparatorBase) {
+	private static <T> Optional<UnaryOperator<T>> createNormalizerFor(@Nonnull ComparatorSource comparatorBase) {
 		if (String.class.isAssignableFrom(comparatorBase.type())) {
-			return text -> text == null ? null : (T) Normalizer.normalize(String.valueOf(text), Normalizer.Form.NFD);
+			return Optional.of(text -> text == null ? null : (T) Normalizer.normalize(String.valueOf(text), Normalizer.Form.NFD));
+		} else if (BigDecimal.class.isAssignableFrom(comparatorBase.type())) {
+			return Optional.of(value -> value == null ? null : (T) NumberUtils.normalize((BigDecimal) value));
 		} else {
-			return UnaryOperator.identity();
+			return Optional.empty();
 		}
 	}
 
@@ -259,7 +263,7 @@ public <T extends Comparable<T>> SortIndex(
 			)
 		};
 		this.attributeKey = attributeKey;
-		this.normalizer = createStringNormalizer(this.comparatorBase[0]);
+		this.normalizer = createNormalizerFor(this.comparatorBase[0]).orElseGet(UnaryOperator::identity);
 		this.comparator = createComparatorFor(this.attributeKey.locale(), this.comparatorBase[0]);
 		this.sortedRecords = new TransactionalUnorderedIntArray();
 		//noinspection rawtypes
@@ -302,7 +306,7 @@ public SortIndex(
 		}
 		this.attributeKey = attributeKey;
 		if (this.comparatorBase.length == 1) {
-			this.normalizer = createStringNormalizer(this.comparatorBase[0]);
+			this.normalizer = createNormalizerFor(this.comparatorBase[0]).orElseGet(UnaryOperator::identity);;
 			this.comparator = createComparatorFor(this.attributeKey.locale(), this.comparatorBase[0]);
 		} else {
 			this.normalizer = createNormalizerFor(this.comparatorBase);

evita_engine/src/main/java/io/evitadb/index/attribute/SortIndexChanges.java

@@ -253,7 +253,7 @@ valueIndex, new ValueStartIndex(value, -1),
 	}
 
 	/**
-	 * Computes value index if it hasn't exist yet. Result of this method is memoized. Method computes startung index
+	 * Computes value index if it hasn't exist yet. Result of this method is memoized. Method computes starting index
 	 * (position) of the record ids block that belongs to specific value from {@link SortIndex#sortedRecordsValues} and
 	 * {@link SortIndex#valueCardinalities} information.
 	 */

evita_engine/src/main/java/module-info.java

@@ -94,7 +94,6 @@
 	requires roaringbitmap;
 	requires com.esotericsoftware.kryo;
 
-	requires jdk.httpserver;
 	requires jdk.jfr;
 
 	opens io.evitadb.core.metric.event to evita.common;

evita_functional_tests/src/test/java/io/evitadb/index/attribute/SortIndexTest.java

@@ -6,7 +6,7 @@
  *             |  __/\ V /| | || (_| | |_| | |_) |
  *              \___| \_/ |_|\__\__,_|____/|____/
  *
- *   Copyright (c) 2023
+ *   Copyright (c) 2023-2024
  *
  *   Licensed under the Business Source License, Version 1.1 (the "License");
  *   you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@
 import org.junit.jupiter.params.provider.ArgumentsSource;
 
 import javax.annotation.Nonnull;
+import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -173,6 +174,52 @@ void shouldIndexRecordsAndReturnInDescendingOrder() {
 		);
 	}
 
+	@Test
+	void shouldCorrectlyOrderLocalizedStrings() {
+		final SortIndex sortIndex = new SortIndex(String.class, new AttributeKey("a", new Locale("cs", "CZ")));
+		sortIndex.addRecord("c", 2);
+		sortIndex.addRecord("č", 3);
+		sortIndex.addRecord("a", 1);
+		sortIndex.addRecord("ch", 5);
+		sortIndex.addRecord("ž", 6);
+		sortIndex.addRecord("h", 4);
+		assertArrayEquals(
+			new int[]{1, 2, 3, 4, 5, 6},
+			sortIndex.getAscendingOrderRecordsSupplier().getSortedRecordIds()
+		);
+
+		sortIndex.removeRecord("č", 2);
+		sortIndex.removeRecord("h", 3);
+
+		assertArrayEquals(
+			new int[]{1, 4, 5, 6},
+			sortIndex.getAscendingOrderRecordsSupplier().getSortedRecordIds()
+		);
+	}
+
+	@Test
+	void shouldCorrectlyOrderBigDecimals() {
+		final SortIndex sortIndex = new SortIndex(BigDecimal.class, new AttributeKey("a", new Locale("cs", "CZ")));
+		sortIndex.addRecord(new BigDecimal("0.00"), 1);
+		sortIndex.addRecord(new BigDecimal("0"), 2);
+		sortIndex.addRecord(new BigDecimal("0.000"), 3);
+		sortIndex.addRecord(new BigDecimal("1.1"), 4);
+		sortIndex.addRecord(new BigDecimal("01.10"), 5);
+		sortIndex.addRecord(new BigDecimal("00002"), 6);
+		assertArrayEquals(
+			new int[]{1, 2, 3, 4, 5, 6},
+			sortIndex.getAscendingOrderRecordsSupplier().getSortedRecordIds()
+		);
+
+		sortIndex.removeRecord(new BigDecimal("0.00"), 2);
+		sortIndex.removeRecord(new BigDecimal("0"), 3);
+
+		assertArrayEquals(
+			new int[]{1, 4, 5, 6},
+			sortIndex.getAscendingOrderRecordsSupplier().getSortedRecordIds()
+		);
+	}
+
 	@Test
 	void shouldIndexCompoundRecordsAndReturnInDescendingOrder() {
 		final SortIndex sortIndex = createCompoundIndexWithBaseCardinalities();
@@ -237,7 +284,7 @@ void generationalProofTest(GenerationalTestInput input) {
 		runFor(
 			input,
 			1_000,
-			new TestState(new StringBuilder(), new SortIndex(String.class, new AttributeKey("whatever"))),
+			new TestState(new StringBuilder(256), new SortIndex(String.class, new AttributeKey("whatever"))),
 			(random, testState) -> {
 				final StringBuilder ops = testState.code();
 				ops.append("final SortIndex sortIndex = new SortIndex(String.class);\n")
@@ -311,15 +358,15 @@ void generationalProofTest(GenerationalTestInput input) {
 				);
 
 				return new TestState(
-					new StringBuilder(),
+					new StringBuilder(512),
 					committedResult.get()
 				);
 			}
 		);
 	}
 
 	@Nonnull
-	private SortIndex createIndexWithBaseCardinalities() {
+	private static SortIndex createIndexWithBaseCardinalities() {
 		final SortIndex sortIndex = new SortIndex(String.class, new AttributeKey("a", Locale.ENGLISH));
 		sortIndex.addRecord("B", 5);
 		sortIndex.addRecord("A", 6);
@@ -333,7 +380,7 @@ private SortIndex createIndexWithBaseCardinalities() {
 	}
 
 	@Nonnull
-	private SortIndex createCompoundIndexWithBaseCardinalities() {
+	private static SortIndex createCompoundIndexWithBaseCardinalities() {
 		final SortIndex sortIndex = new SortIndex(
 			new ComparatorSource[]{
 				new ComparatorSource(String.class, OrderDirection.ASC, OrderBehaviour.NULLS_FIRST),

evita_functional_tests/src/test/java/io/evitadb/utils/NumberUtilsTest.java

@@ -6,7 +6,7 @@
  *             |  __/\ V /| | || (_| | |_| | |_) |
  *              \___| \_/ |_|\__\__,_|____/|____/
  *
- *   Copyright (c) 2023
+ *   Copyright (c) 2023-2024
  *
  *   Licensed under the Business Source License, Version 1.1 (the "License");
  *   you may not use this file except in compliance with the License.
@@ -26,11 +26,13 @@
 import org.junit.jupiter.api.Test;
 
 import java.math.BigDecimal;
+import java.util.Map;
 
 import static io.evitadb.utils.NumberUtils.join;
 import static io.evitadb.utils.NumberUtils.split;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
@@ -160,4 +162,21 @@ void shouldConvertBigDecimalToInt() {
 		assertEquals(11020, NumberUtils.convertToInt(new BigDecimal("110.2000"), 2));
 		assertThrows(IllegalArgumentException.class, () -> NumberUtils.convertToInt(new BigDecimal("110.202"), 2));
 	}
+
+	@Test
+	void shouldCorrectlyNormalizeBigDecimalForMapKey() {
+		final Map<BigDecimal, Integer> map = Map.of(
+			NumberUtils.normalize(new BigDecimal("110.2")), 1
+		);
+
+		assertEquals(1, map.get(NumberUtils.normalize(new BigDecimal("110.2"))));
+		assertEquals(1, map.get(NumberUtils.normalize(new BigDecimal("110.200"))));
+		assertEquals(1, map.get(NumberUtils.normalize(new BigDecimal("+110.2"))));
+		assertEquals(1, map.get(NumberUtils.normalize(new BigDecimal("+0110.2"))));
+		assertEquals(1, map.get(NumberUtils.normalize(new BigDecimal("+0110.200"))));
+		assertNull(map.get(NumberUtils.normalize(new BigDecimal("-0110.200"))));
+		assertNull(map.get(NumberUtils.normalize(new BigDecimal("-00110.200"))));
+		assertNull(map.get(NumberUtils.normalize(new BigDecimal("110.0200"))));
+		assertNull(map.get(NumberUtils.normalize(new BigDecimal("1010.0200"))));
+	}
 }