Phase 4 Implementation #6
Conversation
- Added Hub struct to manage WebSocket connections for customers. - Implemented NotifyAdmitted and BroadcastPosition methods for sending messages. - Created ServeWS method to handle WebSocket connections and disconnections. feat(api): add authentication middleware for organizers and customers - Implemented OrganizerAuthMiddleware and CustomerAuthMiddleware to validate JWTs. - Added bearerToken function to extract tokens from Authorization header. - Introduced OTPStore for managing one-time passwords with Redis. feat(api): set up HTTP server with routing and Swagger documentation - Created NewRouter function to wire routes with authentication middleware. - Defined public and protected routes for organizers and customers. - Integrated Swagger UI for API documentation. fix(auth): refactor password hash decoding for clarity - Updated decodeHash function to return parameters and handle errors more cleanly. - Simplified parameter handling and ensured proper error messages. fix(service): adjust Create method signature for event creation - Changed Create method to accept pointer to CreateEventRequest for better handling.
- Introduced swagger.yaml file to define API endpoints, request/response schemas, and error handling. - Added definitions for authentication, event management, claims, payments, and queue operations. chore: update go.mod dependencies - Added `github.com/swaggo/swag` for Swagger documentation generation. - Updated `golang.org/x/sync` dependency. fix: validate UUIDs for event and claim IDs in handlers - Implemented UUID validation for eventId and claimId in relevant API handlers to ensure proper input format. feat: implement database migration system - Created a migration package to handle SQL migrations embedded in the binary. - Added initial schema migration files for organizers, customers, events, claims, queue entries, and payments. refactor: streamline migration handling in tests - Replaced manual migration execution with the new migration package in test helpers. refactor: update admission worker to use notifier interface - Modified AdmissionWorker to use a Notifier interface for notifying admitted customers, enhancing decoupling.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (30)
📝 WalkthroughWalkthroughThis PR introduces FairQueue's API server, infrastructure, and complete application wiring. It adds HTTP endpoint handlers for authentication, event management, queue operations, and payment flows; WebSocket support for real-time queue updates; authentication middleware and tokenizers; Docker containerization; Swagger/OpenAPI documentation; database migrations with schema updates; background worker integration with Redis notification; end-to-end tests; and supporting infrastructure files (Makefile, docker-compose, .env.example). Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Handler
participant Service as Services
participant Store as Redis/Postgres
participant Worker as Background Worker
participant Hub as WebSocket Hub
Client->>API: POST /auth/customer/otp/request
API->>Service: Customer lookup/creation
Service->>Store: Save customer, generate OTP
Store-->>Service: OTP stored (10min TTL)
Service-->>API: Success
API-->>Client: OTP sent
Note over Client,Hub: Customer submits OTP
Client->>API: POST /auth/customer/otp/verify
API->>Store: Verify OTP from Redis
Store-->>API: Valid OTP
API->>Service: Generate JWT token
Service-->>API: Customer auth token
API-->>Client: Token + CustomerID
Note over Client,Hub: Customer joins queue
Client->>API: POST /events/{eventId}/queue
API->>Service: Queue service join
Service->>Store: Add to queue ZSET (Redis)
Store-->>Service: Position confirmed
Service-->>API: Queue position
API-->>Client: Position + QueueEntryID
Note over Client,Hub: Background admission processing
Worker->>Store: Batch load waiting customers
Worker->>Service: Generate admission tokens
Service->>Store: Add to admitted ZSET (Redis)
Worker->>Hub: NotifyAdmitted(customerID, token)
Hub->>Client: WebSocket: admitted message
Client-->>Hub: Receives token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4b6e47e1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // for customers who missed the WebSocket push or reconnected. | ||
| if position == 0 { | ||
| resp.Status = "ADMITTED" | ||
| token, err := h.custTokens.GenerateAdmission(customerID, chi.URLParam(r, "eventId")) |
There was a problem hiding this comment.
When GetQueuePosition sees position == 0, it generates a brand-new admission token on every poll, which resets the token expiry from "now" each time. Because ClaimService.Claim validates token signature/expiry/event but not current admitted status, an admitted customer can keep polling to obtain a token that remains valid after their admission window has expired, then still claim later. This breaks the admission TTL/fairness boundary and should be fixed by avoiding token reissuance here or by enforcing admission-state checks at claim time.
Useful? React with 👍 / 👎.
| // @Failure 500 {object} ErrorResponse | ||
| // @Router /events/{eventId}/claims [post] | ||
| func (h *Handlers) ClaimTicket(w http.ResponseWriter, r *http.Request) { | ||
| _, ok := customerIDFromCtx(r.Context()) |
There was a problem hiding this comment.
This handler only checks that some customer is authenticated, then ignores that ID and accepts whatever admission token is provided. Since claim creation uses the customer embedded in the admission token, any authenticated user who gets another user's token can submit it to consume inventory and create a claim for that other user (a practical denial-of-service path). The endpoint should reject claims unless the authenticated customer matches the token subject.
Useful? React with 👍 / 👎.
| // ── API ─────────────────────────────────────────────────── | ||
| otpStore := api.NewOTPStore(redisClient) | ||
|
|
||
| handlers := api.NewHandlers( |
There was a problem hiding this comment.
HealthCheck only pings backing services when h.db/h.rdb are set, but this constructor call never wires those fields via WithHealthDeps. As a result, /health will always report healthy/degraded based on default true flags rather than real connectivity after startup failures, which can mislead readiness/liveness monitoring.
Useful? React with 👍 / 👎.
| defer conn.CloseNow() //nolint:errcheck // error check not necessary here | ||
|
|
||
| h.register(customerID, conn) | ||
| defer h.remove(customerID) |
There was a problem hiding this comment.
Each websocket handler defers remove(customerID), but the hub map stores only one entry per customer and register overwrites existing connections. If a second connection is opened before the first closes, the first handler's deferred remove deletes the newer connection's map entry, causing later notifications to be dropped even though a socket is still active. Cleanup should be conditional on the same connection instance (or prior connections should be explicitly closed).
Useful? React with 👍 / 👎.
1 participant
Phase 4: API Layer + E2E Tests
Implements the full HTTP API layer for FairQueue — handlers, middleware, WebSocket hub, auth flows, and Swagger docs — plus a complete E2E test suite that exercises the system through real HTTP against a running server.
What this PR adds
HTTP Handlers (
internal/api/handlers.go)All 16 endpoints implemented as plain chi handlers with swag annotations. Each handler has one job: decode input, call a service, encode output. No business logic. UUID path parameters are validated before reaching the store layer — malformed IDs return 400 rather than surfacing a Postgres syntax error as a 500.
Auth Middleware (
internal/api/middleware.go)Two middleware functions, one per identity type.
OrganizerAuthMiddlewareverifies the organizer JWT and injects the organizer ID into context.CustomerAuthMiddlewaredoes the same for customer JWTs. Context keys are typed (contextKeystring alias) to prevent collisions. Middleware failures return 401 silently — no logging — so credential errors don't fill logs.OTP Store (
internal/api/middleware.go)Redis-backed 6-digit OTP with a 10-minute TTL. Codes are cryptographically random (
crypto/rand), single-use (deleted on successful verification), and stored asotp:{email}keys. Email send is stubbed with a log line — wired to a provider in Phase 5.Auth Tokenizers (
internal/auth/auth_additions.go)Two new tokenizers alongside the existing admission
Tokenizer.OrganizerTokenizerissues and verifies long-lived organizer session JWTs.CustomerTokenizerissues customer session JWTs and delegates admission token generation to the existingTokenizer. All three share the same HMAC-SHA256 signing implementation via package-private helpers. Password hashing uses argon2id (OWASP-recommended minimums: 64MB memory, 3 iterations, 2 parallelism).WebSocket Hub (
internal/api/hub.go)In-memory connection registry keyed by customer ID. The admission worker calls
NotifyAdmittedto push tokens to connected customers. If the customer is not connected, the token is retrievable viaGET /events/{id}/queue/position— the handler regenerates it on demand for customers who missed the WebSocket push or reconnected. The hub satisfies theworker.Notifierinterface, keeping the worker layer independent of the API layer.Router (
internal/api/server.go)chi router with three route groups: public (no auth), organizer-protected, customer-protected. WebSocket registered separately because token auth comes from a query parameter, not an Authorization header. Consistent JSON 404/405 responses. Swagger UI served at
/swagger/index.html.EventService (
internal/service/event.go)New service for event lifecycle.
Createconverts NGN to kobo at the service boundary — the API layer passes NGN, the store layer never sees it.ActivateandEndenforce organizer ownership before calling domain state machine methods.loadAndAuthorizeis the shared ownership check.ClaimService.Release (
internal/service/claims.go)New method for explicit claim release. Marks RELEASED in Postgres first, then increments Redis inventory. Same ordering rule as everywhere else — if Redis write fails, reconciliation heals within 30 seconds.
Swagger docs (
docs/)Full OpenAPI 3.0 spec covering all 16 endpoints with request/response schemas, security schemes, and error codes mapped to domain errors. Swagger UI generated via swag and served at runtime.
Key design decisions
GetPositionchecks the admitted ZSET explicitly. The original implementation only calledZRANKon the waiting ZSET. After admission, the customer moves to the admitted ZSET —ZRANKon waiting returns -1, the code fell back to Postgres, Postgres returned a row number like1, and the handler reported WAITING. Fix: after a waiting-ZSET miss, callIsAdmittedon the admitted ZSET before falling back to Postgres. Return0immediately if admitted —0is the handler's sentinel for ADMITTED status.Admission token generated on demand at position poll. Customers who miss the WebSocket push (browser closed, reconnected) can still retrieve their admission token by polling
GET /events/{id}/queue/position. The handler callscustTokenizer.GenerateAdmissionwhen status is ADMITTED. No separate storage needed — the token is deterministic given the customer ID, event ID, and TTL.Webhook raw body read before any decoding.
PaystackWebhookcallsio.ReadAll(r.Body)before any JSON parsing. HMAC-SHA512 signature verification requires the exact bytes Paystack sent. This is the only handler that reads the body manually; everything else usesjson.NewDecoder.Payment reference captured from
Initializeresponse, not assumed.PaymentService.Initializegenerates its own reference internally. Callers must use the reference returned in the response — not their own value — when constructing webhook payloads. This was a non-obvious contract surfaced by the E2E tests.E2E tests (
internal/e2e/)All tests run against a real HTTP server started in
TestMainon a random port, backed by real Postgres and Redis containers. Workers are called directly (Run) rather than via a scheduler — deterministic, no sleeps, no timing dependencies.Flow 1 — Happy path: organizer login → create event → activate → customer OTP → join queue → admission worker → poll position (ADMITTED + token) → claim → initialize payment → webhook (charge.success) → verify payment CONFIRMED and claim CONFIRMED in Postgres.
Flow 2 — Sold out: 1-ticket event, 2 customers both admitted. Customer 1 claims → event transitions to SOLD_OUT. Customer 2 claims → 410 Gone.
Flow 3 — Claim expiry: customer claims, claim backdated past TTL, expiry worker runs, inventory restored to original count.
Flow 4 — Failed payment: customer claims, payment initialized, webhook (charge.failed), claim RELEASED, inventory restored.
Flow 5 — Auth: wrong password → 401, wrong OTP → 401, no token → 401, customer token on organizer route → 401.
Flow 6 — Queue constraints: duplicate join → 409, join inactive event → 404, abandon WAITING entry → 204 (can rejoin), end-to-end queue lifecycle.
Flow 7 — Ownership: organizer 2 cannot end organizer 1's event → 403.
Bugs surfaced by E2E tests:
GetPositionreported WAITING for admitted customers —ZRANKon the waiting ZSET returns -1 after admission; the admitted ZSET was never checked.Initializegenerates its own. Fixed by capturing the reference from theInitializePaymentresponse.== 9(a transient state) instead of== 10(the final restored state).What's next
Phase 5 — Hardening: graceful shutdown, health check wiring, email/SMS provider integration, load tests (50,000 users, 5,000 tickets), chaos testing (kill server mid-claim, assert no inconsistent state).
Summary by CodeRabbit
Release Notes
New Features
Tests