Skip to content

Commit a04b333

Browse files
committed
fix: fix code smell
1 parent f93a0a2 commit a04b333

3 files changed

Lines changed: 80 additions & 24 deletions

File tree

README.md

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,52 @@ Built for the Freightways take-home exercise.
88

99
```mermaid
1010
flowchart LR
11-
API["Scan Event API"]
12-
DB[("SQL Server\n(ParcelSummary)")]
13-
STATE[("ProcessingState\ntable")]
14-
DLQ["SQS Dead Letter Queue\n(manual redrive)"]
15-
16-
subgraph Worker Service
17-
POLLER["ApiPollerWorker\n(BackgroundService)"]
18-
PROCESSOR["EventProcessorWorker\n(BackgroundService)"]
11+
subgraph exrernal[External Systems]
12+
direction TB
13+
API@{ shape: hex, label: "Scan Event API" }
1914
end
15+
subgraph AWS
16+
direction TB
17+
subgraph sqs[SQS]
18+
direction RL
19+
DLQ@{ shape: das, label: "SQS DLQ<br/>(manual redrive)"}
20+
QUEUE@{ shape: das, label: "SQS Main Queue<br/>(visibility timeout: 30s)" }
21+
end
22+
subgraph worker[Worker in ECS/Fargate]
23+
direction LR
24+
POLLER@{ shape: rounded, label: "ApiPollerWorker" }
25+
PROCESSOR@{ shape: rounded, label: "EventProcessorWorker" }
26+
end
27+
subgraph db[SQL Server DB]
28+
direction TB
29+
PARCEL@{ shape: cyl, label: "ParcelSummary" }
30+
STATE@{ shape: cyl, label: "ProcessingState" }
31+
end
2032
21-
QUEUE[["SQS Main Queue\n(visibility timeout: 30s)"]]
33+
end
34+
exrernal ~~~ AWS
35+
sqs & worker ~~~ db
36+
37+
API -->|"GET /v1/scans/scanevents<br/>?FromEventId=&Limit="| POLLER
38+
STATE -->|"Read LastEventId<br/>on startup"| POLLER
39+
POLLER -->|"Update LastEventId<br/>after batch sent"| STATE
40+
POLLER -->|"SendMessageAsync<br/>(batch of ScanEvents)"| QUEUE
41+
QUEUE -->|"ReceiveMessageAsync<br/>(long-poll 20s)"| PROCESSOR
42+
PROCESSOR -->|"DeleteMessageAsync<br/>on success"| QUEUE
43+
PROCESSOR -->|"MERGE upsert<br/>(idempotent)"| PARCEL
44+
QUEUE -.->|"After 3 failures<br/>(maxReceiveCount)"| DLQ
45+
46+
classDef database stroke-width:2px
47+
class PARCEL,STATE database
48+
49+
classDef queue stroke-width:2px
50+
class QUEUE,DLQ queue
2251
23-
API -- "GET /v1/scans/scanevents\n?FromEventId&Limit" --> POLLER
24-
STATE -- "Read LastEventId\non startup" --> POLLER
25-
POLLER -- "SendMessageAsync\n(batch of ScanEvents)" --> QUEUE
26-
POLLER -- "Update LastEventId\nafter batch sent" --> STATE
27-
QUEUE -- "ReceiveMessageAsync\n(long-poll 20s)" --> PROCESSOR
28-
PROCESSOR -- "MERGE upsert\n(idempotent)" --> DB
29-
PROCESSOR -- "DeleteMessageAsync\non success" --> QUEUE
30-
QUEUE -- "After 3 failures\n(maxReceiveCount)" --> DLQ
52+
classDef api stroke-width:2px
53+
class API api
54+
55+
classDef worker stroke-width:1px
56+
class POLLER,PROCESSOR worker
3157
```
3258

3359
**Two decoupled `BackgroundService` workers:**
@@ -55,18 +81,37 @@ dotnet user-secrets set "ScanEventApi:BaseUrl" "https://your-api-host" --project
5581
dotnet run --project src/ScanEventWorker/ScanEventWorker.csproj
5682
```
5783

58-
---
59-
6084
## Assumptions
6185

6286
1. ~~Events are returned ordered by `EventId` ascending~~
6387
2. ~~`EventId` is monotonically increasing — querying `FromEventId=X` reliably returns all events with ID ≥ X~~
64-
3. The API returns an empty `ScanEvents` array when no more events exist (end-of-feed signal)
65-
4. Only one worker instance runs at a time (no distributed locking required)
88+
3. ~~The API returns an empty `ScanEvents` array when no more events exist (end-of-feed signal)~~
89+
- guarded: `ApiPollerWorker` checks `Count == 0` and backs off
90+
4. ~~Only one worker instance runs at a time~~
91+
- enforced: startup `Mutex` in `Program.cs` aborts a second instance
6692
5. `RunId` comes from the nested `User.RunId` field in the JSON response
93+
- guarded: `null` coalesces to `string.Empty`
6794
6. `StatusCode` may be an empty string
68-
7. `PickedUpAtUtc` and `DeliveredAtUtc` are set on the **first** occurrence of their respective event types and are never overwritten by later events of the same type
69-
8. Unknown `Type` values are stored as-is in `ParcelSummary` without setting pickup/delivery timestamps
95+
- guarded: `null` coalesces to `string.Empty`
96+
7. ~~`PickedUpAtUtc` and `DeliveredAtUtc` are set on the **first** occurrence of their respective event types and are never overwritten~~
97+
- enforced: `??=` in `ParcelSummary` + `IS NULL` in SQL MERGE
98+
8. ~~Unknown `Type` values are stored as-is in `ParcelSummary` without setting pickup/delivery timestamps~~
99+
- enforced: `switch` default + SQL `CASE`
100+
9. `CreatedDateTimeUtc` is the timestamp recorded as `PickedUpAtUtc`/`DeliveredAtUtc`
101+
- the spec sample contains a `CreatedDateTimeUtc` field on each event
102+
- no separate pickup/delivery timestamp field is defined
103+
10. First PICKUP/DELIVERY occurrence sets the timestamp permanently
104+
- the spec is silent on correction or reversal events
105+
- enforced: `??=` in `ParcelSummary` + `IS NULL` guard in SQL MERGE
106+
11. `Device` fields (`DeviceId`, `DeviceType`) and `User.UserId`/`User.CarrierId` from the API response are not persisted
107+
- the spec only defines `ParcelSummary` columns
108+
- extra fields are silently ignored
109+
12. `STATUS` events update the most-recent-event columns (`LatestType`, `LatestStatusCode`, etc.) but do not affect `PickedUpAtUtc`/`DeliveredAtUtc`
110+
13. Polling uses `FromEventId=LastEventId` (inclusive) rather than `LastEventId+1`
111+
- the last processed event is re-fetched on every cycle to avoid missing events if IDs have gaps
112+
- the idempotent MERGE absorbs the duplicate without side effects
113+
14. The event feed starts at `EventId=1`
114+
- `ProcessingState` is seeded with `LastEventId=1` on first run d
70115

71116
## Potential Improvements
72117

src/ScanEventWorker/Infrastructure/ApiClient/ScanEventApiClient.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public static Result<ScanEvent> MapToDomain(ScanEventDto dto)
6767
: Result<ScanEvent>.Success(new ScanEvent(
6868
new EventId(dto.EventId),
6969
new ParcelId(dto.ParcelId),
70-
dto.Type,
70+
dto.Type.ToUpperInvariant(),
7171
dto.CreatedDateTimeUtc,
7272
dto.StatusCode ?? string.Empty,
7373
dto.User?.RunId ?? string.Empty));

tests/ScanEventWorker.Tests/Infrastructure/ScanEventApiClientTests.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,15 @@ public void MapToDomain_UnknownType_StillSucceeds()
9999
Assert.True(result.IsSuccess);
100100
Assert.Equal("UNKNOWN_TYPE", result.Value.Type);
101101
}
102+
103+
[Fact]
104+
public void MapToDomain_LowercaseType_NormalisedToUppercase()
105+
{
106+
var dto = new ScanEventDto { EventId = 1, ParcelId = 1, Type = "pickup", User = null };
107+
108+
Result<ScanEvent> result = ScanEventApiClient.MapToDomain(dto);
109+
110+
Assert.True(result.IsSuccess);
111+
Assert.Equal("PICKUP", result.Value.Type);
112+
}
102113
}

0 commit comments

Comments
 (0)