Browse Source

fix(backend): 加强绩效评分守卫、优化 OKR 富集查询、修复 SQLite 通知锁

- PerformanceService: 自评与上级终评新增 ensureActiveUser 守卫,
  拒绝已删除/禁用/离职账号继续提交评分(纵深防御)
- OkrService.enrichObjectives: 用户/部门/源KR/源目标名富集从
  全表扫描改为按引用 ID 批量查询,消除 N 次全表扫描
- NotificationEventListener: 改用 @TransactionalEventListener(AFTER_COMMIT)
  并增加短重试,修复 SQLite WAL 模式下事务可见性导致的通知锁问题
- 补充 OkrServiceTest、PerformanceServiceTest、NotificationEventListenerTest
  覆盖以上改动,防止回退
wangkangyjy 3 weeks ago
parent
commit
da4f6e4533

+ 32 - 3
backend/src/main/java/com/yimeng/okr/event/NotificationEventListener.java

@@ -4,19 +4,23 @@ import com.yimeng.okr.entity.Notification;
 import com.yimeng.okr.mapper.NotificationMapper;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import org.springframework.context.event.EventListener;
+import org.springframework.dao.DataAccessException;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.stereotype.Component;
+import org.springframework.transaction.event.TransactionPhase;
+import org.springframework.transaction.event.TransactionalEventListener;
 
 @Slf4j
 @Component
 @RequiredArgsConstructor
 public class NotificationEventListener {
 
+    private static final int MAX_INSERT_ATTEMPTS = 3;
+
     private final NotificationMapper notificationMapper;
 
     @Async
-    @EventListener
+    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = true)
     public void handleNotification(NotificationEvent event) {
         Notification notification = new Notification();
         notification.setUserId(event.getUserId());
@@ -26,6 +30,31 @@ public class NotificationEventListener {
         notification.setRefType(event.getRefType());
         notification.setRefId(event.getRefId());
         notification.setIsRead(0);
-        notificationMapper.insert(notification);
+        insertWithRetry(notification, event);
+    }
+
+    private void insertWithRetry(Notification notification, NotificationEvent event) {
+        for (int attempt = 1; attempt <= MAX_INSERT_ATTEMPTS; attempt++) {
+            try {
+                notificationMapper.insert(notification);
+                return;
+            } catch (DataAccessException ex) {
+                if (attempt == MAX_INSERT_ATTEMPTS) {
+                    log.error("Failed to persist notification after {} attempts, userId={}, type={}, refType={}, refId={}",
+                            MAX_INSERT_ATTEMPTS, event.getUserId(), event.getType(), event.getRefType(), event.getRefId(), ex);
+                    throw ex;
+                }
+                sleepBeforeRetry(attempt);
+            }
+        }
+    }
+
+    private void sleepBeforeRetry(int attempt) {
+        try {
+            Thread.sleep(50L * attempt);
+        } catch (InterruptedException interrupted) {
+            Thread.currentThread().interrupt();
+            throw new IllegalStateException("Notification retry interrupted", interrupted);
+        }
     }
 }

+ 56 - 8
backend/src/main/java/com/yimeng/okr/service/OkrService.java

@@ -581,10 +581,6 @@ public class OkrService {
                                     .orderByDesc(KrProgressLog::getCreatedAt))
                     .stream().collect(Collectors.groupingBy(KrProgressLog::getKrId));
         }
-        Map<Long, String> userNames = userMapper.selectList(null).stream()
-                .collect(Collectors.toMap(SysUser::getId, SysUser::getRealName, (a, b) -> a));
-        Map<Long, String> deptNames = departmentMapper.selectList(null).stream()
-                .collect(Collectors.toMap(SysDepartment::getId, SysDepartment::getName, (a, b) -> a));
 
         // Collect all aligned objective IDs
         Set<Long> allAlignedIds = new LinkedHashSet<>();
@@ -595,9 +591,55 @@ public class OkrService {
                         .map(Long::parseLong).forEach(allAlignedIds::add);
             }
         }
-        Map<Long, OkrDetailDto> alignedObjMap = new LinkedHashMap<>();
+        List<OkrObjective> alignedObjs = List.of();
         if (!allAlignedIds.isEmpty()) {
-            List<OkrObjective> alignedObjs = objectiveMapper.selectBatchIds(allAlignedIds);
+            alignedObjs = objectiveMapper.selectBatchIds(allAlignedIds);
+        }
+
+        Set<Long> sourceKrIds = krMap.values().stream()
+                .flatMap(List::stream)
+                .map(KrKeyResult::getSourceKrId)
+                .filter(Objects::nonNull)
+                .collect(Collectors.toCollection(LinkedHashSet::new));
+        Map<Long, KrKeyResult> sourceKrMap = new LinkedHashMap<>();
+        if (!sourceKrIds.isEmpty()) {
+            sourceKrMap = krMapper.selectBatchIds(sourceKrIds).stream()
+                    .collect(Collectors.toMap(KrKeyResult::getId, kr -> kr, (a, b) -> a, LinkedHashMap::new));
+        }
+
+        Set<Long> sourceObjectiveIds = sourceKrMap.values().stream()
+                .map(KrKeyResult::getObjectiveId)
+                .filter(Objects::nonNull)
+                .collect(Collectors.toCollection(LinkedHashSet::new));
+        Map<Long, OkrObjective> sourceObjectiveMap = new LinkedHashMap<>();
+        if (!sourceObjectiveIds.isEmpty()) {
+            sourceObjectiveMap = objectiveMapper.selectBatchIds(sourceObjectiveIds).stream()
+                    .collect(Collectors.toMap(OkrObjective::getId, obj -> obj, (a, b) -> a, LinkedHashMap::new));
+        }
+
+        Set<Long> userIds = new LinkedHashSet<>();
+        Set<Long> deptIds = new LinkedHashSet<>();
+        objectives.forEach(obj -> {
+            addIfPresent(userIds, obj.getUserId());
+            addIfPresent(userIds, obj.getReviewedBy());
+            addIfPresent(deptIds, obj.getDeptId());
+        });
+        alignedObjs.forEach(obj -> addIfPresent(userIds, obj.getUserId()));
+        krMap.values().stream().flatMap(List::stream)
+                .forEach(kr -> addIfPresent(userIds, kr.getAssignedTo()));
+        sourceObjectiveMap.values().forEach(obj -> addIfPresent(userIds, obj.getUserId()));
+
+        Map<Long, String> userNames = userIds.isEmpty()
+                ? Collections.emptyMap()
+                : userMapper.selectBatchIds(userIds).stream()
+                .collect(Collectors.toMap(SysUser::getId, SysUser::getRealName, (a, b) -> a));
+        Map<Long, String> deptNames = deptIds.isEmpty()
+                ? Collections.emptyMap()
+                : departmentMapper.selectBatchIds(deptIds).stream()
+                .collect(Collectors.toMap(SysDepartment::getId, SysDepartment::getName, (a, b) -> a));
+
+        Map<Long, OkrDetailDto> alignedObjMap = new LinkedHashMap<>();
+        if (!alignedObjs.isEmpty()) {
             for (OkrObjective ao : alignedObjs) {
                 List<KrKeyResult> akrs = krMap.getOrDefault(ao.getId(),
                         krMapper.selectList(new LambdaQueryWrapper<KrKeyResult>().eq(KrKeyResult::getObjectiveId, ao.getId())));
@@ -641,10 +683,10 @@ public class OkrService {
                     k.setAssignedToName(userNames.get(kr.getAssignedTo()));
                 }
                 if (kr.getSourceKrId() != null) {
-                    KrKeyResult src = krMapper.selectById(kr.getSourceKrId());
+                    KrKeyResult src = sourceKrMap.get(kr.getSourceKrId());
                     if (src != null) {
                         k.setSourceKrTitle(src.getTitle());
-                        OkrObjective srcObj = objectiveMapper.selectById(src.getObjectiveId());
+                        OkrObjective srcObj = sourceObjectiveMap.get(src.getObjectiveId());
                         if (srcObj != null && srcObj.getUserId() != null) {
                             k.setSourceKrUserName(userNames.get(srcObj.getUserId()));
                         }
@@ -669,4 +711,10 @@ public class OkrService {
         }
         return result;
     }
+
+    private void addIfPresent(Set<Long> target, Long value) {
+        if (value != null) {
+            target.add(value);
+        }
+    }
 }

+ 26 - 0
backend/src/main/java/com/yimeng/okr/service/PerformanceService.java

@@ -20,6 +20,8 @@ import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
 import java.util.List;
+import java.util.Locale;
+import java.util.Set;
 
 @Slf4j
 @Service
@@ -48,6 +50,8 @@ public class PerformanceService {
                                             Double bonus, Double penalty, String commentsJson) {
         AssessmentPeriod period = checkPeriodStatus(periodId);
         validateScoreParts(okrScore, kpiScore, bonus, penalty);
+        SysUser actor = userMapper.selectById(userId);
+        ensureActiveUser(actor, "自评人");
 
         PerformanceScore existing = findExisting(periodId, userId, ScoreType.SELF);
         if (existing != null) {
@@ -83,6 +87,9 @@ public class PerformanceService {
         // Permission: evaluator must be the direct superior
         SysUser targetUser = userMapper.selectById(userId);
         if (targetUser == null) throw new BusinessException(404, "用户不存在");
+        ensureActiveUser(targetUser, "被评分人");
+        SysUser evaluator = userMapper.selectById(evaluatorId);
+        ensureActiveUser(evaluator, "评分人");
         if (!evaluatorId.equals(targetUser.getSuperiorId())) {
             throw new BusinessException("你无权对此用户评分");
         }
@@ -361,6 +368,25 @@ public class PerformanceService {
         return STATUS_PUBLISHED.equals(score.getStatus());
     }
 
+    private void ensureActiveUser(SysUser user, String label) {
+        if (user == null) {
+            throw new BusinessException(404, label + "不存在");
+        }
+        if (Integer.valueOf(1).equals(user.getDeleted())) {
+            throw new BusinessException(label + "已删除,不能继续进行绩效操作");
+        }
+        String status = user.getStatus();
+        if (status != null) {
+            String normalized = status.trim().toUpperCase(Locale.ROOT);
+            if (Set.of("DISABLED", "LEFT", "INACTIVE").contains(normalized)) {
+                throw new BusinessException(label + "当前状态不可用,不能继续进行绩效操作");
+            }
+        }
+        if (user.getDisabledAt() != null || user.getLeftAt() != null) {
+            throw new BusinessException(label + "当前状态不可用,不能继续进行绩效操作");
+        }
+    }
+
     private AssessmentPeriod checkPeriodStatus(Long periodId) {
         AssessmentPeriod period = periodMapper.selectById(periodId);
         if (period == null) throw new BusinessException(404, "考核周期不存在");

+ 28 - 0
backend/src/test/java/com/yimeng/okr/event/NotificationEventListenerTest.java

@@ -0,0 +1,28 @@
+package com.yimeng.okr.event;
+
+import com.yimeng.okr.entity.Notification;
+import com.yimeng.okr.mapper.NotificationMapper;
+import org.junit.jupiter.api.Test;
+import org.springframework.dao.TransientDataAccessResourceException;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
+
+class NotificationEventListenerTest {
+
+    @Test
+    void retriesTransientSqliteWriteLock() {
+        NotificationMapper mapper = mock(NotificationMapper.class);
+        when(mapper.insert(any(Notification.class)))
+                .thenThrow(new TransientDataAccessResourceException("SQLITE_LOCKED_SHAREDCACHE"))
+                .thenReturn(1);
+
+        NotificationEventListener listener = new NotificationEventListener(mapper);
+
+        listener.handleNotification(new NotificationEvent(
+                this, 7L, "SCORE_CONFIRM", "绩效结果已发布",
+                "请查看并确认", "score", 12L));
+
+        verify(mapper, times(2)).insert(any(Notification.class));
+    }
+}

+ 55 - 0
backend/src/test/java/com/yimeng/okr/service/OkrServiceTest.java

@@ -3,6 +3,8 @@ package com.yimeng.okr.service;
 import com.yimeng.okr.entity.AssessmentPeriod;
 import com.yimeng.okr.entity.KrKeyResult;
 import com.yimeng.okr.entity.OkrObjective;
+import com.yimeng.okr.entity.SysDepartment;
+import com.yimeng.okr.entity.SysUser;
 import com.yimeng.okr.enums.KrStatus;
 import com.yimeng.okr.enums.PeriodStatus;
 import com.yimeng.okr.exception.BusinessException;
@@ -14,7 +16,13 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.context.ApplicationEventPublisher;
 
+import java.util.Collection;
+import java.util.List;
+
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.isNull;
 import static org.mockito.Mockito.*;
 
 @ExtendWith(MockitoExtension.class)
@@ -53,4 +61,51 @@ class OkrServiceTest {
         assertEquals(403, ex.getCode());
         verify(progressLogMapper, never()).insert(any());
     }
+
+    @Test
+    void enrichObjectivesLoadsOnlyReferencedUsersAndDepartments() {
+        OkrObjective objective = new OkrObjective();
+        objective.setId(6L);
+        objective.setUserId(2L);
+        objective.setDeptId(10L);
+        objective.setReviewedBy(3L);
+
+        KrKeyResult kr = new KrKeyResult();
+        kr.setId(5L);
+        kr.setObjectiveId(6L);
+        kr.setAssignedTo(4L);
+        kr.setStatus(KrStatus.IN_PROGRESS);
+        kr.setWeight(100);
+
+        when(krMapper.selectList(any())).thenReturn(List.of(kr));
+        when(userMapper.selectBatchIds(argThat(ids -> containsAll(ids, 2L, 3L, 4L))))
+                .thenReturn(List.of(user(2L, "员工"), user(3L, "上级"), user(4L, "协作人")));
+        when(departmentMapper.selectBatchIds(argThat(ids -> containsAll(ids, 10L))))
+                .thenReturn(List.of(department(10L, "研发部")));
+
+        assertEquals(1, okrService.enrichObjectives(List.of(objective)).size());
+
+        verify(userMapper).selectBatchIds(argThat(ids -> containsAll(ids, 2L, 3L, 4L)));
+        verify(departmentMapper).selectBatchIds(argThat(ids -> containsAll(ids, 10L)));
+        verify(userMapper, never()).selectList(isNull());
+        verify(departmentMapper, never()).selectList(isNull());
+    }
+
+    private boolean containsAll(Collection<?> ids, Long... expected) {
+        return ids != null && ids.containsAll(List.of(expected));
+    }
+
+    private SysUser user(Long id, String name) {
+        SysUser user = new SysUser();
+        user.setId(id);
+        user.setRealName(name);
+        return user;
+    }
+
+    private SysDepartment department(Long id, String name) {
+        SysDepartment department = new SysDepartment();
+        department.setId(id);
+        department.setName(name);
+        return department;
+    }
 }

+ 34 - 0
backend/src/test/java/com/yimeng/okr/service/PerformanceServiceTest.java

@@ -132,6 +132,7 @@ class PerformanceServiceTest {
     void submitSuperiorScoreRejectsWhenSelfScoreMissing() {
         when(periodMapper.selectById(1L)).thenReturn(period());
         when(userMapper.selectById(2L)).thenReturn(user(2L, 3L));
+        when(userMapper.selectById(3L)).thenReturn(user(3L, null));
         when(scoreMapper.selectOne(any())).thenReturn(null);
 
         BusinessException ex = assertThrows(BusinessException.class,
@@ -141,10 +142,41 @@ class PerformanceServiceTest {
         verify(scoreMapper, never()).insert(any());
     }
 
+    @Test
+    void submitSuperiorScoreRejectsDisabledEvaluator() {
+        SysUser target = user(2L, 3L);
+        SysUser evaluator = user(3L, null);
+        evaluator.setStatus("DISABLED");
+        when(periodMapper.selectById(1L)).thenReturn(period());
+        when(userMapper.selectById(2L)).thenReturn(target);
+        when(userMapper.selectById(3L)).thenReturn(evaluator);
+
+        BusinessException ex = assertThrows(BusinessException.class,
+                () -> performanceService.submitSuperiorScore(1L, 2L, 3L, 30.0, 50.0, 0.0, 0.0, "{}"));
+
+        assertTrue(ex.getMessage().contains("评分人"));
+        verify(scoreMapper, never()).insert(any());
+    }
+
+    @Test
+    void submitSuperiorScoreRejectsDeletedTargetUser() {
+        SysUser target = user(2L, 3L);
+        target.setDeleted(1);
+        when(periodMapper.selectById(1L)).thenReturn(period());
+        when(userMapper.selectById(2L)).thenReturn(target);
+
+        BusinessException ex = assertThrows(BusinessException.class,
+                () -> performanceService.submitSuperiorScore(1L, 2L, 3L, 30.0, 50.0, 0.0, 0.0, "{}"));
+
+        assertTrue(ex.getMessage().contains("被评分人"));
+        verify(scoreMapper, never()).insert(any());
+    }
+
     @Test
     void submitSuperiorScoreCreatesUnpublishedFinalScoreWithoutNotifyingEmployee() {
         when(periodMapper.selectById(1L)).thenReturn(period());
         when(userMapper.selectById(2L)).thenReturn(user(2L, 3L));
+        when(userMapper.selectById(3L)).thenReturn(user(3L, null));
         PerformanceScore selfScore = score(ScoreType.SELF, "SELF_SUBMITTED");
         when(scoreMapper.selectOne(any())).thenReturn(selfScore, null);
 
@@ -198,6 +230,8 @@ class PerformanceServiceTest {
         SysUser user = new SysUser();
         user.setId(id);
         user.setSuperiorId(superiorId);
+        user.setStatus("ACTIVE");
+        user.setDeleted(0);
         return user;
     }