Vojtech Szocs has uploaded a new change for review. Change subject: webadmin: Fix server-side column sort clash with 'sortby' in search ......................................................................
webadmin: Fix server-side column sort clash with 'sortby' in search This patch fixes an issue when server-side column sort infra appends 'sortby' part in search string regardless of whether the search string is valid and/or already contains 'sortby'. For now, intended behavior is following: * server-side column sort works only if the currently applied search string is complete (without syntax errors) and does not already contain 'sortby' -- otherwise, column sort will be reverted to default (undefined) sort * currently applied search string is not updated according to server-side column sort (so the 'sortby' part gets appended "behind the scenes" for the Search query, current UI search string is not modified) In future, we can consider updating current UI search string (managed by CommonModel and applied into specific list model) when user performs server-side column sort. However, this can be tricky, so it's out of scope for now. Change-Id: I7ce39a4e2b6323e6e0276a3a462a3f9da005344b Signed-off-by: Vojtech Szocs <[email protected]> --- M backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/autocomplete/SearchSuggestModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java 6 files changed, 80 insertions(+), 38 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/57/28557/1 diff --git a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java index c9eb7eb..b830085 100644 --- a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java +++ b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/SyntaxChecker.java @@ -21,7 +21,13 @@ import org.ovirt.engine.core.compat.StringHelper; public class SyntaxChecker implements ISyntaxChecker { + public static final String TAG_COLUMN_NAME_IN_CRITERIA = "tag_name"; + + public static final String SORTBY = "SORTBY"; + public static final String SORTDIR_ASC = "ASC"; + public static final String SORTDIR_DESC = "DESC"; + private SearchObjectAutoCompleter mSearchObjectAC; private BaseAutoCompleter mColonAC; private BaseAutoCompleter mPluralAC; @@ -43,9 +49,9 @@ mSearchObjectAC = new SearchObjectAutoCompleter(); mColonAC = new BaseAutoCompleter(":"); mPluralAC = new BaseAutoCompleter("S"); - mSortbyAC = new BaseAutoCompleter("SORTBY"); + mSortbyAC = new BaseAutoCompleter(SORTBY); mPageAC = new BaseAutoCompleter("PAGE"); - mSortDirectionAC = new BaseAutoCompleter("ASC", "DESC"); + mSortDirectionAC = new BaseAutoCompleter(SORTDIR_ASC, SORTDIR_DESC); mAndAC = new BaseAutoCompleter("AND"); mOrAC = new BaseAutoCompleter("OR"); mDotAC = new BaseAutoCompleter("."); diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java index de83ba6..655710d 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java @@ -395,31 +395,34 @@ Object model = getDataProvider().getModel(); Column<?, ?> column = event.getColumn(); - if (model instanceof SearchableListModel) { + if (model instanceof SearchableListModel && column instanceof SortableColumn) { SearchableListModel<T> searchableModel = (SearchableListModel<T>) model; - SortableColumn<T, ?> sortedColumn = null; - - // Sorted column can be null (which indicates no sort) - if (column instanceof SortableColumn) { - sortedColumn = (SortableColumn<T, ?>) column; - } + SortableColumn<T, ?> sortedColumn = (SortableColumn<T, ?>) column; + boolean sortSuccessful = false; // Apply server-side sorting, if supported by the model if (searchableModel.supportsServerSideSorting()) { - String sortBy = (sortedColumn != null) ? sortedColumn.getSortBy() : null; - searchableModel.updateSortOptions(sortBy, event.isSortAscending()); + sortSuccessful = searchableModel.updateSortOptions(sortedColumn.getSortBy(), event.isSortAscending()); } // Otherwise, fall back to client-side sorting else { - Comparator<? super T> comparator = (sortedColumn != null) ? sortedColumn.getComparator() : null; - searchableModel.setComparator(comparator, event.isSortAscending()); + Comparator<? super T> comparator = sortedColumn.getComparator(); + if (comparator != null) { + searchableModel.setComparator(comparator, event.isSortAscending()); + sortSuccessful = true; + } } - // Synchronize column sort info + // Update column sort status, redrawing table headers if necessary ColumnSortInfo columnSortInfo = event.getColumnSortList().get(0); - tableHeader.getColumnSortList().push(columnSortInfo); - table.getColumnSortList().push(columnSortInfo); + if (sortSuccessful) { + tableHeader.getColumnSortList().push(columnSortInfo); + table.getColumnSortList().push(columnSortInfo); + } else { + tableHeader.getColumnSortList().remove(columnSortInfo); + table.getColumnSortList().remove(columnSortInfo); + } } } }); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java index 67ed1a1..1bca164 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java @@ -14,6 +14,7 @@ import org.ovirt.engine.core.common.businessentities.BusinessEntity; import org.ovirt.engine.core.common.businessentities.HasStoragePool; import org.ovirt.engine.core.common.businessentities.IVdcQueryable; +import org.ovirt.engine.core.common.queries.ConfigurationValues; import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; @@ -23,6 +24,11 @@ import org.ovirt.engine.core.compat.Regex; import org.ovirt.engine.core.compat.RegexOptions; import org.ovirt.engine.core.compat.StringHelper; +import org.ovirt.engine.core.searchbackend.ISyntaxChecker; +import org.ovirt.engine.core.searchbackend.SyntaxChecker; +import org.ovirt.engine.core.searchbackend.SyntaxCheckerFactory; +import org.ovirt.engine.core.searchbackend.SyntaxContainer; +import org.ovirt.engine.core.searchbackend.SyntaxError; import org.ovirt.engine.ui.frontend.AsyncQuery; import org.ovirt.engine.ui.frontend.Frontend; import org.ovirt.engine.ui.frontend.INewAsyncCallback; @@ -56,6 +62,11 @@ private static final Logger logger = Logger.getLogger(SearchableListModel.class.getName()); private static final String PAGE_STRING_REGEX = "[\\s]+page[\\s]+[1-9]+[0-9]*[\\s]*$"; //$NON-NLS-1$ private static final String PAGE_NUMBER_REGEX = "[1-9]+[0-9]*$"; //$NON-NLS-1$ + + // Syntax checker singleton instance. + // Note: must be static since SyntaxCheckerFactory.createUISyntaxChecker method + // works with single syntax checker instance (uiSyntaxChecker) for the entire UI. + protected static ISyntaxChecker syntaxChecker; private UICommand privateSearchCommand; private HandlerRegistration timerChangeHandler; @@ -292,6 +303,11 @@ // should have paging will set it explicitly in their constructors. getSearchNextPageCommand().setIsAvailable(false); getSearchPreviousPageCommand().setIsAvailable(false); + + if (syntaxChecker == null) { + syntaxChecker = SyntaxCheckerFactory.createUISyntaxChecker( + (String) AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.AuthenticationMethod)); + } } /** @@ -719,18 +735,25 @@ protected void syncSearch() { } - // Field to sort by within the search query, or null for undefined sort order private String sortBy; - - // Sort direction for server-side sorting, effective only when sortBy != null private boolean sortAscending; /** * Updates current server-side sort options, performing {@link #refresh} if necessary. + * <p> + * This method should be called only if this model supports server-side sorting. + * <p> + * Returns {@code true} if the sort operation can be applied on current search query. + * + * @param sortBy + * Field to sort by via the search query or {@code null} for undefined sort. + * @param sortAscending + * Sort direction, effective only when {@code sortBy} is not {@code null}. */ - public void updateSortOptions(String sortBy, boolean sortAscending) { - if (!supportsServerSideSorting()) { - return; + public boolean updateSortOptions(String sortBy, boolean sortAscending) { + String searchQuery = getSearchString(); + if (!isSearchComplete(searchQuery) || searchContainsSortByPart(searchQuery)) { + return false; } boolean shouldRefresh = !ObjectUtils.objectsEqual(this.sortBy, sortBy) @@ -742,20 +765,25 @@ if (shouldRefresh) { refresh(); } + + return true; } /** * Returns the given search query with current server-side sort options applied. + * + * @param searchQuery + * Search query to update with current server-side sort options. */ protected String applySortOptions(String searchQuery) { - StringBuilder result = new StringBuilder(searchQuery); + String result = searchQuery; - if (sortBy != null) { - result.append(" SORTBY ").append(sortBy) //$NON-NLS-1$ - .append(" ").append(sortAscending ? "ASC" : "DESC"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ + if (sortBy != null && isSearchComplete(searchQuery) && !searchContainsSortByPart(searchQuery)) { + result += " " + SyntaxChecker.SORTBY + " " + sortBy //$NON-NLS-1$ //$NON-NLS-2$ + + " " + (sortAscending ? SyntaxChecker.SORTDIR_ASC : SyntaxChecker.SORTDIR_DESC); //$NON-NLS-1$ } - return result.toString(); + return result; } /** @@ -765,6 +793,15 @@ return false; } + private boolean isSearchComplete(String searchQuery) { + SyntaxContainer syntaxResult = syntaxChecker.analyzeSyntaxState(searchQuery, true); + return syntaxResult.getError() == SyntaxError.NO_ERROR; + } + + private boolean searchContainsSortByPart(String searchQuery) { + return searchQuery.toLowerCase().contains(SyntaxChecker.SORTBY.toLowerCase()); + } + @Override public void setComparator(Comparator<? super T> comparator, boolean sortAscending) { super.setComparator(comparator, sortAscending); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/autocomplete/SearchSuggestModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/autocomplete/SearchSuggestModel.java index 015200c..52e424b 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/autocomplete/SearchSuggestModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/autocomplete/SearchSuggestModel.java @@ -4,22 +4,16 @@ import java.util.Arrays; import java.util.List; -import org.ovirt.engine.core.common.queries.ConfigurationValues; import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.compat.StringHelper; -import org.ovirt.engine.core.searchbackend.ISyntaxChecker; -import org.ovirt.engine.core.searchbackend.SyntaxCheckerFactory; import org.ovirt.engine.core.searchbackend.SyntaxContainer; import org.ovirt.engine.core.searchbackend.SyntaxError; import org.ovirt.engine.core.searchbackend.SyntaxObjectType; -import org.ovirt.engine.ui.uicommonweb.dataprovider.AsyncDataProvider; import org.ovirt.engine.ui.uicommonweb.models.SearchableListModel; import org.ovirt.engine.ui.uicompat.ObservableCollection; public class SearchSuggestModel extends SearchableListModel { - private final ISyntaxChecker syntaxChecker; - @Override public List getItems() { @@ -73,10 +67,6 @@ public SearchSuggestModel() { setItems(new ObservableCollection<Object>()); - syntaxChecker = - SyntaxCheckerFactory.createUISyntaxChecker((String) - AsyncDataProvider.getConfigValuePreConverted(ConfigurationValues.AuthenticationMethod)); - setIsTimerDisabled(true); } diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java index f167d33..a4daaf3 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterListModel.java @@ -249,10 +249,15 @@ @Override protected void syncSearch() { - SearchParameters tempVar = new SearchParameters(getSearchString(), SearchType.StoragePool, isCaseSensitiveSearch()); + SearchParameters tempVar = new SearchParameters( + applySortOptions(getSearchString()), SearchType.StoragePool, isCaseSensitiveSearch()); tempVar.setMaxCount(getSearchPageSize()); super.syncSearch(VdcQueryType.Search, tempVar); + } + @Override + public boolean supportsServerSideSorting() { + return true; } public void newEntity() diff --git a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java index 13bb2a3..4c95667 100644 --- a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java +++ b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java @@ -54,6 +54,7 @@ return object.getName(); } }; + nameColumn.makeSortable("name"); //$NON-NLS-1$ getTable().addColumn(nameColumn, constants.nameDc(), "150px"); //$NON-NLS-1$ CommentColumn<StoragePool> commentColumn = new CommentColumn<StoragePool>(); -- To view, visit http://gerrit.ovirt.org/28557 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7ce39a4e2b6323e6e0276a3a462a3f9da005344b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
