hamedandClaude Opus 4.8
aa87b4a9cb
A non-unique index on (doctor_id, slot_start) plus a count-then-insert check left a TOCTOU race: two concurrent requests could both pass isSlotTaken and both insert. wrapInTransaction alone doesn't stop the phantom under InnoDB REPEATABLE-READ. Add a nullable, unique active_slot_key on Appointment = "doctorId:slotStart" while the booking occupies the slot (pending/confirmed — in lockstep with isSlotTaken); NULL once expired/completed/no_show/cancelled (NULLs don't collide in a MySQL unique index, so released slots rebook freely). bookAtomically now: catches the unique violation -> SlotTakenException, and expires lapsed pendings in-transaction so the ~1-min window before the expiry cron doesn't wrongly block rebooking. All three booking paths (online / my / admin) routed through it. Migration backfills one row per (doctor, slot) — the latest id — so the index builds even on dirty historical data without destructively cancelling bookings. (Backfill surfaced a real pre-existing double-booked slot in dev data.) Regression: tests/Appointment/SlotUniquenessTest. Adjusted the expiry-service test fixture to use distinct slots (one live booking per slot is now enforced). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
112 lines
11 KiB
Markdown
112 lines
11 KiB
Markdown
# بازبینی کامل بکاند ClinicPro (معماری/امنیت/Performance/کیفیت/DevOps) — تسکبهتسک با تست
|
|
|
|
## پروژه
|
|
|
|
`clinicpro` (Symfony 7.4 / PHP ≥8.2 / Doctrine / MariaDB؛ React 19 admin)
|
|
|
|
## زمینه
|
|
|
|
پروژه بزرگ است: **۳۸ controller، ۵۵ entity، ~۳۰ دامنه** زیر `src/`. تست تقریباً وجود ندارد (فقط `tests/Billing`، `tests/bootstrap.php`)، CI نیست (`.github/workflows` خالی)، phpstan روی level 5. اخیراً چند باگ prod-only رخ داده (نمونهها پایین) که نشان میدهد بازبینی سیستماتیک لازم است.
|
|
|
|
این یک ممیزی (audit) کامل است که **تسکبهتسک، با تست، و با اجرای ایمن** انجام میشود. نه یکجا، نه سطحی.
|
|
|
|
## هدف
|
|
|
|
Backlog کامل از تسکهای کوچک قابلتست بساز، اولویتبندی کن (Critical→High→Medium→Low)، سپس **یکییکی** هر تسک را تحلیل/رفع/تست/گزارش کن تا هیچ Critical/High باقی نماند.
|
|
|
|
## قوانین اجرا (الزامی)
|
|
|
|
1. **اول کل پروژه را اسکن کن و Backlog بساز** (با `TodoWrite` — هر تسک یک آیتم). هیچ کدی قبل از Backlog عوض نشود.
|
|
2. **فقط یک تسک In Progress** در هر لحظه. ترتیب: Critical → High → Medium → Low.
|
|
3. هر تسک کامل میشود فقط وقتی: fix اعمال شد + تست نوشته شد + تست **PASS** شد + گزارش ثبت شد.
|
|
4. برای هر باگ، **تست regression** بنویس که بدون fix fail و با fix pass شود.
|
|
5. تغییرات روی **برنچ جدا** (مثلاً `backend-audit`) — prod فعلی (main) دست نخورد. هر چند تسک، commit جدا.
|
|
6. بعد از هر تغییر API → فایل مربوط در `docs/api/` آپدیت (قانون استاندارد پروژه).
|
|
7. بعد از تغییر Entity → migration بساز (`doctrine:migrations:diff`) و اجرا کن.
|
|
|
|
## محیط و دستورات تست (همه داخل ddev)
|
|
|
|
```bash
|
|
ddev exec php bin/phpunit # کل تستها
|
|
ddev exec php bin/phpunit tests/Path/To/SomeTest.php # یک فایل
|
|
ddev exec php vendor/bin/phpstan analyse # level 5
|
|
ddev exec php bin/console doctrine:migrations:diff --no-interaction
|
|
ddev exec php bin/console doctrine:migrations:migrate --no-interaction
|
|
ddev exec php bin/console lint:container # سلامت DI
|
|
ddev exec php bin/console debug:router | grep api
|
|
```
|
|
|
|
> زیرساخت تست الان ضعیف است. اگر `tests/` پایهی کافی (WebTestCase، DB تستی، fixtures) ندارد، **اولین تسکهای High** باید زیرساخت تست را بسازند (phpunit env، DB تست، factory/fixture سبک، یک `ApiTestCase` پایه که login/JWT را هندل کند).
|
|
|
|
## وضعیت فعلی (واقعی — بررسیشده)
|
|
|
|
- دامنهها: `Admin, Appointment, Auth, Blog, Category, Clinic, ClinicInvitation, ClinicService, Config, Dashboard, Doctor, DoctorService, Insurance, Location, Patient, Payment, Rating, Representation, Secretary, Sms, Specialty, Staff, Subscription, Tag, UserProfile, Settlement, Billing`.
|
|
- همه controllerها از `BaseController` ارث میبرند؛ پاسخها `success()/paginated()/error()/validationError()`.
|
|
- خطاها: `AppException(ErrorCodes::ERR_XXX,...)` → `ExceptionSubscriber` تبدیل به `error()`. کد عمومی prod: `ERR_INTERNAL_001`.
|
|
- Auth: JWT (lexik) + OAuth `oauth/token`؛ firewall `public_endpoints` (stateless) در `config/packages/security.yaml`؛ `access_control` روی `^/api` = `IS_AUTHENTICATED_FULLY`.
|
|
- Rate limiter: `config/packages/rate_limiter.yaml` + `enforceLimit()` در `AuthController`/`PasswordAuthenticator`.
|
|
- تاریخها: Unix timestamp (نه DateTime).
|
|
- لیستهای admin: DQL `getArrayResult()` (۱۲ مورد).
|
|
- ۳۴ entity index صریح دارند، ۲۱ ندارند.
|
|
- Cache: redis (`cache.adapter.redis`). Messenger: `async` (redis) + `scheduler_default` + `failed` (doctrine).
|
|
|
|
## باگکلاسهای شناختهشده در همین کدبیس (بهعنوان seed برای Backlog)
|
|
|
|
اینها واقعیاند و باید در Backlog بهصورت تسکهای با تست regression بیایند + مشابهیاب شوند:
|
|
|
|
1. **repositoryClass گمشده** — قبلاً ۲۵ entity `#[ORM\Entity]` بدون `repositoryClass` داشتند → روی prod (opcache.preload) `getRepository()` repo پیشفرض میداد → `BadMethodCallException` روی متد custom (مثل `findAllActiveBySecretary`) → ۵۰۰. رفع شد، ولی **تست regression ندارد**. تسک: تستی که هر entity دارای custom repo را تأیید کند `getRepository()` همان custom class را برمیگرداند (data provider روی همهی entityها).
|
|
2. **double-nested response** — `$this->success(['data' => ...])` → `{data:{data:...}}`؛ frontend باید `data?.data?.data` بخواند. تسک: یافتن همهی موارد ناخواستهی nest، یکدستسازی قرارداد، تست پاسخ.
|
|
3. **migration روی DB خالی** — قبلاً چند migration (drop قبل از create، FK تکراری) روی fresh DB میترکید. تسک: تست/CI که `migrate` را روی DB کاملاً خالی اجرا کند و سبز بماند (smoke).
|
|
4. **`getArrayResult()` در لیست admin** — برای اجتناب از getter روی entity. تسک: تأیید همهی لیستهای admin این الگو را دارند و N+1 ندارند.
|
|
|
|
## نمونه ابعاد بررسی برای ساخت Backlog (هر کدام به تسکهای کوچک بشکن)
|
|
|
|
- **امنیت:** IDOR روی endpointهای دارای `{uuid}`/`{id}` (آیا ownership/scope چک میشود؟ مخصوصاً doctor/clinic/secretary scope و representation)؛ mass-assignment در bodyها؛ نشت اطلاعات در پاسخ خطا؛ صحت `public_endpoints` (هیچ endpoint حساسی اشتباهی public نباشد)؛ rate limit روی login/OTP/reset؛ اعتبارسنجی ورودی (DTO + validator) بهجای آرایهی خام؛ JWT TTL/refresh؛ بررسی SSRF در `API_IR` و callbackهای پرداخت؛ امضا/verify callback درگاه (mellat/sep).
|
|
- **Performance / Doctrine:** N+1 (join/fetch)، نبود index روی ستونهای فیلتر/FK پرتکرار (۲۱ entity بدون index صریح)، pagination درست (`paginated()` + count بهینه)، کوئریهای سنگین dashboard/admin، استفادهی درست از redis cache و invalidation.
|
|
- **API design:** status codeهای درست (۲۰۰/۲۰۱/۴۰۰/۴۰۱/۴۰۳/۴۲۲/۵۰۰)، یکدستی پاکت پاسخ، ولیدیشن ۴۲۲، نسخهبندی، هماهنگی با `docs/api/*` و مصرفکنندهها (`nobat724_front/services/*` و `clinic-pro-tauri/src/service/*`).
|
|
- **کیفیت/معماری:** controllerهای چاق (منطق به Service منتقل شود)، SOLID، تکرار، `ErrorCodes` کامل، exception handling در boundary نه پراکنده.
|
|
- **DevOps/Observability:** نبود CI → یک workflow `phpunit + phpstan + migrate-on-empty-db`؛ لاگ ساختاریافته (الان monolog نیست — تصمیم بگیر اضافه شود یا نه)؛ healthcheck (`/health` هست)؛ بدون secret در repo.
|
|
- **Database integrity:** FK/onDelete درست، unique constraintها، nullability، یکدستی timestamp.
|
|
|
|
## وظایف
|
|
|
|
### ۱. اسکن و ساخت Backlog (اولین و مهمترین خروجی)
|
|
|
|
کل `src/` را دامنهبهدامنه اسکن کن. برای هر یافته یک تسک کوچک بساز با: عنوان، فایل دقیق، دسته (security/perf/quality/api/db/devops/test)، **سطح ریسک**، و «چطور تست شود». خروجی = جدول Backlog اولویتبندیشده + `TodoWrite`.
|
|
|
|
> برای پوشش گسترده، میتوانی اسکن دامنهها را با subagentهای موازی انجام دهی (هر دامنه/بُعد یک agent، خروجی structured)، سپس نتایج را در یک Backlog واحد ادغام و dedup کن. هر یافته باید قبل از تبدیلشدن به fix، با خواندن کد واقعی تأیید (verify) شود تا false-positive نرود.
|
|
|
|
### ۲. زیرساخت تست (اگر ناکافی بود — قبل از تسکهای دیگر)
|
|
|
|
`ApiTestCase` پایه (WebTestCase + DB تست + helper برای ساخت user/JWT و login)، fixture/factory سبک، تنظیم `phpunit.dist.xml` برای env تست و DB جدا. خروجی: یک تست سبز نمونه (مثلاً `/health` و یک endpoint عمومی).
|
|
|
|
### ۳..N — اجرای تسکبهتسک
|
|
|
|
به ترتیب اولویت، برای هر تسک دقیقاً این چرخه:
|
|
|
|
```
|
|
① تحلیل کد همان تسک ② یافتن bug/smell/security/perf ③ تأیید (verify) یافته با کد واقعی
|
|
④ fix ⑤ تست (unit/functional/security + regression برای باگ) ⑥ اجرای تست تا PASS
|
|
⑦ phpstan + lint:container سبز ⑧ آپدیت docs/api اگر API عوض شد ⑨ گزارش تسک ⑩ تسک بعدی
|
|
```
|
|
|
|
## خروجی هر تسک
|
|
|
|
- عنوان · هدف · مشکلات پیداشده · سطح ریسک (Low/Medium/High/Critical) · اصلاحات · تستهای نوشتهشده · نتیجهی تست (PASS/FAIL) · جمعبندی کوتاه.
|
|
|
|
## شرط پایان + گزارش نهایی
|
|
|
|
پروژه تمام است وقتی: همهی تسکها done، هیچ Critical/High باقی نمانده، همه تستها PASS، هیچ fix بدون تست regression. گزارش نهایی:
|
|
- تعداد باگها · تعداد مشکلات امنیتی · تعداد مشکلات performance · میزان پوشش تست (واقعی، با `--coverage-text` اگر xdebug/pcov هست) · **Production readiness (%)** · لیست اولویتبندیشدهی باقیمانده.
|
|
|
|
## نکات مهم (الگوهای پروژه که باید رعایت شوند)
|
|
|
|
- همه controllerها `BaseController`؛ پاسخها فقط با `success/paginated/error/validationError`. خطاها با `AppException + ErrorCodes` (پیام فارسی).
|
|
- لیست admin: `getArrayResult()` — getter روی entity در این کوئریها استفاده نشود.
|
|
- تاریخها Unix timestamp (نه DateTime).
|
|
- تغییر Entity → migration؛ تغییر API → `docs/api/*`.
|
|
- تستها داخل ddev اجرا شوند؛ DB تست جدا از DB توسعه باشد (دادهی dev خراب نشود).
|
|
- روی **برنچ جدا** کار شود؛ هیچچیز مستقیم روی main نرود تا review نشود.
|
|
- مصرفکنندههای cross-repo (`nobat724_front`, `clinic-pro-tauri`) موقع تغییر قرارداد API لحاظ شوند — در build خطا نمیدهند.
|
|
- دامنهبودن کار: این یک تلاش بزرگ است؛ اگر یکجلسه تمام نشد، Backlog و وضعیت تسکها باید پایدار بماند (TodoWrite + commitهای متوالی) تا ادامهپذیر باشد.
|