fix: outbox self-invocation, optimistic locking, idempotency race - #14
Merged
Conversation
…lama eventos IN_FLIGHT órfãos Extrai claimBatch()/markPublished()/revertToPending() de OutboxRelay pra OutboxClaimCoordinator, um @component novo. OutboxRelay chamava esses métodos via this.*, o que faz o proxy AOP do Spring nunca interceptar a chamada e deixa @transactional como no-op silencioso. Agora OutboxRelay recebe o coordinator por injeção de dependência e chama através dele, passando pelo proxy de verdade. Adiciona reaper de eventos IN_FLIGHT travados: claimBatch() primeiro reclama de volta pra PENDING qualquer evento IN_FLIGHT com claimed_at mais velho que 2 minutos, antes de buscar o próximo lote PENDING. Nova coluna claimed_at (migration V4) marca quando um evento foi reivindicado.
…play correto sob concorrência na idempotência Adiciona @Version (coluna version, migration V5) em charges. ProcessWebhookService e ChargeExpirationJob podiam sobrescrever a mesma charge concorrentemente sem avisar ninguém. No webhook, o conflito agora propaga OptimisticLockingFailureException até o GlobalExceptionHandler, que responde 409 (o provedor reenvia; dedup por event_id garante que reprocessar é seguro). No job de expiração, extrai ChargeExpirationCoordinator (@transactional por charge, evitando o self-invocation que anularia a anotação) para que uma charge em conflito seja pulada nesse ciclo sem impedir as demais de serem processadas. ProcessWebhookService agora valida o status recebido antes de qualquer transição, lançando InvalidChargeStatusException (422) em vez de deixar um IllegalArgumentException cru virar 500. CreateChargeService: duas requisições concorrentes com a mesma Idempotency-Key nova faziam a perdedora receber um 500 da violação de unicidade, mesmo com o rollback já evitando duplicar dados. Extrai ChargeCreationCoordinator (@transactional) para a escrita; ao capturar DataIntegrityViolationException UMA CAMADA ACIMA da transação (depois do rollback completo do Postgres), CreateChargeService busca o registro de idempotência da vencedora numa transação nova e retorna o replay em vez do erro. Testes cobrem os três cenários; a concorrência real (duas threads com a mesma key) é provada via Testcontainers em CreateChargeServiceConcurrencyIT.
… testes ChargeJpaEntity agora implementa Persistable<UUID> com isNew() baseado em (version == null). O id da Charge é atribuído em código (UUID.randomUUID()), não pelo banco, então Spring Data não tinha como distinguir insert de update só pelo id e sempre chamava merge() — com @Version presente, merge() de uma entidade nova faz o Hibernate suspeitar que a linha foi apagada por outra transação e lança ObjectOptimisticLockingFailureException. Corrige também OutboxClaimCoordinatorTransactionalIT: removido um thenCallRealMethod() estubado num método de repository Spring Data (proxy sem corpo Java real, Mockito não consegue chamar); o @MockitoSpyBean já delega pro objeto real automaticamente em qualquer chamada não estubada. Corrige ChargeRepositoryIT: os dois casos que esperavam inserir uma charge nova usavam ChargeTestData.aCharge().build(), que via Charge.restore() monta uma charge com version=0 (representando uma linha já persistida). Isso fazia isNew() reportar corretamente false e o save() tentar um merge() numa linha inexistente. Trocado por Charge.create(...), a fábrica de domínio real usada em produção para charges novas (version=null).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OutboxRelaycalledclaimBatch()/markPublished()/revertToPending()viathis., bypassing the Spring AOP proxy —@Transactionalwas a silent no-op. ExtractedOutboxClaimCoordinator(separate bean, real proxy) and added a reaper for orphanedIN_FLIGHTevents stuck past 2 minutes (newclaimed_atcolumn, migration V4).@Versionoptimistic locking tocharges(migration V5).ChargeExpirationJobnow catches conflicts per-charge via an extractedChargeExpirationCoordinator(own transaction, so one stale charge doesn't block the batch);ProcessWebhookServicelets the conflict propagate to a newGlobalExceptionHandler409 mapping. Webhook status is now validated before any transition (InvalidChargeStatusException, 422) instead of leaking a rawIllegalArgumentException.CreateChargeServicenow handles the idempotency-key race correctly: the loser of a concurrent create gets a proper replay of the winner's result instead of a raw 500 from the unique constraint violation (extractedChargeCreationCoordinatorfor the transactional write).ChargeJpaEntitynow implementsPersistable<UUID>(isNew() = version == null) — since the Charge id is assigned in code (not DB-generated), Spring Data had no way to distinguish insert from update by id alone and always calledmerge(), which produced a falseObjectOptimisticLockingFailureExceptionon brand-new charges.java-payment-hexagonaltojava-payment-core).Test plan
ChargeRepositoryIT,OutboxClaimCoordinatorTransactionalIT,CreateChargeServiceConcurrencyIT, etc.) — pass locally with Docker; CI will confirm via the "Lint + Unit Tests" required check