You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@@ -17,15 +17,15 @@ Do not introduce an `IDbConnection` factory. Accept that `ScanEventRepository` a
17
17
18
18
### The meaningful abstraction already exists
19
19
20
-
`IScanEventRepository` means every consumer —`ApiPollerWorker`, `EventProcessorWorker`, and `ScanEventProcessor`— can be fully tested with NSubstitute mocks without touching a database. The repository implementation being untestable without a DB is a much smaller problem than the workers being untestable.
20
+
`IScanEventRepository` means every consumer -`ApiPollerWorker`, `EventProcessorWorker`, and `ScanEventProcessor`- can be fully tested with NSubstitute mocks without touching a database. The repository implementation being untestable without a DB is a much smaller problem than the workers being untestable.
21
21
22
22
### Mocking IDbConnection is unsafe under Dapper.AOT
23
23
24
-
Dapper.AOT generates source-level interceptors that target specific `SqlConnection` call sites at compile time. A mock `IDbConnection` would not trigger those interceptors — tests would execute different code paths than production, making them unreliable and potentially misleading.
24
+
Dapper.AOT generates source-level interceptors that target specific `SqlConnection` call sites at compile time. A mock `IDbConnection` would not trigger those interceptors - tests would execute different code paths than production, making them unreliable and potentially misleading.
25
25
26
26
### The SQL is the logic
27
27
28
-
The interesting correctness properties of `ScanEventRepository`— the MERGE semantics, the idempotency guard, the PICKUP/DELIVERY timestamp invariant — live in the SQL strings. No amount of `IDbConnection` mocking verifies that the SQL is correct. This is genuine integration-test territory.
28
+
The interesting correctness properties of `ScanEventRepository`- the MERGE semantics, the idempotency guard, the PICKUP/DELIVERY timestamp invariant - live in the SQL strings. No amount of `IDbConnection` mocking verifies that the SQL is correct. This is genuine integration-test territory.
29
29
30
30
### Deadline and risk
31
31
@@ -41,7 +41,7 @@ Introducing a new abstraction layer less than two days before the deadline risks
41
41
42
42
## Consequences
43
43
44
-
-`ScanEventRepository` and `DatabaseInitializer` are covered by integration tests only (requiring a running SQL Server — see Docker Compose setup in README).
44
+
-`ScanEventRepository` and `DatabaseInitializer` are covered by integration tests only (requiring a running SQL Server - see Docker Compose setup in README).
45
45
- All higher-level components (`ScanEventProcessor`, workers) remain fully unit-testable via `IScanEventRepository`.
Copy file name to clipboardExpand all lines: docs/adr/002-no-saga-pattern.md
+6-6Lines changed: 6 additions & 6 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -18,16 +18,16 @@ Don't use it. The design already handles failures correctly without a coordinato
18
18
19
19
## Why
20
20
21
-
If `SendToSqs` fails, `LastEventId` doesn't advance — the next poll re-covers those events.
22
-
If `MergeToDb` fails, the message isn't deleted — SQS redelivers via visibility timeout.
21
+
If `SendToSqs` fails, `LastEventId` doesn't advance - the next poll re-covers those events.
22
+
If `MergeToDb` fails, the message isn't deleted - SQS redelivers via visibility timeout.
23
23
24
24
Both chains self-recover because the MERGE is idempotent:
25
25
26
26
```sql
27
27
incoming.EventId>stored.LatestEventId
28
28
```
29
29
30
-
Replaying the same event is a no-op. Saga exists to handle cases where replay is *not* safe. That case doesn't exist here.
30
+
Replaying the same event is a no-op. Saga exists to handle cases where replay is _not_ safe. That case doesn't exist here.
31
31
32
32
## What Saga Would Have Cost
33
33
@@ -41,12 +41,12 @@ All of that is already implicit in `LastEventId` + SQS visibility timeout. Addin
41
41
42
42
This decision is specific to the current poll-based design. In an event-driven model the calculus changes.
43
43
44
-
Each scan event arriving could naturally produce side-effects across multiple systems — a row written to DynamoDB for real-time lookup, a record appended to S3 for compliance archiving, a downstream notification, an audit trail. Each of those writes is a discrete step with its own failure mode. That's exactly the problem Saga is designed for: coordinating a sequence of steps across systems where partial failure needs explicit handling.
44
+
Each scan event arriving could naturally produce side-effects across multiple systems - a row written to DynamoDB for real-time lookup, a record appended to S3 for compliance archiving, a downstream notification, an audit trail. Each of those writes is a discrete step with its own failure mode. That's exactly the problem Saga is designed for: coordinating a sequence of steps across systems where partial failure needs explicit handling.
45
45
46
-
AWS Step Functions would carry most of the weight here — the state machine, retry policies, compensation routing, and execution history are all provided out of the box. The idempotency invariant still applies and still protects against duplicate executions, but Saga would earn its complexity cost in that design.
46
+
AWS Step Functions would carry most of the weight here - the state machine, retry policies, compensation routing, and execution history are all provided out of the box. The idempotency invariant still applies and still protects against duplicate executions, but Saga would earn its complexity cost in that design.
47
47
48
48
The current poll-based single-DB design doesn't have that fan-out problem, so Saga adds nothing today.
49
49
50
50
## Note on Test Cancellation
51
51
52
-
The `CancellationToken` complexity in the worker tests is a test harness concern — how to drive a `BackgroundService` loop one iteration at a time. It's unrelated to this decision.
52
+
The `CancellationToken` complexity in the worker tests is a test harness concern - how to drive a `BackgroundService` loop one iteration at a time. It's unrelated to this decision.
0 commit comments