mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-15 04:18:20 +00:00
test(review): strengthen failure and concurrency coverage
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS
parent
f98f4b1d78
commit
e7771cf208
5 changed files with 310 additions and 1 deletions
|
|
@ -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<String> roles) {
|
||||
return new PlatformPrincipal(userId, userId, userId + "@example.test", null, "local", roles);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<String> 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"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -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<? extends Throwable> expectedType) {
|
||||
Throwable current = error;
|
||||
while (current != null) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue