Skip to content

feat(fonts): add workspace font pipeline and resources UI#190

Open
sharkAndshark wants to merge 15 commits intomainfrom
feat/font-service-resources-ui
Open

feat(fonts): add workspace font pipeline and resources UI#190
sharkAndshark wants to merge 15 commits intomainfrom
feat/font-service-resources-ui

Conversation

@sharkAndshark
Copy link
Copy Markdown
Owner

Summary

  • Add backend font service with workspace-scoped CRUD, async TTF/OTF -> PBF glyph generation, publish/unpublish, and public glyph endpoint support.
  • Add frontend resources section and fonts management UI, including upload/list/detail/publish flows and i18n keys for zh/en.
  • Extract resource sub-tabs into dedicated panels and add unit tests for resources tab behavior.

Why

  • Enable map style glyph hosting as a first-class managed resource with workspace isolation and controlled public publishing.
  • Prepare the resources information architecture (fonts/icons/styles) while delivering fonts end-to-end first.

Testing

  • cargo clippy --manifest-path backend/Cargo.toml --all-targets --all-features
  • cargo test --manifest-path backend/Cargo.toml
  • cargo fmt --manifest-path backend/Cargo.toml -- --check
  • npm --prefix frontend run format:check
  • npm --prefix frontend run test:unit
  • npm --prefix frontend run build
  • npm --prefix frontend run test:e2e

Introduce end-to-end font management with upload, async PBF glyph generation, publish/unpublish, and public glyph serving, while adding a resources section in the frontend with fonts as the first managed asset.
Drop inline Chinese defaultValue strings from main tab translations so the i18n CI detector no longer flags hardcoded Chinese text.
Copy link
Copy Markdown
Owner Author

@sharkAndshark sharkAndshark left a comment

Choose a reason for hiding this comment

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

Round 1 self-review complete.\n\nWhat I checked:\n- CI failure root cause: frontend i18n detector flagged hardcoded Chinese fallback literals in App main tab labels.\n- Fix: removed Chinese fallbacks and rely on locale keys (, ) present in both and .\n- Re-validation: local === i18n Detection ===

Check 1: Hardcoded Chinese strings...
�[0;32mPASS: No hardcoded Chinese strings found�[0m

Check 2: t() key completeness...
�[0;32mPASS: All t() keys exist in both locale files�[0m

=== Summary ===
�[0;32mPASSED: All i18n checks passed�[0m now passes; unit tests and formatting checks pass.\n\nNo further code issues found in this round.

Copy link
Copy Markdown
Owner Author

@sharkAndshark sharkAndshark left a comment

Choose a reason for hiding this comment

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

Round 1 self-review complete.

What I checked:

  • CI failure root cause: frontend i18n detector flagged hardcoded Chinese fallback literals in App main tab labels.
  • Fix: removed defaultValue Chinese fallbacks and rely on locale keys (app.mainTabData, app.mainTabResources) present in both en.json and zh.json.
  • Re-validation: local bash scripts/ci/check_i18n.sh now passes; unit tests and formatting checks pass.

No further code issues found in this round.

Copy link
Copy Markdown
Owner Author

@sharkAndshark sharkAndshark left a comment

Choose a reason for hiding this comment

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

Round 2 self-review complete.

What I verified after the i18n fix commit:

  • All CI checks are green (backend tests/clippy/fmt/postgis integration, frontend unit/e2e, dependency-review).
  • The previous frontend_unit failure is resolved by removing hardcoded Chinese fallback literals.
  • Local parity checks remain green, including scripts/ci/check_i18n.sh.

No further changes required from my side.

@sharkAndshark
Copy link
Copy Markdown
Owner Author

Addressed the two high-priority review findings in c7e095e:\n\n- API contract fix: font responses now serialize in camelCase (, ) to match frontend expectations (, , , , etc.).\n- Security hardening: public glyph endpoint now strictly validates range format (), rejects slash/backslash patterns, canonicalizes resolved file path, and enforces upload-dir containment before file reads.\n- Added backend unit tests for both range parsing and camelCase serialization regression coverage.\n\nValidation: pre-commit + pre-push suites passed (cargo test/clippy/fmt, frontend unit, frontend e2e).

@sharkAndshark
Copy link
Copy Markdown
Owner Author

Addressed the two high-priority review findings in c7e095e:

  • API contract fix: font responses now serialize in camelCase (FontItem, PublishFontResponse) to match frontend expectations (glyphCount, startCp, isPublic, createdAt, etc.).
  • Security hardening: public glyph endpoint now strictly validates range format (<start>-<end>[.pbf]), rejects slash/backslash patterns, canonicalizes resolved file path, and enforces upload-dir containment before file reads.
  • Added backend unit tests for both range parsing and camelCase serialization regression coverage.

Validation: pre-commit + pre-push suites passed (cargo test/clippy/fmt, frontend unit, frontend e2e).

@sharkAndshark
Copy link
Copy Markdown
Owner Author

Follow-up fix from review applied in 2cd7b99:

  • Added startup reconciliation for font jobs (reconcile_processing_fonts) so interrupted background font processing is marked failed after restart instead of staying processing forever.
  • Wired reconciliation into both server entrypoints:
    • backend/src/main.rs
    • backend/src/bin/mapflow-desktop.rs
  • Exported new reconciliation helper from backend/src/lib.rs.

Validation passed via hooks and reruns (backend tests/clippy/fmt, frontend unit, frontend e2e).

@sharkAndshark
Copy link
Copy Markdown
Owner Author

Applied the latest review findings in 1f9b454:

  • Public glyph visibility parity: get_public_glyph now only returns glyphs when the owning workspace is active (workspaces.deleted_at IS NULL), matching public tile endpoint behavior.
  • Test-mode auth parity: font handlers now support MAPFLOW_TEST_MODE=1 fallback workspace resolution when auth session is absent, aligned with file handlers.

Validation passed through hooks and reruns: backend fmt/clippy/tests + frontend unit/e2e.

@sharkAndshark
Copy link
Copy Markdown
Owner Author

按你的要求,我把这轮 review 相关修复做了完整解释性补强,并补上了回归测试,已同步到 PR。

新增测试(commit 8922915):

  • test_public_font_glyphs_blocked_when_workspace_archived

    • 构造一个 active workspace 与一个 archived workspace,各自挂一个公开字体与可读 glyph 文件。
    • 断言 active workspace 的 glyph URL 返回 200。
    • 断言 archived workspace 的 glyph URL 返回 404。
    • 这个用例直接覆盖了 public glyph 端点是否正确继承“归档工作空间不可公开访问”的契约。
  • test_fonts_api_supports_test_mode_without_auth

    • build_test_router + MAPFLOW_TEST_MODE=1 下,不带认证请求 /api/fonts
    • 断言返回 200 且 JSON 列表可解析。
    • 这个用例锁定了 fonts handler 与 files handler 在 test-mode 下的行为一致性。

验证结果:

  • backend: cargo test(含 api_tests 全量 149 项)、cargo clippy 通过
  • frontend: unit + e2e 通过(由 hooks 触发)

@sharkAndshark
Copy link
Copy Markdown
Owner Author

按你的要求,已使用开源字体补齐自动化测试(不依赖手工测试):

  • 新增开源字体 fixture:
    • backend/tests/fixtures/fonts/PressStart2P-Regular.ttf(Google Fonts / OFL)
    • backend/tests/fixtures/fonts/OFL.txt
  • 新增后端集成测试:backend/tests/font_api_tests.rs
    • test_font_upload_publish_and_public_glyph_lifecycle
    • 覆盖完整链路:上传字体 -> 等待异步处理 ready -> 发布 -> 访问公开 glyph -> 非法 range 返回 400 -> 取消发布后公开 URL 返回 404

提交:4001b23

验证:

  • backend 全量测试通过(含新 font_api_tests
  • frontend unit/e2e 通过(hooks 执行)

@sharkAndshark
Copy link
Copy Markdown
Owner Author

继续按“破坏可发现性”目标补强了字体自动化回归(commit 962bae2):

新增/补强用例(backend/tests/font_api_tests.rs):

  • test_publish_font_rejects_invalid_slug:非法 slug 400
  • test_publish_font_rejects_slug_conflict:slug 冲突 400
  • test_publish_font_rejects_non_ready_status:非 ready 状态发布 409
  • test_delete_font_removes_public_access:删除后 public glyph 404 且字体目录被清理
  • test_list_and_get_font_use_camel_case_contractglyphCount/isPublic/createdAt 等 camelCase 契约断言
  • test_upload_font_rejects_unsupported_extension:非 ttf/otf 上传 400
  • 保留并复用已有 E2E 主链路测试:upload -> ready -> publish -> public glyph -> invalid range -> unpublish

验证结果:

  • backend 全量 cargo test 通过(含 font_api_tests 7 项)
  • frontend unit + e2e 通过(hooks 执行)

这轮后,字体模块对于“主链路 + 关键失败分支 + 字段契约回退”的破坏检出能力显著提升。

…aram

- Merge status check into UPDATE WHERE clause for atomic operation
- Remove unused onProgress parameter from uploadFont function
…red helpers, improve robustness

- Use CURRENT_TIMESTAMP for font created_at and published_at instead of
  Rust-side Utc::now(), consistent with the rest of the codebase
- Extract test-mode workspace bootstrap from font_handlers.rs and
  upload.rs into workspace::ensure_test_mode_workspace(), eliminating
  ~100 lines of duplicated code
- Extract read_font_row() helper to deduplicate the 13-column FontItem
  row mapping used in list_fonts and get_font
- Log DB errors in update_font_error instead of silently discarding them
- Save uploaded font with original file extension (e.g. original.ttf)
  for easier filesystem inspection
…workspace archive

Defense-in-depth: LIMIT 1 prevents query_row().optional() from erroring
if multiple rows somehow match the public glyph query.

Workspace archive now unpublishes fonts (is_public=FALSE, slug=NULL,
published_at=NULL) in the same transaction as file unpublish, closing
a gap where fonts remained publicly accessible after archival.
… dynamic path computation

Fix 1: upload_font now returns 201 Created (matching upload_file semantics).
Fix 2: Extract get_active_workspace_id() into workspace.rs — eliminates ~60
lines of duplicated workspace authorization logic between font_handlers and upload.
Fix 3: Font stored paths are now computed dynamically relative to upload_dir
instead of hardcoding './uploads/' prefix, preventing silent breakage if upload
directory config changes.
…fix start, use index names for error classification

Fix 1: upload_font now cleans up uploaded file and directory if DB INSERT fails,
preventing orphan files on disk.

Fix 2: Unify slug collision suffix to start at 1 (was 2 in workspace_handlers.rs),
consistent with db.rs.

Fix 3: Replace fragile string-matching error classification (err.contains("slug"),
err.contains("UNIQUE")) with explicit index name checks
(idx_workspaces_slug, idx_fonts_workspace_slug). DuckDB error messages include
constraint/index names, making this more robust than generic keyword matching.
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