Skip to content

Commit 4069eea

Browse files
committed
use @david-crespo's lovely timestamp pagination from #7842
1 parent 0ad888b commit 4069eea

File tree

8 files changed

+65
-30
lines changed

8 files changed

+65
-30
lines changed

nexus/db-model/src/webhook_delivery.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ impl WebhookDelivery {
7777
trigger: WebhookDeliveryTrigger,
7878
) -> Self {
7979
Self {
80-
// N.B.: perhaps we ought to use timestamp-based UUIDs for these?
8180
id: WebhookDeliveryUuid::new_v4().into(),
8281
event_id: event.id().into(),
8382
rx_id: (*rx_id).into(),
@@ -134,6 +133,7 @@ impl WebhookDelivery {
134133
.iter()
135134
.map(views::WebhookDeliveryAttempt::from)
136135
.collect(),
136+
time_started: self.time_created,
137137
};
138138
// Make sure attempts are in order; each attempt entry also includes an
139139
// attempt number, which should be used authoritatively to determine the

nexus/db-queries/src/db/datastore/webhook_delivery.rs

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::db::model::WebhookDeliveryState;
1717
use crate::db::model::WebhookDeliveryTrigger;
1818
use crate::db::model::WebhookEvent;
1919
use crate::db::model::WebhookEventClass;
20-
use crate::db::pagination::paginated;
20+
use crate::db::pagination::paginated_multicolumn;
2121
use crate::db::schema;
2222
use crate::db::schema::webhook_delivery::dsl;
2323
use crate::db::schema::webhook_delivery_attempt::dsl as attempt_dsl;
@@ -140,28 +140,32 @@ impl DataStore {
140140
rx_id: &WebhookReceiverUuid,
141141
triggers: &'static [WebhookDeliveryTrigger],
142142
only_states: Vec<WebhookDeliveryState>,
143-
pagparams: &DataPageParams<'_, Uuid>,
143+
pagparams: &DataPageParams<'_, (DateTime<Utc>, Uuid)>,
144144
) -> ListResultVec<(
145145
WebhookDelivery,
146146
WebhookEventClass,
147147
Vec<WebhookDeliveryAttempt>,
148148
)> {
149149
let conn = self.pool_connection_authorized(opctx).await?;
150150
// Paginate the query, ordered by delivery UUID.
151-
let mut query = paginated(dsl::webhook_delivery, dsl::id, pagparams)
152-
// Select only deliveries that are to the receiver we're interested in,
153-
// and were initiated by the triggers we're interested in.
154-
.filter(
155-
dsl::rx_id
156-
.eq(rx_id.into_untyped_uuid())
157-
.and(dsl::trigger.eq_any(triggers)),
158-
)
159-
// Join with the event table on the delivery's event ID,
160-
// so that we can grab the event class of the event that initiated
161-
// this delivery.
162-
.inner_join(
163-
event_dsl::webhook_event.on(dsl::event_id.eq(event_dsl::id)),
164-
);
151+
let mut query = paginated_multicolumn(
152+
dsl::webhook_delivery,
153+
(dsl::time_created, dsl::id),
154+
pagparams,
155+
)
156+
// Select only deliveries that are to the receiver we're interested in,
157+
// and were initiated by the triggers we're interested in.
158+
.filter(
159+
dsl::rx_id
160+
.eq(rx_id.into_untyped_uuid())
161+
.and(dsl::trigger.eq_any(triggers)),
162+
)
163+
// Join with the event table on the delivery's event ID,
164+
// so that we can grab the event class of the event that initiated
165+
// this delivery.
166+
.inner_join(
167+
event_dsl::webhook_event.on(dsl::event_id.eq(event_dsl::id)),
168+
);
165169
if !only_states.is_empty() {
166170
query = query.filter(dsl::state.eq_any(only_states));
167171
}
@@ -531,8 +535,9 @@ mod test {
531535
)
532536
.await
533537
.unwrap();
534-
paginator = p
535-
.found_batch(&deliveries, &|(d, _, _)| *d.id.as_untyped_uuid());
538+
paginator = p.found_batch(&deliveries, &|(d, _, _)| {
539+
(d.time_created, *d.id.as_untyped_uuid())
540+
});
536541
all_deliveries
537542
.extend(deliveries.into_iter().map(|(d, _, _)| dbg!(d).id));
538543
}

nexus/external-api/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ use nexus_types::{
1717
external_api::{params, shared, views},
1818
};
1919
use omicron_common::api::external::{
20-
http_pagination::{PaginatedById, PaginatedByName, PaginatedByNameOrId},
20+
http_pagination::{
21+
PaginatedById, PaginatedByName, PaginatedByNameOrId,
22+
PaginatedByTimeAndId,
23+
},
2124
*,
2225
};
2326
use openapi_manager_types::ValidationContext;
@@ -3659,7 +3662,7 @@ pub trait NexusExternalApi {
36593662
rqctx: RequestContext<Self::Context>,
36603663
receiver: Query<params::WebhookReceiverSelector>,
36613664
state_filter: Query<params::WebhookDeliveryStateFilter>,
3662-
pagination: Query<PaginatedById>,
3665+
pagination: Query<PaginatedByTimeAndId>,
36633666
) -> Result<HttpResponseOk<ResultsPage<views::WebhookDelivery>>, HttpError>;
36643667

36653668
/// Request re-delivery of webhook event

nexus/src/app/background/tasks/webhook_dispatcher.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,9 @@ mod test {
483483
)
484484
.await
485485
.unwrap();
486-
paginator =
487-
p.found_batch(&batch, &|(d, _, _)| d.id.into_untyped_uuid());
486+
paginator = p.found_batch(&batch, &|(d, _, _)| {
487+
(d.time_created, d.id.into_untyped_uuid())
488+
});
488489
deliveries.extend(batch);
489490
}
490491
let event =

nexus/src/app/webhook.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
use crate::Nexus;
134134
use crate::app::external_dns;
135135
use anyhow::Context;
136+
use chrono::DateTime;
136137
use chrono::TimeDelta;
137138
use chrono::Utc;
138139
use hmac::{Hmac, Mac};
@@ -634,7 +635,7 @@ impl Nexus {
634635
opctx: &OpContext,
635636
rx: lookup::WebhookReceiver<'_>,
636637
filter: params::WebhookDeliveryStateFilter,
637-
pagparams: &DataPageParams<'_, Uuid>,
638+
pagparams: &DataPageParams<'_, (DateTime<Utc>, Uuid)>,
638639
) -> ListResultVec<views::WebhookDelivery> {
639640
let (authz_rx,) = rx.lookup_for(authz::Action::ListChildren).await?;
640641
let only_states = if filter.include_all() {

nexus/src/external_api/http_entrypoints.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,11 @@ use omicron_common::api::external::http_pagination::PaginatedBy;
8989
use omicron_common::api::external::http_pagination::PaginatedById;
9090
use omicron_common::api::external::http_pagination::PaginatedByName;
9191
use omicron_common::api::external::http_pagination::PaginatedByNameOrId;
92+
use omicron_common::api::external::http_pagination::PaginatedByTimeAndId;
9293
use omicron_common::api::external::http_pagination::ScanById;
9394
use omicron_common::api::external::http_pagination::ScanByName;
9495
use omicron_common::api::external::http_pagination::ScanByNameOrId;
96+
use omicron_common::api::external::http_pagination::ScanByTimeAndId;
9597
use omicron_common::api::external::http_pagination::ScanParams;
9698
use omicron_common::api::external::http_pagination::data_page_params_for;
9799
use omicron_common::api::external::http_pagination::id_pagination;
@@ -7953,7 +7955,7 @@ impl NexusExternalApi for NexusExternalApiImpl {
79537955
rqctx: RequestContext<Self::Context>,
79547956
receiver: Query<params::WebhookReceiverSelector>,
79557957
filter: Query<params::WebhookDeliveryStateFilter>,
7956-
query: Query<PaginatedById>,
7958+
query: Query<PaginatedByTimeAndId>,
79577959
) -> Result<HttpResponseOk<ResultsPage<views::WebhookDelivery>>, HttpError>
79587960
{
79597961
let apictx = rqctx.context();
@@ -7966,16 +7968,16 @@ impl NexusExternalApi for NexusExternalApiImpl {
79667968
let webhook_selector = receiver.into_inner();
79677969
let filter = filter.into_inner();
79687970
let query = query.into_inner();
7969-
let pagparams = data_page_params_for(&rqctx, &query)?;
7971+
let pag_params = data_page_params_for(&rqctx, &query)?;
79707972
let rx = nexus.webhook_receiver_lookup(&opctx, webhook_selector)?;
79717973
let deliveries = nexus
7972-
.webhook_receiver_delivery_list(&opctx, rx, filter, &pagparams)
7974+
.webhook_receiver_delivery_list(&opctx, rx, filter, &pag_params)
79737975
.await?;
79747976

7975-
Ok(HttpResponseOk(ScanById::results_page(
7977+
Ok(HttpResponseOk(ScanByTimeAndId::results_page(
79767978
&query,
79777979
deliveries,
7978-
&|_, d| d.id,
7980+
&|_, d| (d.time_started, d.id),
79797981
)?))
79807982
};
79817983
apictx

nexus/types/src/external_api/views.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,10 @@ pub struct WebhookDelivery {
11181118

11191119
/// Individual attempts to deliver this webhook event, and their outcomes.
11201120
pub attempts: Vec<WebhookDeliveryAttempt>,
1121+
1122+
/// The time at which this delivery began (i.e. the event was dispatched to
1123+
/// the receiver).
1124+
pub time_started: DateTime<Utc>,
11211125
}
11221126

11231127
/// The state of a webhook delivery attempt.

openapi/nexus.json

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11999,7 +11999,7 @@
1199911999
"in": "query",
1200012000
"name": "sort_by",
1200112001
"schema": {
12002-
"$ref": "#/components/schemas/IdSortMode"
12002+
"$ref": "#/components/schemas/TimeAndIdSortMode"
1200312003
}
1200412004
}
1200512005
],
@@ -25776,6 +25776,25 @@
2577625776
]
2577725777
}
2577825778
]
25779+
},
25780+
"TimeAndIdSortMode": {
25781+
"description": "Supported set of sort modes for scanning by timestamp and ID",
25782+
"oneOf": [
25783+
{
25784+
"description": "sort in increasing order of timestamp and ID, i.e., earliest first",
25785+
"type": "string",
25786+
"enum": [
25787+
"ascending"
25788+
]
25789+
},
25790+
{
25791+
"description": "sort in increasing order of timestamp and ID, i.e., most recent first",
25792+
"type": "string",
25793+
"enum": [
25794+
"descending"
25795+
]
25796+
}
25797+
]
2577925798
}
2578025799
},
2578125800
"responses": {

0 commit comments

Comments
 (0)