Skip to content

Commit 6c732fd

Browse files
bstriebstrie
authored
feat: HTTP endpoint for account deletion (#205)
1 parent de6c5f1 commit 6c732fd

File tree

4 files changed

+147
-21
lines changed

4 files changed

+147
-21
lines changed

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

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ pub trait NodeStore: Clone + Send + Sync + 'static {
2828
account: AccountDetails,
2929
) -> Box<dyn Future<Item = Self::Account, Error = ()> + Send>;
3030

31+
fn remove_account(
32+
&self,
33+
id: <Self::Account as AccountTrait>::AccountId,
34+
) -> Box<dyn Future<Item = Self::Account, Error = ()> + Send>;
35+
3136
// TODO limit the number of results and page through them
3237
fn get_all_accounts(&self) -> Box<dyn Future<Item = Vec<Self::Account>, Error = ()> + Send>;
3338

‎crates/interledger-api/src/routes/accounts.rs

+45-15
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,17 @@ pub struct AccountsApi<T> {
3939

4040
const MAX_RETRIES: usize = 10;
4141

42+
// Convenience function to clean up error handling and reduce unwrap quantity
43+
trait ErrorStatus {
44+
fn error(code: u16) -> Self;
45+
}
46+
47+
impl ErrorStatus for Response<()> {
48+
fn error(code: u16) -> Self {
49+
Response::builder().status(code).body(()).unwrap()
50+
}
51+
}
52+
4253
impl_web! {
4354
impl<T, A> AccountsApi<T>
4455
where T: NodeStore<Account = A> + HttpStore<Account = A> + BalanceStore<Account = A>,
@@ -61,7 +72,7 @@ impl_web! {
6172
ok(self.store.clone())
6273
} else {
6374
error!("Admin API endpoint called with non-admin API key");
64-
err(Response::builder().status(401).body(()).unwrap())
75+
err(Response::error(401))
6576
}
6677
}
6778

@@ -73,14 +84,14 @@ impl_web! {
7384
let se_url = body.settlement_engine_url.clone();
7485
self.validate_admin(authorization)
7586
.and_then(move |store| store.insert_account(body)
76-
.map_err(|_| Response::builder().status(500).body(()).unwrap())
87+
.map_err(|_| Response::error(500))
7788
.and_then(|account| {
7889
// if the account had a SE associated with it, then register
7990
// the account in the SE.
8091
if let Some(se_url) = se_url {
8192
let id = account.id();
8293
Either::A(result(Url::parse(&se_url))
83-
.map_err(|_| Response::builder().status(500).body(()).unwrap())
94+
.map_err(|_| Response::error(500))
8495
.and_then(move |mut se_url| {
8596
se_url
8697
.path_segments_mut()
@@ -109,7 +120,7 @@ impl_web! {
109120
})
110121
};
111122
Retry::spawn(FixedInterval::from_millis(2000).take(MAX_RETRIES), action)
112-
.map_err(|_| Response::builder().status(500).body(()).unwrap())
123+
.map_err(|_| Response::error(500))
113124
.and_then(move |_| {
114125
Ok(json!(account))
115126
})
@@ -126,12 +137,12 @@ impl_web! {
126137
let store = self.store.clone();
127138
if self.is_admin(&authorization) {
128139
Either::A(store.get_all_accounts()
129-
.map_err(|_| Response::builder().status(500).body(()).unwrap())
140+
.map_err(|_| Response::error(500))
130141
.and_then(|accounts| Ok(json!(accounts))))
131142
} else {
132143
// Only allow the user to see their own account
133144
Either::B(store.get_account_from_http_token(authorization.as_str())
134-
.map_err(|_| Response::builder().status(404).body(()).unwrap())
145+
.map_err(|_| Response::error(404))
135146
.and_then(|account| Ok(json!(vec![account]))))
136147
}
137148
}
@@ -143,32 +154,51 @@ impl_web! {
143154
let is_admin = self.is_admin(&authorization);
144155
let parsed_id: Result<A::AccountId, ()> = A::AccountId::from_str(&id).map_err(|_| error!("Invalid id"));
145156
result(parsed_id)
146-
.map_err(|_| Response::builder().status(400).body(()).unwrap())
157+
.map_err(|_| Response::error(400))
147158
.and_then(move |id| {
148159
if is_admin {
149160
Either::A(store.get_accounts(vec![id])
150161
.map_err(move |_| {
151162
debug!("Account not found: {}", id);
152-
Response::builder().status(404).body(()).unwrap()
163+
Response::error(404)
153164
})
154165
.and_then(|mut accounts| Ok(json!(accounts.pop().unwrap()))))
155166
} else {
156167
Either::B(store.get_account_from_http_token(&authorization[BEARER_TOKEN_START..])
157168
.map_err(move |_| {
158169
debug!("No account found with auth: {}", authorization);
159-
Response::builder().status(401).body(()).unwrap()
170+
Response::error(401)
160171
})
161172
.and_then(move |account| {
162173
if account.id() == id {
163174
Ok(json!(account))
164175
} else {
165-
Err(Response::builder().status(401).body(()).unwrap())
176+
Err(Response::error(401))
166177
}
167178
}))
168179
}
169180
})
170181
}
171182

183+
#[delete("/accounts/:id")]
184+
#[content_type("application/json")]
185+
fn delete_account(&self, id: String, authorization: String) -> impl Future<Item = Value, Error = Response<()>> {
186+
let parsed_id: Result<A::AccountId, ()> = A::AccountId::from_str(&id).map_err(|_| error!("Invalid id"));
187+
self.validate_admin(authorization)
188+
.and_then(move |store| match parsed_id {
189+
Ok(id) => Ok((store, id)),
190+
Err(_) => Err(Response::error(400)),
191+
})
192+
.and_then(move |(store, id)|
193+
store.remove_account(id)
194+
.map_err(|_| Response::error(500))
195+
.and_then(move |account| {
196+
// TODO: deregister from SE if url is present
197+
Ok(json!(account))
198+
})
199+
)
200+
}
201+
172202
// TODO should this be combined into the account record?
173203
#[get("/accounts/:id/balance")]
174204
#[content_type("application/json")]
@@ -178,32 +208,32 @@ impl_web! {
178208
let is_admin = self.is_admin(&authorization);
179209
let parsed_id: Result<A::AccountId, ()> = A::AccountId::from_str(&id).map_err(|_| error!("Invalid id"));
180210
result(parsed_id)
181-
.map_err(|_| Response::builder().status(400).body(()).unwrap())
211+
.map_err(|_| Response::error(400))
182212
.and_then(move |id| {
183213
if is_admin {
184214
Either::A(store.get_accounts(vec![id])
185215
.map_err(move |_| {
186216
debug!("Account not found: {}", id);
187-
Response::builder().status(404).body(()).unwrap()
217+
Response::error(404)
188218
})
189219
.and_then(|mut accounts| Ok(accounts.pop().unwrap())))
190220
} else {
191221
Either::B(store.get_account_from_http_token(&authorization[BEARER_TOKEN_START..])
192222
.map_err(move |_| {
193223
debug!("No account found with auth: {}", authorization);
194-
Response::builder().status(401).body(()).unwrap()
224+
Response::error(401)
195225
})
196226
.and_then(move |account| {
197227
if account.id() == id {
198228
Ok(account)
199229
} else {
200-
Err(Response::builder().status(401).body(()).unwrap())
230+
Err(Response::error(401))
201231
}
202232
}))
203233
}
204234
})
205235
.and_then(move |account| store_clone.get_balance(account)
206-
.map_err(|_| Response::builder().status(500).body(()).unwrap()))
236+
.map_err(|_| Response::error(500)))
207237
.and_then(|balance| Ok(BalanceResponse {
208238
balance: balance.to_string(),
209239
}))

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

+78-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
// The informal schema of our data in redis:
2+
// send_routes_to set used for CCP routing
3+
// receive_routes_from set used for CCP routing
4+
// next_account_id string unique ID for each new account
5+
// rates:current hash exchange rates
6+
// routes:current hash dynamic routing table
7+
// routes:static hash static routing table
8+
// accounts:<id> hash details for each account
9+
// http_auth hash maps hmac of cryptographic credentials to an account
10+
// btp_auth hash maps hmac of cryptographic credentials to an account
11+
// btp_outgoing
12+
// For interactive exploration of the store,
13+
// use the redis-cli tool included with your redis install.
14+
// Within redis-cli:
15+
// keys * list all keys of any type in the store
16+
// smembers <key> list the members of a set
17+
// get <key> get the value of a key
18+
// hgetall <key> the flattened list of every key/value entry within a hash
19+
120
use super::account::*;
221
use super::crypto::generate_keys;
322
use bytes::Bytes;
@@ -376,15 +395,52 @@ impl RedisStore {
376395
}),
377396
)
378397
}
379-
}
380398

381-
impl AccountStore for RedisStore {
382-
type Account = Account;
399+
fn delete_account(&self, id: u64) -> Box<dyn Future<Item = Account, Error = ()> + Send> {
400+
let connection = self.connection.as_ref().clone();
401+
let routing_table = self.routes.clone();
383402

384-
// TODO cache results to avoid hitting Redis for each packet
385-
fn get_accounts(
403+
Box::new(
404+
// TODO: a retrieve_account API to avoid making Vecs which we only need one element of
405+
self.retrieve_accounts(vec![id])
406+
.and_then(|accounts| accounts.get(0).cloned().ok_or(()))
407+
.and_then(|account| {
408+
let mut pipe = redis::pipe();
409+
pipe.atomic();
410+
411+
pipe.del(account_details_key(account.id));
412+
413+
if account.send_routes {
414+
pipe.srem("send_routes_to", account.id).ignore();
415+
}
416+
417+
if account.receive_routes {
418+
pipe.srem("receive_routes_from", account.id).ignore();
419+
}
420+
421+
if account.btp_uri.is_some() {
422+
pipe.srem("btp_outgoing", account.id).ignore();
423+
}
424+
425+
pipe.hdel(ROUTES_KEY, account.ilp_address.to_bytes().to_vec())
426+
.ignore();
427+
428+
pipe.query_async(connection)
429+
.map_err(|err| error!("Error deleting account from DB: {:?}", err))
430+
.and_then(move |(connection, _ret): (SharedConnection, Value)| {
431+
update_routes(connection, routing_table)
432+
})
433+
.and_then(move |_| {
434+
debug!("Deleted account {}", account.id);
435+
Ok(account)
436+
})
437+
}),
438+
)
439+
}
440+
441+
fn retrieve_accounts(
386442
&self,
387-
account_ids: Vec<<Self::Account as AccountTrait>::AccountId>,
443+
account_ids: Vec<u64>,
388444
) -> Box<dyn Future<Item = Vec<Account>, Error = ()> + Send> {
389445
let decryption_key = self.decryption_key.clone();
390446
let num_accounts = account_ids.len();
@@ -417,6 +473,18 @@ impl AccountStore for RedisStore {
417473
}
418474
}
419475

476+
impl AccountStore for RedisStore {
477+
type Account = Account;
478+
479+
// TODO cache results to avoid hitting Redis for each packet
480+
fn get_accounts(
481+
&self,
482+
account_ids: Vec<<Self::Account as AccountTrait>::AccountId>,
483+
) -> Box<dyn Future<Item = Vec<Account>, Error = ()> + Send> {
484+
self.retrieve_accounts(account_ids)
485+
}
486+
}
487+
420488
impl BalanceStore for RedisStore {
421489
/// Returns the balance **from the account holder's perspective**, meaning the sum of
422490
/// the Payable Balance and Pending Outgoing minus the Receivable Balance and the Pending Incoming.
@@ -691,6 +759,10 @@ impl NodeStore for RedisStore {
691759
self.create_new_account(account)
692760
}
693761

762+
fn remove_account(&self, id: u64) -> Box<dyn Future<Item = Account, Error = ()> + Send> {
763+
self.delete_account(id)
764+
}
765+
694766
// TODO limit the number of results and page through them
695767
fn get_all_accounts(&self) -> Box<dyn Future<Item = Vec<Self::Account>, Error = ()> + Send> {
696768
let decryption_key = self.decryption_key.clone();

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

+19
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,25 @@ fn insert_accounts() {
2626
.unwrap();
2727
}
2828

29+
#[test]
30+
fn delete_accounts() {
31+
block_on(test_store().and_then(|(store, context)| {
32+
store.get_all_accounts().and_then(move |accounts| {
33+
let id = accounts[0].id();
34+
store.remove_account(id).and_then(move |_| {
35+
store.get_all_accounts().and_then(move |accounts| {
36+
for a in accounts {
37+
assert_ne!(id, a.id());
38+
}
39+
let _ = context;
40+
Ok(())
41+
})
42+
})
43+
})
44+
}))
45+
.unwrap();
46+
}
47+
2948
#[test]
3049
fn starts_with_zero_balance() {
3150
block_on(test_store().and_then(|(store, context)| {

0 commit comments

Comments
 (0)