fix(education): close collection release gate gaps
This commit is contained in:
@@ -66,6 +66,10 @@ public class PracticeSessionServiceImpl implements PracticeSessionService {
|
|||||||
@Override
|
@Override
|
||||||
@Transactional(rollbackFor = Exception.class)
|
@Transactional(rollbackFor = Exception.class)
|
||||||
public PracticeSessionRespVO createPracticeSession(PracticeSessionCreateReqVO reqVO, Long userId, Long tenantId) {
|
public PracticeSessionRespVO createPracticeSession(PracticeSessionCreateReqVO reqVO, Long userId, Long tenantId) {
|
||||||
|
if (reqVO.getCollectionId() != null && !reqVO.getCollectionId().isBlank()
|
||||||
|
&& reqVO.getNodeId() != null && !reqVO.getNodeId().isBlank()) {
|
||||||
|
throw exception(INVALID_PRACTICE_CONFIG, "题集与目录节点不能同时指定");
|
||||||
|
}
|
||||||
// 1. Idempotent check: same tenant + clientSessionId exists → verify ownership before returning
|
// 1. Idempotent check: same tenant + clientSessionId exists → verify ownership before returning
|
||||||
PracticeSessionDO existing = sessionMapper.selectByTenantAndClientSessionId(tenantId, reqVO.getClientSessionId());
|
PracticeSessionDO existing = sessionMapper.selectByTenantAndClientSessionId(tenantId, reqVO.getClientSessionId());
|
||||||
if (existing != null) {
|
if (existing != null) {
|
||||||
|
|||||||
@@ -35,6 +35,10 @@ public class QuestionCatalogServiceImpl implements QuestionCatalogService {
|
|||||||
@Override
|
@Override
|
||||||
public PageResult<SafeQuestionRespVO> pageQuestions(QuestionPageReqVO reqVO) {
|
public PageResult<SafeQuestionRespVO> pageQuestions(QuestionPageReqVO reqVO) {
|
||||||
assertEnabled();
|
assertEnabled();
|
||||||
|
if (reqVO.getCollectionId() != null && !reqVO.getCollectionId().isBlank()
|
||||||
|
&& reqVO.getNodeId() != null && !reqVO.getNodeId().isBlank()) {
|
||||||
|
throw exception(INVALID_PRACTICE_CONFIG, "题集与目录节点不能同时指定");
|
||||||
|
}
|
||||||
|
|
||||||
int pageNo = reqVO.getPageNo() != null ? reqVO.getPageNo() : 1;
|
int pageNo = reqVO.getPageNo() != null ? reqVO.getPageNo() : 1;
|
||||||
int pageSize = reqVO.getPageSize() != null ? reqVO.getPageSize() : 20;
|
int pageSize = reqVO.getPageSize() != null ? reqVO.getPageSize() : 20;
|
||||||
|
|||||||
@@ -16,14 +16,20 @@ BEGIN
|
|||||||
LEFT JOIN education_question_collection_question membership ON membership.collection_id=collection.id AND NOT membership.deleted
|
LEFT JOIN education_question_collection_question membership ON membership.collection_id=collection.id AND NOT membership.deleted
|
||||||
LEFT JOIN education_question question ON question.id=membership.question_id
|
LEFT JOIN education_question question ON question.id=membership.question_id
|
||||||
WHERE collection.scope='TENANT_OWNED' AND collection.is_active AND NOT collection.is_hidden AND NOT collection.deleted
|
WHERE collection.scope='TENANT_OWNED' AND collection.is_active AND NOT collection.is_hidden AND NOT collection.deleted
|
||||||
AND (node.id IS NULL OR node.tenant_id IS DISTINCT FROM collection.tenant_id OR node.scope<>'TENANT_OWNED'
|
AND (collection.collection_type<>'MANUAL'
|
||||||
|
OR node.id IS NULL OR node.tenant_id IS DISTINCT FROM collection.tenant_id OR node.scope<>'TENANT_OWNED'
|
||||||
OR node.deleted OR NOT node.is_active OR node.is_hidden
|
OR node.deleted OR NOT node.is_active OR node.is_hidden
|
||||||
OR collection.entry_id IS DISTINCT FROM node.entry_id
|
OR collection.entry_id IS DISTINCT FROM node.entry_id
|
||||||
OR entry.id IS NULL OR entry.scope NOT IN ('PUBLIC','TENANT_OWNED')
|
OR entry.id IS NULL OR entry.scope NOT IN ('PUBLIC','TENANT_OWNED')
|
||||||
OR (entry.scope='TENANT_OWNED' AND entry.tenant_id IS DISTINCT FROM collection.tenant_id)
|
OR (entry.scope='TENANT_OWNED' AND entry.tenant_id IS DISTINCT FROM collection.tenant_id)
|
||||||
OR entry.deleted OR NOT entry.is_active OR entry.is_hidden
|
OR entry.deleted OR NOT entry.is_active OR entry.is_hidden
|
||||||
OR (membership.id IS NOT NULL AND (question.id IS NULL OR question.tenant_id IS DISTINCT FROM collection.tenant_id
|
OR (membership.id IS NOT NULL AND (membership.sort_order<0
|
||||||
OR question.scope<>'TENANT_OWNED' OR question.deleted OR question.status<>'PUBLISHED' OR NOT question.is_published)))
|
OR question.id IS NULL OR question.tenant_id IS DISTINCT FROM collection.tenant_id
|
||||||
|
OR question.scope<>'TENANT_OWNED' OR question.deleted OR question.status<>'PUBLISHED' OR NOT question.is_published))
|
||||||
|
OR EXISTS (SELECT 1 FROM education_question_collection_question ordered_membership
|
||||||
|
WHERE ordered_membership.collection_id=collection.id AND NOT ordered_membership.deleted
|
||||||
|
GROUP BY ordered_membership.collection_id
|
||||||
|
HAVING count(*)<>count(DISTINCT ordered_membership.sort_order) OR min(ordered_membership.sort_order)<>0 OR max(ordered_membership.sort_order)<>count(*)-1))
|
||||||
ORDER BY collection.id,membership.question_id NULLS FIRST LIMIT 1;
|
ORDER BY collection.id,membership.question_id NULLS FIRST LIMIT 1;
|
||||||
IF bad_collection IS NOT NULL THEN
|
IF bad_collection IS NOT NULL THEN
|
||||||
RAISE EXCEPTION 'invalid historical active collection % member % blocks V4110',bad_collection,COALESCE(bad_question,0) USING ERRCODE='23514';
|
RAISE EXCEPTION 'invalid historical active collection % member % blocks V4110',bad_collection,COALESCE(bad_question,0) USING ERRCODE='23514';
|
||||||
|
|||||||
@@ -168,14 +168,8 @@ class QuestionControllerHttpTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldForwardQueryParamsForPage() throws Exception {
|
void shouldRejectContradictoryQueryParamsForPage() throws Exception {
|
||||||
setLoginUser(100L);
|
setLoginUser(100L);
|
||||||
when(provider.listQuestions("col1", "node1", "choice", "easy", 2, 10))
|
|
||||||
.thenReturn(CatalogQuestionPageResult.builder()
|
|
||||||
.items(Collections.emptyList())
|
|
||||||
.total(0L)
|
|
||||||
.build());
|
|
||||||
|
|
||||||
mockMvc.perform(get("/education/questions/page")
|
mockMvc.perform(get("/education/questions/page")
|
||||||
.param("collectionId", "col1")
|
.param("collectionId", "col1")
|
||||||
.param("nodeId", "node1")
|
.param("nodeId", "node1")
|
||||||
@@ -183,8 +177,9 @@ class QuestionControllerHttpTest {
|
|||||||
.param("difficulty", "easy")
|
.param("difficulty", "easy")
|
||||||
.param("pageNo", "2")
|
.param("pageNo", "2")
|
||||||
.param("pageSize", "10"))
|
.param("pageSize", "10"))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().is(HttpStatus.INTERNAL_SERVER_ERROR.value()))
|
||||||
.andExpect(jsonPath("$.code").value(0));
|
.andExpect(jsonPath("$.code").value(INVALID_PRACTICE_CONFIG.getCode()));
|
||||||
|
verify(provider, never()).listQuestions(any(), any(), any(), any(), anyInt(), anyInt());
|
||||||
}
|
}
|
||||||
|
|
||||||
// ========== Anonymous → 401 ==========
|
// ========== Anonymous → 401 ==========
|
||||||
|
|||||||
@@ -27,7 +27,7 @@ import static cn.iocoder.yudao.framework.test.core.util.AssertUtils.assertServic
|
|||||||
import static cn.iocoder.yudao.module.education.enums.ErrorCodeConstants.*;
|
import static cn.iocoder.yudao.module.education.enums.ErrorCodeConstants.*;
|
||||||
import static org.junit.jupiter.api.Assertions.*;
|
import static org.junit.jupiter.api.Assertions.*;
|
||||||
import static org.mockito.ArgumentMatchers.*;
|
import static org.mockito.ArgumentMatchers.*;
|
||||||
import static org.mockito.Mockito.when;
|
import static org.mockito.Mockito.*;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* PracticeSessionService test — real Mapper + Mock Provider.
|
* PracticeSessionService test — real Mapper + Mock Provider.
|
||||||
@@ -57,6 +57,16 @@ public class PracticeSessionServiceImplTest extends PostgreSqlDbIntegrationTest
|
|||||||
|
|
||||||
// ========== Create: basic ==========
|
// ========== Create: basic ==========
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldRejectCreateWithCollectionAndNodeBeforePersistence() {
|
||||||
|
PracticeSessionCreateReqVO req = createReq("uuid-conflicting-parent", "col-001", 1);
|
||||||
|
req.setNodeId("node-001");
|
||||||
|
ServiceException ex = assertThrows(ServiceException.class, () -> service.createPracticeSession(req, 100L, 1L));
|
||||||
|
assertEquals(INVALID_PRACTICE_CONFIG.getCode(), ex.getCode());
|
||||||
|
verify(provider, never()).listQuestions(any(), any(), any(), any(), anyInt(), anyInt());
|
||||||
|
assertEquals(0L, sessionMapper.selectCount());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldCreateSessionAndReturnQuestions() {
|
void shouldCreateSessionAndReturnQuestions() {
|
||||||
when(provider.isEnabled()).thenReturn(true);
|
when(provider.isEnabled()).thenReturn(true);
|
||||||
@@ -183,16 +193,14 @@ public class PracticeSessionServiceImplTest extends PostgreSqlDbIntegrationTest
|
|||||||
// ========== Create: nodeId forwarding (MEDIUM #3) ==========
|
// ========== Create: nodeId forwarding (MEDIUM #3) ==========
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldForwardNodeIdToProvider() {
|
void shouldRejectNodeIdAlongsideRequiredCollectionId() {
|
||||||
when(provider.isEnabled()).thenReturn(true);
|
when(provider.isEnabled()).thenReturn(true);
|
||||||
when(provider.listQuestions(eq("col-001"), eq("node-042"), isNull(), isNull(), eq(1), eq(1)))
|
|
||||||
.thenReturn(pageResult(List.of(questionDTO("q-001", "v1", "stem"))));
|
|
||||||
|
|
||||||
PracticeSessionCreateReqVO req = createReq("uuid-node", "col-001", 1);
|
PracticeSessionCreateReqVO req = createReq("uuid-node", "col-001", 1);
|
||||||
req.setNodeId("node-042");
|
req.setNodeId("node-042");
|
||||||
|
ServiceException ex = assertThrows(ServiceException.class,
|
||||||
PracticeSessionRespVO resp = service.createPracticeSession(req, 100L, 1L);
|
() -> service.createPracticeSession(req, 100L, 1L));
|
||||||
assertNotNull(resp.getSessionId());
|
assertEquals(INVALID_PRACTICE_CONFIG.getCode(), ex.getCode());
|
||||||
|
verify(provider, never()).listQuestions(any(), any(), any(), any(), anyInt(), anyInt());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -96,6 +96,14 @@ class QuestionCatalogServiceImplTest {
|
|||||||
assertFalse(json.contains("isCorrect"), "JSON must not contain isCorrect");
|
assertFalse(json.contains("isCorrect"), "JSON must not contain isCorrect");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldRejectQuestionPageWithCollectionAndNode() {
|
||||||
|
QuestionPageReqVO req = new QuestionPageReqVO(); req.setCollectionId("col1"); req.setNodeId("node1");
|
||||||
|
ServiceException ex = assertThrows(ServiceException.class, () -> service.pageQuestions(req));
|
||||||
|
assertEquals(INVALID_PRACTICE_CONFIG.getCode(), ex.getCode());
|
||||||
|
verify(provider, never()).listQuestions(any(), any(), any(), any(), anyInt(), anyInt());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldStripAnswerFieldsInPageResponse() {
|
void shouldStripAnswerFieldsInPageResponse() {
|
||||||
CatalogQuestionDTO dto = questionDto("q1", "Q1", "choice", "easy", true,
|
CatalogQuestionDTO dto = questionDto("q1", "Q1", "choice", "easy", true,
|
||||||
|
|||||||
Reference in New Issue
Block a user