diff --git a/api/src/org/labkey/api/data/DataRegionSelection.java b/api/src/org/labkey/api/data/DataRegionSelection.java index afbfcdef495..8eb0ae08c3b 100644 --- a/api/src/org/labkey/api/data/DataRegionSelection.java +++ b/api/src/org/labkey/api/data/DataRegionSelection.java @@ -26,7 +26,6 @@ import org.labkey.api.query.QueryForm; import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryView; -import org.labkey.api.query.UserSchema; import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import org.labkey.api.view.BadRequestException; @@ -42,7 +41,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -64,31 +62,39 @@ public class DataRegionSelection // example usage: set an filtered set of selected values in session public static final String SNAPSHOT_SELECTED_VALUES = ".snapshotSelectValues"; - private static Set getSet(ViewContext context, @Nullable String key, boolean create) + private static @NotNull String getSessionAttributeKey(@NotNull String path, @NotNull String key, boolean useSnapshot) + { + return path + key + (useSnapshot ? SNAPSHOT_SELECTED_VALUES : SELECTED_VALUES); + } + + private static @NotNull Set getSet(ViewContext context, @Nullable String key, boolean create) { return getSet(context, key, create, false); } - private static Set getSet(ViewContext context, @Nullable String key, boolean create, boolean useSnapshot) + private static @NotNull Set getSet(ViewContext context, @Nullable String key, boolean create, boolean useSnapshot) { if (key == null) key = getSelectionKeyFromRequest(context); if (key != null) { - key = context.getContainer().getPath() + key + (useSnapshot ? SNAPSHOT_SELECTED_VALUES : SELECTED_VALUES); + key = getSessionAttributeKey(context.getContainer().getPath(), key, useSnapshot); HttpSession session = context.getRequest().getSession(false); if (session != null) { @SuppressWarnings("unchecked") Set result = (Set)session.getAttribute(key); - if (result == null && create) + if (result == null) { result = new LinkedHashSet<>(); - session.setAttribute(key, result); + + if (create) + session.setAttribute(key, result); } return result; } } + return new LinkedHashSet<>(); } @@ -110,7 +116,6 @@ public static String getSelectionKey(String schemaName, String queryName, String return buf.toString(); } - /** * Get selected items from the request parameters including both current page's selection and session state * @return an unmodifiable copy of the selected item ids @@ -184,18 +189,14 @@ public static String getSelectionKeyFromRequest(ViewContext context) List parameterSelected = values == null ? new ArrayList<>() : Arrays.asList(values); Set result = new LinkedHashSet<>(parameterSelected); - synchronized (lock) { Set sessionSelected = getSet(context, key, false); - if (sessionSelected != null) - { - result.addAll(sessionSelected); - - if (clearSession) - sessionSelected.removeAll(result); - } + result.addAll(sessionSelected); + if (clearSession) + sessionSelected.removeAll(result); } + return Collections.unmodifiableSet(result); } @@ -214,8 +215,7 @@ public static String getSelectionKeyFromRequest(ViewContext context) public static @NotNull Set getSnapshotSelectedIntegers(ViewContext context, @Nullable String key) { - Set selected = getSnapshotSelected(context, key); - return asInts(selected == null ? new HashSet<>() : selected); + return asInts(getSnapshotSelected(context, key)); } private static @NotNull Set asInts(Set ids) @@ -240,9 +240,10 @@ public static int setSelected(ViewContext context, String key, Collection selection, boolean checked, boolean useSnapshot) { synchronized (lock) @@ -257,33 +258,27 @@ public static int setSelected(ViewContext context, String key, Collection getSelected(QueryForm form) throws IOException + public static List getSelected(QueryForm form, boolean clearSelected) throws IOException { - UserSchema schema = form.getSchema(); - if (schema == null) - throw new NotFoundException(); + List items; + var view = getQueryView(form); + + synchronized (lock) + { + var selection = getSet(view.getViewContext(), form.getQuerySettings().getSelectionKey(), true); + items = getSelectedItems(view, selection); + + if (clearSelected && !selection.isEmpty()) + items.forEach(selection::remove); + } - QueryView view = schema.createView(form, null); - return getSelected(view, form.getQuerySettings().getSelectionKey()); + return Collections.unmodifiableList(items); } private static Pair getDataRegionContext(QueryView view) @@ -346,64 +348,22 @@ private static Pair getDataRegionContext(QueryView vi RenderContext rc = v.getRenderContext(); rc.setCache(false); - setDataRegionColumnsForSelection(rgn, rc, view, table ); + setDataRegionColumnsForSelection(rgn, rc, view, table); return Pair.of(rgn, rc); } - public static List getSelected(QueryView view, String key) throws IOException + private static @NotNull QueryView getQueryView(QueryForm form) throws NotFoundException { - Pair dataRegionContext = getDataRegionContext(view); - DataRegion rgn = dataRegionContext.first; - RenderContext rc = dataRegionContext.second; - ViewContext context = view.getViewContext(); - - try (Timing ignored = MiniProfiler.step("getSelected"); Results results = rgn.getResults(rc)) - { - return getSelectedItems(context, key, rc, rgn, results); - } - catch (SQLException e) - { - throw new RuntimeSQLException(e); - } + var schema = form.getSchema(); + if (schema == null) + throw new NotFoundException(); + return schema.createView(form, null); } public static List getValidatedIds(List selection, QueryForm form) throws IOException { - UserSchema schema = form.getSchema(); - if (schema == null) - throw new NotFoundException(); - - QueryView view = schema.createView(form, null); - - Pair dataRegionContext = getDataRegionContext(view); - DataRegion rgn = dataRegionContext.first; - RenderContext ctx = dataRegionContext.second; - - List validatedIds = new ArrayList<>(); - try (Timing ignored = MiniProfiler.step("getSelected"); Results rs = rgn.getResults(ctx)) - { - if (rs == null) - return validatedIds; - - ResultSetRowMapFactory factory = ResultSetRowMapFactory.create(rs); - while (rs.next()) - { - ctx.setRow(factory.getRowMap(rs)); - if (rgn.isRecordSelectorEnabled(ctx)) - { - String value = rgn.getRecordSelectorValue(ctx); - if (selection.contains(value)) - validatedIds.add(value); - } - } - return validatedIds; - } - catch (SQLException e) - { - throw new RuntimeSQLException(e); - } - + return getSelectedItems(getQueryView(form), selection); } /** @@ -415,12 +375,7 @@ public static List getValidatedIds(List selection, QueryForm for */ public static int setSelectionForAll(QueryForm form, boolean checked) throws IOException { - UserSchema schema = form.getSchema(); - if (schema == null) - throw new NotFoundException(); - - QueryView view = schema.createView(form, null); - return setSelectionForAll(view, form.getQuerySettings().getSelectionKey(), checked); + return setSelectionForAll(getQueryView(form), form.getQuerySettings().getSelectionKey(), checked); } private static List setDataRegionColumnsForSelection(DataRegion rgn, RenderContext rc, QueryView view, TableInfo table) @@ -452,34 +407,14 @@ private static List setDataRegionColumnsForSelection(DataRegion rgn, Ren public static int setSelectionForAll(QueryView view, String key, boolean checked) throws IOException { - // Turn off features of QueryView - view.setPrintView(true); - view.setShowConfiguredButtons(false); - view.setShowPagination(false); - view.setShowPaginationCount(false); - view.setShowDetailsColumn(false); - view.setShowUpdateColumn(false); - - ViewContext context = view.getViewContext(); - - TableInfo table = view.getTable(); - - DataView v = view.createDataView(); - DataRegion rgn = v.getDataRegion(); - - // Include all rows - view.getSettings().setShowRows(ShowRows.ALL); - view.getSettings().setOffset(Table.NO_OFFSET); - - RenderContext rc = v.getRenderContext(); - rc.setCache(false); - - List colNames = setDataRegionColumnsForSelection(rgn, rc, view, table ); + var regionCtx = getDataRegionContext(view); + var rgn = regionCtx.first; + var rc = regionCtx.second; try (Timing ignored = MiniProfiler.step("selectAll"); ResultSet rs = rgn.getResults(rc)) { - List selection = createSelectionList(rc, rgn, rs, colNames); - return setSelected(context, key, selection, checked); + var selection = createSelectionList(rc, rgn, rs, null); + return setSelected(view.getViewContext(), key, selection, checked); } catch (SQLException e) { @@ -489,43 +424,52 @@ public static int setSelectionForAll(QueryView view, String key, boolean checked /** * Returns all items in the given result set that are selected and selectable - * @param context the view context from which to retrieve the session variable - * @param key session variable key - * @param ctx the render context - * @param rgn the data region - * @param rs the result set to be filtered - * @return list of items from the result set that are in the selectee session, or an empty list if none. - * @throws SQLException + * @param view the view from which to retrieve the data region context and session variable + * @param selectedValues optionally (nullable) specify a collection of selected values that will be matched + * against when selecting items. If null, then all items will be returned. + * @return list of items from the result set that are in the selected session, or an empty list if none. + * @throws IOException */ - private static List getSelectedItems(ViewContext context, String key, RenderContext ctx, DataRegion rgn, ResultSet rs) throws SQLException + private static List getSelectedItems(QueryView view, Collection selectedValues) throws IOException { - List selected = new LinkedList<>(); - Set selectedValues = getSet(context, key, true); - ResultSetRowMapFactory factory = ResultSetRowMapFactory.create(rs); - while (rs.next()) + var dataRegionContext = getDataRegionContext(view); + var rgn = dataRegionContext.first; + var ctx = dataRegionContext.second; + + try (Timing ignored = MiniProfiler.step("getSelected"); Results rs = rgn.getResults(ctx)) { - ctx.setRow(factory.getRowMap(rs)); - if (rgn.isRecordSelectorEnabled(ctx)) // Don't select unselectables (#35513) - { - String value = rgn.getRecordSelectorValue(ctx); - if (selectedValues.contains(value)) - selected.add(value); - } + return createSelectionList(ctx, rgn, rs, selectedValues); + } + catch (SQLException e) + { + throw new RuntimeSQLException(e); } - - return selected; } - private static List createSelectionList(RenderContext ctx, DataRegion rgn, ResultSet rs, List colNames) throws SQLException + private static List createSelectionList( + RenderContext ctx, + DataRegion rgn, + ResultSet rs, + @Nullable Collection selectedValues + ) throws SQLException { List selected = new LinkedList<>(); - ResultSetRowMapFactory factory = ResultSetRowMapFactory.create(rs); - while (rs.next()) + if (rs != null) { - ctx.setRow(factory.getRowMap(rs)); - if (rgn.isRecordSelectorEnabled(ctx)) // Don't select unselectables (#35513) - selected.add(rgn.getRecordSelectorValue(ctx)); + ResultSetRowMapFactory factory = ResultSetRowMapFactory.create(rs); + while (rs.next()) + { + ctx.setRow(factory.getRowMap(rs)); + + // Issue 35513: Don't select un-selectables + if (rgn.isRecordSelectorEnabled(ctx)) + { + var value = rgn.getRecordSelectorValue(ctx); + if (selectedValues == null || selectedValues.contains(value)) + selected.add(value); + } + } } return selected; diff --git a/api/webapp/clientapi/dom/DataRegion.js b/api/webapp/clientapi/dom/DataRegion.js index 51209ed18c1..1220fa0269d 100644 --- a/api/webapp/clientapi/dom/DataRegion.js +++ b/api/webapp/clientapi/dom/DataRegion.js @@ -4276,14 +4276,14 @@ if (!LABKEY.DataRegions) { * @see LABKEY.DataRegion#clearSelected */ LABKEY.DataRegion.setSelected = function(config) { - // Formerly LABKEY.DataRegion.setSelected - var url = LABKEY.ActionURL.buildURL("query", "setSelected.api", config.containerPath, - { 'key': config.selectionKey, 'checked': config.checked }); - LABKEY.Ajax.request({ - url: url, - method: "POST", - params: { id: config.ids || config.id }, + url: LABKEY.ActionURL.buildURL('query', 'setSelected.api', config.containerPath), + method: 'POST', + jsonData: { + checked: config.checked, + id: config.ids || config.id, + key: config.selectionKey, + }, scope: config.scope, success: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnSuccess(config), config.scope), failure: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnFailure(config), config.scope, true) @@ -4314,12 +4314,10 @@ if (!LABKEY.DataRegions) { * @see LABKEY.DataRegion#getSelected */ LABKEY.DataRegion.clearSelected = function(config) { - var url = LABKEY.ActionURL.buildURL('query', 'clearSelected.api', config.containerPath, - { 'key': config.selectionKey }); - LABKEY.Ajax.request({ + url: LABKEY.ActionURL.buildURL('query', 'clearSelected.api', config.containerPath), method: 'POST', - url: url, + jsonData: { key: config.selectionKey }, success: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnSuccess(config), config.scope), failure: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnFailure(config), config.scope, true) }); @@ -4344,16 +4342,24 @@ if (!LABKEY.DataRegions) { * * @param {Object} [config.scope] An optional scoping object for the success and error callback functions (default to this). * @param {string} [config.containerPath] An alternate container path. If not specified, the current container path will be used. + * @param {boolean} [config.clearSelected] If true, clear the session-based selection for this Data Region after + * retrieving the current selection. Defaults to false. * * @see LABKEY.DataRegion#setSelected * @see LABKEY.DataRegion#clearSelected */ LABKEY.DataRegion.getSelected = function(config) { - var url = LABKEY.ActionURL.buildURL('query', 'getSelected.api', config.containerPath, - { 'key': config.selectionKey }); + var jsonData = { key: config.selectionKey }; + + // Issue 41705: Support clearing selection from getSelected() + if (config.clearSelected) { + jsonData.clearSelected = true; + } LABKEY.Ajax.request({ - url: url, + url: LABKEY.ActionURL.buildURL('query', 'getSelected.api', config.containerPath), + method: 'POST', + jsonData: jsonData, success: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnSuccess(config), config.scope), failure: LABKEY.Utils.getCallbackWrapper(LABKEY.Utils.getOnFailure(config), config.scope, true) }); diff --git a/query/resources/views/QWPDemo.html b/query/resources/views/QWPDemo.html index f347a7cf851..7ea6d76745c 100644 --- a/query/resources/views/QWPDemo.html +++ b/query/resources/views/QWPDemo.html @@ -42,6 +42,7 @@ +
diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index a86cc9f63c3..b89433e25f0 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -5517,8 +5517,19 @@ public ApiResponse execute(final SelectForm form, BindException errors) throws E @SuppressWarnings({"unused", "WeakerAccess"}) public static class SelectForm extends QueryForm { + protected boolean clearSelected; protected String key; + public boolean isClearSelected() + { + return clearSelected; + } + + public void setClearSelected(boolean clearSelected) + { + this.clearSelected = clearSelected; + } + public String getKey() { return key; @@ -5569,12 +5580,12 @@ public ApiResponse execute(final SelectForm form, BindException errors) throws E { if (form.getQueryName() == null) { - Set selected = DataRegionSelection.getSelected(getViewContext(), form.getKey(), false); + Set selected = DataRegionSelection.getSelected(getViewContext(), form.getKey(), form.isClearSelected()); return new ApiSimpleResponse("selected", selected); } else { - List selected = DataRegionSelection.getSelected(form); + List selected = DataRegionSelection.getSelected(form, form.isClearSelected()); return new ApiSimpleResponse("selected", selected); } } diff --git a/query/webapp/QWPDemo.js b/query/webapp/QWPDemo.js index 9aee748e422..fbf165918a4 100644 --- a/query/webapp/QWPDemo.js +++ b/query/webapp/QWPDemo.js @@ -29,7 +29,8 @@ testGetBaseFilters: testGetBaseFilters, testFilterOnSortColumn: testFilterOnSortColumn, testButtonBarConfig: testButtonBarConfig, - testRespectExcludingPrefixes: testRespectExcludingPrefixes + testRespectExcludingPrefixes: testRespectExcludingPrefixes, + testGetSelected: testGetSelected }; var PAGE_OFFSET = 4; @@ -891,6 +892,76 @@ } }); } + + function testGetSelected() { + var loadCount = 0; + new LABKEY.QueryWebPart({ + title: 'Get Selected (Regression #41705)', + schemaName: 'Samples', + queryName: 'sampleDataTest1', + maxRows: 2, // split into 3 pages of results + sort: 'id', + renderTo: RENDERTO, + failure: function() { + alert('Failed test: testGetSelected failed to load'); + }, + listeners: { + render: function(dr) { + loadCount++; + + if (loadCount === 1) { + dr.getSelected({ + success: function(data) { + if (data.selected.length !== 0) { + alert('Failed test: Expected initial selection to be empty. Contained ' + data.selected.length + ' values.'); + return; + } + + dr.selectPage(true); + + // Allow for selection to persist + setTimeout(function() { dr.refresh(); }, 100); + }, + failure: function(err) { + alert('Failed test: Failed to make initial request to region.getSelected()'); + } + }); + } else if (loadCount === 2) { + dr.getSelected({ + clearSelected: true, + success: function(data) { + if (data.selected.length !== 2) { + alert('Failed test: Expected selection to have length 2. Contained ' + data.selected.length + ' values.'); + return; + } + + dr.refresh(); + }, + failure: function(err) { + alert('Failed test: Failed to make second request to region.getSelected()'); + } + }); + } else if (loadCount === 3) { + dr.getSelected({ + success: function(data) { + if (data.selected.length !== 0) { + alert('Failed test: Expected final selection to be empty. Contained ' + data.selected.length + ' values.'); + return; + } + + LABKEY.Utils.signalWebDriverTest('testGetSelected'); + }, + failure: function(err) { + alert('Failed test: Failed to make final request to region.getSelected()'); + } + }); + } else { + alert('Failed test: Unexpected number of requests made.'); + } + } + } + }); + } }); })(jQuery); \ No newline at end of file