Backend code review — kioskgaming_backend

Field	Value
Repo	kioskgaming_backend
Review date	2026-06-20
Stack	Node.js ≥18 · Express · Sequelize · PostgreSQL · Redis · RabbitMQ
Scale (estimated)	~1,004 JS files in src/ · ~162k lines · 76 models · 207 migrations · 160 test files
Related docs	Finance System · Database code review · Architecture rules: kioskgaming_backend/.cursor/rules/ARCHITECTURE_AND_DEV_RULES.md (in repo)

---

1. Executive summary

KioskGaming backend is an intentional Express monolith, gradually migrating from legacy patterns (fat controllers + global.db) to composition root + repository + biz/core/infra services. The payment domain is well tested with many timeout/retry workers; CDN hierarchy (agent/shop/super agent) and cashier module still deviate from the new architecture standard.

Criterion	Assessment
Financial business logic (wallet, settlement, withdrawal)	✅ Strong — complex logic with deep payment tests
Target architecture (layering, DI)	⚠️ Migrating — 417 baseline violations, not yet tightened
Maintainability	❌ High risk — 2 mega payment controllers ~7.5k lines
API consistency	⚠️ Response envelope / validation inconsistent across actors
Security	⚠️ Good building blocks; hardcoded secret fallback exists
Test coverage	⚠️ Payment good; CDN/cashier/controller thin

Verdict: Codebase is operational with a clear refactor direction (src/bootstrap/, cashier module, architecture CI). Short-term priorities: split mega payment controllers, ban new global.db, remove secret fallback, standardize response + transaction boundary.

---

2. Directory structure & layering

2.1 Layer map

src/
├── server.js                 # Entry, graceful shutdown
├── bootstrap/                # Composition root (database → repos → services → routes → workers)
├── controllers/              # HTTP by actor: user, admin, shop, agent, superAgent, portal*, cashier*, payment
├── routes/                   # Mount Express, middleware, validation (inconsistent)
├── services/
│   ├── biz/                  # Orchestration use-case
│   ├── core/                 # Entity / aggregate ops (should be thin)
│   ├── infra/                # Wallet, email, telegram, fraud…
│   ├── payment/              # Gateway, webhook, withdrawal pipeline
│   ├── display/              # Presentation enrich (new, thin)
│   ├── gameProviderAdapters/
│   └── queues/, ops/, outbound/
├── repositories/             # Sequelize access (45 file)
├── database/                 # connection, models, migrations, migrate.js
├── middleware/               # Multi-role auth, signing, CORS, error
├── workers/                  # Timeout, queue consumers, stats report
├── modules/cashier-management/   # Separated bounded context (ports/adapters)
├── thirdpartyapi/            # External game / payment clients
└── utils/, constants/

2.2 Standard startup flow

// src/bootstrap/index.js
async function buildAppWithRoutes() {
  const db = await initDatabase();
  const repos = createRepositories(db);
  const services = initServices(db, repos);
  const { app, authLimiter, portalAuthLimiter } = buildApp();
  mountRoutes(app, authLimiter, db, repos, services, portalAuthLimiter);
  return { app, db, repos };
}

This is the right direction: one composition root, inject dependencies via factory instead of circular requires.

2.3 Two parallel "worlds" (technical debt)

Pattern	Description	Risk
New	Bootstrap → setRepositories / setDependencies on service singleton	Partially testable
Legacy	global.db = { ...models } + setModels(db) on controller	Bypass repository, hard to mock, 122 controller-direct-db violations

System is inconsistent — new and old code coexist; reviewers easily confuse conventions when adding features.

---

3. Architecture patterns

3.1 Strengths to keep

1. Bootstrap composition root — separates database, repositories, services, routes, workers.
2. Repository pattern — class receives model via constructor (src/bootstrap/repositories.js).
3. Intentional service layering — biz orchestration, core entity, infra cross-cutting, payment gateway.
4. Cashier module — src/modules/cashier-management/ has ports/adapters, separate core/biz/repo → template for refactoring other domains.
5. Architecture CI gate — npm run architecture:check against 417-violation baseline; no new regressions allowed.
6. Dev rules in repo — .cursor/rules/ARCHITECTURE_AND_DEV_RULES.md.
7. Workers separated from request path — deposit/withdrawal timeout, queue consumers, stats report.
8. Game provider adapters — gameProviderAdapters/ + thirdpartyapi/ separate integration.
9. Migration discipline — 207 migrations; no sequelize.sync() for production schema.
10. Security primitives — Helmet, rate limit, HMAC request signing, role-based JWT secrets, log redaction.

3.2 System anti-patterns

A. Fat controller (HTTP layer contains business logic)

File	~Lines	Issue
controllers/user/paymentUserController.js	4.311	Deposit/withdraw/webhook/player payment; 35+ import; transaction in controller
controllers/admin/paymentAdminController.js	3.272	Approve/reject, fee config, cache in-memory + setInterval
controllers/user/authController.js	1.285	Auth/register/session
controllers/admin/gameWalletManagementController.js	1.226	Game wallet admin

Consequence: Hard to test, hard to review PRs, bug fixes easily regress across use-cases.

B. global.db / bypass repository

Baseline architecture-violation-baseline.json:

Rule	Count
controller-direct-db	122
repository-conditional-logic	267
service-or-biz-to-repository-import	28
Total	417

Example: modules/cashier-management/controllers/shop/cashierController.js, controllers/agent/agentShopController.js (many direct global.db.Shop queries).

C. God service / cross-layer imports

File	~Lines	Issue
services/core/depositSettlementCoreService.js	1.635	Core layer calls wallet, game, gamify, fee — crosses boundary
services/infra/emailService.js	~1.968	Multi-provider email
services/infra/telegramService.js	~1.695	Notify hub
services/payment/zeroxProcessingWebhookService.js	~902	Webhook + settlement mixed
services/biz/creditTransactionsLedgerBizService.js	~958	Ledger + many global.db

D. Duplicate / parallel implementations

Duplicate pair	Notes
services/gameWalletSyncService.js vs services/infra/gameWalletSyncService.js	Two versions ~900 lines
services/core/cashierCoreService.js vs modules/cashier-management/services/core/	Parallel cashier logic
routes/cashier.js + module routes	Two cashier entry points
Legacy src/services/balanceService.js, reconciliationService.js	Outside standard biz/core/infra

E. Service singleton + setter injection

Most export module.exports = new X() + setRepositories() / setDependencies() instead of pure constructor DI. Acceptable during migration, but hard to unit test when setter is not called before run.

---

4. Code quality

4.1 Naming & conventions

Issue	Example
Inconsistent service naming	playerServiceBiz.js vs *BizService.js
Mixed controller exports	Class vs module.exports = { fn }
Snake vs camel in API/DB	Sequelize field mapping OK; DTO sometimes mixes created_at / createdAt

4.2 Inconsistent response envelope

• User API: COMMON_SUCCESS / COMMON_ERRORS (utils/httpStatus.js)
• Admin/portal: { success: false, message } ad-hoc
• Some controllers: dozens of literal success: true/false without helper

Fix: One standard envelope per actor (or one system-wide envelope) + middleware response formatting.

4.3 Inconsistent validation

• Payment routes have centralized express-validator (routes/payment/validations.js)
• Many CDN/portal/admin routes lack equivalent validators
• Only ~7 controller files use validationResult

4.4 Error handling

• Global middleware/errorHandler.js maps Sequelize/JWT well
• Many controllers catch return own JSON, not next(err) → bypass centralized logging/mapping
• Some adapters: empty catch (e.g. game provider adapters) — swallow errors

4.5 Transaction boundary

sequelize.transaction appears in controller (payment, game wallet, USD wallet reject) instead of biz service.

Rule to apply: Controller only parses input + calls service; all transactions in service (one use-case = one transaction scope).

4.6 Narrow lint & CI scope

// package.json — lint only services biz/core/infra + architecture scripts
"lint": "eslint \"scripts/architecture/**/*.js\" \"src/services/biz/**/*.js\" ..."

Controllers, routes, workers, modules not in ESLint scope → style/bug patterns not gated.

4.7 Dependencies

• bcrypt and bcryptjs both in package.json; actual code mainly uses bcryptjs (native bcrypt may be dead weight)
• Package crypto (^1.0.1) — Node built-in, unnecessary npm dependency

---

5. Most complex files (hotspots)

#	File	Lines	Domain
1	controllers/user/paymentUserController.js	4.311	Player payment
2	controllers/admin/paymentAdminController.js	3.272	Admin payment
3	services/infra/emailService.js	~1.968	Notify
4	services/infra/telegramService.js	~1.695	Notify
5	services/core/depositSettlementCoreService.js	1.635	Settlement
6	controllers/user/authController.js	~1.285	Auth
7	controllers/admin/gameWalletManagementController.js	~1.226	Game wallet
8	services/biz/adminTransactionDetailBizService.js	~1.003	Detail aggregation
9	services/biz/creditTransactionsLedgerBizService.js	~958	Credit ledger
10	workers/pendingDepositTimeoutWorker.js	~911	Worker

Refactor should cut by vertical slice (deposit, withdrawal, webhook, admin approve…) not "read entire file then split".

---

6. Testing

6.1 Raw statistics

Metric	Value
Test files	~160 (136 unit, 13 integration)
Production JS files in src/	~1.004
Ratio of files with tests (raw)	~16%

6.2 Well tested

• Payment: withdrawal approve/reject, webhooks (ZeroX, Meld, BTCPay), deposit guard, fee math
• Wallet: balance/debit, balance conversion, reservation
• Auth/security: portal 2FA, request signing integration test
• New biz: portalLedgerDetailBizService, financeCorrectionBizService, hierarchy withdrawal

6.3 Weak / missing

Area	Gap
Mega-controllers	paymentUserController / paymentAdminController — not tested directly by file
CDN controllers	agent/shop/superAgent — almost no unit tests
Cashier module	~66 module files, ~2 test files
Controllers in general	~114 controllers, ~15 test files named *Controller*
Integration	13 files, maxWorkers: 1 — suitable for serial DB but thin vs surface area

6.4 Test strategy recommendations

1. Do not write tests for 4k-line controller — extract biz service first, test service.
2. Each CDN "hot path" (shop credit transfer, agent adjust) → at least 1 biz service test.
3. Keep integration tests for end-to-end money movement (deposit → settlement → game wallet).
4. Ratchet: new PR touching payment must have tests for new logic.

---

7. Security

Level	Issue	Evidence
P0	Hardcoded DB password fallback	connection.js: password: process.env.DB_PASSWORD || '@Linux121314' (repeated in migrate/config)
P1	global.db in controller	Hard to audit queries; bypass repository
P1	CORS can be disabled	DISABLE_CORS / CORS_DISABLED in bootstrap
P2	JWT secret fallback chain	Many roles use separate secrets (good) but fallback JWT_SECRET if env missing
OK	Helmet, rate limit, request signing HMAC, raw body capture	bootstrap/app.js
OK	Log redaction secrets	Bootstrap redact list
OK	Multi-role auth middleware	user/admin/agent/shop/superAgent/cashier
OK	rejectUnknownBodyFields whitelist	Reduce mass assignment
Note	bcryptjs vs native bcrypt	bcryptjs slower; consider standardizing native for production

P0 action: Fail fast if DB_PASSWORD missing in all non-local environments; remove fallback string from repo.

---

8. Database

Detailed PostgreSQL/Sequelize assessment (schema, migrations, partitioning, idempotency, integrity) is in a separate doc:

→ Database code review — kioskgaming_backend

Quick summary:

Criterion	Assessment
Schema & ledger design	✅ Strong
Migration-first + partitioning	✅ Good
Config / secrets / model drift	❌ Needs tightening (P0)
JSONB + DECIMAL consistency	⚠️ Needs governance

P0 priorities: remove password fallback, register orphan models, verify unique wallet_transactions, automate partition N+1. Details DB-1 → DB-14 in database doc.

---

9. Domain-by-domain snapshot

Domain	Architecture status	Test	Notes
Payment / withdrawal	Biz services growing but controller still huge	✅ Strong	Priority: split controller
Deposit settlement	God service depositSettlementCoreService	⚠️ Partial	Split step pipeline
USD wallet	Good infra service (usdWalletService); display enrich being added	⚠️ Unit infra	Optimistic lock + idempotency OK
Credit / master wallet	walletService, walletTransferService	✅ Good	Good reservation pattern
Game wallet / sync	Duplicate sync service	⚠️	Merge to one source
CDN hierarchy	Fat controllers + global.db	❌ Weak	Copy agent/shop pattern
Cashier	Good new module; parallel legacy routes	❌ Weak	Gradually migrate to module
Auth / portal	Good middleware; controller response inconsistent	⚠️	Standardize envelope
Notify (email/telegram/bell)	Large infra, much duplication	⚠️	Split template + i18n later
Workers	Clear file separation	⚠️	Test worker logic separately
Third-party API	Adapter per provider	⚠️	Empty catch needs audit

---

10. Top 10 priority recommendations

#	Priority	Action	Impact	Effort
1	P0	Split paymentUserController + paymentAdminController into biz services by use-case	Reduce bugs, enable test	L (2–4 weeks incremental)
2	P0	Ban new global.db in controller — ratchet baseline down	122 current violations	M
3	P0	Remove hardcoded DB password — require env	Security	S
3b	P0	Database — see Database code review (DB-1→DB-4)	Data integrity	M
4	P1	Standardize response envelope (admin/portal/CDN → common helper)	Client consistency	M
5	P1	Move sequelize.transaction from controller → service	Data integrity	M
6	P1	Merge duplicate services (gameWalletSyncService, cashier core)	Maintenance	M
7	P1	Extend ESLint + architecture check to controllers/routes/workers	Enforce layering	S
8	P2	Test pyramid CDN + cashier — biz test per hot path	Regression safety	M
9	P2	Complete constructor DI — reduce setModels / global.db	Testability	L
10	P2	Refactor depositSettlementCoreService — pipeline steps + inject deps	Payment stability	L

Size: S ≈ 0.5–1 day · M ≈ 2–5 days · L ≈ 1–4 weeks (incremental PR).

---

11. Proposed refactor roadmap

Phase 0 — Safety (1 sprint)

• P0 security (DB password)
• P0 database — see Database code review
• Document transaction boundary rule + dual-wallet invariant
• Do not increase architecture baseline

Phase 1 — Payment vertical slice (2–3 sprint)

• Extract from paymentUserController: deposit, withdrawal request, webhook handlers → *BizService
• Extract from paymentAdminController: approve, reject, refund → existing + new biz services
• Test new services; controller remains <300 LOC/file

Phase 2 — Layering ratchet (ongoing)

• Migrate CDN controllers off global.db (1 controller / PR)
• Merge duplicate sync/cashier services
• Extend lint scope

Phase 3 — Platform consistency

• Unified response + validation middleware per route group
• Constructor DI factory from bootstrap
• Settlement pipeline refactor

Architecture template: src/modules/cashier-management/ — ports, adapters, separated biz/core/repo, self-bootstrap.

---

12. PR reviewer checklist (backend)

• Controller does not import Sequelize model / global.db directly
• Transaction in biz service, not in controller
• Response via actor standard helper (no scattered literal success:)
• Input validation (express-validator or schema) for new routes
• Idempotency key for money movement
• Unit test for new logic (biz layer)
• npm run architecture:check pass (no baseline increase)
• Do not add hardcoded secret fallback
• Migration / model / partition — see checklist in Database code review

---

13. Definition of Done — health target (6 months)

• paymentUserController + paymentAdminController each < 500 LOC (delegate to biz)
• Architecture baseline < 200 violations (from 417)
• Zero hardcoded credentials in repo
• ESLint covers controllers + routes
• CDN hot paths have biz unit tests
• Single gameWalletSyncService source of truth
• Response envelope documented per API surface (admin/user/portal)
• Database health targets — see Database code review §13

---

14. Changelog

Date	Author	Notes
2026-06-20	Code review	Created doc — full backend review (not tied to specific task)
2026-06-20	Code review	Expanded §8 Database — schema, migration, partition, idempotency, recommendations DB-1→DB-14
2026-06-20	Code review	Split database into kioskgaming-backend-database-review