From 0fa095a83ef1d32e3bcc1b4e3102875afed43dfe Mon Sep 17 00:00:00 2001 From: Aaron Allen Date: Sun, 16 Aug 2026 13:50:03 -0500 Subject: [PATCH] fix(sync): resolve the structures wallet transactions trade in Wallet sync stored a location_id on every transaction and resolved none of them, so a player-owned structure earned a name only when the character also held assets or contracts there. Trading there did not count. Both wallet jobs now pass the ids they store through the same resolver asset sync uses, which records a structure the subject cannot dock at in inaccessible_structures rather than asking ESI for it on every pass. The split of raw ids into stations and structures moves into structure_resolution, where the contracts job now shares it instead of keeping its own copy. Closes #115 --- src/sync/jobs/character_contracts.rs | 27 +---- src/sync/jobs/character_wallet.rs | 147 ++++++++++++++++++++++++++- src/sync/jobs/corporation_wallet.rs | 9 +- src/sync/structure_resolution.rs | 25 +++++ 4 files changed, 181 insertions(+), 27 deletions(-) diff --git a/src/sync/jobs/character_contracts.rs b/src/sync/jobs/character_contracts.rs index 6b006412..558f7216 100644 --- a/src/sync/jobs/character_contracts.rs +++ b/src/sync/jobs/character_contracts.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use super::killmail_value::PriceTable; use crate::{ @@ -20,7 +20,6 @@ use crate::{ }; const AUCTION_TYPE: &str = "auction"; -const STRUCTURE_ID_FLOOR: i64 = 1_000_000_000_000; pub async fn run(ctx: &JobCtx<'_>) -> Result { let Subject::Character(character_id) = ctx.key.subject else { @@ -55,7 +54,7 @@ pub async fn run(ctx: &JobCtx<'_>) -> Result { .flat_map(|contract| contract.start_location_id.into_iter().chain(contract.end_location_id)) .filter(|&id| id != 0) .collect(); - resolve_locations(ctx, &location_ids).await; + structure_resolution::resolve_location_ids(ctx, &location_ids).await; let prices = PriceTable::from_market_prices(&finance::market_prices_all(ctx.db).await?); let client = ctx.esi.character_authenticated(grant); @@ -103,28 +102,6 @@ async fn persist_children( Ok(()) } -async fn resolve_locations(ctx: &JobCtx<'_>, location_ids: &[i64]) { - let mut seen = HashSet::new(); - let mut stations = Vec::new(); - let mut structures = Vec::new(); - for &location_id in location_ids { - if !seen.insert(location_id) { - continue; - } - if location_id >= STRUCTURE_ID_FLOOR { - structures.push(location_id); - } else { - stations.push(location_id); - } - } - - // A player structure the character cannot dock at stays unresolved and is rendered as a raw - // `Structure {id}` label at modal-load time, so an inaccessible structure must not fail the job. - if let Err(error) = structure_resolution::resolve_asset_references(ctx, &[], &stations, &structures).await { - tracing::warn!("character contracts: location resolution failed: {error}"); - } -} - fn bid_rows(character_id: i64, contract_id: i64, bids: &[ContractBid]) -> Vec { bids .iter() diff --git a/src/sync/jobs/character_wallet.rs b/src/sync/jobs/character_wallet.rs index ea00f184..ce0cc602 100644 --- a/src/sync/jobs/character_wallet.rs +++ b/src/sync/jobs/character_wallet.rs @@ -4,7 +4,7 @@ use crate::{ model::{CharacterWalletJournal, CharacterWalletTransaction}, repo::{character, finance}, }, - sync::{job::JobCtx, outcome::Outcome, subject::Subject}, + sync::{job::JobCtx, outcome::Outcome, structure_resolution, subject::Subject}, }; pub async fn run(ctx: &JobCtx<'_>) -> Result { @@ -36,6 +36,13 @@ pub async fn run(ctx: &JobCtx<'_>) -> Result { .map(|transaction| CharacterWalletTransaction::from((character_id, transaction))) .collect(); finance::append_wallet_transaction(ctx.db, &transactions).await?; + + let location_ids: Vec = transactions + .iter() + .map(CharacterWalletTransaction::location_id) + .collect(); + structure_resolution::resolve_location_ids(ctx, &location_ids).await; + Ok(Outcome::from_rows(journal.len() + transactions.len())) } @@ -119,6 +126,19 @@ mod tests { .await; } + async fn mount_transactions_at(server: &MockServer, character_id: i64, location_id: i64) { + mount_json( + server, + &format!("/characters/{character_id}/wallet/transactions/"), + serde_json::json!([ + { "client_id": 1000035, "date": "2026-05-30T12:00:00Z", "is_buy": true, "is_personal": true, + "journal_ref_id": 123456789_i64, "location_id": location_id, "quantity": 10, + "transaction_id": 987654321_i64, "type_id": 34, "unit_price": 5.5 }, + ]), + ) + .await; + } + async fn mount_transfer_leg(server: &MockServer, character_id: i64, journal_id: i64, amount: f64) { mount_paginated( server, @@ -158,6 +178,131 @@ mod tests { mod run { use super::*; + use crate::{ + clients::esi::scopes, + store::{ + model::{Constellation, OwnerType, Region, SolarSystem}, + repo::sde, + }, + }; + + const CONSTELLATION_ID: i64 = 20_000_020; + const OWNER_CORP_ID: i64 = 90_000_001; + const REGION_ID: i64 = 10_000_002; + const STRUCTURE_ID: i64 = 1_051_885_479_017; + const SYSTEM_ID: i64 = 30_000_142; + + async fn seed_geography(db: &store::Database) { + sde::upsert_region( + db, + &Region { + description: None, + id: REGION_ID, + name: "The Forge".to_owned(), + }, + ) + .await + .unwrap(); + sde::upsert_constellation( + db, + &Constellation { + id: CONSTELLATION_ID, + name: "Kimotoro".to_owned(), + position_x: 0.0, + position_y: 0.0, + position_z: 0.0, + region_id: REGION_ID, + }, + ) + .await + .unwrap(); + sde::upsert_solar_system( + db, + &SolarSystem { + constellation_id: CONSTELLATION_ID, + id: SYSTEM_ID, + name: "Jita".to_owned(), + position_x: 0.0, + position_y: 0.0, + position_z: 0.0, + security_class: None, + security_status: 0.9, + star_id: None, + }, + ) + .await + .unwrap(); + } + + fn structure_grant(character_id: i64) -> Grant { + Grant::new_test_with_scopes("token", character_id, vec![scopes::UNIVERSE_STRUCTURES.to_owned()]) + } + + #[tokio::test] + async fn it_resolves_a_structure_a_transaction_traded_in() { + let server = MockServer::start().await; + mount_journal(&server, 42).await; + mount_transactions_at(&server, 42, STRUCTURE_ID).await; + Mock::given(method("GET")) + .and(path(format!("/universe/structures/{STRUCTURE_ID}/"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "name": "A Player Structure", + "owner_id": OWNER_CORP_ID, + "solar_system_id": SYSTEM_ID, + }))) + .mount(&server) + .await; + let db = store::open_test().await.unwrap(); + seed_character(&db, 42).await; + seed_geography(&db).await; + let http = http::Client::builder(http::Cache::new(db.clone())).build(); + let esi = esi::Client::with_base_url(http.clone(), server.uri()); + let image = eve_image::Client::with_base_url(http, server.uri()); + let images_dir = tempfile::tempdir().unwrap(); + let image_store = images::Store::new(images_dir.path().to_path_buf()); + let grant = structure_grant(42); + let ctx = ctx_with_grant(&db, &esi, &image, &image_store, &grant, 42); + + run(&ctx).await.unwrap(); + + let structure = sde::get_structure(&db, STRUCTURE_ID).await.unwrap(); + assert_eq!( + structure.map(|row| row.name().clone()), + Some("A Player Structure".to_owned()) + ); + } + + #[tokio::test] + async fn it_marks_a_structure_it_cannot_reach_inaccessible() { + let server = MockServer::start().await; + mount_journal(&server, 42).await; + mount_transactions_at(&server, 42, STRUCTURE_ID).await; + Mock::given(method("GET")) + .and(path(format!("/universe/structures/{STRUCTURE_ID}/"))) + .respond_with(ResponseTemplate::new(403)) + .mount(&server) + .await; + let db = store::open_test().await.unwrap(); + seed_character(&db, 42).await; + let http = http::Client::builder(http::Cache::new(db.clone())).build(); + let esi = esi::Client::with_base_url(http.clone(), server.uri()); + let image = eve_image::Client::with_base_url(http, server.uri()); + let images_dir = tempfile::tempdir().unwrap(); + let image_store = images::Store::new(images_dir.path().to_path_buf()); + let grant = structure_grant(42); + let ctx = ctx_with_grant(&db, &esi, &image, &image_store, &grant, 42); + + run(&ctx).await.unwrap(); + + assert!(sde::get_structure(&db, STRUCTURE_ID).await.unwrap().is_none()); + assert!( + sde::is_structure_inaccessible(&db, 42, OwnerType::Character, STRUCTURE_ID) + .await + .unwrap(), + "a 403 records the structure inaccessible instead of retrying it every sync" + ); + assert_eq!(finance::wallet_transactions(&db, 42).await.unwrap().len(), 1); + } #[tokio::test] async fn it_errors_and_persists_nothing_when_the_journal_fetch_fails() { diff --git a/src/sync/jobs/corporation_wallet.rs b/src/sync/jobs/corporation_wallet.rs index 252a868f..143d3ef2 100644 --- a/src/sync/jobs/corporation_wallet.rs +++ b/src/sync/jobs/corporation_wallet.rs @@ -6,7 +6,7 @@ use crate::{ model::{CorporationWalletDivision, CorporationWalletJournal, CorporationWalletTransaction, OwnerType}, repo::{finance, infra, org}, }, - sync::{job::JobCtx, outcome::Outcome, subject::Subject}, + sync::{job::JobCtx, outcome::Outcome, structure_resolution, subject::Subject}, }; const ACCOUNTING_ROLES: &[&str] = &["Director", "Accountant", "Junior_Accountant"]; @@ -93,6 +93,13 @@ async fn sync_division( .map(|transaction| CorporationWalletTransaction::from((corporation_id, division_key, transaction))) .collect(); finance::append_corporation_wallet_transaction(ctx.db, &transactions).await?; + + let location_ids: Vec = transactions + .iter() + .map(CorporationWalletTransaction::location_id) + .collect(); + structure_resolution::resolve_location_ids(ctx, &location_ids).await; + Ok(journal.len() + transactions.len()) } diff --git a/src/sync/structure_resolution.rs b/src/sync/structure_resolution.rs index 5f7db239..376d29ea 100644 --- a/src/sync/structure_resolution.rs +++ b/src/sync/structure_resolution.rs @@ -61,6 +61,31 @@ pub async fn resolve_stockpile_location( } } +/// Splits raw location ids into stations and structures on the structure-id floor and resolves +/// both, so a sync that only stores a `location_id` still earns a name for it. +/// +/// Best-effort by design: a structure the subject cannot dock at is recorded in +/// `inaccessible_structures` and costs nothing but a name, so it must never fail the calling job. +pub async fn resolve_location_ids(ctx: &JobCtx<'_>, location_ids: &[i64]) { + let mut seen = HashSet::new(); + let mut stations = Vec::new(); + let mut structures = Vec::new(); + for &location_id in location_ids { + if location_id == 0 || !seen.insert(location_id) { + continue; + } + if location_id >= STRUCTURE_ID_FLOOR { + structures.push(location_id); + } else { + stations.push(location_id); + } + } + + if let Err(error) = resolve_asset_references(ctx, &[], &stations, &structures).await { + tracing::warn!(job = ?ctx.key.kind, "location resolution failed: {error}"); + } +} + pub async fn resolve_asset_references( ctx: &JobCtx<'_>, type_ids: &[i64], -- knotmirror 0.0.1