Preserve pagination after actions which trigger an update of the grid (…

…#5411)

* Adapt the GridState interface to separate sorting and pagination.

This adds `getRowsBefore` methods, counterpart to the existing pagination
methods, so that we can do efficient and correct pagination, solving #33 and #570

* Revert "(I #2638) Feature to Goto a page directly (#2639)"

Go back to simply displaying a range of row numbers, because
that will enable a solution for #33, #570 and #572.
This reverts commit d7aaac2.

* Do not move back to the first page when applying an action which refreshes the grid.

Instead, the current page is refreshed.
This closes #33, closes #570, closes #572.

* Specify which actions preserve row and record ids

This lets us preserve pagination only when it is guaranteed to show the same
part of the data after the operation.

* Make paging user-editable like before

* Adapt cypress tests

extensions/database/module/scripts/index/database-import-controller.js

@@ -309,7 +309,7 @@ Refine.DatabaseImportController.prototype._getPreviewData = function(callback, n
             result.rowModel = data;
             callback(result);
           },
-          "jsonp"
+          "json"
         );
       },
       "json"

extensions/wikibase/module/scripts/dialogs/perform-edits-dialog.js

@@ -60,7 +60,7 @@ PerformEditsDialog.launch = function(logged_in_username, max_severity) {
         tag: WikibaseManager.getSelectedWikibaseTagTemplate(),
         maxEditsPerMinute: WikibaseManager.getSelectedWikibaseMaxEditsPerMinute()
       },
-      { includeEngine: true, cellsChanged: true, columnStatsChanged: true },
+      { includeEngine: true, cellsChanged: true, columnStatsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true },
       { onDone: function() { dismiss(); } }
     );
   };

extensions/wikibase/src/org/openrefine/wikibase/schema/WikibaseSchema.java

@@ -42,7 +42,6 @@
 import org.openrefine.model.IndexedRow;
 import org.openrefine.model.Project;
 import org.openrefine.overlay.OverlayModel;
-import org.openrefine.sorting.SortingConfig;
 import org.openrefine.util.ParsingUtilities;
 import org.openrefine.wikibase.qa.QAWarningStore;
 import org.openrefine.wikibase.schema.exceptions.QAWarningException;
@@ -199,7 +198,7 @@ public List<EntityEdit> evaluate(GridState grid, Engine engine, QAWarningStore w
             throw new IllegalStateException("The schema has not been validated before being evaluated");
         }
         List<EntityEdit> result = new ArrayList<>();
-        for (IndexedRow indexedRow : grid.iterateRows(engine.combinedRowFilters(), SortingConfig.NO_SORTING)) {
+        for (IndexedRow indexedRow : grid.iterateRows(engine.combinedRowFilters())) {
             ExpressionContext ctxt = new ExpressionContext(
                     siteIri,
                     entityTypeSiteIri,

main/src/main/java/org/openrefine/commands/expr/PreviewExpressionCommand.java

@@ -201,7 +201,12 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
                 Properties bindings = ExpressionUtils.createBindings();
 
                 if (Mode.RowBased.equals(engine.getMode())) {
-                    List<IndexedRow> rows = state.getRows(engine.combinedRowFilters(), sortingConfig, 0, limit);
+                    GridState sorted = state;
+                    if (!SortingConfig.NO_SORTING.equals(sortingConfig)) {
+                        // TODO cache appropriately
+                        sorted = state.reorderRows(sortingConfig, false);
+                    }
+                    List<IndexedRow> rows = sorted.getRowsAfter(engine.combinedRowFilters(), 0, limit);
                     for (IndexedRow indexedRow : rows) {
                         Cell cell = indexedRow.getRow().getCell(cellIndex);
                         Record record = null;
@@ -210,7 +215,12 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
                                         repeat, repeatCount));
                     }
                 } else {
-                    List<Record> records = state.getRecords(engine.combinedRecordFilters(), sortingConfig, 0, limit);
+                    GridState sorted = state;
+                    if (!SortingConfig.NO_SORTING.equals(sortingConfig)) {
+                        // TODO cache appropriately
+                        sorted = state.reorderRecords(sortingConfig, false);
+                    }
+                    List<Record> records = sorted.getRecordsAfter(engine.combinedRecordFilters(), 0, limit);
                     for (Record record : records) {
                         for (IndexedRow indexedRow : record.getIndexedRows()) {
                             Cell cell = indexedRow.getRow().getCell(cellIndex);

main/src/main/java/org/openrefine/commands/recon/GuessTypesOfColumnCommand.java

@@ -149,7 +149,7 @@ protected List<TypeGroup> guessTypes(GridState gridState, int cellIndex, String
         List<String> samples = new ArrayList<String>(sampleSize);
         Set<String> sampleSet = new HashSet<String>();
 
-        for (IndexedRow row : gridState.getRows(0, sampleSize)) {
+        for (IndexedRow row : gridState.getRowsAfter(0, sampleSize)) {
             Object value = row.getRow().getCellValue(cellIndex);
             if (ExpressionUtils.isNonBlankData(value)) {
                 String s = CharMatcher.whitespace().trimFrom(value.toString());

main/src/main/java/org/openrefine/commands/recon/PreviewExtendDataCommand.java

@@ -135,7 +135,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
 
             GridState state = project.getCurrentGridState();
             Engine engine = new Engine(state, engineConfig);
-            List<IndexedRow> previewRows = state.getRows(engine.combinedRowFilters(), sortingConfig, 0, limit);
+            GridState sorted = state;
+            if (!SortingConfig.NO_SORTING.equals(sortingConfig)) {
+                sorted = sorted.reorderRows(sortingConfig, false);
+            }
+            List<IndexedRow> previewRows = state.getRowsAfter(engine.combinedRowFilters(), 0, limit);
             for (IndexedRow indexedRow : previewRows) {
                 try {
                     Row row = indexedRow.getRow();

main/src/main/java/org/openrefine/commands/row/GetRowsCommand.java

@@ -34,7 +34,6 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 package org.openrefine.commands.row;
 
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
@@ -63,24 +62,44 @@ SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 import org.openrefine.model.Row;
 import org.openrefine.model.RowFilter;
 import org.openrefine.sorting.SortingConfig;
-import org.openrefine.util.ParsingUtilities;
 
+/**
+ * Retrieves rows from a project (or importing job).
+ *
+ * Those rows can be requested as either: - the batch of rows starting at a given index (included), up to a certain size
+ * - the batch of rows ending at a given index (excluded), again up to a given size. Filters (defined by facets) and the
+ * row/record mode toggle can also be provided.
+ */
 public class GetRowsCommand extends Command {
 
     protected static class WrappedRow {
 
         @JsonUnwrapped
         protected final Row row;
+
+        /**
+         * The logical row index (meaning the one from the original, unsorted grid)
+         */
         @JsonProperty("i")
         protected final long rowIndex;
         @JsonProperty("j")
         @JsonInclude(Include.NON_NULL)
         protected final Long recordIndex;
 
-        protected WrappedRow(Row rowOrRecord, long rowIndex, Long recordIndex) {
+        /**
+         * The row index used for pagination (which can be different from the logical one if a temporary sort is
+         * applied)
+         */
+        @JsonProperty("k")
+        protected final long paginationIndex;
+
+        // TODO add indices for pagination purposes
+
+        protected WrappedRow(Row rowOrRecord, long rowIndex, Long recordIndex, long paginationIndex) {
             this.row = rowOrRecord;
             this.rowIndex = rowIndex;
             this.recordIndex = recordIndex;
+            this.paginationIndex = paginationIndex;
         }
     }
 
@@ -112,21 +131,47 @@ protected static class JsonResult {
          */
         @JsonProperty("total")
         protected final long totalCount;
+        /**
+         * Total number of rows in the unfiltered grid (needed to provide a link to the last page)
+         */
+        @JsonProperty("totalRows")
+        protected final long totalRows;
 
         @JsonProperty("start")
-        protected final long start;
+        @JsonInclude(Include.NON_NULL)
+        protected final Long start;
+        @JsonProperty("end")
+        @JsonInclude(Include.NON_NULL)
+        protected final Long end;
         @JsonProperty("limit")
         protected final int limit;
 
+        /**
+         * The value to use as 'end' when fetching the page before this one. Can be null if there is no such page.
+         */
+        @JsonProperty("previousPageId")
+        @JsonInclude(Include.NON_NULL)
+        protected final Long previousPageId;
+        /**
+         * The value to use as 'start' when fetching the page after this one. Can be null if there is no such page.
+         */
+        @JsonProperty("nextPageId")
+        @JsonInclude(Include.NON_NULL)
+        protected final Long nextPageId;
+
         protected JsonResult(Mode mode, List<WrappedRow> rows, long filtered,
-                long processed, long totalCount, long start, int limit) {
+                long processed, long totalCount, long totalRows, long start, long end, int limit, Long previousPageId, Long nextPageId) {
             this.mode = mode;
             this.rows = rows;
             this.filtered = filtered;
             this.processed = processed;
             this.totalCount = totalCount;
-            this.start = start;
+            this.totalRows = totalRows;
+            this.start = start == -1 ? null : start;
+            this.end = end == -1 ? null : end;
             this.limit = Math.min(rows.size(), limit);
+            this.previousPageId = previousPageId;
+            this.nextPageId = nextPageId;
         }
     }
 
@@ -148,7 +193,11 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro
     protected static List<WrappedRow> recordToWrappedRows(Record record) {
         List<Row> rows = record.getRows();
         return IntStream.range(0, rows.size())
-                .mapToObj(i -> new WrappedRow(rows.get(i), record.getStartRowId() + i, i == 0 ? record.getStartRowId() : null))
+                .mapToObj(i -> new WrappedRow(
+                        rows.get(i),
+                        record.getLogicalStartRowId() + i,
+                        i == 0 ? record.getLogicalStartRowId() : null,
+                        record.getStartRowId() + i))
                 .collect(Collectors.toList());
     }
 
@@ -172,19 +221,18 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
             }
 
             Engine engine = getEngine(request, project);
-            String callback = request.getParameter("callback");
             GridState entireGrid = project.getCurrentGridState();
 
-            long start = getLongParameter(request, "start", 0);
+            long start = getLongParameter(request, "start", -1L);
+            long end = getLongParameter(request, "end", -1L);
             int limit = Math.max(0, getIntegerParameter(request, "limit", 20));
 
             response.setCharacterEncoding("UTF-8");
-            response.setHeader("Content-Type", callback == null ? "application/json" : "text/javascript");
+            response.setHeader("Content-Type", "application/json");
 
-            PrintWriter writer = response.getWriter();
-            if (callback != null) {
-                writer.write(callback);
-                writer.write("(");
+            if ((start == -1L) == (end == -1L)) {
+                respondException(response, new IllegalArgumentException("Exactly one of 'start' and 'end' should be provided."));
+                return;
             }
 
             SortingConfig sortingConfig = SortingConfig.NO_SORTING;
@@ -194,6 +242,8 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
                     sortingConfig = SortingConfig.reconstruct(sortingJson);
                 }
             } catch (IOException e) {
+                respondException(response, e);
+                return;
             }
 
             long filtered;
@@ -205,15 +255,26 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
             if (engine.getMode() == Mode.RowBased) {
                 totalCount = entireGrid.rowCount();
                 RowFilter combinedRowFilters = engine.combinedRowFilters();
-                List<IndexedRow> rows = entireGrid.getRows(combinedRowFilters, sortingConfig, start, limit);
+                GridState sortedGrid = entireGrid;
+                if (!SortingConfig.NO_SORTING.equals(sortingConfig)) {
+                    // TODO cache this appropriately
+                    sortedGrid = entireGrid.reorderRows(sortingConfig, false);
+                }
+                List<IndexedRow> rows;
+                if (start != -1L) {
+                    rows = sortedGrid.getRowsAfter(combinedRowFilters, start, limit);
+                } else {
+                    rows = sortedGrid.getRowsBefore(combinedRowFilters, end, limit);
+                }
 
                 wrappedRows = rows.stream()
-                        .map(tuple -> new WrappedRow(tuple.getRow(), tuple.getIndex(), null))
+                        .map(tuple -> new WrappedRow(tuple.getRow(), tuple.getLogicalIndex(), null, tuple.getIndex()))
                         .collect(Collectors.toList());
                 if (engineConfig.isNeutral()) {
                     filtered = totalCount;
                     processed = totalCount;
                 } else {
+                    // still using the unsorted grid for performance considerations
                     if (engineConfig.getAggregationLimit() == null) {
                         filtered = entireGrid.countMatchingRows(combinedRowFilters);
                         processed = totalCount;
@@ -227,7 +288,17 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
             } else {
                 totalCount = entireGrid.recordCount();
                 RecordFilter combinedRecordFilters = engine.combinedRecordFilters();
-                List<Record> records = entireGrid.getRecords(combinedRecordFilters, sortingConfig, start, limit);
+                GridState sortedGrid = entireGrid;
+                if (!SortingConfig.NO_SORTING.equals(sortingConfig)) {
+                    // TODO cache this appropriately
+                    sortedGrid = entireGrid.reorderRecords(sortingConfig, false);
+                }
+                List<Record> records;
+                if (start != -1L) {
+                    records = sortedGrid.getRecordsAfter(combinedRecordFilters, start, limit);
+                } else {
+                    records = sortedGrid.getRecordsBefore(combinedRecordFilters, end, limit);
+                }
 
                 wrappedRows = records.stream()
                         .flatMap(record -> recordToWrappedRows(record).stream())
@@ -237,6 +308,7 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
                     filtered = totalCount;
                     processed = totalCount;
                 } else {
+                    // still using the unsorted grid for performance considerations
                     if (engineConfig.getAggregationLimit() == null) {
                         filtered = entireGrid.countMatchingRecords(combinedRecordFilters);
                         processed = totalCount;
@@ -249,19 +321,33 @@ protected void internalRespond(HttpServletRequest request, HttpServletResponse r
                 }
             }
 
-            JsonResult result = new JsonResult(engine.getMode(),
-                    wrappedRows, filtered, processed,
-                    totalCount, start, limit);
-
-            ParsingUtilities.defaultWriter.writeValue(writer, result);
-            if (callback != null) {
-                writer.write(")");
+            // Compute the indices of the previous and next pages
+            Long previousPageId = null;
+            Long nextPageId = null;
+            if (start != -1L) {
+                if (start > 0) {
+                    previousPageId = start;
+                }
+                if (!wrappedRows.isEmpty()) {
+                    nextPageId = wrappedRows.get(wrappedRows.size() - 1).paginationIndex + 1;
+                }
+            } else {
+                if (!wrappedRows.isEmpty() && wrappedRows.get(0).paginationIndex > 0) {
+                    previousPageId = wrappedRows.get(0).paginationIndex;
+                }
+                nextPageId = end;
             }
 
             // metadata refresh for row mode and record mode
             if (project.getMetadata() != null) {
                 project.getMetadata().setRowCount(totalCount);
             }
+
+            JsonResult result = new JsonResult(engine.getMode(),
+                    wrappedRows, filtered, processed,
+                    totalCount, entireGrid.rowCount(), start, end, limit, previousPageId, nextPageId);
+
+            respondJSON(response, result);
         } catch (Exception e) {
             respondException(response, e);
         }