From e66db3e432d2da0cf964f68f2a8108ba9d0b1f53 Mon Sep 17 00:00:00 2001 From: let5sne Date: Sun, 12 Jul 2026 11:48:32 +0800 Subject: [PATCH] fix(personal): make delete requests idempotent --- .../service/PersonalCleanupService.java | 15 ++++++- .../personal/PersonalCleanupServiceTest.java | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/backend/ruoyi-modules/ruoyi-aihr/src/main/java/org/dromara/aihr/personal/service/PersonalCleanupService.java b/backend/ruoyi-modules/ruoyi-aihr/src/main/java/org/dromara/aihr/personal/service/PersonalCleanupService.java index 1ed797f2..ff3f1d4d 100644 --- a/backend/ruoyi-modules/ruoyi-aihr/src/main/java/org/dromara/aihr/personal/service/PersonalCleanupService.java +++ b/backend/ruoyi-modules/ruoyi-aihr/src/main/java/org/dromara/aihr/personal/service/PersonalCleanupService.java @@ -73,16 +73,27 @@ public class PersonalCleanupService { and o.create_by = i.owner_user_id and cast(json_unquote(json_extract(o.ext1, '$.itemId')) as unsigned) = i.id where binary i.tenant_id = binary ? and i.owner_user_id = ? and i.id = ? - and i.status in ('QUEUED','PARSING','READY','FAILED') + and i.status in ('QUEUED','PARSING','READY','FAILED','DELETING') for update """, owner.tenantId(), owner.userId(), itemId); } catch (EmptyResultDataAccessException ex) { throw new ServiceException(ITEM_NOT_FOUND); } + String status = String.valueOf(item.get("status")); + if ("DELETING".equals(status)) { + List> jobs = jdbc.queryForList(""" + select id from aihr_personal_cleanup_job + where tenant_id = ? and owner_user_id = ? and item_id = ? + and status in ('PENDING','RETRY') + order by id limit 1 + for update + """, owner.tenantId(), owner.userId(), itemId); + if (jobs.size() != 1) throw new ServiceException("PERSONAL_CLEANUP_STATE_INVALID"); + return number(jobs.get(0), "id"); + } if (item.get("oss_id") != null && item.get("owned_oss_id") == null) { throw new ServiceException(ITEM_NOT_FOUND); } - String status = String.valueOf(item.get("status")); int hidden = jdbc.update(""" update aihr_personal_item set status = 'DELETING', update_time = now() diff --git a/backend/ruoyi-modules/ruoyi-aihr/src/test/java/org/dromara/aihr/personal/PersonalCleanupServiceTest.java b/backend/ruoyi-modules/ruoyi-aihr/src/test/java/org/dromara/aihr/personal/PersonalCleanupServiceTest.java index b3647e33..44df0bd1 100644 --- a/backend/ruoyi-modules/ruoyi-aihr/src/test/java/org/dromara/aihr/personal/PersonalCleanupServiceTest.java +++ b/backend/ruoyi-modules/ruoyi-aihr/src/test/java/org/dromara/aihr/personal/PersonalCleanupServiceTest.java @@ -23,8 +23,10 @@ import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.times; @Tag("dev") class PersonalCleanupServiceTest { @@ -71,6 +73,47 @@ class PersonalCleanupServiceTest { verify(jdbc, never()).update(contains("insert into aihr_personal_cleanup_job"), any(), any(), any(), any()); } + @Test + void repeatedDeleteReturnsExistingOwnerJobWithoutDuplicateInsertOrCleanup() { + JdbcTemplate jdbc = mock(JdbcTemplate.class); + PersonalVectorStore vectors = mock(PersonalVectorStore.class); + PersonalCleanupService.OssCleanup oss = mock(PersonalCleanupService.OssCleanup.class); + when(jdbc.queryForMap(contains("for update"), eq("000000"), eq(101L), eq(9L))) + .thenReturn(item("READY"), item("DELETING")); + when(jdbc.update(contains("set status = 'DELETING'"), eq("000000"), eq(101L), eq(9L), eq("READY"))) + .thenReturn(1); + when(jdbc.update(contains("insert into aihr_personal_cleanup_job"), any(), eq("000000"), eq(101L), eq(9L))) + .thenReturn(1); + when(jdbc.queryForList(contains("from aihr_personal_cleanup_job"), eq("000000"), eq(101L), eq(9L))) + .thenReturn(List.of(Map.of("id", 7001L))); + PersonalCleanupService service = PersonalCleanupService.forTest(jdbc, vectors, oss, () -> 7001L, + action -> action.get()); + + assertEquals(7001L, service.requestDelete(OWNER, 9L)); + assertEquals(7001L, service.requestDelete(OWNER, 9L)); + + verify(jdbc, times(1)).update(contains("insert into aihr_personal_cleanup_job"), eq(7001L), + eq("000000"), eq(101L), eq(9L)); + verify(jdbc).queryForList(contains("from aihr_personal_cleanup_job"), eq("000000"), eq(101L), eq(9L)); + verifyNoInteractions(vectors, oss); + } + + @Test + void deletingItemWithoutOwnerJobFailsWithStableRecoveryError() { + JdbcTemplate jdbc = mock(JdbcTemplate.class); + when(jdbc.queryForMap(contains("for update"), eq("000000"), eq(101L), eq(9L))) + .thenReturn(item("DELETING")); + when(jdbc.queryForList(contains("from aihr_personal_cleanup_job"), eq("000000"), eq(101L), eq(9L))) + .thenReturn(List.of()); + PersonalCleanupService service = PersonalCleanupService.forTest(jdbc, mock(PersonalVectorStore.class), + mock(PersonalCleanupService.OssCleanup.class), () -> 7002L, action -> action.get()); + + ServiceException error = assertThrows(ServiceException.class, () -> service.requestDelete(OWNER, 9L)); + + assertEquals("PERSONAL_CLEANUP_STATE_INVALID", error.getMessage()); + verify(jdbc, never()).update(contains("insert into aihr_personal_cleanup_job"), any(), any(), any(), any()); + } + @Test void cleanupUsesFixedOrderAndIsIdempotent() { JdbcTemplate jdbc = mock(JdbcTemplate.class);