Skip to content

Commit 0ab8b84

Browse files
Turbo87Turbo87
authored
Merge pull request #8260 from eth3lbert/field-struct-seek
Convert seek to named fields
2 parents 74892ed + 03eb6fd commit 0ab8b84

File tree

3 files changed

+164
-56
lines changed

3 files changed

+164
-56
lines changed

‎src/controllers/helpers/pagination.rs

Lines changed: 76 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -402,19 +402,49 @@ impl<T, C> PaginatedQueryWithCountSubq<T, C> {
402402
}
403403

404404
macro_rules! seek {
405+
// Field struct
406+
(@variant_struct $vis:vis $variant:ident {
407+
$($(#[$field_meta:meta])? $field:ident: $ty:ty),* $(,)?
408+
}) => {
409+
paste::item! {
410+
#[derive(Debug, Default, Deserialize, PartialEq)]
411+
#[serde(from = $variant "Helper")]
412+
$vis struct $variant {
413+
$($(#[$field_meta])? pub(super) $field: $ty),*
414+
}
415+
416+
#[derive(Debug, Default, Deserialize, Serialize, PartialEq)]
417+
struct [<$variant Helper>]($($(#[$field_meta])? pub(super) $ty),*);
418+
419+
impl From<[<$variant Helper>]> for $variant {
420+
fn from(helper: [<$variant Helper>]) -> Self {
421+
let [<$variant Helper>]($($field,)*) = helper;
422+
Self { $($field,)* }
423+
}
424+
}
425+
426+
impl serde::Serialize for $variant {
427+
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
428+
where
429+
S: serde::Serializer,
430+
{
431+
let helper = [<$variant Helper>]($(self.$field,)*);
432+
serde::Serialize::serialize(&helper, serializer)
433+
}
434+
}
435+
}
436+
};
405437
(
406438
$vis:vis enum $name:ident {
407439
$(
408-
$variant:ident($($(#[$field_meta:meta])? $ty:ty),*)
440+
$variant:ident $fields:tt,
409441
)*
410442
}
411443
) => {
444+
$(
445+
seek!(@variant_struct $vis $variant $fields);
446+
)*
412447
paste::item! {
413-
$(
414-
#[derive(Debug, Default, Deserialize, Serialize, PartialEq)]
415-
$vis struct $variant($($(#[$field_meta])? pub(super) $ty),*);
416-
)*
417-
418448
#[derive(Debug, Deserialize, Serialize, PartialEq)]
419449
#[serde(untagged)]
420450
$vis enum [<$name Payload>] {
@@ -578,17 +608,27 @@ mod tests {
578608

579609
mod seek {
580610
use chrono::naive::serde::ts_microseconds;
581-
seek! {
611+
seek!(
582612
pub(super) enum Seek {
583-
Id(i32)
584-
New(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32)
585-
RecentDownloads(Option<i64>, i32)
613+
Id {
614+
id: i32,
615+
},
616+
New {
617+
#[serde(with = "ts_microseconds")]
618+
dt: chrono::NaiveDateTime,
619+
id: i32,
620+
},
621+
RecentDownloads {
622+
downloads: Option<i64>,
623+
id: i32,
624+
},
586625
}
587-
}
626+
);
588627
}
589628

590629
#[test]
591630
fn test_seek_macro_encode_and_decode() {
631+
use chrono::naive::serde::ts_microseconds;
592632
use chrono::{NaiveDate, NaiveDateTime};
593633
use seek::*;
594634

@@ -601,8 +641,9 @@ mod tests {
601641
assert_eq!(decoded, expect);
602642
};
603643

644+
let id = 1234;
604645
let seek = Seek::Id;
605-
let payload = SeekPayload::Id(Id(1234));
646+
let payload = SeekPayload::Id(Id { id });
606647
let query = format!("seek={}", encode_seek(&payload).unwrap());
607648
assert_decode_after(seek, &query, Some(payload));
608649

@@ -611,20 +652,21 @@ mod tests {
611652
.and_hms_opt(9, 10, 11)
612653
.unwrap();
613654
let seek = Seek::New;
614-
let payload = SeekPayload::New(New(dt, 1234));
655+
let payload = SeekPayload::New(New { dt, id });
615656
let query = format!("seek={}", encode_seek(&payload).unwrap());
616657
assert_decode_after(seek, &query, Some(payload));
617658

659+
let downloads = Some(5678);
618660
let seek = Seek::RecentDownloads;
619-
let payload = SeekPayload::RecentDownloads(RecentDownloads(Some(5678), 1234));
661+
let payload = SeekPayload::RecentDownloads(RecentDownloads { downloads, id });
620662
let query = format!("seek={}", encode_seek(&payload).unwrap());
621663
assert_decode_after(seek, &query, Some(payload));
622664

623665
let seek = Seek::Id;
624666
assert_decode_after(seek, "", None);
625667

626668
let seek = Seek::Id;
627-
let payload = SeekPayload::RecentDownloads(RecentDownloads(Some(5678), 1234));
669+
let payload = SeekPayload::RecentDownloads(RecentDownloads { downloads, id });
628670
let query = format!("seek={}", encode_seek(payload).unwrap());
629671
let pagination = PaginationOptions::builder()
630672
.enable_seek(true)
@@ -634,23 +676,38 @@ mod tests {
634676
assert_eq!(error.to_string(), "invalid seek parameter");
635677
let response = error.response();
636678
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
679+
680+
// Ensures it still encodes compactly with a field struct
681+
#[derive(Debug, Default, Serialize, PartialEq)]
682+
struct NewTuple(
683+
#[serde(with = "ts_microseconds")] chrono::NaiveDateTime,
684+
i32,
685+
);
686+
assert_eq!(
687+
encode_seek(NewTuple(dt, id)).unwrap(),
688+
encode_seek(SeekPayload::New(New { dt, id })).unwrap()
689+
);
637690
}
638691

639692
#[test]
640693
fn test_seek_macro_conv() {
641694
use chrono::{NaiveDate, NaiveDateTime};
642695
use seek::*;
643-
644-
assert_eq!(Seek::from(SeekPayload::Id(Id(1234))), Seek::Id);
696+
let id = 1234;
697+
assert_eq!(Seek::from(SeekPayload::Id(Id { id })), Seek::Id);
645698

646699
let dt: NaiveDateTime = NaiveDate::from_ymd_opt(2016, 7, 8)
647700
.unwrap()
648701
.and_hms_opt(9, 10, 11)
649702
.unwrap();
650-
assert_eq!(Seek::from(SeekPayload::New(New(dt, 1234))), Seek::New);
703+
assert_eq!(Seek::from(SeekPayload::New(New { dt, id })), Seek::New);
651704

705+
let downloads = None;
652706
assert_eq!(
653-
Seek::from(SeekPayload::RecentDownloads(RecentDownloads(None, 1234))),
707+
Seek::from(SeekPayload::RecentDownloads(RecentDownloads {
708+
downloads,
709+
id
710+
})),
654711
Seek::RecentDownloads
655712
);
656713
}

‎src/controllers/krate/search.rs

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -412,12 +412,12 @@ impl<'a> FilterParams<'a> {
412412
.single_value()
413413
};
414414
let conditions: Vec<BoxedCondition<'_>> = match *seek_payload {
415-
SeekPayload::Name(Name(id)) => {
415+
SeekPayload::Name(Name { id }) => {
416416
// Equivalent of:
417417
// `WHERE name > name'`
418418
vec![Box::new(crates::name.nullable().gt(crate_name_by_id(id)))]
419419
}
420-
SeekPayload::New(New(created_at, id)) => {
420+
SeekPayload::New(New { created_at, id }) => {
421421
// Equivalent of:
422422
// `WHERE (created_at = created_at' AND id < id') OR created_at < created_at'`
423423
vec![
@@ -430,7 +430,7 @@ impl<'a> FilterParams<'a> {
430430
Box::new(crates::created_at.lt(created_at).nullable()),
431431
]
432432
}
433-
SeekPayload::RecentUpdates(RecentUpdates(updated_at, id)) => {
433+
SeekPayload::RecentUpdates(RecentUpdates { updated_at, id }) => {
434434
// Equivalent of:
435435
// `WHERE (updated_at = updated_at' AND id < id') OR updated_at < updated_at'`
436436
vec![
@@ -443,7 +443,10 @@ impl<'a> FilterParams<'a> {
443443
Box::new(crates::updated_at.lt(updated_at).nullable()),
444444
]
445445
}
446-
SeekPayload::RecentDownloads(RecentDownloads(recent_downloads, id)) => {
446+
SeekPayload::RecentDownloads(RecentDownloads {
447+
recent_downloads,
448+
id,
449+
}) => {
447450
// Equivalent of:
448451
// for recent_downloads is not None:
449452
// `WHERE (recent_downloads = recent_downloads' AND id < id')
@@ -477,7 +480,7 @@ impl<'a> FilterParams<'a> {
477480
}
478481
}
479482
}
480-
SeekPayload::Downloads(Downloads(downloads, id)) => {
483+
SeekPayload::Downloads(Downloads { downloads, id }) => {
481484
// Equivalent of:
482485
// `WHERE (downloads = downloads' AND id < id') OR downloads < downloads'`
483486
vec![
@@ -490,7 +493,7 @@ impl<'a> FilterParams<'a> {
490493
Box::new(crate_downloads::downloads.lt(downloads).nullable()),
491494
]
492495
}
493-
SeekPayload::Query(Query(exact_match, id)) => {
496+
SeekPayload::Query(Query { exact_match, id }) => {
494497
// Equivalent of:
495498
// `WHERE (exact_match = exact_match' AND name < name') OR exact_match <
496499
// exact_match'`
@@ -506,7 +509,11 @@ impl<'a> FilterParams<'a> {
506509
Box::new(name_exact_match.lt(exact_match).nullable()),
507510
]
508511
}
509-
SeekPayload::Relevance(Relevance(exact, rank_in, id)) => {
512+
SeekPayload::Relevance(Relevance {
513+
exact_match: exact,
514+
rank: rank_in,
515+
id,
516+
}) => {
510517
// Equivalent of:
511518
// `WHERE (exact_match = exact_match' AND rank = rank' AND name > name')
512519
// OR (exact_match = exact_match' AND rank < rank')
@@ -551,37 +558,74 @@ mod seek {
551558
use crate::models::Crate;
552559
use chrono::naive::serde::ts_microseconds;
553560

554-
seek! {
561+
seek!(
555562
pub enum Seek {
556-
Name(i32)
557-
New(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32)
558-
RecentUpdates(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32)
559-
RecentDownloads(Option<i64>, i32)
560-
Downloads(i64, i32)
561-
Query(bool, i32)
562-
Relevance(bool, f32, i32)
563+
Name {
564+
id: i32,
565+
},
566+
New {
567+
#[serde(with = "ts_microseconds")]
568+
created_at: chrono::NaiveDateTime,
569+
id: i32,
570+
},
571+
RecentUpdates {
572+
#[serde(with = "ts_microseconds")]
573+
updated_at: chrono::NaiveDateTime,
574+
id: i32,
575+
},
576+
RecentDownloads {
577+
recent_downloads: Option<i64>,
578+
id: i32,
579+
},
580+
Downloads {
581+
downloads: i64,
582+
id: i32,
583+
},
584+
Query {
585+
exact_match: bool,
586+
id: i32,
587+
},
588+
Relevance {
589+
exact_match: bool,
590+
rank: f32,
591+
id: i32,
592+
},
563593
}
564-
}
594+
);
565595

566596
impl Seek {
567597
pub(crate) fn to_payload(
568598
&self,
569599
record: &(Crate, bool, i64, Option<i64>, f32),
570600
) -> SeekPayload {
601+
let (
602+
Crate {
603+
id,
604+
updated_at,
605+
created_at,
606+
..
607+
},
608+
exact_match,
609+
downloads,
610+
recent_downloads,
611+
rank,
612+
) = *record;
613+
571614
match *self {
572-
Seek::Name => SeekPayload::Name(Name(record.0.id)),
573-
Seek::New => SeekPayload::New(New(record.0.created_at, record.0.id)),
574-
Seek::RecentUpdates => {
575-
SeekPayload::RecentUpdates(RecentUpdates(record.0.updated_at, record.0.id))
576-
}
577-
Seek::RecentDownloads => {
578-
SeekPayload::RecentDownloads(RecentDownloads(record.3, record.0.id))
579-
}
580-
Seek::Downloads => SeekPayload::Downloads(Downloads(record.2, record.0.id)),
581-
Seek::Query => SeekPayload::Query(Query(record.1, record.0.id)),
582-
Seek::Relevance => {
583-
SeekPayload::Relevance(Relevance(record.1, record.4, record.0.id))
584-
}
615+
Seek::Name => SeekPayload::Name(Name { id }),
616+
Seek::New => SeekPayload::New(New { created_at, id }),
617+
Seek::RecentUpdates => SeekPayload::RecentUpdates(RecentUpdates { updated_at, id }),
618+
Seek::RecentDownloads => SeekPayload::RecentDownloads(RecentDownloads {
619+
recent_downloads,
620+
id,
621+
}),
622+
Seek::Downloads => SeekPayload::Downloads(Downloads { downloads, id }),
623+
Seek::Query => SeekPayload::Query(Query { exact_match, id }),
624+
Seek::Relevance => SeekPayload::Relevance(Relevance {
625+
exact_match,
626+
rank,
627+
id,
628+
}),
585629
}
586630
}
587631
}

‎src/controllers/krate/versions.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ fn list_by_date(
9292
!matches!(&options.page, Page::Numeric(_)),
9393
"?page= is not supported"
9494
);
95-
if let Some(SeekPayload::Date(Date(created_at, id))) = Seek::Date.after(&options.page)? {
95+
if let Some(SeekPayload::Date(Date { created_at, id })) = Seek::Date.after(&options.page)? {
9696
query = query.filter(
9797
versions::created_at
9898
.eq(created_at)
@@ -169,7 +169,7 @@ fn list_by_semver(
169169
"?page= is not supported"
170170
);
171171
let mut idx = Some(0);
172-
if let Some(SeekPayload::Semver(Semver(id))) = Seek::Semver.after(&options.page)? {
172+
if let Some(SeekPayload::Semver(Semver { id })) = Seek::Semver.after(&options.page)? {
173173
idx = sorted_versions
174174
.get_index_of(&id)
175175
.filter(|i| i + 1 < sorted_versions.len())
@@ -234,18 +234,25 @@ mod seek {
234234
// We might consider refactoring this to use named fields, which would be clearer and more
235235
// flexible. It's also worth noting that we currently encode seek compactly as a Vec, which
236236
// doesn't include field names.
237-
seek! {
237+
seek!(
238238
pub enum Seek {
239-
Semver(i32)
240-
Date(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32)
239+
Semver {
240+
id: i32,
241+
},
242+
Date {
243+
#[serde(with = "ts_microseconds")]
244+
created_at: chrono::NaiveDateTime,
245+
id: i32,
246+
},
241247
}
242-
}
248+
);
243249

244250
impl Seek {
245251
pub(crate) fn to_payload(&self, record: &(Version, Option<User>)) -> SeekPayload {
252+
let (Version { id, created_at, .. }, _) = *record;
246253
match *self {
247-
Seek::Semver => SeekPayload::Semver(Semver(record.0.id)),
248-
Seek::Date => SeekPayload::Date(Date(record.0.created_at, record.0.id)),
254+
Seek::Semver => SeekPayload::Semver(Semver { id }),
255+
Seek::Date => SeekPayload::Date(Date { created_at, id }),
249256
}
250257
}
251258
}

0 commit comments

Comments
 (0)