Skip to content

Commit 1f6f9a4

Browse files
gakonstgakonst
committed
feat(settlement/engine): Return None when idempotent data not found instead of Error
1 parent 56237a6 commit 1f6f9a4

File tree

10 files changed

+60
-41
lines changed

10 files changed

+60
-41
lines changed

‎crates/interledger-settlement-engines/src/api.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use futures::{
77
use hyper::{Response, StatusCode};
88
use interledger_settlement::Quantity;
99
use interledger_settlement::{IdempotentData, IdempotentStore};
10-
use log::trace;
10+
use log::error;
1111
use ring::digest::{digest, SHA256};
1212
use tokio::executor::spawn;
1313
use tower_web::{net::ConnectionStream, ServiceBuilder};
@@ -77,19 +77,23 @@ impl_web! {
7777
&self,
7878
idempotency_key: String,
7979
input_hash: [u8; 32],
80-
) -> impl Future<Item = (StatusCode, Bytes), Error = String> {
80+
) -> impl Future<Item = Option<(StatusCode, Bytes)>, Error = String> {
8181
self.store
8282
.load_idempotent_data(idempotency_key.clone())
8383
.map_err(move |_| {
84-
let error_msg = "Couldn't load idempotent data".to_owned();
85-
trace!("{}", error_msg);
84+
let error_msg = format!("Couldn't load idempotent data for idempotency key {:?}", idempotency_key);
85+
error!("{}", error_msg);
8686
error_msg
8787
})
88-
.and_then(move |ret: IdempotentData| {
89-
if ret.2 == input_hash {
90-
Ok((ret.0, ret.1))
88+
.and_then(move |ret: Option<IdempotentData>| {
89+
if let Some(ret) = ret {
90+
if ret.2 == input_hash {
91+
Ok(Some((ret.0, ret.1)))
92+
} else {
93+
Ok(Some((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..]))))
94+
}
9195
} else {
92-
Ok((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..])))
96+
Ok(None)
9397
}
9498
})
9599
}
@@ -104,8 +108,8 @@ impl_web! {
104108
Either::A(
105109
self.check_idempotency(idempotency_key.clone(), input_hash)
106110
.map_err(|err| Response::builder().status(502).body(err).unwrap())
107-
.then(move |ret: Result<(StatusCode, Bytes), Response<String>>| {
108-
if let Ok(ret) = ret {
111+
.and_then(move |ret: Option<(StatusCode, Bytes)>| {
112+
if let Some(ret) = ret {
109113
let resp = Response::builder().status(ret.0).body(String::from_utf8_lossy(&ret.1).to_string()).unwrap();
110114
if ret.0.is_success() {
111115
return Either::A(Either::A(ok(resp)))

‎crates/interledger-settlement-engines/src/engines/ethereum_ledger/eth_engine.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,7 @@ where
769769
// If we receive a Quantity { amount: "1", scale: 9},
770770
// we must normalize it to our engine's scale (typically 18
771771
// for ETH/ERC20).
772+
772773
let amount = amount_from_connector.normalize_scale(ConvertDetails {
773774
from: body.scale,
774775
to: engine_scale,

‎crates/interledger-settlement-engines/src/engines/ethereum_ledger/test_helpers.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,14 @@ impl IdempotentStore for TestStore {
176176
fn load_idempotent_data(
177177
&self,
178178
idempotency_key: String,
179-
) -> Box<dyn Future<Item = IdempotentData, Error = ()> + Send> {
179+
) -> Box<dyn Future<Item = Option<IdempotentData>, Error = ()> + Send> {
180180
let cache = self.cache.read();
181181
if let Some(data) = cache.get(&idempotency_key) {
182182
let mut guard = self.cache_hits.write();
183183
*guard += 1; // used to test how many times this branch gets executed
184-
Box::new(ok((data.0, Bytes::from(data.1.clone()), data.2)))
184+
Box::new(ok(Some((data.0, Bytes::from(data.1.clone()), data.2))))
185185
} else {
186-
Box::new(err(()))
186+
Box::new(ok(None))
187187
}
188188
}
189189

‎crates/interledger-settlement-engines/src/stores/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,10 @@ pub type IdempotentEngineData = (StatusCode, Bytes, [u8; 32]);
1313
pub trait IdempotentEngineStore {
1414
/// Returns the API response that was saved when the idempotency key was used
1515
/// Also returns a hash of the input data which resulted in the response
16-
/// Returns error if the data was not found
1716
fn load_idempotent_data(
1817
&self,
1918
idempotency_key: String,
20-
) -> Box<dyn Future<Item = IdempotentEngineData, Error = ()> + Send>;
19+
) -> Box<dyn Future<Item = Option<IdempotentEngineData>, Error = ()> + Send>;
2120

2221
/// Saves the data that was passed along with the api request for later
2322
/// The store MUST also save a hash of the input, so that it errors out on requests

‎crates/interledger-settlement-engines/src/stores/redis_store_common.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl IdempotentEngineStore for EngineRedisStore {
4343
fn load_idempotent_data(
4444
&self,
4545
idempotency_key: String,
46-
) -> Box<dyn Future<Item = IdempotentEngineData, Error = ()> + Send> {
46+
) -> Box<dyn Future<Item = Option<IdempotentEngineData>, Error = ()> + Send> {
4747
let idempotency_key_clone = idempotency_key.clone();
4848
Box::new(
4949
cmd("HGETALL")
@@ -65,13 +65,13 @@ impl IdempotentEngineStore for EngineRedisStore {
6565
trace!("Loaded idempotency key {:?} - {:?}", idempotency_key, ret);
6666
let mut input_hash: [u8; 32] = Default::default();
6767
input_hash.copy_from_slice(input_hash_slice.as_ref());
68-
Ok((
68+
Ok(Some((
6969
StatusCode::from_str(status_code).unwrap(),
7070
Bytes::from(data.clone()),
7171
input_hash,
72-
))
72+
)))
7373
} else {
74-
Err(())
74+
Ok(None)
7575
}
7676
},
7777
),
@@ -137,20 +137,23 @@ mod tests {
137137
.load_idempotent_data(IDEMPOTENCY_KEY.clone())
138138
.map_err(|err| eprintln!("Redis error: {:?}", err))
139139
.and_then(move |data1| {
140-
assert_eq!(data1, (StatusCode::OK, Bytes::from("TEST"), input_hash));
140+
assert_eq!(
141+
data1.unwrap(),
142+
(StatusCode::OK, Bytes::from("TEST"), input_hash)
143+
);
141144
let _ = context;
142145

143146
store
144147
.load_idempotent_data("asdf".to_string())
145148
.map_err(|err| eprintln!("Redis error: {:?}", err))
146-
.and_then(move |_data2| {
147-
assert_eq!(_data2.0, StatusCode::OK);
149+
.and_then(move |data2| {
150+
assert!(data2.is_none());
148151
let _ = context;
149152
Ok(())
150153
})
151154
})
152155
})
153156
}))
154-
.unwrap_err() // the second idempotent load fails
157+
.unwrap()
155158
}
156159
}

‎crates/interledger-settlement/src/api.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,23 @@ impl_web! {
5757
&self,
5858
idempotency_key: String,
5959
input_hash: [u8; 32],
60-
) -> impl Future<Item = (StatusCode, Bytes), Error = String> {
60+
) -> impl Future<Item = Option<(StatusCode, Bytes)>, Error = String> {
6161
self.store
6262
.load_idempotent_data(idempotency_key.clone())
6363
.map_err(move |_| {
6464
let error_msg = "Couldn't load idempotent data".to_owned();
6565
error!("{}", error_msg);
6666
error_msg
6767
})
68-
.and_then(move |ret: IdempotentData| {
69-
if ret.2 == input_hash {
70-
Ok((ret.0, ret.1))
68+
.and_then(move |ret: Option<IdempotentData>| {
69+
if let Some(ret) = ret {
70+
if ret.2 == input_hash {
71+
Ok(Some((ret.0, ret.1)))
72+
} else {
73+
Ok(Some((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..]))))
74+
}
7175
} else {
72-
Ok((StatusCode::from_u16(409).unwrap(), Bytes::from(&b"Provided idempotency key is tied to other input"[..])))
76+
Ok(None)
7377
}
7478
})
7579
}
@@ -83,8 +87,12 @@ impl_web! {
8387
// key, perform the call and save the idempotent return data
8488
Either::A(
8589
self.check_idempotency(idempotency_key.clone(), input_hash)
86-
.then(move |ret: Result<(StatusCode, Bytes), String>| {
87-
if let Ok(ret) = ret {
90+
.map_err(move |err| {
91+
let status_code = StatusCode::from_u16(500).unwrap();
92+
Response::builder().status(status_code).body(err).unwrap()
93+
})
94+
.and_then(move |ret: Option<(StatusCode, Bytes)>| {
95+
if let Some(ret) = ret {
8896
if ret.0.is_success() {
8997
let resp = Response::builder().status(ret.0).body(ret.1).unwrap();
9098
return Either::A(Either::A(ok(resp)))

‎crates/interledger-settlement/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub trait IdempotentStore {
8181
fn load_idempotent_data(
8282
&self,
8383
idempotency_key: String,
84-
) -> Box<dyn Future<Item = IdempotentData, Error = ()> + Send>;
84+
) -> Box<dyn Future<Item = Option<IdempotentData>, Error = ()> + Send>;
8585

8686
/// Saves the data that was passed along with the api request for later
8787
/// The store MUST also save a hash of the input, so that it errors out on requests

‎crates/interledger-settlement/src/test_helpers.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@ impl IdempotentStore for TestStore {
9797
fn load_idempotent_data(
9898
&self,
9999
idempotency_key: String,
100-
) -> Box<dyn Future<Item = IdempotentData, Error = ()> + Send> {
100+
) -> Box<dyn Future<Item = Option<IdempotentData>, Error = ()> + Send> {
101101
let cache = self.cache.read();
102102
if let Some(data) = cache.get(&idempotency_key) {
103103
let mut guard = self.cache_hits.write();
104104
*guard += 1; // used to test how many times this branch gets executed
105-
Box::new(ok((data.0, data.1.clone(), data.2)))
105+
Box::new(ok(Some((data.0, data.1.clone(), data.2))))
106106
} else {
107-
Box::new(err(()))
107+
Box::new(ok(None))
108108
}
109109
}
110110

‎crates/interledger-store-redis/src/store.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ impl IdempotentStore for RedisStore {
10831083
fn load_idempotent_data(
10841084
&self,
10851085
idempotency_key: String,
1086-
) -> Box<dyn Future<Item = IdempotentData, Error = ()> + Send> {
1086+
) -> Box<dyn Future<Item = Option<IdempotentData>, Error = ()> + Send> {
10871087
let idempotency_key_clone = idempotency_key.clone();
10881088
Box::new(
10891089
cmd("HGETALL")
@@ -1104,13 +1104,13 @@ impl IdempotentStore for RedisStore {
11041104
trace!("Loaded idempotency key {:?} - {:?}", idempotency_key, ret);
11051105
let mut input_hash: [u8; 32] = Default::default();
11061106
input_hash.copy_from_slice(input_hash_slice.as_ref());
1107-
Ok((
1107+
Ok(Some((
11081108
StatusCode::from_str(status_code).unwrap(),
11091109
Bytes::from(data.clone()),
11101110
input_hash,
1111-
))
1111+
)))
11121112
} else {
1113-
Err(())
1113+
Ok(None)
11141114
}
11151115
}),
11161116
)

‎crates/interledger-store-redis/tests/settlement_test.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,24 @@ fn saves_and_loads_idempotency_key_data_properly() {
5353
.load_idempotent_data(IDEMPOTENCY_KEY.clone())
5454
.map_err(|err| eprintln!("Redis error: {:?}", err))
5555
.and_then(move |data1| {
56-
assert_eq!(data1, (StatusCode::OK, Bytes::from("TEST"), input_hash));
56+
assert_eq!(
57+
data1.unwrap(),
58+
(StatusCode::OK, Bytes::from("TEST"), input_hash)
59+
);
5760
let _ = context;
5861

5962
store
6063
.load_idempotent_data("asdf".to_string())
6164
.map_err(|err| eprintln!("Redis error: {:?}", err))
62-
.and_then(move |_data2| {
65+
.and_then(move |data2| {
66+
assert!(data2.is_none());
6367
let _ = context;
6468
Ok(())
6569
})
6670
})
6771
})
6872
}))
69-
.unwrap_err() // second idempotent load fails
73+
.unwrap();
7074
}
7175

7276
#[test]

0 commit comments

Comments
 (0)