From 1a7ffbe6e20c618351b3fe5356d0c9852949c705 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:15:35 +0800 Subject: [PATCH] feat(avatar): stage images before skill publication Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/26-skill-avatars.md | 10 +- docs/skillhub/guide/skill-publish.md | 4 + .../design.md | 16 + .../proposal.md | 4 +- .../specs/skill-avatars/spec.md | 53 ++- .../add-skill-avatars-and-moderation/tasks.md | 14 + .../portal/SkillAvatarController.java | 32 +- .../portal/SkillPublishController.java | 16 +- .../dto/SkillAvatarSubmitRequest.java | 5 + .../dto/SkillAvatarUploadResponse.java | 6 + .../SkillAvatarSubmissionListener.java | 28 ++ .../service/SkillAvatarAppService.java | 104 +++++- .../service/SkillAvatarUploadService.java | 139 +++++++ .../service/SkillPublishAppService.java | 61 +++ .../skillhub/task/SkillAvatarCleanupTask.java | 10 +- .../V71__staged_skill_avatar_uploads.sql | 9 + .../src/main/resources/messages.properties | 4 + .../src/main/resources/messages_ru.properties | 4 + .../src/main/resources/messages_zh.properties | 4 + .../SkillAvatarRecoveryPostgresTest.java | 4 + .../SkillAvatarUploadPostgresTest.java | 351 ++++++++++++++++++ .../service/SkillAvatarAppServiceTest.java | 36 +- .../policy/RouteSecurityPolicyRegistry.java | 4 + .../domain/skill/SkillAvatarUpload.java | 43 +++ .../skill/service/SkillPublishService.java | 25 ++ ...illStorageDeletionCompensationService.java | 7 + .../jpa/SkillAvatarUploadJpaRepository.java | 22 ++ web/src/api/client.ts | 15 +- web/src/api/generated/schema.d.ts | 216 ++++++++++- web/src/app/router.tsx | 4 + .../skill/skill-avatar-editor.test.tsx | 106 ++++++ .../features/skill/skill-avatar-editor.tsx | 100 +++++ .../skill/skill-avatar-manager.test.tsx | 30 +- .../features/skill/skill-avatar-manager.tsx | 74 ++-- web/src/i18n/locales/en.json | 18 + web/src/i18n/locales/ru.json | 18 + web/src/i18n/locales/zh.json | 18 + web/src/pages/dashboard/my-skills.test.ts | 4 + web/src/pages/dashboard/my-skills.tsx | 22 +- .../pages/dashboard/publish-avatar.test.tsx | 83 +++++ web/src/pages/dashboard/publish.test.ts | 5 + web/src/pages/dashboard/publish.tsx | 28 +- web/src/pages/skill-detail.tsx | 2 +- web/src/shared/hooks/use-skill-queries.ts | 5 +- 44 files changed, 1635 insertions(+), 128 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarSubmitRequest.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarUploadResponse.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarSubmissionListener.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarUploadService.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillPublishAppService.java create mode 100644 server/skillhub-app/src/main/resources/db/migration/V71__staged_skill_avatar_uploads.sql create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarUpload.java create mode 100644 server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarUploadJpaRepository.java create mode 100644 web/src/features/skill/skill-avatar-editor.test.tsx create mode 100644 web/src/features/skill/skill-avatar-editor.tsx create mode 100644 web/src/pages/dashboard/publish-avatar.test.tsx diff --git a/docs/26-skill-avatars.md b/docs/26-skill-avatars.md index 8854e55a..5dc7810f 100644 --- a/docs/26-skill-avatars.md +++ b/docs/26-skill-avatars.md @@ -1,6 +1,14 @@ # 技能头像与审核 -技能所有者可在技能详情页上传 PNG/JPEG，网页提供方形裁剪和缩放。服务器仍会独立校验：输入不超过 2 MB、边长不超过 1024 像素，并重新编码为 PNG，避免依赖客户端裁剪或保留原图元数据。头像候选存在私有对象存储；审核完成前，公众继续看到旧头像或技能名首字母。 +技能所有者可在新增／更新技能页面选择自定义 PNG/JPEG 头像，网页提供方形裁剪和缩放；不提供预设图标。更新默认保留头像，也可替换或移除。“我的技能”提供单独修改头像的入口，无需发布新版本；详情页展示头像及审核状态。服务器仍会独立校验：输入不超过 2 MB、边长不超过 1024 像素，并重新编码为 PNG，避免依赖客户端裁剪或保留原图元数据。头像候选存在私有对象存储；审核完成前，公众继续看到旧头像或技能名首字母。 + +## 上传与提交 + +选择图片后先裁剪并上传到私有存储，上传成功后才允许提交技能或头像修改；未选择头像也可提交新技能。上传失败可重试，但不能把失败或尚未完成的上传随技能提交。上传成功仅表示文件已保存，不表示审核通过。 + +提交成功后图片才关联到技能并进入对应审核流程。更新时未修改头像会保留原头像；选择移除时先预览首字母，提交成功后才恢复兜底并取消待审头像申请，历史记录保留。取消编辑或提交失败不会改变原头像及已有待审申请。未关联的临时图片保持私有；上传引用 24 小时后过期，过期未关联图片由清理任务定期回收，提交时过期需要重新上传。 + +技能内容与头像分别显示审核状态。技能发布或内容审核通过不能代替头像审核；新头像通过审核前仍显示旧头像或首字母。单独修改头像不创建新的技能版本。 ## 审核策略 diff --git a/docs/skillhub/guide/skill-publish.md b/docs/skillhub/guide/skill-publish.md index 8d77fcf8..34e21100 100644 --- a/docs/skillhub/guide/skill-publish.md +++ b/docs/skillhub/guide/skill-publish.md @@ -91,6 +91,10 @@ npx @astron-team/skillhub@latest publish ./my-skill --namespace my-team 访问 `http://localhost:3000/dashboard/publish`，选择命名空间、上传 zip 文件、选择可见性后点击「发布」。 +可选上传自定义技能头像：选择 PNG/JPEG、裁剪并等待私有上传成功，再提交技能。头像上传中或失败时不能提交；没有选择头像则正常提交并使用技能名首字母。更新技能时默认保留原头像，可选择替换或移除；移除仅在提交成功后生效，取消或提交失败不影响原头像。 + +技能和头像各有审核状态。文件上传成功不代表头像审核通过，头像所需审核在提交后进行，通过前显示旧头像或首字母。仅需修改头像时，可从“我的技能”操作，不需要发布新的技能版本。 + 4. **使用 REST API 发布** ```bash diff --git a/openspec/changes/add-skill-avatars-and-moderation/design.md b/openspec/changes/add-skill-avatars-and-moderation/design.md index 9e668b08..7fe8a300 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/design.md +++ b/openspec/changes/add-skill-avatars-and-moderation/design.md @@ -20,3 +20,19 @@ - V68 后迁移在已有数据上通过；旧 Skill 无头像时无需回填。 - 服务端图片解码处理需设置资源上限；机器供应商凭证不得进入日志、数据库或前端。 - 原 Issue 的“机器不得直接拒绝”与本轮方案不同。机器单开模式的质量和误判处理需在运行验证中单列，不以外部 API 返回成功代替有效审核验证。 + + +## 已确认的发布交互调整（2026-10-13，待实施） + +本节补充原实现，需求以 `specs/skill-avatars/spec.md` 的 REQ-AV-09/10/11 为准。 + +- 新增和更新页面使用可选自定义上传，不提供预设图标；更新默认保留，支持替换及移除。 +- “我的技能”提供单独修改头像入口，不要求新版本；详情页保留展示与审核状态，移除完整上传表单。统一复用裁剪和上传组件。 +- 先私有上传、后业务提交：新增技能尚无 Skill ID，临时上传不能提前创建已绑定的头像申请。上传引用绑定提交者并校验可用性，临时阶段不改变当前头像或待审申请。 +- 技能提交成功与头像引用关联须共同成功。失败不改变当前头像或已有候选；数据库关联与业务状态共同提交，外部对象写入提前完成，不把对象存储或供应商请求放入长事务。重复提交不重复创建申请。 +- 头像审核在关联提交后执行，遵循原可见范围和策略规则；机器和人工开关在候选创建时快照。上传成功不意味着审核通过，技能与头像分别展示审核状态。 +- 更新页移除仅在提交成功后执行，立即恢复兜底并使待审候选失效，申请历史和失效图片保留策略不变。 +- 未关联临时图片始终私有，按有界任务定期清理；关联与清理并发须保证不会删除已绑定图片。临时引用使用独立 `skill_avatar_upload` 表，24 小时过期；上传接口为 `/api/web/skills/avatar-uploads`，提交技能携带 `avatarAction`（默认 KEEP，可选 REPLACE/REMOVE）和替换时的 `avatarUploadId`。单独维护使用现有头像路径的 JSON 提交；更新携带 `expectedSlug` 校验技能包目标。清理和引用消费均锁定临时上传记录，发布与独立修改均采用技能→上传记录的锁顺序。不引入业务状态 SQL CHECK 约束。 +- 唯一机器供应商的部署选择、后台两个审核开关、PRIVATE 直接使用规则保持不变。 + +原验收只覆盖详情页独立提交流程，不能证明上述新增／更新流程；实施后须补充对应测试和浏览器验收。真实供应商调用仍未验证。 diff --git a/openspec/changes/add-skill-avatars-and-moderation/proposal.md b/openspec/changes/add-skill-avatars-and-moderation/proposal.md index d8cb2f5c..77a74b4c 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/proposal.md +++ b/openspec/changes/add-skill-avatars-and-moderation/proposal.md @@ -5,7 +5,9 @@ Skill 目前没有稳定的图像标识，卡片和详情难以快速区分。 ## What Changes - 为个人、团队和全局 Skill 增加由 Skill 所有者维护的可选头像；所有展示位置使用一致的首字兜底。 -- 上传最终裁剪图片，经服务端规范化后存为私有候选；头像属于 Skill，不随版本更新重置。 +- 在新增／更新技能页面支持可选自定义头像与裁剪，先私有上传成功后随业务提交关联；更新默认保留，可替换或移除，不提供预设图标。 +- “我的技能”支持单独修改头像，无需新版本；详情页展示头像和审核状态。头像属于 Skill，未明确替换或移除时不随版本更新重置。 +- 临时图片与正式申请分离，未关联图片定期清理；技能与头像分别显示审核状态。 - 按实际可见范围处理审核：仅本人可见的 PRIVATE 头像可直接使用；团队可见或公开头像按部署方的机器、人工审核开关处理；全局头像由平台管理员审核。 - 部署时选择唯一机器审核供应商并配置凭证，提供讯飞和阿里云适配；后台仅管理审核开关。机器失败不放行，机器误判可申请人工复核。 - 仅 Skill 所有者可提交、更换或主动删除；管理员可以审核、复核和撤下违规头像。旧候选失效后不能被迟到的审核结果激活。 diff --git a/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md b/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md index a47375e0..8f97d8d0 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md +++ b/openspec/changes/add-skill-avatars-and-moderation/specs/skill-avatars/spec.md @@ -8,14 +8,14 @@ Skill SHALL 支持可选头像和统一兜底。 -头像属于 Skill 容器而非 SkillVersion。摘要与详情只返回当前已激活的稳定头像访问地址；无头像时所有 Skill 展示位置使用展示名称首字符的大写兜底。新版本发布不得重置头像。 +头像属于 Skill 容器而非 SkillVersion。摘要与详情只返回当前已激活的稳定头像访问地址；无头像时所有 Skill 展示位置使用展示名称首字符的大写兜底。新版本发布未明确选择替换或移除头像时不得重置头像。 #### Scenario: 旧 Skill 没有头像 - **WHEN** 客户端读取无头像的 Skill - **THEN** 不需数据回填即可展示一致的首字兜底 #### Scenario: 发布新版本 -- **WHEN** 已有头像的 Skill 发布新版本 +- **WHEN** 已有头像的 Skill 发布新版本，且未选择替换或移除头像 - **THEN** 原头像保持不变 ### Requirement: REQ-AV-02 仅 Skill 所有者 SHALL 维护头像 @@ -110,8 +110,55 @@ PRIVATE 且仅所有者可访问的头像完成文件安全处理后可直接使 前后端 SHALL 提供可理解的头像提交与审核闭环。 -所有者可以在 Skill 详情查看当前头像、提交候选、状态和驳回原因；审核员可以读取其管辖范围候选并处理，机器单开时可以处理例外申诉。所有展示 Skill 身份的位置使用同一头像与兜底组件。 +新增和更新 Skill 页面提供可选自定义头像、方形裁剪、上传预览及上传状态，不提供预设图标选择；未上传时使用首字母兜底。更新页面展示当前头像并提供保留、替换、移除三种动作，默认保留。“我的技能”提供单独修改头像的入口，复用同一套上传与裁剪组件，不要求发布新的 SkillVersion。详情页展示头像、审核状态和驳回原因，不再承载完整上传表单。技能内容与头像分别显示提交和审核状态，技能内容审核通过不代表头像审核通过。审核员可以读取其管辖范围候选并处理，机器单开时可以处理例外申诉。所有展示 Skill 身份的位置使用同一头像与兜底组件。 #### Scenario: 申请等待审核 - **WHEN** 所有者上传候选后需人工决定 - **THEN** 页面展示待审状态且已发布头像维持不变 + + +### Requirement: REQ-AV-10 头像 SHALL 先私有上传再随提交关联 + +新增、更新或单独修改头像时，客户端 SHALL 先裁剪并上传图片到私有存储，上传成功后取得可用于提交的临时引用。选择了新图片但仍在上传或上传失败时，客户端不得提交该次操作；服务端亦须校验引用对应的文件上传成功、未过期、属于当前提交者且不可越权关联。未选择头像的新技能可正常提交。 + +上传成功不等于审核通过。临时上传不改变 Skill 当前头像、待审申请或公开状态；技能提交成功时才关联上传引用并创建头像申请，按当时实际可见范围及审核配置执行，PRIVATE 仍沿用直接使用规则。需要的机器或人工审核在提交后进行，不要求用户停留在发布表单等待审核。关联失败不得留下已成功提交技能但丢失本次头像操作的部分结果；重复提交同一引用不得产生重复申请。 + +未关联、过期或用户放弃提交的临时图片保持私有并定期清理；清理不得删除已成功关联的候选或当前头像。临时图片仅供上传者预览，不因已取得引用而允许公众读取。 + +#### Scenario: 上传失败时提交 +- **WHEN** 所有者选择了新头像，但图片尚未上传成功 +- **THEN** 页面明确显示上传状态或失败原因，并阻止提交，允许重试 +- **AND** 服务端拒绝不存在、未就绪或无权使用的临时引用 + +#### Scenario: 新技能同时提交头像 +- **WHEN** 所有者上传成功并提交有效技能包和头像引用 +- **THEN** 技能提交与头像关联共同成功，头像进入其所需审核流程 +- **AND** 页面分别显示技能和头像的审核状态，候选未通过所需审核时不公开 + +#### Scenario: 取消编辑或提交失败 +- **WHEN** 图片已上传，但所有者取消编辑或技能提交失败 +- **THEN** 当前头像与现有待审申请不变，未关联临时图片可由清理任务回收 + +#### Scenario: 使用他人的上传引用 +- **WHEN** 提交者尝试关联他人的临时图片引用 +- **THEN** 服务端拒绝提交，不更改技能或头像状态 + +#### Scenario: 单独修改头像 +- **WHEN** 所有者从“我的技能”上传并提交新头像 +- **THEN** 复用相同上传、关联及审核流程，不创建新的 SkillVersion + +### Requirement: REQ-AV-11 更新页移除 SHALL 在提交成功后生效 + +更新页的移除操作 SHALL 先提供首字母兜底预览，仅在提交成功后执行。取消编辑或提交失败时，原头像及待审申请不变。执行移除无需头像审核，立即恢复兜底，并取消尚未完成的头像申请；旧机器结果或审核动作不得恢复已移除头像。图片私有保留与申请历史沿用 REQ-AV-08。 + +#### Scenario: 成功提交移除 +- **WHEN** 所有者选择移除头像并成功提交更新 +- **THEN** Skill 恢复首字母兜底，现有待审头像申请失效且历史保留 + +#### Scenario: 移除后旧结果到达 +- **WHEN** 移除已成功执行，而旧申请的机器结果或人工审核动作随后到达 +- **THEN** 已移除头像不被恢复 + +#### Scenario: 放弃移除或提交失败 +- **WHEN** 所有者选择移除，但取消编辑或提交失败 +- **THEN** 原头像及待审申请保持原状 diff --git a/openspec/changes/add-skill-avatars-and-moderation/tasks.md b/openspec/changes/add-skill-avatars-and-moderation/tasks.md index 17d48d70..5d3a4a15 100644 --- a/openspec/changes/add-skill-avatars-and-moderation/tasks.md +++ b/openspec/changes/add-skill-avatars-and-moderation/tasks.md @@ -23,3 +23,17 @@ - [x] 4.3 独立 Review、Draft PR、CI 与验收报告。 验证结果以 PR #947 的检查和交付验收记录为准。真实讯飞/阿里云计费审核接口未发起调用。 + + +## 5. 发布交互调整（2026-10-13，已确认需求，待实施） + +前述勾选项表示原实现及其验证完成，不代表本节完成。PR #947 的原合并就绪结论需要在本节实施后重新验证；当前预览仍为原详情页上传流程。 + +- [x] 5.1 实现所有者临时私有上传、预览、引用校验及未关联文件清理，覆盖越权引用与关联／清理并发。 +- [x] 5.2 扩展技能提交以处理保留、关联新头像和移除；保证技能提交与头像操作共同成功、失败不改变旧状态、重复提交不重复申请。 +- [x] 5.3 在新增／更新页接入统一自定义上传裁剪组件，上传未成功禁止提交，默认保留旧头像，支持移除预览。 +- [x] 5.4 在“我的技能”增加无需新版本的头像维护入口；详情页改为展示及审核状态，技能与头像状态分别呈现，更新三种语言文案。 +- [x] 5.5 覆盖新增、更新保留／替换／移除、取消、上传失败、提交失败、单独维护、旧审核失效、临时清理及原审核策略回归。 +- [ ] 5.6 更新相关用户文档、OpenAPI 类型及独立 Review，构建当前 SHA 的预览并进行真实浏览器验收，更新同一个 PR #947 的验收记录。 + +真实讯飞／阿里云接入是独立的部署联调项，需要已开通能力及部署凭证；模拟测试不得标记为真实调用通过。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillAvatarController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillAvatarController.java index df2efd81..b8d30ffb 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillAvatarController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillAvatarController.java @@ -11,6 +11,10 @@ import com.iflytek.skillhub.dto.SkillAvatarReasonRequest; import com.iflytek.skillhub.dto.SkillAvatarStatusResponse; import com.iflytek.skillhub.service.AuditRequestContext; import com.iflytek.skillhub.service.SkillAvatarAppService; +import com.iflytek.skillhub.service.SkillAvatarUploadService; +import com.iflytek.skillhub.dto.SkillAvatarUploadResponse; +import com.iflytek.skillhub.dto.SkillAvatarSubmitRequest; +import com.iflytek.skillhub.ratelimit.RateLimit; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import java.io.IOException; @@ -38,10 +42,36 @@ import org.springframework.web.multipart.MultipartFile; @RequestMapping({"/api/v1/skills", "/api/web/skills"}) public class SkillAvatarController extends BaseApiController { private final SkillAvatarAppService service; + private final SkillAvatarUploadService uploads; - public SkillAvatarController(SkillAvatarAppService service, ApiResponseFactory responses) { + public SkillAvatarController(SkillAvatarAppService service, SkillAvatarUploadService uploads, ApiResponseFactory responses) { super(responses); this.service = service; + this.uploads = uploads; + } + + @PostMapping(value = "/avatar-uploads", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) + @RateLimit(category = "publish", authenticated = 10, anonymous = 0) + public ApiResponse upload(@RequestParam("file") MultipartFile file, + @AuthenticationPrincipal PlatformPrincipal principal) throws IOException { + if (file.getSize() < 1 || file.getSize() > com.iflytek.skillhub.service.SkillAvatarImageProcessor.MAX_BYTES) { + throw new DomainBadRequestException("error.skill.avatar.invalidImage"); + } + return ok("response.success.created", uploads.upload(userId(principal), file.getBytes())); + } + + @GetMapping("/avatar-uploads/{uploadId}/image") + public ResponseEntity uploadPreview(@PathVariable String uploadId, + @AuthenticationPrincipal PlatformPrincipal principal) { + return png(uploads.preview(uploadId, userId(principal))); + } + + @PostMapping(value = "/{namespace}/{slug}/avatar", consumes = MediaType.APPLICATION_JSON_VALUE) + public ApiResponse submitUpload(@PathVariable String namespace, @PathVariable String slug, + @Valid @RequestBody SkillAvatarSubmitRequest input, @AuthenticationPrincipal PlatformPrincipal principal, + HttpServletRequest request) { + return ok("response.success.created", uploads.submit(namespace, slug, userId(principal), input.uploadId(), + AuditRequestContext.from(request))); } @GetMapping("/{namespace}/{slug}/avatar") diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java index a061afc1..9b9235e5 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java @@ -11,6 +11,9 @@ import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.PublishResponse; import com.iflytek.skillhub.metrics.SkillHubMetrics; +import com.iflytek.skillhub.service.SkillPublishAppService; +import com.iflytek.skillhub.service.AuditRequestContext; +import jakarta.servlet.http.HttpServletRequest; import com.iflytek.skillhub.ratelimit.RateLimit; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.*; @@ -29,11 +32,11 @@ import java.util.List; @RequestMapping({"/api/v1/skills", "/api/web/skills"}) public class SkillPublishController extends BaseApiController { - private final SkillPublishService skillPublishService; + private final SkillPublishAppService skillPublishService; private final SkillPackageArchiveExtractor skillPackageArchiveExtractor; private final SkillHubMetrics skillHubMetrics; - public SkillPublishController(SkillPublishService skillPublishService, + public SkillPublishController(SkillPublishAppService skillPublishService, SkillPackageArchiveExtractor skillPackageArchiveExtractor, ApiResponseFactory responseFactory, SkillHubMetrics skillHubMetrics) { @@ -54,7 +57,10 @@ public class SkillPublishController extends BaseApiController { @RequestParam("file") MultipartFile file, @RequestParam("visibility") String visibility, @RequestParam(value = "confirmWarnings", defaultValue = "false") boolean confirmWarnings, - @AuthenticationPrincipal PlatformPrincipal principal) throws IOException { + @RequestParam(value = "avatarAction", defaultValue = "KEEP") SkillPublishAppService.AvatarAction avatarAction, + @RequestParam(value = "avatarUploadId", required = false) String avatarUploadId, + @RequestParam(value = "expectedSlug", required = false) String expectedSlug, + @AuthenticationPrincipal PlatformPrincipal principal, HttpServletRequest request) throws IOException { SkillVisibility skillVisibility = SkillVisibility.valueOf(visibility.toUpperCase()); @@ -75,13 +81,13 @@ public class SkillPublishController extends BaseApiController { String.join("\n", extractionWarnings)); } - SkillPublishService.PublishResult publishResult = skillPublishService.publishFromEntries( + SkillPublishService.PublishResult publishResult = skillPublishService.publish( namespace, entries, principal.userId(), skillVisibility, principal.platformRoles(), - confirmWarnings + confirmWarnings, avatarAction, avatarUploadId, expectedSlug, AuditRequestContext.from(request) ); PublishResponse response = new PublishResponse( diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarSubmitRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarSubmitRequest.java new file mode 100644 index 00000000..73a40968 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarSubmitRequest.java @@ -0,0 +1,5 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.NotBlank; + +public record SkillAvatarSubmitRequest(@NotBlank String uploadId) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarUploadResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarUploadResponse.java new file mode 100644 index 00000000..6adaa66e --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarUploadResponse.java @@ -0,0 +1,6 @@ +package com.iflytek.skillhub.dto; + +import java.time.Instant; +import jakarta.validation.constraints.NotNull; + +public record SkillAvatarUploadResponse(@NotNull String uploadId, @NotNull String previewUrl, @NotNull Instant expiresAt) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarSubmissionListener.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarSubmissionListener.java new file mode 100644 index 00000000..fd379115 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarSubmissionListener.java @@ -0,0 +1,28 @@ +package com.iflytek.skillhub.listener; + +import com.iflytek.skillhub.service.SkillAvatarAppService; +import com.iflytek.skillhub.service.SkillAvatarUploadService.AvatarSubmittedEvent; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Component; +import org.springframework.transaction.event.TransactionPhase; +import org.springframework.transaction.event.TransactionalEventListener; + +@Component +public class SkillAvatarSubmissionListener { + private static final Logger log = LoggerFactory.getLogger(SkillAvatarSubmissionListener.class); + private final SkillAvatarAppService avatars; + public SkillAvatarSubmissionListener(SkillAvatarAppService avatars) { this.avatars = avatars; } + + @Async("skillhubEventExecutor") + @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) + public void submitted(AvatarSubmittedEvent event) { + try { + avatars.processMachine(event.candidateId(), event.context()); + } catch (Exception failure) { + log.warn("Avatar submission moderation failed for candidate {}, failureType={}", + event.candidateId(), failure.getClass().getSimpleName()); + } + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarAppService.java index fbb2c2ad..59621df3 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarAppService.java @@ -86,7 +86,7 @@ public class SkillAvatarAppService { @Transactional public SkillAvatarStatusResponse submit(String namespaceSlug, String slug, String actor, byte[] image, AuditRequestContext context) { - Skill skill = lockedSkill(namespaceSlug, slug); + Skill skill = lockedOwnedSkill(namespaceSlug, slug, actor); requireOwner(skill, actor); byte[] normalized = imageProcessor.normalize(image); return createCandidate(skill, namespaceSlug, actor, actor, "SKILL_AVATAR_SUBMIT", normalized, context); @@ -102,13 +102,12 @@ public class SkillAvatarAppService { } if (skill.getPendingAvatarId() != null) { SkillAvatarCandidate pending = candidates.findById(skill.getPendingAvatarId()).orElse(null); - if (pending != null && pending.getStatus() == SkillAvatarCandidate.Status.PENDING - && pending.getAudience() == skill.getVisibility()) { + if (pending != null && pending.getStatus() == SkillAvatarCandidate.Status.PENDING) { Instant now = Instant.now(); boolean abandonedBeforeClaim = pending.getMachineStatus() == SkillAvatarCandidate.MachineStatus.PENDING && pending.getCreatedAt() != null && !pending.getCreatedAt().plusSeconds(600).isAfter(now); - return ((pending.canRetryMachine(now) && pending.getMachineAttemptCount() < 3) || abandonedBeforeClaim) + return pending.getAudience() == skill.getVisibility() && ((pending.canRetryMachine(now) && pending.getMachineAttemptCount() < 3) || abandonedBeforeClaim) ? status(skill, pending, namespaceSlug(skill)) : null; } } @@ -135,11 +134,6 @@ public class SkillAvatarAppService { private SkillAvatarStatusResponse createCandidate(Skill skill, String namespaceSlug, String submittedBy, String auditActor, String action, byte[] normalized, AuditRequestContext context) { - AvatarReviewSettingsResponse policy = settings.current(); - boolean privateOnly = skill.getVisibility() == SkillVisibility.PRIVATE; - boolean machineEnabled = !privateOnly && policy.machineEnabled(); - boolean manualEnabled = !privateOnly && policy.manualEnabled(); - String provider = machineEnabled ? moderation.requireConfiguredProvider() : null; String sha256 = sha256(normalized); String key = "skill-avatars/" + skill.getId() + "/" + UUID.randomUUID() + ".png"; storage.putObject(key, new ByteArrayInputStream(normalized), normalized.length, "image/png"); @@ -151,11 +145,35 @@ public class SkillAvatarAppService { } }); + return createStoredCandidate(skill, namespaceSlug, submittedBy, auditActor, action, key, sha256, skill.getVisibility(), context); + } + + @Transactional + public void lockOwnedSkill(String namespaceSlug, String slug, String actor) { + requireOwner(lockedOwnedSkill(namespaceSlug, slug, actor), actor); + } + + @Transactional + public SkillAvatarStatusResponse submitStored(String namespaceSlug, String slug, String actor, + String objectKey, String sha256, SkillVisibility audience, AuditRequestContext context) { + Skill skill = lockedOwnedSkill(namespaceSlug, slug, actor); + requireOwner(skill, actor); + return createStoredCandidate(skill, namespaceSlug, actor, actor, "SKILL_AVATAR_SUBMIT", objectKey, sha256, + audience == null ? skill.getVisibility() : audience, context); + } + + private SkillAvatarStatusResponse createStoredCandidate(Skill skill, String namespaceSlug, String submittedBy, + String auditActor, String action, String key, String sha256, SkillVisibility audience, AuditRequestContext context) { + AvatarReviewSettingsResponse policy = settings.current(); + boolean privateOnly = audience == SkillVisibility.PRIVATE; + boolean machineEnabled = !privateOnly && policy.machineEnabled(); + boolean manualEnabled = !privateOnly && policy.manualEnabled(); + String provider = machineEnabled ? moderation.requireConfiguredProvider() : null; if (skill.getPendingAvatarId() != null) { candidates.findById(skill.getPendingAvatarId()).ifPresent(SkillAvatarCandidate::supersede); } SkillAvatarCandidate candidate = new SkillAvatarCandidate(skill.getId(), submittedBy, key, sha256, - skill.getVisibility(), machineEnabled, manualEnabled, provider); + audience, machineEnabled, manualEnabled, provider); if ("SKILL_AVATAR_SCOPE_REVIEW".equals(action)) { candidate.markScopeReviewVersion(skill.getLatestVersionId()); } @@ -184,6 +202,17 @@ public class SkillAvatarAppService { return status(skill, candidate, namespaceSlug); } + @Transactional(readOnly = true) + public SkillAvatarStatusResponse submittedStatus(String namespaceSlug, String slug, String actor, long candidateId) { + Skill skill = findOwnedSkill(namespaceSlug, slug, actor); + requireOwner(skill, actor); + SkillAvatarCandidate candidate = findCandidate(candidateId); + if (!skill.getId().equals(candidate.getSkillId())) { + throw notFound(); + } + return status(skill, candidate, namespaceSlug); + } + /** Claims and finishes in short transactions; the external call holds no database lock. */ public SkillAvatarStatusResponse processMachine(long candidateId, AuditRequestContext context) { MachineClaim claim = machineTransactions.execute(ignored -> { @@ -231,7 +260,7 @@ public class SkillAvatarAppService { @Transactional(readOnly = true) public SkillAvatarStatusResponse ownerStatus(String namespaceSlug, String slug, String actor) { - Skill skill = findSkill(namespaceSlug, slug); + Skill skill = findOwnedSkill(namespaceSlug, slug, actor); requireOwner(skill, actor); return status(skill, candidates.findBySkillIdOrderByCreatedAtDesc(skill.getId()).stream().findFirst().orElse(null), namespaceSlug); @@ -240,8 +269,12 @@ public class SkillAvatarAppService { @Transactional(readOnly = true) public InputStream readActive(String namespaceSlug, String slug, String actor, Map namespaceRoles, Set platformRoles) { - skillQueries.getSkillDetail(namespaceSlug, slug, actor, namespaceRoles == null ? Map.of() : namespaceRoles); - Skill skill = findSkill(namespaceSlug, slug); + SkillQueryService.SkillDetailDTO detail = skillQueries.getSkillDetail(namespaceSlug, slug, actor, + namespaceRoles == null ? Map.of() : namespaceRoles); + Skill skill = ((SkillRepository) skills).findById(detail.id()).orElseThrow(() -> notFound()); + if (skill.getStatus() != SkillStatus.ACTIVE) { + throw notFound(); + } if (!visibility.canAccess(skill, actor, namespaceRoles, platformRoles) || skill.getVisibility() == SkillVisibility.PRIVATE && !skill.getOwnerId().equals(actor) || SkillAvatarUrls.current(skill, namespaceSlug) == null) { @@ -257,8 +290,8 @@ public class SkillAvatarAppService { @Transactional(readOnly = true) public InputStream readCandidate(String namespaceSlug, String slug, long candidateId, String actor, Map namespaceRoles, Set platformRoles) { - Skill skill = findSkill(namespaceSlug, slug); SkillAvatarCandidate candidate = findCandidate(candidateId); + Skill skill = findCandidateSkill(namespaceSlug, slug, candidate); if (!skill.getId().equals(candidate.getSkillId()) || candidate.getObjectKey() == null || candidate.getAudience() == SkillVisibility.PRIVATE && !skill.getOwnerId().equals(actor) || !skill.getOwnerId().equals(actor) && !canReview(skill, actor, namespaceRoles, platformRoles)) { @@ -270,7 +303,7 @@ public class SkillAvatarAppService { @Transactional public SkillAvatarStatusResponse appeal(String namespaceSlug, String slug, long candidateId, String actor, AuditRequestContext context) { - Skill skill = lockedSkill(namespaceSlug, slug); + Skill skill = lockedOwnedSkill(namespaceSlug, slug, actor); requireOwner(skill, actor); SkillAvatarCandidate candidate = currentPending(skill, candidateId); candidate.appeal(); @@ -280,7 +313,7 @@ public class SkillAvatarAppService { @Transactional(readOnly = true) public void requireCurrentOwnerCandidate(String namespaceSlug, String slug, long candidateId, String actor) { - Skill skill = findSkill(namespaceSlug, slug); + Skill skill = findOwnedSkill(namespaceSlug, slug, actor); requireOwner(skill, actor); if (!Objects.equals(skill.getPendingAvatarId(), candidateId)) { throw new DomainConflictException("error.skill.avatar.notPending"); @@ -291,7 +324,7 @@ public class SkillAvatarAppService { public SkillAvatarStatusResponse decide(String namespaceSlug, String slug, long candidateId, boolean approved, String reason, String actor, Map namespaceRoles, Set platformRoles, AuditRequestContext context) { - Skill skill = lockedSkill(namespaceSlug, slug); + Skill skill = lockAndRefresh(findCandidateSkill(namespaceSlug, slug, findCandidate(candidateId))); requireReviewer(skill, actor, namespaceRoles, platformRoles); SkillAvatarCandidate candidate = currentPending(skill, candidateId); if (candidate.isMachineEnabled() && candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.PASSED @@ -317,7 +350,7 @@ public class SkillAvatarAppService { public void remove(String namespaceSlug, String slug, String actor, boolean byAdmin, String reason, Map namespaceRoles, Set platformRoles, AuditRequestContext context) { - Skill skill = lockedSkill(namespaceSlug, slug); + Skill skill = byAdmin ? lockedSkill(namespaceSlug, slug) : lockedOwnedSkill(namespaceSlug, slug, actor); if (byAdmin) { requireReviewer(skill, actor, namespaceRoles, platformRoles); if (reason == null || reason.isBlank()) { @@ -387,9 +420,40 @@ public class SkillAvatarAppService { .orElseThrow(() -> notFound()); } + private Skill findCandidateSkill(String namespaceSlug, String slug, SkillAvatarCandidate candidate) { + Namespace namespace = namespaces.findBySlug(namespaceSlug).orElseThrow(() -> notFound()); + Skill skill = ((SkillRepository) skills).findById(candidate.getSkillId()).orElseThrow(() -> notFound()); + if (!skill.getNamespaceId().equals(namespace.getId()) || !skill.getSlug().equals(slug) + || skill.getStatus() != SkillStatus.ACTIVE) { + throw notFound(); + } + return skill; + } + + private Skill findOwnedSkill(String namespaceSlug, String slug, String actor) { + if (actor == null) { + throw new DomainForbiddenException("error.skill.avatar.ownerOnly"); + } + Namespace namespace = namespaces.findBySlug(namespaceSlug).orElseThrow(() -> notFound()); + return skills.findByNamespaceIdAndSlugAndOwnerId(namespace.getId(), slug, actor) + .filter(skill -> skill.getStatus() == SkillStatus.ACTIVE) + .orElseThrow(() -> new DomainForbiddenException("error.skill.avatar.ownerOnly")); + } + + private Skill lockedOwnedSkill(String namespaceSlug, String slug, String actor) { + return lockAndRefresh(findOwnedSkill(namespaceSlug, slug, actor)); + } + private Skill lockedSkill(String namespaceSlug, String slug) { - Skill skill = findSkill(namespaceSlug, slug); - return skills.findLockedById(skill.getId()).orElseThrow(() -> notFound()); + return lockAndRefresh(findSkill(namespaceSlug, slug)); + } + + private Skill lockAndRefresh(Skill skill) { + Skill locked = skills.findLockedById(skill.getId()).orElseThrow(() -> notFound()); + // Publication may have local changes; flush them before refreshing a previously loaded row after the lock. + skills.flush(); + entityManager.refresh(locked); + return locked; } private SkillAvatarCandidate findCandidate(Long candidateId) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarUploadService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarUploadService.java new file mode 100644 index 00000000..7ccbcc3c --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillAvatarUploadService.java @@ -0,0 +1,139 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import com.iflytek.skillhub.domain.skill.SkillAvatarUpload; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.dto.SkillAvatarStatusResponse; +import com.iflytek.skillhub.dto.SkillAvatarUploadResponse; +import com.iflytek.skillhub.infra.jpa.SkillAvatarUploadJpaRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Instant; +import java.util.HexFormat; +import java.util.UUID; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +@Service +public class SkillAvatarUploadService { + private final SkillAvatarUploadJpaRepository uploads; + private final SkillAvatarImageProcessor images; + private final ObjectStorageService storage; + private final SkillAvatarAppService avatars; + private final ApplicationEventPublisher events; + + public SkillAvatarUploadService(SkillAvatarUploadJpaRepository uploads, SkillAvatarImageProcessor images, + ObjectStorageService storage, SkillAvatarAppService avatars, ApplicationEventPublisher events) { + this.uploads = uploads; + this.images = images; + this.storage = storage; + this.avatars = avatars; + this.events = events; + } + + @Transactional + public SkillAvatarUploadResponse upload(String actor, byte[] image) { + requireAuthenticated(actor); + byte[] normalized = images.normalize(image); + String id = UUID.randomUUID().toString(); + String key = "skill-avatars/uploads/" + id + ".png"; + storage.putObject(key, new ByteArrayInputStream(normalized), normalized.length, "image/png"); + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override public void afterCompletion(int result) { + if (result != STATUS_COMMITTED) { + storage.deleteObject(key); + } + } + }); + Instant expiresAt = Instant.now().plusSeconds(24 * 60 * 60); + uploads.saveAndFlush(new SkillAvatarUpload(id, actor, key, sha256(normalized), expiresAt)); + return new SkillAvatarUploadResponse(id, "/api/web/skills/avatar-uploads/" + id + "/image", expiresAt); + } + + @Transactional(readOnly = true) + public InputStream preview(String id, String actor) { + SkillAvatarUpload upload = uploads.findById(id).orElseThrow(() -> notFound()); + requireUploader(upload, actor); + if (upload.getObjectKey() == null || !upload.getExpiresAt().isAfter(Instant.now())) { + throw notFound(); + } + return storage.getObject(upload.getObjectKey()); + } + + @Transactional(readOnly = true) + public void validateReference(String namespace, String slug, String actor, String id) { + SkillAvatarUpload upload = uploads.findById(id).orElseThrow(() -> notFound()); + requireUploader(upload, actor); + if (upload.getCandidateId() != null) { + avatars.submittedStatus(namespace, slug, actor, upload.getCandidateId()); + } else if (upload.getObjectKey() == null || !upload.getExpiresAt().isAfter(Instant.now())) { + throw new DomainBadRequestException("error.skill.avatar.uploadExpired"); + } else if (!storage.exists(upload.getObjectKey())) { + throw new DomainBadRequestException("error.skill.avatar.invalidUpload"); + } + } + + @Transactional + public SkillAvatarStatusResponse submit(String namespace, String slug, String actor, String id, + AuditRequestContext context) { + return submitForAudience(namespace, slug, actor, id, null, context); + } + + @Transactional + public SkillAvatarStatusResponse submitForAudience(String namespace, String slug, String actor, String id, + SkillVisibility audience, AuditRequestContext context) { + // Publication already holds this Skill lock; standalone submission follows the same lock order. + avatars.lockOwnedSkill(namespace, slug, actor); + SkillAvatarUpload upload = uploads.findLockedById(id).orElseThrow(() -> notFound()); + requireUploader(upload, actor); + if (upload.getCandidateId() != null) { + // Owner and exact Skill binding are rechecked even for a repeated successful submission. + return avatars.submittedStatus(namespace, slug, actor, upload.getCandidateId()); + } + if (upload.getObjectKey() == null || !upload.getExpiresAt().isAfter(Instant.now())) { + throw new DomainBadRequestException("error.skill.avatar.uploadExpired"); + } + if (!storage.exists(upload.getObjectKey())) { + throw new DomainBadRequestException("error.skill.avatar.invalidUpload"); + } + SkillAvatarStatusResponse status = avatars.submitStored(namespace, slug, actor, + upload.getObjectKey(), upload.getContentSha256(), audience, context); + upload.consume(status.candidateId()); + if ("PENDING".equals(status.machineStatus()) && "PENDING".equals(status.candidateStatus())) { + events.publishEvent(new AvatarSubmittedEvent(status.candidateId(), context)); + } + return status; + } + + public record AvatarSubmittedEvent(long candidateId, AuditRequestContext context) {} + + private void requireAuthenticated(String actor) { + if (actor == null) { + throw new DomainForbiddenException("error.skill.avatar.ownerOnly"); + } + } + private void requireUploader(SkillAvatarUpload upload, String actor) { + requireAuthenticated(actor); + if (!upload.getUploadedBy().equals(actor)) { + throw new DomainForbiddenException("error.skill.avatar.ownerOnly"); + } + } + private String sha256(byte[] image) { + try { + return HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256").digest(image)); + } catch (NoSuchAlgorithmException impossible) { + throw new IllegalStateException("SHA-256 unavailable", impossible); + } + } + private DomainNotFoundException notFound() { + return new DomainNotFoundException("error.skill.avatar.notFound"); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillPublishAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillPublishAppService.java new file mode 100644 index 00000000..5f9828bb --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillPublishAppService.java @@ -0,0 +1,61 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; +import com.iflytek.skillhub.domain.namespace.SlugValidator; +import java.nio.charset.StandardCharsets; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** Commits the skill version and the requested avatar action as one database operation. */ +@Service +public class SkillPublishAppService { + public enum AvatarAction { KEEP, REPLACE, REMOVE } + private final SkillPublishService publications; + private final SkillAvatarUploadService uploads; + private final SkillAvatarAppService avatars; + private final SkillMetadataParser metadata; + + public SkillPublishAppService(SkillPublishService publications, SkillAvatarUploadService uploads, + SkillAvatarAppService avatars, SkillMetadataParser metadata) { + this.publications = publications; + this.uploads = uploads; + this.avatars = avatars; + this.metadata = metadata; + } + + @Transactional + public SkillPublishService.PublishResult publish(String namespace, List entries, String actor, + SkillVisibility visibility, Set roles, boolean confirmWarnings, AvatarAction action, + String uploadId, String expectedSlug, AuditRequestContext context) { + if (action == AvatarAction.REPLACE && (uploadId == null || uploadId.isBlank()) + || action != AvatarAction.REPLACE && uploadId != null && !uploadId.isBlank()) { + throw new DomainBadRequestException("error.skill.avatar.invalidUpload"); + } + if (action == AvatarAction.REPLACE || expectedSlug != null && !expectedSlug.isBlank()) { + PackageEntry skillMd = entries.stream().filter(entry -> "SKILL.md".equals(entry.path())).findFirst() + .orElseThrow(() -> new DomainBadRequestException("error.skill.publish.skillMd.notFound")); + String targetSlug = SlugValidator.slugify(metadata.parse(new String(skillMd.content(), StandardCharsets.UTF_8)).name()); + if (expectedSlug != null && !expectedSlug.isBlank() && !expectedSlug.equals(targetSlug)) { + throw new DomainBadRequestException("error.skill.avatar.skillMismatch"); + } + if (action == AvatarAction.REPLACE) { + uploads.validateReference(namespace, targetSlug, actor, uploadId); + } + } + SkillPublishService.PublishResult result = publications.publishFromEntries( + namespace, entries, actor, visibility, roles, confirmWarnings); + if (action == AvatarAction.REPLACE) { + uploads.submitForAudience(namespace, result.slug(), actor, uploadId, visibility, context); + } else if (action == AvatarAction.REMOVE) { + avatars.remove(namespace, result.slug(), actor, false, null, Map.of(), roles, context); + } + return result; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarCleanupTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarCleanupTask.java index 9a4c6ef9..73c62c96 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarCleanupTask.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarCleanupTask.java @@ -2,6 +2,8 @@ package com.iflytek.skillhub.task; import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; import com.iflytek.skillhub.infra.jpa.SkillAvatarCandidateJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillAvatarUploadJpaRepository; +import org.springframework.data.domain.PageRequest; import com.iflytek.skillhub.storage.ObjectStorageService; import java.time.Instant; import org.springframework.scheduling.annotation.Scheduled; @@ -12,15 +14,21 @@ import org.springframework.transaction.annotation.Transactional; public class SkillAvatarCleanupTask { private final SkillAvatarCandidateJpaRepository candidates; private final ObjectStorageService storage; + private final SkillAvatarUploadJpaRepository uploads; - public SkillAvatarCleanupTask(SkillAvatarCandidateJpaRepository candidates, ObjectStorageService storage) { + public SkillAvatarCleanupTask(SkillAvatarCandidateJpaRepository candidates, ObjectStorageService storage, SkillAvatarUploadJpaRepository uploads) { this.candidates = candidates; this.storage = storage; + this.uploads = uploads; } @Scheduled(cron = "0 35 2 * * ?") @Transactional public void removeExpiredImages() { + for (var upload : uploads.findExpired(Instant.now(), PageRequest.of(0, 100))) { + storage.deleteObject(upload.getObjectKey()); + upload.clearObjectKey(); + } for (SkillAvatarCandidate candidate : candidates.findTop100ByDiscardAtBeforeAndObjectKeyIsNotNull(Instant.now())) { storage.deleteObject(candidate.getObjectKey()); candidate.clearObjectKey(); diff --git a/server/skillhub-app/src/main/resources/db/migration/V71__staged_skill_avatar_uploads.sql b/server/skillhub-app/src/main/resources/db/migration/V71__staged_skill_avatar_uploads.sql new file mode 100644 index 00000000..21946d18 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V71__staged_skill_avatar_uploads.sql @@ -0,0 +1,9 @@ +CREATE TABLE skill_avatar_upload ( + id VARCHAR(36) PRIMARY KEY, + uploaded_by VARCHAR(128) NOT NULL, + object_key VARCHAR(512), + content_sha256 VARCHAR(64) NOT NULL, + expires_at TIMESTAMP NOT NULL, + candidate_id BIGINT +); +CREATE INDEX idx_avatar_upload_expiry ON skill_avatar_upload(expires_at) WHERE candidate_id IS NULL AND object_key IS NOT NULL; diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 34809943..23347650 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -294,3 +294,7 @@ promotion.revocation.required=Use the reviewed revocation workflow to remove a p promotion.revocation.page_invalid=Page must be nonnegative and size must be between 1 and 100 error.auth.settings.unavailable=Authentication settings are unavailable error.system.roleGrant.unverifiedProvider=This identity provider does not attest verified email and cannot use initial role rules + +error.skill.avatar.uploadExpired=The uploaded avatar expired. Please upload it again. +error.skill.avatar.invalidUpload=Select a valid uploaded avatar before submitting. +error.skill.avatar.skillMismatch=The uploaded package does not match the skill being updated. diff --git a/server/skillhub-app/src/main/resources/messages_ru.properties b/server/skillhub-app/src/main/resources/messages_ru.properties index 47b58c94..71dc7ca2 100644 --- a/server/skillhub-app/src/main/resources/messages_ru.properties +++ b/server/skillhub-app/src/main/resources/messages_ru.properties @@ -235,3 +235,7 @@ error.suite.bundle.member.stateChanged=Состояние связанного S error.suite.bundle.actor.inactive=Пользователь операции Skill Suite Bundle больше не активен error.auth.settings.unavailable=Настройки аутентификации временно недоступны error.system.roleGrant.unverifiedProvider=Этот поставщик удостоверений не подтверждает адрес почты и не поддерживает правила первоначального назначения роли + +error.skill.avatar.uploadExpired=Срок действия загрузки аватара истёк. Загрузите изображение снова. +error.skill.avatar.invalidUpload=Перед отправкой выберите успешно загруженный аватар. +error.skill.avatar.skillMismatch=Загруженный пакет не соответствует обновляемому навыку. diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 2647dd00..ea711eba 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -294,3 +294,7 @@ promotion.revocation.required=已提升的技能须通过审核撤销流程删 promotion.revocation.page_invalid=页码不能为负数，每页数量须在 1 到 100 之间 error.auth.settings.unavailable=认证配置暂时不可用 error.system.roleGrant.unverifiedProvider=该身份来源不提供已验证邮箱，不能用于首次角色授权规则 + +error.skill.avatar.uploadExpired=头像上传引用已过期，请重新上传。 +error.skill.avatar.invalidUpload=请选择已上传成功的头像后再提交。 +error.skill.avatar.skillMismatch=上传的技能包与正在更新的技能不一致。 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java index bffac195..e8b2fd57 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarRecoveryPostgresTest.java @@ -443,6 +443,10 @@ class SkillAvatarRecoveryPostgresTest { privateSkill.setVisibility(SkillVisibility.PRIVATE); }); var uploaded = avatars.submit(namespaceSlug(skillId), "skill", "owner", new byte[] {8}, new AuditRequestContext(null, null)); + SkillQueryService.SkillDetailDTO detail = new SkillQueryService.SkillDetailDTO( + skillId, null, null, null, null, null, null, null, null, null, null, null, null, + false, null, null, null, false, false, false, false, null, null, null, null, null, null, null); + when(skillQueries.getSkillDetail(anyString(), eq("skill"), eq("admin"), any())).thenReturn(detail); when(visibility.canAccess(any(), eq("admin"), any(), any())).thenReturn(true); assertThatThrownBy(() -> avatars.readActive(namespaceSlug(skillId), "skill", "admin", adminRole, java.util.Set.of())) .isInstanceOf(com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException.class); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java new file mode 100644 index 00000000..075c162d --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarUploadPostgresTest.java @@ -0,0 +1,351 @@ +package com.iflytek.skillhub.repository; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.*; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadataParser; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.domain.skill.service.SkillStorageDeletionCompensationService; +import com.iflytek.skillhub.domain.skill.validation.*; +import com.iflytek.skillhub.domain.security.SecurityScanService; +import com.iflytek.skillhub.dto.AvatarReviewSettingsResponse; +import com.iflytek.skillhub.infra.jpa.*; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.service.*; +import com.iflytek.skillhub.task.SkillAvatarCleanupTask; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.time.Clock; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +/** Real publication and staged-image transactions, with controlled external dependencies. */ +@DataJpaTest(showSql = false) +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@ActiveProfiles("test") +@Testcontainers +@Import({SkillAvatarAppService.class, SkillAvatarUploadService.class, SkillPublishAppService.class, + SkillPublishService.class, SkillStorageDeletionCompensationService.class, JpaSkillStorageDeletionCompensationRepositoryAdapter.class, SkillAvatarCleanupTask.class, + SkillAvatarUploadPostgresTest.Configuration.class}) +@Transactional(propagation = Propagation.NOT_SUPPORTED) +class SkillAvatarUploadPostgresTest { + @Container static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer<>("postgres:16-alpine"); + @DynamicPropertySource static void database(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); + registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); + registry.add("spring.flyway.enabled", () -> true); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "validate"); + } + @TestConfiguration static class Configuration { + @Bean SkillMetadataParser metadata() { return new SkillMetadataParser(); } + @Bean SkillPackageValidator validator(SkillMetadataParser metadata) { return new SkillPackageValidator(metadata); } + @Bean PrePublishValidator preValidator() { return new NoOpPrePublishValidator(); } + @Bean ObjectMapper mapper() { return new ObjectMapper(); } + @Bean Clock clock() { return Clock.systemUTC(); } + } + @Autowired SkillAvatarUploadService uploads; + @Autowired SkillPublishAppService publications; + @Autowired SkillAvatarAppService avatars; + @Autowired SkillAvatarCleanupTask cleanup; + @Autowired SkillAvatarUploadJpaRepository uploadRows; + @Autowired SkillJpaRepository skills; + @Autowired SkillVersionJpaRepository versions; + @Autowired SkillAvatarCandidateJpaRepository candidates; + @Autowired NamespaceJpaRepository namespaces; + @Autowired UserAccountJpaRepository users; + @Autowired PlatformTransactionManager transactions; + @Autowired JdbcTemplate jdbc; + @MockBean ObjectStorageService storage; + @MockBean SkillAvatarImageProcessor images; + @MockBean AvatarReviewSettingsService settings; + @MockBean AvatarModerationGateway gateway; + @MockBean VisibilityChecker visibility; + @MockBean SkillQueryService queries; + @MockBean AuditLogService audit; + @MockBean RequestIdAccessor requestIds; + @MockBean SecurityScanService scanner; + private String namespace; + private final AuditRequestContext context = new AuditRequestContext(null, null); + + @BeforeEach void fixtures() { + for (String actor : List.of("owner", "stranger")) { + if (((com.iflytek.skillhub.domain.user.UserAccountRepository) users).findById(actor).isEmpty()) { + users.saveAndFlush(new com.iflytek.skillhub.domain.user.UserAccount(actor, actor, null, null)); + } + } + namespace = "upload-" + UUID.randomUUID().toString().substring(0, 8); + namespaces.saveAndFlush(new Namespace(namespace, "Upload testing", "owner")); + when(images.normalize(any())).thenAnswer(call -> call.getArgument(0)); + when(storage.exists(anyString())).thenReturn(true); + when(storage.getObject(anyString())).thenAnswer(call -> new ByteArrayInputStream(new byte[]{1,2,3})); + when(settings.current()).thenReturn(new AvatarReviewSettingsResponse(false, true, 1, null)); + when(scanner.isEnabled()).thenReturn(true); + } + + @Test void sameSlugPrivateDraftOwnedByAnotherUserDoesNotHijackAvatarBindingOrRemoval() { + Long otherId = new TransactionTemplate(transactions).execute(ignored -> + skills.saveAndFlush(new Skill(namespaceId(), "skill", "stranger", SkillVisibility.PRIVATE)).getId()); + String id = upload(); + var mine = publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, id, "skill"); + Long myCandidate = uploadRows.findById(id).orElseThrow().getCandidateId(); + assertThat(candidates.findById(myCandidate).orElseThrow().getSkillId()).isEqualTo(mine.skillId()); + assertThat(avatars.ownerStatus(namespace, "skill", "owner").candidateId()).isEqualTo(myCandidate); + assertThat(avatars.ownerStatus(namespace, "skill", "stranger").candidateId()).isNull(); + assertThat(avatars.readCandidate(namespace, "skill", myCandidate, "owner", java.util.Map.of(), Set.of())).isNotNull(); + avatars.decide(namespace, "skill", myCandidate, true, "approved", "owner", + java.util.Map.of(namespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER), Set.of(), context); + SkillQueryService.SkillDetailDTO detail = new SkillQueryService.SkillDetailDTO( + mine.skillId(), null, null, null, null, null, null, null, null, null, null, null, null, + false, null, null, null, false, false, false, false, null, null, null, null, null, null, null); + when(queries.getSkillDetail(eq(namespace), eq("skill"), eq("owner"), any())).thenReturn(detail); + when(visibility.canAccess(any(Skill.class), eq("owner"), any(), any())).thenReturn(true); + assertThat(avatars.readActive(namespace, "skill", "owner", java.util.Map.of(), Set.of())).isNotNull(); + publish("1.0.1", SkillPublishAppService.AvatarAction.REMOVE, null, "skill"); + assertThat(read(mine.skillId()).getPendingAvatarId()).isNull(); + assertThat(read(otherId).getPendingAvatarId()).isNull(); + assertThat(read(otherId).getVisibility()).isEqualTo(SkillVisibility.PRIVATE); + } + + @Test void uploadDoesNotCreateSkillOrCandidateAndPreviewIsUploaderOnly() { + long initialCandidates = candidates.count(); + String id = upload(); + assertThat(candidates.count()).isEqualTo(initialCandidates); + assertThat(skills.findByNamespaceIdAndSlug(namespaceId(), "skill")).isEmpty(); + assertThat(uploads.preview(id, "owner")).isNotNull(); + assertThatThrownBy(() -> uploads.preview(id, "stranger")).isInstanceOf(DomainForbiddenException.class); + } + + @Test void publishConsumesUploadAndRepeatedReferenceDoesNotCreateAnotherCandidate() { + String id = upload(); + var published = publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, id, null); + var candidate = candidates.findById(uploadRows.findById(id).orElseThrow().getCandidateId()).orElseThrow(); + assertThat(candidate.getSkillId()).isEqualTo(published.skillId()); + assertThat(candidate.getReviewStatus()).isEqualTo(SkillAvatarCandidate.ReviewStatus.PENDING); + assertThat(candidate.getObjectKey()).isEqualTo(uploadRows.findById(id).orElseThrow().getObjectKey()); + assertThat(uploads.submit(namespace, "skill", "owner", id, context).candidateId()).isEqualTo(candidate.getId()); + assertThat(candidates.findBySkillIdOrderByCreatedAtDesc(published.skillId())).hasSize(1); + assertThat(read(published.skillId()).getActiveAvatarId()).isNull(); + verify(storage, times(1)).putObject(startsWith("skill-avatars/"), any(), anyLong(), anyString()); + } + + @Test void invalidReferencesAndWrongTargetRejectBeforePackageWrites() { + String foreign = uploads.upload("stranger", new byte[]{5}).uploadId(); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, foreign, null)) + .isInstanceOf(DomainForbiddenException.class); + String expired = upload(); + jdbc.update("UPDATE skill_avatar_upload SET expires_at = TIMESTAMP '2000-01-01 00:00:00' WHERE id = ?", expired); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, expired, null)) + .isInstanceOf(DomainBadRequestException.class); + String missingObject = upload(); + when(storage.exists(uploadRows.findById(missingObject).orElseThrow().getObjectKey())).thenReturn(false); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, missingObject, null)) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.KEEP, null, "different-skill")) + .isInstanceOf(DomainBadRequestException.class); + verify(storage, never()).putObject(startsWith("skills/"), any(), anyLong(), anyString()); + assertThat(skills.findByNamespaceIdAndSlug(namespaceId(), "skill")).isEmpty(); + } + + @Test void lateAvatarFailureRollsBackVersionReplacementAndPreservesOldCandidateAndObjects() { + var old = publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, upload(), null); + Long oldCandidate = read(old.skillId()).getPendingAvatarId(); + new TransactionTemplate(transactions).executeWithoutResult(ignored -> + ((SkillVersionRepository) versions).findById(old.version().getId()).orElseThrow().setStatus(SkillVersionStatus.UPLOADED)); + String fresh = upload(); + when(audit.record(anyString(), eq("SKILL_AVATAR_SUBMIT"), anyString(), anyLong(), any(), any(), any(), anyString())) + .thenThrow(new IllegalStateException("audit unavailable")); + clearInvocations(storage); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, fresh, null)) + .isInstanceOf(IllegalStateException.class); + assertThat(versions.findBySkillIdAndVersion(old.skillId(), "1.0.0").orElseThrow().getId()).isEqualTo(old.version().getId()); + assertThat(read(old.skillId()).getPendingAvatarId()).isEqualTo(oldCandidate); + assertThat(candidates.findById(oldCandidate).orElseThrow().getStatus()).isEqualTo(SkillAvatarCandidate.Status.PENDING); + assertThat(uploadRows.findById(fresh).orElseThrow().getCandidateId()).isNull(); + @SuppressWarnings("unchecked") + var deleted = (org.mockito.ArgumentCaptor>) (org.mockito.ArgumentCaptor) org.mockito.ArgumentCaptor.forClass(List.class); + verify(storage).deleteObjects(deleted.capture()); + assertThat(deleted.getValue()).hasSize(2).allMatch(key -> !key.contains("/" + old.version().getId() + "/")); + verify(storage, never()).deleteObject(uploadRows.findById(fresh).orElseThrow().getObjectKey()); + } + + @Test void failedNewPublicationLeavesNoSkillVersionOrCandidate() { + String id = upload(); + when(settings.current()).thenThrow(new IllegalStateException("settings unavailable")); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, id, null)) + .isInstanceOf(IllegalStateException.class); + assertThat(skills.findByNamespaceIdAndSlug(namespaceId(), "skill")).isEmpty(); + assertThat(uploadRows.findById(id).orElseThrow().getCandidateId()).isNull(); + assertThat(jdbc.queryForObject("SELECT count(*) FROM skill_avatar_candidate WHERE object_key = ?", Long.class, + uploadRows.findById(id).orElseThrow().getObjectKey())).isZero(); + verify(storage).deleteObjects(argThat(keys -> keys.size() == 2)); + } + + @Test void failedRollbackObjectDeletionIsPersistedForRetryWithoutOriginalErrorText() { + String id = upload(); + when(settings.current()).thenThrow(new IllegalStateException("settings unavailable")); + doThrow(new IllegalStateException("sensitive-storage-error")).when(storage).deleteObjects(anyList()); + assertThatThrownBy(() -> publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, id, null)) + .isInstanceOf(IllegalStateException.class); + assertThat(jdbc.queryForObject("SELECT count(*) FROM skill_storage_delete_compensation WHERE namespace = ? AND status = 'PENDING'", Long.class, namespace)) + .isEqualTo(1L); + assertThat(jdbc.queryForObject("SELECT last_error FROM skill_storage_delete_compensation WHERE namespace = ?", String.class, namespace)) + .isEqualTo("IllegalStateException"); + } + + @Test void removeOnlyTakesEffectOnSuccessfulCommitAndOldReviewCannotRestoreIt() { + var first = publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, upload(), null); + Long candidate = read(first.skillId()).getPendingAvatarId(); + assertThatThrownBy(() -> publish("1.0.1", SkillPublishAppService.AvatarAction.REMOVE, null, "wrong")) + .isInstanceOf(DomainBadRequestException.class); + assertThat(read(first.skillId()).getPendingAvatarId()).isEqualTo(candidate); + publish("1.0.1", SkillPublishAppService.AvatarAction.REMOVE, null, "skill"); + assertThat(read(first.skillId()).getPendingAvatarId()).isNull(); + assertThat(candidates.findById(candidate).orElseThrow().getStatus()).isEqualTo(SkillAvatarCandidate.Status.REMOVED); + assertThatThrownBy(() -> avatars.decide(namespace, "skill", candidate, true, "late", "owner", + java.util.Map.of(namespaceId(), com.iflytek.skillhub.domain.namespace.NamespaceRole.OWNER), Set.of(), context)) + .isInstanceOf(com.iflytek.skillhub.domain.shared.exception.DomainConflictException.class); + } + + @Test void cleanupExcludesLinkedImagesAndWaitsForConsumptionLock() throws Exception { + String abandoned = upload(); + String linked = upload(); + var skill = publish("1.0.0", SkillPublishAppService.AvatarAction.REPLACE, linked, null); + String abandonedKey = uploadRows.findById(abandoned).orElseThrow().getObjectKey(); + String linkedKey = uploadRows.findById(linked).orElseThrow().getObjectKey(); + jdbc.update("UPDATE skill_avatar_upload SET expires_at = TIMESTAMP '2000-01-01 00:00:00' WHERE id IN (?, ?)", abandoned, linked); + cleanup.removeExpiredImages(); + verify(storage).deleteObject(abandonedKey); + verify(storage, never()).deleteObject(linkedKey); + assertThat(uploadRows.findById(abandoned).orElseThrow().getObjectKey()).isNull(); + assertThat(candidates.findById(read(skill.skillId()).getPendingAvatarId()).orElseThrow().getObjectKey()).isEqualTo(linkedKey); + String racing = upload(); + CountDownLatch locked = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + try (var pool = Executors.newFixedThreadPool(2)) { + var consuming = pool.submit(() -> new TransactionTemplate(transactions).execute(status -> { + uploads.submit(namespace, "skill", "owner", racing, context); + jdbc.update("UPDATE skill_avatar_upload SET expires_at = TIMESTAMP '2000-01-01 00:00:00' WHERE id = ?", racing); + locked.countDown(); + try { assertThat(release.await(10, TimeUnit.SECONDS)).isTrue(); } + catch (InterruptedException failure) { throw new RuntimeException(failure); } + return null; + })); + try { + assertThat(locked.await(10, TimeUnit.SECONDS)).isTrue(); + var scanning = pool.submit(cleanup::removeExpiredImages); + release.countDown(); + consuming.get(10, TimeUnit.SECONDS); + scanning.get(10, TimeUnit.SECONDS); + } finally { release.countDown(); } + } + assertThat(uploadRows.findById(racing).orElseThrow().getCandidateId()).isNotNull(); + verify(storage, never()).deleteObject(uploadRows.findById(racing).orElseThrow().getObjectKey()); + } + + @Test void expiredCleanupLocksExcludeConcurrentConsumption() throws Exception { + var published = publish("1.0.0", SkillPublishAppService.AvatarAction.KEEP, null, null); + String id = upload(); + String key = uploadRows.findById(id).orElseThrow().getObjectKey(); + jdbc.update("UPDATE skill_avatar_upload SET expires_at = TIMESTAMP '2000-01-01 00:00:00' WHERE id = ?", id); + CountDownLatch deleting = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + doAnswer(call -> { + deleting.countDown(); + assertThat(release.await(10, TimeUnit.SECONDS)).isTrue(); + return null; + }).when(storage).deleteObject(key); + try (var pool = Executors.newFixedThreadPool(2)) { + var cleaning = pool.submit(cleanup::removeExpiredImages); + try { + assertThat(deleting.await(10, TimeUnit.SECONDS)).isTrue(); + var consuming = pool.submit(() -> uploads.submit(namespace, "skill", "owner", id, context)); + assertThatThrownBy(() -> consuming.get(250, TimeUnit.MILLISECONDS)) + .isInstanceOf(java.util.concurrent.TimeoutException.class); + release.countDown(); + cleaning.get(10, TimeUnit.SECONDS); + assertThatThrownBy(() -> consuming.get(10, TimeUnit.SECONDS)) + .hasCauseInstanceOf(DomainBadRequestException.class); + } finally { release.countDown(); } + } + assertThat(read(published.skillId()).getPendingAvatarId()).isNull(); + assertThat(uploadRows.findById(id).orElseThrow().getCandidateId()).isNull(); + assertThat(uploadRows.findById(id).orElseThrow().getObjectKey()).isNull(); + } + + @Test void concurrentSameReferenceIsConsumedOnceWithoutDeadlock() throws Exception { + var published = publish("1.0.0", SkillPublishAppService.AvatarAction.KEEP, null, null); + String id = upload(); + CountDownLatch start = new CountDownLatch(1); + try (var pool = Executors.newFixedThreadPool(2)) { + var first = pool.submit(() -> { start.await(); return uploads.submit(namespace, "skill", "owner", id, context); }); + var second = pool.submit(() -> { start.await(); return uploads.submit(namespace, "skill", "owner", id, context); }); + start.countDown(); + assertThat(first.get(10, TimeUnit.SECONDS).candidateId()).isEqualTo(second.get(10, TimeUnit.SECONDS).candidateId()); + } + assertThat(candidates.findBySkillIdOrderByCreatedAtDesc(published.skillId())).hasSize(1); + } + + @Test void targetPublicAudienceCannotBypassReviewWhilePrivateVersionWaits() { + var first = publications.publish(namespace, entries("1.0.0"), "owner", SkillVisibility.PRIVATE, Set.of("SUPER_ADMIN"), + false, SkillPublishAppService.AvatarAction.REPLACE, upload(), null, context); + Long active = read(first.skillId()).getActiveAvatarId(); + // The non-admin publisher must have namespace membership for the real domain service. + jdbc.update("INSERT INTO namespace_member(namespace_id, user_id, role) VALUES (?, 'owner', 'OWNER')", namespaceId()); + var expanded = publications.publish(namespace, entries("1.0.1"), "owner", SkillVisibility.PUBLIC, Set.of(), + false, SkillPublishAppService.AvatarAction.REPLACE, upload(), null, context); + var pending = candidates.findById(read(expanded.skillId()).getPendingAvatarId()).orElseThrow(); + assertThat(pending.getAudience()).isEqualTo(SkillVisibility.PUBLIC); + assertThat(pending.isManualEnabled()).isTrue(); + assertThat(read(first.skillId()).getActiveAvatarId()).isEqualTo(active); + assertThat(avatars.prepareScopeReview(first.skillId(), "owner")).isNull(); + assertThat(read(first.skillId()).getPendingAvatarId()).isEqualTo(pending.getId()); + } + + private String upload() { return uploads.upload("owner", UUID.randomUUID().toString().getBytes(StandardCharsets.UTF_8)).uploadId(); } + private Long namespaceId() { return namespaces.findBySlug(namespace).orElseThrow().getId(); } + private Skill read(Long id) { return ((SkillRepository) skills).findById(id).orElseThrow(); } + private SkillPublishService.PublishResult publish(String version, SkillPublishAppService.AvatarAction action, String id, String expected) { + return publications.publish(namespace, entries(version), "owner", SkillVisibility.PUBLIC, Set.of("SUPER_ADMIN"), + false, action, id, expected, context); + } + private List entries(String version) { + byte[] markdown = ("---\nname: skill\ndescription: Test skill\nversion: " + version + "\n---\n# Skill\nInstructions\n").getBytes(StandardCharsets.UTF_8); + return List.of(new PackageEntry("SKILL.md", markdown, markdown.length, "text/markdown")); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java index 5f6e22ee..bfefd6ec 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java @@ -17,6 +17,7 @@ import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.dto.AvatarReviewSettingsResponse; import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.VisibilityChecker; @@ -73,6 +74,8 @@ class SkillAvatarAppServiceTest { ReflectionTestUtils.setField(namespace, "type", NamespaceType.TEAM); skill = new Skill(10L, "foo", "owner", SkillVisibility.PUBLIC); ReflectionTestUtils.setField(skill, "id", 11L); + org.mockito.Mockito.lenient().when(skills.findByNamespaceIdAndSlugAndOwnerId(10L, "foo", "owner")) + .thenReturn(Optional.of(skill)); } @AfterEach @@ -86,8 +89,11 @@ class SkillAvatarAppServiceTest { void privateAvatarCannotBeReadByNamespaceAdmin() { skill.setVisibility(SkillVisibility.PRIVATE); skill.activateAvatar(21L, SkillVisibility.PRIVATE); - when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); - when(skills.findByNamespaceIdAndSlug(10L, "foo")).thenReturn(List.of(skill)); + SkillQueryService.SkillDetailDTO detail = new SkillQueryService.SkillDetailDTO( + 11L, null, null, null, null, null, null, null, null, null, null, null, null, + false, null, null, null, false, false, false, false, null, null, null, null, null, null, null); + when(skillQueries.getSkillDetail(eq("team"), eq("foo"), eq("admin"), any())).thenReturn(detail); + when(((SkillRepository) skills).findById(11L)).thenReturn(Optional.of(skill)); when(visibility.canAccess(eq(skill), eq("admin"), any(), any())).thenReturn(true); assertThatThrownBy(() -> service.readActive("team", "foo", "admin", Map.of(10L, NamespaceRole.ADMIN), Set.of())) @@ -98,7 +104,7 @@ class SkillAvatarAppServiceTest { @Test void privateCandidateCannotBeReadByNamespaceAdmin() { when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); - when(skills.findByNamespaceIdAndSlug(10L, "foo")).thenReturn(List.of(skill)); + when(((SkillRepository) skills).findById(11L)).thenReturn(Optional.of(skill)); SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/private.png", "abc", SkillVisibility.PRIVATE, false, false, null); ReflectionTestUtils.setField(candidate, "id", 21L); @@ -319,7 +325,7 @@ class SkillAvatarAppServiceTest { @Test void nonOwnerCannotSubmitBeforeWritingToStorage() { - stubSkillLookup(); + when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); assertThatThrownBy(() -> service.submit("team", "foo", "stranger", new byte[] { 1 }, new AuditRequestContext(null, null))).isInstanceOf(DomainForbiddenException.class); verifyNoInteractions(storage); @@ -327,17 +333,28 @@ class SkillAvatarAppServiceTest { @Test void staleApprovalCannotReplaceNewerCandidate() { - stubSkillLookup(); + when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); + when(((SkillRepository) skills).findById(11L)).thenReturn(Optional.of(skill)); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/old.png", "abc", + SkillVisibility.PUBLIC, false, true, null); + when(candidates.findById(21L)).thenReturn(Optional.of(candidate)); when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); assertThatThrownBy(() -> service.decide("team", "foo", 21L, true, "fine", "reviewer", Map.of(10L, NamespaceRole.ADMIN), Set.of(), new AuditRequestContext(null, null))) .isInstanceOf(DomainConflictException.class); - verifyNoInteractions(candidates); + org.assertj.core.api.Assertions.assertThat(candidate.getReviewStatus()) + .isEqualTo(SkillAvatarCandidate.ReviewStatus.PENDING); } @Test void reviewRequiresMatchingNamespaceRole() { - stubSkillLookup(); + when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); + when(((SkillRepository) skills).findById(11L)).thenReturn(Optional.of(skill)); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/old.png", "abc", + SkillVisibility.PUBLIC, false, true, null); + when(candidates.findById(21L)).thenReturn(Optional.of(candidate)); when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); assertThatThrownBy(() -> service.decide("team", "foo", 21L, true, "fine", "reviewer", Map.of(), Set.of("SKILL_ADMIN"), new AuditRequestContext(null, null))) @@ -387,9 +404,4 @@ class SkillAvatarAppServiceTest { return candidate; } - private void stubSkillLookup() { - when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); - when(skills.findByNamespaceIdAndSlug(10L, "foo")).thenReturn(List.of(skill)); - when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); - } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java index 3219db40..6f9daa3b 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -55,6 +55,8 @@ public class RouteSecurityPolicyRegistry { RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/reviews/me"), RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/web/skills/*/reviews/me"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills"), + RouteAuthorizationPolicy.authenticated(HttpMethod.POST, "/api/v1/skills/avatar-uploads"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/avatar-uploads/*/image"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/avatar-review/queue"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/avatar"), @@ -72,6 +74,8 @@ public class RouteSecurityPolicyRegistry { RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/tags/*/file"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/labels"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills"), + RouteAuthorizationPolicy.authenticated(HttpMethod.POST, "/api/web/skills/avatar-uploads"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/avatar-uploads/*/image"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/avatar-review/queue"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/avatar"), diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarUpload.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarUpload.java new file mode 100644 index 00000000..a464833a --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarUpload.java @@ -0,0 +1,43 @@ +package com.iflytek.skillhub.domain.skill; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import java.time.Instant; + +/** Private, uploader-owned image awaiting a successful skill submission. */ +@Entity +@Table(name = "skill_avatar_upload") +public class SkillAvatarUpload { + @Id + @Column(length = 36) + private String id; + @Column(name = "uploaded_by", nullable = false, length = 128) + private String uploadedBy; + @Column(name = "object_key", length = 512) + private String objectKey; + @Column(name = "content_sha256", nullable = false, length = 64) + private String contentSha256; + @Column(name = "expires_at", nullable = false) + private Instant expiresAt; + @Column(name = "candidate_id") + private Long candidateId; + + protected SkillAvatarUpload() {} + public SkillAvatarUpload(String id, String uploadedBy, String objectKey, String sha256, Instant expiresAt) { + this.id = id; + this.uploadedBy = uploadedBy; + this.objectKey = objectKey; + this.contentSha256 = sha256; + this.expiresAt = expiresAt; + } + public String getId() { return id; } + public String getUploadedBy() { return uploadedBy; } + public String getObjectKey() { return objectKey; } + public String getContentSha256() { return contentSha256; } + public Instant getExpiresAt() { return expiresAt; } + public Long getCandidateId() { return candidateId; } + public void consume(Long candidateId) { this.candidateId = candidateId; } + public void clearObjectKey() { objectKey = null; } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index 8fb01137..5d668db3 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -565,6 +565,29 @@ public class SkillPublishService { // 9. Upload each file to storage and compute SHA-256 List skillFiles = new ArrayList<>(); + List uploadedKeys = new ArrayList<>(); + if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override public void afterCompletion(int result) { + if (result != STATUS_COMMITTED && !uploadedKeys.isEmpty()) { + try { + objectStorageService.deleteObjects(List.copyOf(uploadedKeys)); + } catch (RuntimeException failure) { + // Keys contain fresh version IDs only. A failed cleanup must never expose provider secrets. + try { + compensationService.recordRollbackFailure(namespaceSlug, skill.getSlug(), + List.copyOf(uploadedKeys), failure.getClass().getSimpleName()); + } catch (RuntimeException compensationFailure) { + log.warn("Failed to record rolled-back publication cleanup, failureType={}", + compensationFailure.getClass().getSimpleName()); + } + log.warn("Failed to clean rolled-back publication objects, failureType={}", + failure.getClass().getSimpleName()); + } + } + } + }); + } long totalSize = 0; Path bundleZip = null; @@ -615,6 +638,7 @@ public class SkillPublishService { zipOutput.finish(); } for (StagedPackageFile staged : stagedFiles) { + uploadedKeys.add(staged.storageKey()); try (InputStream stagedInput = Files.newInputStream(staged.path())) { objectStorageService.putObject( staged.storageKey(), stagedInput, staged.size(), staged.contentType()); @@ -626,6 +650,7 @@ public class SkillPublishService { } String bundleKey = String.format("packages/%d/%d/bundle.zip", skill.getId(), version.getId()); long bundleSize = Files.size(bundleZip); + uploadedKeys.add(bundleKey); try (InputStream bundleInput = Files.newInputStream(bundleZip)) { objectStorageService.putObject( bundleKey, bundleInput, bundleSize, "application/zip"); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java index 3c7b2df0..4c811c73 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java @@ -10,6 +10,7 @@ import com.iflytek.skillhub.storage.ObjectStorageService; import java.util.List; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.annotation.Propagation; @Service public class SkillStorageDeletionCompensationService { @@ -43,6 +44,12 @@ public class SkillStorageDeletionCompensationService { )); } + /** Persists cleanup after a publication transaction rolled back, including a newly-created Skill. */ + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void recordRollbackFailure(String namespace, String slug, List storageKeys, String failureType) { + recordFailure(null, namespace, slug, storageKeys, failureType); + } + @Transactional public int retryPendingCleanup() { List records = diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarUploadJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarUploadJpaRepository.java new file mode 100644 index 00000000..acd51a34 --- /dev/null +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarUploadJpaRepository.java @@ -0,0 +1,22 @@ +package com.iflytek.skillhub.infra.jpa; + +import com.iflytek.skillhub.domain.skill.SkillAvatarUpload; +import jakarta.persistence.LockModeType; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; + +public interface SkillAvatarUploadJpaRepository extends JpaRepository { + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("SELECT u FROM SkillAvatarUpload u WHERE u.id = :id") + Optional findLockedById(@Param("id") String id); + + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("SELECT u FROM SkillAvatarUpload u WHERE u.candidateId IS NULL AND u.objectKey IS NOT NULL AND u.expiresAt <= :now ORDER BY u.expiresAt") + List findExpired(@Param("now") Instant now, Pageable page); +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 7570404e..1e62dd8a 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,6 +1,6 @@ import createClient from 'openapi-fetch' import { BASE_PATH } from '@/shared/lib/base-path' -import type { paths } from './generated/schema' +import type { components, paths } from './generated/schema' import type { ChangePasswordRequest, PasswordResetConfirmRequest, @@ -521,6 +521,19 @@ function avatarPath(namespace: string, slug: string): string { } export const skillAvatarApi = { + stage(file: Blob): Promise { + const body = new FormData() + body.append('file', file, 'avatar.png') + return fetchJson(`${WEB_API_PREFIX}/skills/avatar-uploads`, { + method: 'POST', headers: getCsrfHeaders(), body, + }) + }, + submit(namespace: string, slug: string, uploadId: string): Promise { + return fetchJson(`${WEB_API_PREFIX}/skills/${encodeURIComponent(namespace)}/${encodeURIComponent(slug)}/avatar`, { + method: 'POST', headers: getCsrfHeaders({ 'Content-Type': 'application/json' }), + body: JSON.stringify({ uploadId }), + }) + }, status(namespace: string, slug: string): Promise { return fetchJson(`${avatarPath(namespace, slug)}/status`) }, diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 25d639ef..9e6e46cf 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -1509,7 +1509,7 @@ export interface paths { }; get: operations["current"]; put?: never; - post: operations["submit"]; + post: operations["submit_2"]; delete: operations["delete"]; options?: never; head?: never; @@ -1525,7 +1525,7 @@ export interface paths { }; get: operations["current_1"]; put?: never; - post: operations["submit_1"]; + post: operations["submit_1_1"]; delete: operations["delete_1"]; options?: never; head?: never; @@ -1596,6 +1596,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/avatar-uploads": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["upload"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/avatar-uploads": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["upload_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/reviews/{id}/withdraw": { parameters: { query?: never; @@ -1925,7 +1957,7 @@ export interface paths { }; get?: never; put?: never; - post: operations["submit_2"]; + post: operations["submit"]; delete?: never; options?: never; head?: never; @@ -1941,7 +1973,7 @@ export interface paths { }; get?: never; put?: never; - post: operations["submit_3"]; + post: operations["submit_1"]; delete?: never; options?: never; head?: never; @@ -2115,7 +2147,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["status"]; + get: operations["status_2"]; put?: never; post: operations["create"]; delete: operations["revoke"]; @@ -2131,7 +2163,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["status_1"]; + get: operations["status_3"]; put?: never; post: operations["create_1"]; delete: operations["revoke_1"]; @@ -3915,6 +3947,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/avatar-uploads/{uploadId}/image": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["uploadPreview"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/avatar-uploads/{uploadId}/image": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["uploadPreview_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/skills/avatar-review/queue": { parameters: { query?: never; @@ -6298,6 +6362,9 @@ export interface components { approved?: boolean; reason: string; }; + SkillAvatarSubmitRequest: { + uploadId: string; + }; ApiResponsePublishResponse: { /** Format: int32 */ code?: number; @@ -6319,6 +6386,21 @@ export interface components { /** Format: int64 */ totalSize?: number; }; + ApiResponseSkillAvatarUploadResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SkillAvatarUploadResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + SkillAvatarUploadResponse: { + uploadId: string; + previewUrl: string; + /** Format: date-time */ + expiresAt: string; + }; ReviewActionRequest: { comment?: string; }; @@ -11644,7 +11726,7 @@ export interface operations { }; }; }; - submit: { + submit_2: { parameters: { query?: never; header?: never; @@ -11654,12 +11736,13 @@ export interface operations { }; cookie?: never; }; - requestBody?: { + requestBody: { content: { "multipart/form-data": { /** Format: binary */ file: string; }; + "application/json": components["schemas"]["SkillAvatarSubmitRequest"]; }; }; responses: { @@ -11720,7 +11803,7 @@ export interface operations { }; }; }; - submit_1: { + submit_1_1: { parameters: { query?: never; header?: never; @@ -11730,12 +11813,13 @@ export interface operations { }; cookie?: never; }; - requestBody?: { + requestBody: { content: { "multipart/form-data": { /** Format: binary */ file: string; }; + "application/json": components["schemas"]["SkillAvatarSubmitRequest"]; }; }; responses: { @@ -11832,6 +11916,9 @@ export interface operations { query: { visibility: string; confirmWarnings?: boolean; + avatarAction?: "KEEP" | "REPLACE" | "REMOVE"; + avatarUploadId?: string; + expectedSlug?: string; }; header?: never; path: { @@ -11864,6 +11951,9 @@ export interface operations { query: { visibility: string; confirmWarnings?: boolean; + avatarAction?: "KEEP" | "REPLACE" | "REMOVE"; + avatarUploadId?: string; + expectedSlug?: string; }; header?: never; path: { @@ -11891,6 +11981,60 @@ export interface operations { }; }; }; + upload: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: { + content: { + "multipart/form-data": { + /** Format: binary */ + file: string; + }; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarUploadResponse"]; + }; + }; + }; + }; + upload_1: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: { + content: { + "multipart/form-data": { + /** Format: binary */ + file: string; + }; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarUploadResponse"]; + }; + }; + }; + }; withdrawReview_2: { parameters: { query?: never; @@ -12499,7 +12643,7 @@ export interface operations { }; }; }; - submit_2: { + submit: { parameters: { query?: never; header?: never; @@ -12523,7 +12667,7 @@ export interface operations { }; }; }; - submit_3: { + submit_1: { parameters: { query?: never; header?: never; @@ -12839,7 +12983,7 @@ export interface operations { }; }; }; - status: { + status_2: { parameters: { query?: never; header?: never; @@ -12909,7 +13053,7 @@ export interface operations { }; }; }; - status_1: { + status_3: { parameters: { query?: never; header?: never; @@ -15945,6 +16089,50 @@ export interface operations { }; }; }; + uploadPreview: { + parameters: { + query?: never; + header?: never; + path: { + uploadId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; + uploadPreview_1: { + parameters: { + query?: never; + header?: never; + path: { + uploadId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; queue: { parameters: { query?: never; diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 55e332ea..0bf68da7 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -381,11 +381,15 @@ const dashboardPublishRoute = createRoute({ validateSearch: (search: Record): { namespace?: string visibility?: string + updateName?: string + updateSkill?: string resubmitSkill?: string resubmitVersion?: string } => ({ namespace: typeof search.namespace === 'string' && search.namespace ? search.namespace : undefined, visibility: typeof search.visibility === 'string' && search.visibility ? search.visibility : undefined, + updateName: typeof search.updateName === 'string' && search.updateName ? search.updateName : undefined, + updateSkill: typeof search.updateSkill === 'string' && search.updateSkill ? search.updateSkill : undefined, resubmitSkill: typeof search.resubmitSkill === 'string' && search.resubmitSkill ? search.resubmitSkill : undefined, diff --git a/web/src/features/skill/skill-avatar-editor.test.tsx b/web/src/features/skill/skill-avatar-editor.test.tsx new file mode 100644 index 00000000..c7a06b62 --- /dev/null +++ b/web/src/features/skill/skill-avatar-editor.test.tsx @@ -0,0 +1,106 @@ +/** @vitest-environment jsdom */ +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +const api = vi.hoisted(() => ({ stage: vi.fn() })) +vi.mock('@/api/client', () => ({ skillAvatarApi: api, buildApiUrl: (url: string) => url })) +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) +import { SkillAvatarEditor } from './skill-avatar-editor' + +function selectImage() { + fireEvent.change(screen.getByLabelText('skillAvatar.choose'), { target: { files: [new File(['image'], 'avatar.png', { type: 'image/png' })] } }) +} +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(done => { resolve = done }) + return { promise, resolve } +} +const uploaded = { uploadId: 'private-upload-1', previewUrl: '/private/preview', expiresAt: '2099-01-01T00:00:00Z' } + +describe('SkillAvatarEditor staged uploads', () => { + beforeEach(() => { + api.stage.mockReset() + vi.stubGlobal('Image', class { src = ''; naturalWidth = 1024; naturalHeight = 768; decode() { return Promise.resolve() } }) + vi.stubGlobal('URL', { createObjectURL: vi.fn(() => 'blob:chosen'), revokeObjectURL: vi.fn() }) + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({ drawImage: vi.fn() } as unknown as CanvasRenderingContext2D) + vi.spyOn(HTMLCanvasElement.prototype, 'toBlob').mockImplementation(callback => callback(new Blob(['cropped'], { type: 'image/png' }))) + }) + afterEach(() => { cleanup(); vi.restoreAllMocks(); vi.unstubAllGlobals() }) + + it('requires upload before submit and stages the cropped file without associating a skill', async () => { + const change = vi.fn() + const task = deferred() + api.stage.mockReturnValue(task.promise) + render() + selectImage() + expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) + expect(api.stage.mock.calls[0][0]).toBeInstanceOf(Blob) + expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.UPLOADING') + task.resolve(uploaded) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) + expect(screen.getByAltText('skillAvatar.cropPreview').getAttribute('src')).toBe(uploaded.previewUrl) + }) + + it('blocks invalid selections until canceled without touching the active avatar', () => { + const change = vi.fn() + render() + fireEvent.change(screen.getByLabelText('skillAvatar.choose'), { target: { files: [new File(['bad'], 'bad.svg', { type: 'image/svg+xml' })] } }) + expect(screen.getByRole('alert').textContent).toBe('skillAvatar.invalid') + expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) + expect(api.stage).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.discardChange' })) + expect(change).toHaveBeenLastCalledWith({ action: 'KEEP', ready: true }) + expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.KEEP') + }) + + it('keeps submission blocked after upload failure and allows a successful retry', async () => { + const change = vi.fn() + api.stage.mockRejectedValueOnce(new Error('storage unavailable')).mockResolvedValueOnce(uploaded) + render() + selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + expect((await screen.findByRole('alert')).textContent).toBe('storage unavailable') + expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.uploadRetry' })) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) + }) + + it.each(['discardChange', 'remove'] as const)('ignores an old upload completion after %s', async action => { + const change = vi.fn() + const task = deferred() + api.stage.mockReturnValue(task.promise) + render() + selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + await waitFor(() => expect(api.stage).toHaveBeenCalledOnce()) + fireEvent.click(screen.getByRole('button', { name: `skillAvatar.${action}` })) + task.resolve(uploaded) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: action === 'remove' ? 'REMOVE' : 'KEEP', ready: true })) + expect(change.mock.calls.some(([value]) => value.uploadId)).toBe(false) + expect(screen.queryByAltText('skillAvatar.cropPreview')).toBeNull() + }) + + it('invalidates a staged reference when a different image is selected', async () => { + const change = vi.fn() + api.stage.mockResolvedValue(uploaded) + render() + selectImage() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.cropUpload' })) + await waitFor(() => expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', uploadId: uploaded.uploadId, ready: true })) + selectImage() + expect(change).toHaveBeenLastCalledWith({ action: 'REPLACE', ready: false }) + expect(screen.getByRole('status').textContent).toBe('skillAvatar.uploadStates.SELECTED') + }) + + it('previews removal as a draft and cancel restores the original image', () => { + const change = vi.fn() + render() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.remove' })) + expect(change).toHaveBeenLastCalledWith({ action: 'REMOVE', ready: true }) + expect(screen.getByText('F')).toBeTruthy() + expect(api.stage).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.discardChange' })) + expect(document.querySelector('img')?.getAttribute('src')).toBe('/active') + }) +}) diff --git a/web/src/features/skill/skill-avatar-editor.tsx b/web/src/features/skill/skill-avatar-editor.tsx new file mode 100644 index 00000000..70757bb6 --- /dev/null +++ b/web/src/features/skill/skill-avatar-editor.tsx @@ -0,0 +1,100 @@ +import { useEffect, useRef, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { buildApiUrl, skillAvatarApi } from '@/api/client' +import { Button } from '@/shared/ui/button' +import { Input } from '@/shared/ui/input' +import { SkillAvatar } from './skill-avatar' + +export type AvatarEdit = { action: 'KEEP' | 'REPLACE' | 'REMOVE'; uploadId?: string; ready: boolean } +export const KEEP_AVATAR: AvatarEdit = { action: 'KEEP', ready: true } + +async function cropSquare(source: string, zoom: number): Promise { + const image = new Image() + image.src = source + await image.decode() + const canvas = document.createElement('canvas') + canvas.width = canvas.height = 512 + const context = canvas.getContext('2d') + if (!context) throw new Error('Canvas unavailable') + const side = Math.min(image.naturalWidth, image.naturalHeight) / zoom + context.drawImage(image, (image.naturalWidth - side) / 2, (image.naturalHeight - side) / 2, side, side, 0, 0, 512, 512) + return new Promise((resolve, reject) => canvas.toBlob(blob => blob ? resolve(blob) : reject(new Error('Image conversion failed')), 'image/png')) +} + +/** A private upload is staged here; only the parent's successful submit associates it. */ +export function SkillAvatarEditor({ name, currentUrl, canRemove = false, disabled = false, onChange }: { + name: string; currentUrl?: string | null; canRemove?: boolean; disabled?: boolean; onChange: (edit: AvatarEdit) => void +}) { + const { t } = useTranslation() + const [sourceUrl, setSourceUrl] = useState(null) + const [zoom, setZoom] = useState(1) + const [previewUrl, setPreviewUrl] = useState(null) + const [state, setState] = useState<'KEEP' | 'SELECTED' | 'UPLOADING' | 'READY' | 'ERROR' | 'REMOVE'>('KEEP') + const [error, setError] = useState('') + const generation = useRef(0) + const input = useRef(null) + useEffect(() => () => { generation.current += 1 }, []) + useEffect(() => () => { if (sourceUrl) URL.revokeObjectURL(sourceUrl) }, [sourceUrl]) + + function reset(remove = false) { + generation.current += 1 + setSourceUrl(null) + setPreviewUrl(null) + setError('') + setState(remove ? 'REMOVE' : 'KEEP') + if (input.current) input.current.value = '' + onChange(remove ? { action: 'REMOVE', ready: true } : KEEP_AVATAR) + } + + async function upload() { + if (!sourceUrl) return + const attempt = ++generation.current + setState('UPLOADING') + setError('') + onChange({ action: 'REPLACE', ready: false }) + try { + const staged = await skillAvatarApi.stage(await cropSquare(sourceUrl, zoom)) + if (attempt !== generation.current) return + setPreviewUrl(staged.previewUrl) + setState('READY') + onChange({ action: 'REPLACE', uploadId: staged.uploadId, ready: true }) + } catch (cause) { + if (attempt !== generation.current) return + setState('ERROR') + setError(cause instanceof Error ? cause.message : t('skillAvatar.failed')) + } + } + + return
+

{t('skillAvatar.optionalTitle')}

+

{t('skillAvatar.stagedHelp')}

+
+ {state === 'READY' && previewUrl ? {t('skillAvatar.cropPreview')} + : } +

{t(`skillAvatar.uploadStates.${state}`)}

+
+ { + const file = event.target.files?.[0] + if (!file) return + generation.current += 1 + setPreviewUrl(null) + setSourceUrl(null) + onChange({ action: 'REPLACE', ready: false }) + if (!['image/png', 'image/jpeg'].includes(file.type) || file.size > 2 * 1024 * 1024) { + setState('ERROR'); setError(t('skillAvatar.invalid')); return + } + setSourceUrl(URL.createObjectURL(file)) + setZoom(1); setError(''); setState('SELECTED') + }} /> + {sourceUrl && state !== 'READY' &&
+
{t('skillAvatar.cropPreview')}
+ + +
} +
+ {state !== 'KEEP' && } + {canRemove && state !== 'REMOVE' && } +
+ {error &&

{error}

} +
+} diff --git a/web/src/features/skill/skill-avatar-manager.test.tsx b/web/src/features/skill/skill-avatar-manager.test.tsx index edbf10cc..6c8aa0c1 100644 --- a/web/src/features/skill/skill-avatar-manager.test.tsx +++ b/web/src/features/skill/skill-avatar-manager.test.tsx @@ -1,15 +1,15 @@ /** @vitest-environment jsdom */ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { cleanup, render, screen } from '@testing-library/react' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' -const api = vi.hoisted(() => ({ status: vi.fn() })) +const api = vi.hoisted(() => ({ status: vi.fn(), delete: vi.fn(), submit: vi.fn() })) vi.mock('@/api/client', () => ({ skillAvatarApi: api, buildApiUrl: (url: string) => url })) vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) import { SkillAvatarManager } from './skill-avatar-manager' describe('SkillAvatarManager appeal', () => { - afterEach(() => { cleanup(); api.status.mockReset() }) + afterEach(() => { cleanup(); api.status.mockReset(); api.delete.mockReset(); api.submit.mockReset() }) it.each([ ['PENDING', 'BLOCKED', 'NOT_REQUIRED', true], @@ -21,7 +21,7 @@ describe('SkillAvatarManager appeal', () => { api.status.mockResolvedValue({ avatarUrl: null, candidateId: 7, candidateStatus, machineStatus, reviewStatus, candidateUrl: null, reviewReason: null }) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) render() - expect((await screen.findByRole('status')).textContent).toContain(`skillAvatar.states.${candidateStatus}`) + expect(await screen.findByText(new RegExp(`skillAvatar.states.${candidateStatus}`))).toBeTruthy() expect(screen.queryByRole('button', { name: 'skillAvatar.appeal' }) !== null).toBe(expected) }) @@ -36,4 +36,26 @@ describe('SkillAvatarManager appeal', () => { expect(screen.getByText('skillAvatar.automaticRetryStopped')).toBeTruthy() expect(screen.getByRole('button', { name: 'skillAvatar.retry' }).hasAttribute('disabled')).toBe(true) }) + it('renders detail status without upload, removal or retry controls', async () => { + api.status.mockResolvedValue({ avatarUrl: '/active', candidateId: 7, candidateStatus: 'REJECTED', machineStatus: 'BLOCKED', reviewStatus: 'REJECTED', reviewReason: 'not suitable' }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + expect(await screen.findByText('not suitable')).toBeTruthy() + expect(screen.queryByLabelText('skillAvatar.choose')).toBeNull() + expect(screen.queryByRole('button')).toBeNull() + }) + + it('does not remove an avatar until save and preserves the draft when the server rejects it', async () => { + api.status.mockResolvedValue({ avatarUrl: '/active', candidateId: 7, candidateStatus: 'ACTIVE', machineStatus: 'SKIPPED', reviewStatus: 'APPROVED' }) + api.delete.mockRejectedValue(new Error('save failed')) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + fireEvent.click(await screen.findByRole('button', { name: 'skillAvatar.remove' })) + expect(api.delete).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.saveChanges' })) + expect((await screen.findByRole('alert')).textContent).toBe('save failed') + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('team', 'foo')) + expect(screen.getByText('skillAvatar.uploadStates.REMOVE')).toBeTruthy() + }) + }) diff --git a/web/src/features/skill/skill-avatar-manager.tsx b/web/src/features/skill/skill-avatar-manager.tsx index cca3b279..fe63b349 100644 --- a/web/src/features/skill/skill-avatar-manager.tsx +++ b/web/src/features/skill/skill-avatar-manager.tsx @@ -1,89 +1,57 @@ -import { useEffect, useState } from 'react' +import { useState } from 'react' import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { useTranslation } from 'react-i18next' import { buildApiUrl, skillAvatarApi } from '@/api/client' import { Button } from '@/shared/ui/button' import { Card } from '@/shared/ui/card' -import { Input } from '@/shared/ui/input' +import { SkillAvatarEditor, KEEP_AVATAR, type AvatarEdit } from './skill-avatar-editor' -function cropSquare(image: HTMLImageElement, zoom: number): Promise { - const canvas = document.createElement('canvas') - canvas.width = 512 - canvas.height = 512 - const context = canvas.getContext('2d') - if (!context) return Promise.reject(new Error('Canvas unavailable')) - const side = Math.min(image.naturalWidth, image.naturalHeight) / zoom - context.drawImage(image, (image.naturalWidth - side) / 2, (image.naturalHeight - side) / 2, side, side, 0, 0, 512, 512) - return new Promise((resolve, reject) => canvas.toBlob((blob) => blob ? resolve(blob) : reject(new Error('Image conversion failed')), 'image/png')) -} - -export function SkillAvatarManager({ namespace, slug }: { namespace: string; slug: string }) { +export function SkillAvatarManager({ namespace, slug, name = slug, readOnly = false }: { namespace: string; slug: string; name?: string; readOnly?: boolean }) { const { t } = useTranslation() const queryClient = useQueryClient() const key = ['skill-avatar', namespace, slug] const status = useQuery({ queryKey: key, queryFn: () => skillAvatarApi.status(namespace, slug) }) - const [sourceUrl, setSourceUrl] = useState(null) - const [zoom, setZoom] = useState(1) + const [edit, setEdit] = useState(KEEP_AVATAR) const [error, setError] = useState('') - const [busy, setBusy] = useState(false) - useEffect(() => () => { if (sourceUrl) URL.revokeObjectURL(sourceUrl) }, [sourceUrl]) + const [editorVersion, setEditorVersion] = useState(0) const mutation = useMutation({ - mutationFn: async (action: { kind: 'upload'; blob: Blob } | { kind: 'remove' | 'appeal' | 'retry'; id?: number }) => { - if (action.kind === 'upload') return skillAvatarApi.upload(namespace, slug, action.blob) + mutationFn: async (action: { kind: 'upload'; uploadId: string } | { kind: 'remove' | 'appeal' | 'retry'; id?: number }) => { + if (action.kind === 'upload') return skillAvatarApi.submit(namespace, slug, action.uploadId) if (action.kind === 'remove') return skillAvatarApi.delete(namespace, slug) if (action.kind === 'appeal') return skillAvatarApi.appeal(namespace, slug, action.id!) return skillAvatarApi.retry(namespace, slug, action.id!) }, - onSuccess: async () => { + onSuccess: async (_data, action) => { setError('') - setSourceUrl(null) + if (action.kind === 'upload' || action.kind === 'remove') { + setEdit(KEEP_AVATAR) + setEditorVersion(value => value + 1) + } await Promise.all([queryClient.invalidateQueries({ queryKey: key }), queryClient.invalidateQueries({ queryKey: ['skill'] }), queryClient.invalidateQueries({ queryKey: ['skills'] })]) }, onError: (cause) => setError(cause instanceof Error ? cause.message : t('skillAvatar.failed')), }) - async function submit() { - if (!sourceUrl) return - setBusy(true) - try { - const image = new Image() - image.src = sourceUrl - await image.decode() - await mutation.mutateAsync({ kind: 'upload', blob: await cropSquare(image, zoom) }) - } catch (cause) { - setError(cause instanceof Error ? cause.message : t('skillAvatar.failed')) - } finally { setBusy(false) } - } const candidate = status.data const retryCoolingDown = !!candidate?.machineNextRetryAt && Date.parse(candidate.machineNextRetryAt) > Date.now() return (

{t('skillAvatar.title')}

-

{t('skillAvatar.ownerHelp')}

+

{t(readOnly ? 'skillAvatar.detailHelp' : 'skillAvatar.ownerHelp')}

{candidate?.candidateStatus &&

{t('skillAvatar.status')}: {t(`skillAvatar.states.${candidate.candidateStatus}`)} · {t(`skillAvatar.machine.${candidate.machineStatus ?? 'SKIPPED'}`)}

} {candidate?.reviewReason &&

{candidate.reviewReason}

} {candidate?.machineStatus === 'ERROR' && retryCoolingDown &&

{t('skillAvatar.retryAfter', { time: new Date(candidate.machineNextRetryAt!).toLocaleString() })}

} {candidate?.machineStatus === 'ERROR' && candidate.automaticRetryExhausted &&

{t('skillAvatar.automaticRetryStopped')}

} {candidate?.candidateUrl && {t('skillAvatar.candidatePreview')}} - { - const file = event.target.files?.[0] - if (!file) return - if (!['image/png', 'image/jpeg'].includes(file.type) || file.size > 2 * 1024 * 1024) { setError(t('skillAvatar.invalid')); return } - if (sourceUrl) URL.revokeObjectURL(sourceUrl) - setSourceUrl(URL.createObjectURL(file)) - setZoom(1) - setError('') - }} /> - {sourceUrl &&
-
{t('skillAvatar.cropPreview')}
- - -
} + {!readOnly && <> + + + } + {status.isError &&

{t('skillAvatar.failed')}

}
- {(candidate?.avatarUrl || candidate?.candidateStatus === 'PENDING') && } - {candidate?.candidateStatus === 'PENDING' && candidate.machineStatus === 'BLOCKED' + {!readOnly && candidate?.candidateStatus === 'PENDING' && candidate.machineStatus === 'BLOCKED' && candidate.reviewStatus === 'NOT_REQUIRED' && candidate.candidateId - && } - {candidate?.machineStatus === 'ERROR' && candidate.candidateId && } + && } + {!readOnly && candidate?.machineStatus === 'ERROR' && candidate.candidateId && }
{error &&

{error}

} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 75816c58..100920db 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -2596,6 +2596,24 @@ } }, "skillAvatar": { + "submittedNotice": "Avatar submitted separately. Check its review status under My skills → Manage avatar.", + "removedNotice": "Avatar removed; the first letter is shown.", + "detailHelp": "Avatar and skill content are reviewed separately. To change, appeal or retry, select Manage avatar in My skills.", + "optionalTitle": "Skill avatar (optional)", + "stagedHelp": "Choose a PNG or JPEG under 2 MB, crop and upload it first. Changes take effect only after submission succeeds. Avatar review is separate from skill review.", + "cropUpload": "Upload cropped image", + "uploadRetry": "Retry upload", + "discardChange": "Cancel avatar change", + "saveChanges": "Save avatar changes", + "manage": "Manage avatar", + "uploadStates": { + "KEEP": "Current avatar will be kept. No avatar uses the first letter.", + "SELECTED": "Crop and upload before submitting.", + "UPLOADING": "Uploading image…", + "READY": "Uploaded privately. Submit to apply and start avatar review.", + "ERROR": "Upload failed. Retry or cancel this change before submitting.", + "REMOVE": "Avatar will be removed after successful submission." + }, "title": "Skill avatar", "ownerHelp": "Upload and crop a square image. The current avatar stays visible until the new one is approved.", "status": "Status", diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index 2fafb321..3765d5bd 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -2598,6 +2598,24 @@ "reset": "Попробовать снова" }, "skillAvatar": { + "submittedNotice": "Аватар отправлен отдельно. Статус проверки доступен в «Мои навыки → Управление аватаром».", + "removedNotice": "Аватар удалён; отображается первая буква.", + "detailHelp": "Аватар и содержимое навыка проверяются отдельно. Для изменения, апелляции или повторной проверки выберите «Управление аватаром» в «Мои навыки».", + "optionalTitle": "Аватар навыка (необязательно)", + "stagedHelp": "Выберите PNG или JPEG до 2 МБ, обрежьте и загрузите. Изменения применяются после успешной отправки. Аватар и содержимое навыка проверяются отдельно.", + "cropUpload": "Загрузить обрезанное изображение", + "uploadRetry": "Повторить загрузку", + "discardChange": "Отменить изменение аватара", + "saveChanges": "Сохранить аватар", + "manage": "Управление аватаром", + "uploadStates": { + "KEEP": "Текущий аватар сохраняется. Без аватара отображается первая буква.", + "SELECTED": "Обрежьте и загрузите изображение перед отправкой.", + "UPLOADING": "Загрузка изображения…", + "READY": "Изображение загружено в закрытое хранилище. Отправьте для применения и проверки.", + "ERROR": "Ошибка загрузки. Повторите или отмените изменение перед отправкой.", + "REMOVE": "Аватар будет удалён после успешной отправки." + }, "title": "Аватар навыка", "ownerHelp": "Загрузите и обрежьте квадратное изображение. Текущий аватар останется до одобрения нового.", "status": "Статус", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 64b1cd46..d2a4f3a4 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -2595,6 +2595,24 @@ } }, "skillAvatar": { + "submittedNotice": "头像修改已单独提交，可在“我的技能 → 管理头像”查看结果。", + "removedNotice": "头像已移除，恢复首字母。", + "detailHelp": "头像与技能内容分别审核。如需修改、申诉或重试，请在“我的技能”选择“管理头像”。", + "optionalTitle": "技能头像（选填）", + "stagedHelp": "选择 2 MB 以下的 PNG 或 JPEG，裁剪后先上传。提交成功后才应用修改，头像审核与技能内容审核分别进行。", + "cropUpload": "上传裁剪图片", + "uploadRetry": "重试上传", + "discardChange": "取消头像修改", + "saveChanges": "保存头像修改", + "manage": "管理头像", + "uploadStates": { + "KEEP": "保留当前头像；未设置时显示名称首字母。", + "SELECTED": "请先裁剪并上传，成功后才能提交。", + "UPLOADING": "图片上传中…", + "READY": "已上传至私有存储，提交后才应用并进入头像审核。", + "ERROR": "上传失败，请重试或取消头像修改后再提交。", + "REMOVE": "提交成功后移除头像，恢复首字母。" + }, "title": "技能头像", "ownerHelp": "上传并裁剪方形图片。新头像通过审核后才会替换当前头像。", "status": "状态", diff --git a/web/src/pages/dashboard/my-skills.test.ts b/web/src/pages/dashboard/my-skills.test.ts index 80b38107..81c2bb43 100644 --- a/web/src/pages/dashboard/my-skills.test.ts +++ b/web/src/pages/dashboard/my-skills.test.ts @@ -156,6 +156,8 @@ describe('MySkillsPage', () => { to: '/dashboard/publish', search: { namespace: 'team-ai', + updateSkill: 'team-agent', + updateName: 'Team Agent', visibility: 'PRIVATE', }, }) @@ -219,6 +221,8 @@ describe('MySkillsPage', () => { to: '/dashboard/publish', search: { namespace: 'team-ai', + updateSkill: 'default-visibility-agent', + updateName: 'Default Visibility Agent', visibility: 'PUBLIC', }, }) diff --git a/web/src/pages/dashboard/my-skills.tsx b/web/src/pages/dashboard/my-skills.tsx index 2cd714e5..6b05d2a8 100644 --- a/web/src/pages/dashboard/my-skills.tsx +++ b/web/src/pages/dashboard/my-skills.tsx @@ -18,6 +18,8 @@ import { getHeadlineVersion, getPublishedVersion, getOwnerPreviewVersion, hasPen import { formatCompactCount } from '@/shared/lib/number-format' import { toast } from '@/shared/lib/toast' import { buildReturnTo } from '@/shared/lib/auth-route' +import { SkillAvatarManager } from '@/features/skill/skill-avatar-manager' +import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } from '@/shared/ui/dialog' import { ApiError } from '@/api/client' import { getMySkillEmptyStateKey, getMySkillFilters, type MySkillFilter } from './my-skill-filters' @@ -57,6 +59,7 @@ export function MySkillsPage() { // Input remains local until the user explicitly submits the search. const [keywordInput, setKeywordInput] = useState(keyword) + const [avatarTarget, setAvatarTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [archiveTarget, setArchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [unarchiveTarget, setUnarchiveTarget] = useState<{ namespace: string; slug: string; name: string } | null>(null) const [withdrawTarget, setWithdrawTarget] = useState<{ namespace: string; slug: string; name: string; version: string } | null>(null) @@ -109,10 +112,10 @@ export function MySkillsPage() { updateSearch({ q: undefined, namespace: undefined, page: 0 }) } - const handleUpdateSkill = (namespace: string, visibility?: string) => { + const handleUpdateSkill = (namespace: string, slug: string, name: string, visibility?: string) => { navigate({ to: '/dashboard/publish', - search: { namespace, visibility: visibility || 'PUBLIC' }, + search: { namespace, updateSkill: slug, updateName: name, visibility: visibility || 'PUBLIC' }, }) } @@ -420,7 +423,11 @@ export function MySkillsPage() { ) : null} -
+
+ {filter !== 'HIDDEN' && } {canRestoreHidden ? ( })) +vi.mock('@/features/skill/skill-avatar-editor', () => ({ + KEEP_AVATAR: { action: 'KEEP', ready: true }, + SkillAvatarEditor: ({ onChange }: { onChange: (value: { action: 'KEEP' | 'REPLACE' | 'REMOVE'; ready: boolean; uploadId?: string }) => void }) => <> + + + + , +})) +vi.mock('@/shared/lib/navigate-after-overlays', () => ({ navigateAfterOverlays: (fn: () => void) => fn() })) +vi.mock('@/shared/lib/toast', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) +import { PublishPage } from './publish' + +describe('Publish avatar submission', () => { + beforeEach(() => { mocks.publish.mockReset(); mocks.navigate.mockReset(); mocks.search.updateSkill = 'foo' }) + afterEach(cleanup) + + it('blocks publishing a selected image until staged upload completes', async () => { + mocks.publish.mockResolvedValue({ namespace: 'team', slug: 'foo', version: '1.1', status: 'PENDING_REVIEW' }) + render() + fireEvent.click(screen.getByText('choose package')) + fireEvent.click(screen.getByText('image pending')) + expect(screen.getByRole('button', { name: 'publish.confirm' }).hasAttribute('disabled')).toBe(true) + expect(mocks.publish).not.toHaveBeenCalled() + fireEvent.click(screen.getByText('image uploaded')) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledWith(expect.objectContaining({ avatarAction: 'REPLACE', avatarUploadId: 'private-reference', expectedSlug: 'foo' }))) + }) + + it('preserves the uploaded reference after a failed skill submission so retry does not reupload', async () => { + mocks.publish.mockRejectedValueOnce(new Error('package invalid')).mockResolvedValueOnce({ namespace: 'team', slug: 'foo', version: '1.1', status: 'PUBLISHED' }) + render() + fireEvent.click(screen.getByText('choose package')) + fireEvent.click(screen.getByText('image uploaded')) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledTimes(1)) + expect(mocks.navigate).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledTimes(2)) + expect(mocks.publish.mock.calls[1][0]).toEqual(mocks.publish.mock.calls[0][0]) + }) + + it('sends removal only with the successful update request, not while editing', async () => { + mocks.publish.mockResolvedValue({ namespace: 'team', slug: 'foo', version: '1.1', status: 'PENDING_REVIEW' }) + render() + fireEvent.click(screen.getByText('remove draft')) + expect(mocks.publish).not.toHaveBeenCalled() + fireEvent.click(screen.getByText('choose package')) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledWith(expect.objectContaining({ avatarAction: 'REMOVE', expectedSlug: 'foo' }))) + }) + + it('keeps the existing avatar by default when updating only skill content', async () => { + mocks.publish.mockResolvedValue({ namespace: 'team', slug: 'foo', version: '1.1', status: 'PENDING_REVIEW' }) + render() + fireEvent.click(screen.getByText('choose package')) + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledWith(expect.objectContaining({ avatarAction: 'KEEP', avatarUploadId: undefined, expectedSlug: 'foo' }))) + }) + it('clears the avatar draft when navigating to another skill in the same namespace', async () => { + mocks.publish.mockResolvedValue({ namespace: 'team', slug: 'bar', version: '1.1', status: 'PENDING_REVIEW' }) + const view = render() + fireEvent.click(screen.getByText('choose package')) + fireEvent.click(screen.getByText('remove draft')) + mocks.search.updateSkill = 'bar' + view.rerender() + fireEvent.click(screen.getByRole('button', { name: 'publish.confirm' })) + await waitFor(() => expect(mocks.publish).toHaveBeenCalledWith(expect.objectContaining({ avatarAction: 'KEEP', avatarUploadId: undefined, expectedSlug: 'bar' }))) + }) + +}) diff --git a/web/src/pages/dashboard/publish.test.ts b/web/src/pages/dashboard/publish.test.ts index 932a2f7d..08d4d761 100644 --- a/web/src/pages/dashboard/publish.test.ts +++ b/web/src/pages/dashboard/publish.test.ts @@ -5,6 +5,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const useSearchMock = vi.fn() const selectRecords: Array<{ value?: string }> = [] +vi.mock('@tanstack/react-query', () => ({ useQuery: () => ({ data: null, isLoading: false, isError: false }) })) + vi.mock('@tanstack/react-router', () => ({ useNavigate: () => vi.fn(), useSearch: () => useSearchMock(), @@ -20,6 +22,8 @@ vi.mock('react-i18next', async () => { } }) +vi.mock('@/features/skill/skill-avatar-editor', () => ({ SkillAvatarEditor: () => null, KEEP_AVATAR: { action: 'KEEP', ready: true } })) + vi.mock('@/features/publish/upload-zone', () => ({ UploadZone: () => null, })) @@ -49,6 +53,7 @@ vi.mock('@/shared/ui/card', () => ({ })) vi.mock('@/shared/hooks/use-skill-queries', () => ({ + useSkillDetail: () => ({ data: null, isLoading: false, isError: false }), usePublishSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), })) diff --git a/web/src/pages/dashboard/publish.tsx b/web/src/pages/dashboard/publish.tsx index eba31285..962a76c1 100644 --- a/web/src/pages/dashboard/publish.tsx +++ b/web/src/pages/dashboard/publish.tsx @@ -1,4 +1,5 @@ import { useEffect, useState } from 'react' +import { useQuery } from '@tanstack/react-query' import { useNavigate, useSearch } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { UploadZone } from '@/features/publish/upload-zone' @@ -22,13 +23,14 @@ import { } from '@/shared/ui/select' import { Label } from '@/shared/ui/label' import { Card } from '@/shared/ui/card' +import { SkillAvatarEditor, KEEP_AVATAR, type AvatarEdit } from '@/features/skill/skill-avatar-editor' import { usePublishSkill } from '@/shared/hooks/use-skill-queries' import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' import { ConfirmDialog } from '@/shared/components/confirm-dialog' import { DashboardPageHeader } from '@/shared/components/dashboard-page-header' import { navigateAfterOverlays } from '@/shared/lib/navigate-after-overlays' import { toast } from '@/shared/lib/toast' -import { ApiError } from '@/api/client' +import { ApiError, skillAvatarApi } from '@/api/client' const EMPTY_NAMESPACE_VALUE = '__select_namespace__' @@ -37,6 +39,8 @@ export function PublishPage() { const navigate = useNavigate() const search = useSearch({ from: '/dashboard/publish' }) const prefill = normalizePublishPrefill(search) + const updateSlug = typeof search.updateSkill === 'string' ? search.updateSkill : prefill.resubmitSkill + const [avatarEdit, setAvatarEdit] = useState(KEEP_AVATAR) const [selectedFile, setSelectedFile] = useState(null) const [namespaceSlug, setNamespaceSlug] = useState(prefill.namespace) const [visibility, setVisibility] = useState(prefill.visibility) @@ -46,6 +50,8 @@ export function PublishPage() { const { data: namespaces, isLoading: isLoadingNamespaces } = useMyNamespaces() const publishMutation = usePublishSkill() + const avatarTarget = useQuery({ queryKey: ['skill-avatar', namespaceSlug, updateSlug], queryFn: () => skillAvatarApi.status(namespaceSlug, updateSlug), enabled: !!namespaceSlug && !!updateSlug }) + const avatarUnavailable = !!updateSlug && (avatarTarget.isLoading || avatarTarget.isError) const selectedNamespace = namespaces?.find((ns) => ns.slug === namespaceSlug) const namespaceOnlyLabel = selectedNamespace?.type === 'GLOBAL' ? t('publish.visibilityOptions.loggedInUsersOnly') @@ -54,7 +60,8 @@ export function PublishPage() { useEffect(() => { setNamespaceSlug(prefill.namespace) setVisibility(prefill.visibility) - }, [prefill.namespace, prefill.visibility]) + setAvatarEdit(KEEP_AVATAR) + }, [prefill.namespace, prefill.visibility, updateSlug]) const handleRemoveSelectedFile = () => { setSelectedFile(null) @@ -81,7 +88,7 @@ export function PublishPage() { } const publishSkill = async (confirmWarnings = false) => { - if (!selectedFile || !namespaceSlug) { + if (!selectedFile || !namespaceSlug || !avatarEdit.ready || avatarUnavailable) { toast.error(t('publish.selectRequired')) return } @@ -92,19 +99,23 @@ export function PublishPage() { file: selectedFile, visibility, confirmWarnings, + avatarAction: avatarEdit.action, + avatarUploadId: avatarEdit.uploadId, + expectedSlug: updateSlug || undefined, }) setPrecheckWarnings([]) setWarningDialogOpen(false) + const avatarNotice = avatarEdit.action === 'REPLACE' ? t('skillAvatar.submittedNotice') : avatarEdit.action === 'REMOVE' ? t('skillAvatar.removedNotice') : '' const skillLabel = `${result.namespace}/${result.slug}@${result.version}` if (result.status === 'PUBLISHED') { toast.success( t('publish.publishedTitle'), - t('publish.publishedDescription', { skill: skillLabel }) + [t('publish.publishedDescription', { skill: skillLabel }), avatarNotice].filter(Boolean).join(' ') ) } else { toast.success( t('publish.pendingReviewTitle'), - t('publish.pendingReviewDescription', { skill: skillLabel }) + [t('publish.pendingReviewDescription', { skill: skillLabel }), avatarNotice].filter(Boolean).join(' ') ) } navigateAfterOverlays(() => { @@ -187,9 +198,11 @@ export function PublishPage() {
) : (