-
Notifications
You must be signed in to change notification settings - Fork 4
[FEAT] 애플 로그인 환경 변수 추가 #374
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
Conversation
Walkthrough최근 검색 키워드 추적 기능을 추가합니다. 사용자가 공지사항을 검색할 때 검색어를 자동으로 저장하고 관리하는 새로운 도메인을 구현하며, NoticeService에 이를 통합합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NoticeService
participant RecentSearchService
participant UserService
participant RecentSearchRepository as Repository
participant DB as Database
Client->>NoticeService: searchByKeyword(keyword)
activate NoticeService
Note over NoticeService,RecentSearchService: 키워드 저장 (신규)
NoticeService->>RecentSearchService: save(keyword)
activate RecentSearchService
RecentSearchService->>RecentSearchService: normalizeKeyword(keyword)
RecentSearchService->>UserService: getCurrentUser()
activate UserService
UserService-->>RecentSearchService: User
deactivate UserService
alt 검색어 존재
RecentSearchService->>RecentSearchRepository: findByUserIdAndKeyword()
activate Repository
Repository->>DB: SELECT
DB-->>Repository: RecentSearch 객체
deactivate Repository
RecentSearchService->>RecentSearchService: touch(LocalDateTime.now())
else 검색어 미존재
RecentSearchService->>RecentSearchRepository: save(new RecentSearch(...))
activate Repository
Repository->>DB: INSERT
alt 제약조건 위반 발생
DB-->>Repository: DataIntegrityViolationException
Repository-->>RecentSearchService: Exception
RecentSearchService->>RecentSearchRepository: findByUserIdAndKeyword()
RecentSearchService->>RecentSearchService: touch(LocalDateTime.now())
else 성공
Repository-->>RecentSearchService: 저장됨
end
deactivate Repository
end
RecentSearchService->>RecentSearchService: deleteRecentKeywordsOversize(user)
RecentSearchService->>RecentSearchRepository: findByUserIdOrderByUpdatedAtDesc (LIMIT 21)
activate Repository
Repository->>DB: SELECT (최신순)
DB-->>Repository: 최대 21개 항목
deactivate Repository
RecentSearchService->>RecentSearchRepository: deleteAll(오래된 항목)
activate Repository
Repository->>DB: DELETE (상위 1개 초과)
deactivate Repository
RecentSearchService-->>NoticeService: 완료
deactivate RecentSearchService
Note over NoticeService: 공지사항 검색 수행
NoticeService->>DB: 공지사항 쿼리
NoticeService-->>Client: 검색 결과 반환
deactivate NoticeService
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 2
🧹 Nitpick comments (6)
src/main/java/ku_rum/backend/domain/search/domain/RecentSearch.java (1)
9-11:@NoArgsConstructor에access = AccessLevel.PROTECTED설정 권장JPA 엔티티의 기본 생성자는 프레임워크 내부에서만 사용되므로, 외부에서 직접 호출되지 않도록
PROTECTED접근 수준을 사용하는 것이 일반적인 관례입니다.🔎 제안 수정
@Getter -@NoArgsConstructor +@NoArgsConstructor(access = AccessLevel.PROTECTED) @Entity추가로 import가 필요합니다:
import lombok.AccessLevel;src/main/java/ku_rum/backend/domain/search/domain/repository/RecentSearchRepository.java (1)
16-18: 삭제 메서드에@Modifying어노테이션 추가 권장Spring Data JPA의 파생 삭제 쿼리는 기본적으로 엔티티를 먼저 조회한 후 개별 삭제합니다. 벌크 삭제를 위해
@Modifying과@Query를 사용하면 성능을 개선할 수 있습니다.현재 구현도 동작하지만, 삭제할 레코드가 많을 경우 N+1 쿼리 문제가 발생할 수 있습니다.
🔎 성능 개선 예시 (선택사항)
@Modifying @Query("DELETE FROM RecentSearch rs WHERE rs.id = :id AND rs.userId = :userId") long deleteByIdAndUserId(@Param("id") Long id, @Param("userId") Long userId); @Modifying @Query("DELETE FROM RecentSearch rs WHERE rs.userId = :userId") long deleteByUserId(@Param("userId") Long userId);src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java (1)
22-26: JPA 엔티티 대신 DTO 사용 권장
RecentSearch엔티티를 직접 API 응답으로 반환하고 있습니다. 이 방식은 다음과 같은 문제를 일으킬 수 있습니다:
- 내부 필드(예:
userId)가 클라이언트에 노출됨- 엔티티 구조 변경 시 API 호환성 문제 발생
- 지연 로딩 관계가 있을 경우 직렬화 문제 발생 가능
RecentSearchResponseDTO를 생성하여 필요한 필드만 노출하는 것을 권장합니다.🔎 DTO 예시
public record RecentSearchResponse( Long id, String keyword, LocalDateTime updatedAt ) { public static RecentSearchResponse from(RecentSearch entity) { return new RecentSearchResponse( entity.getId(), entity.getKeyword(), entity.getUpdatedAt() ); } }src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java (2)
51-61: 초과 키워드 삭제 로직 개선 가능현재 구현은 모든 레코드를 조회한 후 Java에서 초과분을 필터링합니다. 레코드 수가 많아지면 비효율적일 수 있습니다.
DB 레벨에서 직접 삭제하는 방식이 더 효율적입니다.
🔎 개선 예시 (네이티브 쿼리)
Repository에 다음 메서드 추가:
@Modifying @Query(value = """ DELETE FROM recent_search WHERE user_id = :userId AND id NOT IN ( SELECT id FROM ( SELECT id FROM recent_search WHERE user_id = :userId ORDER BY updated_at DESC LIMIT :limit ) AS keep ) """, nativeQuery = true) void deleteOldestExceedingLimit(@Param("userId") Long userId, @Param("limit") int limit);
26-30: 빈 문자열/null 검증 위치 확인
normalizeKeyword에서 null과 빈 문자열을 처리하고 있습니다. 컨트롤러 레벨에서@NotBlank검증을 추가하면 더 명확한 오류 메시지를 클라이언트에 전달할 수 있습니다.src/test/java/ku_rum/backend/domain/search/presentation/RecentSearchControllerTest.java (1)
53-68:limit파라미터 검증 경계 케이스 테스트 추가 권장컨트롤러에서
@Min(1) @Max(100)검증이 있지만, 이에 대한 테스트가 없습니다. 경계 조건 테스트 추가를 권장합니다.🔎 추가 테스트 예시
@Test @DisplayName("limit이 0 이하인 경우 400 에러를 반환한다") @WithMockUser void getRecentSearches_invalidLimit() throws Exception { mockMvc.perform(get("/api/v1/notices/searches/recent") .param("limit", "0") .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isBadRequest()); } @Test @DisplayName("limit이 100을 초과하는 경우 400 에러를 반환한다") @WithMockUser void getRecentSearches_exceedsMaxLimit() throws Exception { mockMvc.perform(get("/api/v1/notices/searches/recent") .param("limit", "101") .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isBadRequest()); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.javasrc/main/java/ku_rum/backend/domain/notice/presentation/NoticeController.javasrc/main/java/ku_rum/backend/domain/search/application/RecentSearchService.javasrc/main/java/ku_rum/backend/domain/search/domain/RecentSearch.javasrc/main/java/ku_rum/backend/domain/search/domain/repository/RecentSearchRepository.javasrc/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.javasrc/main/resources/configsrc/test/java/ku_rum/backend/domain/search/presentation/RecentSearchControllerTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/ku_rum/backend/domain/search/application/RecentSearchService.java (1)
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
Service(29-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/main/java/ku_rum/backend/domain/search/domain/RecentSearch.java (1)
29-30:userId노출에 대한 확인 필요
RecentSearchController에서RecentSearch엔티티를 직접 반환하고 있어,userId가 API 응답에 노출됩니다. 만약 의도된 동작이라면 괜찮지만, 보안/프라이버시 측면에서 DTO를 통해 필요한 필드만 노출하는 것을 고려해 주세요.src/main/java/ku_rum/backend/domain/notice/presentation/NoticeController.java (1)
56-57: 변경 사항 확인 완료포맷팅 변경만 있으며 기능적 변경은 없습니다.
src/main/java/ku_rum/backend/domain/notice/application/NoticeService.java (1)
88-93: 검색 키워드 저장 순서 확인현재 검색 키워드가 실제 검색 수행 전에 저장됩니다. 검색이 실패하거나 결과가 없는 경우에도 키워드가 저장됩니다. 의도된 동작인지 확인해 주세요.
만약 성공한 검색에 대해서만 저장하려면 검색 후 저장하도록 순서를 변경하는 것을 고려해 주세요.
src/main/java/ku_rum/backend/domain/search/presentation/RecentSearchController.java (1)
14-15: 도메인 경계와 API 경로 일관성 확인컨트롤러는
search도메인에 위치하지만, API 경로는/api/v1/notices/searches/recent로notices아래에 있습니다. 의도된 설계라면 괜찮지만, 도메인 구조와 API 경로의 일관성을 확인해 주세요.src/test/java/ku_rum/backend/domain/search/presentation/RecentSearchControllerTest.java (1)
50-91: 테스트 구조 및 문서화 잘 되어있습니다RestDocs를 활용한 API 문서화와 기본적인 성공 케이스 테스트가 잘 작성되어 있습니다.
| } catch (DataIntegrityViolationException e) { | ||
| repository.findByUserIdAndKeyword(user.getId(), normalized) | ||
| .ifPresent(rs -> rs.touch(now)); | ||
| } |
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.
DataIntegrityViolationException 발생 시 트랜잭션 롤백 문제
DataIntegrityViolationException이 발생하면 Spring은 트랜잭션을 롤백 전용으로 마킹합니다. catch 블록에서 touch() 호출이 정상적으로 커밋되지 않을 수 있습니다.
경쟁 조건을 안전하게 처리하려면 @Retryable을 사용하거나, 새 트랜잭션에서 재시도하는 것을 고려해 주세요.
🔎 대안 1: REQUIRES_NEW 전파 사용
@Transactional
public void save(String keyword) {
String normalized = normalizeKeyword(keyword);
if (normalized == null) {
return;
}
User user = userService.getUser();
LocalDateTime now = LocalDateTime.now();
repository.findByUserIdAndKeyword(user.getId(), normalized)
.ifPresentOrElse(
rs -> rs.touch(now),
() -> insertNewKeyword(user.getId(), normalized, now)
);
deleteRecentKeywordsOversize(user);
}
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void insertNewKeyword(Long userId, String keyword, LocalDateTime now) {
try {
repository.save(new RecentSearch(userId, keyword, now));
} catch (DataIntegrityViolationException e) {
repository.findByUserIdAndKeyword(userId, keyword)
.ifPresent(rs -> rs.touch(now));
}
}| @@ -1 +1 @@ | |||
| Subproject commit b9a38aae3b125092669fc3d9216c47acd765d8ea | |||
| Subproject commit 963495c41dce741c660957ec16da3727c9f41935 | |||
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.
🧩 Analysis chain
🏁 Script executed:
# Check the repository structure and any changed files
git ls-files | head -20Repository: KU-rum/backend
Length of output: 596
🏁 Script executed:
# Check .gitmodules configuration for the config submodule
cat .gitmodules 2>/dev/null || echo "No .gitmodules found"Repository: KU-rum/backend
Length of output: 183
🏁 Script executed:
# Check what files are in the PR (if we can determine changed files)
git status --short 2>/dev/null || echo "Not a git repo or no changes"Repository: KU-rum/backend
Length of output: 40
🏁 Script executed:
# Look for any files related to Apple login or RecentSearch in the repository
find . -type f -name "*.properties" -o -name "*.yml" -o -name "*.yaml" -o -name "*.json" 2>/dev/null | grep -E "(config|env)" | head -20Repository: KU-rum/backend
Length of output: 98
🏁 Script executed:
# Check if there's a git diff we can examine
git diff --stat --no-patch 2>/dev/null || echo "Unable to get diff"Repository: KU-rum/backend
Length of output: 40
🏁 Script executed:
# Check if there are any PR description files or changelog
find . -type f \( -name "CHANGELOG*" -o -name "*.md" \) -path "*docs*" -o -name "*.md" -path "*root*" 2>/dev/null | head -10Repository: KU-rum/backend
Length of output: 97
🏁 Script executed:
# Look for any actual code files that might have been changed (excluding the submodule)
git diff HEAD~1 --name-only 2>/dev/null || echo "Unable to get previous commit diff"Repository: KU-rum/backend
Length of output: 92
🏁 Script executed:
# Check if there are environment or config files at the root level
ls -la | grep -E "\.(env|properties|yml|yaml)$"Repository: KU-rum/backend
Length of output: 40
🏁 Script executed:
# Search for Apple login or RecentSearch related keywords in the codebase
rg -l "apple|Apple|APPLE|login|LOGIN" --type java --type properties --type yaml -g "!node_modules" 2>/dev/null | head -10Repository: KU-rum/backend
Length of output: 40
🏁 Script executed:
# Search for RecentSearch keyword
rg -l "RecentSearch|recent" --type java -g "!node_modules" 2>/dev/null | head -10Repository: KU-rum/backend
Length of output: 818
🏁 Script executed:
# Check for Apple login related environment variables in the config submodule path
# Since config is a submodule, check if there's any reference to Apple in the main repo
rg -i "apple" --type property-list --type env -g "!node_modules" 2>/dev/null || echo "No Apple-related config found in main repo"Repository: KU-rum/backend
Length of output: 100
🏁 Script executed:
# Verify the actual PR files by checking git log for the submodule update
git log --oneline -1 2>/dev/null || echo "Cannot retrieve git log"Repository: KU-rum/backend
Length of output: 77
🏁 Script executed:
# Check main application properties files for environment variables
find . -type f \( -name "application*.properties" -o -name "application*.yml" \) -not -path "*/node_modules/*" 2>/dev/null | head -5Repository: KU-rum/backend
Length of output: 136
🏁 Script executed:
# Check the actual files that were changed in this specific commit (if accessible)
git show --stat --oneline 0cfa313 2>/dev/null | head -20 || echo "Cannot show commit details"Repository: KU-rum/backend
Length of output: 1239
🏁 Script executed:
# Verify the submodule is correctly configured to point to backend-config repo
cat .gitmodules | grep -A 2 "config"Repository: KU-rum/backend
Length of output: 183
🏁 Script executed:
# Search for any Apple login related code in main source
rg -i "apple|oauth.*apple" --type java -g "src/main/java" 2>/dev/null | head -20Repository: KU-rum/backend
Length of output: 40
🏁 Script executed:
# Check what the RecentSearch files actually do - are they related to this PR?
git log --oneline src/main/java/ku_rum/backend/domain/search/ 2>/dev/null | head -5Repository: KU-rum/backend
Length of output: 77
PR 설명 업데이트 필요 - 여러 기능 포함
현재 PR 커밋 메시지 "애플로그인 설정 추가"와 실제 변경 사항 간 괴리가 있습니다. 검증 결과, 이번 커밋(0cfa313)은 다음 두 가지를 포함하고 있습니다:
- 애플 로그인 환경 변수 -
src/main/resources/config서브모듈 업데이트 - RecentSearch 기능 - 최근 검색 키워드 추적 기능이 메인 리포지토리에 추가됨
PR 제목이나 설명을 수정하여 모든 변경 사항을 명확히 반영해 주세요. 예: "[FEAT] 애플 로그인 환경 변수 및 최근 검색 기능 추가"
🤖 Prompt for AI Agents
In src/main/resources/config around lines 1 to 1, the PR title/description only
mentions "애플로그인 설정 추가" but the commit also updates the config submodule for
Apple login and adds the RecentSearch feature to the main repo; update the PR
title and description to list both changes (config/apple login env vars and
RecentSearch keyword tracking) and include a concise summary and example title
such as "[FEAT] 애플 로그인 환경 변수 및 최근 검색 기능 추가", plus brief bullet points of the two
changes and affected files/commits.
Test Results 38 files 38 suites 11s ⏱️ Results for commit 0cfa313. |
📝작업 내용
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.