diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java index b07f042b..7bc413c3 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java @@ -6,6 +6,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; @@ -73,6 +74,21 @@ class SkillReviewControllerTest { .andExpect(jsonPath("$.code").value(401)); } + @Test + void anonymousUserCannotUpsertOrClearReview() throws Exception { + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"score\":4,\"reviewText\":\"Useful\"}")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + + mockMvc.perform(delete("/api/v1/skills/10/reviews/me") + .with(csrf())) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + @Test void authenticatedUserCanUpsertReview() throws Exception { var principal = principal("user-42", Set.of()); @@ -105,6 +121,19 @@ class SkillReviewControllerTest { .andExpect(jsonPath("$.code").value(400)); } + @Test + void reviewTextIsRequiredByTheApiContract() throws Exception { + var principal = principal("user-42", Set.of()); + + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(authentication(authToken(principal, "ROLE_USER"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"score\":4}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + @Test void skillAdminCanHideReview() throws Exception { var principal = principal("admin", Set.of("SKILL_ADMIN")); @@ -132,6 +161,19 @@ class SkillReviewControllerTest { .andExpect(status().isForbidden()); } + @Test + void moderationReasonRejectsMoreThanFiveHundredCharacters() throws Exception { + var principal = principal("admin", Set.of("SKILL_ADMIN")); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/8/hide") + .with(authentication(authToken(principal, "ROLE_SKILL_ADMIN"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reason\":\"" + "x".repeat(501) + "\"}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + private PlatformPrincipal principal(String userId, Set roles) { return new PlatformPrincipal(userId, userId, userId + "@example.test", null, "local", roles); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java new file mode 100644 index 00000000..8c3d7498 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java @@ -0,0 +1,154 @@ +package com.iflytek.skillhub.controller.admin; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditLog; +import com.iflytek.skillhub.domain.governance.GovernanceNotificationService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillReviewStatus; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.AuditLogJpaRepository; +import com.iflytek.skillhub.infra.jpa.JpaSkillRatingRepository; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import com.iflytek.skillhub.notification.service.NotificationDispatcher; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.context.annotation.Import; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest(classes = SkillhubApplication.class) +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class SkillReviewModerationFlowIntegrationTest { + + private static final String ADMIN_ID = "review-admin"; + private static final String AUTHOR_ID = "review-author"; + + @Autowired private MockMvc mockMvc; + @Autowired private UserAccountJpaRepository userAccountRepository; + @Autowired private NamespaceJpaRepository namespaceRepository; + @Autowired private SkillJpaRepository skillRepository; + @Autowired private JpaSkillRatingRepository ratingRepository; + + @SpyBean private AuditLogJpaRepository auditLogRepository; + @MockBean private DeviceAuthService deviceAuthService; + @MockBean private RbacService rbacService; + @MockBean private GovernanceNotificationService governanceNotificationService; + @MockBean private NotificationDispatcher notificationDispatcher; + + @Test + void hideAndRestorePersistModerationStateAndAuditRows() throws Exception { + SkillRating review = createReview(); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/hide") + .contentType("application/json") + .content("{\"reason\":\"off topic\"}") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.status").value("HIDDEN")); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/restore") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.status").value("VISIBLE")); + + SkillRating saved = ratingRepository.findById(review.getId()).orElseThrow(); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + assertThat(saved.getModeratedBy()).isEqualTo(ADMIN_ID); + + List actions = auditLogRepository.findAll().stream() + .filter(log -> ADMIN_ID.equals(log.getActorUserId())) + .map(AuditLog::getAction) + .toList(); + assertThat(actions).contains("SKILL_REVIEW_HIDE", "SKILL_REVIEW_RESTORE"); + } + + @Test + void hideRollsBackModerationWhenAuditPersistenceFails() throws Exception { + SkillRating review = createReview(); + doThrow(new DataIntegrityViolationException("forced audit failure")) + .when(auditLogRepository).save(any(AuditLog.class)); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/hide") + .contentType("application/json") + .content("{\"reason\":\"off topic\"}") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isInternalServerError()); + + SkillRating saved = ratingRepository.findById(review.getId()).orElseThrow(); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + assertThat(saved.getModeratedBy()).isNull(); + assertThat(saved.getModerationReason()).isNull(); + } + + private SkillRating createReview() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + saveUserIfAbsent(ADMIN_ID, "Review Admin", "review-admin@example.test"); + saveUserIfAbsent(AUTHOR_ID, "Review Author", "review-author@example.test"); + + Namespace namespace = namespaceRepository.saveAndFlush( + new Namespace("review-team-" + suffix, "Review Team " + suffix, AUTHOR_ID)); + Skill skill = new Skill(namespace.getId(), "review-skill-" + suffix, AUTHOR_ID, SkillVisibility.PUBLIC); + skill.setCreatedBy(AUTHOR_ID); + skill.setUpdatedBy(AUTHOR_ID); + skill = skillRepository.saveAndFlush(skill); + + SkillRating review = new SkillRating(skill.getId(), AUTHOR_ID, (short) 4); + review.updateReview((short) 4, "Useful review"); + return ratingRepository.saveAndFlush(review); + } + + private void saveUserIfAbsent(String userId, String displayName, String email) { + if (!userAccountRepository.existsById(userId)) { + userAccountRepository.saveAndFlush(new UserAccount(userId, displayName, email, null)); + } + } + + private UsernamePasswordAuthenticationToken adminAuth() { + PlatformPrincipal principal = new PlatformPrincipal( + ADMIN_ID, + "Review Admin", + "review-admin@example.test", + "", + "session", + Set.of("SKILL_ADMIN") + ); + return new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN")) + ); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java index 6d5996ef..86aa1141 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillReviewStatus; import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.OptimisticLockException; @@ -31,12 +32,21 @@ class SkillRatingOptimisticLockingTest { SkillRating first = firstManager.find(SkillRating.class, ratingId); SkillRating stale = staleManager.find(SkillRating.class, ratingId); - first.updateReview((short) 5, "First update"); + first.hideReview("moderator", "Policy violation"); firstManager.getTransaction().commit(); stale.updateReview((short) 2, "Stale update"); assertThatThrownBy(staleManager.getTransaction()::commit) .satisfies(error -> assertThat(hasCause(error, OptimisticLockException.class)).isTrue()); + + EntityManager verifier = entityManagerFactory.createEntityManager(); + try { + SkillRating saved = verifier.find(SkillRating.class, ratingId); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(saved.getModerationReason()).isEqualTo("Policy violation"); + } finally { + verifier.close(); + } } finally { rollbackIfActive(firstManager); rollbackIfActive(staleManager); @@ -46,6 +56,45 @@ class SkillRatingOptimisticLockingTest { } } + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void duplicateFirstInsertIsRejectedByTheDatabaseUniqueConstraint() { + EntityManager firstManager = entityManagerFactory.createEntityManager(); + EntityManager secondManager = entityManagerFactory.createEntityManager(); + try { + firstManager.getTransaction().begin(); + secondManager.getTransaction().begin(); + assertThat(countRatings(firstManager, 20L, "duplicate-author")).isZero(); + assertThat(countRatings(secondManager, 20L, "duplicate-author")).isZero(); + + SkillRating first = new SkillRating(20L, "duplicate-author", (short) 4); + first.updateReview((short) 4, "First insert"); + firstManager.persist(first); + firstManager.getTransaction().commit(); + + SkillRating duplicate = new SkillRating(20L, "duplicate-author", (short) 5); + duplicate.updateReview((short) 5, "Duplicate insert"); + assertThatThrownBy(() -> { + secondManager.persist(duplicate); + secondManager.flush(); + secondManager.getTransaction().commit(); + }).isInstanceOf(RuntimeException.class); + + EntityManager verifier = entityManagerFactory.createEntityManager(); + try { + assertThat(countRatings(verifier, 20L, "duplicate-author")).isEqualTo(1L); + } finally { + verifier.close(); + } + } finally { + rollbackIfActive(firstManager); + rollbackIfActive(secondManager); + firstManager.close(); + secondManager.close(); + deleteRatings(20L, "duplicate-author"); + } + } + private Long persistRating() { EntityManager entityManager = entityManagerFactory.createEntityManager(); try { @@ -82,6 +131,34 @@ class SkillRatingOptimisticLockingTest { } } + private long countRatings(EntityManager entityManager, Long skillId, String userId) { + return entityManager.createQuery(""" + SELECT COUNT(r) FROM SkillRating r + WHERE r.skillId = :skillId AND r.userId = :userId + """, Long.class) + .setParameter("skillId", skillId) + .setParameter("userId", userId) + .getSingleResult(); + } + + private void deleteRatings(Long skillId, String userId) { + EntityManager entityManager = entityManagerFactory.createEntityManager(); + try { + entityManager.getTransaction().begin(); + entityManager.createQuery(""" + DELETE FROM SkillRating r + WHERE r.skillId = :skillId AND r.userId = :userId + """) + .setParameter("skillId", skillId) + .setParameter("userId", userId) + .executeUpdate(); + entityManager.getTransaction().commit(); + } finally { + rollbackIfActive(entityManager); + entityManager.close(); + } + } + private boolean hasCause(Throwable error, Class expectedType) { Throwable current = error; while (current != null) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java index 53697e56..eb8ff6ed 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java @@ -2,12 +2,14 @@ package com.iflytek.skillhub.service; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; @@ -80,6 +82,21 @@ class SkillReviewAppServiceTest { verify(queryRepository).list(eq(10L), eq("admin"), eq(true), any(Pageable.class)); } + @Test + void reviewPaginationRejectsNegativePageAndSizesOutsideOneToOneHundred() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), -1, 20)) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), 0, 0)) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), 0, 101)) + .isInstanceOf(DomainBadRequestException.class); + + verify(queryRepository, never()).list(any(), any(), anyBoolean(), any(Pageable.class)); + } + @Test void privateSkillReviewMutationRequiresSkillAccess() { Skill skill = publishedSkill(SkillVisibility.PRIVATE); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java index 45b98388..53f3a262 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java @@ -188,6 +188,25 @@ class SkillRatingServiceTest { assertThat(restored.getModerationReason()).isNull(); } + @Test + void moderationReason_acceptsFiveHundredCharactersAndRejectsFiveHundredOne() { + SkillRating valid = new SkillRating(1L, "user-1", (short) 4); + valid.updateReview((short) 4, "Useful skill"); + SkillRating invalid = new SkillRating(1L, "user-2", (short) 4); + invalid.updateReview((short) 4, "Another useful skill"); + when(ratingRepository.findById(7L)).thenReturn(Optional.of(valid)); + when(ratingRepository.findById(8L)).thenReturn(Optional.of(invalid)); + when(ratingRepository.save(valid)).thenReturn(valid); + + SkillRating hidden = service.hideReview(7L, "moderator", "x".repeat(500)); + assertThat(hidden.getModerationReason()).hasSize(500); + + assertThatThrownBy(() -> service.hideReview(8L, "moderator", "x".repeat(501))) + .isInstanceOf(DomainBadRequestException.class) + .hasMessage("error.skillReview.reason.tooLong"); + verify(ratingRepository, never()).save(invalid); + } + @Test void clearReview_throws_when_user_has_only_rating() { when(skillRepository.findById(1L)).thenReturn(Optional.of(skill()));