From 0e42b056bf3cb2392c89f22fe21b28358796e70b Mon Sep 17 00:00:00 2001
From: Danto
여기에 {@code @Transactional} 을 두지 않는 이유: 아래 {@code run()} 은 컨테이너 안의 CLI 가 + * 끝날 때까지 최대 10분을 기다린다. 트랜잭션을 걸면 그 10분 내내 커넥션 하나가 묶이고, 동시 + * CODE 태스크 수만큼 곱해져 풀이 마른다(2026-09-08 dev 고갈 사고와 같은 계열, #337). 이 메서드가 + * DB 에서 하는 일은 자격증명 조회 한 건뿐이라 리포지토리 자신의 짧은 트랜잭션으로 충분하고, + * {@code AiProviderCredential} 은 JPA 엔티티가 아닌 도메인 모델이라 반환 뒤 세션이 필요 없다. + * {@code AgentPlanExecutor.execute()}·{@code InfraOpsAgentService} 와 같은 원칙이다.
*/ @Slf4j @Service @@ -35,12 +41,10 @@ public class CodingAgentExecutionService { * @param provider a coding-agent provider ({@code CLAUDE_CODE} / {@code CODEX}) * @param workspaceDir absolute host path of the checkout the agent may edit */ - @Transactional(readOnly = true) public CodingAgentResult run(Long userId, AiProvider provider, String prompt, String workspaceDir) { return run(userId, provider, prompt, workspaceDir, properties.getTimeout()); } - @Transactional(readOnly = true) public CodingAgentResult run(Long userId, AiProvider provider, String prompt, diff --git a/src/test/java/com/example/dvely/aiaccount/application/service/CodingAgentConnectionHoldIntegrationTest.java b/src/test/java/com/example/dvely/aiaccount/application/service/CodingAgentConnectionHoldIntegrationTest.java new file mode 100644 index 00000000..0513aa1b --- /dev/null +++ b/src/test/java/com/example/dvely/aiaccount/application/service/CodingAgentConnectionHoldIntegrationTest.java @@ -0,0 +1,133 @@ +package com.example.dvely.aiaccount.application.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import com.example.dvely.agent.application.port.out.CodingAgentPort; +import com.example.dvely.agent.application.port.out.CodingAgentResult; +import com.example.dvely.agent.domain.value.AiProvider; +import com.example.dvely.agent.infrastructure.codingagent.CodingAgentRouter; +import com.example.dvely.aiaccount.domain.model.AiProviderCredential; +import com.example.dvely.aiaccount.domain.repository.AiProviderCredentialRepository; +import com.example.dvely.auth.domain.model.User; +import com.example.dvely.auth.domain.repository.UserRepository; +import com.example.dvely.auth.domain.value.GithubId; +import com.zaxxer.hikari.HikariDataSource; +import com.zaxxer.hikari.HikariPoolMXBean; +import java.time.Duration; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import javax.sql.DataSource; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.support.TransactionTemplate; + +/** + * A1 의 실측 증거(#337): 코딩 에이전트가 도는 동안 커넥션이 점유되지 않는다. + * + *2026-09-08 dev 고갈의 핵심은 "커넥션 하나를 오래 쥔다" 였다. 예전 + * {@code CodingAgentExecutionService.run} 은 {@code @Transactional(readOnly = true)} 안에서 + * 컨테이너의 CLI 가 끝날 때까지 최대 10분을 기다렸고, 동시 CODE 태스크 수만큼 곱해졌다. + * 그래서 여기서는 CLI 자리에 "잠깐 도는 가짜 실행"을 끼워 넣고, 그 실행 중에 풀의 active + * 커넥션 수를 직접 샘플링해 비교한다.
+ * + *비교 대상은 같은 테스트 안에서 만든다 — 옛 구조를 {@link TransactionTemplate} 로 그대로 + * 재현해(readOnly 트랜잭션 + DB 접촉) 같은 샘플링을 돌린다. 절대값 대신 두 값을 비교하는 + * 이유는 이 컨텍스트에 {@code @EnableScheduling} 워커들이 함께 떠 있어 풀에 잡음이 있기 + * 때문이다. 잡음은 최댓값만 올리므로 최솟값을 본다 — 트랜잭션이 커넥션을 붙들고 있으면 + * 최솟값이 0 으로 내려갈 수 없고, 붙들지 않으면 내려간다.
+ */ +@SpringBootTest +class CodingAgentConnectionHoldIntegrationTest { + + /** 실제 CLI 대신 끼워 넣는 가짜 실행. 샘플이 충분히 모이도록 이 정도는 돌린다. */ + private static final Duration FAKE_CLI_DURATION = Duration.ofMillis(600); + private static final long SAMPLE_INTERVAL_MS = 10L; + + @MockitoBean + private CodingAgentRouter codingAgentRouter; + + @Autowired + private CodingAgentExecutionService codingAgentExecutionService; + @Autowired + private AiProviderCredentialRepository credentialRepository; + @Autowired + private UserRepository userRepository; + @Autowired + private DataSource dataSource; + @Autowired + private PlatformTransactionManager transactionManager; + + @Test + @DisplayName("CLI 가 도는 동안 트랜잭션도 커넥션 점유도 없다 — 옛 구조는 커넥션을 계속 쥐고 있었다") + void codingAgentRunDoesNotHoldAConnectionWhileTheCliRuns() { + Long userId = seedUserWithCredential(); + HikariPoolMXBean pool = ((HikariDataSource) dataSource).getHikariPoolMXBean(); + + AtomicBoolean transactionActiveDuringRun = new AtomicBoolean(true); + AtomicInteger minActiveDuringRun = new AtomicInteger(Integer.MAX_VALUE); + + CodingAgentPort port = Mockito.mock(CodingAgentPort.class); + when(port.run(any())).thenAnswer(invocation -> { + transactionActiveDuringRun.set(TransactionSynchronizationManager.isActualTransactionActive()); + minActiveDuringRun.set(sampleMinimumActiveConnections(pool)); + return CodingAgentResult.succeeded("done", ""); + }); + when(codingAgentRouter.route(any())).thenReturn(port); + + codingAgentExecutionService.run(userId, AiProvider.CLAUDE_CODE, "프롬프트", "/tmp/ws"); + + // 옛 구조 재현 — readOnly 트랜잭션 안에서 DB 를 한 번 만진 뒤 같은 샘플링을 한다. + TransactionTemplate readOnlyTransaction = new TransactionTemplate(transactionManager); + readOnlyTransaction.setReadOnly(true); + int minActiveInsideTransaction = readOnlyTransaction.execute(status -> { + credentialRepository.findByUserIdAndProvider(userId, AiProvider.ANTHROPIC); + return sampleMinimumActiveConnections(pool); + }); + + assertThat(transactionActiveDuringRun) + .as("CLI 실행 중에는 트랜잭션이 열려 있으면 안 된다(#337)") + .isFalse(); + assertThat(minActiveInsideTransaction) + .as("옛 구조 재현: 트랜잭션이 커넥션을 붙들고 있으므로 active 가 0 으로 내려가지 않는다") + .isGreaterThanOrEqualTo(1); + assertThat(minActiveDuringRun) + .as("지금 구조: CLI 가 도는 동안 이 스레드는 커넥션을 쥐고 있지 않다 " + + "(옛 구조 최솟값=%d)", minActiveInsideTransaction) + .hasValue(0); + } + + /** + * 가짜 CLI 가 도는 동안 풀의 active 를 반복해서 재고 그중 최솟값을 돌려준다. + * 최솟값을 쓰는 이유는 클래스 javadoc 참고 — 스케줄 워커의 순간 점유를 걸러낸다. + */ + private int sampleMinimumActiveConnections(HikariPoolMXBean pool) { + int minimum = Integer.MAX_VALUE; + long deadline = System.nanoTime() + FAKE_CLI_DURATION.toNanos(); + while (System.nanoTime() < deadline) { + minimum = Math.min(minimum, pool.getActiveConnections()); + try { + Thread.sleep(SAMPLE_INTERVAL_MS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + break; + } + } + return minimum; + } + + private Long seedUserWithCredential() { + User owner = userRepository.save( + new User(new GithubId("i337-tx-boundary-" + System.nanoTime()), "octo", null)); + credentialRepository.save(new AiProviderCredential( + owner.getId(), AiProvider.ANTHROPIC, "sk-ant-api03-test-key", "test")); + return owner.getId(); + } +} From 373b711d2ec2131821788218aeeea0fa16e97645 Mon Sep 17 00:00:00 2001 From: Danto실패 시 저장하지 않는 성질은 트랜잭션 롤백이 아니라 순서로 지킨다. 예전에는 + * 외부 호출이 던지면 롤백이 저장을 되돌려 줬다. 트랜잭션이 없는 지금은 세 하위 경로 + * ({@link #bindManagedSubdomain} · {@link #bindCustomDomain} · {@link #bindS3Frontend}) 모두 + * 외부 호출을 유일한 {@code domainBindingRepository.save} 보다 앞에 두어, 외부가 4xx 를 + * 주면 그 자리에서 던지고 저장 줄에 도달하지 못한다. 저장이 경로마다 한 건뿐이라 원자성도 + * 줄지 않는다. 이 순서는 바꾸면 안 된다 — 바꾸는 순간 "외부 실패인데 행은 남는" 상태가 + * 생긴다({@code DomainBindingCommandServiceTest} 가 이를 고정한다).
+ */ public DomainBindingResult bindDomain(Long ownerUserId, Long projectId, BindDomainCommand command) { Project project = resolveProject(ownerUserId, projectId); DomainBindingResult result; @@ -98,7 +111,11 @@ public DomainBindingResult bindDomain(Long ownerUserId, Long projectId, BindDoma return result; } - @Transactional + /** + * 트랜잭션을 걸지 않는다 — {@link #verify} 가 호스팅 어댑터 · Cloudflare · DNS 조회를 잇달아 + * 호출한다(#337). 저장은 {@code verify} 끝의 {@code save} 한 건이고 외부 호출이 전부 그보다 + * 앞이라, 외부가 던지면 저장에 도달하지 않는 것은 이전과 같다. + */ public DomainBindingResult checkVerification(Long ownerUserId, Long domainId) { DomainBinding domain = resolveDomainOwnedBy(domainId, ownerUserId); return verify(domain, resolveProject(ownerUserId, domain.getProjectId()), ownerUserId); @@ -107,8 +124,10 @@ public DomainBindingResult checkVerification(Long ownerUserId, Long domainId) { /** * 검증 워커 경로. 요청한 사용자가 없으므로 도메인이 속한 프로젝트에서 소유자를 찾아 * 같은 검증을 돌린다. 소유권을 확인하는 것이 아니라 검증에 쓸 토큰의 주인을 찾는 것이다. + * + *워커가 배치로 태우는 입구라 {@link #checkVerification} 보다 트랜잭션 제거 효과가 크다 — + * 예전에는 20건을 순차 검증하는 동안 매 건이 외부 응답을 기다리며 커넥션을 붙들었다(#337). */ - @Transactional public DomainBindingResult checkVerificationAsSystem(Long domainId) { DomainBinding domain = domainBindingRepository.findById(domainId) .orElseThrow(() -> new NotFoundException("도메인을 찾을 수 없습니다. domainId=" + domainId)); @@ -158,7 +177,6 @@ private DomainBindingResult verify(DomainBinding domain, Project project, Long o } /** HTTP path — no Agent task (see {@link #deleteDomain(Long, Long, String)}). */ - @Transactional public void deleteDomain(Long ownerUserId, Long domainId) { deleteDomain(ownerUserId, domainId, null); } @@ -167,8 +185,12 @@ public void deleteDomain(Long ownerUserId, Long domainId) { * @param taskId nullable — non-null only when the Agent-driven delete path (design H11, * ADR-A8) called this; a direct HTTP call always passes null via the 2-arg * overload above. + * + *
트랜잭션을 걸지 않는다(#337). 외부 정리(어댑터 unbind · Cloudflare 레코드 삭제 · S3 + * teardown)가 전부 유일한 쓰기인 {@code deleteById} 보다 앞에 있어, 외부가 실패하면 행이 + * 그대로 남는 기존 동작이 순서로 유지된다. 감사 기록은 원래부터 {@code AuditRecorder} 가 + * 별도로 커밋하며 절대 던지지 않는다.
*/ - @Transactional public void deleteDomain(Long ownerUserId, Long domainId, String taskId) { DomainBinding domain = resolveDomainOwnedBy(domainId, ownerUserId); if (domain.getHostingTarget() == DomainHostingTarget.AWS_S3_FRONTEND) { @@ -322,8 +344,11 @@ private DomainBindingResult verifyS3Frontend(DomainBinding domain) { * 프로젝트 삭제 시 그 프로젝트의 S3 프론트 도메인을 정리한다(Cloudflare 레코드·CloudFront·인증서). * 시스템 내부 호출이라 소유권 검사는 상위(프로젝트 삭제)가 이미 했다. 한 도메인 정리가 실패해도 * 나머지는 계속한다(best-effort). + * + *트랜잭션을 걷어냈다(#337). 예전에는 배치 전체가 트랜잭션 하나라, 건별 try/catch 가 + * 있어도 뒤쪽 한 건의 삭제 실패가 앞서 성공한 삭제까지 되돌릴 수 있었다 — best-effort 라는 + * 주석과 실제 동작이 어긋나 있었다. 이제 건별로 커밋된다.
*/ - @Transactional public void cleanupProjectS3Domains(Long projectId) { for (DomainBinding domain : domainBindingRepository.findByProjectIdOrderByCreatedAtDesc(projectId)) { if (domain.getHostingTarget() != DomainHostingTarget.AWS_S3_FRONTEND) { @@ -387,8 +412,11 @@ private void safeCleanup(Runnable cleanup) { * 우리 서브도메인이 남의 서버를 가리키는 dangling DNS(서브도메인 탈취)가 된다 — 그래서 레코드를 반드시 * 지운다. 시스템 내부 호출(종료 정리)이라 소유권 검사는 상위(terminate)가 이미 했다. 한 도메인 정리가 * 실패해도 나머지·종료는 계속한다(best-effort). + * + *{@link #cleanupProjectS3Domains} 와 같은 이유로 트랜잭션을 걷어냈다(#337). Cloudflare + * 삭제가 {@code deleteById} 보다 앞이라, 레코드가 안 지워지면 행도 남는 순서는 그대로다 — + * dangling DNS 를 남기느니 행을 남겨 다음 정리에 걸리게 하는 편이 안전하다.
*/ - @Transactional public void releaseServerDomains(Long projectId, String ipAddress) { if (ipAddress == null || ipAddress.isBlank()) { return; diff --git a/src/test/java/com/example/dvely/domainbinding/application/command/DomainBindingCommandServiceTest.java b/src/test/java/com/example/dvely/domainbinding/application/command/DomainBindingCommandServiceTest.java index a15c770c..85a601e6 100644 --- a/src/test/java/com/example/dvely/domainbinding/application/command/DomainBindingCommandServiceTest.java +++ b/src/test/java/com/example/dvely/domainbinding/application/command/DomainBindingCommandServiceTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -47,6 +48,9 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.web.client.HttpClientErrorException; @ExtendWith(MockitoExtension.class) class DomainBindingCommandServiceTest { @@ -572,6 +576,121 @@ private DomainBinding backendBinding(Long id, String hostname, String ip, String LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now()); } + // ── #337: 외부 호출이 실패하면 저장이 남지 않는다 ──────────────────────────────── + // 예전에는 이 성질을 @Transactional 롤백이 공짜로 줬다. 트랜잭션을 걷어낸 뒤에는 "외부 + // 호출을 유일한 save 보다 앞에 둔다" 는 순서가 유일한 보장이므로, 그 순서를 여기서 고정한다. + // 이 테스트들이 깨진다면 누군가 save 를 외부 호출 앞으로 옮겼다는 뜻이다. + + @Test + void bindManagedSubdomain_cloudflare4xx_leavesNoRow() { + Project project = boundProject("https://octo.github.io/repo/"); + when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); + when(userRepository.findById(1L)).thenReturn(Optional.of(activeUser())); + when(domainBindingRepository.existsByHostnameIgnoreCase("my-project.qeploy.com")).thenReturn(false); + when(hostingAdapter.resolveDnsTarget(any())).thenReturn("octo.github.io"); + when(cloudflareDnsPort.createCnameRecord("my-project.qeploy.com", "octo.github.io")) + .thenThrow(HttpClientErrorException.create( + HttpStatus.BAD_REQUEST, "Bad Request", HttpHeaders.EMPTY, new byte[0], null)); + + assertThatThrownBy(() -> commandService.bindDomain( + 1L, 11L, new BindDomainCommand(DomainType.MANAGED_SUBDOMAIN, "my-project", null, null))) + .isInstanceOf(HttpClientErrorException.class); + + verify(domainBindingRepository, never()).save(any(DomainBinding.class)); + verifyNoInteractions(auditRecorder); + } + + @Test + void bindManagedSubdomain_pagesBind4xx_leavesNoRowAndRollsBackTheDnsRecord() { + Project project = boundProject("https://octo.github.io/repo/"); + when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); + when(userRepository.findById(1L)).thenReturn(Optional.of(activeUser())); + when(domainBindingRepository.existsByHostnameIgnoreCase("my-project.qeploy.com")).thenReturn(false); + when(hostingAdapter.resolveDnsTarget(any())).thenReturn("octo.github.io"); + when(cloudflareDnsPort.createCnameRecord("my-project.qeploy.com", "octo.github.io")) + .thenReturn("cf-record-1"); + org.mockito.Mockito.doThrow(HttpClientErrorException.create( + HttpStatus.UNPROCESSABLE_ENTITY, "Unprocessable", HttpHeaders.EMPTY, new byte[0], null)) + .when(hostingAdapter).bind(any(), eq("my-project.qeploy.com")); + + assertThatThrownBy(() -> commandService.bindDomain( + 1L, 11L, new BindDomainCommand(DomainType.MANAGED_SUBDOMAIN, "my-project", null, null))) + .isInstanceOf(HttpClientErrorException.class); + + verify(domainBindingRepository, never()).save(any(DomainBinding.class)); + // 이미 만든 DNS 레코드는 보상 삭제된다 — 트랜잭션이 해주던 일이 아니라 원래 명시적 코드였다. + verify(cloudflareDnsPort).deleteRecord("my-project.qeploy.com", "cf-record-1"); + verifyNoInteractions(auditRecorder); + } + + @Test + void bindCustomDomain_pagesBind4xx_leavesNoRow() { + Project project = boundProject("https://octo.github.io/repo/"); + when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); + when(userRepository.findById(1L)).thenReturn(Optional.of(activeUser())); + when(domainBindingRepository.existsByHostnameIgnoreCase("www.mysite.com")).thenReturn(false); + when(hostingAdapter.resolveDnsTarget(any())).thenReturn("octo.github.io"); + org.mockito.Mockito.doThrow(HttpClientErrorException.create( + HttpStatus.FORBIDDEN, "Forbidden", HttpHeaders.EMPTY, new byte[0], null)) + .when(hostingAdapter).bind(any(), eq("www.mysite.com")); + + assertThatThrownBy(() -> commandService.bindDomain( + 1L, 11L, new BindDomainCommand(DomainType.CUSTOM_DOMAIN, null, "www.mysite.com", null))) + .isInstanceOf(HttpClientErrorException.class); + + verify(domainBindingRepository, never()).save(any(DomainBinding.class)); + verifyNoInteractions(auditRecorder); + } + + @Test + void deleteDomain_unbind4xx_keepsTheRow() { + Project project = boundProject("https://octo.github.io/repo/"); + DomainBinding domain = new DomainBinding( + 31L, 11L, DomainType.MANAGED_SUBDOMAIN, DomainHostingTarget.GITHUB_PAGES, + "my-project.qeploy.com", DomainStatus.CONNECTED, + com.example.dvely.domainbinding.domain.value.VerificationMethod.CNAME, + "octo.github.io", "record-1", true, CertificateStatus.ACTIVE, null, + LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now() + ); + when(domainBindingRepository.findById(31L)).thenReturn(Optional.of(domain)); + when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); + when(userRepository.findById(1L)).thenReturn(Optional.of(activeUser())); + org.mockito.Mockito.doThrow(HttpClientErrorException.create( + HttpStatus.NOT_FOUND, "Not Found", HttpHeaders.EMPTY, new byte[0], null)) + .when(hostingAdapter).unbind(any(), eq("my-project.qeploy.com")); + + assertThatThrownBy(() -> commandService.deleteDomain(1L, 31L)) + .isInstanceOf(HttpClientErrorException.class); + + verify(domainBindingRepository, never()).deleteById(any()); + verifyNoInteractions(auditRecorder); + } + + @Test + void checkVerification_hostingProbe4xx_leavesTheStoredStateUntouched() { + Project project = boundProject("https://octo.github.io/repo/"); + LocalDateTime now = LocalDateTime.now(); + DomainBinding domain = new DomainBinding( + 31L, 11L, DomainType.MANAGED_SUBDOMAIN, DomainHostingTarget.GITHUB_PAGES, + "my-project.qeploy.com", DomainStatus.VERIFYING, + com.example.dvely.domainbinding.domain.value.VerificationMethod.CNAME, + "octo.github.io", "record-1", false, CertificateStatus.PROVISIONING, null, + now, now, now + ); + when(domainBindingRepository.findById(31L)).thenReturn(Optional.of(domain)); + when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); + when(userRepository.findById(1L)).thenReturn(Optional.of(activeUser())); + when(hostingAdapter.verify(any(), eq("my-project.qeploy.com"))) + .thenThrow(HttpClientErrorException.create( + HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", HttpHeaders.EMPTY, new byte[0], null)); + + assertThatThrownBy(() -> commandService.checkVerification(1L, 31L)) + .isInstanceOf(HttpClientErrorException.class); + + verify(domainBindingRepository, never()).save(any(DomainBinding.class)); + assertThat(domain.getStatus()).isEqualTo(DomainStatus.VERIFYING); + } + private DomainBindingCommandService commandService(CloudflareProperties cloudflareProperties) { return new DomainBindingCommandService( projectRepository, From c3f6c94056229e4aa3f4223eca8ac62bfd37036a Mon Sep 17 00:00:00 2001 From: Danto실패 시 동작은 그대로다: diff 를 뜨다 예외가 나면 아래 저장에 도달하지 못하므로 + * Change 행이 남지 않는다 — 예전에 롤백이 해주던 것과 같은 결과를, 외부 호출을 저장보다 + * 먼저 두는 순서로 얻는다.
+ */ public void record(String taskId, String summary) { AgentTask task = taskStore.get(taskId); PreviewSessionInfo preview = previewSessionService.findByTaskId(taskId) diff --git a/src/test/java/com/example/dvely/change/application/service/ChangeServiceTest.java b/src/test/java/com/example/dvely/change/application/service/ChangeServiceTest.java index 6dcd950d..1853bc4b 100644 --- a/src/test/java/com/example/dvely/change/application/service/ChangeServiceTest.java +++ b/src/test/java/com/example/dvely/change/application/service/ChangeServiceTest.java @@ -1,6 +1,9 @@ package com.example.dvely.change.application.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -150,6 +153,26 @@ void truncatesAnOversizedDiffInsteadOfFailingTheSave() { assertThat(diff).endsWith("… (변경 내역이 너무 커서 이후는 생략했습니다)\n"); } + /** + * #337: Docker 가 통째로 실패하면 Change 행이 남지 않는다. + * + *예전에는 {@code @Transactional} 롤백이 이 성질을 줬다. 트랜잭션을 걷어낸 뒤에는 + * "diff 를 먼저 뜨고 그 다음에 저장한다" 는 순서가 유일한 보장이라 여기서 고정한다. + * (종료 코드가 0 이 아닌 경우는 {@code storesNothingRatherThanGarbage…} 가 다루는 별개의 + * 경로다 — 그쪽은 빈 diff 로 저장한다.)
+ */ + @Test + void savesNothingWhenTheDockerExecItselfFails() { + Fixture f = fixture(); + when(f.docker.exec("container-1", "[ -d /workspace/app/.git ] && echo yes || echo no")) + .thenThrow(new IllegalStateException("docker daemon 무응답")); + + assertThatThrownBy(() -> f.service.record("task-1", "FAQ 추가")) + .isInstanceOf(IllegalStateException.class); + + verify(f.repository, never()).save(any(ChangeEntity.class)); + } + private record Fixture(ChangeService service, SpringDataChangeRepository repository, DockerContainerService docker) {} From 37e65031d1023bd0873f4ca600c36a3c17184cd3 Mon Sep 17 00:00:00 2001 From: Danto제거와 저장 사이에는 "컨테이너는 없는데 행은 아직 ACTIVE" 인 짧은 창이 생기지만, + * 그 상태의 세션은 게이트웨이가 도달 실패로 보고 {@link #reclaimUnreachable} 이 다시 + * 회수한다({@code removeContainer} 는 멱등).
+ */ private void expire(PreviewSessionEntity session, PreviewSessionStatus status) { + dockerService.removeContainer(session.getContainerId()); session.close(status); repository.save(session); - dockerService.removeContainer(session.getContainerId()); log.info("[PreviewSession] 종료: sessionId={} status={}", session.getId(), status); } diff --git a/src/test/java/com/example/dvely/preview/application/service/PreviewSessionServiceTest.java b/src/test/java/com/example/dvely/preview/application/service/PreviewSessionServiceTest.java index 9dd69e00..d79fad26 100644 --- a/src/test/java/com/example/dvely/preview/application/service/PreviewSessionServiceTest.java +++ b/src/test/java/com/example/dvely/preview/application/service/PreviewSessionServiceTest.java @@ -6,6 +6,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -514,4 +515,102 @@ void refusesToGrantAccessToAForeignSession() { assertThatThrownBy(() -> service.grantAccess("session-1", 8L, Duration.ofMinutes(30))) .isInstanceOf(NotFoundException.class); } + + // ── #337: 트랜잭션을 걷어낸 뒤에도 실패 시 동작이 같다 ────────────────────────── + + /** + * 예전에는 컨테이너 제거가 실패하면 트랜잭션 롤백이 상태 변경을 되돌려 세션이 ACTIVE 로 + * 남았다. 이제 제거를 저장보다 먼저 해서 같은 결과를 만든다 — 이 테스트가 그 순서를 고정한다. + */ + @Test + void containerRemovalFailing_leavesTheSessionActiveAndSavesNothing() { + SpringDataPreviewSessionRepository repository = mock(SpringDataPreviewSessionRepository.class); + DockerContainerService dockerService = mock(DockerContainerService.class); + PreviewSessionService service = new PreviewSessionService( + repository, + dockerService, + mock(TaskStore.class), + properties(), + gatewayUrlResolver(), + accessCookies(), mock(PreviewRuntimeConfigService.class) + ); + PreviewSessionEntity session = new PreviewSessionEntity( + "session-1", "token", 7L, 11L, 21L, "task-1", "container-1", 32768, + "https://preview.qeploy.test/session-1/", LocalDateTime.now().plusMinutes(30)); + when(repository.findByIdAndOwnerUserId("session-1", 7L)).thenReturn(Optional.of(session)); + org.mockito.Mockito.doThrow(new IllegalStateException("docker daemon 무응답")) + .when(dockerService).removeContainer("container-1"); + + assertThatThrownBy(() -> service.closeOwned("session-1", 7L)) + .isInstanceOf(IllegalStateException.class); + + assertThat(session.getStatus()).isEqualTo(PreviewSessionStatus.ACTIVE.name()); + verify(repository, never()).save(any(PreviewSessionEntity.class)); + } + + /** + * 예전에는 배치 전체가 트랜잭션 하나라 한 건의 Docker 실패가 나머지 정리까지 통째로 + * 되돌렸다 — 컨테이너 하나가 고장나면 만료 정리가 영영 진행되지 않았다. + */ + @Test + void oneBrokenContainerDoesNotStopTheRestOfTheCleanup() { + SpringDataPreviewSessionRepository repository = mock(SpringDataPreviewSessionRepository.class); + DockerContainerService dockerService = mock(DockerContainerService.class); + PreviewSessionService service = new PreviewSessionService( + repository, + dockerService, + mock(TaskStore.class), + properties(), + gatewayUrlResolver(), + accessCookies(), mock(PreviewRuntimeConfigService.class) + ); + PreviewSessionEntity broken = new PreviewSessionEntity( + "session-broken", "token", 1L, 11L, 21L, "task-1", "container-broken", 32768, + "https://preview.qeploy.test/session-broken/", LocalDateTime.now().minusMinutes(1)); + PreviewSessionEntity healthy = new PreviewSessionEntity( + "session-healthy", "token", 1L, 12L, 22L, "task-2", "container-healthy", 32769, + "https://preview.qeploy.test/session-healthy/", LocalDateTime.now().minusMinutes(1)); + when(repository.findByStatusInAndExpiresAtBefore(any(), any(LocalDateTime.class))) + .thenReturn(List.of(broken, healthy)); + when(repository.save(healthy)).thenReturn(healthy); + org.mockito.Mockito.doThrow(new IllegalStateException("docker daemon 무응답")) + .when(dockerService).removeContainer("container-broken"); + + service.cleanupExpired(); + + // 고장난 쪽은 그대로 남아 다음 주기에 다시 걸린다. + assertThat(broken.getStatus()).isEqualTo(PreviewSessionStatus.ACTIVE.name()); + // 뒤에 있던 정상 세션은 영향을 받지 않는다. + assertThat(healthy.getStatus()).isEqualTo(PreviewSessionStatus.EXPIRED.name()); + verify(dockerService).removeContainer("container-healthy"); + } + + /** 포트 조회·도달 확인이 실패하면 ACTIVE 로 올리지 않는다(예전 롤백과 같은 결과). */ + @Test + void mappedPortLookupFailing_leavesTheSessionProvisioning() { + SpringDataPreviewSessionRepository repository = mock(SpringDataPreviewSessionRepository.class); + DockerContainerService dockerService = mock(DockerContainerService.class); + PreviewSessionService service = new PreviewSessionService( + repository, + dockerService, + mock(TaskStore.class), + properties(), + gatewayUrlResolver(), + accessCookies(), mock(PreviewRuntimeConfigService.class) + ); + PreviewSessionEntity provisioning = new PreviewSessionEntity( + "session-1", "token", 1L, 11L, 21L, "task-1", "container-1", 32768, + "https://preview.qeploy.test/session-1/", LocalDateTime.now().plusMinutes(30), + PreviewSessionStatus.PROVISIONING); + when(repository.findByTaskIdAndStatus("task-1", PreviewSessionStatus.PROVISIONING.name())) + .thenReturn(Optional.of(provisioning)); + when(dockerService.getMappedPort("container-1")) + .thenThrow(new IllegalStateException("포트 조회 실패")); + + assertThatThrownBy(() -> service.markServing("task-1")) + .isInstanceOf(IllegalStateException.class); + + assertThat(provisioning.getStatus()).isEqualTo(PreviewSessionStatus.PROVISIONING.name()); + verify(repository, never()).save(any(PreviewSessionEntity.class)); + } } From 2f40d54266da1a9ae75e33dc52987fd22ad83d30 Mon Sep 17 00:00:00 2001 From: Danto트랜잭션을 걸지 않는다 — GitHub OAuth·User API 두 번을 기다리는 동안 커넥션을 붙들던 + * 자리다(#337). 로그인은 가장 자주 열리는 경로라 그 점유가 그대로 풀 고갈로 이어졌다.
+ * + *실패 시 동작은 그대로다: 외부 호출 두 건이 모두 저장보다 앞에 있어, 둘 중 하나라도 + * 던지면 아래 저장에 도달하지 않는다 — 예전 롤백과 같은 결과다. 저장 두 건(유저·리프레시 + * 토큰)이 더는 한 트랜잭션이 아니지만, 이 경로는 githubId 로 찾아 없으면 만드는 멱등 연산이라 + * 뒤의 저장이 실패해도 재시도가 그대로 복구한다(유저 행만 남고 손상은 없다).
*/ - @Transactional public TokenResult loginWithGithub(GithubLoginCommand command) { oAuthStateManager.verify(command.state()); String oauthToken = githubOAuthPort.getAccessToken(command.code()); @@ -112,19 +119,29 @@ public void logout(Long userId, String accessToken) { /** * GitHub App 설치 완료 콜백 처리 * installation_id 저장 + code가 있으면 GitHub App User Token 발급 + * + *트랜잭션을 걷어내면서(#337) 순서를 바꿨다. 예전에는 installationId 를 먼저 반영하고 + * 그 뒤에 GitHub 토큰 교환을 호출했는데, 교환이 실패하면 롤백이 installationId 반영까지 + * 되돌려 아무것도 저장되지 않는 것이 이 메서드의 실제 동작이었다. 롤백이 사라진 + * 지금 같은 결과를 얻으려면 외부 호출을 저장보다 앞에 두는 수밖에 없다 — 교환이 던지면 + * 아래 저장 구간에 도달하지 않는다.
+ * + *유저 조회는 외부 호출보다 앞에 남겨 둔다. 없는 유저면 code 를 소모하기 전에 404 가 + * 나가던 기존 순서를 그대로 지키기 위해서다.
*/ - @Transactional public void linkGithubApp(Long userId, Long installationId, String code) { User user = userRepository.findById(userId) .orElseThrow(() -> new NotFoundException("유저를 찾을 수 없습니다: " + userId)); + GithubAppPort.GithubUserTokenInfo tokenInfo = code == null ? null : githubAppPort.getUserToken(code); + + // 여기부터가 저장 구간 — 위 외부 호출이 실패했다면 도달하지 않는다. // 재인증 콜백은 installation_id 없이 올 수 있음 — 저장된 값 유지 if (installationId != null) { authDomainService.updateInstallationId(user, installationId); } - if (code != null) { - GithubAppPort.GithubUserTokenInfo tokenInfo = githubAppPort.getUserToken(code); + if (tokenInfo != null) { LocalDateTime expiresAt = LocalDateTime.now().plusSeconds(tokenInfo.expiresInSeconds()); user.updateUserToken(tokenInfo.accessToken(), tokenInfo.refreshToken(), expiresAt); log.info("GitHub App User Token 발급 완료: userId={}", userId); @@ -138,8 +155,10 @@ public void linkGithubApp(Long userId, Long installationId, String code) { /** * GitHub App 설치 설정 페이지(state 없음)에서 오는 콜백 처리 * code로 User Token 발급 → GitHub 유저 정보로 DB 유저 식별 + * + *트랜잭션을 걸지 않는다 — GitHub 호출 두 번이 이미 저장보다 앞에 있어, 실패하면 저장에 + * 도달하지 않는 것은 그대로다(#337). 저장도 {@code save} 한 번뿐이라 원자성이 줄지 않는다.
*/ - @Transactional public void linkGithubAppByCode(Long installationId, String code) { if (code == null) { throw new IllegalArgumentException("code가 없어 유저를 식별할 수 없습니다"); @@ -170,8 +189,12 @@ public void linkGithubAppByCode(Long installationId, String code) { * bad_refresh_token 을 맞는다(2026-08-18 운영 실측: 저장소 연결 승인이 이 경로로 실패했다). * * 그러니 갱신 후에는 다시 읽지 말고 이 반환값을 쓸 것. + * + *여기에는 트랜잭션을 걸지 않는다(#337). 이 메서드는 자기 DB 작업이 없고 아래 두 호출이 + * 모두 {@code REQUIRES_NEW} 라, 바깥 트랜잭션은 GitHub 갱신을 기다리는 내내 아무 일도 하지 + * 않으면서 커넥션 하나를 더 붙들고 있을 뿐이었다(안쪽까지 합쳐 동시에 두 개). 되돌릴 것이 + * 없으니 롤백에 기대던 동작도 없다.
*/ - @Transactional public String refreshGithubUserToken(Long userId) { // 빠른 경로 — 다른 흐름이 이미 갱신했으면 잠금까지 가지 않는다. 그 갱신은 별도 // 트랜잭션으로 커밋되지만 호출자의 영속성 컨텍스트에는 옛 UserEntity 가 남아 여전히 diff --git a/src/test/java/com/example/dvely/auth/application/command/AuthCommandServiceTest.java b/src/test/java/com/example/dvely/auth/application/command/AuthCommandServiceTest.java index e224f72e..141a2b92 100644 --- a/src/test/java/com/example/dvely/auth/application/command/AuthCommandServiceTest.java +++ b/src/test/java/com/example/dvely/auth/application/command/AuthCommandServiceTest.java @@ -30,6 +30,9 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.web.client.HttpClientErrorException; @ExtendWith(MockitoExtension.class) class AuthCommandServiceTest { @@ -147,6 +150,41 @@ void aRefreshThatNoOneElseHandledGoesThroughTheLock() { verify(tokenRefresher).refreshWithLock(10L); } + // ── #337: 외부 호출이 실패하면 저장이 남지 않는다 ──────────────────────────────── + + /** + * 예전에는 installationId 를 먼저 반영하고 그 뒤에 토큰 교환을 호출했고, 교환이 실패하면 + * 롤백이 그 반영까지 되돌려 아무것도 저장되지 않았다. 트랜잭션을 걷어내면서 외부 + * 호출을 저장 앞으로 옮겨 같은 결과를 유지한다 — 이 테스트가 그 순서를 고정한다. + */ + @Test + void githubTokenExchangeFailing_savesNeitherTheTokenNorTheInstallationId() { + when(userRepository.findById(10L)).thenReturn(Optional.of(user(10L, null))); + when(githubAppPort.getUserToken("code")) + .thenThrow(HttpClientErrorException.create( + HttpStatus.BAD_REQUEST, "bad_verification_code", + HttpHeaders.EMPTY, new byte[0], null)); + + assertThatThrownBy(() -> service.linkGithubApp(10L, 123L, "code")) + .isInstanceOf(HttpClientErrorException.class); + + verify(userRepository, never()).save(any()); + } + + @Test + void githubUserLookupFailing_createsNoUserAndNoRefreshToken() { + when(githubOAuthPort.getAccessToken("code")).thenReturn("oauth-token"); + when(githubUserPort.getUser("oauth-token")) + .thenThrow(HttpClientErrorException.create( + HttpStatus.UNAUTHORIZED, "Unauthorized", HttpHeaders.EMPTY, new byte[0], null)); + + assertThatThrownBy(() -> service.loginWithGithub(new GithubLoginCommand("code", "state"))) + .isInstanceOf(HttpClientErrorException.class); + + verify(userRepository, never()).save(any()); + verify(refreshTokenRepository, never()).save(any()); + } + private User user(Long id, Long installationId) { return new User( id, From 9d19f0075ea838e40e843a80afad60d2c5cd65bf Mon Sep 17 00:00:00 2001 From: Danto실패 시 동작은 그대로다: 외부 호출이 전부 유일한 저장({@code bindToProject} 안의 + * {@code projectRepository.save})보다 앞에 있어, 어느 하나가 던지면 저장에 도달하지 않는다. + * 저장이 한 건이라 원자성도 줄지 않는다. {@code create} 모드에서 저장소를 만든 뒤 뒷단계가 + * 실패하면 GitHub 에 고아 저장소가 남는 것은 롤백으로도 되돌릴 수 없던 일이라 이전과 같다.
+ */ public ProjectRepositoryResult connectRepository(Long ownerUserId, Long projectId, ConnectProjectRepositoryCommand command) { @@ -152,18 +164,21 @@ public ProjectDetailResult updateProject(Long ownerUserId, Long projectId, Updat return toDetailResult(savedProject); } - @Transactional + /** + * 트랜잭션을 걸지 않는다 — 저장소까지 지우는 모드가 GitHub 삭제를 기다리는 동안 커넥션을 + * 붙들고 있었다(#337). 로컬 정리(대화 삭제 + 프로젝트 삭제)는 갈라지면 안 되므로 + * {@link ProjectDeletionService} 의 짧은 트랜잭션 하나로 함께 커밋한다. + * + *실패 시 동작은 그대로다: GitHub 삭제가 던지면 로컬 정리에 도달하지 않아 아무것도 + * 지워지지 않는다(예전 롤백과 같은 결과). 반대로 GitHub 삭제가 성공한 뒤 로컬 정리가 + * 실패하는 경우도 이전과 같다 — 되돌릴 수 없는 삭제라 롤백이 해결해 준 적이 없고, 그래서 + * 감사 기록을 그 직후에 남기는 순서(H3)도 그대로 두었다.
+ */ public void deleteProject(Long ownerUserId, Long projectId, ProjectDeleteMode deleteMode) { - Project project = getProject(ownerUserId, projectId); - if (deleteMode == ProjectDeleteMode.PROJECT_AND_REPOSITORY) { - deleteProjectAndRepository(ownerUserId, project); - return; + deleteRemoteRepository(ownerUserId, getProject(ownerUserId, projectId)); } - - chatCommandService.trashConversationsForProject(ownerUserId, projectId); - projectDomainService.delete(project); - projectRepository.save(project); + projectDeletionService.purge(ownerUserId, projectId, deleteMode); } private Project getProject(Long ownerUserId, Long projectId) { @@ -171,7 +186,8 @@ private Project getProject(Long ownerUserId, Long projectId) { .orElseThrow(() -> new ProjectNotFoundException(projectId, ownerUserId)); } - private void deleteProjectAndRepository(Long ownerUserId, Project project) { + /** 되돌릴 수 없는 GitHub 삭제만 담당한다 — 트랜잭션 밖에서, 로컬 정리보다 먼저 끝낸다. */ + private void deleteRemoteRepository(Long ownerUserId, Project project) { if (!project.hasSourceRepository()) { throw new IllegalStateException("프로젝트에 연결된 저장소가 없습니다."); } @@ -196,9 +212,6 @@ private void deleteProjectAndRepository(Long ownerUserId, Project project) { null, null )); - chatCommandService.deleteConversationsForProject(ownerUserId, project.getId()); - projectDomainService.delete(project); - projectRepository.save(project); } private String normalizeRepositoryMode(String repositoryMode) { diff --git a/src/main/java/com/example/dvely/project/application/service/ProjectDeletionService.java b/src/main/java/com/example/dvely/project/application/service/ProjectDeletionService.java new file mode 100644 index 00000000..546b5149 --- /dev/null +++ b/src/main/java/com/example/dvely/project/application/service/ProjectDeletionService.java @@ -0,0 +1,44 @@ +package com.example.dvely.project.application.service; + +import com.example.dvely.chat.application.command.ChatCommandService; +import com.example.dvely.project.application.command.dto.ProjectDeleteMode; +import com.example.dvely.project.domain.exception.ProjectNotFoundException; +import com.example.dvely.project.domain.model.Project; +import com.example.dvely.project.domain.repository.ProjectRepository; +import com.example.dvely.project.domain.service.ProjectDomainService; +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * 프로젝트 삭제의 로컬 정리만 한 트랜잭션으로 묶는다. + * + *따로 둔 이유는 트랜잭션 경계다(#337). 예전에는 {@code ProjectCommandService.deleteProject} + * 하나가 GitHub 저장소 삭제(외부 호출)와 이 정리를 같은 트랜잭션에 담고 있어, GitHub 응답을 + * 기다리는 내내 커넥션이 묶였다. 외부 호출을 트랜잭션 밖으로 빼면서도 대화 삭제와 프로젝트 + * 삭제가 함께 커밋되는 성질은 지켜야 했는데 — 둘이 갈라지면 대화만 사라진 프로젝트가 + * 남는다 — 자기 호출로는 프록시가 걸리지 않으므로 별도 빈으로 뺐다. 같은 패키지의 + * {@link RepositoryProvisioningService} 와 같은 결이다.
+ */ +@Service +@RequiredArgsConstructor +public class ProjectDeletionService { + + private final ProjectRepository projectRepository; + private final ProjectDomainService projectDomainService; + private final ChatCommandService chatCommandService; + + @Transactional + public void purge(Long ownerUserId, Long projectId, ProjectDeleteMode deleteMode) { + Project project = projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(projectId, ownerUserId) + .orElseThrow(() -> new ProjectNotFoundException(projectId, ownerUserId)); + + if (deleteMode == ProjectDeleteMode.PROJECT_AND_REPOSITORY) { + chatCommandService.deleteConversationsForProject(ownerUserId, projectId); + } else { + chatCommandService.trashConversationsForProject(ownerUserId, projectId); + } + projectDomainService.delete(project); + projectRepository.save(project); + } +} diff --git a/src/test/java/com/example/dvely/project/application/command/ProjectCommandServiceTest.java b/src/test/java/com/example/dvely/project/application/command/ProjectCommandServiceTest.java index 43bb401b..3d0e5046 100644 --- a/src/test/java/com/example/dvely/project/application/command/ProjectCommandServiceTest.java +++ b/src/test/java/com/example/dvely/project/application/command/ProjectCommandServiceTest.java @@ -12,7 +12,7 @@ import com.example.dvely.audit.application.AuditEvent; import com.example.dvely.audit.application.AuditRecorder; import com.example.dvely.audit.domain.value.AuditAction; -import com.example.dvely.chat.application.command.ChatCommandService; +import com.example.dvely.project.application.service.ProjectDeletionService; import com.example.dvely.project.application.command.dto.ConnectProjectRepositoryCommand; import com.example.dvely.project.application.command.dto.CreateProjectCommand; import com.example.dvely.project.application.port.out.GithubRepositoryPort; @@ -56,7 +56,7 @@ class ProjectCommandServiceTest { private UserProfilePort userProfilePort; @Mock - private ChatCommandService chatCommandService; + private ProjectDeletionService projectDeletionService; @Mock private AuditRecorder auditRecorder; @@ -73,7 +73,6 @@ void setUp() { new ProjectDomainService(), githubRepositoryPort, userProfilePort, - chatCommandService, auditRecorder, // 실제 구현을 물린다. 저장소 연결의 마지막 순서(preview 브랜치 준비 → 바인딩 → // 저장 → 감사)는 이 서비스로 옮겨갔을 뿐 동작이 바뀐 게 아니라, mock 으로 막으면 @@ -81,7 +80,8 @@ void setUp() { new RepositoryProvisioningService(githubRepositoryPort, projectRepository, auditRecorder), // 같은 이유로 실제 구현을 물린다. blank 로 시작하는 프로젝트는 확인할 템플릿이 // 없어 카탈로그를 건드리지 않는데, mock 으로 막으면 그 사실이 검증되지 않는다. - new TemplateCatalogGuard(templateCatalogPort) + new TemplateCatalogGuard(templateCatalogPort), + projectDeletionService ); } @@ -353,11 +353,14 @@ void disconnectRepository_thenConnectRepository_allowsReconnectingSameProject() void deleteProject_withRepositoryMode_deletesGithubRepositoryAndRecordsAudit() { Project project = boundProject(); when(projectRepository.findByIdAndOwnerUserIdAndDeletedFalse(11L, 1L)).thenReturn(Optional.of(project)); - when(projectRepository.save(any(Project.class))).thenAnswer(invocation -> invocation.getArgument(0)); projectCommandService.deleteProject(1L, 11L, com.example.dvely.project.application.command.dto.ProjectDeleteMode.PROJECT_AND_REPOSITORY); verify(githubRepositoryPort).deleteRepository(1L, "octo/repo"); + // 로컬 정리는 짧은 트랜잭션 하나로 묶여 ProjectDeletionService 가 맡는다(#337) — GitHub + // 삭제가 끝난 뒤에만 불린다. + verify(projectDeletionService).purge( + 1L, 11L, com.example.dvely.project.application.command.dto.ProjectDeleteMode.PROJECT_AND_REPOSITORY); // H3 (design §4): recorded right after the real GitHub deletion succeeds. ArgumentCaptor왜 이런 모양인가: 이 경로들은 Docker · GitHub · Cloudflare · AWS 를 기다리므로 실제로 + * 태우려면 그 외부가 전부 필요하다. 대신 Spring 이 런타임에 실제로 쓰는 바로 그 판정기 + * ({@link AnnotationTransactionAttributeSource} — 메서드와 클래스 레벨 애너테이션을 모두 본다) + * 에 직접 물어, 프록시가 이 메서드에 트랜잭션을 열지 않는다는 사실을 확인한다. 2026-09-08 + * dev 커넥션 풀 고갈의 재발 방지선이라, 누군가 편의로 {@code @Transactional} 을 다시 붙이면 + * 여기서 깨져야 한다.
+ * + *반대 방향도 함께 고정한다(아래 마지막 테스트) — 이 작업은 "트랜잭션을 걷어내는" 것이 + * 아니라 "외부 호출을 트랜잭션 밖으로 옮기는" 것이므로, 외부 호출이 없는 쓰기 경로는 여전히 + * 트랜잭션 안에 있어야 한다.
+ */ +class ExternalIoTransactionBoundaryTest { + + private final AnnotationTransactionAttributeSource attributeSource = + new AnnotationTransactionAttributeSource(); + + @Test + @DisplayName("A1 코딩 에이전트 실행 — 최대 10분짜리 CLI 실행이 트랜잭션 밖에 있다") + void codingAgentExecutionRunsOutsideTransaction() { + assertNoTransaction(CodingAgentExecutionService.class, "run"); + } + + @Test + @DisplayName("A2 배포 상태·로그 조회 — GitHub Actions 호출이 트랜잭션 밖에 있다") + void deploymentPollingRunsOutsideTransaction() { + assertNoTransaction(DeploymentQueryService.class, "getDeploymentStatus"); + assertNoTransaction(DeploymentQueryService.class, "getDeploymentLogs"); + } + + @Test + @DisplayName("A3 프로젝트 조회 — GitHub 을 타는 넷만 클래스 레벨 트랜잭션에서 빠져 있다") + void githubBackedProjectQueriesSuspendTheClassLevelTransaction() { + // 이 클래스는 클래스 레벨 @Transactional(readOnly) 라, 빠지려면 NOT_SUPPORTED 가 필요하다. + assertPropagation(ProjectQueryService.class, "getGithubRepositories", + TransactionDefinition.PROPAGATION_NOT_SUPPORTED); + assertPropagation(ProjectQueryService.class, "getOverview", + TransactionDefinition.PROPAGATION_NOT_SUPPORTED); + assertPropagation(ProjectQueryService.class, "getCommits", + TransactionDefinition.PROPAGATION_NOT_SUPPORTED); + assertPropagation(ProjectQueryService.class, "getRepositoryHealth", + TransactionDefinition.PROPAGATION_NOT_SUPPORTED); + + // 순수 DB 조회는 그대로 클래스 레벨 트랜잭션을 쓴다 — 걷어내기가 과하게 번지지 않았는지 본다. + assertPropagation(ProjectQueryService.class, "getProjects", + TransactionDefinition.PROPAGATION_REQUIRED); + assertPropagation(ProjectQueryService.class, "getActivityLogs", + TransactionDefinition.PROPAGATION_REQUIRED); + } + + @Test + @DisplayName("A4 도메인 바인딩 — Cloudflare·Pages·Thread.sleep 이 트랜잭션 밖에 있다") + void domainBindingRunsOutsideTransaction() { + assertNoTransaction(DomainBindingCommandService.class, "bindDomain"); + assertNoTransaction(DomainBindingCommandService.class, "checkVerification"); + assertNoTransaction(DomainBindingCommandService.class, "checkVerificationAsSystem"); + assertNoTransaction(DomainBindingCommandService.class, "deleteDomain"); // 2·3 인자 오버로드 모두 + assertNoTransaction(DomainBindingCommandService.class, "cleanupProjectS3Domains"); + assertNoTransaction(DomainBindingCommandService.class, "releaseServerDomains"); + } + + @Test + @DisplayName("A5 변경 내역 기록 — Docker exec(apk add git 포함)가 트랜잭션 밖에 있다") + void changeRecordRunsOutsideTransaction() { + assertNoTransaction(ChangeService.class, "record"); + } + + @Test + @DisplayName("A6 프리뷰 세션 — 컨테이너 제거·도달 확인이 트랜잭션 밖에 있다") + void previewSessionDockerWorkRunsOutsideTransaction() { + assertNoTransaction(PreviewSessionService.class, "markServing"); + assertNoTransaction(PreviewSessionService.class, "closeOwned"); + assertNoTransaction(PreviewSessionService.class, "closeAllOwned"); + assertNoTransaction(PreviewSessionService.class, "cleanupExpired"); + assertNoTransaction(PreviewSessionService.class, "reclaimUnreachable"); + } + + @Test + @DisplayName("A7 로그인·App 연동 — GitHub OAuth/User API 가 트랜잭션 밖에 있다") + void authGithubCallsRunOutsideTransaction() { + assertNoTransaction(AuthCommandService.class, "loginWithGithub"); + assertNoTransaction(AuthCommandService.class, "linkGithubApp"); + assertNoTransaction(AuthCommandService.class, "linkGithubAppByCode"); + assertNoTransaction(AuthCommandService.class, "refreshGithubUserToken"); + } + + @Test + @DisplayName("A8 프로젝트 생성·연결·삭제 — GitHub·템플릿 카탈로그 호출이 트랜잭션 밖에 있다") + void projectCommandGithubCallsRunOutsideTransaction() { + assertNoTransaction(ProjectCommandService.class, "createProject"); + assertNoTransaction(ProjectCommandService.class, "connectRepository"); + assertNoTransaction(ProjectCommandService.class, "deleteProject"); + } + + @Test + @DisplayName("외부 호출이 없는 쓰기 경로는 그대로 트랜잭션 안에 있다") + void writesWithoutExternalIoStayTransactional() { + // 삭제의 로컬 정리(대화 삭제 + 프로젝트 삭제)는 갈라지면 안 되므로 한 트랜잭션이어야 한다. + assertPropagation(ProjectDeletionService.class, "purge", + TransactionDefinition.PROPAGATION_REQUIRED); + assertPropagation(ProjectCommandService.class, "disconnectRepository", + TransactionDefinition.PROPAGATION_REQUIRED); + assertPropagation(ProjectCommandService.class, "updateProject", + TransactionDefinition.PROPAGATION_REQUIRED); + assertPropagation(DomainBindingCommandService.class, "abandonVerification", + TransactionDefinition.PROPAGATION_REQUIRED); + assertPropagation(AuthCommandService.class, "logout", + TransactionDefinition.PROPAGATION_REQUIRED); + } + + private void assertNoTransaction(Class> type, String methodName) { + for (Method method : publicMethods(type, methodName)) { + assertThat(attributeSource.getTransactionAttribute(method, type)) + .as("%s#%s 는 외부 I/O 를 기다리므로 트랜잭션을 열면 안 된다(#337)", + type.getSimpleName(), methodName) + .isNull(); + } + } + + private void assertPropagation(Class> type, String methodName, int expectedPropagation) { + for (Method method : publicMethods(type, methodName)) { + TransactionAttribute attribute = attributeSource.getTransactionAttribute(method, type); + assertThat(attribute) + .as("%s#%s 에는 트랜잭션이 있어야 한다", type.getSimpleName(), methodName) + .isNotNull(); + assertThat(attribute.getPropagationBehavior()) + .as("%s#%s 의 전파 속성", type.getSimpleName(), methodName) + .isEqualTo(expectedPropagation); + } + } + + /** 오버로드가 있으면 전부 본다 — 하나만 고쳐 두고 다른 쪽에 트랜잭션이 남는 실수를 막는다. */ + private List