Skip to content

Conversation

@jungdongha
Copy link
Collaborator

close #30

@jungdongha jungdongha self-assigned this Sep 27, 2025
@jungdongha jungdongha added the enhancement New feature or request label Sep 27, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/CustomOAuth2UserService.kt

🟢 좋은점:

  • Kotlin의 when 표현식을 활용해 provider별 파싱 로직을 깔끔하게 분기 처리하여 코드 가독성과 유지보수성을 높임. 이는 Kotlin 최적화 규칙(when 표현식 사용)을 잘 따름.
  • Naver와 Kakao 파싱 함수에서 null safety를 고려한 타입 캐스팅(as? 사용)과 기본값 처리(예: nickname의 "사용자" fallback, 이메일의 가짜 도메인 생성)를 적용하여 런타임 오류를 방지함. 이는 Kotlin null safety 최적화에 부합.
  • OAuthUserInfo를 data class로 사용(코드 끝부분에 명시)하여 불변성과 equals/hashCode 자동 생성 등 Kotlin의 data class 이점을 활용.
  • ktlint 규칙 준수: 네이밍 컨벤션(camelCase 함수/변수명), 포맷팅(들여쓰기, 공백)이 일관되며, 불필요한 코드가 없음.

🟡 개선사항:

  • 카카오 이메일 처리에서 하드코딩된 가짜 이메일("kakao_$[email protected]")을 사용 중인데, 이는 실제 사용자 식별에 혼란을 줄 수 있음. OAuthUserInfo의 email 필드를 nullable로 변경하거나, 별도의 플래그(예: isSocialEmail: Boolean)를 추가해 컨트롤러/서비스에서 구분 처리하는 방식을 고려. 또는 확장 함수로 이메일 유효성 검증 로직을 분리.
  • 파싱 함수들(Google, Naver, Kakao)에서 Map<String, Any>의 타입 캐스팅(as String, as Map 등)이 빈번함. Kotlin의 safe cast(elvis 연산자 ?: )를 더 활용하거나, 공통 유틸 함수(예: extension function safeGetString(map: Map<String, Any>, key: String): String?)를 만들어 중복을 줄이면 코드가 더 간결해짐.
  • provider가 "else" 케이스에서 IllegalArgumentException을 던지는데, 이는 글로벌 익셉션 처리(@ControllerAdvice)에서 표준 에러 응답으로 매핑될 수 있지만, 여기서도 더 구체적인 에러 메시지(예: "Unsupported provider

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/SecurityConfig.kt

🟢 좋은점:

  • Kotlin 최적화 측면에서 우수합니다. addFilterBefore 메서드 호출을 Java 스타일의 ::class.java 대신 Kotlin의 reified 제네릭(<UsernamePasswordAuthenticationFilter>)을 사용해 더 간결하고 타입 안전하게 변경했습니다. 이는 Kotlin의 타입 시스템을 활용한 idiomatic한 코드로, 컴파일 타임에 타입 검증이 강화되어 런타임 오류를 줄입니다.
  • ktlint 규칙 준수: 변경된 코드가 간결하고 포맷팅(들여쓰기, 공백)이 적절하며, 네이밍 컨벤션(기존 변수명 유지)을 위반하지 않습니다.

🟡 개선사항:

  • 글로벌 익셉션 처리나 ApiResponse 사용과 직접 관련은 없으나, SecurityConfig에서 발생할 수 있는 인증/인가 예외(예: JWT 검증 실패)가 @ControllerAdvice로 적절히 처리되는지 전체 프로젝트 맥락에서 확인하는 것이 좋습니다. 만약 이 필터가 예외를 던진다면, 표준 에러 응답 형식(ApiResponse 래핑)을 보장하도록 추가 검토하세요.
  • if (!isDev) 블록 내에서 필터 추가 로직이 조건부이므로, 프로덕션 환경에서의 보안 강화(예: 추가 로깅이나 메트릭스)를 고려해 확장할 수 있습니다.

🔴 문제점:

  • 없음. 변경이 안전하고 기능적으로 동등하며, Spring Security의 HttpSecurity 구성에 적합합니다.

val oAuthUserInfo =
when (provider) {
"google" -> parseGoogle(attributes)
"naver" -> parseNaver(attributes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

저희 네이버 안한다고하지 않았었나요?

registration:
google:
client-id: 11962659605-optp0eb5h1c1ob1b9qie53s83j165qt8.apps.googleusercontent.com
client-secret: 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

env 파일에 올라가야할 내용같습니다 이거는. 시크릿 키 아닌가요?

- email
naver:
client-id: At2KDxDK_ppHMuMYqEDL
client-secret: osKwSljl9i
Copy link
Collaborator

Choose a reason for hiding this comment

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

시크릿 키는 전부 삭제하시고 다시 PR 부탁드립니다

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/CustomOAuth2UserService.kt

🟢 좋은점:

  • Kotlin의 when 표현식을 활용해 provider에 따라 적절히 파싱 로직을 분기 처리하여 코드 가독성과 유지보수성을 높임. (Kotlin 최적화 규칙 준수)
  • parseKakao 함수에서 null safety를 철저히 적용 (as? 연산자와 Elvis 연산자 ?: 사용)하여 런타임 NullPointerException을 방지함. (Kotlin 최적화 규칙 준수)
  • data class OAuthUserInfo를 반환 타입으로 사용해 간결하고 불변성을 유지함. (Kotlin 최적화 규칙 준수)
  • ktlint 규칙에 따라 네이밍 컨벤션 (camelCase)과 포맷팅이 일관되며, 주석으로 카카오 이메일 처리 이유를 명확히 설명함.

🟡 개선사항:

  • parseNaver 함수에서 response["id"], response["email"], response["name"]을 무조건 as String으로 캐스팅하고 있음. Naver API 응답에서 이 필드가 null일 가능성을 고려해 as? String ?: 기본값으로 처리하면 null safety를 더 강화할 수 있음. (Kotlin 최적화 규칙 강화)
  • 카카오 이메일 기본값("kakao_$[email protected]")이 적절하지만, 실제 프로덕션 환경에서 이메일 인증/동의 여부를 추가 검증하거나, OAuthUserInfo의 email 필드를 nullable로 설계해 기본값 대신 null을 허용하는 방식을 고려하면 유연성이 높아짐.
  • 글로벌 익셉션 처리와 연계해 when의 IllegalArgumentException을 커스텀 예외(예: UnsupportedProviderException)로 wrapping하면 표준 에러 응답과 더 잘 맞물림. (현재는 기본 예외지만, @ControllerAdvice에서 처리 가능하므로 큰 문제는 아님)

🔴 문제점:

  • parseNaver 함수의 캐스팅 (as String)이 런타임 에러를 유발할 수 있음. Naver API 문서상 필드가 null일 수 있는 경우를 대비해 null safety를 적용하지 않으면 Kotlin의 장점을 잃음. 반드시 as?와 기본값으로 수정해야 함. (Kotlin 최적

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/SecurityConfig.kt

🟢 좋은점: Kotlin 최적화 측면에서 우수합니다. addFilterBefore 메서드의 Java 스타일 호출(::class.java)을 Kotlin 제네릭 타입(<UsernamePasswordAuthenticationFilter>)으로 변경하여 더 idiomatic하고 간결한 Kotlin 코드를 작성했습니다. 이는 Kotlin의 타입 추론과 제네릭 지원을 활용한 최적화 예시로, 코드 가독성을 높입니다.

🟡 개선사항: 변경 자체는 적절하나, SecurityConfig 전체에서 isDev 플래그나 jwtAuthenticationFilter의 초기화 부분을 확인하여 프로파일별(Dev/Prod) 필터 적용 로직이 일관되게 동작하는지 테스트를 추천합니다. ktlint 규칙 준수 여부는 변경 범위가 작아 문제없으나, 전체 파일 포맷팅을 다시 실행해 보세요.

🔴 문제점: 없음. 변경이 안전하고 기능적으로 올바릅니다.

beekeeper24 and others added 2 commits September 28, 2025 20:52
* feat(be): userChat service,controller,SSE 추가

* feat(be) : controller,service,repository 팀 스타일에 맞게 수정
* fix(be): DisplayName 어노테이션 Test에 반영

* fix(be): TourApiClient 수정, List형태로 결과를 반환

* feat(be): Tour service 구현 및 Dto구조 변경 (#24)

* fix(be) : TourResponse dto 구조 변경

* feat(be): ai codeReview 반영,ã�기본 값 null로 변경

* feat(be) TourResponse Dto 구조 변경에 따른 Client, Service 리팩토링
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/CustomOAuth2UserService.kt

🟢 좋은점:

  • Kotlin의 when 표현식을 활용해 provider에 따라 적절히 분기 처리하며, 코드를 확장성 있게 유지하고 있습니다. (Kotlin 최적화 준수)
  • parseKakao에서 null safety를 고려해 as?와 Elvis 연산자(?:)를 사용해 안전하게 기본값을 제공하는 점이 좋습니다. (Kotlin 최적화 준수)
  • data class OAuthUserInfo를 반환 타입으로 사용해 간결하고 불변성을 유지합니다. (Kotlin 최적화 준수)
  • ktlint 규칙에 따라 네이밍 컨벤션(camelCase)과 포맷팅(들여쓰기, 공백)이 일관되게 적용되어 있습니다.

🟡 개선사항:

  • parseNaver에서 as Map<String, Any>as String 같은 강제 캐스팅을 사용 중인데, attributes나 response가 예상과 다를 경우 ClassCastException이 발생할 수 있습니다. parseKakao처럼 as?와 null 체크를 추가해 더 안전하게 처리하는 것을 고려하세요. (Kotlin 최적화 강화)
  • Kakao의 email 처리에서 가짜 이메일("kakao_$[email protected]")을 생성하는 로직은 임시 방편으로 보이는데, 실제로 Kakao API에서 이메일 동의 여부를 확인하고 로그를 남기는 등의 추가 검증을 넣으면 좋겠습니다. (Kotlin 최적화 및 로직 안정성)
  • 글로벌 익셉션 처리와 연계해 IllegalArgumentException 외에 캐스팅 실패 시 커스텀 예외(예: OAuthParseException)를 던지도록 확장하면 표준 에러 응답과 더 잘 맞물릴 수 있습니다. (글로벌 익셉션 처리 간접 준수)

🔴 문제점:

  • parseNaver의 response["id"], ["email"], ["name"] 등 필드가 null일 경우 강제 as String 캐스팅으로 런타임 ClassCastException 또는 NullPointerException이 발생할 위험이 있습니다. 반드시 null-safe 캐스팅(as? String ?: throw IllegalStateException(...) 등)으로 수정하세요. (Kotlin 최적화 위반, 잠재적 런타임

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/common/security/SecurityConfig.kt

🟢 좋은점: Kotlin 최적화 측면에서 우수합니다. addFilterBefore 메서드 호출을 제네릭 타입 파라미터(<UsernamePasswordAuthenticationFilter>)를 사용한 Kotlin다운 방식으로 변경하여, Java 클래스 참조(::class.java)를 제거하고 더 간결하고 타입 안전한 코드를 작성했습니다. 이는 Kotlin의 제네릭 지원을 활용한 최적화 예시로 적합합니다. ktlint 규칙(포맷팅, 네이밍)도 준수되어 깔끔합니다.

🟡 개선사항: 없음. 변경이 간단하고 목적에 부합합니다.

🔴 문제점: 없음. 글로벌 익셉션 처리나 ApiResponse 규칙은 SecurityConfig와 무관하므로 해당되지 않습니다.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI 리뷰 - src/main/kotlin/com/back/koreaTravelGuide/domain/ai/tour/client/TourApiClient.kt

🟢 좋은점:

  • Kotlin 최적화 측면에서 runCatching, getOrNull(), takeUnless, map 등의 idiomatic Kotlin 함수를 활용해 예외 처리와 데이터 파싱을 안전하고 간결하게 구현했습니다. null safety도 잘 지켜지며 (e.g., takeIf { it.isTextual }?.asText()), data class(TourItem)로 응답 객체를 구성하는 점이 적절합니다.
  • ktlint 규칙 준수: 코드 포맷팅이 일관되며 (들여쓰기, 줄 바꿈), 네이밍 컨벤션(camelCase, 의미 있는 변수명 like 'itemsNode', 'resultCode')이 잘 따르고 있습니다. 주석도 명확하고 위치가 적절합니다.
  • 로깅 개선: println을 SLF4J logger로 대체해 프로덕션 환경에 적합하게 변경되었으며, info/warn/error 레벨을 적절히 사용했습니다. 이는 디버깅과 모니터링에 유리합니다.
  • API 응답 처리 강화: 이전의 단일 아이템 파싱에서 전체 items 배열을 매핑하도록 변경되어 더 robust하며, resultCode 체크를 추가해 API 오류를 사전에 필터링합니다. 빈 응답 시 emptyList() 반환으로 null safety를 높였습니다.

🟡 개선사항:

  • 글로벌 익셉션 처리: runCatching으로 로컬 예외를 잘 처리하지만, 이 클라이언트에서 발생한 예외(예: RestTemplate 실패)가 상위 레이어(컨트롤러)로 전파될 때 @ControllerAdvice에서 표준 에러 응답으로 매핑되도록 설계되었는지 확인하세요. 필요 시 커스텀 예외(예: TourApiException)를 던져서 더 세밀한 핸들링을 고려할 수 있습니다.
  • ApiResponse 사용: 이 코드는 내부 클라이언트이므로 직접 ApiResponse를 감쌀 필요는 없지만, 상위 컨트롤러에서 이 TourResponse를 ApiResponse로 래핑하는지 검토하세요. 만약 컨트롤러에서 사용된다면 일관성을 위해 클라이언트 응답도 ApiResponse 형태로 통합하는 확장 함수를 추가하는 게 좋을 수 있습니다.
  • Kotlin 최적화 추가:

@jungdongha jungdongha closed this Sep 28, 2025
@jungdongha jungdongha reopened this Sep 28, 2025
@jungdongha jungdongha closed this Sep 28, 2025
@Mrbaeksang Mrbaeksang deleted the feat/be/30 branch September 30, 2025 05:35
@Mrbaeksang Mrbaeksang restored the feat/be/30 branch September 30, 2025 05:35
@Mrbaeksang Mrbaeksang deleted the feat/be/30 branch September 30, 2025 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

소셜 로그인 구현

5 participants