Skip to content

feat: complete business improvement plan implementation - #3

Merged
DavidHLP merged 14 commits into
mainfrom
feat/ci-cd-workflow
Mar 1, 2026
Merged

feat: complete business improvement plan implementation#3
DavidHLP merged 14 commits into
mainfrom
feat/ci-cd-workflow

Conversation

@DavidHLP

@DavidHLP DavidHLP commented Mar 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Backend enhancements: Achievement triggers, admin analytics/monitoring, problem version history, submission management, sandbox monitoring, Stripe tests, Rust runner, Redis caching with TTLs
  • Console improvements: PWA offline support, offline queue indicator, code autosave, accessibility settings, keyboard shortcuts, learning progress charts, enhanced i18n
  • Management additions: Analytics dashboard, submission management, problem version history, enhanced settings

Changes by Area

Backend (/backend)

  • Achievement System: New trigger service with comprehensive tests
  • Admin APIs: Analytics, monitoring, problem versions, and submission management endpoints
  • Sandbox: Monitoring service for Docker/VM sandboxes with health metrics
  • Payments: Stripe service tests for subscription workflows
  • Judge System: Rust language runner support
  • Database: Updated Prisma schema with new indexes and models
  • Caching: Redis TTLs added to ProblemService and ContestQueryService
  • Testing: New E2E test infrastructure

Console (/console)

  • PWA/Offline: Service worker, offline queue, queue indicator component
  • Editor: Accessibility settings, code autosave, keyboard shortcuts modal
  • Dashboard: Learning progress and submission history charts
  • i18n: Achievement and shortcuts translations (EN/ZH)
  • Composables: useBreakpoints, useCodeAutosave, useGlobalShortcuts, useProblemShortcuts

Management (/management)

  • Analytics: New analytics dashboard with API integration
  • Submissions: Submission management views
  • Problems: Version history component
  • Settings: Enhanced configuration options

Test plan

  • Verify backend services start correctly
  • Run backend unit tests: cd backend && pnpm test
  • Run backend E2E tests: cd backend && pnpm test:e2e
  • Run frontend tests: cd console && pnpm test
  • Verify PWA offline functionality in console
  • Test admin analytics dashboard
  • Test problem version history in management panel

…ement plan

- Add Stripe payment service with checkout session creation
- Add webhook handler for Stripe events (checkout, subscription updates)
- Add user-facing subscription API endpoints (checkout, portal, cancel, reactivate)
- Add frontend subscription page with plan selection
- Update Prisma schema with Stripe customer/subscription fields
- Update business improvement plan to reflect actual project status
- Mark sandbox system as complete (already fully implemented)
- Mark payment integration as complete

Implementation:
- backend/src/subscription/payment/stripe.service.ts
- backend/src/subscription/payment/webhook.controller.ts
- backend/src/subscription/user-subscription.controller.ts
- console/src/views/personal/SubscriptionView.vue
- console/src/api/subscription.ts
- Add actual GitHub and Google OAuth API integration
- Add Google OAuth button to login form
- Update oauth.service.ts to call real GitHub/Google APIs
- Use mock data fallback when OAuth credentials not configured
- Update BUSINESS_IMPROVEMENT_PLAN to reflect OAuth completion
- Add default email templates (welcome, password-reset, verification, etc.)
- Add email templates seeder module
- Update BUSINESS_IMPROVEMENT_PLAN to reflect 95% backend completion

Completed in this session:
- ✅ Stripe payment integration (checkout, portal, webhooks)
- ✅ Frontend subscription page
- ✅ OAuth with actual GitHub/Google API calls
- ✅ Email templates (7 default templates)
- ✅ Updated priority matrix and implementation status
- Add VitePWA plugin with prompt-based update registration
- Configure PWA manifest with app name, icons, and theme colors
- Set up Workbox for service worker generation
- Add runtime caching for Google Fonts (1 year cache)
- Configure precaching for static assets (js, css, html, ico, png, svg, woff2)
- Disable PWA in dev mode for faster HMR
- Add maximumFileSizeToCacheInBytes (7 MB) to workbox config to handle Monaco Editor chunks
- Move eslint-disable comment to cover the as any cast on correct line
Add a Vue composable for managing PWA (Progressive Web App) state:

- Export isOfflineReady ref for offline status tracking
- Export needRefresh ref for update availability detection
- Export updateServiceWorker function to trigger updates
- Export close function to dismiss update prompts
- Use global shared state across all composable instances
- Initialize PWA on mount via setUpdateCallback

Tests cover all core functionality including state management,
function exports, callback registration, and shared state behavior.

Also update vitest.config.ts to include a virtual module plugin
that mocks 'virtual:pwa-register' during test execution.
- Import pwa-register in main.ts to enable service worker
- Add PWAUpdatePrompt component to App.vue for update notifications
- Create PWAUpdatePrompt component with usePWA composable integration
- Add PWA translations for en-US and zh-CN locales
Add submitQueue utility for offline-first code submission:
- Uses IndexedDB via idb library for persistence
- QueuedSubmission interface with id, problemId, language, code, queuedAt
- initSubmitQueue() - Initialize database
- addToQueue() - Add submission, return generated id
- getQueue() - Get all queued submissions (oldest first)
- getQueueLength() - Get count
- removeFromQueue() - Remove by id
- clearQueue() - Clear all
- processQueue() - Process each with handler, remove on success

Database schema:
- DB Name: ulticode-offline
- DB Version: 1
- Store: submission-queue
- Key: id
- Index: by-queuedAt on queuedAt

Includes comprehensive test suite with 10 tests covering all functions.
Add OfflineQueueIndicator.vue component that displays the number of queued
submissions when offline and provides auto-sync functionality when coming
back online.

Features:
- Shows queued submissions count when > 0
- Different icons for syncing, offline, and online with pending states
- Auto-sync when connection is restored
- Toast notifications on sync complete/failure
- i18n support for both en-US and zh-CN
Major enhancements across backend, console, and management:

Backend:
- Add achievement trigger service with tests
- Add admin analytics and monitoring controllers
- Add problem version history management
- Add admin submission management with service and tests
- Add sandbox monitoring service with tests
- Add Stripe payment service tests
- Enhance caching with Redis TTLs in ProblemService and ContestQueryService
- Add Rust language runner for judge system
- Update Prisma schema with new models and indexes
- Add comprehensive test directory structure

Console:
- Add PWA offline support with service worker
- Add offline queue indicator component
- Add code autosave composable
- Add accessibility settings for editor
- Add keyboard shortcuts modal and composables
- Add breakpoint utility composable
- Add learning progress and submission history charts
- Enhance i18n with achievement and shortcut translations
- Update stores for better state management

Management:
- Add analytics dashboard views and API
- Add submission management views
- Add problem version history component
- Enhance settings with new configuration options
- Update sidebar with new navigation items

Documentation:
- Remove completed BUSINESS_IMPROVEMENT_PLAN.md
- Add comprehensive docs directory

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9771a979a5

ℹ️ 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".

Comment thread backend/src/subscription/payment/webhook.controller.ts
Comment thread backend/src/subscription/user-subscription.controller.ts
Comment thread backend/src/subscription/user-subscription.controller.ts
@DavidHLP
DavidHLP merged commit d85423a into main Mar 1, 2026
14 checks passed
@DavidHLP
DavidHLP deleted the feat/ci-cd-workflow branch March 1, 2026 10:40
DavidHLP added a commit that referenced this pull request Jun 8, 2026
Three defects from docs/comments-api-test-report.md (curl test report):

#1 GET /admin/comments/{type}/{id} with invalid 'type' returned HTTP 500
   instead of 400. Root cause: `@PathVariable @Pattern` on
   `@Validated` controllers throws ConstraintViolationException which
   had no `@ExceptionHandler` in GlobalExceptionHandler, falling
   through to handleGenericException.
   Fix: add handleConstraintViolation mirroring the response shape
   of the existing handleValidationException (@Valid @RequestBody).

#2 unflagComment left flagged_reason / flagged_at stale in the row
   even though the service code called setFlaggedReason(null) /
   setFlaggedAt(null). Root cause: MyBatis-Plus
   FieldStrategy.NOT_NULL (the default) silently drops null columns
   in updateById, and the entity-level @TableField updateStrategy
   may apply similar rules. The setter was a no-op at the SQL layer.
   Fix: switch unflagComment to mapper.update(null, LambdaUpdateWrapper)
   with explicit .set(field, null) clauses.

#3 deleteComment set only the audit columns (deleted_at, deleted_by)
   but is_deleted remained 0, so GET endpoints happily returned
   already-soft-deleted comments. Same root cause as #2 plus the
   Boolean wrapper for is_deleted. Fix: switch to
   LambdaUpdateWrapper with explicit .set(IsDeleted, true).

bulkCommentAction automatically benefits since it dispatches to
the patched deleteComment / unflagComment methods.

Verification: real curl regression against ulticode-9001 (PM2,
dev profile) — three defects each return the expected 2xx/4xx and
the DB columns reflect the intended final state.

Tests: AdminCommentServiceImplTest covers 3 smoke cases (construct,
invalid-type throw, deleteComment unknown-id throw). Full mutator
SQL inspection requires `@SpringBootTest` because MyBatis-Plus
LambdaUpdateWrapper relies on a Spring-initialized lambda-method
cache (AbstractLambdaWrapper.tryInitCache) that is absent under
plain Mockito; integration coverage is recorded in
docs/comments-api-test-report.md §9.

Audit: AuditContext.setUserId / setOldValues / setNewValues calls
are preserved unchanged before each mapper.update invocation so
the @Audited behavior on the three methods is identical to before.

Out of scope: getComment / getComments queries do not yet filter
`is_deleted=1` (GET still returns 200 for a deleted row). Tracked
in a separate follow-up plan:
.claude/PRPs/plans/admin-comments-get-filter-soft-deleted.plan.md
DavidHLP added a commit that referenced this pull request Jun 8, 2026
Fixes three defects from docs/comments-api-test-report.md:
#1 GET /admin/comments/{type}/{id} invalid 'type' (500 → 400)
#2 unflagComment left flagged_reason / flagged_at stale (null 字段未被 MyBatis-Plus 持久化)
#3 deleteComment only set audit columns, is_deleted remained 0

Plus unit test scaffolding and follow-up plan for getComment soft-delete filtering.
DavidHLP added a commit that referenced this pull request Jun 8, 2026
…_ADMIN + rankings 404 + is_deleted filter + Unicode title + @Audited + new ErrorCode + controller XSS guard + mutator tests)

修复 docs/contests-api-test-report.md 报告的 7 处缺陷 (1 HIGH + 6 MEDIUM/LOW):

缺陷 #1 (HIGH) — DELETE 软删除 is_deleted 字段未持久化
  - 根因:Contest.isDeleted 字段 @TableLogic + mapper.updateById(entity) 自动忽略逻辑删除字段
  - 修复:改用 mapper.update(null, LambdaUpdateWrapper.set(...)) 显式写入

缺陷 #2 — PATCH on FINISHED/RUNNING 比赛无状态守卫
  - 修复:service 层加 status guard,仅允许 UPCOMING 修改
  - 新增 ErrorCode CONTEST_ONLY_UPDATE_UPCOMING (70006) 区分注册与修改语义

缺陷 #3 — service 层 hasRole("ADMIN") 拒绝 SUPER_ADMIN,与控制器 @PreAuthorize 不一致
  - 修复:SecurityUtil 新增 hasAnyRole(String...) 工具方法,ContestServiceImpl 7 处 mutator 替换

缺陷 #4 — CreateContestDTO/UpdateContestDTO title @pattern 拒绝 CJK 字符
  - 修复:正则放宽为 [\p{L}\p{N}\s\p{P}]+ 接受 Unicode 字母
  - AdminContestController 新增 rejectUnsafeTitleChars helper 显式排除 < 和 >

缺陷 #5 — GET /admin/contest/{id}/rankings 对不存在 contest 返 200+空
  - 修复:getAdminContestRanking 入口加 contest 存在性 + is_deleted 校验

缺陷 #6 — addProblem/removeProblem/startContest/endContest 未校验 is_deleted
  - 修复:4 个 mutator 入口加 .filter(c -> !isDeleted).orElseThrow(COMPETITION_NOT_FOUND)

缺陷 #7@Audited 注解缺失导致 audit_logs 无 contest 记录
  - 修复:7 个 mutator 方法加 @Audited + AuditContext.setOldValues/setNewValues/setUserId
  - 复用 AuditActionUtil.CREATE_CONTEST/UPDATE_CONTEST/DELETE_CONTEST 常量

新增测试 ContestServiceImplMutatorTest:5 case 覆盖缺陷 #2/#5/#6 的状态守卫 + 软删除过滤 + 404 路径。
缺陷 #1 的 LambdaUpdateWrapper.getSqlSet() 断言需 @SpringBootTest 上下文,保留为 follow-up,
运行时 curl 验证已确认三列同改 (docs/contests-api-test-report.md §3.5)。

验证:
- mvnw compile -B:0 错误
- mvnw test -Dtest=ContestServiceImplMutatorTest:5/5 通过,2.0s
- 运行时 curl 5/5 critical 缺陷修复确认 (PM2 重启 ulticode-9001)

Artifacts:
- Plan: .claude/PRPs/plans/completed/admin-contests-mutator-fixes.plan.md
- Report: .claude/PRPs/reports/admin-contests-mutator-fixes-report.md
- Review v1: .claude/reviews/admin-contests-mutator-review.md
- Review v2: .claude/reviews/admin-contests-mutator-review-v2.md
- Test report: docs/contests-api-test-report.md
DavidHLP added a commit that referenced this pull request Jun 9, 2026
…ject empty PATCH fields

Fixes4 defects in AdminTagController discovered via docs/admin-tags-test-plan.md:

- Bug #1: GET/DELETE /admin/tags/{id} without ?type= returned500. Added
 MissingServletRequestParameterException handler in GlobalExceptionHandler
 returning400 with field-level error map (mirrors handleConstraintViolation).

- Bug #2: ?type=GARBAGE silently fell back to PROBLEM. Added @validated +
 @pattern on controller query params; added @pattern on TagQueryDTO.type,
 UpdateTagDTO.type, CreateTagDTO.type. New dto.tag.TagTypes class is the
 single source of truth for the whitelist (PROBLEM|FORUM). Service-layer
 normalizeType() helper enforces the same whitelist for direct callers.

- Bug #3: PROBLEM tag list ignored sortBy. getProblemTags now dispatches on
 usageCount / createdAt / slug / label, mirroring getForumTags.

- Bug #4: PATCH with empty name silently ignored. Added @SiZe(min=1) on
 UpdateTagDTO.name and slug; @NotNull already covered type.

Adds39 regression tests (15 Mockito service +24 WebMvcTest controller) with
exact code=40000 + data.<field> assertions.

Co-Authored-By: Claude Opus4.8 <noreply@anthropic.com>
DavidHLP pushed a commit that referenced this pull request Jun 11, 2026
… handler, compute real progress

Fixes 6 issues from docs/achievement-api-test-report-2026-06-11.md
plus 2 newly-discovered typeHandler bugs (Bug #7, Bug #8) surfaced
during validation.

CRITICAL #1 — escape `key` column via @TableField(value = "`key`")
  - Achievement.java:19 — affects MyBatis-Plus auto-generated column
    lists in selectById / selectPage / selectBatchIds.
  - Without this, every SQL that lists the achievements columns fails
    with 'You have an error in your SQL syntax ... near key,...'.

CRITICAL #2 — escape `key` in @select SQL
  - AchievementMapper.findByKey:24 — WHERE clause now backticks.
  - Fixes admin POST /achievements which called findByKey for dedup.

HIGH #3 — T2b /achievements/{invalid} now returns 404 (was 500)
  - Follows from CRITICAL #1+#2: the BadSqlGrammarException
    no longer fires before the business-level ACHIEVEMENT_NOT_FOUND
    can be raised.

MEDIUM #4 — BadSqlGrammarException → DATABASE_ERROR (50001)
  - GlobalExceptionHandler:227 — new handler preserves root cause
    in server log while returning generic 'Database error' to clients.
  - Pairs with existing MyBatisSystemException / BindingException /
    DataIntegrityViolationException handlers (no fallback to 50000
    'Unknown error').

LOW #5 — getUserAchievements progress is no longer hard-coded to 0
  - AchievementServiceImpl:280 — mirrors getUserProgress logic,
    reads criteria.type from JSON Map, dispatches to SubmissionMapper
    counter.

LOW #6 — re-scoped Javadoc: getUserAchievements and
  getUserProgress are NOT duplicates (verified UserController
  /me/achievements/progress uses the latter). Both serve different
  endpoints with different DTO shapes.

Bug #7 — findAllActive missing JacksonTypeHandler
  - AchievementMapper:46 — @select bypassed @TableField(typeHandler=
    JacksonTypeHandler.class), returning criteria=null.
  - Converted to default method via BaseMapper.selectList.

Bug #8 — findByKey same typeHandler bypass
  - Discovered by AchievementMapperIT (L2) on first run;
    findByKey_criteriaIsDeserializedAsMap failed.
  - AchievementMapper:24 — also converted to default method.
  - Latent: only caller (create) never read criteria.

Refactors
  - M1 (review): getUserAchievements extracted buildProgressDTO helper
    (52 → 16 lines).
  - L1 (review): findAllActive uses LambdaQueryWrapper<Achievement>
    method refs instead of string column names.
  - L3 (review): Javadoc clarified the two service methods.

Tests
  + AchievementMapperSQLGuardTest: 3 reflection-guard tests
    (CRITICAL #1, CRITICAL #2, no @select on findAllActive/findByKey).
  + AchievementMapperIT: 5 Testcontainers MySQL 8.0 IT tests
    verifying JacksonTypeHandler runtime behavior + Bug #7/#8
    regression guards.
  + AchievementServiceTest: 3 progress tests + 2 new mock fields
    (SubmissionMapper, ContestParticipantMapper).
  + GlobalExceptionHandlerTest: 2 BadSqlGrammarException tests.

Validation
  - compile / package: BUILD SUCCESS
  - Unit tests: 36/36 pass
  - Testcontainers IT: 5/5 pass
  - 8 curl integration tests on worktree:9091: all match expected
    HTTP codes (200/400/404/409) and real progress values
    (admin 6 accepted problems → progress=6 for all 3 rows).

Refs: docs/achievement-api-test-report-2026-06-11.md,
      .claude/PRPs/plans/completed/achievement-api-fixes.plan.md,
      .claude/PRPs/reports/achievement-api-fixes-report.md,
      .claude/reviews/achievement-api-fixes-review.md
DavidHLP pushed a commit that referenced this pull request Jun 11, 2026
…dation

Merges the fix/achievement-api worktree changes:
- CRITICAL #1, #2 (MySQL reserved word `key`)
- HIGH #3 (T2b 404)
- MEDIUM #4 (BadSqlGrammarException handler)
- LOW #5 (real progress computation)
- LOW #6 (Javadoc re-scoped)
- Bug #7 (findAllActive typeHandler)
- Bug #8 (findByKey typeHandler, IT-discovered)

M1/L1/L2/L3/L4 from .claude/reviews/achievement-api-fixes-review.md
all addressed. Test coverage: 36 unit + 5 Testcontainers IT.
DavidHLP added a commit that referenced this pull request Jun 13, 2026
…ete legacy SandboxService (ADR-002 §1.1)

This is the wire-up commit. The old SandboxService /
SandboxServiceImpl pair is deleted; CodeExecutionService now
depends on the Hexagonal SandboxExecutor port.

SandboxExecutorImpl (production, default-on via
@ConditionalOnProperty matchIfMissing=true):
  * Injects List<LanguageProfile> + DockerSandboxConfig +
    CodeExecutionHelper
  * Fail-fast on duplicate languageId in the constructor
  * commonSecurityArgs() prepends --network none / --cap-drop
    ALL / --read-only / --user 1000:1000 / seccomp /
    no-new-privileges; profiles cannot weaken isolation
  * runDProcess keeps the Phase 3.5 #3 concurrent stdout
    drainer (64 KiB pipe buffer deadlock fix)
  * isSandboxForkFailure / isDockerDaemonForkFailure static
    methods moved here per ADR-002 §2.5
  * DTO<->port translation lives at the seam:
    toRunTestCase (port->DTO, for helper) and toPortResult
    (DTO->port, with SubmissionStatusCodec.fromWire)

InMemorySandboxAdapter (@ConditionalOnProperty havingValue=inmemory):
  * Routes on job.code() — explicit // verdict: NAME or
    # verdict: NAME markers win; otherwise keyword heuristics
  * Used by unit tests so the sandbox path can be exercised
    without a docker daemon

CodeExecutionService:
  * Depends on SandboxExecutor instead of SandboxService
  * Translates RunSubmissionDTO.RunTestCase<->sandbox.TestCase
    and sandbox.RunCaseResult<->RunResultDTO.RunCaseResult at
    the facade boundary
  * Per-run defaults (timeoutSeconds=2, memoryMb=256) match
    the pre-M2a dForm defaults; a follow-up wires per-problem
    resource limits from the controller

This is the commit that flips the import: any code that still
imports SandboxService will not compile. The pre-M2a
SandboxServiceImplTest is deleted (its coverage moved to
SandboxExecutorImplForkDetectionTest in the next commit).
DavidHLP pushed a commit that referenced this pull request Jun 13, 2026
… / reaper reclaim

codex 6-commit 对抗审查发现 3 个 P1 真缺陷,本 commit 全修。ADR-003
Status 保持 Accepted(修复等价于补完 F12 验证)。

P1 #1: 真 cutover 不发生 (SubmissionServiceImpl)
- 问题: use-judge-outbox=true 时 submit 写 is_shadow=true,但
  claimRealDispatch 只选 is_shadow=0 → dispatcher 永远不接收
  新行,旧 RQueue 仍是唯一 active producer
- 修复: is_shadow = !judgeQueueUsePort (切流时写 is_shadow=false);
  portActive=true 时**不**调 enqueueJudgeJob (避免双投递);
  portActive=false 时调 RQueue (M3a 影子 + legacy 真投递)
- 范围: 仅改 SubmissionServiceImpl.submit 主路径;
  AdminSubmissionServiceImpl.rejudge + JudgingLeaseReaper.afterCommit
  两条次路径 commit message 标记 follow-up,影响低(rejudge 与
  lease 恢复频次远低于 submit)

P1 #2: stream.add 失败时静默丢消息 (RedissonStreamsJudgeQueueAdapter)
- 问题: SETNX 成功但 stream.add 抛异常时,dedup key 未清除,
  dispatcher retry 时 enqueue 误判已投递,outbox 标 SENT 但 stream
  无 entry → 消息永久丢失
- 修复: try/catch 包裹 stream.add,失败时 delete bucket (dedup key
  rollback) 后 rethrow。原 JSON 序列化失败时 delete 已存在,新
  路径覆盖同样的回滚契约

P1 #3: reaper reclaim 路径无效 (UnackedStreamEntriesReaper +
JudgeWorkerProcessor)
- 问题: claimIdle 返回 reclaimed handle 但 reaper 只 log 不消费;
  worker poll 用 neverDelivered() 不会读 PEL,reclaimed entry
  永远不被消费
- 修复: reaper 注入 ObjectProvider<JudgeWorkerProcessor>(provider
  模式让无 worker bean 时仍可编译),reclaim 后调
  worker.processReclaimedHandle(port, handle); worker 加
  processReclaimedHandle public 入口复用 processJobFromPort
  (fenced 核心)

未动: AdminSubmissionServiceImpl 3 处 rejudge 路径 + JudgingLeaseReaper
2 处 afterCommit 路径 (commit message 标记 follow-up;主路径修了
80% cutover 行为)

ADR-003 Status: Accepted (保持)— 修复等价于补完 F12 验证,README
状态转换规则的"M3c merged + F12 验证"门禁现满足

验证: ./mvnw compile 通过, ./mvnw test 1091 tests / 8 失败 + 2
错误 与 M3a+M3b+M3c-1+M3c-2+M3c-3a+M3c-3b 预存基线一致,
本 commit 零回归(失败数未变)。
DavidHLP pushed a commit that referenced this pull request Jun 13, 2026
… perf + per-channel tests

New test files (5):
- NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel
  failure isolation), #3 (intentId idempotency), #6 (latency < 50ms),
  category preference suppression, ledger short-circuit, failure_reason
  truncation, ledger state sanity.
- NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at
  least 1 supporting channel; matrix sanity for InApp/Email/WebSocket.
- InAppNotificationChannelTest: channelId, supports, send → row-only path
  with type=recordSimpleName, metadata includes isAccepted, follow link
  + title.
- EmailNotificationChannelTest: channelId, supports matrix, in-flight
  rejection, missing-email BusinessException, send routes to
  EmailService.sendEmail with templateId.
- WebSocketNotificationChannelTest: channelId, achievement emits
  BadgeEarnedPayload (badgeTier slug), submission emits
  NotificationPayload with isAccepted, contest reminder type tag.

EmailNotificationChannel.userMapper: package-private (was private) so
unit tests in the same package can inject a mock directly. Production
wiring is still via @Autowired(required=false).

mvn test on the 5 new test classes + the 4 existing legacy tests:
- 5 new classes: 28 tests, 0 failures
- 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified)

Pre-existing ContestPublicControllerTest failure (ApplicationContext load
issue, unrelated to ADR-004) confirmed to fail on main without my changes.

Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP pushed a commit that referenced this pull request Jun 13, 2026
…t-skip

3 review findings addressed in 1 commit because they are all 1-line
behavior fixes (no record schema change, no test infrastructure change):

- EmailTemplates.forIntent: 5 null guards added to Map.of(...) calls
  (achievementName/contestTitle/replierUsername/title). Map.of throws
  NPE on null values; legacy code coalesced similar fields to '' but
  missed these. Fixes finding #1.
- WebSocketNotificationChannel.send: 5 NotificationPayload type strings
  flipped from lowercase to UPPERCASE to match the legacy wire contract
  (SUBMISSION / CONTEST REMINDER / FOLLOW / REPLY / SYSTEM). Frontend
  branches on payload.type case-sensitively; this would have been a
  silent frontend regression. Fixes finding #2.
- EmailNotificationChannel.send: missing-email path now silently returns
  (log.debug) instead of throwing BusinessException. ADR-004 §2.5 says
  email failures are best-effort; throwing caused warn-spam + a
  FAILED ledger row per dispatch for every user that hasn't filled in
  an email. Fixes finding #3.

Tests:
- WebSocketNotificationChannelTest updated to expect UPPERCASE type
  strings.
- EmailNotificationChannelTest#sendSilentlySkipsWhenUserHasNoEmail
  replaces the old #sendThrowsBusinessExceptionWhenUserHasNoEmail.

mvn test on the notification/follow/submission/email slice: green.

Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings
#1, #2, #3).
DavidHLP pushed a commit that referenced this pull request Jun 13, 2026
新增 10 flag 一览表 (5 产品 + 5 cutover), env var 名取自 application.yml
中 ${XXX:default} 占位符 (项目自定义, 非 Spring Boot 默认 APP_FEATURES_*).
§10.2 切换流程含 pm2 reload + 3 CI job 全绿门禁.
§10.3 紧急回滚用 git revert + pm2 reload.
§10.4 引到 ADR-005 §2.6 + 新建 drill 协议.
§10.5 临时方案看启动日志, 等 ADR-008 Nacos Config client.

Refs: docs/adr/ADR-005-rolling-deploy-playbook.md §4 Row #3
DavidHLP added a commit that referenced this pull request Jun 13, 2026
…ete legacy SandboxService (ADR-002 §1.1)

This is the wire-up commit. The old SandboxService /
SandboxServiceImpl pair is deleted; CodeExecutionService now
depends on the Hexagonal SandboxExecutor port.

SandboxExecutorImpl (production, default-on via
@ConditionalOnProperty matchIfMissing=true):
  * Injects List<LanguageProfile> + DockerSandboxConfig +
    CodeExecutionHelper
  * Fail-fast on duplicate languageId in the constructor
  * commonSecurityArgs() prepends --network none / --cap-drop
    ALL / --read-only / --user 1000:1000 / seccomp /
    no-new-privileges; profiles cannot weaken isolation
  * runDProcess keeps the Phase 3.5 #3 concurrent stdout
    drainer (64 KiB pipe buffer deadlock fix)
  * isSandboxForkFailure / isDockerDaemonForkFailure static
    methods moved here per ADR-002 §2.5
  * DTO<->port translation lives at the seam:
    toRunTestCase (port->DTO, for helper) and toPortResult
    (DTO->port, with SubmissionStatusCodec.fromWire)

InMemorySandboxAdapter (@ConditionalOnProperty havingValue=inmemory):
  * Routes on job.code() — explicit // verdict: NAME or
    # verdict: NAME markers win; otherwise keyword heuristics
  * Used by unit tests so the sandbox path can be exercised
    without a docker daemon

CodeExecutionService:
  * Depends on SandboxExecutor instead of SandboxService
  * Translates RunSubmissionDTO.RunTestCase<->sandbox.TestCase
    and sandbox.RunCaseResult<->RunResultDTO.RunCaseResult at
    the facade boundary
  * Per-run defaults (timeoutSeconds=2, memoryMb=256) match
    the pre-M2a dForm defaults; a follow-up wires per-problem
    resource limits from the controller

This is the commit that flips the import: any code that still
imports SandboxService will not compile. The pre-M2a
SandboxServiceImplTest is deleted (its coverage moved to
SandboxExecutorImplForkDetectionTest in the next commit).
DavidHLP added a commit that referenced this pull request Jun 13, 2026
… / reaper reclaim

codex 6-commit 对抗审查发现 3 个 P1 真缺陷,本 commit 全修。ADR-003
Status 保持 Accepted(修复等价于补完 F12 验证)。

P1 #1: 真 cutover 不发生 (SubmissionServiceImpl)
- 问题: use-judge-outbox=true 时 submit 写 is_shadow=true,但
  claimRealDispatch 只选 is_shadow=0 → dispatcher 永远不接收
  新行,旧 RQueue 仍是唯一 active producer
- 修复: is_shadow = !judgeQueueUsePort (切流时写 is_shadow=false);
  portActive=true 时**不**调 enqueueJudgeJob (避免双投递);
  portActive=false 时调 RQueue (M3a 影子 + legacy 真投递)
- 范围: 仅改 SubmissionServiceImpl.submit 主路径;
  AdminSubmissionServiceImpl.rejudge + JudgingLeaseReaper.afterCommit
  两条次路径 commit message 标记 follow-up,影响低(rejudge 与
  lease 恢复频次远低于 submit)

P1 #2: stream.add 失败时静默丢消息 (RedissonStreamsJudgeQueueAdapter)
- 问题: SETNX 成功但 stream.add 抛异常时,dedup key 未清除,
  dispatcher retry 时 enqueue 误判已投递,outbox 标 SENT 但 stream
  无 entry → 消息永久丢失
- 修复: try/catch 包裹 stream.add,失败时 delete bucket (dedup key
  rollback) 后 rethrow。原 JSON 序列化失败时 delete 已存在,新
  路径覆盖同样的回滚契约

P1 #3: reaper reclaim 路径无效 (UnackedStreamEntriesReaper +
JudgeWorkerProcessor)
- 问题: claimIdle 返回 reclaimed handle 但 reaper 只 log 不消费;
  worker poll 用 neverDelivered() 不会读 PEL,reclaimed entry
  永远不被消费
- 修复: reaper 注入 ObjectProvider<JudgeWorkerProcessor>(provider
  模式让无 worker bean 时仍可编译),reclaim 后调
  worker.processReclaimedHandle(port, handle); worker 加
  processReclaimedHandle public 入口复用 processJobFromPort
  (fenced 核心)

未动: AdminSubmissionServiceImpl 3 处 rejudge 路径 + JudgingLeaseReaper
2 处 afterCommit 路径 (commit message 标记 follow-up;主路径修了
80% cutover 行为)

ADR-003 Status: Accepted (保持)— 修复等价于补完 F12 验证,README
状态转换规则的"M3c merged + F12 验证"门禁现满足

验证: ./mvnw compile 通过, ./mvnw test 1091 tests / 8 失败 + 2
错误 与 M3a+M3b+M3c-1+M3c-2+M3c-3a+M3c-3b 预存基线一致,
本 commit 零回归(失败数未变)。
DavidHLP added a commit that referenced this pull request Jun 13, 2026
… perf + per-channel tests

New test files (5):
- NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel
  failure isolation), #3 (intentId idempotency), #6 (latency < 50ms),
  category preference suppression, ledger short-circuit, failure_reason
  truncation, ledger state sanity.
- NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at
  least 1 supporting channel; matrix sanity for InApp/Email/WebSocket.
- InAppNotificationChannelTest: channelId, supports, send → row-only path
  with type=recordSimpleName, metadata includes isAccepted, follow link
  + title.
- EmailNotificationChannelTest: channelId, supports matrix, in-flight
  rejection, missing-email BusinessException, send routes to
  EmailService.sendEmail with templateId.
- WebSocketNotificationChannelTest: channelId, achievement emits
  BadgeEarnedPayload (badgeTier slug), submission emits
  NotificationPayload with isAccepted, contest reminder type tag.

EmailNotificationChannel.userMapper: package-private (was private) so
unit tests in the same package can inject a mock directly. Production
wiring is still via @Autowired(required=false).

mvn test on the 5 new test classes + the 4 existing legacy tests:
- 5 new classes: 28 tests, 0 failures
- 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified)

Pre-existing ContestPublicControllerTest failure (ApplicationContext load
issue, unrelated to ADR-004) confirmed to fail on main without my changes.

Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP added a commit that referenced this pull request Jun 13, 2026
…t-skip

3 review findings addressed in 1 commit because they are all 1-line
behavior fixes (no record schema change, no test infrastructure change):

- EmailTemplates.forIntent: 5 null guards added to Map.of(...) calls
  (achievementName/contestTitle/replierUsername/title). Map.of throws
  NPE on null values; legacy code coalesced similar fields to '' but
  missed these. Fixes finding #1.
- WebSocketNotificationChannel.send: 5 NotificationPayload type strings
  flipped from lowercase to UPPERCASE to match the legacy wire contract
  (SUBMISSION / CONTEST REMINDER / FOLLOW / REPLY / SYSTEM). Frontend
  branches on payload.type case-sensitively; this would have been a
  silent frontend regression. Fixes finding #2.
- EmailNotificationChannel.send: missing-email path now silently returns
  (log.debug) instead of throwing BusinessException. ADR-004 §2.5 says
  email failures are best-effort; throwing caused warn-spam + a
  FAILED ledger row per dispatch for every user that hasn't filled in
  an email. Fixes finding #3.

Tests:
- WebSocketNotificationChannelTest updated to expect UPPERCASE type
  strings.
- EmailNotificationChannelTest#sendSilentlySkipsWhenUserHasNoEmail
  replaces the old #sendThrowsBusinessExceptionWhenUserHasNoEmail.

mvn test on the notification/follow/submission/email slice: green.

Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings
#1, #2, #3).
DavidHLP added a commit that referenced this pull request Jun 13, 2026
新增 10 flag 一览表 (5 产品 + 5 cutover), env var 名取自 application.yml
中 ${XXX:default} 占位符 (项目自定义, 非 Spring Boot 默认 APP_FEATURES_*).
§10.2 切换流程含 pm2 reload + 3 CI job 全绿门禁.
§10.3 紧急回滚用 git revert + pm2 reload.
§10.4 引到 ADR-005 §2.6 + 新建 drill 协议.
§10.5 临时方案看启动日志, 等 ADR-008 Nacos Config client.

Refs: docs/adr/ADR-005-rolling-deploy-playbook.md §4 Row #3
DavidHLP added a commit that referenced this pull request Jun 14, 2026
- CaseScope enum (SAMPLE/HIDDEN, null 视作 legacy sample 在投影层)
- JudgeSourceProperties (app.features.judge-source.use-test-cases 默认 true)
- Submission.TestCaseDetail 加 nullable caseId + caseScope (保护历史 JSON)
- TestCaseMapper.findActiveCasesForJudging (XOR 过滤 isSample↔isHidden)
- JudgeWorkerProcessor 双源分支: flag=true→test_cases+写scope; false→legacy problem_examples; fail-closed (0 用例→System Error 无 fallback)
- SubmissionServiceImpl.toVO() 重写: hidden 不进 vo.tests; HIDDEN-only 失败仅 errorDetail 不暴露 I/O
- 7 个测试 21 case (含 Testcontainers MySQL 真测 @TableLogic + NULL 处理)

Refs: task #3 (P0-1 backend)
Reviewed-by: @ulticode-reviewer (approved)

Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 3, 2026
feat: complete business improvement plan implementation
DavidHLP added a commit that referenced this pull request Jul 3, 2026
Three defects from docs/comments-api-test-report.md (curl test report):

#1 GET /admin/comments/{type}/{id} with invalid 'type' returned HTTP 500
   instead of 400. Root cause: `@PathVariable @Pattern` on
   `@Validated` controllers throws ConstraintViolationException which
   had no `@ExceptionHandler` in GlobalExceptionHandler, falling
   through to handleGenericException.
   Fix: add handleConstraintViolation mirroring the response shape
   of the existing handleValidationException (@Valid @RequestBody).

#2 unflagComment left flagged_reason / flagged_at stale in the row
   even though the service code called setFlaggedReason(null) /
   setFlaggedAt(null). Root cause: MyBatis-Plus
   FieldStrategy.NOT_NULL (the default) silently drops null columns
   in updateById, and the entity-level @TableField updateStrategy
   may apply similar rules. The setter was a no-op at the SQL layer.
   Fix: switch unflagComment to mapper.update(null, LambdaUpdateWrapper)
   with explicit .set(field, null) clauses.

#3 deleteComment set only the audit columns (deleted_at, deleted_by)
   but is_deleted remained 0, so GET endpoints happily returned
   already-soft-deleted comments. Same root cause as #2 plus the
   Boolean wrapper for is_deleted. Fix: switch to
   LambdaUpdateWrapper with explicit .set(IsDeleted, true).

bulkCommentAction automatically benefits since it dispatches to
the patched deleteComment / unflagComment methods.

Verification: real curl regression against ulticode-9001 (PM2,
dev profile) — three defects each return the expected 2xx/4xx and
the DB columns reflect the intended final state.

Tests: AdminCommentServiceImplTest covers 3 smoke cases (construct,
invalid-type throw, deleteComment unknown-id throw). Full mutator
SQL inspection requires `@SpringBootTest` because MyBatis-Plus
LambdaUpdateWrapper relies on a Spring-initialized lambda-method
cache (AbstractLambdaWrapper.tryInitCache) that is absent under
plain Mockito; integration coverage is recorded in
docs/comments-api-test-report.md §9.

Audit: AuditContext.setUserId / setOldValues / setNewValues calls
are preserved unchanged before each mapper.update invocation so
the @Audited behavior on the three methods is identical to before.

Out of scope: getComment / getComments queries do not yet filter
`is_deleted=1` (GET still returns 200 for a deleted row). Tracked
in a separate follow-up plan:
.claude/PRPs/plans/admin-comments-get-filter-soft-deleted.plan.md
DavidHLP added a commit that referenced this pull request Jul 3, 2026
Fixes three defects from docs/comments-api-test-report.md:
#1 GET /admin/comments/{type}/{id} invalid 'type' (500 → 400)
#2 unflagComment left flagged_reason / flagged_at stale (null 字段未被 MyBatis-Plus 持久化)
#3 deleteComment only set audit columns, is_deleted remained 0

Plus unit test scaffolding and follow-up plan for getComment soft-delete filtering.
DavidHLP added a commit that referenced this pull request Jul 3, 2026
…_ADMIN + rankings 404 + is_deleted filter + Unicode title + @Audited + new ErrorCode + controller XSS guard + mutator tests)

修复 docs/contests-api-test-report.md 报告的 7 处缺陷 (1 HIGH + 6 MEDIUM/LOW):

缺陷 #1 (HIGH) — DELETE 软删除 is_deleted 字段未持久化
  - 根因:Contest.isDeleted 字段 @TableLogic + mapper.updateById(entity) 自动忽略逻辑删除字段
  - 修复:改用 mapper.update(null, LambdaUpdateWrapper.set(...)) 显式写入

缺陷 #2 — PATCH on FINISHED/RUNNING 比赛无状态守卫
  - 修复:service 层加 status guard,仅允许 UPCOMING 修改
  - 新增 ErrorCode CONTEST_ONLY_UPDATE_UPCOMING (70006) 区分注册与修改语义

缺陷 #3 — service 层 hasRole("ADMIN") 拒绝 SUPER_ADMIN,与控制器 @PreAuthorize 不一致
  - 修复:SecurityUtil 新增 hasAnyRole(String...) 工具方法,ContestServiceImpl 7 处 mutator 替换

缺陷 #4 — CreateContestDTO/UpdateContestDTO title @pattern 拒绝 CJK 字符
  - 修复:正则放宽为 [\p{L}\p{N}\s\p{P}]+ 接受 Unicode 字母
  - AdminContestController 新增 rejectUnsafeTitleChars helper 显式排除 < 和 >

缺陷 #5 — GET /admin/contest/{id}/rankings 对不存在 contest 返 200+空
  - 修复:getAdminContestRanking 入口加 contest 存在性 + is_deleted 校验

缺陷 #6 — addProblem/removeProblem/startContest/endContest 未校验 is_deleted
  - 修复:4 个 mutator 入口加 .filter(c -> !isDeleted).orElseThrow(COMPETITION_NOT_FOUND)

缺陷 #7@Audited 注解缺失导致 audit_logs 无 contest 记录
  - 修复:7 个 mutator 方法加 @Audited + AuditContext.setOldValues/setNewValues/setUserId
  - 复用 AuditActionUtil.CREATE_CONTEST/UPDATE_CONTEST/DELETE_CONTEST 常量

新增测试 ContestServiceImplMutatorTest:5 case 覆盖缺陷 #2/#5/#6 的状态守卫 + 软删除过滤 + 404 路径。
缺陷 #1 的 LambdaUpdateWrapper.getSqlSet() 断言需 @SpringBootTest 上下文,保留为 follow-up,
运行时 curl 验证已确认三列同改 (docs/contests-api-test-report.md §3.5)。

验证:
- mvnw compile -B:0 错误
- mvnw test -Dtest=ContestServiceImplMutatorTest:5/5 通过,2.0s
- 运行时 curl 5/5 critical 缺陷修复确认 (PM2 重启 ulticode-9001)

Artifacts:
- Plan: .claude/PRPs/plans/completed/admin-contests-mutator-fixes.plan.md
- Report: .claude/PRPs/reports/admin-contests-mutator-fixes-report.md
- Review v1: .claude/reviews/admin-contests-mutator-review.md
- Review v2: .claude/reviews/admin-contests-mutator-review-v2.md
- Test report: docs/contests-api-test-report.md
DavidHLP added a commit that referenced this pull request Jul 3, 2026
…ject empty PATCH fields

Fixes4 defects in AdminTagController discovered via docs/admin-tags-test-plan.md:

- Bug #1: GET/DELETE /admin/tags/{id} without ?type= returned500. Added
 MissingServletRequestParameterException handler in GlobalExceptionHandler
 returning400 with field-level error map (mirrors handleConstraintViolation).

- Bug #2: ?type=GARBAGE silently fell back to PROBLEM. Added @validated +
 @pattern on controller query params; added @pattern on TagQueryDTO.type,
 UpdateTagDTO.type, CreateTagDTO.type. New dto.tag.TagTypes class is the
 single source of truth for the whitelist (PROBLEM|FORUM). Service-layer
 normalizeType() helper enforces the same whitelist for direct callers.

- Bug #3: PROBLEM tag list ignored sortBy. getProblemTags now dispatches on
 usageCount / createdAt / slug / label, mirroring getForumTags.

- Bug #4: PATCH with empty name silently ignored. Added @SiZe(min=1) on
 UpdateTagDTO.name and slug; @NotNull already covered type.

Adds39 regression tests (15 Mockito service +24 WebMvcTest controller) with
exact code=40000 + data.<field> assertions.

Co-Authored-By: Claude Opus4.8 <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 3, 2026
… handler, compute real progress

Fixes 6 issues from docs/achievement-api-test-report-2026-06-11.md
plus 2 newly-discovered typeHandler bugs (Bug #7, Bug #8) surfaced
during validation.

CRITICAL #1 — escape `key` column via @TableField(value = "`key`")
  - Achievement.java:19 — affects MyBatis-Plus auto-generated column
    lists in selectById / selectPage / selectBatchIds.
  - Without this, every SQL that lists the achievements columns fails
    with 'You have an error in your SQL syntax ... near key,...'.

CRITICAL #2 — escape `key` in @select SQL
  - AchievementMapper.findByKey:24 — WHERE clause now backticks.
  - Fixes admin POST /achievements which called findByKey for dedup.

HIGH #3 — T2b /achievements/{invalid} now returns 404 (was 500)
  - Follows from CRITICAL #1+#2: the BadSqlGrammarException
    no longer fires before the business-level ACHIEVEMENT_NOT_FOUND
    can be raised.

MEDIUM #4 — BadSqlGrammarException → DATABASE_ERROR (50001)
  - GlobalExceptionHandler:227 — new handler preserves root cause
    in server log while returning generic 'Database error' to clients.
  - Pairs with existing MyBatisSystemException / BindingException /
    DataIntegrityViolationException handlers (no fallback to 50000
    'Unknown error').

LOW #5 — getUserAchievements progress is no longer hard-coded to 0
  - AchievementServiceImpl:280 — mirrors getUserProgress logic,
    reads criteria.type from JSON Map, dispatches to SubmissionMapper
    counter.

LOW #6 — re-scoped Javadoc: getUserAchievements and
  getUserProgress are NOT duplicates (verified UserController
  /me/achievements/progress uses the latter). Both serve different
  endpoints with different DTO shapes.

Bug #7 — findAllActive missing JacksonTypeHandler
  - AchievementMapper:46 — @select bypassed @TableField(typeHandler=
    JacksonTypeHandler.class), returning criteria=null.
  - Converted to default method via BaseMapper.selectList.

Bug #8 — findByKey same typeHandler bypass
  - Discovered by AchievementMapperIT (L2) on first run;
    findByKey_criteriaIsDeserializedAsMap failed.
  - AchievementMapper:24 — also converted to default method.
  - Latent: only caller (create) never read criteria.

Refactors
  - M1 (review): getUserAchievements extracted buildProgressDTO helper
    (52 → 16 lines).
  - L1 (review): findAllActive uses LambdaQueryWrapper<Achievement>
    method refs instead of string column names.
  - L3 (review): Javadoc clarified the two service methods.

Tests
  + AchievementMapperSQLGuardTest: 3 reflection-guard tests
    (CRITICAL #1, CRITICAL #2, no @select on findAllActive/findByKey).
  + AchievementMapperIT: 5 Testcontainers MySQL 8.0 IT tests
    verifying JacksonTypeHandler runtime behavior + Bug #7/#8
    regression guards.
  + AchievementServiceTest: 3 progress tests + 2 new mock fields
    (SubmissionMapper, ContestParticipantMapper).
  + GlobalExceptionHandlerTest: 2 BadSqlGrammarException tests.

Validation
  - compile / package: BUILD SUCCESS
  - Unit tests: 36/36 pass
  - Testcontainers IT: 5/5 pass
  - 8 curl integration tests on worktree:9091: all match expected
    HTTP codes (200/400/404/409) and real progress values
    (admin 6 accepted problems → progress=6 for all 3 rows).

Refs: docs/achievement-api-test-report-2026-06-11.md,
      .claude/PRPs/plans/completed/achievement-api-fixes.plan.md,
      .claude/PRPs/reports/achievement-api-fixes-report.md,
      .claude/reviews/achievement-api-fixes-review.md
DavidHLP added a commit that referenced this pull request Jul 3, 2026
…dation

Merges the fix/achievement-api worktree changes:
- CRITICAL #1, #2 (MySQL reserved word `key`)
- HIGH #3 (T2b 404)
- MEDIUM #4 (BadSqlGrammarException handler)
- LOW #5 (real progress computation)
- LOW #6 (Javadoc re-scoped)
- Bug #7 (findAllActive typeHandler)
- Bug #8 (findByKey typeHandler, IT-discovered)

M1/L1/L2/L3/L4 from .claude/reviews/achievement-api-fixes-review.md
all addressed. Test coverage: 36 unit + 5 Testcontainers IT.
DavidHLP added a commit that referenced this pull request Jul 3, 2026
…ete legacy SandboxService (ADR-002 §1.1)

This is the wire-up commit. The old SandboxService /
SandboxServiceImpl pair is deleted; CodeExecutionService now
depends on the Hexagonal SandboxExecutor port.

SandboxExecutorImpl (production, default-on via
@ConditionalOnProperty matchIfMissing=true):
  * Injects List<LanguageProfile> + DockerSandboxConfig +
    CodeExecutionHelper
  * Fail-fast on duplicate languageId in the constructor
  * commonSecurityArgs() prepends --network none / --cap-drop
    ALL / --read-only / --user 1000:1000 / seccomp /
    no-new-privileges; profiles cannot weaken isolation
  * runDProcess keeps the Phase 3.5 #3 concurrent stdout
    drainer (64 KiB pipe buffer deadlock fix)
  * isSandboxForkFailure / isDockerDaemonForkFailure static
    methods moved here per ADR-002 §2.5
  * DTO<->port translation lives at the seam:
    toRunTestCase (port->DTO, for helper) and toPortResult
    (DTO->port, with SubmissionStatusCodec.fromWire)

InMemorySandboxAdapter (@ConditionalOnProperty havingValue=inmemory):
  * Routes on job.code() — explicit // verdict: NAME or
    # verdict: NAME markers win; otherwise keyword heuristics
  * Used by unit tests so the sandbox path can be exercised
    without a docker daemon

CodeExecutionService:
  * Depends on SandboxExecutor instead of SandboxService
  * Translates RunSubmissionDTO.RunTestCase<->sandbox.TestCase
    and sandbox.RunCaseResult<->RunResultDTO.RunCaseResult at
    the facade boundary
  * Per-run defaults (timeoutSeconds=2, memoryMb=256) match
    the pre-M2a dForm defaults; a follow-up wires per-problem
    resource limits from the controller

This is the commit that flips the import: any code that still
imports SandboxService will not compile. The pre-M2a
SandboxServiceImplTest is deleted (its coverage moved to
SandboxExecutorImplForkDetectionTest in the next commit).
DavidHLP added a commit that referenced this pull request Jul 3, 2026
… / reaper reclaim

codex 6-commit 对抗审查发现 3 个 P1 真缺陷,本 commit 全修。ADR-003
Status 保持 Accepted(修复等价于补完 F12 验证)。

P1 #1: 真 cutover 不发生 (SubmissionServiceImpl)
- 问题: use-judge-outbox=true 时 submit 写 is_shadow=true,但
  claimRealDispatch 只选 is_shadow=0 → dispatcher 永远不接收
  新行,旧 RQueue 仍是唯一 active producer
- 修复: is_shadow = !judgeQueueUsePort (切流时写 is_shadow=false);
  portActive=true 时**不**调 enqueueJudgeJob (避免双投递);
  portActive=false 时调 RQueue (M3a 影子 + legacy 真投递)
- 范围: 仅改 SubmissionServiceImpl.submit 主路径;
  AdminSubmissionServiceImpl.rejudge + JudgingLeaseReaper.afterCommit
  两条次路径 commit message 标记 follow-up,影响低(rejudge 与
  lease 恢复频次远低于 submit)

P1 #2: stream.add 失败时静默丢消息 (RedissonStreamsJudgeQueueAdapter)
- 问题: SETNX 成功但 stream.add 抛异常时,dedup key 未清除,
  dispatcher retry 时 enqueue 误判已投递,outbox 标 SENT 但 stream
  无 entry → 消息永久丢失
- 修复: try/catch 包裹 stream.add,失败时 delete bucket (dedup key
  rollback) 后 rethrow。原 JSON 序列化失败时 delete 已存在,新
  路径覆盖同样的回滚契约

P1 #3: reaper reclaim 路径无效 (UnackedStreamEntriesReaper +
JudgeWorkerProcessor)
- 问题: claimIdle 返回 reclaimed handle 但 reaper 只 log 不消费;
  worker poll 用 neverDelivered() 不会读 PEL,reclaimed entry
  永远不被消费
- 修复: reaper 注入 ObjectProvider<JudgeWorkerProcessor>(provider
  模式让无 worker bean 时仍可编译),reclaim 后调
  worker.processReclaimedHandle(port, handle); worker 加
  processReclaimedHandle public 入口复用 processJobFromPort
  (fenced 核心)

未动: AdminSubmissionServiceImpl 3 处 rejudge 路径 + JudgingLeaseReaper
2 处 afterCommit 路径 (commit message 标记 follow-up;主路径修了
80% cutover 行为)

ADR-003 Status: Accepted (保持)— 修复等价于补完 F12 验证,README
状态转换规则的"M3c merged + F12 验证"门禁现满足

验证: ./mvnw compile 通过, ./mvnw test 1091 tests / 8 失败 + 2
错误 与 M3a+M3b+M3c-1+M3c-2+M3c-3a+M3c-3b 预存基线一致,
本 commit 零回归(失败数未变)。
DavidHLP added a commit that referenced this pull request Jul 3, 2026
… perf + per-channel tests

New test files (5):
- NotificationDispatcherTest: 8 tests covering ADR-004 §4 #2 (channel
  failure isolation), #3 (intentId idempotency), #6 (latency < 50ms),
  category preference suppression, ledger short-circuit, failure_reason
  truncation, ledger state sanity.
- NotificationChannelContractTest: ADR-004 §4 #1 — every intent has at
  least 1 supporting channel; matrix sanity for InApp/Email/WebSocket.
- InAppNotificationChannelTest: channelId, supports, send → row-only path
  with type=recordSimpleName, metadata includes isAccepted, follow link
  + title.
- EmailNotificationChannelTest: channelId, supports matrix, in-flight
  rejection, missing-email BusinessException, send routes to
  EmailService.sendEmail with templateId.
- WebSocketNotificationChannelTest: channelId, achievement emits
  BadgeEarnedPayload (badgeTier slug), submission emits
  NotificationPayload with isAccepted, contest reminder type tag.

EmailNotificationChannel.userMapper: package-private (was private) so
unit tests in the same package can inject a mock directly. Production
wiring is still via @Autowired(required=false).

mvn test on the 5 new test classes + the 4 existing legacy tests:
- 5 new classes: 28 tests, 0 failures
- 4 existing tests: 30 tests, 0 failures (legacy flag-off path verified)

Pre-existing ContestPublicControllerTest failure (ApplicationContext load
issue, unrelated to ADR-004) confirmed to fail on main without my changes.

Refs: docs/adr/ADR-004-notification-intents.md §4.
DavidHLP added a commit that referenced this pull request Jul 3, 2026
…t-skip

3 review findings addressed in 1 commit because they are all 1-line
behavior fixes (no record schema change, no test infrastructure change):

- EmailTemplates.forIntent: 5 null guards added to Map.of(...) calls
  (achievementName/contestTitle/replierUsername/title). Map.of throws
  NPE on null values; legacy code coalesced similar fields to '' but
  missed these. Fixes finding #1.
- WebSocketNotificationChannel.send: 5 NotificationPayload type strings
  flipped from lowercase to UPPERCASE to match the legacy wire contract
  (SUBMISSION / CONTEST REMINDER / FOLLOW / REPLY / SYSTEM). Frontend
  branches on payload.type case-sensitively; this would have been a
  silent frontend regression. Fixes finding #2.
- EmailNotificationChannel.send: missing-email path now silently returns
  (log.debug) instead of throwing BusinessException. ADR-004 §2.5 says
  email failures are best-effort; throwing caused warn-spam + a
  FAILED ledger row per dispatch for every user that hasn't filled in
  an email. Fixes finding #3.

Tests:
- WebSocketNotificationChannelTest updated to expect UPPERCASE type
  strings.
- EmailNotificationChannelTest#sendSilentlySkipsWhenUserHasNoEmail
  replaces the old #sendThrowsBusinessExceptionWhenUserHasNoEmail.

mvn test on the notification/follow/submission/email slice: green.

Refs: docs/adr/ADR-004-notification-intents.md (M4d-1 review findings
#1, #2, #3).
DavidHLP added a commit that referenced this pull request Jul 3, 2026
新增 10 flag 一览表 (5 产品 + 5 cutover), env var 名取自 application.yml
中 ${XXX:default} 占位符 (项目自定义, 非 Spring Boot 默认 APP_FEATURES_*).
§10.2 切换流程含 pm2 reload + 3 CI job 全绿门禁.
§10.3 紧急回滚用 git revert + pm2 reload.
§10.4 引到 ADR-005 §2.6 + 新建 drill 协议.
§10.5 临时方案看启动日志, 等 ADR-008 Nacos Config client.

Refs: docs/adr/ADR-005-rolling-deploy-playbook.md §4 Row #3
DavidHLP added a commit that referenced this pull request Jul 3, 2026
- CaseScope enum (SAMPLE/HIDDEN, null 视作 legacy sample 在投影层)
- JudgeSourceProperties (app.features.judge-source.use-test-cases 默认 true)
- Submission.TestCaseDetail 加 nullable caseId + caseScope (保护历史 JSON)
- TestCaseMapper.findActiveCasesForJudging (XOR 过滤 isSample↔isHidden)
- JudgeWorkerProcessor 双源分支: flag=true→test_cases+写scope; false→legacy problem_examples; fail-closed (0 用例→System Error 无 fallback)
- SubmissionServiceImpl.toVO() 重写: hidden 不进 vo.tests; HIDDEN-only 失败仅 errorDetail 不暴露 I/O
- 7 个测试 21 case (含 Testcontainers MySQL 真测 @TableLogic + NULL 处理)

Refs: task #3 (P0-1 backend)
Reviewed-by: @ulticode-reviewer (approved)

Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 10, 2026
…terface

Architecture review candidate #3 — HttpClient.axiosInstance was exposed
as 'escape hatch — exposed for download endpoints and tests', but:
  - Production: ZERO callers actually used it. Verified by grep — all 31
    imports from @/utils/request use named API (apiGet/apiPost/...).
    The default export of raw axios from management/request.ts was dead
    code, but a footgun: any future import axios from '@/utils/request'
    would have bypassed every interceptor (CSRF, dedup, retry, 401).
  - Tests: 9 sites used client.axiosInstance.defaults.adapter to inject
    a mock adapter — legitimate testing need, but achieved by reaching
    through the public interface into the implementation.

Deepening:
  - Remove axiosInstance from HttpClient interface type
  - Add __testAdapter?: AxiosAdapter to HttpClientConfig — wires the
    mock adapter into the underlying axios instance at construction
    time, before any interceptors fire. Production MUST NOT set this
    (documented in javadoc).
  - Remove dead 'axiosInstance' destructure + dead 'export default
    axiosInstance' from management/src/utils/request.ts
  - Update shared/http-client tests: 9 sites pass __testAdapter in
    config instead of casting through the interface
  - Delete obsolete 'exposes the underlying axios instance' test

Console request.ts already clean (never destructured axiosInstance).

Verified:
  - shared/http-client: 10/10 tests pass
  - console: type-check clean
  - management: type-check clean, 275/275 tests pass

Wins (glossary terms):
  - locality: interceptors can no longer be skipped via the public seam
  - leverage: one interface (apiGet/apiPost/...), all paths enforced
  - two adapters justify the seam (prod: real network; test: __testAdapter)
  - CSRF / 401 / dedup now enforced on every request, no escape hatch
DavidHLP added a commit that referenced this pull request Jul 10, 2026
#1 SecurityUtil → CurrentUserProvider port
  - New CurrentUserProvider interface in common/auth/
  - SecurityCurrentUserProvider @component adapter (wraps SecurityContextHolder)
  - 37 files migrated from static SecurityUtil calls to injected interface
  - Eliminates duplicated userId==null guards across controller layer

#2 DashboardMapper+Service → DashboardStatsProjection
  - New DashboardStatsProjection interface + DefaultDashboardStatsProjection
  - Absorbs 7 private sub-aggregators + 6 mapper default methods
  - DashboardService + DashboardServiceImpl deleted
  - DashboardController now injects the projection directly
  - Mirrors ADR-0011 *Projection pattern

#3 AdminCommentService → CommentModerator polymorphic seam
  - New CommentModerator interface with ForumCommentModerator + SolutionCommentModerator
  - AdminCommentServiceImpl refactored to thin router
  - Eliminates 5x duplicated forum/solution type-string switch

#5 SubmissionMapper.calculateStreak → SubmissionStreakCalculator
  - New SubmissionStreakCalculator interface + JdbcSubmissionStreakCalculator
  - Streak algorithm (recursive CTE) now behind a JVM-testable interface
  - 3 production callers redirected + 3 test classes updated

#7 Frontend wrapper cleanup
  - 8 re-export wrapper files deleted (markdown, useTheme, useTypographyDensity, cn)
  - tsconfig path remaps @/lib/utils → @/shared/auth-core/src/utils
  - shared/sidebar-menu cn() consolidated to import from auth-core
  - LOCALE_HEADER_KEY duplicate declarations removed

#8 OAuthService → OAuthClient port + adapters
  - New OAuthClient interface + OAuthTokenResponse/OAuthUserInfo DTOs
  - GithubOAuthClient + GoogleOAuthClient @component adapters
  - OAuthService refactored to thin coordinator (221→195 LoC)
  - 11 unit tests covering dispatch, callback, DB upsert

Production code compiles (BUILD SUCCESS). 53 test compilation errors
from constructor signature changes — mechanical @mock field additions.
Candidates #4 (analytics projections) and #6 (contest.ts split) pending.
DavidHLP added a commit that referenced this pull request Jul 10, 2026
Arch review 2026-07-10 candidate #3: 7 shadow DTOs and 4 enums in
shared/domain-types had zero production callers (only PageResult<T>
is consumed — 2 console files + 8 management files). Both apps
maintain parallel definitions in their own types/ and api/admin/
trees, so the shared copy was a canonical-facade illusion.

Deleted:
  - Problem, ProblemDifficulty, ProblemStatus
  - ContestStatus, ContestType, ParticipantStatus, ContestScoringMode,
    ContestProblem, RankingEntry
  - Comment, ForumPost, ForumCommunity, ForumUser
  - UserStats, ProblemList

Kept: PageResult<T> (only proven contract).

Future types enter shared/domain-types only when a second adapter
is proven to need the same shape.
DavidHLP added a commit that referenced this pull request Jul 10, 2026
…eview #3)

The summary projection already crossed the seam; the admin detail
projection still lived inline beside the write paths. Move it
behind the same ProblemListProjection interface so:

- The admin service owns only mutations and audit; cross-mapper
  reads (author / problems / tags / stats) all live in one
  DefaultProblemListProjection module.
- The detail shaping becomes testable behind a small typed
  interface (3-arg toAdminDetailVO) instead of ~110 lines of
  inline streams beside a write state machine.
- Per-app differences stay in the projection: admin gets
  isOwner=false, isSaved=false, viewer=null, empty categories;
  the user-facing getListOverview stays unchanged.

DefaultProblemListProjection gains two private helpers
(assembleProblemInList, assembleStats) to keep the public
toAdminDetailVO method short and the helper contracts obvious.
DavidHLP added a commit that referenced this pull request Jul 10, 2026
…module (arch-review #3)

Two shallow shared packages jointly owned one locale-change lifecycle:
@ulticode/i18n-storage (persist + fallback + toast) and
@ulticode/locale-composable (switch + DOM language, which reached across
to call setStoredLocale via a bare @ulticode/i18n-storage specifier with
no tsconfig mapping — fragile resolution). One user action crossed every
shallow module.

Collapse both into one deep @ulticode/locale-preference module owning
the whole lifecycle (switch · persist · fallback · notify · DOM
language). The one genuine per-app variation (management's backend
PUT /users/me sync) stays an adapter via the onLocaleChange hook.

Console + management now import only @/shared/locale-preference/src;
the cross-package bare-specifier import is gone. Behaviour preserved
byte-for-byte (storage layer, fallback messages, debounce, toggle).

Validated: locale-preference tsc, console + management vue-tsc all 0
errors; pnpm install clean.

Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 16, 2026
Implements candidates from /tmp/architecture-review-1783827842.html:

#1 AdminBulkExecutor (admin/bulk): lifts the duplicated per-item try/catch
loop + counting + optional existence-guard out of AdminForum/Comment/
Solution/Problem bulk actions into one consumer-owned executor with a
canonical ItemOutcome/Run. batchRejudge withdrawn with evidence (rich
RejudgeResult shape ≠ void-action aggregated contract).

#2 ProblemExportService + ExportPayload: moves format validation, the 10k
size cap, CSV header/escaping, and the LocalDate.now() time-leak out of
AdminProblemController into a Clock-injected service; controller shrinks
to response-writing.

#3 LegacyRejudgeStrategy + RejudgePolicy.rejudge dispatcher: moves the
50-line inline legacy rejudge state machine out of AdminSubmissionServiceImpl
behind the policy port; fenced vs legacy selection now lives in
DefaultRejudgePolicy. AdminSubmissionServiceImpl.rejudge = 3-line dispatch.

#4 createAuthStore factory (shared/auth-core): owns the duplicated
login/logout/fetchUser/loadPermissions/initialize/clearUser/hasPermission
chain + CSRF contract once; management store migrated. Console deferred
(already composable-shaped with a richer status state machine).

#6 AdminContestReadPort + adapter: ADR-0011 phase 2 (contest) — admin
contest reads cross the seam instead of reaching into ContestProblemMapper.

#5 (AdminCrudListView wrapper) withdrawn: scaffolded then deleted after
code-review flagged it as dead code (0 views wired); the genuine shared
seam already lives in DataTable + useDataTable + DataTableToolbar.

Validation: mvn compile + test-compile green; 37 targeted admin/submission
tests pass (incl. new AdminBulkExecutorTest); management eslint green on
auth store. code-review skill run (Standards pass, Spec gaps documented
in-code).

Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 16, 2026
…aborator

Architecture-review candidate #3 (safe subset). Extract the two pure
DTO <-> port-type translations (toRunTestCase, toPortResult) from
SandboxExecutorImpl into a package-private internal collaborator,
SandboxResultTranslator, so the outcome-translation logic (ADR-001
wire->enum status, ADR-002 §8 Layer-B memory-ceiling backstop via
SandboxOutcomeClassifier.applyMemoryCeiling, harness output/input echo)
concentrates in one internal module.

- SandboxResultTranslator is constructed inside SandboxExecutorImpl's
  constructor from collaborators it already holds (helper,
  outcomeClassifier) — no Spring DI, no new adapter seam, no public
  constructor signature change.
- The five call sites in SandboxExecutorImpl route through
  resultTranslator; drop the now-unused SubmissionStatusCodec import.
- Security-sensitive surface (docker command, seccomp, fork detection,
  process lifecycle) stays untouched in SandboxExecutorImpl, centrally
  owned per ADR-002. Broader lifecycle/command decomposition is
  intentionally deferred pending Docker E2E coverage.

Co-Authored-By: Claude <noreply@anthropic.com>
DavidHLP added a commit that referenced this pull request Jul 19, 2026
Architecture review candidate #3 — notification push seam. The intermediate
projection between NotificationIntent and transport-owned wire DTOs
(NotificationPayload on WebSocket today). Field set lives in the
notification module so transport adapters (SSE/FCM future) translate
to their own wire shape without touching intent code.

Wiring (intent.toPushMessage() + channel translates to NotificationPayload)
is deferred — the prior session documented that closing the seam is a
polymorphism-removal refactor without behavior gain; revisit only when
introducing a second transport.
DavidHLP pushed a commit that referenced this pull request Jul 19, 2026
…s, type-safe tests

Two-axis code review of the 13-commit arch-review-20260718 branch
emitted 6 hard + 2 judgement Standards findings and 6 missing/partial
+ 5 scope-creep + 4 looks-wrong Spec findings. This commit closes
all hard Standards findings, 1 missing Spec, and the volatile-comment
documentation hygiene; defers 1 missing (Cand1 SCAN aggregation)
and 1 judgement (settings emit helper) to keep the fix-up focused.

Standards (6 hard):
- useSolutionAuthoring.publish: add isPublishing ref + re-entry guard,
  wire SolutionsEditView so the submit button is disabled and shows
  'Publishing...' while a create/update is in flight. i18n key
  'solution.editor.publishing' added in both locales.
- CommunityMembershipServiceImpl.joinCommunity: log loud if
  incrementMembers matched no row, so a deleted-in-flight community
  cannot silently leave the new membership without a counter bump.
  Leave path already checks affected rows.
- settings.ts FieldMapping: type the wireKey / dtoKey constraint to
  keyof W / keyof D, so a misspelled mapping entry fails to compile
  rather than silently dropping the field at runtime. The inner
  as Partial<W> / as D casts remain — TypeScript cannot prove value
  compatibility, but every key was already validated by the generic
  constraint.
- useSolutionAuthoring.spec.ts: type the mockRoute as a real
  RouteLocationNormalized (not 'as never'); useContestAuthoring.test.ts:
  type the createContest / addProblem mock returns as Contest /
  ContestProblem. Both are now real typed mocks at the public boundary.
- CommunityMembershipServiceImplTest: pin the Clock to a fixed
  Instant + ZoneOffset.UTC and assert joinedAt deterministically.
  Replaces the previous Instant.now() / ZoneId.systemDefault() which
  drifted between CI machines.
- Volatile review references removed from comments:
  'Architecture review candidate #3' (system-settings.ts:47),
  'residual-risk note in the task report' (useContestAuthoring.ts:272),
  'CreateContestDTO.java ~line 55' (contests.ts:88 + :229),
  'arch review 2026-07-10' (separator/index.ts in both apps),
  'arch review Card 7' (i18n/utils.ts), 'arch review #3' (date.ts),
  'arch review #4' (datetime.ts), 'arch review candidate #3'
  (storage.ts). All replaced with self-contained descriptions.

Spec (1 missing addressed + 1 documented):
- Cand3 clearCache: replace untyped Map<String,Object> with a typed
  ClearCacheResponseVO(List<String> clearedScopes, String timestamp)
  on backend (impl + service + controller), and update the two
  pre-existing tests to match. The frontend type was already
  correct; the contract is now typed at the API boundary.
- Cand2 StepScoringRule: comment updated to document that the
  Selector owns the on-mount default-pick (which needs the rules
  endpoint) rather than duplicating the fetch in the authoring module.

Verified:
- backend-spring mvn -Dtest=... test: 54/54 green (forum 33 + admin 21)
- console pnpm vitest useSolutionAuthoring.spec.ts: 9/9
- management pnpm vitest useContestAuthoring.test.ts: 16/16
- pnpm type-check clean on both frontends

Deferred:
- Cand1 QueueHealthSnapshot SCAN aggregation for failedCount /
  completedCount (still hardcoded 0L with TODO). The inFlight field
  is also missing. Both are still acknowledged in the plan; deferred
  to keep this fix-up focused.
- J1 settings emit helper extraction (judgement smell): 5 settings
  components repeat emit('update:settings', { [key]: value }); left
  in place; refactor would touch all 5 view files for marginal DRY.

24 files / +224 / -82. No pre-existing dirty worktree files modified.
DavidHLP added a commit that referenced this pull request Jul 20, 2026
…eads

Architecture-review 2026-07-19 candidate #3 noted the projection
interface mirrored implementation choices: admin callers reached through
toSummaryVO and toAdminDetailVO conversion helpers, forcing the page
assembly mechanics to leak across the module boundary.

- Adds two intent-level admin reads on the public ProblemListProjection
  interface: findAdminLists(AdminProblemListQueryDTO) owns page
  normalization + filter-wrapper assembly + selectPage + entity→VO
  projection, returning PageResult<ProblemListSummaryVO>; and
  getAdminListDetail(String) owns the entity load (404 on missing) +
  admin-detail shaping. Mirrors the existing DefaultAdminContestProjection
  / DefaultAdminSubmissionProjection / DefaultAdminUserProjection shape
  (PaginationRequest default page-size 10, selectPage, shape, PageResult).
- Removes the cross-module toAdminDetailVO conversion helper from the
  interface and from the impl. The projection body is preserved verbatim
  in a private assembleAdminDetailVO(list) helper so future tweaks land
  in one place; getAdminListDetail delegates load + assemble.
- Rewires AdminProblemListServiceImpl to call the new intent reads;
  drops the now-unused ProblemListMapper field and seven unused imports.
- Cascade-fixes AdminProblemListServiceImplTest: drops the removed-mock
  references, updates getProblemLists / getProblemList tests to assert
  intent-read delegation, and adjusts the constructor-assertion to the
  new 3-param shape.
- Adds Phase A coverage in DefaultProblemListProjectionTest for the two
  new intent reads: FindAdminListsTests (happy-path + empty) and
  GetAdminListDetailTests (happy-path + not-found).

Conversion helpers (toSummaryVO / toSummaryVOWithSavedStatus /
toCategorySummaryVO) remain on the interface because the in-module write
state machine legitimately needs them to shape create / update / fork /
category-CRUD return values.

Validation: backend mvnw verify (1744 tests, JaCoCo threshold met);
admin problem-list suite (22 tests) and projection suite (7 tests) green.
DavidHLP added a commit that referenced this pull request Jul 20, 2026
Architecture-review 2026-07-19 candidate #3 noted 'effective tests
cover only part of that interface'. Phase A added coverage for the two
new admin intent reads (findAdminLists / getAdminListDetail). Phase B
closes the remaining pre-existing coverage gap on the rest of the
ProblemListProjection surface so the candidate's test-coverage
complaint is fully resolved.

Adds eight nested test classes / cases:
- FindAllTests: unauthenticated overview populates featured + public
  lists with empty saved/categories.
- GetUserProblemListsTests: own + saved + featured + categories sections
  all populated, with saved-list isSaved=true on the bookmark path.
- GetListOverviewTests: access-control smoke tests — PROBLEM_LIST_NOT_FOUND
  when findById is empty; PROBLEM_LIST_PRIVATE when the list is private
  and the viewer is not the owner.
- ConversionHelperTests: toSummaryVO problem-count + author enrichment;
  toSummaryVOWithSavedStatus saved=true (bookmark exists) and saved=false
  (unauthenticated, null userId); toCategorySummaryVO list-count
  enrichment from problemListBookmarkMapper.findByCategoryId.

Lifts a shared listEntity helper to the outer test class so all nested
classes can build entities without duplicating field sets.

Validation: DefaultProblemListProjectionTest 15 tests green (4 Phase A +
8 Phase B + 3 pre-existing); full mvnw verify (1744 tests, JaCoCo
threshold met).
DavidHLP added a commit that referenced this pull request Jul 20, 2026
…eview fix)

Code-review on the C3 narrowing flagged a hard Standards violation:
ProblemListProjection (feature module) had grown an import of
AdminProblemListQueryDTO (admin module), creating a reverse
problemlist → admin.dto dependency. The repo convention (verified
against DefaultAdminContestProjection, DefaultAdminSubmissionProjection,
DefaultAdminUserProjection) is admin projections live in
admin/projection/ and depend inward on feature mappers + entities —
never the reverse.

This commit restores the convention:

- Adds admin/projection/AdminProblemListProjection and
  DefaultAdminProblemListProjection mirroring the existing admin
  projection series. Owns findAdminLists(query) + getAdminListDetail(id),
  including page-assembly, entity load, and entity→VO projection.
  Dependency direction is admin → feature (mapper + entity only).
- Reverts the feature-side ProblemListProjection to drop
  toAdminDetailVO and the two admin intent reads (findAdminLists /
  getAdminListDetail). The candidate #3 complaint about cross-module
  conversion-helper leakage stays closed — admin no longer reaches into
  feature-side projection mechanics.
- Reverts DefaultProblemListProjection to pre-C3 shape minus
  toAdminDetailVO (which candidate #3 explicitly called out as
  leakage).
- Rewires AdminProblemListServiceImpl to inject AdminProblemListProjection
  instead of ProblemListProjection.
- Moves the 4 admin-read tests (FindAdminListsTests,
  GetAdminListDetailTests) from DefaultProblemListProjectionTest to a
  new DefaultAdminProblemListProjectionTest. Drops now-unused imports.
- Drops 5 hollow noDirectMapperMutation tests in
  AdminProblemListServiceImplTest: their verify(problemListMapper, never())
  assertions referenced a field stripped from production in C3, leaving
  the test bodies as mutation-only no-ops. The structural invariant
  (no ProblemListMapper / ProblemListProblemMapper field in the
  constructor) is now pinned by the existing
  architecturalInvariant_noProblemMapperDependency test, which also
  adds ProblemListMapper to its doesNotContain assertion.

Validation: backend mvnw verify BUILD SUCCESS, JaCoCo threshold met;
affected suites green — AdminProblemListServiceImplTest 17,
DefaultProblemListProjectionTest 11, DefaultAdminProblemListProjectionTest 4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant