From 64c9370aa0c009635e26066676869f960843dba1 Mon Sep 17 00:00:00 2001 From: sechmachine <97589681+sechmachine727@users.noreply.github.com> Date: Sat, 15 Aug 2026 15:01:48 +0700 Subject: [PATCH] feat(projects): add Intern members atomically --- .../integration/project-batch-member-add.md | 43 +++++++++++++++++ .../project/service/ProjectService.java | 41 +++++++++++++++- .../ProjectServiceIntegrationTest.java | 47 +++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 docs/tests/integration/project-batch-member-add.md diff --git a/docs/tests/integration/project-batch-member-add.md b/docs/tests/integration/project-batch-member-add.md new file mode 100644 index 0000000..610019b --- /dev/null +++ b/docs/tests/integration/project-batch-member-add.md @@ -0,0 +1,43 @@ +# Integration Test Evidence + +## Requirement and scenario IDs + +- AUTH-001, AUTH-002, AUTH-011; PRJ-003, PRJ-004, PRJ-017; ERR-001, ERR-003; TST-001 through TST-010. +- AC-AUTH-001, AC-AUTH-010, AC-PRJ-001, AC-TST-001. + +## Behavior under test + +The owning Mentor adds several eligible nonmembers under one Project lock and transaction. Null, empty, duplicate, current-member, invalid, or stale/noneligible selections reject the whole batch; no valid prefix becomes a membership. + +## Expected result derivation + +The fixture begins with one Leader. A successful two-Intern batch must yield three current memberships. Every rejected batch leaves the eligible and stale candidate membership count at zero. + +## RED + +`env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH ./mvnw '-Dtest=ProjectControllerTest,ProjectServiceIntegrationTest' test` failed during test compilation with eight `cannot find symbol` errors for the requested `ProjectService.addMembers(long,long,List)` API. Production compiled first; the failure was the missing behavior boundary rather than the environment or fixture. + +## GREEN + +The focused PostgreSQL command was: + +`env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH DOCKER_HOST=unix:///Users/sechmachine/.orbstack/run/docker.sock ./mvnw '-Dtest=ProjectServiceIntegrationTest#ownerAddsSeveralEligibleMembersInOneLockedTransaction+memberBatchRejectsMissingDuplicateCurrentAndStaleSelectionsWithoutPartialMutation' test` + +Result: 2 tests, 0 failures, 0 errors, 0 skipped against PostgreSQL 18.4. The +successful case added two memberships; the rejection case covered null, empty, duplicate, +invalid, current-member, and one-valid-plus-one-stale selections without partial persistence. + +## Affected suite + +`env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH DOCKER_HOST=unix:///Users/sechmachine/.orbstack/run/docker.sock ./mvnw '-Dtest=ProjectServiceIntegrationTest' test` +passed 9/9 tests with no failures, errors, or skips. + +The complete Project plus layer-architecture command was: + +`env JAVA_HOME=/opt/homebrew/opt/openjdk@25 PATH=/opt/homebrew/opt/openjdk@25/bin:$PATH DOCKER_HOST=unix:///Users/sechmachine/.orbstack/run/docker.sock ./mvnw '-Dtest=ProjectControllerTest,ProjectEntityTest,ProjectPersistenceStructureTest,ProjectServiceIntegrationTest,ProjectTaskMutationContextTest,LayerStructureTest' test` + +Result: 38 tests, 0 failures, 0 errors, 0 skipped. + +## External boundaries + +PostgreSQL 18.4 Testcontainers provides the real schema, constraints, JPA transaction, and Project pessimistic lock path. The test does not exercise concurrent requests; existing Project locking coverage remains unchanged. These pre-merge results will be rerun after merging the taskmaster-specified exact `main` SHA. diff --git a/src/main/java/com/lab/labtimesheet/feature/project/service/ProjectService.java b/src/main/java/com/lab/labtimesheet/feature/project/service/ProjectService.java index c20489c..c370cfe 100644 --- a/src/main/java/com/lab/labtimesheet/feature/project/service/ProjectService.java +++ b/src/main/java/com/lab/labtimesheet/feature/project/service/ProjectService.java @@ -1,6 +1,7 @@ package com.lab.labtimesheet.feature.project.service; import com.lab.labtimesheet.feature.project.exception.ProjectAccessDeniedException; +import com.lab.labtimesheet.feature.project.exception.ProjectRuleViolationException; import com.lab.labtimesheet.feature.account.service.AccountService; import com.lab.labtimesheet.feature.project.model.ProjectInternEligibility; import com.lab.labtimesheet.feature.project.model.dto.ProjectCreateCommand; @@ -9,6 +10,9 @@ import com.lab.labtimesheet.feature.project.model.entity.ProjectEntity; import com.lab.labtimesheet.feature.project.repository.ProjectRepository; import com.lab.labtimesheet.feature.task.service.TaskQueryService; import java.time.Clock; +import java.time.LocalDate; +import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; @@ -68,9 +72,42 @@ public class ProjectService { */ @Transactional public void addMember(long actorUserId, long projectId, long internUserId) { + addMembers(actorUserId, projectId, List.of(internUserId)); + } + + /** + * Adds a complete selection of eligible nonmembers while holding one Project write lock. + * Every identifier is revalidated after owner authorization and before the aggregate changes, + * so missing, duplicate, stale, ineligible, or current-member selections leave membership + * unchanged. + * + * @param actorUserId authenticated owning Mentor + * @param projectId Project to update + * @param internUserIds distinct eligible Intern account identifiers + * @throws com.lab.labtimesheet.feature.project.exception.ProjectRuleViolationException when + * the selection is null, empty, malformed, duplicate, stale, ineligible, or already + * contains a current member + */ + @Transactional + public void addMembers(long actorUserId, long projectId, List internUserIds) { var project = lockedProject(projectId); project.authorizeOwner(actorUserId); - project.addMember(actorUserId, eligibleIntern(internUserId), clock.instant()); + if (internUserIds == null || internUserIds.isEmpty()) { + throw new ProjectRuleViolationException("Select at least one Intern"); + } + if (internUserIds.stream().anyMatch(userId -> userId == null || userId <= 0) + || new HashSet<>(internUserIds).size() != internUserIds.size()) { + throw new ProjectRuleViolationException("Intern selection is invalid"); + } + + var selectedInterns = internUserIds.stream().map(this::eligibleIntern).toList(); + if (selectedInterns.stream().anyMatch(intern -> !intern.isEligible()) + || selectedInterns.stream().anyMatch(intern -> project.hasCurrentMember(intern.userId()))) { + throw new ProjectRuleViolationException("One or more selected Interns are no longer eligible"); + } + + var addedAt = clock.instant(); + selectedInterns.forEach(intern -> project.addMember(actorUserId, intern, addedAt)); projects.flush(); } @@ -145,7 +182,7 @@ public class ProjectService { } private ProjectInternEligibility eligibleIntern(long userId) { - return new ProjectInternEligibility(userId, accounts.isEligibleIntern(userId)); + return new ProjectInternEligibility(userId, accounts.isEligibleIntern(userId, LocalDate.now(clock))); } private void requireActiveMentor(long userId) { diff --git a/src/test/java/com/lab/labtimesheet/feature/project/service/ProjectServiceIntegrationTest.java b/src/test/java/com/lab/labtimesheet/feature/project/service/ProjectServiceIntegrationTest.java index f4a12ff..61ccd26 100644 --- a/src/test/java/com/lab/labtimesheet/feature/project/service/ProjectServiceIntegrationTest.java +++ b/src/test/java/com/lab/labtimesheet/feature/project/service/ProjectServiceIntegrationTest.java @@ -106,6 +106,53 @@ class ProjectServiceIntegrationTest { user("other-mentor@example.test", "MENTOR"), projectId, Long.MAX_VALUE)); } + @Test + void ownerAddsSeveralEligibleMembersInOneLockedTransaction() { + long mentorId = user("mentor-batch-add@example.test", "MENTOR"); + long leaderId = intern("leader-batch-add@example.test", "I017"); + long firstMemberId = intern("first-batch-add@example.test", "I018"); + long secondMemberId = intern("second-batch-add@example.test", "I019"); + long projectId = createProject(mentorId, leaderId, "Batch membership"); + + projectService.addMembers(mentorId, projectId, List.of(firstMemberId, secondMemberId)); + + assertEquals(3, count(""" + select count(*) from project_memberships + where project_id = ? and left_at is null + """, projectId)); + } + + @Test + void memberBatchRejectsMissingDuplicateCurrentAndStaleSelectionsWithoutPartialMutation() { + long mentorId = user("mentor-batch-guard@example.test", "MENTOR"); + long leaderId = intern("leader-batch-guard@example.test", "I020"); + long eligibleId = intern("eligible-batch-guard@example.test", "I021"); + long staleId = intern("stale-batch-guard@example.test", "I022"); + long projectId = createProject(mentorId, leaderId, "Batch guard"); + jdbc.update("update intern_profiles set internship_end_date = date '2026-08-13' where user_id = ?", staleId); + entityManager.clear(); + + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, null)); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of())); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of(eligibleId, eligibleId))); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of(Long.MAX_VALUE))); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of(mentorId))); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of(leaderId))); + assertThrows(ProjectRuleViolationException.class, + () -> projectService.addMembers(mentorId, projectId, List.of(eligibleId, staleId))); + + assertEquals(0, count(""" + select count(*) from project_memberships + where project_id = ? and intern_user_id in (?, ?) and left_at is null + """, projectId, eligibleId, staleId)); + } + @Test void leaderChangeClosesOneTermAndDoesNotMoveTaskAssignments() { long mentorId = user("mentor-leader@example.test", "MENTOR");