Fix range facets (#679)

Author: lauraluiz

common/app/com/commercetools/sunrise/search/facetedsearch/RangeUtils.java

@@ -8,6 +8,7 @@
 import org.apache.commons.lang3.tuple.Pair;
 
 import javax.annotation.Nullable;
+import java.math.BigDecimal;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -21,11 +22,15 @@ public final class RangeUtils {
     private RangeUtils() {
     }
 
-    public static Map<FacetRange<String>, RangeStats> mapRangeToStats(final RangeFacetResult facetResult) {
-        final Map<FacetRange<String>, RangeStats> rangeToStats = new HashMap<>();
+    public static Map<String, RangeStats> mapRangeToStats(final RangeFacetResult facetResult) {
+        final Map<String, RangeStats> rangeToStats = new HashMap<>();
         facetResult.getRanges()
-                .forEach(stats -> parseFacetRange(stats.getLowerEndpoint(), stats.getUpperEndpoint())
-                        .ifPresent(range -> rangeToStats.put(range, stats)));
+                .forEach(stats -> {
+                    final String lowerEndpoint = cleanEndpointDecimal(stats.getLowerEndpoint());
+                    final String upperEndpoint = cleanEndpointDecimal(stats.getUpperEndpoint());
+                    parseFacetRange(lowerEndpoint, upperEndpoint)
+                            .ifPresent(range -> rangeToStats.put(range.toString(), stats));
+                });
         return rangeToStats;
     }
 
@@ -41,6 +46,7 @@ public static Optional<FacetRange<String>> parseFacetRange(@Nullable final Strin
 
     /**
      * Parses a range of the form {@code (x to y)} to a {@link FilterRange}.
+     * Ranges of the form {@code (* to *)} are not supported.
      * @param rangeAsString range of the form {@code (x to y)}
      * @return the {@link FilterRange} corresponding to that range, or empty if it could not be parsed
      */
@@ -55,11 +61,13 @@ public static Optional<FilterRange<String>> parseFilterRange(final String rangeA
                     } else {
                         return FilterRange.atMost(pair.getRight());
                     }
-                });
+                })
+                .filter(RangeUtils::hasNoBounds);
     }
 
     /**
      * Parses a range of the form {@code (x to y)} to a {@link FacetRange}.
+     * Ranges of the form {@code (* to *)} are not supported.
      * @param rangeAsString range of the form {@code (x to y)}
      * @return the {@link FacetRange} corresponding to that range, or empty if it could not be parsed
      */
@@ -74,7 +82,8 @@ public static Optional<FacetRange<String>> parseFacetRange(final String rangeAsS
                     } else {
                         return FacetRange.lessThan(pair.getRight());
                     }
-                });
+                })
+                .filter(RangeUtils::hasNoBounds);
     }
 
     @Nullable
@@ -89,17 +98,48 @@ private static Pair<String, String> parseRangeToPair(final String rangeAsString)
     }
 
     private static String buildRange(@Nullable final String lowerEndpoint, @Nullable final String upperEndpoint) {
-        return String.format("(\"%s\" to \"%s\")",
+        return String.format("(%s to %s)",
                 boundEndpointOrAsterisk(lowerEndpoint),
                 boundEndpointOrAsterisk(upperEndpoint));
     }
 
     @Nullable
     private static String boundEndpointOrNull(@Nullable final String endpoint) {
-        return endpoint == null || endpoint.equals("*") ? null : endpoint;
+        return isEndpointBoundless(endpoint) ? null : endpoint;
     }
 
     private static String boundEndpointOrAsterisk(@Nullable final String endpoint) {
-        return endpoint == null ? "*" : endpoint;
+        return isEndpointBoundless(endpoint) ? "*" : endpoint;
+    }
+
+    private static boolean isEndpointBoundless(@Nullable final String endpoint) {
+        return endpoint == null || endpoint.equals("*");
+    }
+
+    private static <T extends Comparable<T>> boolean hasNoBounds(final FacetRange<T> range) {
+        return range.lowerBound() != null || range.upperBound() != null;
+    }
+
+    private static <T extends Comparable<T>> boolean hasNoBounds(final FilterRange<T> range) {
+        return range.lowerBound() != null || range.upperBound() != null;
+    }
+
+    /**
+     * Cleans the decimal part of the endpoint value due to the backend answering with decimals but not accepting them.
+     * For example, the backend might answer with 9002.0, but it needs to be converted to 9002.
+     * @param endpointValue the value of the endpoint to be cleaned
+     * @return the value of the endpoint without the decimal part, in case the value was a number
+     */
+    @Nullable
+    private static String cleanEndpointDecimal(@Nullable final String endpointValue) {
+        if (endpointValue != null) {
+            try {
+                final int value = new BigDecimal(endpointValue).intValue();
+                return String.valueOf(value);
+            } catch (NumberFormatException e) {
+                // Fallbacks to default case
+            }
+        }
+        return endpointValue;
     }
 }

common/app/com/commercetools/sunrise/search/facetedsearch/bucketranges/BucketRangeUtils.java

@@ -14,27 +14,17 @@ public final class BucketRangeUtils {
 
     public static List<FilterRange<String>> optionsToFilterRange(final List<BucketRangeFacetedSearchFormOption> rangeOptions) {
         return rangeOptions.stream()
-                .map(BucketRangeUtils::optionToFilterRange)
+                .map(option -> parseFilterRange(option.getValue()))
                 .filter(Optional::isPresent)
                 .map(Optional::get)
                 .collect(toList());
     }
 
     public static List<FacetRange<String>> optionsToFacetRange(final List<BucketRangeFacetedSearchFormOption> rangeOptions) {
         return rangeOptions.stream()
-                .map(BucketRangeUtils::optionToFacetRange)
+                .map(option -> parseFacetRange(option.getValue()))
                 .filter(Optional::isPresent)
                 .map(Optional::get)
                 .collect(toList());
     }
-
-    public static Optional<FilterRange<String>> optionToFilterRange(final BucketRangeFacetedSearchFormOption rangeOption) {
-        return parseFilterRange(rangeOption.getValue())
-                .filter(range -> range.isBounded());
-    }
-
-    public static Optional<FacetRange<String>> optionToFacetRange(final BucketRangeFacetedSearchFormOption rangeOption) {
-        return parseFacetRange(rangeOption.getValue())
-                .filter(range -> range.isBounded());
-    }
 }

common/app/com/commercetools/sunrise/search/facetedsearch/bucketranges/viewmodels/BucketRangeFacetViewModelFactory.java

@@ -7,7 +7,6 @@
 import com.commercetools.sunrise.search.facetedsearch.viewmodels.AbstractFacetWithOptionsViewModelFactory;
 import com.commercetools.sunrise.search.facetedsearch.viewmodels.FacetOptionViewModel;
 import io.sphere.sdk.search.RangeFacetResult;
-import io.sphere.sdk.search.model.FacetRange;
 import io.sphere.sdk.search.model.RangeStats;
 import play.mvc.Http;
 
@@ -18,7 +17,7 @@
 import java.util.Objects;
 
 import static com.commercetools.sunrise.search.facetedsearch.RangeUtils.mapRangeToStats;
-import static com.commercetools.sunrise.search.facetedsearch.bucketranges.BucketRangeUtils.optionToFacetRange;
+import static com.commercetools.sunrise.search.facetedsearch.RangeUtils.parseFacetRange;
 import static java.util.stream.Collectors.toList;
 
 @RequestScoped
@@ -67,11 +66,11 @@ protected void fillLimitedOptions(final BucketRangeFacetViewModel viewModel, fin
         final List<String> selectedValues = settings.getAllSelectedOptions(Http.Context.current()).stream()
                 .map(FormOption::getFieldValue)
                 .collect(toList());
-        final Map<FacetRange<String>, RangeStats> rangeToStatsMap = mapRangeToStats(facetResult);
+        final Map<String, RangeStats> rangeToStatsMap = mapRangeToStats(facetResult);
         final List<FacetOptionViewModel> options = new ArrayList<>();
         settings.getOptions()
-                .forEach(option -> optionToFacetRange(option)
-                        .map(rangeToStatsMap::get)
+                .forEach(option -> parseFacetRange(option.getValue())
+                        .map(range -> rangeToStatsMap.get(range.toString()))
                         .filter(Objects::nonNull)
                         .ifPresent(rangeStats -> options.add(bucketRangeFacetOptionViewModelFactory.create(rangeStats, option, selectedValues))));
         viewModel.setLimitedOptions(options);