diff --git a/Cargo.lock b/Cargo.lock index 14fc59ed1a4..56640ce4c2c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1018,6 +1018,7 @@ dependencies = [ "once_cell", "p256", "parking_lot", + "paste", "prometheus", "rand", "regex", @@ -3051,6 +3052,12 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "paste" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" + [[package]] name = "pem-rfc7468" version = "0.7.0" diff --git a/Cargo.toml b/Cargo.toml index 7bfaf0c69aa..e8eda828094 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,6 +96,7 @@ object_store = { version = "=0.9.0", features = ["aws"] } once_cell = "=1.19.0" p256 = "=0.13.2" parking_lot = "=0.12.1" +paste = "=1.0.14" prometheus = { version = "=0.13.3", default-features = false } rand = "=0.8.5" reqwest = { version = "=0.11.24", features = ["gzip", "json"] } diff --git a/src/controllers/helpers/pagination.rs b/src/controllers/helpers/pagination.rs index 327b4bc3186..4c91289de45 100644 --- a/src/controllers/helpers/pagination.rs +++ b/src/controllers/helpers/pagination.rs @@ -198,6 +198,31 @@ impl Paginated { Some(opts) } + pub(crate) fn next_seek_params(&self, f: F) -> AppResult>> + 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 { self.records_and_total.iter().map(|row| &row.record) } @@ -372,6 +397,73 @@ impl PaginatedQueryWithCountSubq { } } +macro_rules! seek { + ( + $vis:vis enum $name:ident { + $( + $variant:ident($($(#[$field_meta:meta])? $ty:ty),*) + )* + } + ) => { + paste::item! { + $( + #[derive(Debug, Default, Deserialize, Serialize, PartialEq)] + $vis struct $variant($($(#[$field_meta])? pub(super) $ty),*); + )* + + #[derive(Debug, Deserialize, Serialize, PartialEq)] + #[serde(untagged)] + $vis enum [<$name Payload>] { + $( + $variant($variant), + )* + } + + #[derive(Debug, PartialEq)] + $vis enum $name { + $( + $variant, + )* + } + + $( + impl From<$variant> for [<$name Payload>] { + fn from(value: $variant) -> Self { + [<$name Payload>]::$variant(value) + } + } + )* + impl From<[<$name Payload>]> for $name { + fn from(value: [<$name Payload>]) -> Self { + match value { + $( + [<$name Payload>]::$variant(_) => $name::$variant, + )* + } + } + } + + use crate::util::errors::AppResult; + use crate::controllers::helpers::pagination::Page; + impl $name { + pub fn after(&self, page: &Page) -> AppResult]>> { + let Page::Seek(ref encoded) = *page else { + return Ok(None); + }; + + Ok(Some(match self { + $( + $name::$variant => encoded.decode::<$variant>()?.into(), + )* + })) + } + } + } + }; +} + +pub(crate) use seek; + #[cfg(test)] mod tests { use super::*; @@ -476,6 +568,85 @@ mod tests { ); } + mod seek { + use chrono::naive::serde::ts_microseconds; + seek! { + pub(super) enum Seek { + Id(i32) + New(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32) + RecentDownloads(Option, i32) + } + } + } + + #[test] + fn test_seek_macro_encode_and_decode() { + use chrono::{NaiveDate, NaiveDateTime}; + use seek::*; + + let assert_decode_after = |seek: Seek, query: &str, expect| { + let pagination = PaginationOptions::builder() + .enable_seek(true) + .gather(&mock(query)) + .unwrap(); + let decoded = seek.after(&pagination.page).unwrap(); + assert_eq!(decoded, expect); + }; + + let seek = Seek::Id; + let payload = SeekPayload::Id(Id(1234)); + let query = format!("seek={}", encode_seek(&payload).unwrap()); + assert_decode_after(seek, &query, Some(payload)); + + let dt: NaiveDateTime = NaiveDate::from_ymd_opt(2016, 7, 8) + .unwrap() + .and_hms_opt(9, 10, 11) + .unwrap(); + let seek = Seek::New; + let payload = SeekPayload::New(New(dt, 1234)); + let query = format!("seek={}", encode_seek(&payload).unwrap()); + assert_decode_after(seek, &query, Some(payload)); + + let seek = Seek::RecentDownloads; + let payload = SeekPayload::RecentDownloads(RecentDownloads(Some(5678), 1234)); + let query = format!("seek={}", encode_seek(&payload).unwrap()); + assert_decode_after(seek, &query, Some(payload)); + + let seek = Seek::Id; + assert_decode_after(seek, "", None); + + let seek = Seek::Id; + let payload = SeekPayload::RecentDownloads(RecentDownloads(Some(5678), 1234)); + let query = format!("seek={}", encode_seek(payload).unwrap()); + let pagination = PaginationOptions::builder() + .enable_seek(true) + .gather(&mock(&query)) + .unwrap(); + let error = seek.after(&pagination.page).unwrap_err(); + assert_eq!(error.to_string(), "invalid seek parameter"); + let response = error.response(); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + #[test] + fn test_seek_macro_conv() { + use chrono::{NaiveDate, NaiveDateTime}; + use seek::*; + + assert_eq!(Seek::from(SeekPayload::Id(Id(1234))), Seek::Id); + + let dt: NaiveDateTime = NaiveDate::from_ymd_opt(2016, 7, 8) + .unwrap() + .and_hms_opt(9, 10, 11) + .unwrap(); + assert_eq!(Seek::from(SeekPayload::New(New(dt, 1234))), Seek::New); + + assert_eq!( + Seek::from(SeekPayload::RecentDownloads(RecentDownloads(None, 1234))), + Seek::RecentDownloads + ); + } + fn mock(query: &str) -> Request<()> { Request::builder() .method(Method::GET) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index afcdba826a9..de2290e9ae2 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -2,9 +2,8 @@ use crate::auth::AuthCheck; use diesel::dsl::*; -use diesel::sql_types::{Array, Text}; +use diesel::sql_types::{Array, Bool, Text}; use diesel_full_text_search::*; -use indexmap::IndexMap; use once_cell::sync::OnceCell; use crate::controllers::cargo_prelude::*; @@ -41,7 +40,8 @@ use crate::sql::{array_agg, canon_crate_name, lower}; /// for them. pub async fn search(app: AppState, req: Parts) -> AppResult> { spawn_blocking(move || { - use diesel::sql_types::Bool; + use diesel::sql_types::Float; + use seek::*; let params = req.query(); let option_param = |s| params.get(s).map(|v| v.as_str()); @@ -73,28 +73,20 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { ALL_COLUMNS, false.into_sql::(), recent_crate_downloads::downloads.nullable(), + 0_f32.into_sql::(), ); let conn = &mut *app.db_read()?; - let mut supports_seek = filter_params.supports_seek(); + let mut seek: Option = None; let mut query = filter_params .make_query(&req, conn)? .left_join(recent_crate_downloads::table) .select(selection); if let Some(q_string) = &q_string { - // Searching with a query string always puts the exact match at the start of the results, - // so we can't support seek-based pagination with it. - supports_seek = false; - if !q_string.is_empty() { let sort = sort.unwrap_or("relevance"); - query = query.select(( - ALL_COLUMNS, - Crate::with_name(q_string), - recent_crate_downloads::downloads.nullable(), - )); query = query.order(Crate::with_name(q_string).desc()); if sort == "relevance" { @@ -102,60 +94,70 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { .bind::(q_string) .sql(")"); let rank = ts_rank_cd(crates::textsearchable_index_col, q); + query = query.select(( + ALL_COLUMNS, + Crate::with_name(q_string), + recent_crate_downloads::downloads.nullable(), + rank.clone(), + )); + seek = Some(Seek::Relevance); query = query.then_order_by(rank.desc()) + } else { + query = query.select(( + ALL_COLUMNS, + Crate::with_name(q_string), + recent_crate_downloads::downloads.nullable(), + 0_f32.into_sql::(), + )); + seek = Some(Seek::Query); } } } // Any sort other than 'relevance' (default) would ignore exact crate name matches + // Seek-based pagination requires a unique ordering to avoid unexpected row skipping + // during pagination. + // Therefore, when the ordering isn't unique an auxiliary ordering column should be added + // to ensure predictable pagination behavior. if sort == Some("downloads") { - // Custom sorting is not supported yet with seek. - supports_seek = false; - - query = query.order(crates::downloads.desc()) + seek = Some(Seek::Downloads); + query = query.order((crates::downloads.desc(), crates::id.desc())) } else if sort == Some("recent-downloads") { - // Custom sorting is not supported yet with seek. - supports_seek = false; - - query = query.order(recent_crate_downloads::downloads.desc().nulls_last()) + seek = Some(Seek::RecentDownloads); + query = query.order(( + recent_crate_downloads::downloads.desc().nulls_last(), + crates::id.desc(), + )) } else if sort == Some("recent-updates") { - // Custom sorting is not supported yet with seek. - supports_seek = false; - - query = query.order(crates::updated_at.desc()); + seek = Some(Seek::RecentUpdates); + query = query.order((crates::updated_at.desc(), crates::id.desc())); } else if sort == Some("new") { - // Custom sorting is not supported yet with seek. - supports_seek = false; - - query = query.order(crates::created_at.desc()); + seek = Some(Seek::New); + query = query.order((crates::created_at.desc(), crates::id.desc())); } else { + seek = seek.or(Some(Seek::Name)); + // Since the name is unique value, the inherent ordering becomes naturally unique. + // Therefore, an additional auxiliary ordering column is unnecessary in this case. query = query.then_order_by(crates::name.asc()) } let pagination: PaginationOptions = PaginationOptions::builder() .limit_page_numbers() - .enable_seek(supports_seek) + .enable_seek(true) .gather(&req)?; - let (explicit_page, seek) = match pagination.page { - Page::Numeric(_) => (true, None), - Page::Seek(ref s) => (false, Some(s.decode::()?)), - Page::Unspecified => (false, None), - }; + let explicit_page = matches!(pagination.page, Page::Numeric(_)); // To avoid breaking existing users, seek-based pagination is only used if an explicit page has // not been provided. This way clients relying on meta.next_page will use the faster seek-based // paginations, while client hardcoding pages handling will use the slower offset-based code. - 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) - .select(crates::name) - .get_result(conn)?; - query = query.filter(crates::name.gt(crate_name)); + let (total, next_page, prev_page, data, conn) = if !explicit_page && seek.is_some() { + let seek = seek.unwrap(); + if let Some(condition) = seek + .after(&pagination.page)? + .map(|s| filter_params.seek_after(&s)) + { + query = query.filter(condition); } // This does a full index-only scan over the crates table to gather how many crates were @@ -164,32 +166,28 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { // // 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)> = - 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, f32)> = + 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| seek.to_payload(last))? + .map(|p| req.query_with_params(p)), + None, + data.into_iter().collect::>(), + conn, + ) } else { let query = query.pages_pagination_with_count_query( pagination, filter_params.make_query(&req, conn)?.count(), ); - let data: Paginated<(Crate, bool, Option)> = + let data: Paginated<(Crate, bool, Option, f32)> = info_span!("db.query", message = "SELECT ..., COUNT(*) FROM crates") .in_scope(|| query.load(conn))?; ( @@ -201,12 +199,12 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { ) }; - let perfect_matches = data.iter().map(|&(_, b, _)| b).collect::>(); + let perfect_matches = data.iter().map(|&(_, b, _, _)| b).collect::>(); let recent_downloads = data .iter() - .map(|&(_, _, s)| s.unwrap_or(0)) + .map(|&(_, _, s, _)| s.unwrap_or(0)) .collect::>(); - let crates = data.into_iter().map(|(c, _, _)| c).collect::>(); + let crates = data.into_iter().map(|(c, _, _, _)| c).collect::>(); let versions: Vec = info_span!("db.query", message = "SELECT ... FROM versions") .in_scope(|| crates.versions().load(conn))?; @@ -285,20 +283,6 @@ impl<'a> FilterParams<'a> { }) } - fn supports_seek(&self) -> bool { - // Calculating the total number of results with filters is supported but paging is not supported yet. - !(self.q_string.is_some() - || self.category.is_some() - || self.all_keywords.is_some() - || self.keyword.is_some() - || self.letter.is_some() - || self.user_id.is_some() - || self.team_id.is_some() - || self.following - || self.has_ids - || !self.include_yanked) - } - fn make_query( &'a self, req: &Parts, @@ -410,6 +394,199 @@ impl<'a> FilterParams<'a> { Ok(query) } + + fn seek_after(&self, seek_payload: &seek::SeekPayload) -> BoxedCondition<'a> { + use seek::*; + + let crates_aliased = alias!(crates as crates_aliased); + let crate_name_by_id = |id: i32| { + crates_aliased + .find(id) + .select(crates_aliased.field(crates::name)) + .single_value() + }; + let conditions: Vec> = match *seek_payload { + SeekPayload::Name(Name(id)) => { + // Equivalent of: + // `WHERE name > name'` + vec![Box::new(crates::name.nullable().gt(crate_name_by_id(id)))] + } + SeekPayload::New(New(created_at, id)) => { + // Equivalent of: + // `WHERE (created_at = created_at' AND id < id') OR created_at < created_at'` + vec![ + Box::new( + crates::created_at + .eq(created_at) + .and(crates::id.lt(id)) + .nullable(), + ), + Box::new(crates::created_at.lt(created_at).nullable()), + ] + } + SeekPayload::RecentUpdates(RecentUpdates(updated_at, id)) => { + // Equivalent of: + // `WHERE (updated_at = updated_at' AND id < id') OR updated_at < updated_at'` + vec![ + Box::new( + crates::updated_at + .eq(updated_at) + .and(crates::id.lt(id)) + .nullable(), + ), + Box::new(crates::updated_at.lt(updated_at).nullable()), + ] + } + SeekPayload::RecentDownloads(RecentDownloads(recent_downloads, id)) => { + // Equivalent of: + // for recent_downloads is not None: + // `WHERE (recent_downloads = recent_downloads' AND id < id') + // OR (recent_downloads < recent_downloads' OR recent_downloads IS NULL)` + // for recent_downloads is None: + // `WHERE (recent_downloads IS NULL AND id < id')` + match recent_downloads { + Some(dl) => { + vec![ + Box::new( + recent_crate_downloads::downloads + .eq(dl) + .and(crates::id.lt(id)) + .nullable(), + ), + Box::new( + recent_crate_downloads::downloads + .lt(dl) + .or(recent_crate_downloads::downloads.is_null()) + .nullable(), + ), + ] + } + None => { + vec![Box::new( + recent_crate_downloads::downloads + .is_null() + .and(crates::id.lt(id)) + .nullable(), + )] + } + } + } + SeekPayload::Downloads(Downloads(downloads, id)) => { + // Equivalent of: + // `WHERE (downloads = downloads' AND id < id') OR downloads < downloads'` + vec![ + Box::new( + crates::downloads + .eq(downloads) + .and(crates::id.lt(id)) + .nullable(), + ), + Box::new(crates::downloads.lt(downloads).nullable()), + ] + } + SeekPayload::Query(Query(exact_match, id)) => { + // Equivalent of: + // `WHERE (exact_match = exact_match' AND name < name') OR exact_match < + // exact_match'` + let q_string = self.q_string.expect("q_string should not be None"); + let name_exact_match = Crate::with_name(q_string); + vec![ + Box::new( + name_exact_match + .eq(exact_match) + .and(crates::name.nullable().gt(crate_name_by_id(id))) + .nullable(), + ), + Box::new(name_exact_match.lt(exact_match).nullable()), + ] + } + SeekPayload::Relevance(Relevance(exact, rank_in, id)) => { + // Equivalent of: + // `WHERE (exact_match = exact_match' AND rank = rank' AND name > name') + // OR (exact_match = exact_match' AND rank < rank') + // OR exact_match < exact_match'` + let q_string = self.q_string.expect("q_string should not be None"); + let q = to_tsquery_with_search_config( + configuration::TsConfigurationByName("english"), + q_string, + ); + let rank = ts_rank_cd(crates::textsearchable_index_col, q); + let name_exact_match = Crate::with_name(q_string); + vec![ + Box::new( + name_exact_match + .eq(exact) + .and(rank.eq(rank_in)) + .and(crates::name.nullable().gt(crate_name_by_id(id))) + .nullable(), + ), + Box::new(name_exact_match.eq(exact).and(rank.lt(rank_in)).nullable()), + Box::new(name_exact_match.lt(exact).nullable()), + ] + } + }; + + conditions + .into_iter() + .fold( + None, + |merged_condition: Option>, condition| { + Some(match merged_condition { + Some(merged) => Box::new(merged.or(condition)), + None => condition, + }) + }, + ) + .expect("should be a reduced BoxedCondition") + } } +mod seek { + use crate::controllers::helpers::pagination::seek; + use crate::models::Crate; + use chrono::naive::serde::ts_microseconds; + + seek! { + pub enum Seek { + Name(i32) + New(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32) + RecentUpdates(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32) + RecentDownloads(Option, i32) + Downloads(i32, i32) + Query(bool, i32) + Relevance(bool, f32, i32) + } + } + + impl Seek { + pub(crate) fn to_payload(&self, record: &(Crate, bool, Option, f32)) -> SeekPayload { + match *self { + Seek::Name => SeekPayload::Name(Name(record.0.id)), + Seek::New => SeekPayload::New(New(record.0.created_at, record.0.id)), + Seek::RecentUpdates => { + SeekPayload::RecentUpdates(RecentUpdates(record.0.updated_at, record.0.id)) + } + Seek::RecentDownloads => { + SeekPayload::RecentDownloads(RecentDownloads(record.2, record.0.id)) + } + Seek::Downloads => { + SeekPayload::Downloads(Downloads(record.0.downloads, record.0.id)) + } + Seek::Query => SeekPayload::Query(Query(record.1, record.0.id)), + Seek::Relevance => { + SeekPayload::Relevance(Relevance(record.1, record.3, record.0.id)) + } + } + } + } +} + +type BoxedCondition<'a> = Box< + dyn BoxableExpression< + LeftJoinQuerySource, + diesel::pg::Pg, + SqlType = diesel::sql_types::Nullable, + > + 'a, +>; + diesel::infix_operator!(Contains, "@>"); diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index f370b50ed4e..b2a6b2a633e 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -7,13 +7,16 @@ use diesel::{dsl::*, prelude::*, update}; use googletest::prelude::*; use http::StatusCode; use insta::assert_json_snapshot; +use once_cell::sync::Lazy; +use regex::Regex; #[test] fn index() { let (app, anon) = TestApp::init().empty(); - let json = anon.search(""); - assert_eq!(json.crates.len(), 0); - assert_eq!(json.meta.total, 0); + for json in search_both(&anon, "") { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } let krate = app.db(|conn| { let u = new_user("foo") @@ -22,11 +25,12 @@ fn index() { CrateBuilder::new("fooindex", u.id).expect_build(conn) }); - let json = anon.search(""); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); - assert_eq!(json.crates[0].name, krate.name); - assert_eq!(json.crates[0].id, krate.name); + for json in search_both(&anon, "") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + assert_eq!(json.crates[0].name, krate.name); + assert_eq!(json.crates[0].id, krate.name); + } } #[test] @@ -57,64 +61,97 @@ fn index_queries() { (krate, krate2) }); - assert_eq!(anon.search("q=baz").meta.total, 0); + for json in search_both(&anon, "q=baz") { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } // All of these fields should be indexed/searched by the queries - assert_eq!(anon.search("q=foo").meta.total, 2); - assert_eq!(anon.search("q=kw1").meta.total, 3); - assert_eq!(anon.search("q=readme").meta.total, 1); - assert_eq!(anon.search("q=description").meta.total, 1); + for json in search_both(&anon, "q=foo") { + assert_eq!(json.crates.len(), 2); + assert_eq!(json.meta.total, 2); + } + + for json in search_both(&anon, "q=kw1") { + assert_eq!(json.crates.len(), 3); + assert_eq!(json.meta.total, 3); + } + + for json in search_both(&anon, "q=readme") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } + + for json in search_both(&anon, "q=description") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } // Query containing a space - assert_eq!(anon.search("q=foo%20kw3").meta.total, 1); + for json in search_both(&anon, "q=foo%20kw3") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } - let json = anon.search_by_user_id(user.id); - assert_eq!(json.crates.len(), 4); - assert_eq!(json.meta.total, 4); + for json in search_both_by_user_id(&anon, user.id) { + assert_eq!(json.crates.len(), 4); + assert_eq!(json.meta.total, 4); + } - let json = anon.search_by_user_id(0); - assert_eq!(json.crates.len(), 0); - assert_eq!(json.meta.total, 0); + for json in search_both_by_user_id(&anon, 0) { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } - let json = anon.search("letter=F"); - assert_eq!(json.crates.len(), 2); - assert_eq!(json.meta.total, 2); + for json in search_both(&anon, "letter=F") { + assert_eq!(json.crates.len(), 2); + assert_eq!(json.meta.total, 2); + } - let json = anon.search("letter=B"); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); + for json in search_both(&anon, "letter=B") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } - let json = anon.search("letter=b"); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); + for json in search_both(&anon, "letter=b") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } - let json = anon.search("letter=c"); - assert_eq!(json.crates.len(), 0); - assert_eq!(json.meta.total, 0); + for json in search_both(&anon, "letter=c") { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } - let json = anon.search("keyword=kw1"); - assert_eq!(json.crates.len(), 3); - assert_eq!(json.meta.total, 3); + for json in search_both(&anon, "keyword=kw1") { + assert_eq!(json.crates.len(), 3); + assert_eq!(json.meta.total, 3); + } - let json = anon.search("keyword=KW1"); - assert_eq!(json.crates.len(), 3); - assert_eq!(json.meta.total, 3); + for json in search_both(&anon, "keyword=KW1") { + assert_eq!(json.crates.len(), 3); + assert_eq!(json.meta.total, 3); + } - let json = anon.search("keyword=kw2"); - assert_eq!(json.crates.len(), 0); - assert_eq!(json.meta.total, 0); + for json in search_both(&anon, "keyword=kw2") { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } - let json = anon.search("all_keywords=kw1%20kw3"); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); + for json in search_both(&anon, "all_keywords=kw1%20kw3") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } - let json = anon.search("q=foo&keyword=kw1"); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); + for json in search_both(&anon, "q=foo&keyword=kw1") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } - let json = anon.search("q=foo2&keyword=kw1"); - assert_eq!(json.crates.len(), 0); - assert_eq!(json.meta.total, 0); + for json in search_both(&anon, "q=foo2&keyword=kw1") { + assert_eq!(json.crates.len(), 0); + assert_eq!(json.meta.total, 0); + } app.db(|conn| { new_category("Category 1", "cat1", "Category 1 crates") @@ -127,32 +164,40 @@ fn index_queries() { Category::update_crate(conn, &krate2, &["cat1::bar"]).unwrap(); }); - let cl = anon.search("category=cat1"); - assert_eq!(cl.crates.len(), 2); - assert_eq!(cl.meta.total, 2); + for cl in search_both(&anon, "category=cat1") { + assert_eq!(cl.crates.len(), 2); + assert_eq!(cl.meta.total, 2); + } - let cl = anon.search("category=cat1::bar"); - assert_eq!(cl.crates.len(), 1); - assert_eq!(cl.meta.total, 1); + for cl in search_both(&anon, "category=cat1::bar") { + assert_eq!(cl.crates.len(), 1); + assert_eq!(cl.meta.total, 1); + } - let cl = anon.search("keyword=cat2"); - assert_eq!(cl.crates.len(), 0); - assert_eq!(cl.meta.total, 0); + for cl in search_both(&anon, "keyword=cat2") { + assert_eq!(cl.crates.len(), 0); + assert_eq!(cl.meta.total, 0); + } - let cl = anon.search("q=readme&category=cat1"); - assert_eq!(cl.crates.len(), 1); - assert_eq!(cl.meta.total, 1); + for cl in search_both(&anon, "q=readme&category=cat1") { + assert_eq!(cl.crates.len(), 1); + assert_eq!(cl.meta.total, 1); + } - let cl = anon.search("keyword=kw1&category=cat1"); - assert_eq!(cl.crates.len(), 2); - assert_eq!(cl.meta.total, 2); + for cl in search_both(&anon, "keyword=kw1&category=cat1") { + assert_eq!(cl.crates.len(), 2); + assert_eq!(cl.meta.total, 2); + } - let cl = anon.search("keyword=kw3&category=cat1"); - assert_eq!(cl.crates.len(), 0); - assert_eq!(cl.meta.total, 0); + for cl in search_both(&anon, "keyword=kw3&category=cat1") { + assert_eq!(cl.crates.len(), 0); + assert_eq!(cl.meta.total, 0); + } // ignores 0x00 characters that Postgres does not support - assert_eq!(anon.search("q=k%00w1").meta.total, 3); + for cl in search_both(&anon, "q=k%00w1") { + assert_eq!(cl.meta.total, 3); + } } #[test] @@ -165,9 +210,10 @@ fn search_includes_crates_where_name_is_stopword() { .readme("crate which does things") .expect_build(conn); }); - let json = anon.search("q=which"); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.meta.total, 1); + for json in search_both(&anon, "q=which") { + assert_eq!(json.crates.len(), 1); + assert_eq!(json.meta.total, 1); + } } #[test] @@ -193,23 +239,26 @@ fn exact_match_first_on_queries() { .expect_build(conn); }); - let json = anon.search("q=foo-exact"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "foo_exact"); - assert_eq!(json.crates[1].name, "baz_exact"); - assert_eq!(json.crates[2].name, "bar-exact"); - - let json = anon.search("q=bar_exact"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "bar-exact"); - assert_eq!(json.crates[1].name, "baz_exact"); - assert_eq!(json.crates[2].name, "foo_exact"); - - let json = anon.search("q=baz_exact"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "baz_exact"); - assert_eq!(json.crates[1].name, "bar-exact"); - assert_eq!(json.crates[2].name, "foo_exact"); + for json in search_both(&anon, "q=foo-exact") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "foo_exact"); + assert_eq!(json.crates[1].name, "baz_exact"); + assert_eq!(json.crates[2].name, "bar-exact"); + } + + for json in search_both(&anon, "q=bar_exact") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "bar-exact"); + assert_eq!(json.crates[1].name, "baz_exact"); + assert_eq!(json.crates[2].name, "foo_exact"); + } + + for json in search_both(&anon, "q=baz_exact") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "baz_exact"); + assert_eq!(json.crates[1].name, "bar-exact"); + assert_eq!(json.crates[2].name, "foo_exact"); + } } #[test] @@ -218,6 +267,8 @@ fn index_sorting() { let (app, anon, user) = TestApp::init().with_user(); let user = user.as_model(); + // To test that the unique ordering of seed-based pagination is correct, we need to + // set some columns to the same value. app.db(|conn| { let krate1 = CrateBuilder::new("foo_sort", user.id) .description("bar_sort baz_sort const") @@ -234,12 +285,12 @@ fn index_sorting() { let krate3 = CrateBuilder::new("baz_sort", user.id) .description("foo_sort bar_sort foo_sort bar_sort foo_sort bar_sort const") .downloads(100_000) - .recent_downloads(10) + .recent_downloads(50) .expect_build(conn); let krate4 = CrateBuilder::new("other_sort", user.id) .description("other_sort const") - .downloads(999_999) + .downloads(100_000) .expect_build(conn); // Set the created at column for each crate @@ -251,11 +302,7 @@ fn index_sorting() { .set(crates::created_at.eq(now - 1.weeks())) .execute(conn) .unwrap(); - update(&krate3) - .set(crates::created_at.eq(now - 2.weeks())) - .execute(conn) - .unwrap(); - update(&krate4) + update(crates::table.filter(crates::id.eq_any(vec![krate3.id, krate4.id]))) .set(crates::created_at.eq(now - 3.weeks())) .execute(conn) .unwrap(); @@ -265,14 +312,10 @@ fn index_sorting() { .set(crates::updated_at.eq(now - 3.weeks())) .execute(conn) .unwrap(); - update(&krate2) + update(crates::table.filter(crates::id.eq_any(vec![krate2.id, krate3.id]))) .set(crates::updated_at.eq(now - 5.days())) .execute(conn) .unwrap(); - update(&krate3) - .set(crates::updated_at.eq(now - 10.seconds())) - .execute(conn) - .unwrap(); update(&krate4) .set(crates::updated_at.eq(now)) .execute(conn) @@ -280,45 +323,161 @@ fn index_sorting() { }); // Sort by downloads - let json = anon.search("sort=downloads"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "other_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "foo_sort"); + for json in search_both(&anon, "sort=downloads") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "other_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "foo_sort"); + } + let (resp, calls) = page_with_seek(&anon, "sort=downloads"); + assert_eq!(resp[0].crates[0].name, "other_sort"); + assert_eq!(resp[1].crates[0].name, "baz_sort"); + assert_eq!(resp[2].crates[0].name, "bar_sort"); + assert_eq!(resp[3].crates[0].name, "foo_sort"); + assert_eq!(resp[3].meta.total, 4); + assert_eq!(calls, 5); // Sort by recent-downloads - let json = anon.search("sort=recent-downloads"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "foo_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "other_sort"); + for json in search_both(&anon, "sort=recent-downloads") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "foo_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "other_sort"); + } + let (resp, calls) = page_with_seek(&anon, "sort=recent-downloads"); + assert_eq!(resp[0].crates[0].name, "baz_sort"); + assert_eq!(resp[1].crates[0].name, "foo_sort"); + assert_eq!(resp[2].crates[0].name, "bar_sort"); + assert_eq!(resp[3].crates[0].name, "other_sort"); + assert_eq!(resp[3].meta.total, 4); + assert_eq!(calls, 5); // Sort by recent-updates - let json = anon.search("sort=recent-updates"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "other_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "foo_sort"); + for json in search_both(&anon, "sort=recent-updates") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "other_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "foo_sort"); + } + let (resp, calls) = page_with_seek(&anon, "sort=recent-updates"); + assert_eq!(resp[0].crates[0].name, "other_sort"); + assert_eq!(resp[1].crates[0].name, "baz_sort"); + assert_eq!(resp[2].crates[0].name, "bar_sort"); + assert_eq!(resp[3].crates[0].name, "foo_sort"); + assert_eq!(resp[3].meta.total, 4); + assert_eq!(calls, 5); // Sort by new - let json = anon.search("sort=new"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "bar_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "other_sort"); - assert_eq!(json.crates[3].name, "foo_sort"); + for json in search_both(&anon, "sort=new") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "bar_sort"); + assert_eq!(json.crates[1].name, "other_sort"); + assert_eq!(json.crates[2].name, "baz_sort"); + assert_eq!(json.crates[3].name, "foo_sort"); + } + let (resp, calls) = page_with_seek(&anon, "sort=new"); + assert_eq!(resp[0].crates[0].name, "bar_sort"); + assert_eq!(resp[1].crates[0].name, "other_sort"); + assert_eq!(resp[2].crates[0].name, "baz_sort"); + assert_eq!(resp[3].crates[0].name, "foo_sort"); + assert_eq!(resp[3].meta.total, 4); + assert_eq!(calls, 5); + + use std::cmp::Reverse; + + fn decode_seek serde::Deserialize<'a>>(seek: &str) -> anyhow::Result { + use base64::{engine::general_purpose, Engine}; + let decoded = serde_json::from_slice(&general_purpose::URL_SAFE_NO_PAD.decode(seek)?)?; + Ok(decoded) + } + + // Sort by alpha with query + for query in ["sort=alpha&q=bar_sort", "sort=alpha&q=sort"] { + let (resp, calls) = page_with_seek(&anon, query); + assert_eq!(calls, resp[0].meta.total + 1); + let decoded_seeks = resp + .iter() + .filter_map(|cl| { + cl.meta + .next_page + .as_ref() + .map(|next_page| (next_page, cl.crates[0].name.to_owned())) + }) + .filter_map(|(q, name)| { + let query = url::form_urlencoded::parse(q.trim_start_matches('?').as_bytes()) + .into_owned() + .collect::>(); + query.get("seek").map(|s| { + let d = decode_seek::<(bool, i32)>(s).unwrap(); + (d.0, name) + }) + }) + .collect::>(); + // ordering (exact match desc, name asc) + let mut sorted = decoded_seeks.to_vec(); + sorted.sort_by_key(|k| (Reverse(k.0), k.1.to_owned())); + assert_eq!(sorted, decoded_seeks); + for json in search_both(&anon, query) { + assert_eq!(json.meta.total, resp[0].meta.total); + for (c, r) in json.crates.iter().zip(&resp) { + assert_eq!(c.name, r.crates[0].name); + } + } + } + + // Sort by relevance + for query in ["q=foo_sort", "q=sort"] { + let (resp, calls) = page_with_seek(&anon, query); + assert_eq!(calls, resp[0].meta.total + 1); + let decoded_seeks = resp + .iter() + .filter_map(|cl| { + cl.meta + .next_page + .as_ref() + .map(|next_page| (next_page, cl.crates[0].name.to_owned())) + }) + .filter_map(|(q, name)| { + let query = url::form_urlencoded::parse(q.trim_start_matches('?').as_bytes()) + .into_owned() + .collect::>(); + query.get("seek").map(|s| { + let d = decode_seek::<(bool, f32, i32)>(s).unwrap(); + (d.0, (d.1 * 1e12) as i64, name) + }) + }) + .collect::>(); + // ordering (exact match desc, rank desc, name asc) + let mut sorted = decoded_seeks.clone(); + sorted.sort_by_key(|k| (Reverse(k.0), Reverse(k.1), k.2.to_owned())); + assert_eq!(sorted, decoded_seeks); + for json in search_both(&anon, query) { + assert_eq!(json.meta.total, resp[0].meta.total); + for (c, r) in json.crates.iter().zip(&resp) { + assert_eq!(c.name, r.crates[0].name); + } + } + } // Test for bug with showing null results first when sorting // by descending downloads - let json = anon.search("sort=recent-downloads"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "foo_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "other_sort"); + for json in search_both(&anon, "sort=recent-downloads") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "foo_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "other_sort"); + } + let (resp, calls) = page_with_seek(&anon, "sort=recent-downloads"); + assert_eq!(resp[0].crates[0].name, "baz_sort"); + assert_eq!(resp[1].crates[0].name, "foo_sort"); + assert_eq!(resp[2].crates[0].name, "bar_sort"); + assert_eq!(resp[3].crates[0].name, "other_sort"); + assert_eq!(resp[3].meta.total, 4); + assert_eq!(calls, 5); } #[test] @@ -389,60 +548,68 @@ fn ignore_exact_match_on_queries_with_sort() { }); // Sort by downloads, order always the same no matter the crate name query - let json = anon.search("q=foo_sort&sort=downloads"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "baz_sort"); - assert_eq!(json.crates[1].name, "bar_sort"); - assert_eq!(json.crates[2].name, "foo_sort"); - - let json = anon.search("q=bar_sort&sort=downloads"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "baz_sort"); - assert_eq!(json.crates[1].name, "bar_sort"); - assert_eq!(json.crates[2].name, "foo_sort"); - - let json = anon.search("q=baz_sort&sort=downloads"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "baz_sort"); - assert_eq!(json.crates[1].name, "bar_sort"); - assert_eq!(json.crates[2].name, "foo_sort"); - - let json = anon.search("q=const&sort=downloads"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "other_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "foo_sort"); + for json in search_both(&anon, "q=foo_sort&sort=downloads") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "bar_sort"); + assert_eq!(json.crates[2].name, "foo_sort"); + } + + for json in search_both(&anon, "q=bar_sort&sort=downloads") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "bar_sort"); + assert_eq!(json.crates[2].name, "foo_sort"); + } + + for json in search_both(&anon, "q=baz_sort&sort=downloads") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "bar_sort"); + assert_eq!(json.crates[2].name, "foo_sort"); + } + + for json in search_both(&anon, "q=const&sort=downloads") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "other_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "foo_sort"); + } // Sort by recent-downloads, order always the same no matter the crate name query - let json = anon.search("q=bar_sort&sort=recent-downloads"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "foo_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); + for json in search_both(&anon, "q=bar_sort&sort=recent-downloads") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "foo_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + } // Test for bug with showing null results first when sorting // by descending downloads - let json = anon.search("sort=recent-downloads"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "foo_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "bar_sort"); - assert_eq!(json.crates[3].name, "other_sort"); + for json in search_both(&anon, "sort=recent-downloads") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "foo_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "other_sort"); + } // Sort by recent-updates - let json = anon.search("q=bar_sort&sort=recent-updates"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "baz_sort"); - assert_eq!(json.crates[1].name, "bar_sort"); - assert_eq!(json.crates[2].name, "foo_sort"); + for json in search_both(&anon, "q=bar_sort&sort=recent-updates") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "baz_sort"); + assert_eq!(json.crates[1].name, "bar_sort"); + assert_eq!(json.crates[2].name, "foo_sort"); + } // Sort by new - let json = anon.search("q=bar_sort&sort=new"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "bar_sort"); - assert_eq!(json.crates[1].name, "baz_sort"); - assert_eq!(json.crates[2].name, "foo_sort"); + for json in search_both(&anon, "q=bar_sort&sort=new") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "bar_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "foo_sort"); + } } #[test] @@ -457,12 +624,15 @@ fn multiple_ids() { CrateBuilder::new("other", user.id).expect_build(conn); }); - let json = - anon.search("ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "bar"); - assert_eq!(json.crates[1].name, "baz"); - assert_eq!(json.crates[2].name, "foo"); + for json in search_both( + &anon, + "ids%5B%5D=foo&ids%5B%5D=bar&ids%5B%5D=baz&ids%5B%5D=baz&ids%5B%5D=unknown", + ) { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "bar"); + assert_eq!(json.crates[1].name, "baz"); + assert_eq!(json.crates[2].name, "foo"); + } } #[test] @@ -497,16 +667,18 @@ fn loose_search_order() { .expect_build(conn); vec![one, two, three, four] }); - let search_temp = anon.search("q=temp"); - assert_eq!(search_temp.meta.total, 4); - assert_eq!(search_temp.crates.len(), 4); - for (lhs, rhs) in search_temp.crates.iter().zip(ordered) { - assert_eq!(lhs.name, rhs.name); + for search_temp in search_both(&anon, "q=temp") { + assert_eq!(search_temp.meta.total, 4); + assert_eq!(search_temp.crates.len(), 4); + for (lhs, rhs) in search_temp.crates.iter().zip(&ordered) { + assert_eq!(lhs.name, rhs.name); + } } - let search_temp = anon.search("q=te"); - assert_eq!(search_temp.meta.total, 3); - assert_eq!(search_temp.crates.len(), 3); + for search_temp in search_both(&anon, "q=te") { + assert_eq!(search_temp.meta.total, 3); + assert_eq!(search_temp.crates.len(), 3); + } } #[test] @@ -537,19 +709,21 @@ fn index_include_yanked() { }); // Include fully yanked (all versions were yanked) crates - let json = anon.search("include_yanked=yes&sort=alphabetical"); - assert_eq!(json.meta.total, 4); - assert_eq!(json.crates[0].name, "all_yanked"); - assert_eq!(json.crates[1].name, "newest_yanked"); - assert_eq!(json.crates[2].name, "oldest_yanked"); - assert_eq!(json.crates[3].name, "unyanked"); + for json in search_both(&anon, "include_yanked=yes&sort=alphabetical") { + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "all_yanked"); + assert_eq!(json.crates[1].name, "newest_yanked"); + assert_eq!(json.crates[2].name, "oldest_yanked"); + assert_eq!(json.crates[3].name, "unyanked"); + } // Do not include fully yanked (all versions were yanked) crates - let json = anon.search("include_yanked=no&sort=alphabetical"); - assert_eq!(json.meta.total, 3); - assert_eq!(json.crates[0].name, "newest_yanked"); - assert_eq!(json.crates[1].name, "oldest_yanked"); - assert_eq!(json.crates[2].name, "unyanked"); + for json in search_both(&anon, "include_yanked=no&sort=alphabetical") { + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "newest_yanked"); + assert_eq!(json.crates[1].name, "oldest_yanked"); + assert_eq!(json.crates[2].name, "unyanked"); + } } #[test] @@ -565,9 +739,10 @@ fn yanked_versions_are_not_considered_for_max_version() { .expect_build(conn); }); - let json = anon.search("q=foo"); - assert_eq!(json.meta.total, 1); - assert_eq!(json.crates[0].max_version, "1.0.0"); + for json in search_both(&anon, "q=foo") { + assert_eq!(json.meta.total, 1); + assert_eq!(json.crates[0].max_version, "1.0.0"); + } } #[test] @@ -586,9 +761,10 @@ fn max_stable_version() { .expect_build(conn); }); - let json = anon.search("q=foo"); - assert_eq!(json.meta.total, 1); - assert_eq!(json.crates[0].max_stable_version, Some("1.0.0".to_string())); + for json in search_both(&anon, "q=foo") { + assert_eq!(json.meta.total, 1); + assert_eq!(json.crates[0].max_stable_version, Some("1.0.0".to_string())); + } } /// Given two crates, one with downloads less than 90 days ago, the @@ -617,18 +793,18 @@ fn test_recent_download_count() { .expect_build(conn); }); - let json = anon.search("sort=recent-downloads"); + for json in search_both(&anon, "sort=recent-downloads") { + assert_eq!(json.meta.total, 2); - assert_eq!(json.meta.total, 2); + assert_eq!(json.crates[0].name, "sweet_potato_snack"); + assert_eq!(json.crates[1].name, "green_ball"); - assert_eq!(json.crates[0].name, "sweet_potato_snack"); - assert_eq!(json.crates[1].name, "green_ball"); + assert_eq!(json.crates[0].recent_downloads, Some(2)); + assert_eq!(json.crates[0].downloads, 5); - assert_eq!(json.crates[0].recent_downloads, Some(2)); - assert_eq!(json.crates[0].downloads, 5); - - assert_eq!(json.crates[1].recent_downloads, Some(0)); - assert_eq!(json.crates[1].downloads, 10); + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 10); + } } /// Given one crate with zero downloads, check that the crate @@ -648,11 +824,12 @@ fn test_zero_downloads() { .expect_build(conn); }); - let json = anon.search("sort=recent-downloads"); - assert_eq!(json.meta.total, 1); - assert_eq!(json.crates[0].name, "green_ball"); - assert_eq!(json.crates[0].recent_downloads, Some(0)); - assert_eq!(json.crates[0].downloads, 0); + for json in search_both(&anon, "sort=recent-downloads") { + assert_eq!(json.meta.total, 1); + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[0].recent_downloads, Some(0)); + assert_eq!(json.crates[0].downloads, 0); + } } /// Given two crates, one with more all-time downloads, the other with @@ -684,18 +861,18 @@ fn test_default_sort_recent() { // test that index for keywords is sorted by recent_downloads // by default - let json = anon.search("keyword=dog"); + for json in search_both(&anon, "keyword=dog") { + assert_eq!(json.meta.total, 2); - assert_eq!(json.meta.total, 2); + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[1].name, "sweet_potato_snack"); - assert_eq!(json.crates[0].name, "green_ball"); - assert_eq!(json.crates[1].name, "sweet_potato_snack"); + assert_eq!(json.crates[0].recent_downloads, Some(10)); + assert_eq!(json.crates[0].downloads, 10); - assert_eq!(json.crates[0].recent_downloads, Some(10)); - assert_eq!(json.crates[0].downloads, 10); - - assert_eq!(json.crates[1].recent_downloads, Some(0)); - assert_eq!(json.crates[1].downloads, 20); + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 20); + } app.db(|conn| { new_category("Animal", "animal", "animal crates") @@ -707,18 +884,18 @@ fn test_default_sort_recent() { // test that index for categories is sorted by recent_downloads // by default - let json = anon.search("category=animal"); + for json in search_both(&anon, "category=animal") { + assert_eq!(json.meta.total, 2); - assert_eq!(json.meta.total, 2); + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[1].name, "sweet_potato_snack"); - assert_eq!(json.crates[0].name, "green_ball"); - assert_eq!(json.crates[1].name, "sweet_potato_snack"); + assert_eq!(json.crates[0].recent_downloads, Some(10)); + assert_eq!(json.crates[0].downloads, 10); - assert_eq!(json.crates[0].recent_downloads, Some(10)); - assert_eq!(json.crates[0].downloads, 10); - - assert_eq!(json.crates[1].recent_downloads, Some(0)); - assert_eq!(json.crates[1].downloads, 20); + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 20); + } } #[test] @@ -732,17 +909,16 @@ fn pagination_links_included_if_applicable() { CrateBuilder::new("pagination_links_3", user.id).expect_build(conn); }); - // This uses a filter (`letter=p`) to disable seek-based pagination, as seek-based pagination - // does not return page numbers. If the test fails after expanding the scope of seek-based - // pagination replace the filter with something else still using pages. + // This uses a filter (`page=n`) to disable seek-based pagination, as seek-based pagination + // does not return page numbers. - let page1 = anon.search("letter=p&per_page=1"); + let page1 = anon.search("letter=p&page=1&per_page=1"); let page2 = anon.search("letter=p&page=2&per_page=1"); let page3 = anon.search("letter=p&page=3&per_page=1"); let page4 = anon.search("letter=p&page=4&per_page=1"); assert_eq!( - Some("?letter=p&per_page=1&page=2".to_string()), + Some("?letter=p&page=2&per_page=1".to_string()), page1.meta.next_page ); assert_eq!(None, page1.meta.prev_page); @@ -794,12 +970,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); @@ -880,9 +1057,10 @@ fn crates_by_user_id() { CrateBuilder::new("foo_my_packages", id).expect_build(conn); }); - let response = user.search_by_user_id(id); - assert_eq!(response.crates.len(), 1); - assert_eq!(response.meta.total, 1); + for response in search_both_by_user_id(&user, id) { + assert_eq!(response.crates.len(), 1); + assert_eq!(response.meta.total, 1); + } } #[test] @@ -895,7 +1073,61 @@ fn crates_by_user_id_not_including_deleted_owners() { krate.owner_remove(conn, "foo").unwrap(); }); - let response = anon.search_by_user_id(user.id); - assert_eq!(response.crates.len(), 0); - assert_eq!(response.meta.total, 0); + for response in search_both_by_user_id(&anon, user.id) { + assert_eq!(response.crates.len(), 0); + assert_eq!(response.meta.total, 0); + } +} + +static PAGE_RE: Lazy = Lazy::new(|| Regex::new(r"((?:^page|&page|\?page)=\d+)").unwrap()); + +// search with both offset-based (prepend with `page=1` query) and seek-based pagination +fn search_both(anon: &U, query: &str) -> [crate::CrateList; 2] { + if PAGE_RE.is_match(query) { + panic!("url already contains page param"); + } + let (offset, seek) = (anon.search(&format!("page=1&{query}")), anon.search(query)); + assert!(offset + .meta + .next_page + .as_deref() + .unwrap_or("page=2") + .contains("page=2")); + assert!(seek + .meta + .next_page + .as_deref() + .unwrap_or("seek=") + .contains("seek=")); + [offset, seek] +} + +fn search_both_by_user_id(anon: &U, id: i32) -> [crate::CrateList; 2] { + let url = format!("user_id={id}"); + search_both(anon, &url) +} + +fn page_with_seek(anon: &U, query: &str) -> (Vec, i32) { + let mut url = Some(format!("?per_page=1&{query}")); + let mut results = Vec::new(); + let mut calls = 0; + while let Some(current_url) = url.take() { + let resp = anon.search(current_url.trim_start_matches('?')); + calls += 1; + if calls > 200 { + panic!("potential infinite loop detected!") + } + + if let Some(ref new_url) = resp.meta.next_page { + assert!(new_url.contains("seek=")); + assert_that!(resp.crates, len(eq(1))); + url = Some(new_url.to_owned()); + assert_ne!(resp.meta.total, 0); + } else { + assert_that!(resp.crates, empty()); + assert_eq!(resp.meta.total, 0); + } + results.push(resp); + } + (results, calls) }