fix(market): relisting an expired auction actually relists it
The client's relist arrives as a fresh ISStart (`POST /auctionhouse`) for an item that ALREADY has a listing row, so `create_listing` hit a primary-key conflict. The handler treated `Err(Conflict)` as success: it logged `listed=true`, handed the client its trade id, and persisted nothing. The stale row kept its old `created_at`, so the card stayed expired and the relist appeared to do nothing -- observed live, with the client's price-limits fetch and the ISStart POST both in the log. The PK conflict IS the relist path. `relist_listing` now resets `created_at` to now and takes the new prices and duration, so the auction actually returns to the market with a fresh countdown. Refuses to revive a `sold` or `reserved` row: re-opening a sold auction would sell the same card twice. `cancelled` rows ARE relistable (the card is back in the pile). Missing rows report NotFound rather than silently succeeding. The failure paths still ack so the screen cannot wedge, but they now say `relisted=false reason=...` in the log instead of claiming success. Three store tests: the clock/price reset, the sold+reserved revival guard (plus the cancelled-is-relistable case), and NotFound. 336 tests pass, 0 failed, clippy clean.
This commit is contained in:
@@ -302,13 +302,48 @@ pub async fn handle_market_list(
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) | Err(MarketError::Conflict) => {
|
||||
Ok(_) => {
|
||||
eprintln!(
|
||||
"utas-host owner=RUST route=market-list POST item_id={} listed=true trade_id={trade_id}",
|
||||
r.item_id
|
||||
);
|
||||
ok_json(&json!({ "id": trade_id }))
|
||||
}
|
||||
// A row for this item already exists, so this POST is a RELIST.
|
||||
// FIFA 17 relists by re-sending ISStart, so the PK conflict is the
|
||||
// normal relist path — NOT an error, and NOT a success to swallow.
|
||||
// Acking it without resetting the clock is why relisting an expired
|
||||
// card appeared to do nothing: the client got its id back while the
|
||||
// stale row stayed expired.
|
||||
Err(MarketError::Conflict) => {
|
||||
match store
|
||||
.relist_listing(
|
||||
&listing_id,
|
||||
r.start,
|
||||
r.buy_now,
|
||||
r.duration,
|
||||
r.item_json.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => {
|
||||
eprintln!(
|
||||
"utas-host owner=RUST route=market-list POST item_id={} relisted=true trade_id={trade_id}",
|
||||
r.item_id
|
||||
);
|
||||
ok_json(&json!({ "id": trade_id }))
|
||||
}
|
||||
// Sold or in-flight: never revive it. Ack so the screen does
|
||||
// not wedge, but say so plainly in the log.
|
||||
Err(e) => {
|
||||
eprintln!(
|
||||
"utas-host owner=RUST route=market-list POST item_id={} relisted=false reason={e:?} trade_id={trade_id}",
|
||||
r.item_id
|
||||
);
|
||||
ok_json(&json!({ "id": trade_id }))
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(_) => json_body(503, &json!({ "error": "market_store" })),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -545,6 +545,53 @@ impl MarketStore {
|
||||
}
|
||||
outcome
|
||||
}
|
||||
|
||||
/// RE-LIST an existing auction row for the same item: reset the clock to now
|
||||
/// and take the new prices/duration.
|
||||
///
|
||||
/// FIFA 17 relists by sending a fresh `ISStart` POST for an item that already
|
||||
/// has a listing row, so the primary-key conflict is EXPECTED and means
|
||||
/// "relist", not "error". Treating that conflict as success is how a relist
|
||||
/// silently did nothing: the client was acked while the stale, already-expired
|
||||
/// row kept its old `created_at` and stayed expired.
|
||||
///
|
||||
/// Only an `active` (including aged-out) or `cancelled` row may be relisted. A
|
||||
/// `sold` or `reserved` row is NEVER resurrected — the card is gone or in
|
||||
/// flight, and re-opening that auction would sell a card twice.
|
||||
pub async fn relist_listing(
|
||||
&self,
|
||||
listing_id: &str,
|
||||
start_price: i64,
|
||||
buy_now_price: i64,
|
||||
duration_secs: Option<i64>,
|
||||
item_json: Option<&str>,
|
||||
) -> Result<Listing, MarketError> {
|
||||
let created_at = now_millis();
|
||||
let affected = sqlx::query(
|
||||
"UPDATE listings SET state = 'active', created_at = ?, start_price = ?, \
|
||||
buy_now_price = ?, duration_secs = ?, item_json = COALESCE(?, item_json) \
|
||||
WHERE listing_id = ? AND state IN ('active', 'cancelled')",
|
||||
)
|
||||
.bind(&created_at)
|
||||
.bind(start_price)
|
||||
.bind(buy_now_price)
|
||||
.bind(duration_secs)
|
||||
.bind(item_json)
|
||||
.bind(listing_id)
|
||||
.execute(&self.pool)
|
||||
.await
|
||||
.map_err(db)?
|
||||
.rows_affected();
|
||||
if affected == 0 {
|
||||
// Either no such row, or it is sold/reserved and must not be revived.
|
||||
return Err(match self.get_listing(listing_id).await {
|
||||
Ok(l) if l.state == "sold" => MarketError::Sold,
|
||||
Ok(_) => MarketError::Conflict,
|
||||
Err(e) => e,
|
||||
});
|
||||
}
|
||||
self.get_listing(listing_id).await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -576,6 +623,92 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relist_resets_the_clock_and_takes_the_new_prices() {
|
||||
// FIFA 17 relists by re-sending ISStart for an item that already has a row,
|
||||
// so the PK conflict is the relist path. Before this existed the conflict was
|
||||
// acked as success and the stale expired row kept its old created_at, so the
|
||||
// card never came back to the market.
|
||||
let (store, _d) = temp_store().await;
|
||||
let first = seed(&store, "900000001").await;
|
||||
// Age it out by rewriting created_at to well past its default duration.
|
||||
let stale = (now_secs() - DEFAULT_DURATION_SECS - 600) * 1000;
|
||||
sqlx::query("UPDATE listings SET created_at = ? WHERE listing_id = ?")
|
||||
.bind(stale.to_string())
|
||||
.bind("900000001")
|
||||
.execute(&store.pool)
|
||||
.await
|
||||
.unwrap();
|
||||
let expired = store.get_listing("900000001").await.unwrap();
|
||||
assert_eq!(
|
||||
expired.expires_in_secs(now_secs()),
|
||||
0,
|
||||
"precondition: the listing has run out"
|
||||
);
|
||||
|
||||
let relisted = store
|
||||
.relist_listing("900000001", 250, 5000, Some(10_800), None)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(relisted.state, "active");
|
||||
assert_eq!(relisted.start_price, 250, "new start price applied");
|
||||
assert_eq!(relisted.buy_now_price, 5000, "new buy-now applied");
|
||||
assert_eq!(relisted.duration_secs, Some(10_800));
|
||||
assert!(
|
||||
relisted.expires_in_secs(now_secs()) > 0,
|
||||
"the clock actually restarted"
|
||||
);
|
||||
assert_ne!(
|
||||
relisted.created_at, expired.created_at,
|
||||
"created_at moved forward"
|
||||
);
|
||||
// The snapshot is preserved when the relist does not supply a new one.
|
||||
assert_eq!(relisted.item_json, first.item_json);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relist_never_revives_a_sold_or_reserved_auction() {
|
||||
// Re-opening a sold auction would sell the same card twice.
|
||||
let (store, _d) = temp_store().await;
|
||||
seed(&store, "900000001").await;
|
||||
assert!(store.reserve_listing("900000001").await.unwrap());
|
||||
assert!(
|
||||
matches!(
|
||||
store.relist_listing("900000001", 1, 2, None, None).await,
|
||||
Err(MarketError::Conflict)
|
||||
),
|
||||
"a reserved (in-flight) auction is not relistable"
|
||||
);
|
||||
store.complete_sale("900000001").await.unwrap();
|
||||
assert!(
|
||||
matches!(
|
||||
store.relist_listing("900000001", 1, 2, None, None).await,
|
||||
Err(MarketError::Sold)
|
||||
),
|
||||
"a sold auction is never resurrected"
|
||||
);
|
||||
assert_eq!(store.get_listing("900000001").await.unwrap().state, "sold");
|
||||
|
||||
// A cancelled listing IS relistable (the card came back to the pile).
|
||||
seed(&store, "900000002").await;
|
||||
store.cancel_listing("900000002", None).await.unwrap();
|
||||
let back = store
|
||||
.relist_listing("900000002", 300, 900, None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(back.state, "active");
|
||||
assert_eq!(back.start_price, 300);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relist_of_a_missing_row_is_not_found() {
|
||||
let (store, _d) = temp_store().await;
|
||||
assert!(matches!(
|
||||
store.relist_listing("900000999", 1, 2, None, None).await,
|
||||
Err(MarketError::NotFound)
|
||||
));
|
||||
}
|
||||
|
||||
async fn temp_store() -> (MarketStore, TempDb) {
|
||||
let db = TempDb::new();
|
||||
let store = MarketStore::open(db.path()).await.unwrap();
|
||||
|
||||
Reference in New Issue
Block a user