Skip to content

Conversation

@rladmstn
Copy link

안녕하세요 짱구!

그동안 바빠서.. 미션을 한참 미루다가 이제 제출합니다..ㅎㅎ
이번엔 TransactionSynchronizationManager를 활용해 커넥션을 공유하도록 수정해보았습니다!

구현 내용에 대해 질문이나 피드백 부분 있다면 언제든 코멘트 부탁드려요! 감사합니다!

@rladmstn rladmstn self-assigned this Oct 26, 2025
@rladmstn rladmstn added the step4 label Oct 26, 2025
@rladmstn
Copy link
Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

Walkthrough

UserDao와 UserHistoryDao의 Connection 인자 오버로드가 제거되었습니다. UserService는 구현 클래스에서 인터페이스로 변경되었고, AppUserService와 트랜잭션 경계를 담당하는 TxUserService가 새로 추가되었습니다. JdbcTemplate은 per-call 연결 획득을 중단하고 DataSourceUtils로 연결을 취득·해제하도록 변경되었으며 해당 클래스의 update(Connection...) 오버로드가 제거되었습니다. DataSourceUtils.releaseConnection은 TransactionSynchronizationManager에 바인드된 리소스 존재 여부에 따라 연결을 조건부로 닫도록 변경되었고, TransactionSynchronizationManager는 ThreadLocal 맵 기반의 bind/unbind/existsResource API로 전환되었습니다.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed PR 제목은 주요 변경사항인 "Transaction synchronization 적용"을 명확하게 전달합니다. raw_summary에서 확인할 수 있듯이 TransactionSynchronizationManager를 활용한 커넥션 공유 메커니즘 구현, JdbcTemplate의 DataSourceUtils 기반 변경, 그리고 관련 DAO/Service 클래스들의 리팩토링이 핵심 변경사항이며, 제목이 이를 직접적으로 반영하고 있습니다. 다만 "피글렛(김은수) 미션 제출합니다"라는 부분은 미션 제출 상태를 나타내는 부가정보로, 기술적 변경사항보다는 행정적 정보입니다. 그럼에도 불구하고 대괄호 앞부분의 핵심 내용이 충분히 구체적이고 명확하여 동료들이 이력을 검토할 때 주요 변경사항을 이해할 수 있습니다.
Description Check ✅ Passed PR 설명은 변경사항과 명확하게 관련되어 있습니다. 작성자가 "TransactionSynchronizationManager를 활용해 커넥션을 공유하도록 수정했다"고 명시한 것이 raw_summary의 핵심 변경사항과 정확히 일치합니다. TransactionSynchronizationManager의 ThreadLocal 맵 기반 구현, JdbcTemplate과 DataSourceUtils의 변경, 그리고 DAO/Service 레이어의 Connection 파라미터 제거 등이 모두 이 목표를 달성하기 위한 것입니다. 설명이 다소 간단하지만 변경사항의 의도를 충분히 전달하고 있습니다.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f53bd9 and 8563bf0.

📒 Files selected for processing (1)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (2)

14-27: ThreadLocal 바인딩 전략이 트랜잭션 외부에서도 항상 활성화됩니다

현재 구현은 트랜잭션 여부와 무관하게 getConnection 호출 시 매번 ThreadLocal에 바인딩합니다. 반면 JdbcTemplate은 연결을 닫지 않으므로(아래 파일 참조) 트랜잭션 경계 밖에서 호출되면 커넥션이 ThreadLocal에 남아 풀 고갈로 이어질 수 있습니다.

  • 만약 TxUserService가 경계에서만 바인딩/해제를 책임지도록 의도했다면, getConnection이 “트랜잭션 활성 시에만 바인딩”하도록 설계하는 것이 합리적일 수 있습니다.
  • 또는 “참조 카운팅/소유권 플래그”를 두어 최초 획득자가 해제하도록 하는 방식도 고려해볼 수 있습니다.
  • 어떤 접근을 선택하든 “트랜잭션 외부 호출 시 커넥션 생명주기”를 먼저 정의해보면 어떨까요?

29-36: close 예외 시 unbind 누락 가능성과 진단 정보 손실

releaseConnection에서 close가 실패하면 unbind가 수행되지 않아 ThreadLocal에 리소스가 남을 수 있습니다. 또한 예외 래핑 시 원인(원인 예외, DataSource/SQL 상태 등)이 손실되어 진단이 어렵습니다.

  • close 성공/실패와 무관하게 자원 해제가 보장되도록 정리 블록을 분리하는 방식을 고려해보셨나요?
  • unbind 시 “해제 대상이 현재 묶인 리소스와 일치하는지” 검증 로직이 있으면 디버깅에 도움이 됩니다.
  • 실패 시 로그에 DataSource 식별자(예: hashCode 또는 name)와 cause를 남기면 운영 가시성이 좋아집니다.
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (2)

26-36: 커넥션 해제 경로 부재로 인한 누수 위험

DataSourceUtils.getConnection으로 획득하지만, update는 커넥션을 닫거나 해제하지 않습니다. 현재 DataSourceUtils가 항상 바인딩하기 때문에, 트랜잭션 경계 밖 호출 시 ThreadLocal에 남아 누수/풀 고갈을 유발할 수 있습니다.

  • “트랜잭션 활성 시에만 바인딩” 또는 “JdbcTemplate가 소유자일 때만 해제” 같은 명확한 규칙이 필요합니다.
  • 어떤 규칙을 채택할지 먼저 결정한 뒤, 호출자/경계(TxUserService)와의 책임을 재배치해보세요.

38-48: query 메서드에서 Connection 수동 해제 필요

DataSourceUtils 설계를 확인한 결과, getConnection()으로 얻은 Connection은 TransactionSynchronizationManager에 바인드되며, 명시적으로 releaseConnection(conn, dataSource)으로 해제해야 합니다.

현재 query 메서드는 try-with-resources가 PreparedStatement만 닫고 있어 Connection이 해제되지 않습니다. update 메서드도 동일한 문제를 가지고 있으며, 리포지토리 전체에서 releaseConnection() 호출이 없는 상태입니다.

두 메서드 모두 finally 블록이나 try-catch-finally 구조에서 DataSourceUtils.releaseConnection(conn, dataSource)를 호출하여 리소스를 명시적으로 해제해야 합니다.

🧹 Nitpick comments (16)
app/src/test/java/com/techcourse/service/UserServiceTest.java (2)

37-41: 서비스 합성은 명확합니다만, 경계 책임을 더 분명히 해보면 어떨까요?

AppUserService는 순수 애플리케이션 로직, TxUserService는 경계(트랜잭션) 역할로 분리되어 좋아 보입니다. 다만 “TxUserService만이 커넥션 생명주기를 책임진다”는 전제가 있다면, 이 테스트가 그러한 계약을 검증하는 단서를 더 포함해도 좋겠습니다(예: 예외 후 커넥션 해제가 보장되는지).


42-51: 트랜잭션 롤백 검증을 선택적으로 확장해보세요.

현재 테스트는 비밀번호 변경이 롤백되었음을 적절히 검증합니다. 추가로 user_history 테이블에 기록이 남지 않았음을 명시적으로 확인하면 테스트 의도가 더욱 명확해질 수 있습니다. 예를 들어:

// 트랜잭션 롤백 후, user_history 테이블이 비어있음을 확인
final var historyCount = jdbcTemplate.queryForObject(
    "select count(*) from user_history where user_id = 1", 
    Integer.class
);
assertThat(historyCount).isEqualTo(0);

다만 현재 MockUserHistoryDao가 예외를 발생시키므로 기록이 생성되지 않는 것이 보장되어 있어, 이 검증은 필수는 아닙니다.

jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (2)

10-10: ThreadLocal Map은 편리하지만, 풀링된 스레드 환경에서 누수 위험을 인지하고 계신가요?

스레드 풀에서 ThreadLocal은 경계 종료 시 명시적 정리가 필요합니다. 현재 clear/all‑unbind API가 없어 경계 누락 시 리소스가 남을 수 있습니다.

  • 트랜잭션 종료 훅(성공/실패 모두)에서 반드시 비우는 규칙을 두거나, 전용 clear 메서드를 제공하는 방식을 고려해보세요.
  • 운영 환경에서 디버깅을 돕기 위해 리소스 개수/키를 관찰할 수 있는 진단 포인트도 유용합니다.

14-24: API 계약의 엄격함을 높여보세요

get/bind/unbind는 null 키/값 허용 여부, 중복 바인딩시 정책(덮어쓰기 vs 예외), 미존재 unbind시 정책을 명확히 하는 편이 안전합니다. 명시적 계약이 있어야 상위 계층(TxUserService, JdbcTemplate)에서 예측 가능성이 올라갑니다.

app/src/main/java/com/techcourse/service/AppUserService.java (2)

28-34: 트랜잭션 경계 가정하에 일관성은 좋습니다만, 실패 시 일관성 규칙을 명문화해보면 어떨까요?

비밀번호 변경 → 업데이트 → 이력 로그 순서가 명확합니다. 다만 이력이 실패할 때 사용자 상태를 어떻게 할지(모두 롤백 vs 일부 성공 허용)가 서비스 계약의 중요한 부분이므로, “항상 원자성 보장(트랜잭션 필수)”을 계약으로 명시하면 사용자가 AppUserService를 직접 호출할 때의 오해를 줄일 수 있습니다.


29-34: 원시값 포장과 이름 일관성

  • 규칙 3(원시값 포장): id, newPassword, createdBy는 도메인 값 객체(예: UserId, Password, CreatedBy)로 감싸면 검증/의미 부여가 쉬워집니다.
  • 파라미터 이름(createBy vs createdBy) 일관성을 맞추면 의사소통 비용이 줄어듭니다.
app/src/main/java/com/techcourse/service/UserService.java (1)

5-9: 인터페이스로의 전환은 👍, 도메인 언어 다듬기 제안

  • save/changePassword/findById라는 행동 중심 인터페이스로 단순·명확해졌습니다.
  • 규칙 3(원시값 포장): id, password, createdBy를 값 객체로 승격하면 유효성·의미가 강화됩니다.
  • 명명: save vs insert의 의미 차이를 팀 내에서 합의해두면(등록/수정/갱신의 경계) 구현체 교체 시 혼란을 줄일 수 있습니다.
app/src/main/java/com/techcourse/service/TxUserService.java (9)

13-13: 클래스 확장 가능성 점검 (불변 트랜잭션 규약 보호)

트랜잭션 경계를 다루는 데코레이터는 규약이 흔들리면 치명적입니다. 의도적으로 확장 가능하게 두셨나요? 아니라면 상속을 막아 트랜잭션 규칙을 고정하는 방향을 고려해보세요.


30-33: save의 트랜잭션 경계 정의가 불명확합니다

작성(쓰기) 작업을 명시적 트랜잭션으로 감싸지 않았습니다. 의도인가요? “쓰기 작업은 모두 트랜잭션”이라는 정책으로 일관시키면 사고를 줄일 수 있습니다. 어떤 기준으로 changePassword만 트랜잭션 처리했는지 근거를 명확히 해보세요.


36-44: 예외 매핑 일관성 확보 필요

doInTransaction 내부에서는 대부분 RuntimeException으로 전환되지만, 여기서는 SQLException만 DataAccessException으로 래핑합니다. setAutoCommit/commit/rollback 등 어느 경로에서 어떤 예외가 외부로 전달되는지 “서비스 경계에서의 단일 예외 정책”을 정해 통일하세요. 테스트로 커밋 실패/롤백 실패/비즈니스 예외 각각의 매핑을 검증해보면 좋습니다.


46-58: 중첩 트랜잭션(참여/전파) 시나리오 고려

이미 같은 DataSource에 커넥션이 바인딩되어 있다면 “새 트랜잭션 시작자”가 아닐 수 있습니다. 이 경우 커밋/언바인드 책임을 지면 외부 범위가 깨집니다. “내가 시작한 트랜잭션인지”를 구분해 참여만 할지, 커밋/해제까지 할지 결정하는 로직을 도입해보셨나요?


52-54: 로그에 예외 스택 포함 필요

에러 로그가 원인(throwable)을 포함하지 않아 추적이 어렵습니다. 메시지와 함께 예외 객체를 전달해 디버깅 가능성을 높여보세요.


46-58: 메서드 길이·들여쓰기 규칙 준수

doInTransaction가 10줄을 넘고(규칙 6), 내부 중첩으로 들여쓰기 depth가 커지기 쉽습니다(규칙 1). “트랜잭션 시작/실행/커밋/예외-롤백/정리”를 작은 메서드로 분리하거나 가드 클로즈를 적용해 복잡도를 낮춰보세요.


36-36: 원시값 포장 고려 (Primitive Obsession 완화)

id, newPassword, createdBy를 도메인 값 객체(UserId, Password, Auditor 등)로 감싸면 검증, 로깅, 책임 배분이 명확해집니다. 특히 Password는 생성 시 정책 검증·마스킹 책임을 부여할 수 있습니다.


38-41: Consumer 시그니처의 목적성 점검

람다에서 커넥션 파라미터를 사용하지 않습니다. 실제로 연결이 필요 없다면 더 단순한 실행 인터페이스로 축소하거나, 필요하다면 내부 서비스가 명시적으로 해당 커넥션을 활용하도록 책임을 정리해보세요.

Also applies to: 46-46


1-80: 재사용 가능한 트랜잭션 템플릿 도입 검토

서비스별로 트랜잭션 보일러플레이트가 반복될 가능성이 큽니다. 전용 TransactionTemplate/Callback 스타일로 추상화하면 정책(전파, 예외 매핑, 로깅, 자원 관리)을 한곳에서 통제할 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5db4981 and 4ad8d14.

📒 Files selected for processing (10)
  • app/src/main/java/com/techcourse/dao/UserDao.java (0 hunks)
  • app/src/main/java/com/techcourse/dao/UserHistoryDao.java (0 hunks)
  • app/src/main/java/com/techcourse/service/AppUserService.java (1 hunks)
  • app/src/main/java/com/techcourse/service/TxUserService.java (1 hunks)
  • app/src/main/java/com/techcourse/service/UserService.java (1 hunks)
  • app/src/test/java/com/techcourse/service/MockUserHistoryDao.java (1 hunks)
  • app/src/test/java/com/techcourse/service/UserServiceTest.java (1 hunks)
  • jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (3 hunks)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (2 hunks)
  • jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1 hunks)
💤 Files with no reviewable changes (2)
  • app/src/main/java/com/techcourse/dao/UserDao.java
  • app/src/main/java/com/techcourse/dao/UserHistoryDao.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인

이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.

📚 학습 원칙

  • 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
  • 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
  • 작은 단계로 문제를 분해하여 접근하도록 도와주세요

💡 피드백 방법

  • 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
  • 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
  • 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
  • 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록

⚡ 객체지향 생활 체조 원칙 검토

다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:

규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만

  • 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
    • 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
    • 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"

규칙 2: else 예약어 금지

  • else, switch/case 사용 금지
    • 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
    • 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"

규칙 3: 모든 원시값과 문자열 포장

  • 원시 타입과 String을 객체로 포장
    • 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
    • 💡 예시: int portPort port, String nameName name

규칙 4: 한 줄에 점 하나만 (디미터 법칙)

  • 메서드 체이닝 제한
    • 📖 이유: 결합도 감소, 캡슐화 향상
    • 💡 나쁜 예: request.getUri().getPath().substring()
    • 💡 좋은 예: request.extractPath()

규칙 5: 축약 금지

  • 명확한 이름 사용 (축약어 금지)
    • 📖 이유: 코드 가독성, 의도 명확화
    • 💡 예시: reqrequest, calcAmtcalculateAmount

규칙 6: 모든 엔티티를 작게 유지

  • 클래스 50줄, 메서드 10줄 이하
    • 📖 이유: 단일 책임, 이해와 테스트 용이성
    • 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"

규칙 7: 인스턴스 변수 3개 이하

  • 클래스당 최대 3개의 인스턴스 변수
    • 📖 이유: 높은 응집도, 단일 책임 유지
    • 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"

규칙 8: 일급 컬렉션 사용

  • 컬렉션을 감싸는 클래스 사용
    • 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
    • 💡 예시: List<HttpHeader>HttpHeaders 클래스

규칙 9: 게터/세터/프로퍼티 금지

  • Tell, Don't Ask 원칙 준수
    • 📖 이유: 진정한 캡슐화, 객체 자율성
    • 💡 나쁜 예: if (user.getPassword().equals(input))
    • 💡 좋은 예: if (user.authenticate(input))

🎯 추가 프로그래밍 요구사항

코드 스타일

  • Java Style Guide 준수
  • 3항 연산자 사용 금지

테스트 및 구조

  • **모든 기능 T...

Files:

  • app/src/test/java/com/techcourse/service/UserServiceTest.java
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java
  • app/src/main/java/com/techcourse/service/AppUserService.java
  • jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java
  • app/src/test/java/com/techcourse/service/MockUserHistoryDao.java
  • app/src/main/java/com/techcourse/service/TxUserService.java
  • app/src/main/java/com/techcourse/service/UserService.java
  • jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java
🧬 Code graph analysis (5)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1)
  • TransactionSynchronizationManager (8-25)
app/src/main/java/com/techcourse/service/AppUserService.java (1)
app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
  • UserHistoryDao (8-29)
app/src/main/java/com/techcourse/service/TxUserService.java (1)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
  • DataSourceUtils (10-37)
app/src/main/java/com/techcourse/service/UserService.java (1)
study/src/test/java/aop/stage1/UserService.java (1)
  • UserService (9-36)
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (1)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
  • DataSourceUtils (10-37)
🔇 Additional comments (2)
app/src/test/java/com/techcourse/service/MockUserHistoryDao.java (1)

15-17: 목적이 분명한 테스트 더블로 적절합니다

예외를 강제로 던져 롤백 경로를 검증하려는 의도가 명확합니다. 해당 예외 타입/메시지가 상위 계층에서 기대하는 것과 일치하는지만 한 번 더 확인해보세요(메시지 어서션 등).

app/src/main/java/com/techcourse/service/TxUserService.java (1)

25-28: 읽기 작업 위임은 적절해 보입니다

조회 경로에 트랜잭션을 강제하지 않는 선택은 합리적입니다. 테스트로 조회가 기존 트랜잭션 존재 시 동일 커넥션을 공유하는지 확인만 부탁드립니다.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
app/src/main/java/com/techcourse/service/TxUserService.java (1)

69-69: 규칙 위반: 3항 연산자 사용 금지

예외 재던지기에서 3항 연산자를 사용하고 있습니다. 가드(early return/throw) 형태로 단순화하면 가독성과 정책 명시성이 올라갑니다. 동시에 어떤 예외를 어떤 런타임 예외로 전환할지 정책을 메서드명/타입으로 드러내보세요. As per coding guidelines.

🧹 Nitpick comments (5)
app/src/main/java/com/techcourse/service/TxUserService.java (5)

53-54: 로그에 예외 객체 포함 및 레벨 정합성

“트랜잭션 수행 중 예외 발생” 로그에 throwable이 누락되어 스택트레이스를 잃습니다. 롤백 성공 로그는 info보다 debug가 더 적절할 수 있습니다(운영 소음 최소화). 실패는 error로 유지하세요. As per coding guidelines.


46-51: 설계: Consumer은 JDBC 세부를 API 표면으로 드러냅니다

현재 호출자는 커넥션을 사용하지 않는데도 시그니처가 노출되어 있습니다. 서비스 경계를 인프라로부터 분리하기 위해 실행 단위를 Runnable 또는 반환값을 허용하는 함수형 타입(값을 반환해야 한다면)으로 추상화하는 방식을 고려해보세요. 트랜잭션 템플릿(예: executeInTransaction) 형태로 표현하면 의도가 더 명확해집니다. As per coding guidelines.


36-40: 규칙 3: 원시값 포장 고려

id, newPassword, createdBy가 원시/문자열로 노출됩니다. 도메인 개념(사용자 식별자, 비밀번호 정책, 변경 주체)을 값 객체로 포장하면 유효성, 포맷, 정책을 한 곳에 응집할 수 있습니다. As per coding guidelines.


25-33: 트랜잭션 범위 의도 확인

findById/save가 트랜잭션 밖에서 동작합니다. 읽기 전용 트랜잭션(격리수준 포함) 또는 동일 서비스 단위 내 불변성 보장을 의도했다면, 범위를 어떻게 잡을지 검토해볼까요? 읽기 전용 힌트/플래그를 부여하면 의도가 더 명확해질 수 있습니다. As per coding guidelines.


46-70: 규칙 6: 클래스/메서드 크기 축소

클래스가 50줄을 넘고, doInTransaction/handleTransactionException도 10줄을 초과합니다. 롤백 처리/예외 전환/리소스 해제를 작은 메서드(또는 별도 협력자, 예: TransactionTemplate)로 분리하면 응집도가 올라가고 테스트가 쉬워집니다. As per coding guidelines.

Also applies to: 72-76

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ad8d14 and cc99149.

📒 Files selected for processing (1)
  • app/src/main/java/com/techcourse/service/TxUserService.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인

이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.

📚 학습 원칙

  • 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
  • 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
  • 작은 단계로 문제를 분해하여 접근하도록 도와주세요

💡 피드백 방법

  • 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
  • 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
  • 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
  • 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록

⚡ 객체지향 생활 체조 원칙 검토

다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:

규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만

  • 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
    • 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
    • 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"

규칙 2: else 예약어 금지

  • else, switch/case 사용 금지
    • 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
    • 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"

규칙 3: 모든 원시값과 문자열 포장

  • 원시 타입과 String을 객체로 포장
    • 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
    • 💡 예시: int portPort port, String nameName name

규칙 4: 한 줄에 점 하나만 (디미터 법칙)

  • 메서드 체이닝 제한
    • 📖 이유: 결합도 감소, 캡슐화 향상
    • 💡 나쁜 예: request.getUri().getPath().substring()
    • 💡 좋은 예: request.extractPath()

규칙 5: 축약 금지

  • 명확한 이름 사용 (축약어 금지)
    • 📖 이유: 코드 가독성, 의도 명확화
    • 💡 예시: reqrequest, calcAmtcalculateAmount

규칙 6: 모든 엔티티를 작게 유지

  • 클래스 50줄, 메서드 10줄 이하
    • 📖 이유: 단일 책임, 이해와 테스트 용이성
    • 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"

규칙 7: 인스턴스 변수 3개 이하

  • 클래스당 최대 3개의 인스턴스 변수
    • 📖 이유: 높은 응집도, 단일 책임 유지
    • 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"

규칙 8: 일급 컬렉션 사용

  • 컬렉션을 감싸는 클래스 사용
    • 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
    • 💡 예시: List<HttpHeader>HttpHeaders 클래스

규칙 9: 게터/세터/프로퍼티 금지

  • Tell, Don't Ask 원칙 준수
    • 📖 이유: 진정한 캡슐화, 객체 자율성
    • 💡 나쁜 예: if (user.getPassword().equals(input))
    • 💡 좋은 예: if (user.authenticate(input))

🎯 추가 프로그래밍 요구사항

코드 스타일

  • Java Style Guide 준수
  • 3항 연산자 사용 금지

테스트 및 구조

  • **모든 기능 T...

Files:

  • app/src/main/java/com/techcourse/service/TxUserService.java
🧬 Code graph analysis (1)
app/src/main/java/com/techcourse/service/TxUserService.java (1)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
  • DataSourceUtils (10-37)
🔇 Additional comments (1)
app/src/main/java/com/techcourse/service/TxUserService.java (1)

72-76: 좋은 수정: 해제 책임 단일화 확인

closeConnection에서 DataSourceUtils.releaseConnection만 호출하도록 단순화된 점 좋습니다. 이중 종료/언바인드 위험이 줄었습니다. As per coding guidelines.

Comment on lines +36 to +44
public void changePassword(final long id, final String newPassword, final String createdBy){
try {
doInTransaction(connection -> {
userService.changePassword(id, newPassword, createdBy);
});
} catch (SQLException e) {
throw new DataAccessException(e);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

예외 전파/전환 정책 불일치 — 현재 SQLException 전환 경로가 사실상 작동하지 않음

doInTransaction 내부에서 모든 예외를 잡아 RuntimeException으로 재던지므로, changePassword의 catch(SQLException)은 도달하지 않습니다. 예외 전환 정책을 한 곳으로 모아 일관되게 유지하세요. 선택지는 대략 두 가지입니다:

  • 트랜잭션 경계(doInTransaction)에서 표준 런타임 예외(예: DataAccessException 파생)로 전환해 상위는 그대로 전파받게 하거나,
  • doInTransaction이 체크 예외를 전달하고, 퍼블릭 서비스 계층에서만 DataAccessException으로 일괄 전환.

현재 구조에 맞는 하나의 정책을 정하고 전역으로 일치시켜보세요. As per coding guidelines.

Also applies to: 46-58


예외 처리 정책 일관성을 검증하기 위해 저장소를 탐색하겠습니다.


---

🏁 Script executed:

```shell
#!/bin/bash
# Find TxUserService and related transaction handler files
fd -e java -E target -E build | head -20

Length of output: 1262


🏁 Script executed:

#!/bin/bash
# Look at the TxUserService file structure
find . -name "TxUserService.java" -type f 2>/dev/null

Length of output: 114


🏁 Script executed:

#!/bin/bash
# Search for doInTransaction method implementation
rg -n "doInTransaction" --type java -A 10 -B 2

Length of output: 2147


🏁 Script executed:

#!/bin/bash
# Get the complete TxUserService file to understand full exception handling pattern
rg -n "class TxUserService" --type java -A 50 2>/dev/null | head -80

Length of output: 7096



---

🏁 Script executed:

```shell
#!/bin/bash
# Get full TxUserService.java file
cat ./app/src/main/java/com/techcourse/service/TxUserService.java

Length of output: 2415


🏁 Script executed:

#!/bin/bash
# Search for handleTransactionException method
rg -n "handleTransactionException" --type java -A 5 -B 2

Length of output: 1377


🏁 Script executed:

#!/bin/bash
# Search for closeConnection to understand complete flow
rg -n "closeConnection" --type java -A 5 -B 2

Length of output: 1444


예외 전파 정책 불일치 확인됨 — changePassword의 SQLException 캐치 블록은 도달 불가능

현재 코드의 문제점:

  1. handleTransactionException (60줄)에서 모든 예외를 RuntimeException으로 변환하여 던짐

    • throw e instanceof RuntimeException ? (RuntimeException) e : new RuntimeException(e);
  2. doInTransaction은 "throws SQLException" 선언하지만 실제로는 RuntimeException만 발생

    • 선언과 구현의 불일치
  3. changePassword의 catch(SQLException e) 블록 (41줄)은 실행되지 않음

    • RuntimeException은 SQLException이 아니므로 캐치되지 않음

선택할 수 있는 일관된 정책:

  • 옵션 1: 트랜잭션 경계(doInTransaction)에서만 예외 전환 → doInTransaction이 throws SQLException 선언 제거하고 RuntimeException만 전파하도록 수정
  • 옵션 2: 퍼블릭 메서드 계층에서만 예외 전환 → changePassword와 모든 공개 메서드에서 일관되게 처리

하나의 정책을 선택하여 doInTransaction, handleTransactionException, changePassword 간 예외 처리 흐름을 일관되게 재설계하세요.

🤖 Prompt for AI Agents
In app/src/main/java/com/techcourse/service/TxUserService.java around lines 36
to 44, the current catch(SQLException) in changePassword is unreachable because
doInTransaction/handleTransactionException convert exceptions to
RuntimeException; choose the consistent policy of propagating RuntimeException
only: remove the "throws SQLException" from doInTransaction signature and ensure
handleTransactionException continues converting to RuntimeException, then update
changePassword to stop catching SQLException and instead catch RuntimeException
(or let it propagate) and wrap it in DataAccessException so public API surfaces
DataAccessException consistently.

Copy link
Member

@dlfmadlanjwl dlfmadlanjwl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 피글렛! 마지막 단계도 잘 구현해주셨네요 👍
코멘트 확인하시고 궁금한 점 있으면 남겨주세요!

*/
public void update(Connection connection, String sql, Object... params){
try (PreparedStatement pstmt = connection.prepareStatement(sql)){
Connection conn = DataSourceUtils.getConnection(dataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

트랜잭션 내부에서 실행되는 쿼리가 아닐 경우에는 커넥션을 닫아주지 못할 것 같아요!
트랜잭션 여부에 따라서 커넥션을 닫아주는 로직을 추가하는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇네요!!
짱구 말대로 finally에 DataSourceUtils.releaseConnection()을 추가해주는게 좋을 것 같아서 추가했습니다!
좋은 피드백 감사합니다!!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다 👍

public static void releaseConnection(Connection connection, DataSource dataSource) {
try {
connection.close();
TransactionSynchronizationManager.unbindResource(dataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connection.close() 도중에 예외가 발생하면 커넥션을 unbind할 수 없을 것 같습니다!
TransactionSynchronizationManager.unbindResource(dataSource)가 먼저 실행되게 하는게 좋을 것 같은데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

놓친 부분 꼼꼼하게 피드백 해주셔서 감사해요!!

다시 고민해보니 트랜잭션에 대해 unbind 하는 역할은 (TransactionSynchronizationManager.unbindResource(dataSource)) TxUserService에 위임하는 것이 적절할 것 같아서 이동했습니다!

fix: 트랜잭션 존재 여부 판단을 DataSourceUtils에서 하도록 위임

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다 👍

다만, 트랜잭션 외부에서 실행되는 쿼리는 TransactionSynchronizationManager.unbindResource()가 되지 않고 있어요. 따라서 DataSourceUtils.releaseConnection()에서 커넥션이 닫히지 않는 것 같습니다.
한 번 확인해보셔도 좋을 것 같아요!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc99149 and 8c312a2.

📒 Files selected for processing (2)
  • jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (2 hunks)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인

이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.

📚 학습 원칙

  • 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
  • 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
  • 작은 단계로 문제를 분해하여 접근하도록 도와주세요

💡 피드백 방법

  • 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
  • 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
  • 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
  • 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록

⚡ 객체지향 생활 체조 원칙 검토

다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:

규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만

  • 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
    • 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
    • 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"

규칙 2: else 예약어 금지

  • else, switch/case 사용 금지
    • 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
    • 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"

규칙 3: 모든 원시값과 문자열 포장

  • 원시 타입과 String을 객체로 포장
    • 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
    • 💡 예시: int portPort port, String nameName name

규칙 4: 한 줄에 점 하나만 (디미터 법칙)

  • 메서드 체이닝 제한
    • 📖 이유: 결합도 감소, 캡슐화 향상
    • 💡 나쁜 예: request.getUri().getPath().substring()
    • 💡 좋은 예: request.extractPath()

규칙 5: 축약 금지

  • 명확한 이름 사용 (축약어 금지)
    • 📖 이유: 코드 가독성, 의도 명확화
    • 💡 예시: reqrequest, calcAmtcalculateAmount

규칙 6: 모든 엔티티를 작게 유지

  • 클래스 50줄, 메서드 10줄 이하
    • 📖 이유: 단일 책임, 이해와 테스트 용이성
    • 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"

규칙 7: 인스턴스 변수 3개 이하

  • 클래스당 최대 3개의 인스턴스 변수
    • 📖 이유: 높은 응집도, 단일 책임 유지
    • 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"

규칙 8: 일급 컬렉션 사용

  • 컬렉션을 감싸는 클래스 사용
    • 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
    • 💡 예시: List<HttpHeader>HttpHeaders 클래스

규칙 9: 게터/세터/프로퍼티 금지

  • Tell, Don't Ask 원칙 준수
    • 📖 이유: 진정한 캡슐화, 객체 자율성
    • 💡 나쁜 예: if (user.getPassword().equals(input))
    • 💡 좋은 예: if (user.authenticate(input))

🎯 추가 프로그래밍 요구사항

코드 스타일

  • Java Style Guide 준수
  • 3항 연산자 사용 금지

테스트 및 구조

  • **모든 기능 T...

Files:

  • jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: egaeng09
PR: woowacourse/java-jdbc#1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
🧬 Code graph analysis (1)
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (1)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
  • DataSourceUtils (10-37)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)

29-37: 치명적: 커넥션 해제 조건이 반대로 구현되어 있습니다

현재 로직은 리소스가 존재할 때 커넥션을 닫고 있는데, 이는 정반대입니다.

문제점:

  • 트랜잭션 진행 중(리소스 존재): 커넥션을 닫아버림 → 트랜잭션 실패
  • 트랜잭션 종료 후(리소스 미존재): 커넥션을 닫지 않음 → 커넥션 누수

동작 흐름:

  1. TxUserService에서 트랜잭션 시작 → getConnection()으로 커넥션 생성 및 바인딩
  2. 트랜잭션 진행 중 → JdbcTemplatereleaseConnection() 호출 → 현재 코드는 리소스가 존재하므로 close() 실행 → ❌ 트랜잭션이 아직 진행 중인데 커넥션이 닫힘
  3. 트랜잭션 종료 → unbindResource()releaseConnection() 호출 → 현재 코드는 리소스가 없으므로 close() 미실행 → ❌ 커넥션이 닫히지 않고 누수 발생

조건문을 !existsResource(dataSource)로 수정해주세요. 트랜잭션 컨텍스트에 리소스가 없을 때만 커넥션을 닫아야 합니다.

♻️ Duplicate comments (1)
app/src/main/java/com/techcourse/service/TxUserService.java (1)

37-76: 이전 리뷰 코멘트의 주요 이슈들이 아직 해결되지 않았습니다

다음 이슈들이 여전히 남아있어 확인이 필요합니다:

  1. Line 42: SQLException catch 블록이 도달 불가능합니다. handleTransactionException이 모든 예외를 RuntimeException으로 변환하여 던지기 때문입니다.

  2. Line 50: connection.setAutoCommit(false) 실행 후 원래 상태를 복원하지 않습니다. 커넥션 풀에 반환될 때 다음 사용자가 잘못된 상태를 물려받을 수 있습니다.

  3. Lines 47-59: 중첩 트랜잭션(이미 바인딩된 커넥션) 상황에서 현재 메서드가 commit/rollback/close를 수행하면 외부 트랜잭션을 손상시킬 수 있습니다. 커넥션 획득 시점에 이미 바인딩 여부를 확인해 "소유권"을 구분하는 방식을 고려해보세요.

  4. Lines 56-58: finally 블록에서 releaseConnection 실패 시 원래 발생한 비즈니스 예외가 가려질 수 있습니다.

이전 리뷰어분들께서 상세히 설명해주신 내용들을 참고하여 수정해주세요.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c312a2 and 8f53bd9.

📒 Files selected for processing (3)
  • app/src/main/java/com/techcourse/service/TxUserService.java (1 hunks)
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (2 hunks)
  • jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인

이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.

📚 학습 원칙

  • 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
  • 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
  • 작은 단계로 문제를 분해하여 접근하도록 도와주세요

💡 피드백 방법

  • 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
  • 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
  • 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
  • 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록

⚡ 객체지향 생활 체조 원칙 검토

다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:

규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만

  • 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
    • 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
    • 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"

규칙 2: else 예약어 금지

  • else, switch/case 사용 금지
    • 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
    • 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"

규칙 3: 모든 원시값과 문자열 포장

  • 원시 타입과 String을 객체로 포장
    • 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
    • 💡 예시: int portPort port, String nameName name

규칙 4: 한 줄에 점 하나만 (디미터 법칙)

  • 메서드 체이닝 제한
    • 📖 이유: 결합도 감소, 캡슐화 향상
    • 💡 나쁜 예: request.getUri().getPath().substring()
    • 💡 좋은 예: request.extractPath()

규칙 5: 축약 금지

  • 명확한 이름 사용 (축약어 금지)
    • 📖 이유: 코드 가독성, 의도 명확화
    • 💡 예시: reqrequest, calcAmtcalculateAmount

규칙 6: 모든 엔티티를 작게 유지

  • 클래스 50줄, 메서드 10줄 이하
    • 📖 이유: 단일 책임, 이해와 테스트 용이성
    • 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"

규칙 7: 인스턴스 변수 3개 이하

  • 클래스당 최대 3개의 인스턴스 변수
    • 📖 이유: 높은 응집도, 단일 책임 유지
    • 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"

규칙 8: 일급 컬렉션 사용

  • 컬렉션을 감싸는 클래스 사용
    • 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
    • 💡 예시: List<HttpHeader>HttpHeaders 클래스

규칙 9: 게터/세터/프로퍼티 금지

  • Tell, Don't Ask 원칙 준수
    • 📖 이유: 진정한 캡슐화, 객체 자율성
    • 💡 나쁜 예: if (user.getPassword().equals(input))
    • 💡 좋은 예: if (user.authenticate(input))

🎯 추가 프로그래밍 요구사항

코드 스타일

  • Java Style Guide 준수
  • 3항 연산자 사용 금지

테스트 및 구조

  • **모든 기능 T...

Files:

  • app/src/main/java/com/techcourse/service/TxUserService.java
  • jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java
  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: egaeng09
PR: woowacourse/java-jdbc#1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
📚 Learning: 2025-10-21T09:53:41.119Z
Learnt from: egaeng09
PR: woowacourse/java-jdbc#1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.

Applied to files:

  • jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java
🧬 Code graph analysis (2)
app/src/main/java/com/techcourse/service/TxUserService.java (2)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
  • DataSourceUtils (10-38)
jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1)
  • TransactionSynchronizationManager (8-29)
jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java (1)
jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1)
  • TransactionSynchronizationManager (8-29)
🔇 Additional comments (2)
jdbc/src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java (1)

10-28: 잘 구현하셨습니다!

ThreadLocal 기반 리소스 관리가 올바르게 구현되었습니다. withInitial(HashMap::new)를 사용해 NPE를 방지하고, 각 메서드가 명확한 책임을 가지고 있네요. existsResource 메서드 추가로 리소스 바인딩 여부를 확인할 수 있어 좋습니다.

app/src/main/java/com/techcourse/service/TxUserService.java (1)

21-34: 위임 패턴 적용이 명확합니다

생성자와 위임 메서드들이 깔끔하게 구현되어 있습니다. Decorator 패턴을 활용해 트랜잭션 관심사를 분리한 설계가 좋습니다.

Copy link
Member

@dlfmadlanjwl dlfmadlanjwl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 피글렛! 피드백 잘 반영해주신 것 확인했습니다
코멘트 하나 확인 부탁드리고, 마지막 미션 고생하셨습니다 👍

*/
public void update(Connection connection, String sql, Object... params){
try (PreparedStatement pstmt = connection.prepareStatement(sql)){
Connection conn = DataSourceUtils.getConnection(dataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다 👍

public static void releaseConnection(Connection connection, DataSource dataSource) {
try {
connection.close();
TransactionSynchronizationManager.unbindResource(dataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다 👍

다만, 트랜잭션 외부에서 실행되는 쿼리는 TransactionSynchronizationManager.unbindResource()가 되지 않고 있어요. 따라서 DataSourceUtils.releaseConnection()에서 커넥션이 닫히지 않는 것 같습니다.
한 번 확인해보셔도 좋을 것 같아요!

@dlfmadlanjwl dlfmadlanjwl merged commit fedbb80 into woowacourse:rladmstn Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants