Make ensure_key settle on one app when two first calls race
After creating an app, ensure_key lists again and picks the oldest active-or-provisioning app with that name, so two concurrent first calls return the same key. The doc comment now says what holds: the extra app can still exist, since the client has no delete call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: siGit Code v1.6.3 <noreply@sigit.si>
Seto Elkahfi committed
Oct 10, 2026 at 01:24 UTC
ba341cfddafd984930a9d2cd2e9248dfb1f958fb
2 files changed
+57
-11
crates/ed-onde-account/src/lib.rs
+31
-11
@@ -304,20 +304,24 @@ impl OndeAccount {
304
}
305
306
/// Reuse the user's app named `name` (or create it), activate it if needed and return
307
- /// its key. Safe to call again: it never creates a second app with the same name.
307
+ /// its key. Calls made one after another reuse the app. Nothing makes list-then-create
308
+ /// atomic, so two concurrent first calls can each create one; both then list again and
309
+ /// settle on the same (oldest) app, so they return the same key. The extra app stays in
310
+ /// the account, because this client has no delete call.
311
pub async fn ensure_key(&self, token: &AccessToken, name: &str) -> Result<ApiKey> {
312
let apps = self.apps(token).await?;
310
- let named = || apps.iter().filter(|a| a.name == name);
311
- let existing = named()
312
- .find(|a| a.status == AppStatus::Active)
313
- .or_else(|| named().find(|a| a.status != AppStatus::Suspended))
314
- .cloned();
315
-
316
- let mut app = match existing {
317
- Some(app) => app,
313
+ let mut app = match pick(&apps, name) {
314
+ Some(app) => app.clone(),
315
// Whatever is left under this name is suspended.
319
- None if named().next().is_some() => return Err(Error::Suspended(name.to_owned())),
320
- None => self.create_app(token, name).await?,
316
+ None if apps.iter().any(|a| a.name == name) => {
317
+ return Err(Error::Suspended(name.to_owned()));
318
+ }
319
+ None => {
320
+ let created = self.create_app(token, name).await?;
321
+ // Another caller may have created one too: settle on the same app they do.
322
+ let apps = self.apps(token).await?;
323
+ pick(&apps, name).cloned().unwrap_or(created)
324
+ }
325
};
326
if app.status != AppStatus::Active {
327
app = self.activate(token, &app.id).await?;
@@ -391,3 +395,19 @@ fn opt_string_or_number<'de, D: Deserializer<'de>>(d: D) -> Result<Option<String
395
))),
396
}
397
}
398
+
399
+/// The app every caller agrees on among those named `name`: an active one before a
400
+/// provisioning one, then the lowest id (numeric when it parses). Suspended apps never win.
401
+fn pick<'a>(apps: &'a [OndeApp], name: &str) -> Option<&'a OndeApp> {
402
+ apps.iter()
403
+ .filter(|a| a.name == name && a.status != AppStatus::Suspended)
404
+ .min_by_key(|a| {
405
+ let numeric = a.id.parse::<u64>().ok();
406
+ (
407
+ a.status != AppStatus::Active,
408
+ numeric.is_none(),
409
+ numeric,
410
+ a.id.as_str(),
411
+ )
412
+ })
413
+}
crates/ed-onde-account/tests/api.rs
+26
@@ -15,6 +15,8 @@ const TOKEN: &str = "jwt-123";
15
#[derive(Default)]
16
struct Mock {
17
apps: Vec<Value>,
18
+ /// An app another caller creates just before this server handles a `POST`.
19
+ rival: Option<Value>,
20
/// Every `(method, path, env)` the server saw.
21
calls: Vec<String>,
22
}
@@ -35,6 +37,7 @@ fn authorized(headers: &HeaderMap) -> bool {
37
async fn serve(apps: Vec<Value>) -> (OndeAccount, Shared) {
38
let state: Shared = Arc::new(Mutex::new(Mock {
39
apps,
40
+ rival: None,
41
calls: vec![],
42
}));
43
@@ -88,6 +91,9 @@ async fn serve(apps: Vec<Value>) -> (OndeAccount, Shared) {
91
async fn create(State(s): State<Shared>, Json(body): Json<Value>) -> (StatusCode, Json<Value>) {
92
let mut s = s.lock().unwrap();
93
s.calls.push("create".into());
94
+ if let Some(rival) = s.rival.take() {
95
+ s.apps.push(rival);
96
+ }
97
let created = app(
98
"new-1",
99
body["gresiq_app"]["name"].as_str().unwrap(),
@@ -215,6 +221,26 @@ async fn ensure_key_creates_then_activates_when_none_exists() {
221
assert_eq!(state.lock().unwrap().calls.len(), 2);
222
}
223
224
+#[tokio::test]
225
+async fn ensure_key_converges_when_a_create_races() {
226
+ let (account, state) = serve(vec![]).await;
227
+ // The other caller's create lands first, so its app has the lower id.
228
+ state.lock().unwrap().rival = Some(app("7", "KaroKowe", "provisioning"));
229
+ let key = account
230
+ .ensure_key(&AccessToken::new(TOKEN), "KaroKowe")
231
+ .await
232
+ .unwrap();
233
+ assert_eq!(
234
+ key.as_str(),
235
+ "7:sec-7",
236
+ "the oldest app wins, not the one just created"
237
+ );
238
+ assert_eq!(
239
+ state.lock().unwrap().calls,
240
+ vec!["create".to_owned(), r#"activate 7 "active""#.to_owned()]
241
+ );
242
+}
243
+
244
#[tokio::test]
245
async fn ensure_key_refuses_a_suspended_app() {
246
let (account, _) = serve(vec![app("a1", "KaroKowe", "suspended")]).await;