Refactor pagination to prepare for expanding the scope of seek-based pagination on search

Author: eth3lbert

src/controllers/helpers/pagination.rs

@@ -198,6 +198,31 @@ impl<T> Paginated<T> {
         Some(opts)
     }
 
+    pub(crate) fn next_seek_params<S, F>(&self, f: F) -> AppResult<Option<IndexMap<String, String>>>
+    where
+        F: Fn(&T) -> S,
+        S: Serialize,
+    {
+        if self.is_explicit_page() || self.records_and_total.len() < self.options.per_page as usize
+        {
+            return Ok(None);
+        }
+
+        let mut opts = IndexMap::new();
+        match self.options.page {
+            Page::Unspecified | Page::Seek(_) => {
+                let seek = f(&self.records_and_total.last().unwrap().record);
+                opts.insert("seek".into(), encode_seek(seek)?);
+            }
+            Page::Numeric(_) => unreachable!(),
+        };
+        Ok(Some(opts))
+    }
+
+    fn is_explicit_page(&self) -> bool {
+        matches!(&self.options.page, Page::Numeric(_))
+    }
+
     pub(crate) fn iter(&self) -> impl Iterator<Item = &T> {
         self.records_and_total.iter().map(|row| &row.record)
     }

src/controllers/krate/search.rs

@@ -4,7 +4,6 @@ use crate::auth::AuthCheck;
 use diesel::dsl::*;
 use diesel::sql_types::Array;
 use diesel_full_text_search::*;
-use indexmap::IndexMap;
 use once_cell::sync::OnceCell;
 
 use crate::controllers::cargo_prelude::*;
@@ -150,7 +149,6 @@ pub async fn search(app: AppState, req: Parts) -> AppResult<Json<Value>> {
         let (total, next_page, prev_page, data, conn) = if supports_seek && !explicit_page {
             // Equivalent of:
             // `WHERE name > (SELECT name FROM crates WHERE id = $1) LIMIT $2`
-            query = query.limit(pagination.per_page);
             if let Some(seek) = seek {
                 let crate_name: String = crates::table
                     .find(seek)
@@ -165,26 +163,22 @@ pub async fn search(app: AppState, req: Parts) -> AppResult<Json<Value>> {
             //
             // If this becomes a problem in the future the crates count could be denormalized, at least
             // for the filterless happy path.
-            let count_query = filter_params.make_query(&req, conn)?.count();
-            let total: i64 = info_span!("db.query", message = "SELECT COUNT(*) FROM crates")
-                .in_scope(|| count_query.get_result(conn))?;
-
-            let results: Vec<(Crate, bool, Option<i64>)> =
-                info_span!("db.query", message = "SELECT ... FROM crates")
+            let query = query.pages_pagination_with_count_query(
+                pagination,
+                filter_params.make_query(&req, conn)?.count(),
+            );
+            let data: Paginated<(Crate, bool, Option<i64>)> =
+                info_span!("db.query", message = "SELECT ..., COUNT(*) FROM crates")
                     .in_scope(|| query.load(conn))?;
 
-            let next_page = if let Some(last) = results.last() {
-                let mut params = IndexMap::new();
-                params.insert(
-                    "seek".into(),
-                    crate::controllers::helpers::pagination::encode_seek(last.0.id)?,
-                );
-                Some(req.query_with_params(params))
-            } else {
-                None
-            };
-
-            (total, next_page, None, results, conn)
+            (
+                data.total(),
+                data.next_seek_params(|last| last.0.id)?
+                    .map(|p| req.query_with_params(p)),
+                None,
+                data.into_iter().collect::<Vec<_>>(),
+                conn,
+            )
         } else {
             let query = query.pages_pagination_with_count_query(
                 pagination,

src/tests/routes/crates/list.rs

@@ -791,12 +791,13 @@ fn seek_based_pagination() {
         if let Some(new_url) = resp.meta.next_page {
             assert_that!(resp.crates, len(eq(1)));
             url = Some(new_url);
+            assert_eq!(resp.meta.total, 3);
         } else {
             assert_that!(resp.crates, empty());
+            assert_eq!(resp.meta.total, 0);
         }
 
         assert_eq!(resp.meta.prev_page, None);
-        assert_eq!(resp.meta.total, 3);
     }
 
     assert_eq!(calls, 4);