From 17a18b931973fd99c4fd223f80a4e75be4bcf3a3 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:42:26 +0800 Subject: [PATCH] fix(avatar): recover pending reviews and cover moderation flows Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/26-skill-avatars.md | 4 +- server/skillhub-app/pom.xml | 6 + .../controller/portal/SkillController.java | 6 +- .../dto/SkillAvatarStatusResponse.java | 3 +- .../SkillAvatarScopeReviewListener.java | 43 + .../service/SkillAvatarAppService.java | 139 ++- .../service/SkillSearchAppService.java | 13 +- .../task/SkillAvatarMachineRecoveryTask.java | 54 ++ .../SkillAvatarScopeReviewRecoveryTask.java | 50 + .../db/migration/V69__skill_avatars.sql | 6 + .../SkillAvatarScopeReviewListenerTest.java | 45 + .../SkillAvatarMachineRecoveryQueryTest.java | 73 ++ .../SkillAvatarScopeRecoveryQueryTest.java | 125 +++ .../AvatarReviewSettingsServiceTest.java | 69 ++ .../service/SkillAvatarAppServiceTest.java | 395 ++++++++ .../skillhub/service/SkillAvatarUrlsTest.java | 20 + .../SkillAvatarMachineRecoveryTaskTest.java | 47 + ...killAvatarScopeReviewRecoveryTaskTest.java | 69 ++ .../policy/RouteSecurityPolicyRegistry.java | 4 + .../RouteSecurityPolicyRegistryTest.java | 14 + .../domain/skill/SkillAvatarCandidate.java | 59 +- .../skill/SkillAvatarCandidateTest.java | 32 + .../SkillAvatarCandidateJpaRepository.java | 21 + .../infra/jpa/SkillJpaRepository.java | 34 + web/src/api/generated/schema.d.ts | 881 +++++++++++++++++- web/src/api/types.ts | 6 +- .../skill/avatar-review-settings.test.tsx | 34 + .../features/skill/avatar-review-settings.tsx | 1 + .../skill/skill-avatar-manager.test.tsx | 39 + .../features/skill/skill-avatar-manager.tsx | 9 +- web/src/features/skill/skill-avatar.test.tsx | 22 + web/src/features/skill/skill-card.test.ts | 2 + web/src/features/skill/skill-card.tsx | 2 +- web/src/i18n/locales/en.json | 4 + web/src/i18n/locales/ru.json | 4 + web/src/i18n/locales/zh.json | 4 + web/src/pages/admin/system-config.test.tsx | 3 + .../pages/dashboard/namespace-reviews.test.ts | 2 + web/src/pages/dashboard/reviews.test.ts | 2 + web/src/pages/dashboard/reviews.tsx | 2 +- 40 files changed, 2293 insertions(+), 55 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListener.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTask.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTask.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListenerTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarMachineRecoveryQueryTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarScopeRecoveryQueryTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AvatarReviewSettingsServiceTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarUrlsTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTaskTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTaskTest.java create mode 100644 web/src/features/skill/avatar-review-settings.test.tsx create mode 100644 web/src/features/skill/skill-avatar-manager.test.tsx create mode 100644 web/src/features/skill/skill-avatar.test.tsx diff --git a/docs/26-skill-avatars.md b/docs/26-skill-avatars.md index 958287f5..8854e55a 100644 --- a/docs/26-skill-avatars.md +++ b/docs/26-skill-avatars.md @@ -8,6 +8,8 @@ 团队范围由团队管理员审核，全局范围由平台管理员审核。头像只能由技能所有者提交、替换或删除；管理员可审核和撤下，不可代传。新申请使旧待审申请失效；旧审核决定不能覆盖新头像。被拒绝、替换、删除的候选图片私有保留 30 天后清理，文字审核记录继续保留。头像撤下后显示首字母兜底。 +技能发布后若可见范围扩大，旧头像会立即从新增受众的详情中隐藏，系统从私有存储复制同一图片为新的候选，按目标范围重新走当前审核策略。已有相同目标范围的待审申请不会被覆盖；新旧候选使用不同对象键，避免旧文件到期清理影响当前头像。自动建候选失败时旧头像仍保持隐藏；每五分钟分批检查并补建候选。若候选已建但机器读取或审核失败，状态记为 `ERROR`，一小时冷却后自动重试；同一候选自动审核最多调用三次（含首次），到达上限后由所有者在冷却结束后主动重试或重新提交。普通上传在机器调用前宕机留下的 `PENDING` 候选，以及机器处理中宕机留下的 `PROCESSING` 候选，十分钟后也由补偿任务重领。机器调用前后分别使用短事务核验候选指针，外部调用期间不持有数据库锁；过期尝试的迟到结果不会激活头像。当前发布版已驳回的自动申请不会反复入队，历史发布版的驳回不阻止新发布。服务日志仅记录技能或候选 ID 和异常类型，不记录可能包含凭证的供应商请求信息。`PRIVATE` 头像及候选图片只有技能所有者可以读取。 + ## 部署配置 在部署密钥配置中指定唯一供应商及其凭证，然后才可从后台开启机器审核。Spring 属性如下；将值映射到部署 Secret/环境变量时请遵循当前部署平台的配置方式，不要写入仓库或后台数据库： @@ -20,7 +22,7 @@ 后台只显示两个启停开关，不回显供应商或密钥。阿里云适配器使用上海地域的 `profilePhotoCheck`，通过短期上传令牌把私有候选送到官方 OSS 临时对象，再调用同步审核；不会公开候选 URL。官方返回 `RiskLevel=none/low` 通过，`medium/high` 保持私有并允许人工复核。把 `medium` 归为阻止是本产品策略。响应非 200、空结果或未知值均不放行。机器接口可能按量计费，开启前由部署方确认供应商账号与费用。 -讯飞官方图片审核接口要求把 `accessKeySecret` 放在请求 URL 参数中。应用不会记录完整 URL 或异常原文，但反向代理、出口代理、链路追踪及第三方日志仍可能记录该 URL。部署方使用讯飞适配器前，必须确认相关网络组件不采集请求 URL，限制凭证权限并建立轮换机制。若无法满足，先不要开启该适配器。 +讯飞官方图片审核接口要求把 `accessKeySecret` 放在请求 URL 参数中。应用不会记录完整 URL 或异常原文，但 Java HTTP 客户端调试日志、反向代理、出口代理、链路追踪及第三方日志仍可能记录该 URL。部署方使用讯飞适配器前，必须确认相关组件不采集请求 URL，限制凭证权限并建立轮换机制。若无法满足，先不要开启该适配器。 ## 与原 Issue 的差异 diff --git a/server/skillhub-app/pom.xml b/server/skillhub-app/pom.xml index 0dc91ad7..856939bd 100644 --- a/server/skillhub-app/pom.xml +++ b/server/skillhub-app/pom.xml @@ -28,6 +28,12 @@ com.aliyun.oss aliyun-sdk-oss 3.16.3 + + + commons-logging + commons-logging + + org.springframework.boot diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java index 1062db56..d81e1637 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java @@ -130,8 +130,10 @@ public class SkillController extends BaseApiController { .filter(SkillSuiteReferenceResponse::currentSkillEntry) .toList(), memberOfSuites, - SkillAvatarUrls.current(detail.activeAvatarId(), detail.activeAvatarAudience(), - SkillVisibility.valueOf(detail.visibility()), namespace, detail.slug()) + SkillVisibility.PRIVATE == SkillVisibility.valueOf(detail.visibility()) + && !detail.ownerId().equals(userId) ? null + : SkillAvatarUrls.current(detail.activeAvatarId(), detail.activeAvatarAudience(), + SkillVisibility.valueOf(detail.visibility()), namespace, detail.slug()) ); return ok("response.success.read", response); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarStatusResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarStatusResponse.java index d0439e72..4d62d69b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarStatusResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillAvatarStatusResponse.java @@ -2,4 +2,5 @@ package com.iflytek.skillhub.dto; public record SkillAvatarStatusResponse(String avatarUrl, Long candidateId, String candidateStatus, String machineStatus, String reviewStatus, String reviewReason, - String candidateUrl, String namespace, String slug) {} + String candidateUrl, String namespace, String slug, + java.time.Instant machineNextRetryAt, boolean automaticRetryExhausted) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListener.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListener.java new file mode 100644 index 00000000..1781bce2 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListener.java @@ -0,0 +1,43 @@ +package com.iflytek.skillhub.listener; + +import com.iflytek.skillhub.domain.event.SkillPublishedEvent; +import com.iflytek.skillhub.dto.SkillAvatarStatusResponse; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillAvatarAppService; +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 SkillAvatarScopeReviewListener { + private static final Logger log = LoggerFactory.getLogger(SkillAvatarScopeReviewListener.class); + private final SkillAvatarAppService avatars; + + public SkillAvatarScopeReviewListener(SkillAvatarAppService avatars) { + this.avatars = avatars; + } + + @Async("skillhubEventExecutor") + @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = true) + public void onSkillPublished(SkillPublishedEvent event) { + reviewScope(event.skillId(), event.publisherId()); + } + + public void reviewScope(Long skillId, String publisherId) { + try { + SkillAvatarStatusResponse candidate = avatars.prepareScopeReview(skillId, publisherId); + if (candidate != null && ("PENDING".equals(candidate.machineStatus()) + || "PROCESSING".equals(candidate.machineStatus()) + || "ERROR".equals(candidate.machineStatus()))) { + avatars.processMachine(candidate.candidateId(), new AuditRequestContext(null, null)); + } + } catch (Exception failure) { + // Provider errors can include credential-bearing request URLs. Never log their message or stack here. + log.warn("Avatar scope review failed for skill {}, failureType={}", + skillId, 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 5183be87..fbb2c2ad 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 @@ -38,6 +38,10 @@ import java.util.Set; import java.util.UUID; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -55,13 +59,14 @@ public class SkillAvatarAppService { private final AuditLogService audit; private final RequestIdAccessor requestIds; private final EntityManager entityManager; + private final TransactionTemplate machineTransactions; public SkillAvatarAppService(SkillJpaRepository skills, NamespaceRepository namespaces, SkillAvatarCandidateJpaRepository candidates, ObjectStorageService storage, SkillAvatarImageProcessor imageProcessor, AvatarReviewSettingsService settings, AvatarModerationGateway moderation, VisibilityChecker visibility, SkillQueryService skillQueries, AuditLogService audit, RequestIdAccessor requestIds, - EntityManager entityManager) { + EntityManager entityManager, PlatformTransactionManager transactionManager) { this.skills = skills; this.namespaces = namespaces; this.candidates = candidates; @@ -74,6 +79,8 @@ public class SkillAvatarAppService { this.audit = audit; this.requestIds = requestIds; this.entityManager = entityManager; + this.machineTransactions = new TransactionTemplate(transactionManager); + this.machineTransactions.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); } @Transactional @@ -82,6 +89,52 @@ public class SkillAvatarAppService { Skill skill = lockedSkill(namespaceSlug, slug); requireOwner(skill, actor); byte[] normalized = imageProcessor.normalize(image); + return createCandidate(skill, namespaceSlug, actor, actor, "SKILL_AVATAR_SUBMIT", normalized, context); + } + + /** Reassesses the existing image when publication expands its audience. */ + @Transactional(propagation = Propagation.REQUIRES_NEW) + public SkillAvatarStatusResponse prepareScopeReview(Long skillId, String publisher) { + Skill skill = skills.findLockedById(skillId).orElseThrow(() -> notFound()); + if (skill.getActiveAvatarId() == null || skill.getActiveAvatarAudience() == null + || SkillAvatarUrls.current(skill, namespaceSlug(skill)) != null) { + return null; + } + if (skill.getPendingAvatarId() != null) { + SkillAvatarCandidate pending = candidates.findById(skill.getPendingAvatarId()).orElse(null); + if (pending != null && pending.getStatus() == SkillAvatarCandidate.Status.PENDING + && pending.getAudience() == skill.getVisibility()) { + 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) + ? status(skill, pending, namespaceSlug(skill)) : null; + } + } + if (candidates.existsBySkillIdAndAudienceAndScopeReviewVersionIdAndStatus(skill.getId(), + skill.getVisibility(), skill.getLatestVersionId(), SkillAvatarCandidate.Status.REJECTED)) { + return null; + } + SkillAvatarCandidate active = findCandidate(skill.getActiveAvatarId()); + if (active.getStatus() != SkillAvatarCandidate.Status.ACTIVE || active.getObjectKey() == null) { + return null; + } + byte[] image; + try (InputStream input = storage.getObject(active.getObjectKey())) { + image = input.readAllBytes(); + } catch (IOException failure) { + throw new IllegalStateException("Cannot read private avatar candidate", failure); + } + // A fresh object key keeps 30-day candidate cleanup from deleting the active image. + return createCandidate(skill, namespaceSlug(skill), skill.getOwnerId(), + publisher == null ? skill.getOwnerId() : publisher, "SKILL_AVATAR_SCOPE_REVIEW", image, + new AuditRequestContext(null, null)); + } + + 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(); @@ -101,8 +154,11 @@ public class SkillAvatarAppService { if (skill.getPendingAvatarId() != null) { candidates.findById(skill.getPendingAvatarId()).ifPresent(SkillAvatarCandidate::supersede); } - SkillAvatarCandidate candidate = new SkillAvatarCandidate(skill.getId(), actor, key, sha256, + SkillAvatarCandidate candidate = new SkillAvatarCandidate(skill.getId(), submittedBy, key, sha256, skill.getVisibility(), machineEnabled, manualEnabled, provider); + if ("SKILL_AVATAR_SCOPE_REVIEW".equals(action)) { + candidate.markScopeReviewVersion(skill.getLatestVersionId()); + } if (machineEnabled) { // Only the machine verdict is reusable. Every new audience has its own human decision. boolean reusedPass = !candidates.findByContentSha256AndMachineProviderAndMachineEnabledTrueAndManualEnabledAndMachineStatus( @@ -123,46 +179,56 @@ public class SkillAvatarAppService { || candidate.getMachineStatus() == SkillAvatarCandidate.MachineStatus.PASSED && !manualEnabled) { activate(skill, candidate); } - log(actor, "SKILL_AVATAR_SUBMIT", candidate.getId(), context, + log(auditActor, action, candidate.getId(), context, AuditDetail.builder().put("skillId", skill.getId()).put("audience", candidate.getAudience().name()).build()); return status(skill, candidate, namespaceSlug); } - /** Runs outside submit's transaction; result application locks the Skill before changing pointers. */ - @Transactional + /** Claims and finishes in short transactions; the external call holds no database lock. */ public SkillAvatarStatusResponse processMachine(long candidateId, AuditRequestContext context) { - SkillAvatarCandidate candidate = findCandidate(candidateId); - if (candidate.getStatus() != SkillAvatarCandidate.Status.PENDING - || candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.PENDING - && candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.ERROR) { - throw new DomainConflictException("error.skill.avatar.notPending"); - } - byte[] bytes; - try (InputStream input = storage.getObject(candidate.getObjectKey())) { - bytes = input.readAllBytes(); - } catch (IOException failure) { - throw new IllegalStateException("Cannot read private avatar candidate", failure); - } - AvatarModerationProvider.Verdict verdict = moderation.moderate(candidate.getMachineProvider(), bytes); - Skill skill = skills.findLockedById(candidate.getSkillId()).orElseThrow(() -> notFound()); - entityManager.refresh(candidate); - if (!Objects.equals(skill.getPendingAvatarId(), candidateId) - || candidate.getStatus() != SkillAvatarCandidate.Status.PENDING) { - return status(skill, candidate, namespaceSlug(skill)); - } - candidate.machineResult(switch (verdict) { - case PASS -> SkillAvatarCandidate.MachineStatus.PASSED; - case BLOCK -> SkillAvatarCandidate.MachineStatus.BLOCKED; - case ERROR -> SkillAvatarCandidate.MachineStatus.ERROR; + MachineClaim claim = machineTransactions.execute(ignored -> { + SkillAvatarCandidate candidate = findCandidate(candidateId); + Skill skill = skills.findLockedById(candidate.getSkillId()).orElseThrow(() -> notFound()); + entityManager.refresh(candidate); + String attemptId = UUID.randomUUID().toString(); + Instant now = Instant.now(); + if (!Objects.equals(skill.getPendingAvatarId(), candidateId) + || !candidate.claimMachineAttempt(attemptId, now, now.plusSeconds(600))) { + throw new DomainConflictException("error.skill.avatar.notPending"); + } + return new MachineClaim(candidate.getObjectKey(), candidate.getMachineProvider(), attemptId); }); - if (verdict == AvatarModerationProvider.Verdict.PASS && !candidate.isManualEnabled()) { - activate(skill, candidate); + AvatarModerationProvider.Verdict verdict; + try (InputStream input = storage.getObject(claim.objectKey())) { + verdict = moderation.moderate(claim.provider(), input.readAllBytes()); + } catch (IOException | RuntimeException failure) { + // Leave an explicit retriable state. Storage and provider exceptions can contain secret URLs. + verdict = AvatarModerationProvider.Verdict.ERROR; } - log(candidate.getSubmittedBy(), "SKILL_AVATAR_MACHINE_RESULT", candidate.getId(), context, - AuditDetail.of("verdict", verdict.name())); - return status(skill, candidate, namespaceSlug(skill)); + AvatarModerationProvider.Verdict result = verdict; + return machineTransactions.execute(ignored -> { + SkillAvatarCandidate candidate = findCandidate(candidateId); + Skill skill = skills.findLockedById(candidate.getSkillId()).orElseThrow(() -> notFound()); + entityManager.refresh(candidate); + if (!Objects.equals(skill.getPendingAvatarId(), candidateId) + || !candidate.completeMachineAttempt(claim.attemptId(), switch (result) { + case PASS -> SkillAvatarCandidate.MachineStatus.PASSED; + case BLOCK -> SkillAvatarCandidate.MachineStatus.BLOCKED; + case ERROR -> SkillAvatarCandidate.MachineStatus.ERROR; + })) { + return status(skill, candidate, namespaceSlug(skill)); + } + if (result == AvatarModerationProvider.Verdict.PASS && !candidate.isManualEnabled()) { + activate(skill, candidate); + } + log(candidate.getSubmittedBy(), "SKILL_AVATAR_MACHINE_RESULT", candidate.getId(), context, + AuditDetail.of("verdict", result.name())); + return status(skill, candidate, namespaceSlug(skill)); + }); } + private record MachineClaim(String objectKey, String provider, String attemptId) {} + @Transactional(readOnly = true) public SkillAvatarStatusResponse ownerStatus(String namespaceSlug, String slug, String actor) { Skill skill = findSkill(namespaceSlug, slug); @@ -177,6 +243,7 @@ public class SkillAvatarAppService { skillQueries.getSkillDetail(namespaceSlug, slug, actor, namespaceRoles == null ? Map.of() : namespaceRoles); Skill skill = findSkill(namespaceSlug, slug); if (!visibility.canAccess(skill, actor, namespaceRoles, platformRoles) + || skill.getVisibility() == SkillVisibility.PRIVATE && !skill.getOwnerId().equals(actor) || SkillAvatarUrls.current(skill, namespaceSlug) == null) { throw notFound(); } @@ -193,6 +260,7 @@ public class SkillAvatarAppService { Skill skill = findSkill(namespaceSlug, slug); SkillAvatarCandidate candidate = findCandidate(candidateId); 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)) { throw notFound(); } @@ -283,6 +351,7 @@ public class SkillAvatarAppService { SkillAvatarCandidate.Status.PENDING, SkillAvatarCandidate.ReviewStatus.APPEAL_PENDING).stream()) .filter(candidate -> candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.ERROR) .filter(candidate -> candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.PENDING) + .filter(candidate -> candidate.getMachineStatus() != SkillAvatarCandidate.MachineStatus.PROCESSING) .filter(candidate -> ((SkillRepository) skills).findById(candidate.getSkillId()) .filter(skill -> canReview(skill, actor, namespaceRoles, platformRoles)).isPresent()) .map(candidate -> { @@ -366,7 +435,9 @@ public class SkillAvatarAppService { candidate == null ? null : candidate.getReviewReason(), candidate == null || candidate.getObjectKey() == null ? null : SkillAvatarUrls.candidate(namespaceSlug, skill, candidate.getId()), - namespaceSlug, skill.getSlug()); + namespaceSlug, skill.getSlug(), + candidate == null ? null : candidate.getMachineNextRetryAt(), + candidate != null && candidate.getMachineAttemptCount() >= 3); } private String sha256(byte[] image) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java index 8af16c49..60bb9cdd 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillSearchAppService.java @@ -187,7 +187,7 @@ public class SkillSearchAppService { normalizeLabelSlugs(labelSlugs), requireInstallableLatest )); - List pageItems = mapVisibleSkillSummaries(result.skillIds()); + List pageItems = mapVisibleSkillSummaries(result.skillIds(), scope.userId()); return new SearchResponse(pageItems, result.total(), page, size); } @@ -202,7 +202,7 @@ public class SkillSearchAppService { .toList(); } - private List mapVisibleSkillSummaries(List skillIds) { + private List mapVisibleSkillSummaries(List skillIds, String userId) { if (skillIds.isEmpty()) { return List.of(); } @@ -235,7 +235,8 @@ public class SkillSearchAppService { skill, namespaceSlugsById, ownersById, - projectionsBySkillId.get(skill.getId()) + projectionsBySkillId.get(skill.getId()), + userId )) .toList(); } @@ -244,7 +245,8 @@ public class SkillSearchAppService { Skill skill, Map namespaceSlugsById, Map ownersById, - SkillLifecycleProjectionService.Projection projection) { + SkillLifecycleProjectionService.Projection projection, + String userId) { String namespaceSlug = namespaceSlugsById.get(skill.getNamespaceId()); UserAccount owner = ownersById.get(skill.getOwnerId()); SkillLifecycleProjectionService.VersionProjection headlineVersion = projection.headlineVersion(); @@ -275,7 +277,8 @@ public class SkillSearchAppService { ? complianceSnapshotProjectionService.fromParsedMetadataJson(headlineVersion.parsedMetadataJson()) : null, null, - SkillAvatarUrls.current(skill, namespaceSlug) + skill.getVisibility() == com.iflytek.skillhub.domain.skill.SkillVisibility.PRIVATE + && !skill.getOwnerId().equals(userId) ? null : SkillAvatarUrls.current(skill, namespaceSlug) ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTask.java new file mode 100644 index 00000000..ddc40e18 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTask.java @@ -0,0 +1,54 @@ +package com.iflytek.skillhub.task; + +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.infra.jpa.SkillAvatarCandidateJpaRepository; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillAvatarAppService; +import java.time.Instant; +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.data.domain.PageRequest; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +/** Recovers ordinary uploads abandoned before or during machine moderation. */ +@Component +public class SkillAvatarMachineRecoveryTask { + private static final Logger log = LoggerFactory.getLogger(SkillAvatarMachineRecoveryTask.class); + private final SkillAvatarCandidateJpaRepository candidates; + private final SkillAvatarAppService avatars; + private final AtomicLong lastScannedId = new AtomicLong(); + + public SkillAvatarMachineRecoveryTask(SkillAvatarCandidateJpaRepository candidates, SkillAvatarAppService avatars) { + this.candidates = candidates; + this.avatars = avatars; + } + + @Scheduled(fixedDelay = 300000) + public void recover() { + List batch = findAfter(lastScannedId.get()); + if (batch.isEmpty() && lastScannedId.get() != 0L) { + lastScannedId.set(0L); + batch = findAfter(0L); + } + for (SkillAvatarCandidate candidate : batch) { + lastScannedId.set(candidate.getId()); + try { + avatars.processMachine(candidate.getId(), new AuditRequestContext(null, null)); + } catch (Exception failure) { + // Provider exceptions may contain credential-bearing URLs; log no message or stack. + log.warn("Avatar machine recovery failed for candidate {}, failureType={}", + candidate.getId(), failure.getClass().getSimpleName()); + } + } + } + + private List findAfter(Long afterId) { + Instant now = Instant.now(); + return candidates.findAbandonedMachineCandidates(afterId, SkillAvatarCandidate.Status.PENDING, + SkillAvatarCandidate.MachineStatus.PENDING, SkillAvatarCandidate.MachineStatus.PROCESSING, + now.minusSeconds(600), now, PageRequest.of(0, 100)); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTask.java new file mode 100644 index 00000000..cc2721e7 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTask.java @@ -0,0 +1,50 @@ +package com.iflytek.skillhub.task; + +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.listener.SkillAvatarScopeReviewListener; +import org.springframework.data.domain.PageRequest; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; +import java.time.Instant; + +/** Recovers scope reviews lost after publication because private storage was unavailable. */ +@Component +public class SkillAvatarScopeReviewRecoveryTask { + private final SkillJpaRepository skills; + private final SkillAvatarScopeReviewListener listener; + private final AtomicLong lastScannedId = new AtomicLong(); + + public SkillAvatarScopeReviewRecoveryTask(SkillJpaRepository skills, SkillAvatarScopeReviewListener listener) { + this.skills = skills; + this.listener = listener; + } + + @Scheduled(fixedDelay = 300000) + public void recover() { + List batch = findAfter(lastScannedId.get()); + if (batch.isEmpty() && lastScannedId.get() != 0L) { + lastScannedId.set(0L); + batch = findAfter(0L); + } + for (var skill : batch) { + lastScannedId.set(skill.getId()); + listener.reviewScope(skill.getId(), null); + } + } + + private List findAfter(Long afterId) { + Instant now = Instant.now(); + return skills.findScopeReviewRecoveryCandidates(afterId, SkillStatus.ACTIVE, SkillVisibility.PRIVATE, + SkillVisibility.NAMESPACE_ONLY, SkillVisibility.PUBLIC, + SkillAvatarCandidate.Status.PENDING, SkillAvatarCandidate.MachineStatus.PENDING, + SkillAvatarCandidate.MachineStatus.PROCESSING, SkillAvatarCandidate.MachineStatus.ERROR, + 3, SkillAvatarCandidate.Status.REJECTED, now.minusSeconds(600), now, PageRequest.of(0, 100)); + } +} diff --git a/server/skillhub-app/src/main/resources/db/migration/V69__skill_avatars.sql b/server/skillhub-app/src/main/resources/db/migration/V69__skill_avatars.sql index 18e990de..19171b2a 100644 --- a/server/skillhub-app/src/main/resources/db/migration/V69__skill_avatars.sql +++ b/server/skillhub-app/src/main/resources/db/migration/V69__skill_avatars.sql @@ -12,6 +12,12 @@ CREATE TABLE skill_avatar_candidate ( machine_enabled BOOLEAN NOT NULL, manual_enabled BOOLEAN NOT NULL, machine_provider VARCHAR(32), + scope_review_version_id BIGINT, + scope_review BOOLEAN NOT NULL DEFAULT FALSE, + machine_attempt_id VARCHAR(36), + machine_lease_until TIMESTAMP, + machine_next_retry_at TIMESTAMP, + machine_attempt_count INTEGER NOT NULL DEFAULT 0, machine_status VARCHAR(32) NOT NULL, review_status VARCHAR(32) NOT NULL, status VARCHAR(32) NOT NULL, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListenerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListenerTest.java new file mode 100644 index 00000000..51b3fbca --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SkillAvatarScopeReviewListenerTest.java @@ -0,0 +1,45 @@ +package com.iflytek.skillhub.listener; + +import com.iflytek.skillhub.domain.event.SkillPublishedEvent; +import com.iflytek.skillhub.service.SkillAvatarAppService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.after; + +@SpringBootTest +@ActiveProfiles("test") +class SkillAvatarScopeReviewListenerTest { + @Autowired private ApplicationEventPublisher events; + @Autowired private PlatformTransactionManager transactions; + @MockBean private SkillAvatarAppService avatars; + + @Test + void committedPublicationTriggersScopeReview() { + new TransactionTemplate(transactions).executeWithoutResult(ignored -> { + events.publishEvent(new SkillPublishedEvent(912345L, 1L, "publisher")); + verify(avatars, never()).prepareScopeReview(912345L, "publisher"); + }); + + verify(avatars, timeout(5000)).prepareScopeReview(912345L, "publisher"); + } + + @Test + void rolledBackPublicationDoesNotTriggerScopeReview() { + new TransactionTemplate(transactions).executeWithoutResult(status -> { + events.publishEvent(new SkillPublishedEvent(912346L, 1L, "publisher")); + status.setRollbackOnly(); + }); + + verify(avatars, after(1000).never()).prepareScopeReview(912346L, "publisher"); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarMachineRecoveryQueryTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarMachineRecoveryQueryTest.java new file mode 100644 index 00000000..36042c2a --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarMachineRecoveryQueryTest.java @@ -0,0 +1,73 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillAvatarCandidateJpaRepository; +import jakarta.persistence.EntityManager; +import java.time.Instant; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.data.domain.PageRequest; +import org.springframework.test.context.ActiveProfiles; + +import static org.assertj.core.api.Assertions.assertThat; + +@DataJpaTest +@ActiveProfiles("test") +class SkillAvatarMachineRecoveryQueryTest { + @Autowired private EntityManager entities; + @Autowired private SkillAvatarCandidateJpaRepository candidates; + + @Test + void selectsOnlyCurrentOrdinaryCandidatesAfterAbandonmentDeadline() { + Namespace namespace = new Namespace("machine-recovery", "Machine recovery", "owner"); + entities.persist(namespace); + SkillAvatarCandidate pending = candidate(namespace.getId(), "pending", false); + SkillAvatarCandidate processing = candidate(namespace.getId(), "processing", true); + SkillAvatarCandidate superseded = candidate(namespace.getId(), "superseded", false); + SkillAvatarCandidate scoped = candidate(namespace.getId(), "scope", false); + scoped.markScopeReviewVersion(null); + Skill supersededSkill = entities.createQuery("SELECT s FROM Skill s WHERE s.id = :id", Skill.class) + .setParameter("id", superseded.getSkillId()).getSingleResult(); + supersededSkill.setPendingAvatarId(null); + entities.flush(); + entities.clear(); + + Instant now = Instant.now(); + assertThat(recoverable(now)).isEmpty(); + assertThat(recoverable(now.plusSeconds(601))).extracting(SkillAvatarCandidate::getId) + .containsExactly(pending.getId(), processing.getId()); + assertThat(recoverable(now.plusSeconds(601), pending.getId())) + .extracting(SkillAvatarCandidate::getId).containsExactly(processing.getId()); + } + + private SkillAvatarCandidate candidate(Long namespaceId, String slug, boolean claim) { + Skill skill = new Skill(namespaceId, slug, "owner", SkillVisibility.PUBLIC); + entities.persist(skill); + entities.flush(); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(skill.getId(), "owner", slug + ".png", + "sha", SkillVisibility.PUBLIC, true, false, "xfyun"); + if (claim) { + Instant now = Instant.now(); + candidate.claimMachineAttempt("attempt", now, now.plusSeconds(600)); + } + entities.persist(candidate); + entities.flush(); + skill.setPendingAvatarId(candidate.getId()); + entities.flush(); + return candidate; + } + + private java.util.List recoverable(Instant now) { + return recoverable(now, 0L); + } + + private java.util.List recoverable(Instant now, Long afterId) { + return candidates.findAbandonedMachineCandidates(afterId, SkillAvatarCandidate.Status.PENDING, + SkillAvatarCandidate.MachineStatus.PENDING, SkillAvatarCandidate.MachineStatus.PROCESSING, + now.minusSeconds(600), now, PageRequest.of(0, 100)); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarScopeRecoveryQueryTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarScopeRecoveryQueryTest.java new file mode 100644 index 00000000..4b59a446 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillAvatarScopeRecoveryQueryTest.java @@ -0,0 +1,125 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager; +import org.springframework.data.domain.PageRequest; +import org.springframework.test.context.ActiveProfiles; +import java.time.Instant; + +import static org.assertj.core.api.Assertions.assertThat; + +@DataJpaTest +@ActiveProfiles("test") +class SkillAvatarScopeRecoveryQueryTest { + @Autowired private TestEntityManager entities; + @Autowired private SkillJpaRepository skills; + + @Test + void selectsMissingAndErroredReviewsButSkipsCurrentRejectedCandidate() { + Namespace namespace = entities.persistAndFlush(new Namespace("scope-recovery", "Scope recovery", "owner")); + Skill needsReview = expandedSkill(namespace.getId(), "needs-review"); + Skill rejected = expandedSkill(namespace.getId(), "rejected"); + Skill oldRejection = expandedSkill(namespace.getId(), "old-rejection"); + Skill retry = expandedSkill(namespace.getId(), "retry"); + Skill fresh = expandedSkill(namespace.getId(), "fresh"); + Skill processing = expandedSkill(namespace.getId(), "processing"); + SkillAvatarCandidate rejectedCandidate = new SkillAvatarCandidate(rejected.getId(), "owner", "old/rejected.png", + "sha", SkillVisibility.PUBLIC, false, true, null); + rejectedCandidate.markScopeReviewVersion(4L); + rejectedCandidate.reject("reviewer", "blocked"); + entities.persistAndFlush(rejectedCandidate); + SkillAvatarCandidate historical = new SkillAvatarCandidate(oldRejection.getId(), "owner", "old/historical.png", + "sha", SkillVisibility.PUBLIC, false, true, null); + historical.markScopeReviewVersion(3L); + historical.reject("reviewer", "blocked on older publication"); + entities.persistAndFlush(historical); + SkillAvatarCandidate errored = new SkillAvatarCandidate(retry.getId(), "owner", "old/error.png", "sha", + SkillVisibility.PUBLIC, true, false, "xfyun"); + errored.machineResult(SkillAvatarCandidate.MachineStatus.ERROR); + entities.persistAndFlush(errored); + retry.setPendingAvatarId(errored.getId()); + entities.persistAndFlush(retry); + SkillAvatarCandidate freshCandidate = new SkillAvatarCandidate(fresh.getId(), "owner", "old/fresh.png", "sha", + SkillVisibility.PUBLIC, true, false, "xfyun"); + entities.persistAndFlush(freshCandidate); + fresh.setPendingAvatarId(freshCandidate.getId()); + entities.persistAndFlush(fresh); + SkillAvatarCandidate processingCandidate = new SkillAvatarCandidate(processing.getId(), "owner", + "old/processing.png", "sha", SkillVisibility.PUBLIC, true, false, "xfyun"); + Instant claimTime = Instant.now(); + processingCandidate.claimMachineAttempt("first-attempt", claimTime, claimTime.plusSeconds(600)); + entities.persistAndFlush(processingCandidate); + processing.setPendingAvatarId(processingCandidate.getId()); + entities.persistAndFlush(processing); + entities.clear(); + + Instant now = Instant.now(); + var results = recoverable(now); + + assertThat(results).extracting(Skill::getId) + .containsExactly(needsReview.getId(), oldRejection.getId(), retry.getId()); + assertThat(recoverable(now.plusSeconds(601))).extracting(Skill::getId) + .containsExactly(needsReview.getId(), oldRejection.getId(), retry.getId(), + fresh.getId(), processing.getId()); + } + + @Test + void cooledDownErrorIsNotRetriedOnConsecutiveTicksAndStopsAfterThreeAttempts() { + Namespace namespace = entities.persistAndFlush(new Namespace("scope-cooldown", "Scope cooldown", "owner")); + Skill cooling = expandedSkill(namespace.getId(), "cooling"); + Skill exhausted = expandedSkill(namespace.getId(), "exhausted"); + SkillAvatarCandidate first = errorCandidate(cooling, 1); + SkillAvatarCandidate third = errorCandidate(exhausted, 3); + cooling.setPendingAvatarId(first.getId()); + exhausted.setPendingAvatarId(third.getId()); + entities.persistAndFlush(cooling); + entities.persistAndFlush(exhausted); + entities.clear(); + + Instant now = Instant.now(); + assertThat(recoverable(now)).isEmpty(); + assertThat(recoverable(now.plusSeconds(300))).isEmpty(); + assertThat(recoverable(now.plusSeconds(3601))).extracting(Skill::getId) + .containsExactly(cooling.getId()); + } + + private SkillAvatarCandidate errorCandidate(Skill skill, int attempts) { + SkillAvatarCandidate candidate = new SkillAvatarCandidate(skill.getId(), "owner", "old/error.png", + "sha", SkillVisibility.PUBLIC, true, false, "xfyun"); + for (int n = 0; n < attempts; n++) { + Instant at = Instant.now().plusSeconds(n * 3601L); + candidate.claimMachineAttempt("attempt-" + n, at, at.plusSeconds(600)); + candidate.completeMachineAttempt("attempt-" + n, SkillAvatarCandidate.MachineStatus.ERROR); + } + return entities.persistAndFlush(candidate); + } + + private java.util.List recoverable(Instant now) { + return skills.findScopeReviewRecoveryCandidates(0L, SkillStatus.ACTIVE, + SkillVisibility.PRIVATE, SkillVisibility.NAMESPACE_ONLY, SkillVisibility.PUBLIC, + SkillAvatarCandidate.Status.PENDING, SkillAvatarCandidate.MachineStatus.PENDING, + SkillAvatarCandidate.MachineStatus.PROCESSING, SkillAvatarCandidate.MachineStatus.ERROR, 3, + SkillAvatarCandidate.Status.REJECTED, now.minusSeconds(600), now, PageRequest.of(0, 100)); + } + + private Skill expandedSkill(Long namespaceId, String slug) { + Skill skill = entities.persistAndFlush(new Skill(namespaceId, slug, "owner", SkillVisibility.PRIVATE)); + SkillAvatarCandidate active = new SkillAvatarCandidate(skill.getId(), "owner", "old/" + slug + ".png", + "sha", SkillVisibility.PRIVATE, false, false, null); + active.activate(); + entities.persistAndFlush(active); + skill.activateAvatar(active.getId(), SkillVisibility.PRIVATE); + skill.setLatestVersionId(4L); + skill.setVisibility(SkillVisibility.PUBLIC); + entities.persistAndFlush(skill); + return skill; + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AvatarReviewSettingsServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AvatarReviewSettingsServiceTest.java new file mode 100644 index 00000000..a742bd84 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/AvatarReviewSettingsServiceTest.java @@ -0,0 +1,69 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.auth.settings.SystemSetting; +import com.iflytek.skillhub.auth.settings.SystemSettingRepository; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; +import com.iflytek.skillhub.dto.AvatarReviewSettingsUpdateRequest; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class AvatarReviewSettingsServiceTest { + @Mock private SystemSettingRepository repository; + @Mock private AvatarModerationGateway gateway; + @Mock private AuditLogService audit; + @Mock private RequestIdAccessor requestIds; + private AvatarReviewSettingsService service; + private SystemSetting setting; + + @BeforeEach + void setup() { + service = new AvatarReviewSettingsService(repository, gateway, audit, requestIds); + setting = new SystemSetting(AvatarReviewSettingsService.SETTING_KEY, + Map.of("machineEnabled", false, "manualEnabled", true)); + } + + @ParameterizedTest + @CsvSource({ "false,false", "false,true", "true,false", "true,true" }) + void updatesAllFourPolicies(boolean machine, boolean manual) { + when(repository.findBySettingKey(AvatarReviewSettingsService.SETTING_KEY)).thenReturn(Optional.of(setting)); + if (machine) when(gateway.requireConfiguredProvider()).thenReturn("xfyun"); + var updated = service.update(new AvatarReviewSettingsUpdateRequest(machine, manual, 0), "admin", + new AuditRequestContext(null, null)); + assertThat(updated.machineEnabled()).isEqualTo(machine); + assertThat(updated.manualEnabled()).isEqualTo(manual); + verify(repository).saveAndFlush(setting); + } + + @Test + void cannotEnableMachineReviewWithoutProvider() { + when(gateway.requireConfiguredProvider()).thenThrow(new DomainBadRequestException("providerUnavailable")); + assertThatThrownBy(() -> service.update(new AvatarReviewSettingsUpdateRequest(true, false, 0), "admin", + new AuditRequestContext(null, null))).isInstanceOf(DomainBadRequestException.class); + verify(repository, never()).saveAndFlush(setting); + } + + @Test + void staleVersionDoesNotOverwriteSettings() { + when(repository.findBySettingKey(AvatarReviewSettingsService.SETTING_KEY)).thenReturn(Optional.of(setting)); + assertThatThrownBy(() -> service.update(new AvatarReviewSettingsUpdateRequest(false, false, 1), "admin", + new AuditRequestContext(null, null))).isInstanceOf(DomainConflictException.class); + verify(repository, never()).saveAndFlush(setting); + } +} 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 new file mode 100644 index 00000000..5f6e22ee --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarAppServiceTest.java @@ -0,0 +1,395 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; + +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; +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.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; +import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.infra.jpa.SkillAvatarCandidateJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.storage.ObjectStorageService; +import jakarta.persistence.EntityManager; +import java.io.ByteArrayInputStream; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.SimpleTransactionStatus; +import org.junit.jupiter.api.AfterEach; + +@ExtendWith(MockitoExtension.class) +class SkillAvatarAppServiceTest { + @Mock private SkillJpaRepository skills; + @Mock private NamespaceRepository namespaces; + @Mock private SkillAvatarCandidateJpaRepository candidates; + @Mock private ObjectStorageService storage; + @Mock private SkillAvatarImageProcessor imageProcessor; + @Mock private AvatarReviewSettingsService settings; + @Mock private AvatarModerationGateway moderation; + @Mock private VisibilityChecker visibility; + @Mock private SkillQueryService skillQueries; + @Mock private AuditLogService audit; + @Mock private RequestIdAccessor requestIds; + @Mock private EntityManager entityManager; + @Mock private PlatformTransactionManager transactionManager; + private SkillAvatarAppService service; + private Namespace namespace; + private Skill skill; + + @BeforeEach + void setup() { + service = new SkillAvatarAppService(skills, namespaces, candidates, storage, imageProcessor, + settings, moderation, visibility, skillQueries, audit, requestIds, entityManager, transactionManager); + org.mockito.Mockito.lenient().when(transactionManager.getTransaction(any())) + .thenAnswer(ignored -> new SimpleTransactionStatus()); + namespace = new Namespace("team", "Team", "owner"); + ReflectionTestUtils.setField(namespace, "id", 10L); + ReflectionTestUtils.setField(namespace, "type", NamespaceType.TEAM); + skill = new Skill(10L, "foo", "owner", SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(skill, "id", 11L); + } + + @AfterEach + void clearTransactionSynchronization() { + if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.clearSynchronization(); + } + } + + @Test + 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)); + when(visibility.canAccess(eq(skill), eq("admin"), any(), any())).thenReturn(true); + assertThatThrownBy(() -> service.readActive("team", "foo", "admin", + Map.of(10L, NamespaceRole.ADMIN), Set.of())) + .isInstanceOf(DomainNotFoundException.class); + verifyNoInteractions(storage); + } + + @Test + void privateCandidateCannotBeReadByNamespaceAdmin() { + when(namespaces.findBySlug("team")).thenReturn(Optional.of(namespace)); + when(skills.findByNamespaceIdAndSlug(10L, "foo")).thenReturn(List.of(skill)); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/private.png", "abc", + SkillVisibility.PRIVATE, false, false, null); + ReflectionTestUtils.setField(candidate, "id", 21L); + when(candidates.findById(21L)).thenReturn(Optional.of(candidate)); + assertThatThrownBy(() -> service.readCandidate("team", "foo", 21L, "admin", + Map.of(10L, NamespaceRole.ADMIN), Set.of())) + .isInstanceOf(DomainNotFoundException.class); + verifyNoInteractions(storage); + } + + @Test + void expandingVisibilityCreatesNewCandidateWithDistinctObjectKey() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + skill.setLatestVersionId(4L); + SkillAvatarCandidate old = new SkillAvatarCandidate(11L, "owner", "avatar/private.png", "abc", + SkillVisibility.PRIVATE, false, false, null); + old.activate(); + ReflectionTestUtils.setField(old, "id", 21L); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(21L)).thenReturn(Optional.of(old)); + when(storage.getObject("avatar/private.png")).thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(settings.current()).thenReturn(new AvatarReviewSettingsResponse(false, true, 1, null)); + when(candidates.saveAndFlush(any())).thenAnswer(invocation -> { + SkillAvatarCandidate created = invocation.getArgument(0); + ReflectionTestUtils.setField(created, "id", 22L); + return created; + }); + TransactionSynchronizationManager.initSynchronization(); + + var result = service.prepareScopeReview(11L, "publisher"); + + org.assertj.core.api.Assertions.assertThat(result.candidateStatus()).isEqualTo("PENDING"); + org.assertj.core.api.Assertions.assertThat(result.avatarUrl()).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isEqualTo(22L); + org.assertj.core.api.Assertions.assertThat(old.getStatus()).isEqualTo(SkillAvatarCandidate.Status.ACTIVE); + org.mockito.ArgumentCaptor key = org.mockito.ArgumentCaptor.forClass(String.class); + verify(storage).putObject(key.capture(), any(), eq(1L), eq("image/png")); + org.assertj.core.api.Assertions.assertThat(key.getValue()).isNotEqualTo("avatar/private.png"); + org.mockito.ArgumentCaptor created = + org.mockito.ArgumentCaptor.forClass(SkillAvatarCandidate.class); + verify(candidates).saveAndFlush(created.capture()); + org.assertj.core.api.Assertions.assertThat(created.getValue().getScopeReviewVersionId()).isEqualTo(4L); + } + + @Test + void expandingVisibilityKeepsExistingTargetScopeCandidate() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + skill.setPendingAvatarId(22L); + SkillAvatarCandidate pending = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, false, true, null); + ReflectionTestUtils.setField(pending, "id", 22L); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(22L)).thenReturn(Optional.of(pending)); + + org.assertj.core.api.Assertions.assertThat(service.prepareScopeReview(11L, "publisher")).isNull(); + verifyNoInteractions(storage); + } + + @Test + void storageFailureLeavesOldAvatarHiddenAndLaterRecoveryCanCreateCandidate() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + SkillAvatarCandidate old = new SkillAvatarCandidate(11L, "owner", "avatar/private.png", "abc", + SkillVisibility.PRIVATE, false, false, null); + old.activate(); + ReflectionTestUtils.setField(old, "id", 21L); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(21L)).thenReturn(Optional.of(old)); + when(storage.getObject("avatar/private.png")) + .thenThrow(new IllegalStateException("temporary storage outage")) + .thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(settings.current()).thenReturn(new AvatarReviewSettingsResponse(false, true, 1, null)); + when(candidates.saveAndFlush(any())).thenAnswer(invocation -> { + SkillAvatarCandidate created = invocation.getArgument(0); + ReflectionTestUtils.setField(created, "id", 22L); + return created; + }); + + assertThatThrownBy(() -> service.prepareScopeReview(11L, "publisher")) + .isInstanceOf(IllegalStateException.class); + org.assertj.core.api.Assertions.assertThat(SkillAvatarUrls.current(skill, "team")).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isNull(); + + TransactionSynchronizationManager.initSynchronization(); + var recovered = service.prepareScopeReview(11L, "publisher"); + org.assertj.core.api.Assertions.assertThat(recovered.candidateId()).isEqualTo(22L); + org.assertj.core.api.Assertions.assertThat(recovered.avatarUrl()).isNull(); + } + + @Test + void machineStorageFailureBecomesRetriableErrorThenPassesOnRecovery() { + skill.setPendingAvatarId(21L); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + ReflectionTestUtils.setField(candidate, "id", 21L); + when(candidates.findById(21L)).thenReturn(Optional.of(candidate)); + when(storage.getObject("avatar/new.png")) + .thenThrow(new IllegalStateException("temporary storage outage")) + .thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(moderation.moderate(eq("xfyun"), any(byte[].class))) + .thenReturn(AvatarModerationProvider.Verdict.PASS); + + var failed = service.processMachine(21L, new AuditRequestContext(null, null)); + org.assertj.core.api.Assertions.assertThat(failed.machineStatus()).isEqualTo("ERROR"); + org.assertj.core.api.Assertions.assertThat(failed.avatarUrl()).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isEqualTo(21L); + assertThatThrownBy(() -> service.processMachine(21L, new AuditRequestContext(null, null))) + .isInstanceOf(DomainConflictException.class); + ReflectionTestUtils.setField(candidate, "machineNextRetryAt", Instant.now().minusSeconds(1)); + + var recovered = service.processMachine(21L, new AuditRequestContext(null, null)); + org.assertj.core.api.Assertions.assertThat(recovered.machineStatus()).isEqualTo("PASSED"); + org.assertj.core.api.Assertions.assertThat(recovered.avatarUrl()).isNotNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isNull(); + } + + @Test + void scopeRecoveryReusesPendingErroredCandidateInsteadOfCreatingAnother() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + skill.setPendingAvatarId(22L); + SkillAvatarCandidate pending = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + pending.machineResult(SkillAvatarCandidate.MachineStatus.ERROR); + ReflectionTestUtils.setField(pending, "id", 22L); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(22L)).thenReturn(Optional.of(pending)); + + var result = service.prepareScopeReview(11L, "publisher"); + + org.assertj.core.api.Assertions.assertThat(result.candidateId()).isEqualTo(22L); + org.assertj.core.api.Assertions.assertThat(result.machineStatus()).isEqualTo("ERROR"); + verifyNoInteractions(storage); + } + + @Test + void abandonedPendingMachineCandidateIsRecoveredWithoutCreatingAnother() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + skill.setPendingAvatarId(22L); + SkillAvatarCandidate pending = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + ReflectionTestUtils.setField(pending, "id", 22L); + ReflectionTestUtils.setField(pending, "createdAt", Instant.now().minusSeconds(601)); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(22L)).thenReturn(Optional.of(pending)); + when(storage.getObject("avatar/new.png")).thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(moderation.moderate(eq("xfyun"), any(byte[].class))) + .thenReturn(AvatarModerationProvider.Verdict.PASS); + + var abandoned = service.prepareScopeReview(11L, "publisher"); + var recovered = service.processMachine(abandoned.candidateId(), new AuditRequestContext(null, null)); + + org.assertj.core.api.Assertions.assertThat(recovered.machineStatus()).isEqualTo("PASSED"); + org.assertj.core.api.Assertions.assertThat(skill.getActiveAvatarId()).isEqualTo(22L); + org.mockito.Mockito.verify(candidates, org.mockito.Mockito.never()).saveAndFlush(any()); + } + + @Test + void lateMachineResultCannotOverwriteNewerAttempt() { + SkillAvatarCandidate candidate = pendingCandidate(); + when(moderation.moderate(eq("xfyun"), any(byte[].class))).thenAnswer(ignored -> { + Instant afterLease = Instant.now().plusSeconds(601); + org.assertj.core.api.Assertions.assertThat(candidate.claimMachineAttempt( + "newer-attempt", afterLease, afterLease.plusSeconds(600))).isTrue(); + return AvatarModerationProvider.Verdict.PASS; + }); + + var result = service.processMachine(21L, new AuditRequestContext(null, null)); + + org.assertj.core.api.Assertions.assertThat(result.machineStatus()).isEqualTo("PROCESSING"); + org.assertj.core.api.Assertions.assertThat(skill.getActiveAvatarId()).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isEqualTo(21L); + } + + @Test + void expiredMachineLeaseIsReclaimedAfterCrash() { + skill.activateAvatar(21L, SkillVisibility.PRIVATE); + skill.setPendingAvatarId(22L); + SkillAvatarCandidate pending = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + ReflectionTestUtils.setField(pending, "id", 22L); + Instant old = Instant.now().minusSeconds(601); + pending.claimMachineAttempt("crashed-attempt", old, old.plusSeconds(600)); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + when(candidates.findById(22L)).thenReturn(Optional.of(pending)); + when(storage.getObject("avatar/new.png")).thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(moderation.moderate(eq("xfyun"), any(byte[].class))) + .thenReturn(AvatarModerationProvider.Verdict.PASS); + + var expired = service.prepareScopeReview(11L, "publisher"); + var recovered = service.processMachine(expired.candidateId(), new AuditRequestContext(null, null)); + + org.assertj.core.api.Assertions.assertThat(expired.machineStatus()).isEqualTo("PROCESSING"); + org.assertj.core.api.Assertions.assertThat(recovered.machineStatus()).isEqualTo("PASSED"); + org.assertj.core.api.Assertions.assertThat(skill.getActiveAvatarId()).isEqualTo(22L); + org.mockito.Mockito.verify(candidates, org.mockito.Mockito.never()).saveAndFlush(any()); + } + + @Test + void processingMachineCandidateDoesNotEnterHumanReviewQueue() { + SkillAvatarCandidate pending = new SkillAvatarCandidate(11L, "owner", "avatar/new.png", "abc", + SkillVisibility.PUBLIC, true, true, "xfyun"); + pending.claimMachineAttempt("in-flight", Instant.now(), Instant.now().plusSeconds(600)); + when(candidates.findByStatusAndReviewStatusOrderByCreatedAtAsc( + SkillAvatarCandidate.Status.PENDING, SkillAvatarCandidate.ReviewStatus.PENDING)) + .thenReturn(List.of(pending)); + + org.assertj.core.api.Assertions.assertThat(service.reviewQueue("reviewer", + Map.of(10L, NamespaceRole.ADMIN), Set.of())).isEmpty(); + verifyNoInteractions(skills); + } + + @Test + void nonOwnerCannotSubmitBeforeWritingToStorage() { + stubSkillLookup(); + assertThatThrownBy(() -> service.submit("team", "foo", "stranger", new byte[] { 1 }, + new AuditRequestContext(null, null))).isInstanceOf(DomainForbiddenException.class); + verifyNoInteractions(storage); + } + + @Test + void staleApprovalCannotReplaceNewerCandidate() { + stubSkillLookup(); + 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); + } + + @Test + void reviewRequiresMatchingNamespaceRole() { + stubSkillLookup(); + 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))) + .isInstanceOf(DomainForbiddenException.class); + } + + @Test + void unknownMachineResultLeavesCandidatePrivate() { + SkillAvatarCandidate candidate = pendingCandidate(); + when(moderation.moderate(org.mockito.ArgumentMatchers.eq("xfyun"), any(byte[].class))) + .thenReturn(AvatarModerationProvider.Verdict.ERROR); + + var result = service.processMachine(21L, new AuditRequestContext(null, null)); + + org.assertj.core.api.Assertions.assertThat(result.machineStatus()).isEqualTo("ERROR"); + org.assertj.core.api.Assertions.assertThat(result.avatarUrl()).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isEqualTo(21L); + } + + @Test + void staleMachineResultCannotActivateSupersededCandidate() { + SkillAvatarCandidate candidate = pendingCandidate(); + when(moderation.moderate(org.mockito.ArgumentMatchers.eq("xfyun"), any(byte[].class))) + .thenAnswer(ignored -> { + candidate.supersede(); + skill.setPendingAvatarId(22L); + return AvatarModerationProvider.Verdict.PASS; + }); + + var result = service.processMachine(21L, new AuditRequestContext(null, null)); + + org.assertj.core.api.Assertions.assertThat(result.avatarUrl()).isNull(); + org.assertj.core.api.Assertions.assertThat(skill.getPendingAvatarId()).isEqualTo(22L); + org.assertj.core.api.Assertions.assertThat(candidate.getStatus()) + .isEqualTo(SkillAvatarCandidate.Status.SUPERSEDED); + } + + private SkillAvatarCandidate pendingCandidate() { + skill.setPendingAvatarId(21L); + SkillAvatarCandidate candidate = new SkillAvatarCandidate(11L, "owner", "avatar/one.png", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + ReflectionTestUtils.setField(candidate, "id", 21L); + when(candidates.findById(21L)).thenReturn(Optional.of(candidate)); + when(storage.getObject("avatar/one.png")).thenReturn(new ByteArrayInputStream(new byte[] { 1 })); + when(skills.findLockedById(11L)).thenReturn(Optional.of(skill)); + when(namespaces.findById(10L)).thenReturn(Optional.of(namespace)); + 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-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarUrlsTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarUrlsTest.java new file mode 100644 index 00000000..33c6ef42 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillAvatarUrlsTest.java @@ -0,0 +1,20 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import org.junit.jupiter.api.Test; + +class SkillAvatarUrlsTest { + @Test + void hidesAvatarWhenCurrentVisibilityExceedsApprovedAudience() { + assertThat(SkillAvatarUrls.current(1L, SkillVisibility.PRIVATE, SkillVisibility.NAMESPACE_ONLY, + "team", "foo")).isNull(); + assertThat(SkillAvatarUrls.current(1L, SkillVisibility.NAMESPACE_ONLY, SkillVisibility.PUBLIC, + "team", "foo")).isNull(); + assertThat(SkillAvatarUrls.current(1L, SkillVisibility.PUBLIC, SkillVisibility.PUBLIC, + "global", "foo")).isEqualTo("/api/v1/skills/global/foo/avatar"); + assertThat(SkillAvatarUrls.current(null, SkillVisibility.PUBLIC, SkillVisibility.PUBLIC, + "global", "foo")).isNull(); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTaskTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTaskTest.java new file mode 100644 index 00000000..4c913e36 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarMachineRecoveryTaskTest.java @@ -0,0 +1,47 @@ +package com.iflytek.skillhub.task; + +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillAvatarCandidateJpaRepository; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillAvatarAppService; +import java.time.Instant; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.data.domain.PageRequest; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class SkillAvatarMachineRecoveryTaskTest { + @Test + void recoversAbandonedOrdinaryUploadAndContinuesAfterFailure() { + SkillAvatarCandidateJpaRepository candidates = mock(SkillAvatarCandidateJpaRepository.class); + SkillAvatarAppService avatars = mock(SkillAvatarAppService.class); + SkillAvatarCandidate first = candidate(11L); + SkillAvatarCandidate second = candidate(12L); + when(candidates.findAbandonedMachineCandidates(eq(0L), eq(SkillAvatarCandidate.Status.PENDING), + eq(SkillAvatarCandidate.MachineStatus.PENDING), eq(SkillAvatarCandidate.MachineStatus.PROCESSING), + any(Instant.class), any(Instant.class), eq(PageRequest.of(0, 100)))) + .thenReturn(List.of(first, second)); + doThrow(new IllegalStateException("secret-bearing failure")) + .when(avatars).processMachine(eq(11L), any(AuditRequestContext.class)); + + new SkillAvatarMachineRecoveryTask(candidates, avatars).recover(); + + verify(avatars).processMachine(eq(11L), any(AuditRequestContext.class)); + verify(avatars).processMachine(eq(12L), any(AuditRequestContext.class)); + } + + private SkillAvatarCandidate candidate(Long id) { + SkillAvatarCandidate candidate = new SkillAvatarCandidate(1L, "owner", "private.png", "sha", + SkillVisibility.PUBLIC, true, false, "xfyun"); + ReflectionTestUtils.setField(candidate, "id", id); + return candidate; + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTaskTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTaskTest.java new file mode 100644 index 00000000..33e92f13 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillAvatarScopeReviewRecoveryTaskTest.java @@ -0,0 +1,69 @@ +package com.iflytek.skillhub.task; + +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.listener.SkillAvatarScopeReviewListener; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.data.domain.PageRequest; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.any; +import java.time.Instant; + +class SkillAvatarScopeReviewRecoveryTaskTest { + @Test + void scansOnlyUnreviewedExpandedScopesInBoundedIdOrder() { + SkillJpaRepository skills = mock(SkillJpaRepository.class); + SkillAvatarScopeReviewListener listener = mock(SkillAvatarScopeReviewListener.class); + Skill skill = new Skill(10L, "foo", "owner", SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(skill, "id", 11L); + when(skills.findScopeReviewRecoveryCandidates(eq(0L), eq(SkillStatus.ACTIVE), eq(SkillVisibility.PRIVATE), + eq(SkillVisibility.NAMESPACE_ONLY), eq(SkillVisibility.PUBLIC), eq(SkillAvatarCandidate.Status.PENDING), + eq(SkillAvatarCandidate.MachineStatus.PENDING), eq(SkillAvatarCandidate.MachineStatus.PROCESSING), + eq(SkillAvatarCandidate.MachineStatus.ERROR), eq(3), eq(SkillAvatarCandidate.Status.REJECTED), + any(Instant.class), any(Instant.class), eq(PageRequest.of(0, 100)))) + .thenReturn(List.of(skill)); + + new SkillAvatarScopeReviewRecoveryTask(skills, listener).recover(); + + verify(listener).reviewScope(11L, null); + } + + @Test + void failedSkillDoesNotStarveLaterBatches() { + SkillJpaRepository skills = mock(SkillJpaRepository.class); + SkillAvatarScopeReviewListener listener = mock(SkillAvatarScopeReviewListener.class); + Skill first = new Skill(10L, "first", "owner", SkillVisibility.PUBLIC); + Skill second = new Skill(10L, "second", "owner", SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(first, "id", 11L); + ReflectionTestUtils.setField(second, "id", 12L); + when(skills.findScopeReviewRecoveryCandidates(eq(0L), eq(SkillStatus.ACTIVE), eq(SkillVisibility.PRIVATE), + eq(SkillVisibility.NAMESPACE_ONLY), eq(SkillVisibility.PUBLIC), eq(SkillAvatarCandidate.Status.PENDING), + eq(SkillAvatarCandidate.MachineStatus.PENDING), eq(SkillAvatarCandidate.MachineStatus.PROCESSING), + eq(SkillAvatarCandidate.MachineStatus.ERROR), eq(3), eq(SkillAvatarCandidate.Status.REJECTED), + any(Instant.class), any(Instant.class), eq(PageRequest.of(0, 100)))) + .thenReturn(List.of(first)); + when(skills.findScopeReviewRecoveryCandidates(eq(11L), eq(SkillStatus.ACTIVE), eq(SkillVisibility.PRIVATE), + eq(SkillVisibility.NAMESPACE_ONLY), eq(SkillVisibility.PUBLIC), eq(SkillAvatarCandidate.Status.PENDING), + eq(SkillAvatarCandidate.MachineStatus.PENDING), eq(SkillAvatarCandidate.MachineStatus.PROCESSING), + eq(SkillAvatarCandidate.MachineStatus.ERROR), eq(3), eq(SkillAvatarCandidate.Status.REJECTED), + any(Instant.class), any(Instant.class), eq(PageRequest.of(0, 100)))) + .thenReturn(List.of(second)); + doThrow(new IllegalStateException("storage unavailable")).when(listener).reviewScope(11L, null); + SkillAvatarScopeReviewRecoveryTask task = new SkillAvatarScopeReviewRecoveryTask(skills, listener); + + org.assertj.core.api.Assertions.assertThatThrownBy(task::recover).isInstanceOf(IllegalStateException.class); + task.recover(); + + verify(listener).reviewScope(12L, null); + } +} 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 4603ad11..3219db40 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,7 +55,9 @@ 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.GET, "/api/v1/skills/avatar-review/queue"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/avatar"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/download"), @@ -70,7 +72,9 @@ 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.GET, "/api/web/skills/avatar-review/queue"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/avatar"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/versions/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/download"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index 33fa4ec4..949a9710 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -34,6 +34,20 @@ class RouteSecurityPolicyRegistryTest { registry.accessLevel("GET", "/api/v1/resolve/team/demo")); } + @Test + void currentAvatarMayBePublicButCandidateAndReviewRoutesRequireAuthentication() { + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, + registry.accessLevel("GET", "/api/v1/skills/global/demo/avatar")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, + registry.accessLevel("GET", "/api/web/skills/global/demo/avatar")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("GET", "/api/v1/skills/global/demo/avatar/candidates/1/image")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("GET", "/api/v1/skills/avatar-review/queue")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("GET", "/api/web/skills/avatar-review/queue")); + } + @Test void reviewRoutesExposePublicListingButProtectCurrentUserMutations() { assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidate.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidate.java index db1c3205..7ae139fe 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidate.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidate.java @@ -15,7 +15,7 @@ import java.time.Instant; @Entity @Table(name = "skill_avatar_candidate") public class SkillAvatarCandidate { - public enum MachineStatus { SKIPPED, PENDING, PASSED, BLOCKED, ERROR } + public enum MachineStatus { SKIPPED, PENDING, PROCESSING, PASSED, BLOCKED, ERROR } public enum ReviewStatus { NOT_REQUIRED, PENDING, APPROVED, REJECTED, APPEAL_PENDING } public enum Status { PENDING, ACTIVE, REJECTED, SUPERSEDED, REMOVED } @@ -39,6 +39,18 @@ public class SkillAvatarCandidate { private boolean manualEnabled; @Column(name = "machine_provider", length = 32) private String machineProvider; + @Column(name = "scope_review_version_id") + private Long scopeReviewVersionId; + @Column(name = "scope_review", nullable = false) + private boolean scopeReview; + @Column(name = "machine_attempt_id", length = 36) + private String machineAttemptId; + @Column(name = "machine_lease_until") + private Instant machineLeaseUntil; + @Column(name = "machine_next_retry_at") + private Instant machineNextRetryAt; + @Column(name = "machine_attempt_count", nullable = false) + private int machineAttemptCount; @Enumerated(EnumType.STRING) @Column(name = "machine_status", nullable = false, length = 32) private MachineStatus machineStatus; @@ -93,6 +105,11 @@ public class SkillAvatarCandidate { public boolean isMachineEnabled() { return machineEnabled; } public boolean isManualEnabled() { return manualEnabled; } public String getMachineProvider() { return machineProvider; } + public Long getScopeReviewVersionId() { return scopeReviewVersionId; } + public boolean isScopeReview() { return scopeReview; } + public Instant getMachineLeaseUntil() { return machineLeaseUntil; } + public Instant getMachineNextRetryAt() { return machineNextRetryAt; } + public int getMachineAttemptCount() { return machineAttemptCount; } public MachineStatus getMachineStatus() { return machineStatus; } public ReviewStatus getReviewStatus() { return reviewStatus; } public Status getStatus() { return status; } @@ -101,8 +118,46 @@ public class SkillAvatarCandidate { public Instant getCreatedAt() { return createdAt; } public Instant getDiscardAt() { return discardAt; } + public void markScopeReviewVersion(Long versionId) { scopeReviewVersionId = versionId; scopeReview = true; } + + public boolean canRetryMachine(Instant now) { + return status == Status.PENDING && machineEnabled + && (machineStatus == MachineStatus.ERROR + && (machineNextRetryAt == null || !machineNextRetryAt.isAfter(now)) + || machineStatus == MachineStatus.PROCESSING + && machineLeaseUntil != null && !machineLeaseUntil.isAfter(now)); + } + + public boolean claimMachineAttempt(String attemptId, Instant now, Instant leaseUntil) { + if (status != Status.PENDING || !machineEnabled + || machineStatus != MachineStatus.PENDING && !canRetryMachine(now)) { + return false; + } + machineStatus = MachineStatus.PROCESSING; + machineAttemptId = attemptId; + machineLeaseUntil = leaseUntil; + machineNextRetryAt = null; + machineAttemptCount++; + return true; + } + + public boolean completeMachineAttempt(String attemptId, MachineStatus result) { + if (status != Status.PENDING || machineStatus != MachineStatus.PROCESSING + || !attemptId.equals(machineAttemptId) + || result == MachineStatus.PENDING || result == MachineStatus.PROCESSING + || result == MachineStatus.SKIPPED) { + return false; + } + machineStatus = result; + machineAttemptId = null; + machineLeaseUntil = null; + machineNextRetryAt = result == MachineStatus.ERROR ? Instant.now().plusSeconds(3600) : null; + return true; + } + public void machineResult(MachineStatus result) { - if (status != Status.PENDING || !machineEnabled || result == MachineStatus.SKIPPED || result == MachineStatus.PENDING) { + if (status != Status.PENDING || !machineEnabled || result == MachineStatus.SKIPPED + || result == MachineStatus.PENDING || result == MachineStatus.PROCESSING) { throw new IllegalStateException("Avatar candidate cannot accept this machine result"); } machineStatus = result; diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidateTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidateTest.java index 7f52e2fc..2c65f209 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidateTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/SkillAvatarCandidateTest.java @@ -4,8 +4,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import org.junit.jupiter.api.Test; +import java.time.Instant; class SkillAvatarCandidateTest { + @Test + void initialStatesCoverFourReviewPolicies() { + for (boolean machine : new boolean[] { false, true }) { + for (boolean manual : new boolean[] { false, true }) { + SkillAvatarCandidate candidate = new SkillAvatarCandidate(10L, "owner", "avatars/one", "abc", + SkillVisibility.PUBLIC, machine, manual, machine ? "xfyun" : null); + assertThat(candidate.getMachineStatus()).isEqualTo(machine + ? SkillAvatarCandidate.MachineStatus.PENDING : SkillAvatarCandidate.MachineStatus.SKIPPED); + assertThat(candidate.getReviewStatus()).isEqualTo(manual + ? SkillAvatarCandidate.ReviewStatus.PENDING : SkillAvatarCandidate.ReviewStatus.NOT_REQUIRED); + } + } + } + @Test void supersededCandidateCannotBeActivatedByLateMachineResult() { SkillAvatarCandidate candidate = new SkillAvatarCandidate(10L, "owner", "avatars/one", "abc", @@ -31,4 +46,21 @@ class SkillAvatarCandidateTest { assertThat(candidate.getStatus()).isEqualTo(SkillAvatarCandidate.Status.ACTIVE); assertThat(candidate.getReviewedBy()).isEqualTo("reviewer"); } + + @Test + void errorEnforcesOneHourCooldownBeforeManualOrAutomaticRetry() { + SkillAvatarCandidate candidate = new SkillAvatarCandidate(10L, "owner", "avatars/one", "abc", + SkillVisibility.PUBLIC, true, false, "xfyun"); + Instant now = Instant.now(); + assertThat(candidate.claimMachineAttempt("first", now, now.plusSeconds(600))).isTrue(); + assertThat(candidate.completeMachineAttempt("first", SkillAvatarCandidate.MachineStatus.ERROR)).isTrue(); + assertThat(candidate.getMachineAttemptCount()).isEqualTo(1); + assertThat(candidate.canRetryMachine(now.plusSeconds(3599))).isFalse(); + assertThat(candidate.claimMachineAttempt("too-early", now.plusSeconds(3599), now.plusSeconds(4199))) + .isFalse(); + Instant due = candidate.getMachineNextRetryAt(); + assertThat(candidate.canRetryMachine(due)).isTrue(); + assertThat(candidate.claimMachineAttempt("second", due, due.plusSeconds(600))).isTrue(); + assertThat(candidate.getMachineAttemptCount()).isEqualTo(2); + } } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarCandidateJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarCandidateJpaRepository.java index cfc6971f..ee346211 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarCandidateJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillAvatarCandidateJpaRepository.java @@ -1,17 +1,38 @@ package com.iflytek.skillhub.infra.jpa; import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; +import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.SkillAvatarCandidateRepository; import java.time.Instant; import java.util.List; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; public interface SkillAvatarCandidateJpaRepository extends JpaRepository, SkillAvatarCandidateRepository { List findBySkillIdOrderByCreatedAtDesc(Long skillId); + boolean existsBySkillIdAndAudienceAndScopeReviewVersionIdAndStatus(Long skillId, + SkillVisibility audience, Long versionId, + SkillAvatarCandidate.Status status); List findByContentSha256AndMachineProviderAndMachineEnabledTrueAndManualEnabledAndMachineStatus( String contentSha256, String machineProvider, boolean manualEnabled, SkillAvatarCandidate.MachineStatus machineStatus); List findByStatusAndReviewStatusOrderByCreatedAtAsc( SkillAvatarCandidate.Status status, SkillAvatarCandidate.ReviewStatus reviewStatus); List findTop100ByDiscardAtBeforeAndObjectKeyIsNotNull(Instant before); + + @Query(""" + SELECT c FROM SkillAvatarCandidate c, Skill s + WHERE c.id > :afterId AND s.id = c.skillId AND s.pendingAvatarId = c.id + AND c.scopeReview = false AND c.status = :pendingStatus + AND ((c.machineStatus = :pendingMachine AND c.createdAt <= :staleBefore) + OR (c.machineStatus = :processingMachine AND c.machineLeaseUntil <= :now)) + ORDER BY c.id ASC + """) + List findAbandonedMachineCandidates(@Param("afterId") Long afterId, + @Param("pendingStatus") SkillAvatarCandidate.Status pendingStatus, + @Param("pendingMachine") SkillAvatarCandidate.MachineStatus pendingMachine, + @Param("processingMachine") SkillAvatarCandidate.MachineStatus processingMachine, + @Param("staleBefore") Instant staleBefore, @Param("now") Instant now, Pageable pageable); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillJpaRepository.java index a7cb8e40..c75e3d51 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillJpaRepository.java @@ -3,6 +3,8 @@ package com.iflytek.skillhub.infra.jpa; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.SkillAvatarCandidate; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; @@ -15,6 +17,7 @@ import org.springframework.transaction.annotation.Transactional; import java.util.List; import java.util.Optional; +import java.time.Instant; import jakarta.persistence.LockModeType; /** @@ -26,6 +29,37 @@ public interface SkillJpaRepository extends JpaRepository, SkillRep @Query("SELECT s FROM Skill s WHERE s.id = :id") Optional findLockedById(@Param("id") Long id); List findByIdIn(List ids); + @Query(""" + SELECT s FROM Skill s WHERE s.id > :afterId AND s.status = :activeStatus AND s.activeAvatarId IS NOT NULL + AND ((s.activeAvatarAudience = :privateVisibility + AND s.visibility IN (:teamVisibility, :publicVisibility)) + OR (s.activeAvatarAudience = :teamVisibility AND s.visibility = :publicVisibility)) + AND NOT EXISTS (SELECT c.id FROM SkillAvatarCandidate c WHERE c.id = s.pendingAvatarId + AND c.audience = s.visibility AND c.status = :pendingStatus + AND (c.machineStatus NOT IN (:pendingMachine, :processingMachine, :errorMachine) + OR (c.machineStatus = :pendingMachine AND c.createdAt > :staleBefore) + OR (c.machineStatus = :processingMachine AND c.machineLeaseUntil > :now) + OR (c.machineStatus = :errorMachine AND (c.machineAttemptCount >= :maxAutoAttempts + OR c.machineNextRetryAt > :now)))) + AND NOT EXISTS (SELECT c.id FROM SkillAvatarCandidate c WHERE c.skillId = s.id + AND c.audience = s.visibility AND c.scopeReviewVersionId = s.latestVersionId + AND c.status = :rejectedStatus) + ORDER BY s.id ASC + """) + List findScopeReviewRecoveryCandidates(@Param("afterId") Long afterId, + @Param("activeStatus") SkillStatus activeStatus, + @Param("privateVisibility") SkillVisibility privateVisibility, + @Param("teamVisibility") SkillVisibility teamVisibility, + @Param("publicVisibility") SkillVisibility publicVisibility, + @Param("pendingStatus") SkillAvatarCandidate.Status pendingStatus, + @Param("pendingMachine") SkillAvatarCandidate.MachineStatus pendingMachine, + @Param("processingMachine") SkillAvatarCandidate.MachineStatus processingMachine, + @Param("errorMachine") SkillAvatarCandidate.MachineStatus errorMachine, + @Param("maxAutoAttempts") int maxAutoAttempts, + @Param("rejectedStatus") SkillAvatarCandidate.Status rejectedStatus, + @Param("staleBefore") Instant staleBefore, + @Param("now") Instant now, + Pageable pageable); List findByNamespaceIdAndSlug(Long namespaceId, String slug); List findByNamespaceIdInAndSlugIn(List namespaceIds, List slugs); Optional findByNamespaceIdAndSlugAndOwnerId(Long namespaceId, String slug, String ownerId); diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 8884f9ca..91c94743 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -490,6 +490,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/admin/system-config/avatar-review": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["current_2"]; + put: operations["update"]; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/admin/system-config/auth/local": { parameters: { query?: never; @@ -1356,6 +1372,166 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/{namespace}/{slug}/avatar/takedown": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["takedown"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar/takedown": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["takedown_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/retry": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["retry"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/retry": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["retry_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/decision": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["decide"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/decision": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["decide_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/appeal": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["appeal"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/appeal": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["appeal_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["current"]; + put?: never; + post: operations["submit"]; + delete: operations["delete"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["current_1"]; + put?: never; + post: operations["submit_1"]; + delete: operations["delete_1"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/skills/{namespace}/{slug}/archive": { parameters: { query?: never; @@ -1749,7 +1925,7 @@ export interface paths { }; get?: never; put?: never; - post: operations["submit"]; + post: operations["submit_2"]; delete?: never; options?: never; head?: never; @@ -1765,7 +1941,7 @@ export interface paths { }; get?: never; put?: never; - post: operations["submit_1"]; + post: operations["submit_3"]; delete?: never; options?: never; head?: never; @@ -3547,6 +3723,70 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/skills/{namespace}/{slug}/avatar/status": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["status"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar/status": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["status_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/image": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["candidate"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/skills/{namespace}/{slug}/avatar/candidates/{candidateId}/image": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["candidate_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/skills/{namespace}/{slug}": { parameters: { query?: never; @@ -3579,6 +3819,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/avatar-review/queue": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["queue"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/avatar-review/queue": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["queue_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/reviews/{id}/skill-detail": { parameters: { query?: never; @@ -5619,6 +5891,29 @@ export interface components { /** Format: date-time */ updatedAt?: string; }; + AvatarReviewSettingsUpdateRequest: { + machineEnabled?: boolean; + manualEnabled?: boolean; + /** Format: int64 */ + version?: number; + }; + ApiResponseAvatarReviewSettingsResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["AvatarReviewSettingsResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + AvatarReviewSettingsResponse: { + machineEnabled?: boolean; + manualEnabled?: boolean; + /** Format: int64 */ + version?: number; + /** Format: date-time */ + updatedAt?: string; + }; SystemAuthSettingsUpdateRequest: { passwordLoginEnabled: boolean; selfRegistrationEnabled: boolean; @@ -5876,6 +6171,37 @@ export interface components { ConfirmPublishRequest: { version: string; }; + SkillAvatarReasonRequest: { + reason: string; + }; + ApiResponseSkillAvatarStatusResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SkillAvatarStatusResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + SkillAvatarStatusResponse: { + avatarUrl?: string; + /** Format: int64 */ + candidateId?: number; + candidateStatus?: string; + machineStatus?: string; + reviewStatus?: string; + reviewReason?: string; + candidateUrl?: string; + namespace?: string; + slug?: string; + /** Format: date-time */ + machineNextRetryAt?: string; + automaticRetryExhausted?: boolean; + }; + SkillAvatarDecisionRequest: { + approved?: boolean; + reason: string; + }; ApiResponsePublishResponse: { /** Format: int32 */ code?: number; @@ -6691,6 +7017,7 @@ export interface components { resolutionMode?: string; complianceSnapshot?: components["schemas"]["ComplianceSnapshotResponse"]; labels?: components["schemas"]["SkillLabelDto"][]; + avatarUrl?: string; }; ApiResponseBoolean: { /** Format: int32 */ @@ -6984,6 +7311,16 @@ export interface components { resolutionMode?: string; entryForSuites?: components["schemas"]["SkillSuiteReferenceResponse"][]; memberOfSuites?: components["schemas"]["PageResponseSkillSuiteReferenceResponse"]; + avatarUrl?: string; + }; + ApiResponseListSkillAvatarStatusResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SkillAvatarStatusResponse"][]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; }; ApiResponseReviewSkillDetailResponse: { /** Format: int32 */ @@ -9538,6 +9875,50 @@ export interface operations { }; }; }; + current_2: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseAvatarReviewSettingsResponse"]; + }; + }; + }; + }; + update: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["AvatarReviewSettingsUpdateRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseAvatarReviewSettingsResponse"]; + }; + }; + }; + }; getLocalSettings: { parameters: { query?: never; @@ -10873,6 +11254,364 @@ export interface operations { }; }; }; + takedown: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillAvatarReasonRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; + takedown_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillAvatarReasonRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; + retry: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + retry_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + decide: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillAvatarDecisionRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + decide_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillAvatarDecisionRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + appeal: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + appeal_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + current: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; + submit: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: { + content: { + "multipart/form-data": { + /** Format: binary */ + file: string; + }; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + delete: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; + current_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; + submit_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: { + content: { + "multipart/form-data": { + /** Format: binary */ + file: string; + }; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + delete_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseVoid"]; + }; + }; + }; + }; archiveSkill: { parameters: { query?: never; @@ -11599,7 +12338,7 @@ export interface operations { }; }; }; - submit: { + submit_2: { parameters: { query?: never; header?: never; @@ -11623,7 +12362,7 @@ export interface operations { }; }; }; - submit_1: { + submit_3: { parameters: { query?: never; header?: never; @@ -14619,6 +15358,100 @@ export interface operations { }; }; }; + status: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + status_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillAvatarStatusResponse"]; + }; + }; + }; + }; + candidate: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; + candidate_1: { + parameters: { + query?: never; + header?: never; + path: { + namespace: string; + slug: string; + candidateId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": string; + }; + }; + }; + }; getSkillDetail: { parameters: { query?: never; @@ -14715,6 +15548,46 @@ export interface operations { }; }; }; + queue: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListSkillAvatarStatusResponse"]; + }; + }; + }; + }; + queue_1: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListSkillAvatarStatusResponse"]; + }; + }; + }; + }; getReviewSkillDetail: { parameters: { query?: never; diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 9120b4e7..9f36bbf6 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -47,7 +47,9 @@ export interface SkillAvatarStatus { avatarUrl: string | null candidateId: number | null candidateStatus: 'PENDING' | 'ACTIVE' | 'REJECTED' | 'SUPERSEDED' | 'REMOVED' | null - machineStatus: 'SKIPPED' | 'PENDING' | 'PASSED' | 'BLOCKED' | 'ERROR' | null + machineStatus: 'SKIPPED' | 'PENDING' | 'PROCESSING' | 'PASSED' | 'BLOCKED' | 'ERROR' | null + machineNextRetryAt: string | null + automaticRetryExhausted: boolean reviewStatus: 'NOT_REQUIRED' | 'PENDING' | 'APPROVED' | 'REJECTED' | 'APPEAL_PENDING' | null reviewReason: string | null candidateUrl: string | null @@ -363,7 +365,7 @@ export type SkillDetail = Omit< | 'entryForSuites' | 'memberOfSuites' >, - 'labels' | 'headlineVersion' | 'publishedVersion' | 'ownerPreviewVersion' | 'entryForSuites' | 'memberOfSuites' + 'avatarUrl' | 'labels' | 'headlineVersion' | 'publishedVersion' | 'ownerPreviewVersion' | 'entryForSuites' | 'memberOfSuites' > & { avatarUrl?: string | null labels?: LabelItem[] diff --git a/web/src/features/skill/avatar-review-settings.test.tsx b/web/src/features/skill/avatar-review-settings.test.tsx new file mode 100644 index 00000000..9763eb8f --- /dev/null +++ b/web/src/features/skill/avatar-review-settings.test.tsx @@ -0,0 +1,34 @@ +/** @vitest-environment jsdom */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const api = vi.hoisted(() => ({ getAvatarReview: vi.fn(), updateAvatarReview: vi.fn() })) +vi.mock('@/api/client', () => ({ systemConfigApi: api })) +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })) +import { AvatarReviewSettingsPanel } from './avatar-review-settings' + +describe('AvatarReviewSettingsPanel', () => { + afterEach(() => { cleanup(); api.getAvatarReview.mockReset(); api.updateAvatarReview.mockReset() }) + + it('submits both switches with the observed version', async () => { + api.getAvatarReview.mockResolvedValue({ machineEnabled: false, manualEnabled: true, version: 3 }) + api.updateAvatarReview.mockResolvedValue({ machineEnabled: true, manualEnabled: false, version: 4 }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + fireEvent.click(await screen.findByRole('checkbox', { name: 'skillAvatar.machineEnabled' })) + fireEvent.click(screen.getByRole('checkbox', { name: 'skillAvatar.manualEnabled' })) + fireEvent.click(screen.getByRole('button', { name: 'skillAvatar.save' })) + await waitFor(() => expect(api.updateAvatarReview.mock.calls[0]?.[0]).toEqual({ + machineEnabled: true, manualEnabled: false, version: 3, + })) + }) + + it('warns before saving when both review switches are off', async () => { + api.getAvatarReview.mockResolvedValue({ machineEnabled: false, manualEnabled: true, version: 3 }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + fireEvent.click(await screen.findByRole('checkbox', { name: 'skillAvatar.manualEnabled' })) + expect(screen.getByRole('alert').textContent).toBe('skillAvatar.noReviewRisk') + }) +}) diff --git a/web/src/features/skill/avatar-review-settings.tsx b/web/src/features/skill/avatar-review-settings.tsx index 615c9c9a..4cfa58d4 100644 --- a/web/src/features/skill/avatar-review-settings.tsx +++ b/web/src/features/skill/avatar-review-settings.tsx @@ -26,6 +26,7 @@ export function AvatarReviewSettingsPanel() { {settings.isError &&

{t('skillAvatar.failed')}

} {settings.data && <> {(['machineEnabled', 'manualEnabled'] as const).map((field) => )} + {!draft.machineEnabled && !draft.manualEnabled &&

{t('skillAvatar.noReviewRisk')}

} } {saved &&

{t('skillAvatar.saved')}

} diff --git a/web/src/features/skill/skill-avatar-manager.test.tsx b/web/src/features/skill/skill-avatar-manager.test.tsx new file mode 100644 index 00000000..edbf10cc --- /dev/null +++ b/web/src/features/skill/skill-avatar-manager.test.tsx @@ -0,0 +1,39 @@ +/** @vitest-environment jsdom */ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const api = vi.hoisted(() => ({ status: 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() }) + + it.each([ + ['PENDING', 'BLOCKED', 'NOT_REQUIRED', true], + ['PENDING', 'BLOCKED', 'APPEAL_PENDING', false], + ['PENDING', 'PROCESSING', 'NOT_REQUIRED', false], + ['REJECTED', 'BLOCKED', 'REJECTED', false], + ['PENDING', 'ERROR', 'NOT_REQUIRED', false], + ] as const)('shows appeal only for machine-only blocked candidate: %s/%s/%s', async (candidateStatus, machineStatus, reviewStatus, expected) => { + 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(screen.queryByRole('button', { name: 'skillAvatar.appeal' }) !== null).toBe(expected) + }) + + it('disables paid retry during cooldown and explains when automatic retries stop', async () => { + api.status.mockResolvedValue({ avatarUrl: null, candidateId: 7, candidateStatus: 'PENDING', + machineStatus: 'ERROR', reviewStatus: 'NOT_REQUIRED', candidateUrl: null, reviewReason: null, + machineNextRetryAt: new Date(Date.now() + 3600000).toISOString(), automaticRetryExhausted: true }) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + + expect(await screen.findByText('skillAvatar.retryAfter')).toBeTruthy() + expect(screen.getByText('skillAvatar.automaticRetryStopped')).toBeTruthy() + expect(screen.getByRole('button', { name: 'skillAvatar.retry' }).hasAttribute('disabled')).toBe(true) + }) +}) diff --git a/web/src/features/skill/skill-avatar-manager.tsx b/web/src/features/skill/skill-avatar-manager.tsx index ba54ef50..cca3b279 100644 --- a/web/src/features/skill/skill-avatar-manager.tsx +++ b/web/src/features/skill/skill-avatar-manager.tsx @@ -54,12 +54,15 @@ export function SkillAvatarManager({ namespace, slug }: { namespace: string; slu } finally { setBusy(false) } } const candidate = status.data + const retryCoolingDown = !!candidate?.machineNextRetryAt && Date.parse(candidate.machineNextRetryAt) > Date.now() return (

{t('skillAvatar.title')}

{t('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] @@ -77,8 +80,10 @@ export function SkillAvatarManager({ namespace, slug }: { namespace: string; slu }
{(candidate?.avatarUrl || candidate?.candidateStatus === 'PENDING') && } - {candidate?.candidateStatus === 'REJECTED' && candidate.candidateId && } - {candidate?.machineStatus === 'ERROR' && candidate.candidateId && } + {candidate?.candidateStatus === 'PENDING' && candidate.machineStatus === 'BLOCKED' + && candidate.reviewStatus === 'NOT_REQUIRED' && candidate.candidateId + && } + {candidate?.machineStatus === 'ERROR' && candidate.candidateId && }
{error &&

{error}

} diff --git a/web/src/features/skill/skill-avatar.test.tsx b/web/src/features/skill/skill-avatar.test.tsx new file mode 100644 index 00000000..95161bec --- /dev/null +++ b/web/src/features/skill/skill-avatar.test.tsx @@ -0,0 +1,22 @@ +/** @vitest-environment jsdom */ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/api/client', () => ({ buildApiUrl: (url: string) => `/proxy${url}` })) +import { SkillAvatar } from './skill-avatar' + +describe('SkillAvatar', () => { + afterEach(cleanup) + it('shows the skill initial when no image is approved', () => { + render() + expect(screen.getByText('W')).toBeTruthy() + }) + + it('falls back to the initial if an approved image is unavailable', () => { + const { container } = render() + const image = container.querySelector('img')! + expect(image.getAttribute('src')).toBe('/proxy/avatar') + fireEvent.error(image) + expect(screen.getByText('W')).toBeTruthy() + }) +}) diff --git a/web/src/features/skill/skill-card.test.ts b/web/src/features/skill/skill-card.test.ts index 3b3a05fb..b4847b20 100644 --- a/web/src/features/skill/skill-card.test.ts +++ b/web/src/features/skill/skill-card.test.ts @@ -4,6 +4,8 @@ import { describe, expect, it, vi } from 'vitest' import * as mod from './skill-card' import { SkillCard } from './skill-card' +vi.mock('@/api/client', () => ({ buildApiUrl: (url: string) => url })) + vi.mock('@/features/auth/use-auth', () => ({ useAuth: () => ({ isAuthenticated: false, diff --git a/web/src/features/skill/skill-card.tsx b/web/src/features/skill/skill-card.tsx index 27098660..fef95a48 100644 --- a/web/src/features/skill/skill-card.tsx +++ b/web/src/features/skill/skill-card.tsx @@ -52,7 +52,7 @@ export function SkillCard({ skill, onClick, highlightStarred = true }: SkillCard >
-
+

{skill.displayName} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 2daf9d12..fb123666 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -2581,6 +2581,8 @@ "remove": "Remove current avatar", "appeal": "Request review", "retry": "Retry machine review", + "retryAfter": "Machine review failed. You can retry after {{time}}.", + "automaticRetryStopped": "Automatic retries have stopped. Retry manually after the cooldown or submit a new avatar.", "failed": "Action failed. Please try again.", "invalid": "Choose a PNG or JPEG image under 2 MB.", "reviewTitle": "Avatar review", @@ -2593,6 +2595,7 @@ "settingsHelp": "The machine provider is set at deployment; these switches control machine and manual review.", "machineEnabled": "Enable machine review", "manualEnabled": "Enable manual review", + "noReviewRisk": "Both reviews are off. New avatars will appear immediately. Confirm the risk before saving.", "save": "Save settings", "saved": "Saved", "states": { @@ -2605,6 +2608,7 @@ "machine": { "SKIPPED": "Machine review off", "PENDING": "Machine review pending", + "PROCESSING": "Machine review in progress", "PASSED": "Machine review passed", "BLOCKED": "Machine review blocked", "ERROR": "Machine review failed" diff --git a/web/src/i18n/locales/ru.json b/web/src/i18n/locales/ru.json index 4cd4fdf5..8f508ef6 100644 --- a/web/src/i18n/locales/ru.json +++ b/web/src/i18n/locales/ru.json @@ -2583,6 +2583,8 @@ "remove": "Удалить аватар", "appeal": "Запросить пересмотр", "retry": "Повторить автоматическую проверку", + "retryAfter": "Автоматическая проверка не удалась. Повторить можно после {{time}}.", + "automaticRetryStopped": "Автоматические повторы остановлены. После ожидания повторите вручную или загрузите новый аватар.", "failed": "Ошибка. Повторите попытку.", "invalid": "Выберите PNG или JPEG размером до 2 МБ.", "reviewTitle": "Проверка аватаров", @@ -2595,6 +2597,7 @@ "settingsHelp": "Поставщик автоматической проверки задаётся при развертывании; здесь включаются виды проверки.", "machineEnabled": "Автоматическая проверка", "manualEnabled": "Ручная проверка", + "noReviewRisk": "Обе проверки отключены. Новые аватары будут опубликованы сразу. Подтвердите риск перед сохранением.", "save": "Сохранить", "saved": "Сохранено", "states": { @@ -2607,6 +2610,7 @@ "machine": { "SKIPPED": "Проверка выключена", "PENDING": "Проверяется", + "PROCESSING": "Проверяется", "PASSED": "Проверка пройдена", "BLOCKED": "Заблокирован", "ERROR": "Ошибка проверки" diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 3ba0f918..4956cf6e 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -2580,6 +2580,8 @@ "remove": "删除当前头像", "appeal": "申请复核", "retry": "重试机器审核", + "retryAfter": "机器审核暂时失败，可在 {{time}} 后手动重试。", + "automaticRetryStopped": "自动重试已停止；冷却结束后可手动重试或提交新头像。", "failed": "操作失败，请稍后重试", "invalid": "请选择不超过 2 MB 的 PNG 或 JPEG 图片", "reviewTitle": "头像审核", @@ -2592,6 +2594,7 @@ "settingsHelp": "机器审核服务在部署时指定；这里只控制是否启用机器与人工审核。", "machineEnabled": "启用机器审核", "manualEnabled": "启用人工审核", + "noReviewRisk": "机器和人工审核均已关闭，新头像将直接展示，请确认风险后保存。", "save": "保存设置", "saved": "已保存", "states": { @@ -2604,6 +2607,7 @@ "machine": { "SKIPPED": "未启用机器审核", "PENDING": "机器审核中", + "PROCESSING": "机器审核中", "PASSED": "机器审核通过", "BLOCKED": "机器审核阻止", "ERROR": "机器审核失败" diff --git a/web/src/pages/admin/system-config.test.tsx b/web/src/pages/admin/system-config.test.tsx index bf84dac6..6c0d125d 100644 --- a/web/src/pages/admin/system-config.test.tsx +++ b/web/src/pages/admin/system-config.test.tsx @@ -11,6 +11,8 @@ const api = vi.hoisted(() => ({ createRoleGrant: vi.fn(), updateRoleGrant: vi.fn(), disableRoleGrant: vi.fn(), + getAvatarReview: vi.fn(), + updateAvatarReview: vi.fn(), })) vi.mock('@/api/client', () => ({ @@ -35,6 +37,7 @@ describe('SystemConfigPage', () => { api.getLocalAuth.mockResolvedValue({ passwordLoginEnabled: true, selfRegistrationEnabled: true, version: 3 }) api.listRoles.mockResolvedValue([{ code: 'SUPER_ADMIN', name: 'Super Admin' }, { code: 'USER', name: 'User' }]) api.listRoleGrants.mockResolvedValue(ruleList) + api.getAvatarReview.mockResolvedValue({ machineEnabled: false, manualEnabled: true, version: 0 }) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) render() return client diff --git a/web/src/pages/dashboard/namespace-reviews.test.ts b/web/src/pages/dashboard/namespace-reviews.test.ts index 735307ca..9868c27d 100644 --- a/web/src/pages/dashboard/namespace-reviews.test.ts +++ b/web/src/pages/dashboard/namespace-reviews.test.ts @@ -67,6 +67,8 @@ vi.mock('@/features/namespace/namespace-header', () => ({ NamespaceHeader: () => null, })) +vi.mock('@/features/skill/skill-avatar-review', () => ({ SkillAvatarReviewQueue: () => null })) + import { NamespaceReviewsPage } from './namespace-reviews' describe('NamespaceReviewsPage', () => { diff --git a/web/src/pages/dashboard/reviews.test.ts b/web/src/pages/dashboard/reviews.test.ts index 186c15eb..d5e181e5 100644 --- a/web/src/pages/dashboard/reviews.test.ts +++ b/web/src/pages/dashboard/reviews.test.ts @@ -91,6 +91,8 @@ vi.mock('./profile-review-table', () => ({ ProfileReviewTable: () => null, })) +vi.mock('@/features/skill/skill-avatar-review', () => ({ SkillAvatarReviewQueue: () => null })) + import { ReviewsPage } from './reviews' describe('ReviewsPage', () => { diff --git a/web/src/pages/dashboard/reviews.tsx b/web/src/pages/dashboard/reviews.tsx index 90cbc27b..c14255b3 100644 --- a/web/src/pages/dashboard/reviews.tsx +++ b/web/src/pages/dashboard/reviews.tsx @@ -266,7 +266,7 @@ export function ReviewsPage() {
- {isSkillAdmin && } + {isSkillAdmin && } {showTypeTabs ? (