Make properties sortable (#2972)

FAForever · May 10, 2023 · 8cab3e4 · 8cab3e4
1 parent ed3b029
commit 8cab3e4
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 90 deletions.
diff --git a/src/main/java/com/faforever/client/map/MapService.java b/src/main/java/com/faforever/client/map/MapService.java
@@ -672,10 +672,10 @@ public CompletableFuture<Tuple2<List<MapVersionBean>, Integer>> getOwnedMapsWith
   @Cacheable(value = CacheNames.MAPS, sync = true)
   public CompletableFuture<Tuple2<List<MapVersionBean>, Integer>> findByQueryWithPageCount(SearchConfig searchConfig,
                                                                                            int count, int page) {
-    SortConfig sortConfig = searchConfig.getSortConfig();
+    SortConfig sortConfig = searchConfig.sortConfig();
     ElideNavigatorOnCollection<Map> navigator = ElideNavigator.of(Map.class).collection()
-        .addSortingRule(sortConfig.getSortProperty(), sortConfig.getSortOrder().equals(SortOrder.ASC));
-    return getMapPage(navigator, searchConfig.getSearchQuery(), count, page);
+        .addSortingRule(sortConfig.sortProperty(), sortConfig.sortOrder().equals(SortOrder.ASC));
+    return getMapPage(navigator, searchConfig.searchQuery(), count, page);
   }
 
   @Cacheable(value = CacheNames.MAPS, sync = true)

diff --git a/src/main/java/com/faforever/client/map/MapVaultController.java b/src/main/java/com/faforever/client/map/MapVaultController.java
@@ -36,9 +36,6 @@
 @Slf4j
 @Component
 @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)
-// TODO I'd like to avoid the additional "getMost*" methods and always use the map query function instead, however,
-// this is currently not viable since Elide can't yet sort by relationship attributes. Once it supports that
-// (see https://github.com/yahoo/elide/issues/353), this can be refactored.
 public class MapVaultController extends VaultEntityController<MapVersionBean> {
 
   private final MapService mapService;

diff --git a/src/main/java/com/faforever/client/mod/ModService.java b/src/main/java/com/faforever/client/mod/ModService.java
@@ -518,11 +518,11 @@ public CompletableFuture<List<FeaturedModBean>> getFeaturedMods() {
   @Cacheable(value = CacheNames.MODS, sync = true)
   public CompletableFuture<Tuple2<List<ModVersionBean>, Integer>> findByQueryWithPageCount(SearchConfig searchConfig,
                                                                                            int count, int page) {
-    SortConfig sortConfig = searchConfig.getSortConfig();
+    SortConfig sortConfig = searchConfig.sortConfig();
     ElideNavigatorOnCollection<Mod> navigator = ElideNavigator.of(Mod.class)
         .collection()
-        .addSortingRule(sortConfig.getSortProperty(), sortConfig.getSortOrder().equals(SortOrder.ASC));
-    return getModPage(navigator, searchConfig.getSearchQuery(), count, page);
+        .addSortingRule(sortConfig.sortProperty(), sortConfig.sortOrder().equals(SortOrder.ASC));
+    return getModPage(navigator, searchConfig.searchQuery(), count, page);
   }
 
   @Cacheable(value = CacheNames.MODS, sync = true)

diff --git a/src/main/java/com/faforever/client/query/SearchablePropertyMappings.java b/src/main/java/com/faforever/client/query/SearchablePropertyMappings.java
@@ -1,46 +1,42 @@
 package com.faforever.client.query;
 
 import com.google.common.collect.ImmutableMap;
-import lombok.Data;
-import lombok.EqualsAndHashCode;
 
 import java.util.Map;
 
 /**
- * Contains mappings of searchable properties (as the API expects it) to their respective i18n key and if they are considered for sorting the result list. The reason
- * the i18n keys are not build dynamically is that it makes it impossible for the IDE to detect which key is used where,
- * breaks its refactor capability, and the actual UI text might depend on the context it is used in. Also, this way
- * i18n keys and API keys are nicely decoupled and can therefore be changed independently.
+ * Contains mappings of searchable properties (as the API expects it) to their respective i18n key and if they are
+ * considered for sorting the result list. The reason the i18n keys are not build dynamically is that it makes it
+ * impossible for the IDE to detect which key is used where, breaks its refactor capability, and the actual UI text
+ * might depend on the context it is used in. Also, this way i18n keys and API keys are nicely decoupled and can
+ * therefore be changed independently.
  */
 public class SearchablePropertyMappings {
-  public static final String NEWEST_KEY = "latestVersion.createTime";
-  public static final String HIGHEST_RATED_KEY = "reviewsSummary.lowerBound";
-
   public static final Map<String, Property> GAME_PROPERTY_MAPPING = ImmutableMap.<String, Property>builder()
-      .put("playerStats.player.login", new Property("game.player.username", false))
-      .put("featuredMod.technicalName", new Property("featuredMod.technicalName", false))
-      .put("mapVersion.map.displayName", new Property("game.map.displayName", false))
-      .put("playerStats.faction", new Property("game.player.faction", false))
-      .put("playerStats.startSpot", new Property("game.player.startSpot", false))
-      .put("mapVersion.maxPlayers", new Property("map.maxPlayers", false))
-      .put("mapVersion.ranked", new Property("game.map.isRanked", false))
+      .put("playerStats.player.login", new Property("game.player.username", true))
+      .put("featuredMod.technicalName", new Property("featuredMod.technicalName", true))
+      .put("mapVersion.map.displayName", new Property("game.map.displayName", true))
+      .put("playerStats.faction", new Property("game.player.faction", true))
+      .put("playerStats.startSpot", new Property("game.player.startSpot", true))
+      .put("mapVersion.maxPlayers", new Property("map.maxPlayers", true))
+      .put("mapVersion.ranked", new Property("game.map.isRanked", true))
       .put("id", new Property("game.id", true))
-      .put("playerStats.player.id", new Property("game.player.id", false))
+      .put("playerStats.player.id", new Property("game.player.id", true))
       .put("name", new Property("game.title", true))
       .put("replayTicks", new Property("game.replayTicks", true))
       .put("startTime", new Property("game.startTime", true))
       .put("endTime", new Property("game.endTime", true))
       .put("validity", new Property("game.validity", true))
       .put("victoryCondition", new Property("game.victoryCondition", true))
-      .put("playerStats.team", new Property("game.player.team", false))
-      .put("host.login", new Property("game.host.username", false))
-      .put("host.id", new Property("game.host.id", false))
-      .put("featuredMod.displayName", new Property("featuredMod.displayName", false))
-      .put("mapVersion.description", new Property("map.description", false))
-      .put("mapVersion.width", new Property("map.widthPixels", false))
-      .put("mapVersion.height", new Property("map.heightPixels", false))
-      .put("mapVersion.folderName", new Property("game.map.folderName", false))
-      .put("mapVersion.map.author.login", new Property("game.map.author", false))
+      .put("playerStats.team", new Property("game.player.team", true))
+      .put("host.login", new Property("game.host.username", true))
+      .put("host.id", new Property("game.host.id", true))
+      .put("featuredMod.displayName", new Property("featuredMod.displayName", true))
+      .put("mapVersion.description", new Property("map.description", true))
+      .put("mapVersion.width", new Property("map.widthPixels", true))
+      .put("mapVersion.height", new Property("map.heightPixels", true))
+      .put("mapVersion.folderName", new Property("game.map.folderName", true))
+      .put("mapVersion.map.author.login", new Property("game.map.author", true))
 
       .build();
 
@@ -52,35 +48,30 @@ public class SearchablePropertyMappings {
 
       .put("latestVersion.createTime", new Property("map.uploadedDateTime", true))
       .put("latestVersion.updateTime", new Property("map.updatedDateTime", true))
-      .put("latestVersion.description", new Property("map.description", false))
+      .put("latestVersion.description", new Property("map.description", true))
       .put("latestVersion.maxPlayers", new Property("map.maxPlayers", true))
-      .put("latestVersion.width", new Property("map.widthPixels", false))
-      .put("latestVersion.height", new Property("map.heightPixels", false))
-      .put("latestVersion.folderName", new Property("map.folderName", false))
-      .put("latestVersion.ranked", new Property("map.ranked", false))
-      .put("latestVersion.id", new Property("map.versionId", false))
+      .put("latestVersion.width", new Property("map.widthPixels", true))
+      .put("latestVersion.height", new Property("map.heightPixels", true))
+      .put("latestVersion.folderName", new Property("map.folderName", true))
+      .put("latestVersion.ranked", new Property("map.ranked", true))
+      .put("latestVersion.id", new Property("map.versionId", true))
 
       .build();
 
   public static final Map<String, Property> MOD_PROPERTY_MAPPING = ImmutableMap.<String, Property>builder()
       .put("displayName", new Property("mod.displayName", true))
       .put("author", new Property("mod.author", true))
-      .put("uploader.login", new Property("mod.uploader.login", false))
+      .put("uploader.login", new Property("mod.uploader.login", true))
       .put("latestVersion.createTime", new Property("mod.uploadedDateTime", true))
       .put("latestVersion.updateTime", new Property("mod.updatedDateTime", true))
-      .put("latestVersion.description", new Property("mod.description", false))
-      .put("latestVersion.id", new Property("mod.id", false))
-      .put("latestVersion.uid", new Property("mod.uid", false))
-      .put("latestVersion.type", new Property("mod.type", false))
-      .put("latestVersion.ranked", new Property("mod.ranked", false))
-      .put("latestVersion.filename", new Property("mod.filename", false))
+      .put("latestVersion.description", new Property("mod.description", true))
+      .put("latestVersion.id", new Property("mod.id", true))
+      .put("latestVersion.uid", new Property("mod.uid", true))
+      .put("latestVersion.type", new Property("mod.type", true))
+      .put("latestVersion.ranked", new Property("mod.ranked", true))
+      .put("latestVersion.filename", new Property("mod.filename", true))
 
       .build();
 
-  @Data
-  @EqualsAndHashCode(of = "i18nKey")
-  public static class Property {
-    private final String i18nKey;
-    private final boolean sortable;
-  }
+  public record Property(String i18nKey, boolean sortable) {}
 }
diff --git a/src/main/java/com/faforever/client/query/SpecificationController.java b/src/main/java/com/faforever/client/query/SpecificationController.java
@@ -133,7 +133,7 @@ public ComparisonOperator fromString(String string) {
     propertyField.setConverter(new StringConverter<String>() {
       @Override
       public String toString(String object) {
-        return i18n.get(properties.get(object).getI18nKey());
+        return i18n.get(properties.get(object).i18nKey());
       }
 
       @Override

diff --git a/src/main/java/com/faforever/client/replay/ReplayService.java b/src/main/java/com/faforever/client/replay/ReplayService.java
@@ -423,11 +423,11 @@ public CompletableFuture<Tuple2<List<ReplayBean>, Integer>> getHighestRatedRepla
   @Cacheable(value = CacheNames.REPLAYS_SEARCH, sync = true)
   public CompletableFuture<Tuple2<List<ReplayBean>, Integer>> findByQueryWithPageCount(SearchConfig searchConfig,
                                                                                        int count, int page) {
-    SortConfig sortConfig = searchConfig.getSortConfig();
+    SortConfig sortConfig = searchConfig.sortConfig();
     ElideNavigatorOnCollection<Game> navigator = ElideNavigator.of(Game.class)
         .collection()
-        .addSortingRule(sortConfig.getSortProperty(), sortConfig.getSortOrder().equals(SortOrder.ASC));
-    return getReplayPage(navigator, searchConfig.getSearchQuery(), count, page);
+        .addSortingRule(sortConfig.sortProperty(), sortConfig.sortOrder().equals(SortOrder.ASC));
+    return getReplayPage(navigator, searchConfig.searchQuery(), count, page);
   }
 
   @Cacheable(value = CacheNames.REPLAYS_SEARCH, sync = true)

diff --git a/src/main/java/com/faforever/client/vault/search/SearchController.java b/src/main/java/com/faforever/client/vault/search/SearchController.java
@@ -34,7 +34,6 @@
 import javafx.scene.layout.StackPane;
 import javafx.util.StringConverter;
 import lombok.Getter;
-import lombok.Value;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
@@ -73,7 +72,8 @@ public class SearchController implements Controller<Pane> {
   public HBox sortBox;
   public FlowPane filterPane;
   public CheckBox onlyShowLastYearCheckBox;
-  private final SimpleInvalidationListener queryInvalidationListener = () -> queryTextField.setText(buildQuery());;
+  private final SimpleInvalidationListener queryInvalidationListener = () -> queryTextField.setText(buildQuery());
+  ;
   /**
    * Called with the query string when the user hits "search".
    */
@@ -119,7 +119,7 @@ private void initSorting() {
     sortPropertyComboBox.setConverter(new StringConverter<>() {
       @Override
       public String toString(Property property) {
-        return i18n.get(property.getI18nKey());
+        return i18n.get(property.i18nKey());
       }
 
       @Override
@@ -148,14 +148,14 @@ public void setSearchableProperties(Map<String, Property> searchableProperties)
 
   public void setSortConfig(ObjectProperty<SortConfig> sortConfigObjectProperty) {
     List<Property> sortableProperties = searchableProperties.values().stream()
-        .filter(Property::isSortable)
+        .filter(Property::sortable)
         .toList();
     sortPropertyComboBox.getItems().addAll(sortableProperties);
-    sortOrderChoiceBox.getSelectionModel().select(sortConfigObjectProperty.get().getSortOrder());
+    sortOrderChoiceBox.getSelectionModel().select(sortConfigObjectProperty.get().sortOrder());
 
-    Property savedSortProperty = searchableProperties.get(sortConfigObjectProperty.get().getSortProperty());
+    Property savedSortProperty = searchableProperties.get(sortConfigObjectProperty.get().sortProperty());
 
-    if (savedSortProperty == null || !savedSortProperty.isSortable()) {
+    if (savedSortProperty == null || !savedSortProperty.sortable()) {
       savedSortProperty = sortableProperties.iterator().next();
     }
 
@@ -184,7 +184,8 @@ private void selectValueBeforeLosingFocusOnValueField(LogicalNodeController logi
       boolean isFreeTextField = !logicalNodeController.specificationController.valueField.valueProperty().isBound()
           && logicalNodeController.specificationController.valueField.getItems().isEmpty();
       if (isFreeTextField) {
-        JavaFxUtil.runLater(() -> logicalNodeController.specificationController.valueField.setValue(logicalNodeController.specificationController.valueField.getEditor().getText()));
+        JavaFxUtil.runLater(() -> logicalNodeController.specificationController.valueField.setValue(logicalNodeController.specificationController.valueField.getEditor()
+            .getText()));
       }
     });
   }
@@ -205,7 +206,7 @@ private String getCurrentEntityKey() {
     return searchableProperties.entrySet().stream()
         .filter(stringStringEntry -> stringStringEntry.getValue().equals(sortPropertyComboBox.getValue()))
         .findFirst()
-        .get()
+        .orElseThrow()
         .getKey();
   }
 
@@ -307,7 +308,8 @@ public CategoryFilterController addCategoryFilter(String propertyName, String ti
     return categoryFilterController;
   }
 
-  public RangeFilterController addRangeFilter(String propertyName, String title, double min, double max, double tickUnit, Function<Double, ? extends Number> valueTransform) {
+  public RangeFilterController addRangeFilter(String propertyName, String title, double min, double max,
+                                              double tickUnit, Function<Double, ? extends Number> valueTransform) {
     RangeFilterController rangeFilterController = uiService.loadFxml("theme/vault/search/rangeFilter.fxml");
     rangeFilterController.setTitle(title);
     rangeFilterController.setPropertyName(propertyName);
@@ -340,7 +342,8 @@ public ToggleFilterController addToggleFilter(String propertyName, String title,
     return toggleFilterController;
   }
 
-  public BinaryFilterController addBinaryFilter(String propertyName, String title, String firstValue, String secondValue, String firstLabel, String secondLabel) {
+  public BinaryFilterController addBinaryFilter(String propertyName, String title, String firstValue,
+                                                String secondValue, String firstLabel, String secondLabel) {
     BinaryFilterController binaryFilterController = uiService.loadFxml("theme/vault/search/binaryFilter.fxml");
     binaryFilterController.setTitle(title);
     binaryFilterController.setPropertyName(propertyName);
@@ -381,7 +384,8 @@ private Condition getCompositeCondition(List<? extends FilterNodeController> que
     }
   }
 
-  private Condition getCompositeCondition(SpecificationController initialSpecification, List<LogicalNodeController> queryNodes) {
+  private Condition getCompositeCondition(SpecificationController initialSpecification,
+                                          List<LogicalNodeController> queryNodes) {
     QBuilder qBuilder = new QBuilder<>();
     boolean isLastYearChecked = onlyShowLastYearCheckBox.isVisible() && onlyShowLastYearCheckBox.isSelected();
     Optional<Condition> condition = initialSpecification.appendTo(qBuilder);
@@ -399,7 +403,10 @@ private Condition getCompositeCondition(SpecificationController initialSpecifica
       condition = currentCondition;
     }
 
-    return isLastYearChecked ? condition.get().and().instant("endTime").after(OffsetDateTime.now().minusYears(1).toInstant(), false)
+    return isLastYearChecked ? condition.get()
+        .and()
+        .instant("endTime")
+        .after(OffsetDateTime.now().minusYears(1).toInstant(), false)
         : condition.get();
   }
 
@@ -449,23 +456,7 @@ public enum SortOrder {
     }
   }
 
-  @Value
-  public static class SortConfig {
-    String sortProperty;
-    SortOrder sortOrder;
+  public record SortConfig(String sortProperty, SortOrder sortOrder) {}
 
-    public String toQuery() {
-      return sortOrder.getQuery() + sortProperty;
-    }
-  }
-
-  @Value
-  public static class SearchConfig {
-    SortConfig sortConfig;
-    String searchQuery;
-
-    public boolean hasCustomQuery() {
-      return searchQuery != null && !searchQuery.isEmpty();
-    }
-  }
+  public record SearchConfig(SortConfig sortConfig, String searchQuery) {}
 }
diff --git a/src/test/java/com/faforever/client/vault/search/SearchControllerTest.java b/src/test/java/com/faforever/client/vault/search/SearchControllerTest.java
@@ -223,7 +223,7 @@ public void testSorting() throws Exception {
     instance.onSearchButtonClicked();
 
     SortConfig mapSortConfig = vaultPrefs.getOnlineReplaySortConfig();
-    assertEquals(mapSortConfig.getSortOrder(), SortOrder.ASC);
+    assertEquals(mapSortConfig.sortOrder(), SortOrder.ASC);
     verify(searchListener).accept(new SearchConfig(mapSortConfig, "query"));
   }