-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor/#36 모듈 의존성 정리 #57
Conversation
개요워크스루이 풀 리퀘스트는 프로젝트의 의존성 관리 구조를 재설계하는 작업을 포함합니다. 주요 변경 사항은 루트 변경 사항
평가 (연결된 이슈 대비)
관련 가능성 있는 PR
시퀀스 다이어그램sequenceDiagram
participant Client
participant NoiseService
participant NoiseRepository
participant JpaNoiseRepository
participant QueryNoiseRepository
Client->>NoiseService: 요청
NoiseService->>NoiseRepository: getNearbyNoises(point)
NoiseRepository->>JpaNoiseRepository: 데이터 조회
NoiseRepository->>QueryNoiseRepository: 지리적 근접성 쿼리
NoiseRepository-->>NoiseService: 결과 반환
NoiseService-->>Client: 응답
시 (토끼의 노래)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🔭 Outside diff range comments (3)
soridam-api/build.gradle (1)
Line range hint
8-26
: 중복 의존성 제거 필요현재 많은 의존성들이 하위 모듈에서 이미 포함되어 있어 중복 선언되어 있습니다:
- JPA, Security 의존성은 이미 domain 모듈을 통해 제공됩니다
- JTS 의존성은 global-util 모듈을 통해 제공됩니다
- Swagger 의존성은 이미 infra 모듈에 포함되어 있습니다
불필요한 중복을 제거하고 하위 모듈의 의존성을 활용하세요.
soridam-common/build.gradle (1)
Line range hint
8-25
: 공통 모듈의 계층 구조 위반현재 구조에서 심각한 아키텍처 문제가 있습니다:
- 공통 모듈이 인프라 모듈에 의존하는 것은 계층 구조를 위반합니다
- 프레임워크 의존성(JPA, Web)은 공통 모듈이 아닌 상위 계층에서 관리되어야 합니다
- Querydsl 관련 설정은 영속성 계층으로 이동해야 합니다
제안사항:
- 공통 모듈은 순수 Java 코드만 포함하도록 리팩토링
- 프레임워크 의존성은 도메인 또는 인프라 계층으로 이동
- infra 모듈에 대한 의존성 제거
soridam-api/src/main/java/sorisoop/soridam/api/noise/application/NoiseService.java (1)
Line range hint
91-104
: createNoise 메서드의 심각한 문제점이 있습니다.
- 하드코딩된 사용자 ID (
"qnpranpswnps12@"
)가 있습니다.- 소음 생성 시 입력값 유효성 검증이 누락되었습니다.
다음과 같은 수정을 제안드립니다:
@Transactional - public NoisePersistResponse createNoise(NoiseCreateRequest request) { - User user = userService.getById(USER.getPrefix() + "qnpranpswnps12@"); + public NoisePersistResponse createNoise(String userId, NoiseCreateRequest request) { + validateNoiseRequest(request); + User user = userService.getById(USER.getPrefix() + userId); Point point = createPoint(request.x(), request.y()); Noise noise = Noise.create(
🧹 Nitpick comments (9)
soridam-global-util/build.gradle (1)
9-21
: 전역 유틸리티 모듈의 의존성 재검토 필요유틸리티 모듈은 프레임워크나 인프라에 독립적이어야 하는데, 현재 Spring Security, JPA, Web 등 프레임워크 의존성이 포함되어 있습니다. 이는 클린 아키텍처 원칙에 위배됩니다.
다음 사항들을 고려해주세요:
- 프레임워크 의존성을 제거하고 순수 Java 유틸리티로 구현
- 지리 데이터 관련 기능이 필요하다면 별도의 모듈로 분리
soridam-infra/build.gradle (1)
8-23
: 인프라 모듈의 의존성 구성이 적절합니다외부 프레임워크 통합을 담당하는 인프라 계층의 특성에 맞게 의존성이 잘 구성되어 있습니다.
제안사항:
- 프로젝트 전반에서 사용되는 공통 라이브러리 버전은 root build.gradle에서 중앙 관리하는 것을 고려해보세요.
soridam-auth/build.gradle (1)
Line range hint
9-24
: 인증 모듈의 의존성 구조 개선 필요현재 구조에서 몇 가지 고려사항이 있습니다:
- 인증 모듈이 도메인 모듈에 의존하는 것은 의존성 역전 원칙에 위배될 수 있습니다. 대신 인증 관련 인터페이스를 도메인에 정의하고, 구현을 인증 모듈에서 하는 방식을 고려해보세요.
- Swagger 의존성은 API 문서화를 위한 것이므로 API 모듈로 이동하는 것이 좋습니다.
soridam-domain/build.gradle (2)
23-24
: PostgreSQL 버전 명시 권장PostgreSQL 드라이버의 버전이 명시되어 있지 않습니다. 특정 버전을 지정하여 예기치 않은 업데이트로 인한 문제를 방지하는 것이 좋습니다.
다음과 같이 버전을 명시하는 것을 권장드립니다:
- runtimeOnly 'org.postgresql:postgresql' + runtimeOnly 'org.postgresql:postgresql:42.7.1'
26-30
: 보안 및 JPA 의존성의 모듈 분리 검토 필요domain 모듈에 Spring Security와 JPA 의존성을 직접 추가하는 것이 적절한지 검토가 필요합니다. domain 모듈은 순수한 도메인 로직만 포함하는 것이 좋습니다.
다음 사항들을 고려해보세요:
- Spring Security 의존성은 api 모듈로 이동
- JPA 관련 설정은 infra 모듈로 이동
- domain 모듈은 순수 Java 의존성만 유지
soridam-domain/src/main/java/sorisoop/soridam/domain/noise/domain/NoiseRepository.java (1)
8-20
: 리포지토리 인터페이스 설계가 깔끔합니다!도메인 계층에 리포지토리 인터페이스를 정의하여 의존성 역전 원칙(DIP)을 잘 준수하였습니다.
메서드 명명 규칙 검토가 필요합니다.
getNearbyNoises
와findByAvgDecibleAndPoint
메서드가 혼재되어 있습니다. JPA 네이밍 컨벤션을 따라 모든 조회 메서드를findXXX
형태로 통일하는 것이 좋겠습니다.- List<Noise> getNearbyNoises(Point point); + List<Noise> findNearbyNoises(Point point);soridam-domain/src/main/java/sorisoop/soridam/domain/noise/infrastructure/NoiseRepositoryImpl.java (1)
21-29
: 쿼리 메서드 네이밍 일관성 확인이 필요합니다.인터페이스와 동일한 이슈로,
getNearbyNoises
와findByAvgDecibleAndPoint
의 네이밍 패턴이 일관되지 않습니다.- public List<Noise> getNearbyNoises(Point point) { + public List<Noise> findNearbyNoises(Point point) { return queryNoiseRepository.getNearbyNoises(point); }soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/UserRepositoryImpl.java (2)
12-14
: @transactional 어노테이션 추가를 고려해보세요.리포지토리 구현체에 트랜잭션 관리를 위한 @transactional 어노테이션 추가를 권장드립니다.
17-20
: 예외 처리 로직 추가를 고려해보세요.JPA 작업 중 발생할 수 있는 예외들(EntityNotFoundException, DataIntegrityViolationException 등)에 대한 처리가 필요해 보입니다. 도메인 특화 예외로 변환하여 처리하는 것을 권장드립니다.
예시:
@Override public User save(User user) { try { return jpaUserRepository.save(user); } catch (DataIntegrityViolationException e) { throw new UserPersistenceException("사용자 저장 중 오류가 발생했습니다", e); } }Also applies to: 22-25, 27-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
build.gradle
(0 hunks)soridam-api/build.gradle
(1 hunks)soridam-api/src/main/java/sorisoop/soridam/api/noise/application/NoiseService.java
(5 hunks)soridam-auth/build.gradle
(2 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java
(2 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java
(2 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java
(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java
(1 hunks)soridam-common/build.gradle
(1 hunks)soridam-domain/build.gradle
(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/noise/domain/NoiseRepository.java
(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/noise/infrastructure/NoiseRepositoryImpl.java
(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/UserRepository.java
(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/infrastructure/UserRepositoryImpl.java
(1 hunks)soridam-global-util/build.gradle
(1 hunks)soridam-infra/build.gradle
(1 hunks)soridam-infra/src/main/java/sorisoop/soridam/infra/config/jpa/JpaConfig.java
(0 hunks)
💤 Files with no reviewable changes (2)
- soridam-infra/src/main/java/sorisoop/soridam/infra/config/jpa/JpaConfig.java
- build.gradle
🔇 Additional comments (11)
soridam-domain/build.gradle (2)
18-21
: Querydsl 설정이 완료되었는지 확인 필요Querydsl 의존성이 추가되었으나, Q클래스 생성을 위한 추가 설정이 필요할 수 있습니다.
다음 사항들을 확인해주세요:
- build.gradle에 Q클래스 생성 경로 설정
- .gitignore에 생성된 Q클래스 포함 여부
Querydsl 설정에 대한 상세한 가이드가 필요하시다면 제가 도움을 드릴 수 있습니다.
14-16
: 지리 데이터 관련 의존성 버전 검토 필요지리 데이터 처리를 위한 라이브러리 추가는 적절해 보이나, 버전 호환성을 확인해야 합니다.
다음 스크립트를 실행하여 Spring Boot 버전과의 호환성을 확인하세요:
✅ Verification successful
지리 데이터 라이브러리 버전 호환성 확인 완료
Spring Boot 3.4.1과 Hibernate Spatial 6.6.4.Final, JTS Core 1.20.0 버전은 모두 호환됩니다. 현재 설정된 버전 조합은 안정적입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Spring Boot version and its compatibility with spatial libraries # Find Spring Boot version fd -e gradle -e properties | xargs grep -l "org.springframework.boot" # Check if there are any known compatibility issues gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "org.hibernate:hibernate-spatial") { nodes { advisory { summary severity } vulnerableVersionRange } } }'Length of output: 541
Script:
#!/bin/bash # Get Spring Boot version from build files echo "=== Spring Boot Version ===" rg "org\.springframework\.boot" -A 1 build.gradle rg "springBootVersion" build.gradle # Check hibernate dependencies echo -e "\n=== Hibernate Dependencies ===" rg "hibernate" build.gradle soridam-domain/build.gradleLength of output: 882
soridam-domain/src/main/java/sorisoop/soridam/domain/noise/infrastructure/NoiseRepositoryImpl.java (1)
15-20
: 리포지토리 구현체의 의존성 주입이 적절합니다.
@RequiredArgsConstructor
와final
필드를 사용한 생성자 주입 패턴이 잘 적용되었습니다.트랜잭션 경계 설정을 검토해주세요.
리포지토리 구현체에
@Transactional
어노테이션 추가를 고려해보세요. JPA의 영속성 컨텍스트 관리를 위해 필요할 수 있습니다.soridam-api/src/main/java/sorisoop/soridam/api/noise/application/NoiseService.java (1)
36-38
: 의존성 통합이 잘 이루어졌습니다.개별 리포지토리들을 하나의
NoiseRepository
인터페이스로 통합한 것이 좋은 설계 결정입니다.soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/UserRepository.java (1)
7-13
: 도메인 계층의 리포지토리 인터페이스가 잘 설계되었습니다!인터페이스가 명확하고 응집도가 높으며, Optional을 활용한 null 처리가 적절합니다. 도메인 주도 설계(DDD) 원칙을 잘 따르고 있습니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/kakao/KakaoOidcService.java (1)
11-11
: 의존성 역전 원칙을 잘 적용하였습니다!구체적인 구현체(JpaUserRepository) 대신 추상화된 인터페이스(UserRepository)를 사용하도록 변경한 것이 좋습니다. 이는 코드의 유연성과 테스트 용이성을 향상시킵니다.
Also applies to: 17-18
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/google/GoogleOidcService.java (1)
11-11
: 일관된 리포지토리 인터페이스 적용이 잘 되었습니다!KakaoOidcService와 마찬가지로 추상화된 UserRepository 인터페이스를 사용하도록 일관성 있게 변경되었습니다. 이러한 일관성은 코드베이스의 유지보수성을 향상시킵니다.
Also applies to: 17-18
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java (2)
19-19
: 의존성 주입 개선이 잘 이루어졌습니다!구체적인 구현체(
JpaUserRepository
)에서 인터페이스(UserRepository
)로 변경함으로써 결합도를 낮추었습니다. 이는 SOLID 원칙 중 의존성 역전 원칙(DIP)을 잘 준수한 변경입니다.Also applies to: 23-23
77-86
: 리포지토리 메서드 호출이 적절히 변경되었습니다.
findByOauthIdentityAndProvider
와save
메서드 호출이 새로운 인터페이스를 통해 이루어지도록 잘 수정되었습니다. 기존 기능을 그대로 유지하면서 구현체에 대한 의존성을 제거했습니다.soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/CustomOidcUserService.java (2)
15-15
: 의존성 분리가 일관성 있게 적용되었습니다.
OidcService
와 마찬가지로 구체적인 구현체에서 인터페이스로의 전환이 잘 이루어졌습니다.Also applies to: 20-20
39-47
: 트랜잭션 처리 확인이 필요합니다.사용자 정보를 수정하고 저장하는 로직에 트랜잭션 처리가 필요할 수 있습니다. 상위 클래스나 구현체에서 트랜잭션이 처리되고 있는지 확인해주세요.
다음 스크립트로 트랜잭션 처리 여부를 확인할 수 있습니다:
public void deleteNoise(String id) { | ||
if (!jpaNoiseRepository.existsById(id)) { | ||
if (!noiseRepository.existsById(id)) { | ||
throw new NoiseNotFoundException(); | ||
} | ||
jpaNoiseRepository.deleteById(id); | ||
noiseRepository.deleteById(id); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteNoise 메서드의 ID 처리가 불완전합니다.
ID 접두사(prefix) 처리가 누락되었습니다. getNoise
메서드와 동일하게 NOISE.getPrefix()
를 사용해야 합니다.
@Transactional
public void deleteNoise(String id) {
- if (!noiseRepository.existsById(id)) {
+ String noiseId = NOISE.getPrefix() + id;
+ if (!noiseRepository.existsById(noiseId)) {
throw new NoiseNotFoundException();
}
- noiseRepository.deleteById(id);
+ noiseRepository.deleteById(noiseId);
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public void deleteNoise(String id) { | |
if (!jpaNoiseRepository.existsById(id)) { | |
if (!noiseRepository.existsById(id)) { | |
throw new NoiseNotFoundException(); | |
} | |
jpaNoiseRepository.deleteById(id); | |
noiseRepository.deleteById(id); | |
} | |
@Transactional | |
public void deleteNoise(String id) { | |
String noiseId = NOISE.getPrefix() + id; | |
if (!noiseRepository.existsById(noiseId)) { | |
throw new NoiseNotFoundException(); | |
} | |
noiseRepository.deleteById(noiseId); | |
} |
Summary
Tasks
Summary by CodeRabbit
릴리스 노트
의존성 변경
리포지토리 리팩토링
기능 개선
API 문서화