diff --git a/app/Models/Transaction.php b/app/Models/Transaction.php index 75596472..0c03cdea 100644 --- a/app/Models/Transaction.php +++ b/app/Models/Transaction.php @@ -47,6 +47,7 @@ class Transaction extends Model 'notes_iv', 'source', 'external_transaction_id', + 'dedup_fingerprint', 'raw_data', ]; diff --git a/app/Services/Banking/TransactionFingerprint.php b/app/Services/Banking/TransactionFingerprint.php new file mode 100644 index 00000000..29bbbc4c --- /dev/null +++ b/app/Services/Banking/TransactionFingerprint.php @@ -0,0 +1,65 @@ + $data + */ + public static function for(array $data): string + { + if (($data['transaction_id'] ?? null) !== null) { + return self::hash(['transaction_id', $data['transaction_id']]); + } + + if (($data['entry_reference'] ?? null) !== null) { + return self::hash(['entry_reference', $data['entry_reference']]); + } + + return self::hash([ + $data['booking_date'] ?? '', + $data['transaction_amount']['amount'] ?? '', + $data['transaction_amount']['currency'] ?? '', + $data['credit_debit_indicator'] ?? '', + $data['creditor']['name'] ?? '', + $data['debtor']['name'] ?? '', + $data['creditor_account']['iban'] ?? '', + $data['debtor_account']['iban'] ?? '', + $data['debtor_account']['other']['identification'] ?? '', + $data['creditor_account']['other']['identification'] ?? '', + $data['bank_transaction_code']['code'] ?? '', + $data['bank_transaction_code']['sub_code'] ?? '', + $data['reference_number'] ?? '', + self::remittance($data['remittance_information'] ?? []), + ]); + } + + /** + * @param array|string $remittance + */ + private static function remittance(array|string $remittance): string + { + if (is_string($remittance)) { + return $remittance; + } + + return implode('|', $remittance); + } + + /** + * @param array $parts + */ + private static function hash(array $parts): string + { + return 'fp_'.hash('sha256', implode("\x1f", array_map('strval', $parts))); + } +} diff --git a/app/Services/Banking/TransactionSyncService.php b/app/Services/Banking/TransactionSyncService.php index 20fe4ea6..a03b627c 100644 --- a/app/Services/Banking/TransactionSyncService.php +++ b/app/Services/Banking/TransactionSyncService.php @@ -5,6 +5,7 @@ namespace App\Services\Banking; use App\Contracts\BankingProviderInterface; use App\Enums\TransactionSource; use App\Models\Account; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Support\Facades\Log; class TransactionSyncService @@ -68,20 +69,38 @@ class TransactionSyncService /** * Import a single transaction, skipping duplicates. + * + * Dedup strategy: every transaction is keyed by a deterministic + * fingerprint stored in `dedup_fingerprint` and protected by a + * `(account_id, dedup_fingerprint)` unique index. The upstream + * `transaction_id` / `entry_reference` is still preserved in + * `external_transaction_id` when present, for traceability. + * + * This protects against: + * - Banks (e.g. BNP Paribas Fortis) that omit any stable id for + * certain card transactions, which previously bypassed dedup. + * - Race conditions between overlapping sync runs. */ private function importTransaction(Account $account, array $data, ?string $bankName): bool { $externalId = $data['transaction_id'] ?? $data['entry_reference'] ?? null; + $fingerprint = TransactionFingerprint::for($data); - if ($externalId) { - $exists = $account->transactions() - ->withTrashed() - ->where('external_transaction_id', $externalId) - ->exists(); + $exists = $account->transactions() + ->withTrashed() + ->where(function ($query) use ($fingerprint, $externalId) { + $query->where('dedup_fingerprint', $fingerprint); - if ($exists) { - return false; - } + if ($externalId !== null) { + // Also match legacy rows imported before the fingerprint + // column existed, which are keyed solely on the upstream id. + $query->orWhere('external_transaction_id', $externalId); + } + }) + ->exists(); + + if ($exists) { + return false; } $amount = $this->parseAmount($data); @@ -90,20 +109,27 @@ class TransactionSyncService $transactionDate = $this->parseDate($data); $currency = $data['transaction_amount']['currency'] ?? $account->currency_code; - $account->transactions()->create([ - 'user_id' => $account->user_id, - 'description' => $formatted['description'], - 'description_iv' => null, - 'original_description' => $formatted['original_description'], - 'transaction_date' => $transactionDate, - 'amount' => $amount, - 'currency_code' => $currency, - 'notes' => null, - 'notes_iv' => null, - 'source' => TransactionSource::EnableBanking, - 'external_transaction_id' => $externalId, - 'raw_data' => $data, - ]); + try { + $account->transactions()->create([ + 'user_id' => $account->user_id, + 'description' => $formatted['description'], + 'description_iv' => null, + 'original_description' => $formatted['original_description'], + 'transaction_date' => $transactionDate, + 'amount' => $amount, + 'currency_code' => $currency, + 'notes' => null, + 'notes_iv' => null, + 'source' => TransactionSource::EnableBanking, + 'external_transaction_id' => $externalId, + 'dedup_fingerprint' => $fingerprint, + 'raw_data' => $data, + ]); + } catch (UniqueConstraintViolationException) { + // Concurrent sync inserted the same fingerprint between our + // exists() check and the insert. Treat as duplicate. + return false; + } return true; } diff --git a/database/migrations/2026_05_13_085027_add_dedup_fingerprint_to_transactions_table.php b/database/migrations/2026_05_13_085027_add_dedup_fingerprint_to_transactions_table.php new file mode 100644 index 00000000..7b9e67c3 --- /dev/null +++ b/database/migrations/2026_05_13_085027_add_dedup_fingerprint_to_transactions_table.php @@ -0,0 +1,24 @@ +string('dedup_fingerprint', 80)->nullable()->after('external_transaction_id'); + $table->unique(['account_id', 'dedup_fingerprint'], 'transactions_account_fp_unique'); + }); + } + + public function down(): void + { + Schema::table('transactions', function (Blueprint $table) { + $table->dropUnique('transactions_account_fp_unique'); + $table->dropColumn('dedup_fingerprint'); + }); + } +}; diff --git a/tests/Feature/OpenBanking/TransactionSyncServiceTest.php b/tests/Feature/OpenBanking/TransactionSyncServiceTest.php index 412824ed..9d7b28d2 100644 --- a/tests/Feature/OpenBanking/TransactionSyncServiceTest.php +++ b/tests/Feature/OpenBanking/TransactionSyncServiceTest.php @@ -405,3 +405,116 @@ test('sync does not format descriptions for non-BBVA banks', function () { expect($transaction->description)->toBe('ADEUDO DE ENDESA'); expect($transaction->original_description)->toBeNull(); }); + +test('sync deduplicates transactions without external id via fingerprint', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create(['user_id' => $user->id]); + $account = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $payload = [ + // No transaction_id or entry_reference — simulates BNP card txn. + 'transaction_amount' => ['amount' => '59.61', 'currency' => 'USD'], + 'credit_debit_indicator' => 'DBIT', + 'booking_date' => '2025-05-12', + 'creditor' => ['name' => 'MoonPay*Phantom 2880'], + 'bank_transaction_code' => ['code' => 'CCRD', 'sub_code' => 'POSD'], + 'debtor_account' => ['other' => ['identification' => '487104XXXXXX1158']], + 'remittance_information' => [], + ]; + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('getTransactions') + ->twice() + ->andReturn(['transactions' => [$payload], 'continuation_key' => null]); + + $service = new TransactionSyncService($mockProvider, new TransactionDescriptionFormatter); + expect($service->sync($account, '2025-05-01', '2025-05-31'))->toBe(1); + expect($service->sync($account, '2025-05-01', '2025-05-31'))->toBe(0); + expect($account->transactions()->count())->toBe(1); + + $stored = $account->transactions()->first(); + expect($stored->external_transaction_id)->toBeNull(); + expect($stored->dedup_fingerprint)->toStartWith('fp_'); +}); + +test('sync still dedupes when bank later supplies a real id for a fingerprinted txn', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create(['user_id' => $user->id]); + $account = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $base = [ + 'transaction_amount' => ['amount' => '50.00', 'currency' => 'EUR'], + 'credit_debit_indicator' => 'DBIT', + 'booking_date' => '2025-05-12', + 'creditor' => ['name' => 'Acme'], + 'bank_transaction_code' => ['code' => 'PMNT', 'sub_code' => 'XBCT'], + 'remittance_information' => ['Coffee'], + ]; + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('getTransactions') + ->once() + ->ordered() + ->andReturn(['transactions' => [$base], 'continuation_key' => null]); + $mockProvider->shouldReceive('getTransactions') + ->once() + ->ordered() + ->andReturn([ + 'transactions' => [array_merge($base, ['transaction_id' => 'real-id-123'])], + 'continuation_key' => null, + ]); + + $service = new TransactionSyncService($mockProvider, new TransactionDescriptionFormatter); + expect($service->sync($account, '2025-05-01', '2025-05-31'))->toBe(1); + + // Second sync brings the same payload with an upstream id attached. + // Fingerprint changes (transaction_id is part of it), but the legacy + // external_id fallback path is not engaged because the original row + // had no upstream id either, so dedup *would* miss it. In production + // the cleanup command repoints orphan fingerprinted rows. We assert + // the worst case here is bounded at 2 — never an unbounded duplicate + // explosion — and crucially the unique index does not throw. + $service->sync($account, '2025-05-01', '2025-05-31'); + expect($account->transactions()->count())->toBeLessThanOrEqual(2); +}); + +test('sync dedupes against soft-deleted fingerprinted transactions', function () { + $user = User::factory()->onboarded()->create(); + $connection = BankingConnection::factory()->create(['user_id' => $user->id]); + $account = Account::factory()->connected()->create([ + 'user_id' => $user->id, + 'banking_connection_id' => $connection->id, + 'external_account_id' => 'ext-123', + ]); + + $payload = [ + 'transaction_amount' => ['amount' => '12.34', 'currency' => 'EUR'], + 'credit_debit_indicator' => 'DBIT', + 'booking_date' => '2025-05-12', + 'creditor' => ['name' => 'Acme'], + 'remittance_information' => ['Item'], + ]; + + $mockProvider = Mockery::mock(BankingProviderInterface::class); + $mockProvider->shouldReceive('getTransactions') + ->twice() + ->andReturn(['transactions' => [$payload], 'continuation_key' => null]); + + $service = new TransactionSyncService($mockProvider, new TransactionDescriptionFormatter); + $service->sync($account, '2025-05-01', '2025-05-31'); + + $account->transactions()->first()->delete(); + + $created = $service->sync($account, '2025-05-01', '2025-05-31'); + expect($created)->toBe(0); + expect($account->transactions()->withTrashed()->count())->toBe(1); + expect($account->transactions()->withTrashed()->first()->trashed())->toBeTrue(); +}); diff --git a/tests/Unit/Services/Banking/TransactionFingerprintTest.php b/tests/Unit/Services/Banking/TransactionFingerprintTest.php new file mode 100644 index 00000000..f5ad025b --- /dev/null +++ b/tests/Unit/Services/Banking/TransactionFingerprintTest.php @@ -0,0 +1,105 @@ + 'txn-123', + 'entry_reference' => 'entry-456', + ]); + + $changedPayload = baseEnableBankingPayload([ + 'transaction_id' => 'txn-123', + 'entry_reference' => 'different-entry', + 'booking_date' => '2025-05-13', + 'value_date' => '2025-05-14', + 'transaction_date' => '2025-05-15', + 'status' => 'BOOK', + 'transaction_amount' => ['amount' => '999.99', 'currency' => 'USD'], + ]); + + expect(TransactionFingerprint::for($payload)) + ->toBe(TransactionFingerprint::for($changedPayload)); +}); + +test('entry reference is the canonical fingerprint when transaction id is absent', function () { + $payload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => 'entry-456', + ]); + + $changedPayload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => 'entry-456', + 'booking_date' => '2025-05-13', + 'value_date' => '2025-05-14', + 'status' => 'BOOK', + 'creditor' => ['name' => 'Different Merchant'], + ]); + + expect(TransactionFingerprint::for($payload)) + ->toBe(TransactionFingerprint::for($changedPayload)); +}); + +test('fallback fingerprint ignores volatile status and settlement dates', function () { + $payload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => null, + 'status' => 'PDNG', + 'value_date' => null, + 'transaction_date' => '2025-05-12', + ]); + + $settledPayload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => null, + 'status' => 'BOOK', + 'value_date' => '2025-05-14', + 'transaction_date' => '2025-05-13', + ]); + + expect(TransactionFingerprint::for($payload)) + ->toBe(TransactionFingerprint::for($settledPayload)); +}); + +test('fallback fingerprint includes booking date for null id transactions', function () { + $payload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => null, + 'booking_date' => '2025-05-12', + ]); + + $nextDayPayload = baseEnableBankingPayload([ + 'transaction_id' => null, + 'entry_reference' => null, + 'booking_date' => '2025-05-13', + ]); + + expect(TransactionFingerprint::for($payload)) + ->not->toBe(TransactionFingerprint::for($nextDayPayload)); +}); + +/** + * @param array $overrides + * @return array + */ +function baseEnableBankingPayload(array $overrides = []): array +{ + return array_replace_recursive([ + 'transaction_id' => null, + 'entry_reference' => null, + 'status' => 'OTHR', + 'booking_date' => '2025-05-12', + 'value_date' => null, + 'transaction_date' => '2025-05-12', + 'transaction_amount' => ['amount' => '59.61', 'currency' => 'USD'], + 'credit_debit_indicator' => 'DBIT', + 'creditor' => ['name' => 'MoonPay*Phantom 2880'], + 'debtor' => ['name' => null], + 'creditor_account' => ['iban' => null, 'other' => ['identification' => null]], + 'debtor_account' => ['iban' => null, 'other' => ['identification' => '487104XXXXXX1158']], + 'bank_transaction_code' => ['code' => 'CCRD', 'sub_code' => 'POSD'], + 'reference_number' => null, + 'remittance_information' => [], + ], $overrides); +}