Skip to content

Commit

Permalink
Make properties sortable
Browse files Browse the repository at this point in the history
  • Loading branch information
Sheikah45 committed May 10, 2023
1 parent ed3b029 commit 9b43b11
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 90 deletions.
6 changes: 3 additions & 3 deletions src/main/java/com/faforever/client/map/MapService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/faforever/client/mod/ModService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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();

Expand All @@ -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) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Check warning on line 136 in src/main/java/com/faforever/client/query/SpecificationController.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/client/query/SpecificationController.java#L136

Added line #L136 was not covered by tests
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/faforever/client/replay/ReplayService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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".
*/
Expand Down Expand Up @@ -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());

Check warning on line 122 in src/main/java/com/faforever/client/vault/search/SearchController.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/client/vault/search/SearchController.java#L122

Added line #L122 was not covered by tests
}

@Override
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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()));

Check warning on line 188 in src/main/java/com/faforever/client/vault/search/SearchController.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/client/vault/search/SearchController.java#L187-L188

Added lines #L187 - L188 were not covered by tests
}
});
}
Expand All @@ -205,7 +206,7 @@ private String getCurrentEntityKey() {
return searchableProperties.entrySet().stream()
.filter(stringStringEntry -> stringStringEntry.getValue().equals(sortPropertyComboBox.getValue()))
.findFirst()
.get()
.orElseThrow()
.getKey();
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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)

Check warning on line 409 in src/main/java/com/faforever/client/vault/search/SearchController.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/faforever/client/vault/search/SearchController.java#L407-L409

Added lines #L407 - L409 were not covered by tests
: condition.get();
}

Expand Down Expand Up @@ -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) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down

0 comments on commit 9b43b11

Please sign in to comment.