Browse Source

fix: enforce onboarding item limit under repeatable read

backup/master-local-20261012
pwklppg 1 month ago
parent
commit
97b3b16261
  1. 13
      mydreamjob-server/src/main/java/com/mydreamjob/onboarding/OnboardingChecklistItemRepository.java
  2. 5
      mydreamjob-server/src/main/java/com/mydreamjob/onboarding/OnboardingChecklistService.java
  3. 8
      mydreamjob-server/src/test/java/com/mydreamjob/onboarding/OnboardingCompanyChecklistServiceTest.java
  4. 43
      mydreamjob-server/src/test/java/com/mydreamjob/onboarding/OnboardingConcurrencyIntegrationTest.java

13
mydreamjob-server/src/main/java/com/mydreamjob/onboarding/OnboardingChecklistItemRepository.java

@ -1,6 +1,8 @@
package com.mydreamjob.onboarding; package com.mydreamjob.onboarding;
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query; import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param; import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository; import org.springframework.stereotype.Repository;
@ -23,4 +25,15 @@ public interface OnboardingChecklistItemRepository extends JpaRepository<Onboard
ORDER BY item.sortOrder ASC, item.id ASC ORDER BY item.sortOrder ASC, item.id ASC
""") """)
List<OnboardingChecklistItem> findEffectiveChecklistItems(@Param("companyId") Long companyId); List<OnboardingChecklistItem> findEffectiveChecklistItems(@Param("companyId") Long companyId);
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("""
SELECT item
FROM OnboardingChecklistItem item
WHERE item.scopeType = 'COMPANY'
AND item.scopeId = :companyId
AND item.companyId = :companyId
ORDER BY item.id ASC
""")
List<OnboardingChecklistItem> findCompanyItemsForUpdate(@Param("companyId") Long companyId);
} }

5
mydreamjob-server/src/main/java/com/mydreamjob/onboarding/OnboardingChecklistService.java

@ -21,6 +21,7 @@ import lombok.RequiredArgsConstructor;
import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.orm.ObjectOptimisticLockingFailureException;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Isolation;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
@ -146,7 +147,7 @@ public class OnboardingChecklistService {
.toList(); .toList();
} }
@Transactional @Transactional(isolation = Isolation.READ_COMMITTED)
public CompanyChecklistItemView createCompanyChecklistItem( public CompanyChecklistItemView createCompanyChecklistItem(
Long userId, Long userId,
CompanyChecklistItemCreateRequest request) { CompanyChecklistItemCreateRequest request) {
@ -165,7 +166,7 @@ public class OnboardingChecklistService {
title, description, category, sortOrder, required); title, description, category, sortOrder, required);
List<OnboardingChecklistItem> companyItems = List<OnboardingChecklistItem> companyItems =
ownedCompanyItems(checklistItemRepository.findAll(), company.getId()); checklistItemRepository.findCompanyItemsForUpdate(company.getId());
if (idempotencyKey != null) { if (idempotencyKey != null) {
OnboardingChecklistItem existing = companyItems.stream() OnboardingChecklistItem existing = companyItems.stream()
.filter(item -> Objects.equals(item.getIdempotencyKey(), idempotencyKey)) .filter(item -> Objects.equals(item.getIdempotencyKey(), idempotencyKey))

8
mydreamjob-server/src/test/java/com/mydreamjob/onboarding/OnboardingCompanyChecklistServiceTest.java

@ -90,7 +90,7 @@ class OnboardingCompanyChecklistServiceTest {
@Test @Test
void createCompanyChecklistItem_shouldPersistNormalizedItemAndStableHash() { void createCompanyChecklistItem_shouldPersistNormalizedItemAndStableHash() {
stubCompanyOwner(); stubCompanyOwner();
when(checklistItemRepository.findAll()).thenReturn(List.of()); when(checklistItemRepository.findCompanyItemsForUpdate(COMPANY_ID)).thenReturn(List.of());
ArgumentCaptor<OnboardingChecklistItem> captor = ArgumentCaptor<OnboardingChecklistItem> captor =
ArgumentCaptor.forClass(OnboardingChecklistItem.class); ArgumentCaptor.forClass(OnboardingChecklistItem.class);
when(checklistItemRepository.saveAndFlush(any(OnboardingChecklistItem.class))) when(checklistItemRepository.saveAndFlush(any(OnboardingChecklistItem.class)))
@ -125,7 +125,7 @@ class OnboardingCompanyChecklistServiceTest {
void createCompanyChecklistItem_idempotentReplay_shouldReturnOriginalWithoutSecondSave() { void createCompanyChecklistItem_idempotentReplay_shouldReturnOriginalWithoutSecondSave() {
stubCompanyOwner(); stubCompanyOwner();
AtomicReference<OnboardingChecklistItem> stored = new AtomicReference<>(); AtomicReference<OnboardingChecklistItem> stored = new AtomicReference<>();
when(checklistItemRepository.findAll()).thenAnswer(invocation -> stored.get() == null when(checklistItemRepository.findCompanyItemsForUpdate(COMPANY_ID)).thenAnswer(invocation -> stored.get() == null
? List.of() : List.of(stored.get())); ? List.of() : List.of(stored.get()));
when(checklistItemRepository.saveAndFlush(any(OnboardingChecklistItem.class))) when(checklistItemRepository.saveAndFlush(any(OnboardingChecklistItem.class)))
.thenAnswer(invocation -> { .thenAnswer(invocation -> {
@ -154,7 +154,7 @@ class OnboardingCompanyChecklistServiceTest {
OnboardingChecklistItem existing = item( OnboardingChecklistItem existing = item(
31L, COMPANY_ID, "existing", "ADMIN", 200, true, 0L, "same-key"); 31L, COMPANY_ID, "existing", "ADMIN", 200, true, 0L, "same-key");
existing.setRequestHash("a".repeat(64)); existing.setRequestHash("a".repeat(64));
when(checklistItemRepository.findAll()).thenReturn(List.of(existing)); when(checklistItemRepository.findCompanyItemsForUpdate(COMPANY_ID)).thenReturn(List.of(existing));
assertThatThrownBy(() -> service.createCompanyChecklistItem( assertThatThrownBy(() -> service.createCompanyChecklistItem(
COMPANY_USER_ID, COMPANY_USER_ID,
@ -173,7 +173,7 @@ class OnboardingCompanyChecklistServiceTest {
existing.add(item((long) index + 1, COMPANY_ID, "item-" + index, existing.add(item((long) index + 1, COMPANY_ID, "item-" + index,
"OTHER", index, true, 0L, null)); "OTHER", index, true, 0L, null));
} }
when(checklistItemRepository.findAll()).thenReturn(existing); when(checklistItemRepository.findCompanyItemsForUpdate(COMPANY_ID)).thenReturn(existing);
assertThatThrownBy(() -> service.createCompanyChecklistItem( assertThatThrownBy(() -> service.createCompanyChecklistItem(
COMPANY_USER_ID, COMPANY_USER_ID,

43
mydreamjob-server/src/test/java/com/mydreamjob/onboarding/OnboardingConcurrencyIntegrationTest.java

@ -13,6 +13,7 @@ import com.mydreamjob.onboarding.dto.CompanyKnowledgeView;
import com.mydreamjob.onboarding.dto.ProgressUpdateRequest; import com.mydreamjob.onboarding.dto.ProgressUpdateRequest;
import com.mydreamjob.onboarding.dto.ProgressUpdateResponse; import com.mydreamjob.onboarding.dto.ProgressUpdateResponse;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
@ -97,23 +98,6 @@ class OnboardingConcurrencyIntegrationTest {
.hasSize(1); .hasSize(1);
} }
@Test
void concurrentCreatesAtLimitBoundaryNeverWriteFiftyFirstItem() throws Exception {
CompanyOwner owner = createCompanyOwner("上限企业");
seedCompanyItems(owner.companyId(), 49);
List<Attempt<CompanyChecklistItemView>> attempts = runConcurrently(
() -> checklistService.createCompanyChecklistItem(
owner.userId(), createRequest("边界A", "limit-a")),
() -> checklistService.createCompanyChecklistItem(
owner.userId(), createRequest("边界B", "limit-b")));
assertThat(attempts).filteredOn(attempt -> attempt.error() == null).hasSize(1);
assertThat(attempts).filteredOn(Attempt::failedWithItemLimit).hasSize(1);
assertThat(companyRows(owner.companyId()))
.filteredOn(item -> Boolean.TRUE.equals(item.getActive()))
.hasSize(50);
}
@Test @Test
void inactiveItemsDoNotConsumeLimitAndExistingKeyStillReplays() throws Exception { void inactiveItemsDoNotConsumeLimitAndExistingKeyStillReplays() throws Exception {
@ -241,6 +225,31 @@ class OnboardingConcurrencyIntegrationTest {
.isEqualTo(1L); .isEqualTo(1L);
} }
@Nested
@TestPropertySource(properties =
"spring.datasource.hikari.transaction-isolation=TRANSACTION_REPEATABLE_READ")
class RepeatableReadLimitBoundary {
@Test
void concurrentCreatesAtLimitBoundaryNeverWriteFiftyFirstItem() throws Exception {
CompanyOwner owner = createCompanyOwner("上限企业");
seedCompanyItems(owner.companyId(), 49);
List<Attempt<CompanyChecklistItemView>> attempts = runConcurrently(
() -> checklistService.createCompanyChecklistItem(
owner.userId(), createRequest("边界A", "limit-a")),
() -> checklistService.createCompanyChecklistItem(
owner.userId(), createRequest("边界B", "limit-b")));
assertThat(attempts).filteredOn(attempt -> attempt.error() == null).hasSize(1);
assertThat(attempts).filteredOn(Attempt::failedWithItemLimit).hasSize(1);
assertThat(companyRows(owner.companyId()))
.filteredOn(item -> Boolean.TRUE.equals(item.getActive()))
.hasSize(50);
}
}
private int failedWith(ThrowingSupplier<?> supplier) { private int failedWith(ThrowingSupplier<?> supplier) {
try { try {
supplier.get(); supplier.get();

Loading…
Cancel
Save