Skip to content

Commit 3f142f8

Browse files
committed
controllers/krate/search: Enable seeking backward for search
1 parent 847e8bc commit 3f142f8

File tree

2 files changed

+127
-44
lines changed

2 files changed

+127
-44
lines changed

src/controllers/krate/search.rs

+1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pub async fn list_crates(
119119
let pagination: PaginationOptions = PaginationOptions::builder()
120120
.limit_page_numbers()
121121
.enable_seek(true)
122+
.enable_seek_backward(true)
122123
.gather(&req)?;
123124
let is_forward = !pagination.is_backward();
124125

src/tests/routes/crates/list.rs

+126-44
Original file line numberDiff line numberDiff line change
@@ -352,13 +352,20 @@ async fn index_sorting() -> anyhow::Result<()> {
352352
assert_eq!(json.crates[2].name, "bar_sort");
353353
assert_eq!(json.crates[3].name, "foo_sort");
354354
}
355-
let (resp, calls) = page_with_seek(&anon, "sort=downloads").await;
355+
let (resp, calls) = page_with_seek_forward(&anon, "sort=downloads").await;
356356
assert_eq!(resp[0].crates[0].name, "other_sort");
357357
assert_eq!(resp[1].crates[0].name, "baz_sort");
358358
assert_eq!(resp[2].crates[0].name, "bar_sort");
359359
assert_eq!(resp[3].crates[0].name, "foo_sort");
360360
assert_eq!(resp[3].meta.total, 4);
361361
assert_eq!(calls, 5);
362+
let (resp, calls) = page_with_seek_backward(&anon, "sort=downloads").await;
363+
assert_eq!(resp[0].crates[0].name, "foo_sort");
364+
assert_eq!(resp[1].crates[0].name, "bar_sort");
365+
assert_eq!(resp[2].crates[0].name, "baz_sort");
366+
assert_eq!(resp[3].crates[0].name, "other_sort");
367+
assert_eq!(resp[3].meta.total, 4);
368+
assert_eq!(calls, 5);
362369

363370
// Sort by recent-downloads
364371
for json in search_both(&anon, "sort=recent-downloads").await {
@@ -368,13 +375,20 @@ async fn index_sorting() -> anyhow::Result<()> {
368375
assert_eq!(json.crates[2].name, "bar_sort");
369376
assert_eq!(json.crates[3].name, "other_sort");
370377
}
371-
let (resp, calls) = page_with_seek(&anon, "sort=recent-downloads").await;
378+
let (resp, calls) = page_with_seek_forward(&anon, "sort=recent-downloads").await;
372379
assert_eq!(resp[0].crates[0].name, "baz_sort");
373380
assert_eq!(resp[1].crates[0].name, "foo_sort");
374381
assert_eq!(resp[2].crates[0].name, "bar_sort");
375382
assert_eq!(resp[3].crates[0].name, "other_sort");
376383
assert_eq!(resp[3].meta.total, 4);
377384
assert_eq!(calls, 5);
385+
let (resp, calls) = page_with_seek_backward(&anon, "sort=recent-downloads").await;
386+
assert_eq!(resp[0].crates[0].name, "other_sort");
387+
assert_eq!(resp[1].crates[0].name, "bar_sort");
388+
assert_eq!(resp[2].crates[0].name, "foo_sort");
389+
assert_eq!(resp[3].crates[0].name, "baz_sort");
390+
assert_eq!(resp[3].meta.total, 4);
391+
assert_eq!(calls, 5);
378392

379393
// Sort by recent-updates
380394
for json in search_both(&anon, "sort=recent-updates").await {
@@ -384,13 +398,20 @@ async fn index_sorting() -> anyhow::Result<()> {
384398
assert_eq!(json.crates[2].name, "bar_sort");
385399
assert_eq!(json.crates[3].name, "foo_sort");
386400
}
387-
let (resp, calls) = page_with_seek(&anon, "sort=recent-updates").await;
401+
let (resp, calls) = page_with_seek_forward(&anon, "sort=recent-updates").await;
388402
assert_eq!(resp[0].crates[0].name, "other_sort");
389403
assert_eq!(resp[1].crates[0].name, "baz_sort");
390404
assert_eq!(resp[2].crates[0].name, "bar_sort");
391405
assert_eq!(resp[3].crates[0].name, "foo_sort");
392406
assert_eq!(resp[3].meta.total, 4);
393407
assert_eq!(calls, 5);
408+
let (resp, calls) = page_with_seek_backward(&anon, "sort=recent-updates").await;
409+
assert_eq!(resp[0].crates[0].name, "foo_sort");
410+
assert_eq!(resp[1].crates[0].name, "bar_sort");
411+
assert_eq!(resp[2].crates[0].name, "baz_sort");
412+
assert_eq!(resp[3].crates[0].name, "other_sort");
413+
assert_eq!(resp[3].meta.total, 4);
414+
assert_eq!(calls, 5);
394415

395416
// Sort by new
396417
for json in search_both(&anon, "sort=new").await {
@@ -400,28 +421,41 @@ async fn index_sorting() -> anyhow::Result<()> {
400421
assert_eq!(json.crates[2].name, "baz_sort");
401422
assert_eq!(json.crates[3].name, "foo_sort");
402423
}
403-
let (resp, calls) = page_with_seek(&anon, "sort=new").await;
424+
let (resp, calls) = page_with_seek_forward(&anon, "sort=new").await;
404425
assert_eq!(resp[0].crates[0].name, "bar_sort");
405426
assert_eq!(resp[1].crates[0].name, "other_sort");
406427
assert_eq!(resp[2].crates[0].name, "baz_sort");
407428
assert_eq!(resp[3].crates[0].name, "foo_sort");
408429
assert_eq!(resp[3].meta.total, 4);
409430
assert_eq!(calls, 5);
431+
let (resp, calls) = page_with_seek_backward(&anon, "sort=new").await;
432+
assert_eq!(resp[0].crates[0].name, "foo_sort");
433+
assert_eq!(resp[1].crates[0].name, "baz_sort");
434+
assert_eq!(resp[2].crates[0].name, "other_sort");
435+
assert_eq!(resp[3].crates[0].name, "bar_sort");
436+
assert_eq!(resp[3].meta.total, 4);
437+
assert_eq!(calls, 5);
410438

411439
// Sort by alpha with query
412440
// ordering (exact match desc, name asc)
413441
let query = "sort=alpha&q=bar_sort";
414-
let (resp, calls) = page_with_seek(&anon, query).await;
442+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
415443
for json in search_both(&anon, query).await {
416444
assert_eq!(json.meta.total, 3);
417445
assert_eq!(resp[0].crates[0].name, "bar_sort");
418446
assert_eq!(resp[1].crates[0].name, "baz_sort");
419447
assert_eq!(resp[2].crates[0].name, "foo_sort");
420448
}
421449
assert_eq!(calls, 4);
450+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
451+
assert_eq!(resp[0].crates[0].name, "foo_sort");
452+
assert_eq!(resp[1].crates[0].name, "baz_sort");
453+
assert_eq!(resp[2].crates[0].name, "bar_sort");
454+
assert_eq!(resp[2].meta.total, 3);
455+
assert_eq!(calls, 4);
422456

423457
let query = "sort=alpha&q=sort";
424-
let (resp, calls) = page_with_seek(&anon, query).await;
458+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
425459
for json in search_both(&anon, query).await {
426460
assert_eq!(json.meta.total, 4);
427461
assert_eq!(resp[0].crates[0].name, "bar_sort");
@@ -430,11 +464,18 @@ async fn index_sorting() -> anyhow::Result<()> {
430464
assert_eq!(resp[3].crates[0].name, "other_sort");
431465
}
432466
assert_eq!(calls, 5);
467+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
468+
assert_eq!(resp[0].crates[0].name, "other_sort");
469+
assert_eq!(resp[1].crates[0].name, "foo_sort");
470+
assert_eq!(resp[2].crates[0].name, "baz_sort");
471+
assert_eq!(resp[3].crates[0].name, "bar_sort");
472+
assert_eq!(resp[3].meta.total, 4);
473+
assert_eq!(calls, 5);
433474

434475
// Sort by relevance
435476
// ordering (exact match desc, rank desc, name asc)
436477
let query = "q=foo_sort";
437-
let (resp, calls) = page_with_seek(&anon, query).await;
478+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
438479
for json in search_both(&anon, query).await {
439480
assert_eq!(json.meta.total, 3);
440481
assert_eq!(resp[0].crates[0].name, "foo_sort");
@@ -443,13 +484,19 @@ async fn index_sorting() -> anyhow::Result<()> {
443484
assert_eq!(resp[2].crates[0].name, "baz_sort");
444485
}
445486
assert_eq!(calls, 4);
487+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
488+
assert_eq!(resp[0].crates[0].name, "baz_sort");
489+
assert_eq!(resp[1].crates[0].name, "bar_sort");
490+
assert_eq!(resp[2].crates[0].name, "foo_sort");
491+
assert_eq!(resp[2].meta.total, 3);
492+
assert_eq!(calls, 4);
446493
let ranks = querystring_rank(&mut conn, "foo_sort").await;
447494
assert_eq!(ranks.get("bar_sort"), ranks.get("baz_sort"));
448495

449496
// Add query containing a space to ensure tsquery works
450497
// "foo_sort" and "foo sort" would generate same tsquery
451498
let query = "q=foo%20sort";
452-
let (resp, calls) = page_with_seek(&anon, query).await;
499+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
453500
for json in search_both(&anon, query).await {
454501
assert_eq!(json.meta.total, 3);
455502
assert_eq!(resp[0].crates[0].name, "foo_sort");
@@ -458,11 +505,17 @@ async fn index_sorting() -> anyhow::Result<()> {
458505
assert_eq!(resp[2].crates[0].name, "baz_sort");
459506
}
460507
assert_eq!(calls, 4);
508+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
509+
assert_eq!(resp[0].crates[0].name, "baz_sort");
510+
assert_eq!(resp[1].crates[0].name, "bar_sort");
511+
assert_eq!(resp[2].crates[0].name, "foo_sort");
512+
assert_eq!(resp[2].meta.total, 3);
513+
assert_eq!(calls, 4);
461514
let ranks = querystring_rank(&mut conn, "foo%20sort").await;
462515
assert_eq!(ranks.get("bar_sort"), ranks.get("baz_sort"));
463516

464517
let query = "q=sort";
465-
let (resp, calls) = page_with_seek(&anon, query).await;
518+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
466519
for json in search_both(&anon, query).await {
467520
assert_eq!(json.meta.total, 4);
468521
// by rank desc (items with more "sort" should have a hider rank value)
@@ -472,6 +525,13 @@ async fn index_sorting() -> anyhow::Result<()> {
472525
assert_eq!(resp[3].crates[0].name, "other_sort");
473526
}
474527
assert_eq!(calls, 5);
528+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
529+
assert_eq!(resp[0].crates[0].name, "other_sort");
530+
assert_eq!(resp[1].crates[0].name, "foo_sort");
531+
assert_eq!(resp[2].crates[0].name, "bar_sort");
532+
assert_eq!(resp[3].crates[0].name, "baz_sort");
533+
assert_eq!(resp[3].meta.total, 4);
534+
assert_eq!(calls, 5);
475535
let ranks = querystring_rank(&mut conn, "sort").await;
476536
assert_eq!(
477537
ranks.keys().collect::<Vec<_>>(),
@@ -480,20 +540,28 @@ async fn index_sorting() -> anyhow::Result<()> {
480540

481541
// Test for bug with showing null results first when sorting
482542
// by descending downloads
483-
for json in search_both(&anon, "sort=recent-downloads").await {
543+
let query = "sort=recent-downloads";
544+
for json in search_both(&anon, query).await {
484545
assert_eq!(json.meta.total, 4);
485546
assert_eq!(json.crates[0].name, "baz_sort");
486547
assert_eq!(json.crates[1].name, "foo_sort");
487548
assert_eq!(json.crates[2].name, "bar_sort");
488549
assert_eq!(json.crates[3].name, "other_sort");
489550
}
490-
let (resp, calls) = page_with_seek(&anon, "sort=recent-downloads").await;
551+
let (resp, calls) = page_with_seek_forward(&anon, query).await;
491552
assert_eq!(resp[0].crates[0].name, "baz_sort");
492553
assert_eq!(resp[1].crates[0].name, "foo_sort");
493554
assert_eq!(resp[2].crates[0].name, "bar_sort");
494555
assert_eq!(resp[3].crates[0].name, "other_sort");
495556
assert_eq!(resp[3].meta.total, 4);
496557
assert_eq!(calls, 5);
558+
let (resp, calls) = page_with_seek_backward(&anon, query).await;
559+
assert_eq!(resp[0].crates[0].name, "other_sort");
560+
assert_eq!(resp[1].crates[0].name, "bar_sort");
561+
assert_eq!(resp[2].crates[0].name, "foo_sort");
562+
assert_eq!(resp[3].crates[0].name, "baz_sort");
563+
assert_eq!(resp[3].meta.total, 4);
564+
assert_eq!(calls, 5);
497565

498566
Ok(())
499567
}
@@ -1052,45 +1120,35 @@ async fn seek_based_pagination() -> anyhow::Result<()> {
10521120
.expect_build(&mut conn)
10531121
.await;
10541122

1055-
let mut url = Some("?per_page=1".to_string());
1056-
let mut results = Vec::new();
1057-
let mut calls = 0;
1058-
while let Some(current_url) = url.take() {
1059-
let resp = anon.search(current_url.trim_start_matches('?')).await;
1060-
calls += 1;
1061-
1062-
results.append(
1063-
&mut resp
1064-
.crates
1065-
.iter()
1066-
.map(|res| res.name.clone())
1067-
.collect::<Vec<_>>(),
1068-
);
1069-
1070-
if let Some(new_url) = resp.meta.next_page {
1071-
assert_that!(resp.crates, len(eq(1)));
1072-
url = Some(new_url);
1073-
assert_eq!(resp.meta.total, 3);
1074-
assert!(default_versions_iter(&resp.crates).all(Option::is_some));
1075-
} else {
1076-
assert_that!(resp.crates, empty());
1077-
assert_eq!(resp.meta.total, 0);
1078-
}
1079-
1080-
if calls == 1 {
1081-
assert_eq!(resp.meta.prev_page, None);
1082-
}
1123+
fn names(resp: &[crate::tests::CrateList]) -> std::vec::Vec<&str> {
1124+
resp.iter()
1125+
.flat_map(|r| r.crates.iter().map(|c| c.name.as_str()))
1126+
.collect::<Vec<_>>()
10831127
}
10841128

1129+
let (resp, calls) = page_with_seek_forward(&anon, "").await;
10851130
assert_eq!(calls, 4);
10861131
assert_eq!(
10871132
vec![
10881133
"pagination_links_1",
10891134
"pagination_links_2",
1090-
"pagination_links_3"
1135+
"pagination_links_3",
1136+
],
1137+
names(&resp)
1138+
);
1139+
assert_eq!(resp[0].meta.prev_page, None);
1140+
1141+
let (resp, calls) = page_with_seek_backward(&anon, "").await;
1142+
assert_eq!(calls, 4);
1143+
assert_eq!(
1144+
vec![
1145+
"pagination_links_3",
1146+
"pagination_links_2",
1147+
"pagination_links_1",
10911148
],
1092-
results
1149+
names(&resp)
10931150
);
1151+
assert_eq!(resp[0].meta.next_page, None);
10941152

10951153
Ok(())
10961154
}
@@ -1250,11 +1308,29 @@ async fn search_both_by_user_id<U: RequestHelper>(
12501308
search_both(anon, &url).await
12511309
}
12521310

1253-
async fn page_with_seek<U: RequestHelper>(
1311+
async fn page_with_seek_forward<U: RequestHelper>(
1312+
anon: &U,
1313+
query: &str,
1314+
) -> (Vec<crate::tests::CrateList>, i32) {
1315+
_page_with_seek(anon, query, true).await
1316+
}
1317+
1318+
async fn page_with_seek_backward<U: RequestHelper>(
12541319
anon: &U,
12551320
query: &str,
12561321
) -> (Vec<crate::tests::CrateList>, i32) {
1257-
let mut url = Some(format!("?per_page=1&{query}"));
1322+
_page_with_seek(anon, query, false).await
1323+
}
1324+
1325+
async fn _page_with_seek<U: RequestHelper>(
1326+
anon: &U,
1327+
query: &str,
1328+
forward: bool,
1329+
) -> (Vec<crate::tests::CrateList>, i32) {
1330+
let mut url = Some(format!(
1331+
"?per_page=1&{query}{}",
1332+
(!forward).then_some("&seek=-").unwrap_or_default()
1333+
));
12581334
let mut results = Vec::new();
12591335
let mut calls = 0;
12601336
while let Some(current_url) = url.take() {
@@ -1264,7 +1340,13 @@ async fn page_with_seek<U: RequestHelper>(
12641340
panic!("potential infinite loop detected!")
12651341
}
12661342

1267-
if let Some(ref new_url) = resp.meta.next_page {
1343+
let next_url = if forward {
1344+
resp.meta.next_page.as_ref()
1345+
} else {
1346+
resp.meta.prev_page.as_ref()
1347+
};
1348+
1349+
if let Some(new_url) = next_url {
12681350
assert!(new_url.contains("seek="));
12691351
assert_that!(resp.crates, len(eq(1)));
12701352
url = Some(new_url.to_owned());

0 commit comments

Comments
 (0)