diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java index 806f910d..8c068d18 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java @@ -23,6 +23,7 @@ import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; +import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.domain.social.SkillStarService; import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.service.SkillSearchAppService; @@ -55,6 +56,7 @@ public class ClawHubCompatController { private final NamespaceRepository namespaceRepository; private final SkillVersionRepository skillVersionRepository; private final SkillStarService skillStarService; + private final SkillSlugResolutionService skillSlugResolutionService; public ClawHubCompatController(CanonicalSlugMapper mapper, SkillSearchAppService skillSearchAppService, @@ -66,7 +68,8 @@ public class ClawHubCompatController { SkillRepository skillRepository, NamespaceRepository namespaceRepository, SkillVersionRepository skillVersionRepository, - SkillStarService skillStarService) { + SkillStarService skillStarService, + SkillSlugResolutionService skillSlugResolutionService) { this.mapper = mapper; this.skillSearchAppService = skillSearchAppService; this.skillQueryService = skillQueryService; @@ -78,6 +81,7 @@ public class ClawHubCompatController { this.namespaceRepository = namespaceRepository; this.skillVersionRepository = skillVersionRepository; this.skillStarService = skillStarService; + this.skillSlugResolutionService = skillSlugResolutionService; } @GetMapping("/search") @@ -420,24 +424,14 @@ public class ClawHubCompatController { } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - java.util.List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { + try { + return skillSlugResolutionService.resolve( + namespaceId, + slug, + currentUserId, + SkillSlugResolutionService.Preference.PUBLISHED); + } catch (com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException ex) { throw new DomainNotFoundException("error.skill.notFound", slug); } - java.util.Optional published = skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst(); - if (published.isPresent()) { - return published.get(); - } - if (currentUserId != null) { - java.util.Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - return skills.get(0); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java index 42b357cc..d1db37a2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java @@ -8,11 +8,11 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.review.ReviewService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.Skill; -import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.service.SkillGovernanceService; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.AdminSkillActionRequest; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; @@ -34,29 +34,29 @@ import org.springframework.web.bind.annotation.RestController; public class SkillLifecycleController extends BaseApiController { private final NamespaceRepository namespaceRepository; - private final SkillRepository skillRepository; private final SkillVersionRepository skillVersionRepository; private final SkillGovernanceService skillGovernanceService; private final ReviewService reviewService; private final SkillPublishService skillPublishService; private final AuditLogService auditLogService; + private final SkillSlugResolutionService skillSlugResolutionService; public SkillLifecycleController(NamespaceRepository namespaceRepository, - SkillRepository skillRepository, SkillVersionRepository skillVersionRepository, SkillGovernanceService skillGovernanceService, ReviewService reviewService, SkillPublishService skillPublishService, AuditLogService auditLogService, + SkillSlugResolutionService skillSlugResolutionService, ApiResponseFactory responseFactory) { super(responseFactory); this.namespaceRepository = namespaceRepository; - this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.skillGovernanceService = skillGovernanceService; this.reviewService = reviewService; this.skillPublishService = skillPublishService; this.auditLogService = auditLogService; + this.skillSlugResolutionService = skillSlugResolutionService; } @PostMapping("/{namespace}/{slug}/archive") @@ -189,24 +189,10 @@ public class SkillLifecycleController extends BaseApiController { } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - java.util.List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { - throw new DomainBadRequestException("error.skill.notFound", slug); - } - java.util.Optional published = skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst(); - if (published.isPresent()) { - return published.get(); - } - if (currentUserId != null) { - java.util.Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - return skills.get(0); + return skillSlugResolutionService.resolve( + namespaceId, + slug, + currentUserId, + SkillSlugResolutionService.Preference.CURRENT_USER); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReportController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReportController.java index a8daa230..c6180ef6 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReportController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReportController.java @@ -6,7 +6,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.report.SkillReportService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.skill.Skill; -import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.SkillReportMutationResponse; @@ -24,17 +24,17 @@ import org.springframework.web.bind.annotation.RestController; public class SkillReportController extends BaseApiController { private final NamespaceRepository namespaceRepository; - private final SkillRepository skillRepository; private final SkillReportService skillReportService; + private final SkillSlugResolutionService skillSlugResolutionService; public SkillReportController(NamespaceRepository namespaceRepository, - SkillRepository skillRepository, SkillReportService skillReportService, + SkillSlugResolutionService skillSlugResolutionService, ApiResponseFactory responseFactory) { super(responseFactory); this.namespaceRepository = namespaceRepository; - this.skillRepository = skillRepository; this.skillReportService = skillReportService; + this.skillSlugResolutionService = skillSlugResolutionService; } @PostMapping("/{namespace}/{slug}/reports") @@ -59,28 +59,10 @@ public class SkillReportController extends BaseApiController { String cleanNamespace = namespaceSlug.startsWith("@") ? namespaceSlug.substring(1) : namespaceSlug; Namespace namespace = namespaceRepository.findBySlug(cleanNamespace) .orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", cleanNamespace)); - return resolveVisibleSkill(namespace.getId(), skillSlug, currentUserId); + return skillSlugResolutionService.resolve( + namespace.getId(), + skillSlug, + currentUserId, + SkillSlugResolutionService.Preference.PUBLISHED); } - - private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - java.util.List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { - throw new DomainBadRequestException("error.skill.notFound", slug); - } - java.util.Optional published = skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst(); - if (published.isPresent()) { - return published.get(); - } - if (currentUserId != null) { - java.util.Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - return skills.get(0); - } -} \ No newline at end of file +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index f7e047b9..93330f71 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -154,6 +154,9 @@ public class ReviewService { SkillVersion skillVersion = skillVersionRepository.findById(task.getSkillVersionId()) .orElseThrow(() -> new DomainNotFoundException("skill_version.not_found", task.getSkillVersionId())); + if (skillVersion.getStatus() != SkillVersionStatus.PENDING_REVIEW) { + throw new DomainBadRequestException("review.not_pending", reviewTaskId); + } Skill skill = skillRepository.findById(skillVersion.getSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java index 2598a0bf..a57ba4e7 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/VisibilityChecker.java @@ -7,6 +7,9 @@ import java.util.Map; public class VisibilityChecker { public boolean canAccess(Skill skill, String currentUserId, Map userNamespaceRoles) { + if (skill.getLatestVersionId() == null) { + return isOwner(skill, currentUserId); + } return switch (skill.getVisibility()) { case PUBLIC -> true; case NAMESPACE_ONLY -> userNamespaceRoles.containsKey(skill.getNamespaceId()); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java index a53f1bbc..490f525b 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java @@ -14,9 +14,7 @@ import org.springframework.stereotype.Service; import java.io.InputStream; import java.time.Duration; -import java.util.List; import java.util.Map; -import java.util.Optional; @Service public class SkillDownloadService { @@ -28,6 +26,7 @@ public class SkillDownloadService { private final ObjectStorageService objectStorageService; private final VisibilityChecker visibilityChecker; private final ApplicationEventPublisher eventPublisher; + private final SkillSlugResolutionService skillSlugResolutionService; public SkillDownloadService( NamespaceRepository namespaceRepository, @@ -36,7 +35,8 @@ public class SkillDownloadService { SkillTagRepository skillTagRepository, ObjectStorageService objectStorageService, VisibilityChecker visibilityChecker, - ApplicationEventPublisher eventPublisher) { + ApplicationEventPublisher eventPublisher, + SkillSlugResolutionService skillSlugResolutionService) { this.namespaceRepository = namespaceRepository; this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; @@ -44,6 +44,7 @@ public class SkillDownloadService { this.objectStorageService = objectStorageService; this.visibilityChecker = visibilityChecker; this.eventPublisher = eventPublisher; + this.skillSlugResolutionService = skillSlugResolutionService; } public record DownloadResult( @@ -155,25 +156,11 @@ public class SkillDownloadService { } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { - throw new DomainBadRequestException("error.skill.notFound", slug); - } - Optional published = skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst(); - if (published.isPresent()) { - return published.get(); - } - if (currentUserId != null) { - Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - return skills.get(0); + return skillSlugResolutionService.resolve( + namespaceId, + slug, + currentUserId, + SkillSlugResolutionService.Preference.CURRENT_USER); } private void assertPublishedAccessible(Skill skill) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index 406bf6f1..5d9ff848 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -8,6 +8,7 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.namespace.SlugValidator; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; import com.iflytek.skillhub.domain.review.ReviewTask; import com.iflytek.skillhub.domain.review.ReviewTaskRepository; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; @@ -219,6 +220,8 @@ public class SkillPublishService { List pendingVersions = skillVersionRepository .findBySkillIdAndStatus(skill.getId(), SkillVersionStatus.PENDING_REVIEW); for (SkillVersion pending : pendingVersions) { + reviewTaskRepository.findBySkillVersionIdAndStatus(pending.getId(), ReviewTaskStatus.PENDING) + .ifPresent(reviewTaskRepository::delete); pending.setStatus(SkillVersionStatus.DRAFT); skillVersionRepository.save(pending); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java index 4b192bf2..6ef970e8 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java @@ -40,6 +40,7 @@ public class SkillQueryService { private final ObjectStorageService objectStorageService; private final VisibilityChecker visibilityChecker; private final PromotionRequestRepository promotionRequestRepository; + private final SkillSlugResolutionService skillSlugResolutionService; public SkillQueryService( NamespaceRepository namespaceRepository, @@ -49,7 +50,8 @@ public class SkillQueryService { SkillTagRepository skillTagRepository, ObjectStorageService objectStorageService, VisibilityChecker visibilityChecker, - PromotionRequestRepository promotionRequestRepository) { + PromotionRequestRepository promotionRequestRepository, + SkillSlugResolutionService skillSlugResolutionService) { this.namespaceRepository = namespaceRepository; this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; @@ -58,6 +60,7 @@ public class SkillQueryService { this.objectStorageService = objectStorageService; this.visibilityChecker = visibilityChecker; this.promotionRequestRepository = promotionRequestRepository; + this.skillSlugResolutionService = skillSlugResolutionService; } public record SkillDetailDTO( @@ -345,24 +348,11 @@ public class SkillQueryService { } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { - throw new DomainBadRequestException("error.skill.notFound", slug); - } - - if (currentUserId != null) { - Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - - return skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst() - .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", slug)); + return skillSlugResolutionService.resolve( + namespaceId, + slug, + currentUserId, + SkillSlugResolutionService.Preference.CURRENT_USER); } private SkillVersion findVersion(Skill skill, String version) { diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java new file mode 100644 index 00000000..d7f9d1bf --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionService.java @@ -0,0 +1,46 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import org.springframework.stereotype.Service; + +import java.util.List; +import java.util.Optional; + +@Service +public class SkillSlugResolutionService { + + public enum Preference { + CURRENT_USER, + PUBLISHED + } + + private final SkillRepository skillRepository; + + public SkillSlugResolutionService(SkillRepository skillRepository) { + this.skillRepository = skillRepository; + } + + public Skill resolve(Long namespaceId, String slug, String currentUserId, Preference preference) { + List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); + if (skills.isEmpty()) { + throw new DomainBadRequestException("error.skill.notFound", slug); + } + + Optional ownSkill = currentUserId == null + ? Optional.empty() + : skills.stream().filter(skill -> currentUserId.equals(skill.getOwnerId())).findFirst(); + Optional publishedSkill = skills.stream() + .filter(skill -> skill.getLatestVersionId() != null) + .findFirst(); + + if (preference == Preference.CURRENT_USER) { + return ownSkill.or(() -> publishedSkill) + .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", slug)); + } + + return publishedSkill.or(() -> ownSkill) + .orElseThrow(() -> new DomainBadRequestException("error.skill.notFound", slug)); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java index b99ac0e2..07c3fdc0 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillTagService.java @@ -11,7 +11,6 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import java.util.List; -import java.util.Optional; @Service public class SkillTagService { @@ -24,6 +23,7 @@ public class SkillTagService { private final SkillVersionRepository skillVersionRepository; private final SkillTagRepository skillTagRepository; private final VisibilityChecker visibilityChecker; + private final SkillSlugResolutionService skillSlugResolutionService; public SkillTagService( NamespaceRepository namespaceRepository, @@ -31,13 +31,15 @@ public class SkillTagService { SkillRepository skillRepository, SkillVersionRepository skillVersionRepository, SkillTagRepository skillTagRepository, - VisibilityChecker visibilityChecker) { + VisibilityChecker visibilityChecker, + SkillSlugResolutionService skillSlugResolutionService) { this.namespaceRepository = namespaceRepository; this.namespaceMemberRepository = namespaceMemberRepository; this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.skillTagRepository = skillTagRepository; this.visibilityChecker = visibilityChecker; + this.skillSlugResolutionService = skillSlugResolutionService; } public List listTags(String namespaceSlug, @@ -124,25 +126,11 @@ public class SkillTagService { } private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) { - List skills = skillRepository.findByNamespaceIdAndSlug(namespaceId, slug); - if (skills.isEmpty()) { - throw new DomainBadRequestException("error.skill.notFound", slug); - } - Optional published = skills.stream() - .filter(s -> s.getLatestVersionId() != null) - .findFirst(); - if (published.isPresent()) { - return published.get(); - } - if (currentUserId != null) { - Optional ownSkill = skills.stream() - .filter(s -> currentUserId.equals(s.getOwnerId())) - .findFirst(); - if (ownSkill.isPresent()) { - return ownSkill.get(); - } - } - return skills.get(0); + return skillSlugResolutionService.resolve( + namespaceId, + slug, + currentUserId, + SkillSlugResolutionService.Preference.CURRENT_USER); } private void assertAdminOrOwner(Long namespaceId, String operatorId) { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java index b71cac4a..ab659faa 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java @@ -319,6 +319,24 @@ class ReviewServiceTest { () -> reviewService.approveReview(REVIEW_TASK_ID, REVIEWER_ID, "ok", Map.of(), Set.of())); } + @Test + void shouldRejectApproveWhenSkillVersionWasWithdrawnBackToDraft() { + ReviewTask task = createPendingReviewTask(); + Namespace ns = createTeamNamespace(); + SkillVersion sv = createPendingReviewSkillVersion(); + sv.setStatus(SkillVersionStatus.DRAFT); + + when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); + when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(ns)); + when(permissionChecker.canReview(any(), any(), any(), anyMap(), anySet())).thenReturn(true); + when(reviewTaskRepository.updateStatusWithVersion(any(), any(), any(), any(), any())).thenReturn(1); + when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv)); + + assertThrows(DomainBadRequestException.class, + () -> reviewService.approveReview(REVIEW_TASK_ID, REVIEWER_ID, "ok", + Map.of(NAMESPACE_ID, NamespaceRole.ADMIN), Set.of())); + } + @Test void shouldThrowWhenNoPermission() { ReviewTask task = createPendingReviewTask(); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/VisibilityCheckerTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/VisibilityCheckerTest.java index 4d273345..46d8601b 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/VisibilityCheckerTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/VisibilityCheckerTest.java @@ -14,6 +14,7 @@ class VisibilityCheckerTest { private Skill publicSkill; private Skill namespaceOnlySkill; private Skill privateSkill; + private Skill unpublishedPublicSkill; private static final Long NAMESPACE_ID = 1L; private static final String OWNER_ID = "user-100"; @@ -26,8 +27,12 @@ class VisibilityCheckerTest { checker = new VisibilityChecker(); publicSkill = new Skill(NAMESPACE_ID, "public-skill", OWNER_ID, SkillVisibility.PUBLIC); + publicSkill.setLatestVersionId(10L); namespaceOnlySkill = new Skill(NAMESPACE_ID, "namespace-skill", OWNER_ID, SkillVisibility.NAMESPACE_ONLY); + namespaceOnlySkill.setLatestVersionId(11L); privateSkill = new Skill(NAMESPACE_ID, "private-skill", OWNER_ID, SkillVisibility.PRIVATE); + privateSkill.setLatestVersionId(12L); + unpublishedPublicSkill = new Skill(NAMESPACE_ID, "draft-public-skill", OWNER_ID, SkillVisibility.PUBLIC); } @Test @@ -99,4 +104,29 @@ class VisibilityCheckerTest { boolean canAccess = checker.canAccess(privateSkill, OTHER_USER_ID, Map.of()); assertFalse(canAccess); } + + @Test + void testUnpublishedSkillNotAccessibleByAnonymousEvenWhenPublic() { + boolean canAccess = checker.canAccess(unpublishedPublicSkill, null, Map.of()); + assertFalse(canAccess); + } + + @Test + void testUnpublishedSkillNotAccessibleByOtherUserEvenWhenPublic() { + boolean canAccess = checker.canAccess(unpublishedPublicSkill, OTHER_USER_ID, Map.of()); + assertFalse(canAccess); + } + + @Test + void testUnpublishedSkillNotAccessibleByAdmin() { + Map roles = Map.of(NAMESPACE_ID, NamespaceRole.ADMIN); + boolean canAccess = checker.canAccess(unpublishedPublicSkill, ADMIN_USER_ID, roles); + assertFalse(canAccess); + } + + @Test + void testUnpublishedSkillAccessibleByOwner() { + boolean canAccess = checker.canAccess(unpublishedPublicSkill, OWNER_ID, Map.of()); + assertTrue(canAccess); + } } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java index 696c12fb..819f0b0f 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java @@ -46,9 +46,11 @@ class SkillDownloadServiceTest { private ApplicationEventPublisher eventPublisher; private SkillDownloadService service; + private SkillSlugResolutionService skillSlugResolutionService; @BeforeEach void setUp() { + skillSlugResolutionService = new SkillSlugResolutionService(skillRepository); service = new SkillDownloadService( namespaceRepository, skillRepository, @@ -56,7 +58,8 @@ class SkillDownloadServiceTest { skillTagRepository, objectStorageService, visibilityChecker, - eventPublisher + eventPublisher, + skillSlugResolutionService ); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index 27e87acc..7987dae0 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -643,6 +643,7 @@ class SkillPublishServiceTest { SkillVersion pendingV1 = new SkillVersion(1L, "1.0.0", publisherId); pendingV1.setStatus(SkillVersionStatus.PENDING_REVIEW); setId(pendingV1, 5L); + ReviewTask pendingTask = new ReviewTask(5L, 1L, publisherId); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(namespaceMemberRepository.findByNamespaceIdAndUserId(any(), eq(publisherId))).thenReturn(Optional.of(member)); @@ -652,6 +653,8 @@ class SkillPublishServiceTest { when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(List.of(skill)); when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("test-skill"), eq(publisherId))).thenReturn(Optional.of(skill)); when(skillVersionRepository.findBySkillIdAndStatus(1L, SkillVersionStatus.PENDING_REVIEW)).thenReturn(List.of(pendingV1)); + when(reviewTaskRepository.findBySkillVersionIdAndStatus(5L, com.iflytek.skillhub.domain.review.ReviewTaskStatus.PENDING)) + .thenReturn(Optional.of(pendingTask)); when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("2.0.0"))).thenReturn(Optional.empty()); when(skillVersionRepository.save(any(SkillVersion.class))).thenAnswer(invocation -> { SkillVersion saved = invocation.getArgument(0); @@ -664,6 +667,7 @@ class SkillPublishServiceTest { // Verify pending version was withdrawn to DRAFT assertEquals(SkillVersionStatus.DRAFT, pendingV1.getStatus()); + verify(reviewTaskRepository).delete(pendingTask); verify(skillVersionRepository).save(pendingV1); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java index 79f9e611..59d3c432 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java @@ -52,9 +52,11 @@ class SkillQueryServiceTest { private PromotionRequestRepository promotionRequestRepository; private SkillQueryService service; + private SkillSlugResolutionService skillSlugResolutionService; @BeforeEach void setUp() { + skillSlugResolutionService = new SkillSlugResolutionService(skillRepository); service = new SkillQueryService( namespaceRepository, skillRepository, @@ -63,7 +65,8 @@ class SkillQueryServiceTest { skillTagRepository, objectStorageService, visibilityChecker, - promotionRequestRepository + promotionRequestRepository, + skillSlugResolutionService ); } @@ -101,6 +104,41 @@ class SkillQueryServiceTest { assertEquals("1.0.0", result.latestVersion()); } + @Test + void testGetSkillDetail_PrefersCurrentUsersOwnSkillOverOtherPublishedSkill() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String userId = "user-100"; + Map userNsRoles = Map.of(1L, NamespaceRole.MEMBER); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + + Skill publishedSkill = new Skill(1L, skillSlug, "user-200", SkillVisibility.PUBLIC); + setId(publishedSkill, 1L); + publishedSkill.setDisplayName("Published Skill"); + publishedSkill.setLatestVersionId(11L); + + Skill ownSkill = new Skill(1L, skillSlug, userId, SkillVisibility.PUBLIC); + setId(ownSkill, 2L); + ownSkill.setDisplayName("Own Skill"); + ownSkill.setLatestVersionId(22L); + + SkillVersion ownVersion = new SkillVersion(2L, "2.0.0", userId); + setId(ownVersion, 22L); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(publishedSkill, ownSkill)); + when(visibilityChecker.canAccess(ownSkill, userId, userNsRoles)).thenReturn(true); + when(skillVersionRepository.findById(22L)).thenReturn(Optional.of(ownVersion)); + + SkillQueryService.SkillDetailDTO result = service.getSkillDetail(namespaceSlug, skillSlug, userId, userNsRoles); + + assertEquals(2L, result.id()); + assertEquals("Own Skill", result.displayName()); + assertEquals("2.0.0", result.latestVersion()); + } + @Test void testGetSkillDetail_AccessDenied() throws Exception { // Arrange @@ -113,6 +151,7 @@ class SkillQueryServiceTest { setId(namespace, 1L); Skill skill = new Skill(1L, skillSlug, "user-200", SkillVisibility.PRIVATE); setId(skill, 1L); + skill.setLatestVersionId(11L); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); @@ -134,9 +173,10 @@ class SkillQueryServiceTest { setId(namespace, 1L); Skill skill = new Skill(1L, skillSlug, "user-200", SkillVisibility.PUBLIC); setId(skill, 1L); + skill.setLatestVersionId(11L); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); - when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(Optional.of(skill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); assertThrows(DomainForbiddenException.class, () -> service.getSkillDetail(namespaceSlug, skillSlug, null, Map.of())); @@ -170,6 +210,51 @@ class SkillQueryServiceTest { assertEquals("skill1", result.getContent().get(0).getSlug()); } + @Test + void testGetSkillDetail_ShouldHideOtherUsersUnpublishedSkill() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String viewerId = "user-300"; + Map userNsRoles = Map.of(1L, NamespaceRole.ADMIN); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + Skill unpublishedSkill = new Skill(1L, skillSlug, "user-200", SkillVisibility.PUBLIC); + setId(unpublishedSkill, 1L); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(unpublishedSkill)); + + assertThrows(DomainBadRequestException.class, () -> + service.getSkillDetail(namespaceSlug, skillSlug, viewerId, userNsRoles)); + } + + @Test + void testListSkillsByNamespace_ShouldHideOtherUsersUnpublishedSkills() throws Exception { + String namespaceSlug = "test-ns"; + String userId = "user-100"; + Map userNsRoles = Map.of(1L, NamespaceRole.MEMBER); + Pageable pageable = PageRequest.of(0, 10); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + Skill ownUnpublishedSkill = new Skill(1L, "own-skill", userId, SkillVisibility.PUBLIC); + setId(ownUnpublishedSkill, 1L); + Skill othersUnpublishedSkill = new Skill(1L, "other-skill", "user-200", SkillVisibility.PUBLIC); + setId(othersUnpublishedSkill, 2L); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndStatus(1L, SkillStatus.ACTIVE)) + .thenReturn(List.of(ownUnpublishedSkill, othersUnpublishedSkill)); + when(visibilityChecker.canAccess(ownUnpublishedSkill, userId, userNsRoles)).thenReturn(true); + when(visibilityChecker.canAccess(othersUnpublishedSkill, userId, userNsRoles)).thenReturn(false); + + Page result = service.listSkillsByNamespace(namespaceSlug, userId, userNsRoles, pageable); + + assertEquals(1, result.getTotalElements()); + assertEquals("own-skill", result.getContent().get(0).getSlug()); + } + @Test void testListFiles() throws Exception { // Arrange @@ -487,7 +572,7 @@ class SkillQueryServiceTest { published.setStatus(SkillVersionStatus.PUBLISHED); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); - when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(Optional.of(skill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(published)); when(promotionRequestRepository.findBySourceSkillIdAndStatus(1L, ReviewTaskStatus.PENDING)).thenReturn(Optional.empty()); @@ -518,7 +603,7 @@ class SkillQueryServiceTest { published.setStatus(SkillVersionStatus.PUBLISHED); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); - when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(Optional.of(skill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(published)); when(promotionRequestRepository.findBySourceSkillIdAndStatus(1L, ReviewTaskStatus.PENDING)) @@ -548,7 +633,7 @@ class SkillQueryServiceTest { published.setStatus(SkillVersionStatus.PUBLISHED); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); - when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(Optional.of(skill)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(published)); when(promotionRequestRepository.findBySourceSkillIdAndStatus(1L, ReviewTaskStatus.PENDING)).thenReturn(Optional.empty()); @@ -572,10 +657,16 @@ class SkillQueryServiceTest { Skill skill = new Skill(1L, skillSlug, "owner-1", SkillVisibility.PUBLIC); setId(skill, 1L); skill.setStatus(SkillStatus.ACTIVE); + skill.setLatestVersionId(11L); + + SkillVersion published = new SkillVersion(1L, "1.0.0", "owner-1"); + setId(published, 11L); + published.setStatus(SkillVersionStatus.PUBLISHED); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); when(visibilityChecker.canAccess(skill, userId, userNsRoles)).thenReturn(true); + when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(published)); SkillQueryService.SkillDetailDTO result = service.getSkillDetail(namespaceSlug, skillSlug, userId, userNsRoles); @@ -737,6 +828,7 @@ class SkillQueryServiceTest { SkillVersion pending = new SkillVersion(1L, version, "owner-1"); setId(pending, 11L); pending.setStatus(SkillVersionStatus.PENDING_REVIEW); + skill.setLatestVersionId(10L); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); @@ -803,6 +895,7 @@ class SkillQueryServiceTest { SkillVersion published = new SkillVersion(1L, "1.0.0", "owner-1"); setId(published, 11L); published.setStatus(SkillVersionStatus.PUBLISHED); + skill.setLatestVersionId(11L); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java new file mode 100644 index 00000000..8fdbdac1 --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillSlugResolutionServiceTest.java @@ -0,0 +1,69 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class SkillSlugResolutionServiceTest { + + private final SkillRepository skillRepository = mock(SkillRepository.class); + private final SkillSlugResolutionService service = new SkillSlugResolutionService(skillRepository); + + @Test + void prefersCurrentUsersOwnSkillWhenRequested() throws Exception { + Skill publishedSkill = createSkill(1L, "demo", "user-2", 11L); + Skill ownSkill = createSkill(2L, "demo", "user-1", 22L); + when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of(publishedSkill, ownSkill)); + + Skill resolved = service.resolve(1L, "demo", "user-1", SkillSlugResolutionService.Preference.CURRENT_USER); + + assertEquals(2L, resolved.getId()); + } + + @Test + void prefersPublishedSkillForPublicInteractions() throws Exception { + Skill ownDraft = createSkill(2L, "demo", "user-1", null); + Skill publishedSkill = createSkill(1L, "demo", "user-2", 11L); + when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of(ownDraft, publishedSkill)); + + Skill resolved = service.resolve(1L, "demo", "user-1", SkillSlugResolutionService.Preference.PUBLISHED); + + assertEquals(1L, resolved.getId()); + } + + @Test + void throwsWhenNoSkillMatchesSlug() { + when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of()); + + assertThrows(DomainBadRequestException.class, () -> + service.resolve(1L, "demo", "user-1", SkillSlugResolutionService.Preference.CURRENT_USER)); + } + + @Test + void throwsWhenOnlyUnpublishedSkillsBelongToOtherUsers() throws Exception { + Skill otherUsersDraft = createSkill(3L, "demo", "user-2", null); + when(skillRepository.findByNamespaceIdAndSlug(1L, "demo")).thenReturn(List.of(otherUsersDraft)); + + assertThrows(DomainBadRequestException.class, () -> + service.resolve(1L, "demo", null, SkillSlugResolutionService.Preference.CURRENT_USER)); + } + + private Skill createSkill(Long id, String slug, String ownerId, Long latestVersionId) throws Exception { + Skill skill = new Skill(1L, slug, ownerId, SkillVisibility.PUBLIC); + Field idField = Skill.class.getDeclaredField("id"); + idField.setAccessible(true); + idField.set(skill, id); + skill.setLatestVersionId(latestVersionId); + return skill; + } +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillTagServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillTagServiceTest.java index 32c1ee71..40889385 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillTagServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillTagServiceTest.java @@ -39,16 +39,19 @@ class SkillTagServiceTest { private VisibilityChecker visibilityChecker; private SkillTagService service; + private SkillSlugResolutionService skillSlugResolutionService; @BeforeEach void setUp() { + skillSlugResolutionService = new SkillSlugResolutionService(skillRepository); service = new SkillTagService( namespaceRepository, namespaceMemberRepository, skillRepository, skillVersionRepository, skillTagRepository, - visibilityChecker + visibilityChecker, + skillSlugResolutionService ); }