Refactor pagination to prepare for cursor-based pagination.

src/controllers.rs

@@ -24,7 +24,7 @@ mod prelude {
         fn json<T: Serialize>(&self, t: &T) -> Response;
         fn query(&self) -> IndexMap<String, String>;
         fn wants_json(&self) -> bool;
-        fn pagination(&self, default: usize, max: usize) -> CargoResult<(i64, i64)>;
+        fn query_with_params(&self, params: IndexMap<String, String>) -> String;
     }
 
     impl<'a> RequestUtils for dyn Request + 'a {
@@ -55,26 +55,13 @@ mod prelude {
                 .unwrap_or(false)
         }
 
-        fn pagination(&self, default: usize, max: usize) -> CargoResult<(i64, i64)> {
-            let query = self.query();
-            let page = query
-                .get("page")
-                .and_then(|s| s.parse::<usize>().ok())
-                .unwrap_or(1);
-            let limit = query
-                .get("per_page")
-                .and_then(|s| s.parse::<usize>().ok())
-                .unwrap_or(default);
-            if limit > max {
-                return Err(human(&format_args!(
-                    "cannot request more than {} items",
-                    max
-                )));
-            }
-            if page == 0 {
-                return Err(human("page indexing starts from 1, page 0 is invalid"));
-            }
-            Ok((((page - 1) * limit) as i64, limit as i64))
+        fn query_with_params(&self, new_params: IndexMap<String, String>) -> String {
+            let mut params = self.query().clone();
+            params.extend(new_params);
+            let query_string = url::form_urlencoded::Serializer::new(String::new())
+                .extend_pairs(params)
+                .finish();
+            format!("?{}", query_string)
         }
     }
 }

src/controllers/category.rs

@@ -1,3 +1,4 @@
+use super::helpers::pagination::*;
 use super::prelude::*;
 
 use crate::models::Category;
@@ -7,11 +8,16 @@ use crate::views::{EncodableCategory, EncodableCategoryWithSubcategories};
 /// Handles the `GET /categories` route.
 pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
     let conn = req.db_conn()?;
-    let (offset, limit) = req.pagination(10, 100)?;
     let query = req.query();
+    // FIXME: There are 69 categories, 47 top level. This isn't going to
+    // grow by an OoM. We need a limit for /summary, but we don't need
+    // to paginate this.
+    let options = PaginationOptions::new(&query)?;
+    let offset = options.offset().unwrap_or_default();
     let sort = query.get("sort").map_or("alpha", String::as_str);
 
-    let categories = Category::toplevel(&conn, sort, limit, offset)?;
+    let categories =
+        Category::toplevel(&conn, sort, i64::from(options.per_page), i64::from(offset))?;
     let categories = categories.into_iter().map(Category::encodable).collect();
 
     // Query for the total count of categories

src/controllers/helpers.rs

@@ -1,9 +1,9 @@
 use crate::util::{json_response, CargoResult};
 use conduit::Response;
 
-pub mod pagination;
+pub(crate) mod pagination;
 
-pub use self::pagination::Paginate;
+pub(crate) use self::pagination::Paginate;
 
 pub fn ok_true() -> CargoResult<Response> {
     #[derive(Serialize)]

src/controllers/helpers/pagination.rs

@@ -1,44 +1,184 @@
+use crate::models::helpers::with_count::*;
+use crate::util::errors::*;
 use diesel::pg::Pg;
 use diesel::prelude::*;
 use diesel::query_builder::*;
+use diesel::query_dsl::LoadQuery;
 use diesel::sql_types::BigInt;
+use indexmap::IndexMap;
+
+#[derive(Debug, Clone, Copy)]
+pub(crate) enum Page {
+    Numeric(u32),
+    Unspecified,
+}
+
+impl Page {
+    fn new(params: &IndexMap<String, String>) -> CargoResult<Self> {
+        if let Some(s) = params.get("page") {
+            let numeric_page = s.parse()?;
+            if numeric_page < 1 {
+                return Err(human(&format_args!(
+                    "page indexing starts from 1, page {} is invalid",
+                    numeric_page,
+                )));
+            }
+
+            Ok(Page::Numeric(numeric_page))
+        } else {
+            Ok(Page::Unspecified)
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct PaginationOptions {
+    page: Page,
+    pub(crate) per_page: u32,
+}
+
+impl PaginationOptions {
+    pub(crate) fn new(params: &IndexMap<String, String>) -> CargoResult<Self> {
+        const DEFAULT_PER_PAGE: u32 = 10;
+        const MAX_PER_PAGE: u32 = 100;
+
+        let per_page = params
+            .get("per_page")
+            .map(|s| s.parse())
+            .unwrap_or(Ok(DEFAULT_PER_PAGE))?;
+
+        if per_page > MAX_PER_PAGE {
+            return Err(human(&format_args!(
+                "cannot request more than {} items",
+                MAX_PER_PAGE,
+            )));
+        }
+
+        Ok(Self {
+            page: Page::new(params)?,
+            per_page,
+        })
+    }
+
+    pub(crate) fn offset(&self) -> Option<u32> {
+        if let Page::Numeric(p) = self.page {
+            Some((p - 1) * self.per_page)
+        } else {
+            None
+        }
+    }
+}
+
+pub(crate) trait Paginate: Sized {
+    fn paginate(self, params: &IndexMap<String, String>) -> CargoResult<PaginatedQuery<Self>> {
+        Ok(PaginatedQuery {
+            query: self,
+            options: PaginationOptions::new(params)?,
+        })
+    }
+}
+
+impl<T> Paginate for T {}
 
-#[derive(Debug, QueryId)]
 pub struct Paginated<T> {
-    query: T,
-    limit: i64,
-    offset: i64,
+    records_and_total: Vec<WithCount<T>>,
+    options: PaginationOptions,
 }
 
-pub trait Paginate: AsQuery + Sized {
-    fn paginate(self, limit: i64, offset: i64) -> Paginated<Self::Query> {
-        Paginated {
-            query: self.as_query(),
-            limit,
-            offset,
+impl<T> Paginated<T> {
+    pub(crate) fn total(&self) -> Option<i64> {
+        Some(
+            self.records_and_total
+                .get(0)
+                .map(|row| row.total)
+                .unwrap_or_default(),
+        )
+    }
+
+    pub(crate) fn next_page_params(&self) -> Option<IndexMap<String, String>> {
+        if self.records_and_total.len() < self.options.per_page as usize {
+            return None;
         }
+
+        let mut opts = IndexMap::new();
+        match self.options.page {
+            Page::Numeric(n) => opts.insert("page".into(), (n + 1).to_string()),
+            Page::Unspecified => opts.insert("page".into(), 2.to_string()),
+        };
+        Some(opts)
+    }
+
+    pub(crate) fn prev_page_params(&self) -> Option<IndexMap<String, String>> {
+        if let Page::Numeric(1) | Page::Unspecified = self.options.page {
+            return None;
+        }
+
+        let mut opts = IndexMap::new();
+        match self.options.page {
+            Page::Numeric(n) => opts.insert("page".into(), (n - 1).to_string()),
+            Page::Unspecified => unreachable!(),
+        };
+        Some(opts)
+    }
+
+    pub(crate) fn iter(&self) -> impl Iterator<Item = &T> {
+        self.records_and_total.iter().map(|row| &row.record)
+    }
+}
+
+impl<T: 'static> IntoIterator for Paginated<T> {
+    type IntoIter = Box<dyn Iterator<Item = Self::Item>>;
+    type Item = T;
+
+    fn into_iter(self) -> Self::IntoIter {
+        Box::new(self.records_and_total.into_iter().map(|row| row.record))
     }
 }
 
-impl<T: AsQuery> Paginate for T {}
+#[derive(Debug)]
+pub(crate) struct PaginatedQuery<T> {
+    query: T,
+    options: PaginationOptions,
+}
 
-impl<T: Query> Query for Paginated<T> {
+impl<T> PaginatedQuery<T> {
+    pub(crate) fn load<U>(self, conn: &PgConnection) -> QueryResult<Paginated<U>>
+    where
+        Self: LoadQuery<PgConnection, WithCount<U>>,
+    {
+        let options = self.options;
+        let records_and_total = self.internal_load(conn)?;
+        Ok(Paginated {
+            records_and_total,
+            options,
+        })
+    }
+}
+
+impl<T> QueryId for PaginatedQuery<T> {
+    const HAS_STATIC_QUERY_ID: bool = false;
+    type QueryId = ();
+}
+
+impl<T: Query> Query for PaginatedQuery<T> {
     type SqlType = (T::SqlType, BigInt);
 }
 
-impl<T, DB> RunQueryDsl<DB> for Paginated<T> {}
+impl<T, DB> RunQueryDsl<DB> for PaginatedQuery<T> {}
 
-impl<T> QueryFragment<Pg> for Paginated<T>
+impl<T> QueryFragment<Pg> for PaginatedQuery<T>
 where
     T: QueryFragment<Pg>,
 {
     fn walk_ast(&self, mut out: AstPass<'_, Pg>) -> QueryResult<()> {
         out.push_sql("SELECT *, COUNT(*) OVER () FROM (");
         self.query.walk_ast(out.reborrow())?;
         out.push_sql(") t LIMIT ");
-        out.push_bind_param::<BigInt, _>(&self.limit)?;
-        out.push_sql(" OFFSET ");
-        out.push_bind_param::<BigInt, _>(&self.offset)?;
+        out.push_bind_param::<BigInt, _>(&i64::from(self.options.per_page))?;
+        if let Some(offset) = self.options.offset() {
+            out.push_sql(" OFFSET ");
+            out.push_bind_param::<BigInt, _>(&i64::from(offset))?;
+        }
         Ok(())
     }
 }

src/controllers/keyword.rs

@@ -9,7 +9,6 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
     use crate::schema::keywords;
 
     let conn = req.db_conn()?;
-    let (offset, limit) = req.pagination(10, 100)?;
     let query = req.query();
     let sort = query.get("sort").map(|s| &s[..]).unwrap_or("alpha");
 
@@ -21,14 +20,9 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
         query = query.order(keywords::keyword.asc());
     }
 
-    let data = query
-        .paginate(limit, offset)
-        .load::<(Keyword, i64)>(&*conn)?;
-    let total = data.get(0).map(|&(_, t)| t).unwrap_or(0);
-    let kws = data
-        .into_iter()
-        .map(|(k, _)| k.encodable())
-        .collect::<Vec<_>>();
+    let data = query.paginate(&req.query())?.load::<Keyword>(&*conn)?;
+    let total = data.total();
+    let kws = data.into_iter().map(Keyword::encodable).collect::<Vec<_>>();
 
     #[derive(Serialize)]
     struct R {
@@ -37,7 +31,7 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
     }
     #[derive(Serialize)]
     struct Meta {
-        total: i64,
+        total: Option<i64>,
     }
 
     Ok(req.json(&R {

src/controllers/krate/metadata.rs

@@ -212,8 +212,7 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> CargoResult<Response> {
     let name = &req.params()["crate_id"];
     let conn = req.db_conn()?;
     let krate = Crate::by_name(name).first::<Crate>(&*conn)?;
-    let (offset, limit) = req.pagination(10, 100)?;
-    let (rev_deps, total) = krate.reverse_dependencies(&*conn, offset, limit)?;
+    let (rev_deps, total) = krate.reverse_dependencies(&*conn, &req.query())?;
     let rev_deps: Vec<_> = rev_deps
         .into_iter()
         .map(|dep| dep.encodable(&krate.name))