Skip to content

Commit 0d60f17

Browse files
committed
refactor(aggregator): move parameters module to http_server directly
instead of being a child of the `middleware` file and module
1 parent 19ae551 commit 0d60f17

File tree

6 files changed

+199
-203
lines changed

6 files changed

+199
-203
lines changed

mithril-aggregator/src/http_server/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod parameters;
12
pub mod routes;
23
pub mod validators;
34

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
//! Helpers when working with parameters from either the query string or the path of an HTTP request
2+
3+
use anyhow::Context;
4+
use std::ops::Deref;
5+
6+
use mithril_common::StdResult;
7+
use mithril_common::entities::Epoch;
8+
9+
use crate::dependency_injection::EpochServiceWrapper;
10+
11+
/// An epoch parsed from a http request parameter
12+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
13+
pub enum ExpandedEpoch {
14+
/// Epoch was explicitly provided as a number (e.g., "123")
15+
Parsed(Epoch),
16+
/// Epoch was provided as "latest" (e.g., "latest")
17+
Latest(Epoch),
18+
/// Epoch was provided as "latest-{offset}" (e.g., "latest-100")
19+
LatestMinusOffset(Epoch, u64),
20+
}
21+
22+
impl Deref for ExpandedEpoch {
23+
type Target = Epoch;
24+
25+
fn deref(&self) -> &Self::Target {
26+
match self {
27+
ExpandedEpoch::Parsed(epoch)
28+
| ExpandedEpoch::Latest(epoch)
29+
| ExpandedEpoch::LatestMinusOffset(epoch, _) => epoch,
30+
}
31+
}
32+
}
33+
34+
impl ExpandedEpoch {
35+
/// Returns true if this epoch was expanded from 'latest-{offset}' and the offset
36+
/// is greater than the given value.
37+
pub fn has_offset_greater_than(&self, value: u64) -> bool {
38+
match self {
39+
ExpandedEpoch::Parsed(_) | ExpandedEpoch::Latest(_) => false,
40+
ExpandedEpoch::LatestMinusOffset(_, offset) => offset > &value,
41+
}
42+
}
43+
44+
/// Apply an additional negative offset to the epoch, but only if it was derived from 'latest'.
45+
pub fn apply_offset_for_latest(&self, additional_offset: u64) -> Epoch {
46+
match self {
47+
ExpandedEpoch::Parsed(epoch) => *epoch,
48+
ExpandedEpoch::Latest(epoch) | ExpandedEpoch::LatestMinusOffset(epoch, ..) => {
49+
Epoch(epoch.saturating_sub(additional_offset))
50+
}
51+
}
52+
}
53+
}
54+
55+
/// Parse the string into an [Epoch] if it's a number, or if it's 'latest{-{offset}}' expand
56+
/// into the actual epoch minus the optional given offset.
57+
///
58+
/// Return the expanded epoch and the eventual offset that was applied
59+
pub async fn expand_epoch(
60+
epoch_str: &str,
61+
epoch_service: EpochServiceWrapper,
62+
) -> StdResult<ExpandedEpoch> {
63+
let epoch_str = epoch_str.to_lowercase();
64+
if epoch_str == "latest" {
65+
epoch_service
66+
.read()
67+
.await
68+
.epoch_of_current_data()
69+
.map(ExpandedEpoch::Latest)
70+
} else if epoch_str.starts_with("latest-") {
71+
let (_, offset_str) = epoch_str.split_at("latest-".len());
72+
let offset = offset_str
73+
.parse::<u64>()
74+
.with_context(|| "Invalid epoch offset: must be a number")?;
75+
76+
epoch_service.read().await.epoch_of_current_data().map(|epoch| {
77+
ExpandedEpoch::LatestMinusOffset(Epoch(epoch.saturating_sub(offset)), offset)
78+
})
79+
} else {
80+
epoch_str
81+
.parse::<u64>()
82+
.map(|epoch| ExpandedEpoch::Parsed(Epoch(epoch)))
83+
.with_context(|| "Invalid epoch: must be a number or 'latest'")
84+
}
85+
}
86+
87+
#[cfg(test)]
88+
mod tests {
89+
use std::sync::Arc;
90+
use tokio::sync::RwLock;
91+
92+
use crate::services::FakeEpochServiceBuilder;
93+
94+
use super::*;
95+
96+
fn fake_epoch_service(returned_epoch: Epoch) -> EpochServiceWrapper {
97+
Arc::new(RwLock::new(
98+
FakeEpochServiceBuilder::dummy(returned_epoch).build(),
99+
))
100+
}
101+
102+
#[tokio::test]
103+
async fn use_given_epoch_if_valid_number() {
104+
let epoch_service = fake_epoch_service(Epoch(300));
105+
let expanded_epoch = expand_epoch("456", epoch_service).await.unwrap();
106+
107+
assert_eq!(expanded_epoch, ExpandedEpoch::Parsed(Epoch(456)));
108+
}
109+
110+
#[tokio::test]
111+
async fn use_epoch_service_current_epoch_if_latest() {
112+
let epoch_service = fake_epoch_service(Epoch(89));
113+
let expanded_epoch = expand_epoch("latest", epoch_service).await.unwrap();
114+
115+
assert_eq!(expanded_epoch, ExpandedEpoch::Latest(Epoch(89)));
116+
}
117+
118+
#[tokio::test]
119+
async fn use_offset_epoch_service_current_epoch_if_latest_minus_a_number() {
120+
let epoch_service = fake_epoch_service(Epoch(89));
121+
let expanded_epoch = expand_epoch("latest-13", epoch_service.clone()).await.unwrap();
122+
assert_eq!(
123+
expanded_epoch,
124+
ExpandedEpoch::LatestMinusOffset(Epoch(76), 13)
125+
);
126+
127+
let expanded_epoch = expand_epoch("latest-0", epoch_service).await.unwrap();
128+
assert_eq!(
129+
expanded_epoch,
130+
ExpandedEpoch::LatestMinusOffset(Epoch(89), 0)
131+
);
132+
}
133+
134+
#[tokio::test]
135+
async fn error_if_given_epoch_is_not_a_number_nor_latest_nor_latest_minus_a_number() {
136+
let epoch_service = fake_epoch_service(Epoch(78));
137+
for invalid_epoch in [
138+
"invalid",
139+
"latest-",
140+
"latest+",
141+
"latest+0",
142+
"latest+293",
143+
"latest-4.5",
144+
"latest+2.9",
145+
"latest-invalid",
146+
] {
147+
expand_epoch(invalid_epoch, epoch_service.clone()).await.expect_err(
148+
"Should fail if epoch is not a number nor 'latest' nor 'latest-{offset}'",
149+
);
150+
}
151+
}
152+
153+
#[tokio::test]
154+
async fn dont_overflow_if_epoch_minus_offset_is_negative() {
155+
let epoch_service = fake_epoch_service(Epoch(89));
156+
let expanded_epoch = expand_epoch(&format!("latest-{}", u64::MAX), epoch_service)
157+
.await
158+
.unwrap();
159+
160+
assert_eq!(
161+
expanded_epoch,
162+
ExpandedEpoch::LatestMinusOffset(Epoch(0), u64::MAX)
163+
);
164+
}
165+
166+
#[test]
167+
fn test_expanded_epoch_is_parsed_offset_greater_than() {
168+
assert!(!ExpandedEpoch::Parsed(Epoch(90)).has_offset_greater_than(15));
169+
assert!(!ExpandedEpoch::Latest(Epoch(90)).has_offset_greater_than(15));
170+
assert!(!ExpandedEpoch::LatestMinusOffset(Epoch(90), 10).has_offset_greater_than(10));
171+
assert!(ExpandedEpoch::LatestMinusOffset(Epoch(90), 11).has_offset_greater_than(10));
172+
}
173+
174+
#[test]
175+
fn test_expanded_epoch_apply_additional_offset() {
176+
assert_eq!(
177+
ExpandedEpoch::Parsed(Epoch(90)).apply_offset_for_latest(15),
178+
Epoch(90)
179+
);
180+
assert_eq!(
181+
ExpandedEpoch::Latest(Epoch(90)).apply_offset_for_latest(15),
182+
Epoch(75)
183+
);
184+
assert_eq!(
185+
ExpandedEpoch::LatestMinusOffset(Epoch(90), 13).apply_offset_for_latest(15),
186+
Epoch(75)
187+
);
188+
assert_eq!(
189+
ExpandedEpoch::Latest(Epoch(90)).apply_offset_for_latest(u64::MAX),
190+
Epoch(0)
191+
);
192+
}
193+
}

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_database.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ mod handlers {
8484

8585
use crate::MetricsService;
8686
use crate::dependency_injection::EpochServiceWrapper;
87-
use crate::http_server::routes::middlewares::{ClientMetadata, parameters};
88-
use crate::http_server::routes::reply;
87+
use crate::http_server::{parameters, routes::middlewares::ClientMetadata, routes::reply};
8988
use crate::services::MessageService;
9089

9190
pub const LIST_MAX_ITEMS: usize = 20;

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_stake_distribution.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ pub mod handlers {
5959

6060
use crate::MetricsService;
6161
use crate::dependency_injection::EpochServiceWrapper;
62-
use crate::http_server::routes::middlewares::{ClientMetadata, parameters};
63-
use crate::http_server::routes::reply;
62+
use crate::http_server::{parameters, routes::middlewares::ClientMetadata, routes::reply};
6463
use crate::services::MessageService;
6564

6665
pub const LIST_MAX_ITEMS: usize = 20;

0 commit comments

Comments
 (0)