Skip to content

[Refac] 전반적인 코드 개선#64

Merged
Yeobi00 merged 9 commits into
mainfrom
refactor/overall-cleanup
Apr 30, 2026
Merged

[Refac] 전반적인 코드 개선#64
Yeobi00 merged 9 commits into
mainfrom
refactor/overall-cleanup

Conversation

@Yeobi00
Copy link
Copy Markdown
Contributor

@Yeobi00 Yeobi00 commented Apr 30, 2026

#️⃣ Issue Number

📝 요약(Summary)

🛠️ PR 유형

어떤 변경 사항이 있나요?

  • 새로운 기능 추가
  • 버그 수정
  • CSS 등 사용자 UI 디자인 변경
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
  • 코드 리팩토링
  • 주석 추가 및 수정
  • 문서 수정
  • 테스트 추가, 테스트 리팩토링
  • 빌드 부분 혹은 패키지 매니저 수정
  • 파일 혹은 폴더명 수정
  • 파일 혹은 폴더 삭제

📸스크린샷 (선택)

💬 공유사항 to 리뷰어

✅ PR Checklist

PR이 다음 요구 사항을 충족하는지 확인하세요.

  • 커밋 메시지 컨벤션에 맞게 작성했습니다.
  • 변경 사항에 대한 테스트를 했습니다.(버그 수정/기능에 대한 테스트).

@github-actions
Copy link
Copy Markdown

코드 리뷰

긍정적인 변경 사항

  • @Transactional(readOnly = true) 적절히 적용 (조회 메서드들)
  • jakarta.transactionorg.springframework.transaction.annotation.Transactional 통일
  • TRIP_MEMBER_EXISTTRIP_MEMBER_NOT_EXIST 오류 코드 버그 수정
  • Optional<Member> findMember 미사용 변수 및 중복 DB 조회 제거
  • TripConverter에서 photoRepository 의존성 제거 (Converter → Service 책임 분리)
  • JwtTokenProvider.key static 제거 (스레드 안전성 개선)
  • e.printStackTrace()log.error(...) 전환
  • ResponseDTO, CustomRuntimeException, WebConfig 등 미사용 파일 삭제

심각한 문제

🔴 보안: dev-login API 프로파일 제한 없음

// AuthController.java
@PostMapping("/dev-login")
@Operation(summary = "개발 전용 로그인 API")
public ApiResponse<KakaoResponseDTO> devLogin(@RequestParam String kakaoId) {

/auth/**PUBLIC_PATHS에 포함되어 인증 없이 접근 가능하며, 프로파일 제한이 없어 프로덕션 환경에서도 임의의 kakaoId로 JWT 발급이 가능합니다.

// 수정 방법
@Profile({"dev", "local"})  // 클래스 또는 메서드 레벨에 추가
@PostMapping("/dev-login")
public ApiResponse<KakaoResponseDTO> devLogin(@RequestParam String kakaoId) {

🟠 보안: webSecurityCustomizer.ignoring()/api/kakaopay/** 추가

PUBLIC_PATHS 상수 공유로 인해 이전에 filterChain.permitAll()에만 있던 /api/kakaopay/**webSecurityCustomizer.ignoring()에도 추가되었습니다.

  • permitAll(): 필터 체인은 실행되지만 인증 없이도 통과
  • ignoring(): Spring Security 필터 체인 자체를 완전히 건너뜀 (JwtFilter 포함)

의도적인 변경이라면 괜찮지만, 두 설정의 의미가 달라 PUBLIC_PATHS를 공유하는 것은 혼동을 유발할 수 있습니다. 두 개의 상수로 분리하거나 주석으로 의도를 명시하는 것을 권장합니다.


🟠 JPA N+1 쿼리: TripServiceImpl.detailTrip()

photoRepository.findByExpense(expense) 호출이 지출(expense) 수만큼 반복됩니다.

// 현재: expenses.size() 만큼 쿼리 발생
Map<Long, List<String>> photoUrlsMap = trip.getExpenses().stream()
        .collect(Collectors.toMap(
                Expense::getId,
                expense -> photoRepository.findByExpense(expense).stream()  // N번 호출
                        .map(Photo::getPhotoUrl)
                        .collect(Collectors.toList())
        ));

PhotoRepositoryIN 절 쿼리를 추가하여 한 번에 조회하는 방식으로 개선을 권장합니다:

// PhotoRepository에 추가
List<Photo> findByExpenseIn(List<Expense> expenses);

// TripServiceImpl에서 사용
List<Expense> expenses = trip.getExpenses();
Map<Long, List<String>> photoUrlsMap = photoRepository.findByExpenseIn(expenses).stream()
        .collect(Collectors.groupingBy(
                photo -> photo.getExpense().getId(),
                Collectors.mapping(Photo::getPhotoUrl, Collectors.toList())
        ));

확인 필요

🟡 spring-dotenv 버전 다운그레이드 (4.0.0 → 3.0.0)

다운그레이드 이유가 PR 설명에 없습니다. Spring Boot 3.4.5와의 호환성 문제인지 확인이 필요합니다.


🟡 RestTemplate 직접 필드 생성

// AuthService.java
private final RestTemplate restTemplate = new RestTemplate();

@Bean으로 등록하지 않아 커넥션 풀, 타임아웃 설정이 적용되지 않으며 테스트 시 mocking이 어렵습니다. TripServiceImpl처럼 빈으로 주입받는 방식을 일관되게 적용하는 것을 권장합니다.

@Yeobi00 Yeobi00 merged commit 4c638b7 into main Apr 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant