refactor: 학기를 테이블로 관리하도록 변경 #526
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 분
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
Gyuhyeok99
requested review from
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch,
whqtker and
wibaek
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/term/repository/TermRepository.java (1)
1-10: 변경사항 확인 및 제안 사항
- 서비스 레이어 Optional 처리
findByIsCurrentTrue().orElseThrow(...)로 CURRENT_TERM_NOT_FOUND 예외가 올바르게 발생- 현재 학기 유일성 보장 검토
• 테이블에isCurrent=true에 대한 UNIQUE 제약조건 또는 partial index 추가
• Term 전환 시 기존 current 값을 false로 업데이트하는 트랜잭션 로직 고려
• Flyway 마이그레이션 스크립트에 반영src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java (1)
53-54: 현재 학기 조회 로직의 중복을 줄이는 것을 고려해보세요.세 개의 메서드(
getApplicants,getApplicantsByUserApplications,validateSiteUserCanViewApplicants)에서 동일한 패턴으로 현재 학기를 조회하고 있습니다. 에러 처리가 일관되어 좋지만, private 헬퍼 메서드로 추출하면 코드 중복을 제거하고 유지보수성을 높일 수 있습니다.다음과 같이 헬퍼 메서드를 추가하는 것을 고려해보세요:
+ private Term getCurrentTerm() { + return termRepository.findByIsCurrentTrue() + .orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND)); + }그런 다음 각 메서드에서 다음과 같이 사용:
- Term term = termRepository.findByIsCurrentTrue() - .orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND)); + Term term = getCurrentTerm();Also applies to: 70-71, 140-141
src/main/java/com/example/solidconnection/term/domain/Term.java (1)
34-35: 현재 학기 유일성 검증을 고려해보세요.
isCurrent는 비즈니스 로직상 하나의 Term만true여야 합니다. 현재는 애플리케이션 레벨에서만 관리되므로, 실수로 여러 Term이 current가 되는 것을 방지하기 어렵습니다.다음 중 하나를 고려해보세요:
옵션 1: 데이터베이스 레벨 제약 (PostgreSQL 등)
CREATE UNIQUE INDEX uk_term_is_current ON term (is_current) WHERE is_current = true;옵션 2: 서비스 레벨 트랜잭션 관리
현재 학기를 변경할 때 이전 current를 false로 설정하는 로직을 서비스 메서드에서 트랜잭션으로 보장:@Transactional public void setCurrentTerm(Long termId) { termRepository.findByIsCurrentTrue() .ifPresent(Term::setAsNotCurrent); Term newCurrentTerm = termRepository.findById(termId) .orElseThrow(...); newCurrentTerm.setAsCurrent(); }src/main/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepository.java (1)
81-92: 2.getTermIdToNameMap메서드가 일관된 패턴으로 구현되었으나, 데이터 무결성 검증을 고려해 보세요.새로운 메서드가 기존
getMentorIdToXxxMap패턴과 일관되게 구현되었고, 빈 리스트에 대한 early return도 적절합니다. 다만, 기존 메서드들(getMentorIdToSiteUserMap,getMentorIdToUniversityMap)은 데이터 무결성 문제 발생 시DATA_INTEGRITY_VIOLATION예외를 던지는 반면, 이 메서드는 존재하지 않는 termId가 있어도 조용히 해당 매핑을 누락시킵니다.데이터 일관성 관점에서, mentor.termId에 해당하는 Term이 없다면 무결성 문제로 간주하여 예외를 던지는 것을 고려해 보세요.
다음과 같은 검증 로직 추가를 고려해 보세요:
public Map<Long, String> getTermIdToNameMap(List<Mentor> mentors) { if (mentors.isEmpty()) { return Collections.emptyMap(); } Set<Long> termIds = mentors.stream() .map(Mentor::getTermId) .collect(Collectors.toSet()); - return termRepository.findAllById(termIds).stream() - .collect(Collectors.toMap(Term::getId, Term::getName)); + List<Term> terms = termRepository.findAllById(termIds); + Map<Long, String> termIdToNameMap = terms.stream() + .collect(Collectors.toMap(Term::getId, Term::getName)); + + // 데이터 무결성 검증 + for (Long termId : termIds) { + if (!termIdToNameMap.containsKey(termId)) { + throw new CustomException(DATA_INTEGRITY_VIOLATION, + "mentor.term_id에 해당하는 term이 존재하지 않음"); + } + } + + return termIdToNameMap; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
src/main/java/com/example/solidconnection/application/domain/Application.java(6 hunks)src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java(1 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java(7 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java(4 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentor.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MatchedMentorResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorDetailResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewResponse.java(1 hunks)src/main/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepository.java(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java(3 hunks)src/main/java/com/example/solidconnection/mentor/service/MentorQueryService.java(5 hunks)src/main/java/com/example/solidconnection/mentor/service/MentoringQueryService.java(2 hunks)src/main/java/com/example/solidconnection/term/domain/Term.java(1 hunks)src/main/java/com/example/solidconnection/term/repository/TermRepository.java(1 hunks)src/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.java(1 hunks)src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java(1 hunks)src/main/java/com/example/solidconnection/university/dto/UnivApplyInfoDetailResponse.java(1 hunks)src/main/java/com/example/solidconnection/university/dto/UnivApplyInfoPreviewResponse.java(2 hunks)src/main/java/com/example/solidconnection/university/repository/UnivApplyInfoRepository.java(2 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java(1 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java(5 hunks)src/main/java/com/example/solidconnection/university/service/GeneralUnivApplyInfoRecommendService.java(2 hunks)src/main/java/com/example/solidconnection/university/service/LikedUnivApplyInfoService.java(2 hunks)src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java(3 hunks)src/main/java/com/example/solidconnection/university/service/UnivApplyInfoRecommendService.java(5 hunks)src/main/resources/data.sql(2 hunks)src/main/resources/secret(1 hunks)src/test/java/com/example/solidconnection/application/fixture/ApplicationFixture.java(2 hunks)src/test/java/com/example/solidconnection/application/fixture/ApplicationFixtureBuilder.java(3 hunks)src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java(22 hunks)src/test/java/com/example/solidconnection/application/service/ApplicationSubmissionServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorFixture.java(2 hunks)src/test/java/com/example/solidconnection/mentor/fixture/MentorFixtureBuilder.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/mentor/service/MentorQueryServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(4 hunks)src/test/java/com/example/solidconnection/term/fixture/TermFixture.java(1 hunks)src/test/java/com/example/solidconnection/term/fixture/TermFixtureBuilder.java(1 hunks)src/test/java/com/example/solidconnection/term/repository/TermRepositoryForTest.java(1 hunks)src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java(1 hunks)src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixtureBuilder.java(2 hunks)src/test/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepositoryTest.java(4 hunks)src/test/java/com/example/solidconnection/university/service/GeneralUnivApplyInfoRecommendServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/university/service/LikedUnivApplyInfoServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java(8 hunks)src/test/java/com/example/solidconnection/university/service/UnivApplyInfoRecommendServiceTest.java(8 hunks)src/test/resources/application.yml(0 hunks)
💤 Files with no reviewable changes (1)
- src/test/resources/application.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T05:07:54.817Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:0-0
Timestamp: 2025-08-26T05:07:54.817Z
Learning: 대학지원정보 텍스트 검색에서 region과 country 간의 INNER JOIN은 데이터 무결성을 위한 의도된 설계이다. country.region_code가 region 테이블에 존재하지 않는 경우는 잘못된 데이터 상태로 간주하여 검색 결과에서 제외한다.
Applied to files:
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
📚 Learning: 2025-08-26T05:00:56.556Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:166-178
Timestamp: 2025-08-26T05:00:56.556Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.
Applied to files:
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java
🧬 Code graph analysis (4)
src/test/java/com/example/solidconnection/term/fixture/TermFixtureBuilder.java (1)
src/test/java/com/example/solidconnection/term/fixture/TermFixture.java (1)
TestComponent(7-26)
src/test/java/com/example/solidconnection/term/fixture/TermFixture.java (1)
src/test/java/com/example/solidconnection/term/fixture/TermFixtureBuilder.java (1)
TestComponent(8-35)
src/test/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepositoryTest.java (1)
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (4)
Nested(58-99)Nested(101-163)Nested(165-291)Nested(185-244)
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (1)
src/test/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepositoryTest.java (1)
Nested(43-93)
🔇 Additional comments (41)
src/main/resources/secret (1)
1-1: 서브모듈 커밋 업데이트 확인 완료!
새 SHA 반영 확인했습니다. 추가적인 조치 없이도 머지해도 괜찮아 보여요.src/test/java/com/example/solidconnection/university/repository/LikedUnivApplyInfoRepositoryTest.java (2)
8-41: LGTM! 학기 컨텍스트 설정이 올바르게 적용되었습니다.변경사항:
- TermFixture를 주입받아 테스트 시작 시 현재 학기를 초기화
- 모든 테스트가 동일한 학기 컨텍스트를 공유하도록 설정
- @beforeeach에서 "2025-2" 학기를 현재 학기로 설정
이 패턴은 다른 테스트 파일들과 일관성이 있어 좋습니다.
50-83: LGTM! 픽스처 호출이 termId 기반으로 올바르게 업데이트되었습니다.모든 UnivApplyInfoFixture 호출에서
term.getId()를 전달하여 학기 ID 기반 데이터 흐름과 일치하도록 수정되었습니다.src/main/java/com/example/solidconnection/university/dto/UnivApplyInfoPreviewResponse.java (1)
19-28: LGTM! DTO 팩토리 메서드가 termName을 외부에서 받도록 올바르게 수정되었습니다.변경사항:
- 메서드 시그니처에
String termName파라미터 추가univApplyInfo.getTerm()대신 전달받은 termName 사용- 관심사 분리 - 학기 이름 조회 책임을 서비스 레이어로 이동
이 패턴은 UnivApplyInfo 엔티티가 더 이상 term 문자열을 직접 보유하지 않는 새로운 구조와 일치합니다.
src/test/java/com/example/solidconnection/mentor/service/MentorQueryServiceTest.java (1)
21-63: LGTM! 테스트 설정이 다른 테스트 파일들과 일관된 패턴을 따릅니다.변경사항:
- TermFixture 의존성 주입
- setUp 메서드에서 "2025-2" 학기를 현재 학기로 초기화
- 모든 멘토 테스트가 동일한 학기 컨텍스트에서 실행
코드베이스 전반에 걸쳐 일관된 학기 설정 패턴이 적용되어 좋습니다.
src/test/java/com/example/solidconnection/mentor/service/MentorMyPageServiceTest.java (1)
22-66: LGTM! 멘토 마이페이지 테스트에 학기 컨텍스트가 올바르게 추가되었습니다.변경사항은 다른 테스트 파일들과 동일한 패턴을 따르며, setUp 메서드에서 TermFixture를 통해 현재 학기를 초기화합니다.
src/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.java (2)
102-102: 동일한 패턴이 텍스트 검색 메서드에도 일관되게 적용되었습니다.searchUnivApplyInfoByText 메서드에서도 term 파라미터가 제거되어 일관성이 유지됩니다.
94-94: LGTM! 컨트롤러 책임이 서비스 레이어로 깔끔히 이동되었습니다.변경사항:
1. @value로 주입받던term필드 제거
2. 서비스 메서드 호출 시term파라미터 제거
3.UnivApplyInfoQueryService내부에서TermRepository.findByIsCurrentTrue()로 현재 학기 조회 로직 캡슐화searchUnivApplyInfoByFilter와 searchUnivApplyInfoByText 메서드에서
TermRepository.findByIsCurrentTrue()를 통해 현재 학기를 정상적으로 조회하고 있습니다.src/test/java/com/example/solidconnection/university/service/GeneralUnivApplyInfoRecommendServiceTest.java (2)
8-46: LGTM! 추천 서비스 테스트가 termId 기반으로 올바르게 마이그레이션되었습니다.변경사항:
- TermFixture를 통한 현재 학기 초기화
- 모든 UnivApplyInfoFixture 호출에
term.getId()전달- 10개의 대학 지원 정보 픽스처가 모두 동일한 학기 ID를 사용하도록 설정
setUp 메서드가 테스트에 필요한 모든 데이터를 일관된 학기 컨텍스트 내에서 준비합니다.
58-59: LGTM! 검증 로직이 새로운 필드명과 일치하도록 업데이트되었습니다.
extracting("term")대신extracting("termId")를 사용하고,term.getId()와 비교하도록 수정되어 엔티티 구조 변경사항을 정확히 반영합니다.src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java (1)
35-36: UnivApplyInfo termId 할당 검증 및 FK 제약조건 추적 요청
- termId 할당 검증
- main 코드에서 UnivApplyInfo 인스턴스 생성(생성자/세터/DTO 매핑 등) 모든 경로에서 termId가 명시적으로 설정되는지
rg -n 'new UnivApplyInfo\(',rg -n 'setTermId'등으로 확인해 주세요.- FK 제약조건 추적
- PR 목표에 언급된 term_id FK 제약 추가가 별도 이슈/PR로 관리되고 있는지 확인 후 관련 링크 또는 이슈 번호를 남겨 주세요.
src/test/java/com/example/solidconnection/mentor/fixture/MentorFixtureBuilder.java (1)
20-20: 깔끔한 타입 변경이에요!테스트 픽스처 빌더가
term(String)에서termId(long)로 전환되었어요. 변경 사항:
- 필드 타입 변경:
private String term→private long termId- 빌더 메서드 시그니처 변경:
term(String)→termId(long)- Mentor 생성 시 termId 전달
일관되고 올바르게 리팩터링되었습니다.
Also applies to: 56-59, 70-70
src/test/java/com/example/solidconnection/term/repository/TermRepositoryForTest.java (1)
1-10: 테스트용 리포지토리 추가가 적절해요!
TermRepositoryForTest는 테스트 컨텍스트에서 학기 이름으로 Term 엔티티를 조회할 수 있게 해줍니다.변경 내용:
JpaRepository<Term, Long>확장findByName(String name)쿼리 메서드 선언- 테스트 픽스처가 학기 이름으로 Term을 찾을 수 있도록 지원
테스트와 프로덕션 리포지토리를 명확하게 분리한 좋은 설계입니다.
src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageResponse.java (1)
22-22: DTO 팩토리 메서드 시그니처 변경이 올바릅니다!
MentorMyPageResponse.of()메서드가termName파라미터를 추가로 받도록 변경되었어요.변경 사항:
- 메서드 시그니처:
of(Mentor, SiteUser, University)→of(Mentor, SiteUser, University, String termName)- 응답 생성 시
termName파라미터 사용이제 서비스 레이어에서
termId로부터termName을 조회하여 전달해야 합니다. 다른 DTO들과 일관된 패턴이에요.Also applies to: 29-29
src/test/java/com/example/solidconnection/university/service/LikedUnivApplyInfoServiceTest.java (1)
13-14: 테스트 컨텍스트에 학기 설정 추가가 적절해요!테스트에 Term 컨텍스트가 추가되어 termId 기반 픽스처를 사용할 수 있게 되었습니다.
변경 내용:
TermFixture의존성 주입 추가 (lines 44-45)Term필드 추가 (line 48)setUp()에서 현재 학기 초기화:termFixture.현재_학기("2025-2")(line 53)- 모든
UnivApplyInfo픽스처 호출에term.getId()전달 (lines 55, 61-62)테스트 데이터가 일관되게 termId 컨텍스트를 사용하도록 잘 구성되었습니다.
Also applies to: 44-48, 53-53, 55-55, 61-62
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1)
40-41: 마이페이지 테스트도 학기 컨텍스트로 일관되게 업데이트되었어요!
MyPageServiceTest가 Term 기반 테스트 데이터를 사용하도록 변경되었습니다.주요 변경 사항:
TermFixture의존성 주입 추가 (lines 94-95)Term필드 추가 (line 104)setUp()에서 현재 학기 초기화 (line 109)- 멘토 및 좋아요 관련 픽스처에
term.getId()전달 (lines 141, 163-165)다른 서비스 테스트들과 동일한 패턴을 따르고 있어 일관성이 좋습니다.
Also applies to: 94-95, 104-104, 109-109, 141-141, 163-165
src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java (1)
23-24: 학기 조회 로직이 잘 구현되었어요!
ApplicationSubmissionService가 현재 학기를TermRepository에서 조회하여 사용하도록 변경되었습니다.올바른 변경 사항:
TermRepository의존성 추가 (lines 23-24, 40)- 현재 학기 조회:
termRepository.findByIsCurrentTrue()(line 51)- 기존 지원서 조회 시
termId사용 (line 58)- 새 지원서 생성 시
term.getId()전달 (line 71)String 기반에서 Long 기반 termId로의 전환이 잘 이루어졌습니다.
Also applies to: 40-40, 58-58, 71-71
src/main/resources/data.sql (2)
44-44: 학기 테이블 초기 데이터 추가가 적절해요!
term테이블에 첫 학기 데이터가 삽입되었습니다.INSERT INTO term (name, is_current) VALUES ('2024-1', true);'2024-1' 학기가 현재 학기(
is_current = true)로 설정되었어요.확인 필요:
- 현재 하나의 학기만 삽입되어 있는데, 이전 학기나 향후 학기 데이터도 필요한지 검토해주세요
- 프로덕션 환경에서는 여러 학기 데이터가 필요할 수 있어요
149-224: 대학 지원 정보의 학기 참조가 올바르게 변경되었어요!
university_info_for_apply테이블의 학기 참조가 String에서 FK로 전환되었습니다.변경 내용:
- 컬럼명 변경:
term→term_id(line 149)- 모든 INSERT 문에서 학기 값 변경:
- 이전:
('2024-1', 1, ...)(String)- 현재:
(1, 1, ...)(FK to term table)- 총 24개의 대학 지원 정보 레코드가 모두
term_id = 1참조데이터 마이그레이션이 일관되게 적용되었습니다.
참고:
- PR 목표에서 언급했듯이 Flyway 스크립트는 아직 작성되지 않았어요
- 프로덕션 마이그레이션 시 기존 데이터의 term String 값을 term_id로 매핑하는 스크립트가 필요합니다
- FK 제약조건 추가도 Flyway 스크립트에 포함되어야 해요
src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java (2)
4-4: 새로운 에러 코드 추가가 적절합니다.현재 학기를 찾지 못했을 때 명확한 에러를 던지도록
CURRENT_TERM_NOT_FOUND를 추가한 점이 좋습니다.
60-60: term.getId() 사용이 올바릅니다.String term에서 Long termId로의 전환이 일관되게 적용되었습니다. 리포지토리 호출 시
term.getId()를 정확하게 전달하고 있습니다.Also applies to: 73-73, 87-87, 143-143
src/main/java/com/example/solidconnection/university/dto/UnivApplyInfoDetailResponse.java (1)
35-42: 팩토리 메서드 시그니처 변경이 적절합니다.
termName매개변수를 추가하여 호출자가 학기 이름을 명시적으로 제공하도록 변경한 것이 좋습니다. 이는UnivApplyInfo에서 term 필드를 제거하고 termId로 관리하는 리팩터링과 일관됩니다.src/main/java/com/example/solidconnection/mentor/dto/MentorPreviewResponse.java (1)
22-32: 학기 이름 매개변수 추가가 일관성 있게 적용되었습니다.
MentorPreviewResponse.of메서드에termName매개변수를 추가한 변경이 다른 DTO 응답 클래스들과 일관성을 유지합니다.mentor.getTerm()에서 외부 제공termName으로 전환이 올바르게 구현되었습니다.src/test/java/com/example/solidconnection/application/fixture/ApplicationFixture.java (1)
16-35: 테스트 픽스처가 올바르게 업데이트되었습니다.
지원서메서드의 매개변수를String term에서long termId로 변경하고, 빌더 호출도.termId(termId)로 업데이트한 것이 도메인 엔티티 변경사항과 일치합니다.src/main/java/com/example/solidconnection/term/domain/Term.java (2)
1-49: Term 엔티티 구조가 적절합니다.
id(자동 생성),name(유니크 제약),isCurrent(기본값 false) 필드 구성이 합리적입니다.setAsCurrent()와setAsNotCurrent()메서드로 현재 학기 상태를 관리하는 방식이 명확합니다.
31-32: 학기 이름 길이 제한이 충분한지 확인하세요.
name필드의 최대 길이가 20자입니다. 현재 "2025-1" 형식을 사용한다면 충분하지만, 향후 더 긴 형식(예: "2025-1학기", "2025년 1학기")을 사용할 가능성이 있다면 길이를 늘리는 것을 고려해보세요.프로젝트의 학기 명명 규칙을 확인하고 20자가 충분한지 검토해주세요.
src/test/java/com/example/solidconnection/mentor/fixture/MentorFixture.java (1)
15-23: TermFixture 통합이 올바르게 구현되었습니다.
termFixture.현재_학기("2025-1").getId()를 사용하여 현재 학기 ID를 가져오는 방식이 적절합니다. 테스트에서 학기 컨텍스트를 명확하게 제공합니다.참고: "2025-1"이라는 하드코딩된 값이 테스트 전반에 걸쳐 일관되게 사용되는지 확인하세요.
src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (3)
59-60: 테스트 설정이 명확하게 개선되었습니다.
TermFixture를 주입하여 학기 컨텍스트를 명시적으로 관리@BeforeEach에서 현재 학기를 초기화하여 모든 테스트에서 일관된 학기 사용테스트의 가독성과 유지보수성이 향상되었습니다.
Also applies to: 78-82
257-263: 이전 학기 테스트 케이스가 훌륭합니다.
이전_학기_지원자는_조회되지_않는다테스트에서termFixture.이전_학기("2024-2")를 사용하여 현재 학기와 이전 학기를 구분하는 시나리오를 명확하게 검증합니다. 이는 학기 필터링 로직의 정확성을 보장합니다.
96-98: term.getId() 사용이 일관되게 적용되었습니다.모든 테스트 케이스에서
ApplicationFixture.지원서호출 시term.getId()를 전달하도록 업데이트되어 String term에서 Long termId로의 전환이 완전히 반영되었습니다.Also applies to: 110-110, 120-120, 130-130, 162-162, 172-172, 182-182, 212-212, 222-222, 232-232, 263-263, 291-291, 303-303, 335-335, 345-345, 355-355, 378-378, 388-388, 398-398
src/main/java/com/example/solidconnection/mentor/dto/MentorDetailResponse.java (1)
23-33: MentorDetailResponse 팩토리 메서드 변경이 일관성 있습니다.
termName매개변수를 추가하고mentor.getTerm()대신 이를 사용하도록 변경한 것이 다른 멘토/대학 관련 DTO들과 동일한 패턴을 따릅니다. 학기 정보를 외부에서 제공받도록 하는 리팩터링 목표와 부합합니다.src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (3)
77-79: 1. 메서드 시그니처와 내부 조건절이termId기반으로 깔끔하게 변경되었습니다.
findAllByFilter메서드가String term대신Long termId를 받도록 업데이트되었고, 내부 where 절에서도termIdEq(univApplyInfo, termId)로 일관되게 전환되었습니다. 타입 변경으로 인해 null/blank 체크가 단순 null 체크로 간소화된 점도 좋습니다.Also applies to: 92-92
121-126: 2.termIdEq헬퍼 메서드의 타입 변경이 적절합니다.기존
termEq(String)메서드가termIdEq(Long)으로 이름과 타입이 변경되었고, null 체크만 수행하도록 단순화되었습니다. Long 타입 비교로 인한 성능상 이점도 기대할 수 있습니다.
146-146: 3. 텍스트 검색 메서드도 동일하게termId기반으로 전환되었습니다.
findAllByText메서드가String term대신Long termId를 받도록 변경되었고, where 절에서termIdEq를 호출합니다. 검색 로직의 일관성이 유지되었습니다.Also applies to: 158-158
src/main/java/com/example/solidconnection/mentor/repository/MentorBatchQueryRepository.java (1)
10-11: 1. Term 관련 의존성이 추가되어 termId → termName 매핑을 지원합니다.
TermRepository와Term엔터티가 새로 추가되어, 멘토 정보를 조회할 때 학기 이름을 함께 조회할 수 있게 되었습니다.Also applies to: 30-30
src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java (2)
18-28: 1.findAllByUnivApplyInfoIds쿼리 메서드가termId기반으로 변경되었습니다.JPQL 쿼리의 조건절이
a.term = :term에서a.termId = :termId로 변경되었고, 메서드 파라미터도String term에서long termId로 변경되어 일관성이 유지되었습니다.
30-42: 2.findBySiteUserIdAndTermId메서드와 디폴트 헬퍼 메서드가 일관되게 업데이트되었습니다.
- 쿼리 메서드가
findBySiteUserIdAndTermId로 이름이 변경되고long termId파라미터를 받습니다.- 디폴트 메서드
getApplicationBySiteUserIdAndTermId도 동일하게 업데이트되어 네이밍 일관성이 유지됩니다.src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (3)
14-15: 1. TermFixture를 활용한 테스트 초기화가 명확하게 추가되었습니다.
TermFixture가 의존성으로 추가되었고,@BeforeEach에서 현재 학기를 "2025-2"로 설정합니다. 이를 통해 모든 테스트가 일관된 학기 컨텍스트에서 실행됩니다.Also applies to: 48-56
64-65: 2. 픽스처 호출과 예상 응답 생성이termId와termName기반으로 일관되게 업데이트되었습니다.
- 모든 픽스처 메서드 호출 시
term.getId()를 전달하여 termId 기반 엔터티를 생성합니다.- 예상 응답 생성 시
UnivApplyInfoPreviewResponse.from(univApplyInfo, term.getName())로 termName을 함께 전달합니다.- 리포지토리 메서드 검증 시에도
term.getId()를 사용하여 일관성이 유지됩니다.Also applies to: 76-77, 108-119
267-283: 3. 캐싱 테스트도termId기반으로 적절히 업데이트되었습니다.캐싱 검증 테스트에서 리포지토리 메서드 호출 확인 시
findAllByText(text, term.getId())를 사용하여, termId 기반 쿼리가 캐싱되는지 확인합니다.src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java (1)
14-20: 모든 픽스처 메서드가termId파라미터 기반으로 일관되게 리팩터링되었습니다.
- 기존에 주입된 term 필드를 사용하던 방식에서, 각 메서드가
long termId파라미터를 받아 명시적으로 설정하는 방식으로 변경되었습니다.- 모든 메서드에서
.termId(termId)를 호출하여 UnivApplyInfo 엔터티를 생성합니다.- 테스트에서 학기를 명시적으로 전달하도록 강제하여, 각 테스트의 학기 컨텍스트가 더 명확해졌습니다.
Also applies to: 22-28, 30-36, 38-44, 46-52, 54-60, 62-68, 70-76, 78-84, 86-92, 94-100
src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java
Outdated
Show resolved
Hide resolved
| termRepository.findByIsCurrentTrue().ifPresent(term -> { | ||
| Pageable page = PageRequest.of(0, RECOMMEND_UNIV_APPLY_INFO_NUM); | ||
| generalRecommends = univApplyInfoRepository.findRandomByTermId(term.getId(), page); | ||
| }); |
There was a problem hiding this comment.
현재 학기 미설정 시 추천 목록이 null로 남는 문제
현재 학기가 설정되어 있지 않으면 generalRecommends가 그대로 null로 남아 이후 접근 지점에서 NPE가 발생해요. 의도한 CURRENT_TERM_NOT_FOUND 예외도 던지지 못하고 있어서 디버깅이 더 어렵습니다. Kindly 하지만 확실하게, 아래처럼 즉시 예외를 던지면서 리스트를 채우도록 바꿔주세요.
- termRepository.findByIsCurrentTrue().ifPresent(term -> {
- Pageable page = PageRequest.of(0, RECOMMEND_UNIV_APPLY_INFO_NUM);
- generalRecommends = univApplyInfoRepository.findRandomByTermId(term.getId(), page);
- });
+ Term term = termRepository.findByIsCurrentTrue()
+ .orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND));
+ Pageable page = PageRequest.of(0, RECOMMEND_UNIV_APPLY_INFO_NUM);
+ generalRecommends = univApplyInfoRepository.findRandomByTermId(term.getId(), page);📝 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.
| termRepository.findByIsCurrentTrue().ifPresent(term -> { | |
| Pageable page = PageRequest.of(0, RECOMMEND_UNIV_APPLY_INFO_NUM); | |
| generalRecommends = univApplyInfoRepository.findRandomByTermId(term.getId(), page); | |
| }); | |
| Term term = termRepository.findByIsCurrentTrue() | |
| .orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND)); | |
| Pageable page = PageRequest.of(0, RECOMMEND_UNIV_APPLY_INFO_NUM); | |
| generalRecommends = univApplyInfoRepository.findRandomByTermId(term.getId(), page); |
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/service/GeneralUnivApplyInfoRecommendService.java
around lines 35-38, replace the current optional-handling so that if no current
term is found you immediately throw the CURRENT_TERM_NOT_FOUND exception, and
ensure generalRecommends is always non-null by initializing it before use;
specifically, call termRepository.findByIsCurrentTrue().orElseThrow(() -> new
CustomException(ErrorCode.CURRENT_TERM_NOT_FOUND)) to get the term, then
populate generalRecommends =
univApplyInfoRepository.findRandomByTermId(term.getId(), PageRequest.of(0,
RECOMMEND_UNIV_APPLY_INFO_NUM)), and initialize generalRecommends to an empty
list at declaration (or set to Collections.emptyList()) to avoid NPEs if
something else goes wrong.
| Set<Long> termIds = univApplyInfos.stream() | ||
| .map(UnivApplyInfo::getTermId) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| Map<Long, String> termMap = termRepository.findAllById(termIds).stream() | ||
| .collect(Collectors.toMap(Term::getId, Term::getName)); | ||
|
|
||
| return univApplyInfos.stream() | ||
| .map(UnivApplyInfoPreviewResponse::from) | ||
| .map(univApplyInfo -> { | ||
| String termName = termMap.getOrDefault(univApplyInfo.getTermId(), "Unknown"); | ||
| return UnivApplyInfoPreviewResponse.from(univApplyInfo, termName); | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
효율적인 배치 조회 패턴이지만 폴백 전략을 검토해주세요.
좋아요한 대학 정보 목록 조회 시 termId를 기반으로 학기 이름을 효율적으로 조회하도록 변경되었어요.
변경 내용:
- 모든
UnivApplyInfo의termId를 Set으로 수집 (lines 39-41) TermRepository로 해당 Term들을 일괄 조회하여 Map 생성 (lines 43-44)- 각 응답 생성 시 Map에서 termName 조회 (lines 47-50)
검토 필요 사항:
- Line 48에서
getOrDefault(termId, "Unknown")사용 - Term이 존재하지 않는 경우 "Unknown"을 반환하는데, 이는 데이터 무결성 문제를 숨길 수 있어요
- FK 제약조건이 있다면 이런 경우가 발생하지 않아야 하지만, 마이그레이션 중이거나 데이터 불일치 시 조용히 실패합니다
제안:
- 프로덕션 환경에서 데이터 무결성이 보장된다면 "Unknown" 대신 예외를 던지는 것을 고려해보세요
- 또는 로깅을 추가하여 이런 케이스를 추적하는 것도 좋아요
다음 스크립트로 UnivApplyInfo의 termId가 모두 유효한 Term을 참조하는지 확인할 수 있어요:
🏁 Script executed:
#!/bin/bash
# UnivApplyInfo의 termId가 존재하지 않는 Term을 참조하는지 확인
# UnivApplyInfo 엔티티 파일에서 termId 필드 확인
echo "=== UnivApplyInfo 엔티티의 termId 필드 확인 ==="
rg -A 5 "class UnivApplyInfo" --type java -g "**/UnivApplyInfo.java" | rg -A 3 "termId"
# FK 제약조건이 설정되어 있는지 확인
echo -e "\n=== FK 제약조건 확인 (Flyway 스크립트) ==="
fd -e sql . | xargs rg "FOREIGN KEY.*term_id|termId.*REFERENCES.*term" 2>/dev/null || echo "FK 제약조건을 찾을 수 없습니다 (Flyway 스크립트 미작성으로 예상됨)"
# 관련 에러 코드 확인
echo -e "\n=== Term 관련 에러 코드 확인 ==="
rg "TERM_NOT_FOUND|CURRENT_TERM_NOT_FOUND" --type javaLength of output: 3391
불완전한 termId 참조 시 폴백 전략 검토 필요
- 모든
UnivApplyInfo의termId를 Set으로 수집했습니다 (lines 39-41). TermRepository로 일괄 조회 후termMap을 생성했습니다 (lines 43-44).termMap.getOrDefault(..., "Unknown")으로 존재하지 않는 term을"Unknown"으로 처리합니다 (lines 47-50).
다음 사항을 검토해주세요.
"Unknown"폴백은 누락된 term을 조용히 숨길 수 있습니다.- Flyway 마이그레이션 스크립트에 FK 제약이 없어 실제 누락된
termId가 발생할 수 있습니다. - 다른 서비스에서는 누락 시
CustomException(TERM_NOT_FOUND/CURRENT_TERM_NOT_FOUND)을 던지는 패턴을 사용하고 있습니다.
다음 제안을 고려하세요.
- 누락 시 예외를 던져 데이터 무결성을 보장하세요.
- 또는 로깅을 추가해 누락 케이스를 추적하세요.
- 필요 시 DB에 FK 제약을 추가하는 방안을 검토하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/service/LikedUnivApplyInfoService.java
around lines 39-50, replace the silent "Unknown" fallback with explicit
handling: after building termMap, compute missingTermIds = termIds -
termMap.keySet(); if missingTermIds is not empty, either throw the existing
CustomException (e.g., TERM_NOT_FOUND or a new specific exception containing the
missing ids) to enforce data integrity, or at minimum log an error with the
missingTermIds and fail fast; then map univApplyInfos to responses using the
confirmed termMap (no "Unknown" fallback).
There was a problem hiding this comment.
@Gyuhyeok99, 누락된 term ID를 감지하고 예외를 던지는 방법을 보여드릴게요:
@Transactional(readOnly = true)
public List<UnivApplyInfoPreviewResponse> getLikedUnivApplyInfos(long siteUserId) {
List<UnivApplyInfo> univApplyInfos = likedUnivApplyInfoRepository.findUnivApplyInfosBySiteUserId(siteUserId);
Set<Long> termIds = univApplyInfos.stream()
.map(UnivApplyInfo::getTermId)
.collect(Collectors.toSet());
Map<Long, String> termMap = termRepository.findAllById(termIds).stream()
.collect(Collectors.toMap(Term::getId, Term::getName));
// 누락된 term ID가 있는지 확인
Set<Long> missingTermIds = termIds.stream()
.filter(termId -> !termMap.containsKey(termId))
.collect(Collectors.toSet());
if (!missingTermIds.isEmpty()) {
throw new CustomException(TERM_NOT_FOUND);
}
return univApplyInfos.stream()
.map(univApplyInfo -> {
String termName = termMap.get(univApplyInfo.getTermId()); // getOrDefault 대신 get 사용
return UnivApplyInfoPreviewResponse.from(univApplyInfo, termName);
})
.toList();
}주요 변경 사항:
- 누락 감지 (lines 11-14):
termIds에는 있지만termMap에는 없는 ID들을 찾습니다 - 예외 던지기 (lines 16-18): 누락된 ID가 있으면 즉시
CustomException(TERM_NOT_FOUND)예외를 던집니다 - 안전한 접근 (line 22):
getOrDefault대신get()을 사용해도 안전합니다 (이미 모든 term이 존재함을 확인했으므로)
추가 옵션 - 로깅과 함께 사용:
더 자세한 디버깅을 원하시면 로깅을 추가할 수도 있어요:
if (!missingTermIds.isEmpty()) {
log.error("존재하지 않는 term ID가 감지되었습니다. missing IDs: {}", missingTermIds);
throw new CustomException(TERM_NOT_FOUND);
}이렇게 하면 데이터 무결성 문제를 조기에 발견하고 추적할 수 있어요!
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java
Show resolved
Hide resolved
| public Term 현재_학기(String name) { | ||
| return termFixtureBuilder.term() | ||
| .name(name) | ||
| .isCurrent(true) | ||
| .findOrCreate(); | ||
| } | ||
|
|
||
| public Term 이전_학기(String name) { | ||
| return termFixtureBuilder.term() | ||
| .name(name) | ||
| .isCurrent(false) | ||
| .findOrCreate(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
기존 Term가 이미 존재할 때 isCurrent가 갱신되지 않습니다.
같은 이름으로 저장된 Term가 isCurrent=false 상태라면 현재_학기(...)를 호출해도 그대로 false가 유지되어, 테스트 전제(현재 학기)와 실제 데이터가 어긋납니다. 하나의 테스트가 Term를 만들고 다른 테스트가 재사용하는 흐름에서 특히 문제가 됩니다. TermFixtureBuilder.findOrCreate() 단계에서 기존 Term의 isCurrent 값을 요청한 상태로 맞춰 주도록 수정해 주세요.
다음과 같이 기존 엔터티의 상태를 동기화하는 편이 안전합니다(실제 Term 엔터티의 갱신 메서드 이름에 맞게 조정해 주세요):
- public Term findOrCreate() {
- return termRepositoryForTest.findByName(name)
- .orElseGet(() -> termRepositoryForTest.save(new Term(name, isCurrent)));
- }
+ public Term findOrCreate() {
+ return termRepositoryForTest.findByName(name)
+ .map(existing -> {
+ if (existing.isCurrent() != isCurrent) {
+ existing.updateIsCurrent(isCurrent);
+ }
+ return existing;
+ })
+ .orElseGet(() -> termRepositoryForTest.save(new Term(name, isCurrent)));
+ }📝 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 Term 현재_학기(String name) { | |
| return termFixtureBuilder.term() | |
| .name(name) | |
| .isCurrent(true) | |
| .findOrCreate(); | |
| } | |
| public Term 이전_학기(String name) { | |
| return termFixtureBuilder.term() | |
| .name(name) | |
| .isCurrent(false) | |
| .findOrCreate(); | |
| } | |
| public Term findOrCreate() { | |
| return termRepositoryForTest.findByName(name) | |
| .map(existing -> { | |
| if (existing.isCurrent() != isCurrent) { | |
| existing.updateIsCurrent(isCurrent); | |
| } | |
| return existing; | |
| }) | |
| .orElseGet(() -> termRepositoryForTest.save(new Term(name, isCurrent))); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java (1)
52-53: 선택적 개선: 현재 학기 조회 성능 최적화 고려현재
apply메서드가 호출될 때마다 현재 학기를 DB에서 조회하고 있어요. 학기가 자주 변경되지 않는다면, 캐싱을 통해 성능을 개선할 수 있습니다.개선 방안:
- Spring Cache를 활용하여 현재 학기 정보를 캐싱
- 관리자 API로 학기 변경 시 캐시 무효화
다만 이는 현재 PR의 범위를 벗어나는 최적화이므로, 향후 성능 이슈가 발생할 경우 고려하시면 좋을 것 같아요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java(5 hunks)src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java(2 hunks)
⏰ 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 (4)
src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java (2)
52-53: ✅ 이전 리뷰 이슈가 올바르게 수정되었습니다!과거 리뷰에서 지적되었던 잘못된 예외 코드(
USER_NOT_FOUND) 사용 문제가 올바르게 수정되었어요:수정된 내용:
- Line 4:
CURRENT_TERM_NOT_FOUNDimport 추가 ✓- Line 53:
USER_NOT_FOUND→CURRENT_TERM_NOT_FOUND로 변경 ✓현재 학기를 찾지 못했을 때 적절한 예외가 발생하도록 수정되었습니다.
24-25: LGTM! Term 테이블 기반 리팩터링이 깔끔하게 적용되었습니다.전체적인 변경 사항이 일관되고 올바르게 구현되었어요:
주요 변경 사항:
의존성 추가 (Lines 24-25, 41)
Term엔티티와TermRepositoryimport 및 필드 추가현재 학기 조회 로직 (Lines 52-53)
termRepository.findByIsCurrentTrue()로 현재 학기 조회- 학기가 없을 경우 적절한 예외(
CURRENT_TERM_NOT_FOUND) 발생termId 기반 조회 (Line 59)
- 기존 지원서 조회 시
term.getId()를 사용하여 termId 기반으로 변경Application 생성 (Line 72)
- 새 지원서 생성 시
term.getId()를 전달하여 termId 저장모든 변경 사항이 PR 목표와 일치하며, 코드의 일관성이 유지되고 있습니다.
Also applies to: 41-41, 52-53, 59-59, 72-72
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (2)
40-42: Term 조회 및 예외 처리가 적절합니다변경사항 요약:
termId를 통해 Term 엔티티 조회- 존재하지 않을 경우
TERM_NOT_FOUND예외 발생- 응답에
term.getName()포함로직이 명확하고 에러 핸들링도 적절합니다.
46-59: 변경 영향 및 확인 완료
- 메서드가 항상 현재 학기를 조회하도록 변경됨.
- 모든 호출부는 이미
searchUnivApplyInfoByFilter(request)형태로 일관되어 있어 영향이 없음.- 과거 학기 조회 기능이 제거되어 필요한 경우 별도 구현이 필요함.
| public UnivApplyInfoPreviewResponses searchUnivApplyInfoByText(String text) { | ||
| Term term = termRepository.findByIsCurrentTrue() | ||
| .orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND)); | ||
|
|
||
| List<UnivApplyInfoPreviewResponse> responses = univApplyInfoRepository.findAllByText(text, term.getId()) | ||
| .stream() | ||
| .map(UnivApplyInfoPreviewResponse::from) | ||
| .map(univApplyInfo -> UnivApplyInfoPreviewResponse.from( | ||
| univApplyInfo, | ||
| term.getName() | ||
| )) | ||
| .toList(); | ||
| return new UnivApplyInfoPreviewResponses(responses); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
API 변경 검증 및 캐시 전략 개선 권장
변경사항 요약:
- 캐시 키가
"univApplyInfoTextSearch:{0}"로 수정됨 (이전 리뷰 반영 완료) term파라미터 제거- 항상 현재 학기만 조회하도록 변경
주의사항:
- 이 메서드의 모든 호출부에서
term인자가 제거되었는지 확인 필요 - 캐시 키에 현재 학기 정보가 포함되지 않아, 학기가 변경(예: 2024-1 → 2024-2)되어도 이전 학기 데이터가 최대 24시간 동안 캐시에 남아있을 수 있습니다
호출부 검증:
캐시 전략 개선 권장사항:
학기 변경 시 캐시 무효화를 위해 다음 중 하나를 고려해 주세요:
- 방안 1: 캐시 키에 term ID 포함 (예:
"univApplyInfoTextSearch:{0}:{1}"형태로 text와 현재 termId 조합) - 방안 2: 현재 학기가 변경될 때 해당 캐시를 명시적으로 evict하는 로직 추가
현재 구조에서는 학기 전환 시점에 잘못된 데이터가 제공될 수 있으므로, 향후 관리자 API로 학기를 변경하는 기능 개발 시 캐시 무효화 전략도 함께 고려해 주세요.
🏁 Script executed:
#!/bin/bash
# Description: searchUnivApplyInfoByText 메서드의 모든 호출부 확인
rg -nP --type=java -C3 'searchUnivApplyInfoByText\s*\('Length of output: 8545
변경 사항을 요약합니다.
- 호출부 검증:
- searchUnivApplyInfoByText 호출부에서 term 인자 제거가 이미 적용되어 추가 수정이 불필요합니다.
- 캐시 전략 개선 제안:
- 캐시 키에 term ID를 포함하도록 변경하여 학기 변경 시에도 올바른 데이터를 조회하도록 합니다 (예: univApplyInfoTextSearch:{text}:{termId}).
- 또는 학기 변경 시점에 해당 캐시를 명시적으로 무효화하는 로직을 추가합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java
around lines 63 to 75, the search result currently only keys/addresses cache by
text which will serve stale results across term changes; update any caching
logic used for searchUnivApplyInfoByText to include the current term ID in the
cache key (for example "univApplyInfoTextSearch:{text}:{termId}") or add
explicit cache invalidation when the current term changes so search results are
correct for each term; ensure you obtain term.getId() and incorporate it into
the cache key or invalidation routine wherever the search results are cached.
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
1-6번 동일
7. 추가한 term_id 에 대해 FK 설정
8. term_id 기반 새로운 인덱스 생성
9. 기존 term 컬럼 기반 인덱스 제거
10. 기존 term 컬럼 제거
저는 이렇게 생각해봤습니다 ! term_id 관련 작업을 진행하고 term을 제거하는 식으로 가면 잘 되지 않을까 생각합니다.
| @UniqueConstraint( | ||
| name = "uk_term_name", | ||
| columnNames = {"name"} | ||
| ) |
There was a problem hiding this comment.
전 암묵적으로 UK에 속하는 필드가 1개면 @Column의 unique 옵션을 사용하고 2개 이상이라면 @UniqueConstraint 를 사용했습니다 ! 만약 UK 필드가 1개라면 unique 옵션을 사용하는 건 어떻게 생각하시나요 ?
There was a problem hiding this comment.
엇.. 이거 한 번 이야기해보죠 회의 때!
There was a problem hiding this comment.
저도 단일 필드 유니크는 unique 옵션으로 사용한 적이 많아서 더 익숙합니다!
| private Long id; | ||
|
|
||
| @Column(length = 20) | ||
| private String name; |
There was a problem hiding this comment.
엔티티 또는 DTO에 @Pattern 으로 XXXX-X 형식의 정규표현식 검증이 있으면 더욱 좋을 거 같습니다!
There was a problem hiding this comment.
이게 추가학기의 경우는 2025-1-a 이런식으로 되어있어서 굳이 정규표현식 검증 안하긴 했습니다..
필요할 거 같으면 어드민 API에서 dto에서 검증하겠습니다~
- 수정 필요
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/V35__migrate_term_to_table.sql(1 hunks)
⏰ 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
| ALTER TABLE application | ||
| MODIFY COLUMN term_id BIGINT NOT NULL; | ||
|
|
||
| ALTER TABLE mentor | ||
| MODIFY COLUMN term_id BIGINT NOT NULL; | ||
|
|
||
| ALTER TABLE university_info_for_apply | ||
| MODIFY COLUMN term_id BIGINT NOT NULL; | ||
|
|
There was a problem hiding this comment.
새 term_id 전환 후 기존 term 컬럼이 남아 신규 insert를 막습니다.
이번 PR에서 Application/Mentor/UnivApplyInfo 엔터티가 문자열 term 필드를 제거했는데, 마이그레이션에서는 해당 컬럼을 그대로 두고 NOT NULL 제약도 유지됩니다. 배포 후 새 레코드를 저장하면 JPA가 term 값을 넣지 않아 Field 'term' doesn't have a default value 오류가 즉시 발생합니다. 이번 스크립트 안에서 term 관련 인덱스를 먼저 제거한 뒤 ALTER TABLE … DROP COLUMN term;(또는 최소한 nullable 처리)까지 완료해야 합니다. 미루면 신규 쓰기가 전부 실패하니 꼭 이번 단계에서 정리해 주세요.
+-- 예시: 기존 term 컬럼 및 관련 인덱스 제거
+ALTER TABLE application DROP INDEX idx_app_user_term_delete;
+ALTER TABLE application DROP INDEX idx_app_first_choice_term_search;
+ALTER TABLE application DROP INDEX idx_app_second_choice_term_search;
+ALTER TABLE application DROP INDEX idx_app_third_choice_term_search;
+ALTER TABLE application DROP COLUMN term;
+
+ALTER TABLE mentor DROP COLUMN term;
+ALTER TABLE university_info_for_apply DROP COLUMN term;📝 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.
| ALTER TABLE application | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| ALTER TABLE mentor | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| ALTER TABLE university_info_for_apply | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| ALTER TABLE application | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| -- 예시: 기존 term 컬럼 및 관련 인덱스 제거 | |
| ALTER TABLE application DROP INDEX idx_app_user_term_delete; | |
| ALTER TABLE application DROP INDEX idx_app_first_choice_term_search; | |
| ALTER TABLE application DROP INDEX idx_app_second_choice_term_search; | |
| ALTER TABLE application DROP INDEX idx_app_third_choice_term_search; | |
| ALTER TABLE application DROP COLUMN term; | |
| ALTER TABLE mentor | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| ALTER TABLE mentor DROP COLUMN term; | |
| ALTER TABLE university_info_for_apply | |
| MODIFY COLUMN term_id BIGINT NOT NULL; | |
| ALTER TABLE university_info_for_apply DROP COLUMN term; |
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V35__migrate_term_to_table.sql around lines
57 to 65, the migration leaves the old string term column in place (and keeps
NOT NULL), which will cause new inserts to fail because JPA no longer sets term;
fix it by first dropping any indexes/constraints referencing the term column for
application, mentor, and university_info_for_apply, then either ALTER TABLE ...
DROP COLUMN term for each table (preferred) or ALTER TABLE ... MODIFY COLUMN
term VARCHAR(...) NULL to make it nullable before removing the column in a later
migration; ensure index drops occur before the DROP COLUMN and verify column
names and index names match the schema.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/resources/db/migration/V36__migrate_term_to_table.sql (3)
41-54: 기존 term을 term_id로 매핑하는 INNER JOIN UPDATE 성능 고려.대용량 데이터를 다루는 세 테이블에서 모두 동일한 패턴의 UPDATE가 실행됩니다. 현재 구조:
- application, mentor, university_info_for_apply 각각에서 INNER JOIN을 통한 UPDATE 수행
- 기존 term 컬럼에 인덱스가 있다면 효율적이겠지만, 없다면 풀 스캔 가능성
검증: 기존 스키마에서 term 컬럼의 인덱스 존재 여부를 확인하고, 필요시 마이그레이션 전 임시 인덱스 생성을 고려하세요.
93-151: FK 드롭 동적 SQL 패턴의 복잡도 및 유지보수성.같은 패턴이 6번 반복됩니다 (@drop_fk1~4, PREPARE/EXECUTE/DEALLOCATE 반복):
SET @drop_fk_n = (SELECT CONCAT(...) FROM INFORMATION_SCHEMA...); SET @drop_fk_n = IFNULL(@drop_fk_n, 'SELECT 1'); PREPARE stmt_n FROM @drop_fk_n; EXECUTE stmt_n; DEALLOCATE PREPARE stmt_n;개선 아이디어:
이 패턴을 저장 프로시저나 Flyway 자체의 플레이스홀더 기능으로 추상화하면 가독성과 유지보수성이 향상될 수 있습니다. 그러나 현재 단계에서는 동작하는 것을 우선하므로, 적어도 변수명을 더 명확하게 (@drop_fk_first_choice, @drop_fk_second_choice 등) 개선하세요.
1-183: 마이그레이션 검증 및 롤백 전략 가이드 추가 권장.1. 마이그레이션 후 검증:
- 모든 application, mentor, university_info_for_apply 행이 유효한 term_id를 가지는지 확인
- 고아 레코드(term_id = NULL) 유무 체크
- FK 제약 조건 검증 쿼리
2. 롤백 고려사항:
- 마이그레이션이 실패한 경우 롤백 스크립트나 가이드 필요
- 특히 FK 삭제 후 인덱스 변경 구간에서 중단되면 복구가 복잡할 수 있음
- 프로덕션 배포 전 스테이징 환경에서 충분한 테스트 필수
3. 운영 가이드:
- 이 마이그레이션을 실행하기 전 데이터베이스 백업 필수
- 마이그레이션 실행 시간 및 예상 다운타임 공지
- 마이그레이션 후 검증 체크리스트 작성
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/V36__migrate_term_to_table.sql(1 hunks)
⏰ 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 (1)
src/main/resources/db/migration/V36__migrate_term_to_table.sql (1)
79-90: 기존 인덱스와 신규 인덱스의 생성·삭제 순서 검증 필요.변경 내용:
- 라인 79-90: term_id 기반 신규 인덱스 3개 생성 (idx_app_first_choice_term_id_search 등)
- 라인 154-157: 기존 term 기반 인덱스 4개 삭제 (idx_app_first_choice_search 등)
확인 사항:
- 신규 인덱스가 이미 존재하는지 확인 (CREATE INDEX 시 IF NOT EXISTS 미사용)
- 기존 인덱스 삭제 시 다른 쿼리의 동시 실행으로 인한 성능 영향
- 인덱스 이름이 실제 스키마와 정확히 일치하는지 확인
Also applies to: 154-157
| INSERT INTO term (name, is_current) | ||
| VALUES ('2026-1', TRUE) | ||
| ON DUPLICATE KEY UPDATE is_current = TRUE; |
There was a problem hiding this comment.
현재 학기('2026-1')가 마이그레이션 스크립트에 하드코딩되어 있습니다.
마이그레이션 실행 시점에 관계없이 '2026-1'이 강제로 현재 학기로 설정되는 구조입니다. 다음 우려 사항이 있습니다:
- 실행 시기에 맞지 않는 term 값 설정 가능성
- 다양한 학기 형식이나 변경 요청 시 스크립트 재수정 필요
- 향후 관리자 API 추가 시 일관성 문제 가능
제안: 마이그레이션 가이드 또는 환경 변수/매개변수화 방식을 통해 현재 학기를 지정하도록 개선하거나, 적어도 주석에 수동 조정 필요성을 명시하세요.
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V36__migrate_term_to_table.sql around lines
26 to 28, the migration hardcodes the current term as '2026-1', which forces
that value on every run; change the script to avoid a fixed term by either
reading the current term from an environment variable or migration parameter
(and using that variable in the INSERT ... VALUES), or add a clear top-of-file
comment instructing operators to set the term value manually before running and
documenting the expected format; ensure the chosen approach validates non-empty
input and uses a safe UPSERT (ON DUPLICATE KEY UPDATE) with the provided value
rather than a literal.
There was a problem hiding this comment.
@coderabbitai
이 스크립트 반영되는 시기엔 2026-1로 할거야
There was a problem hiding this comment.
@Gyuhyeok99, 알겠습니다! 배포 시점에 2026-1 학기가 적용될 예정이라면 현재 스크립트가 적절하네요. 확인해주셔서 감사합니다. 👍
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/resources/db/migration/V36__migrate_term_to_table.sql (3)
42-56: 데이터 마이그레이션 완성도 검증 추가 권장마이그레이션의 핵심 단계인 기존 term 값을 term_id로 변환하는 부분에서, 다음과 같은 개선이 있으면 좋겠습니다:
- 검증 로직 부재: UPDATE 이후 각 테이블에서 여전히 term_id가 NULL인 행이 있는지 확인하는 절차가 없습니다.
- 부분 마이그레이션 위험성: WHERE 절로 이미 마이그레이션된 행은 재처리하지 않지만, 혹시 놓친 행이 있을 수 있습니다.
제안:
다음을 추가하여 마이그레이션 완전성을 보증하세요:
- 각 UPDATE 후 NULL 체크 (SELECT COUNT(*) WHERE term_id IS NULL)
- 실패 시 트랜잭션 롤백 전략
- 로그/확인용 SELECT 쿼리 주석으로 포함
-- 각 UPDATE 후 NULL 검증 추가 SELECT COUNT(*) AS null_count FROM application WHERE term_id IS NULL; SELECT COUNT(*) AS null_count FROM mentor WHERE term_id IS NULL; SELECT COUNT(*) AS null_count FROM university_info_for_apply WHERE term_id IS NULL; -- 위 쿼리가 모두 0을 반환해야 진행
94-153: 동적 SQL 기반 FK 삭제 로직 복잡도 및 안정성 우려학기 정보와 관련된 기존 FK를 조건부로 삭제하는 로직(lines 94~153)이 상당히 복잡하며, 몇 가지 우려 사항이 있습니다:
- 중복되는 패턴: 4개의 FK 삭제가 거의 동일한 PREPARE/EXECUTE 사이클을 반복합니다.
- 조용한 실패 가능성: INFORMATION_SCHEMA 쿼리가 결과를 찾지 못할 경우 IFNULL로 'SELECT 1'을 실행하는데, 이는 실제 오류를 감춘 채 진행할 수 있습니다.
- FK 명명 규칙 의존성: CONSTRAINT_NAME을 동적으로 조회하는데, 기존 제약 조건이 예상과 다른 이름일 경우 문제 발생.
개선 방안:
- 동적 SQL을 한 번의 PROCEDURE 또는 더 간단한 로직으로 통합
- 또는 명시적 FK 이름이 고정되어 있다면 직접 DROP 사용
- 에러 로깅 추가하여 FK 삭제 실패 감지
-- 현재 방식 (복잡): -- 4개의 PREPARE/EXECUTE 반복 -- 개선 방안 1: 고정 FK 이름 사용 (가능한 경우) ALTER TABLE application DROP FOREIGN KEY IF EXISTS fk_application_first_choice_university_info_for_apply_id; -- ...기타 FK도 동일 -- 개선 방안 2: 단일 동적 SQL 절차 -- (복잡도는 유사하지만 유지보수성 향상)
1-185: 마이그레이션 전략 전반에 대한 롤백 계획 및 문서화이번 마이그레이션은 여러 테이블을 동시에 수정하는 대규모 작업이므로, 다음 사항이 권장됩니다:
- 롤백 스크립트: 마이그레이션 실패 시 원래 상태로 복구하는 역방향 마이그레이션 스크립트(V37__rollback_term_migration.sql 등)를 준비하세요.
- 실행 전 백업: 프로덕션 환경에서 마이그레이션 실행 전 데이터베이스 전체 백업을 권장합니다.
- 단계별 검증:
- 각 UPDATE 후 행 수 확인
- FK 재생성 이후 데이터 무결성 체크 (참조 정합성 검증)
- 인덱스 생성 후 쿼리 성능 확인
- 배포 시점 재확인: 하드코딩된 '2026-1' 외 추가 학기가 필요하면 스크립트 재조정 필요.
이는 필수 사항은 아니지만, 대규모 마이그레이션 안정성을 위해 강력히 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/V36__migrate_term_to_table.sql(1 hunks)
🔇 Additional comments (2)
src/main/resources/db/migration/V36__migrate_term_to_table.sql (2)
81-93: 인덱스 생성 시점 및 FK 제약 조건 순서 확인새로운 term_id 기반 인덱스를 lines 81
93에서 생성하는데, FK 제약 조건은 lines 6879에서 이미 추가되었으므로 시간 순서상 문제는 없습니다. 다만 다음을 확인하세요:
- FK 제약 추가 이후 인덱스 생성이 더 명확: 현재 순서(FK 먼저, 인덱스 나중)가 MySQL에서 정상 작동하지만, 일반적 관례상 인덱스는 테이블 구조 확정 후 추가하는 것이 맞습니다.
- 인덱스 이름 충돌 여부: 기존 인덱스 삭제(lines 155~159) 이후 새 인덱스 생성이므로 이름 충돌은 없습니다. ✓
코멘트: 전체 시퀀스 흐름상 논리적으로는 안전해 보이며, 추가 조치는 필수적이지 않습니다.
155-164: 기존 인덱스 및 컬럼 삭제 전 최종 검증 필요term 컬럼과 관련 인덱스를 삭제하는 단계(lines 155~164)에서 다음을 점검하세요:
- 인덱스 이름 확인: 삭제할 인덱스 4개(idx_app_user_term_delete, idx_app_first/second/third_choice_search)가 현재 DB에 실제로 존재하는가?
- term 컬럼 의존성: application, mentor, university_info_for_apply 테이블의 term 컬럼을 참조하는 다른 객체(뷰, 트리거 등)가 없는가?
제안:
-- 마이그레이션 전 사전 점검 쿼리 SHOW INDEXES FROM application WHERE Key_name IN ( 'idx_app_user_term_delete', 'idx_app_first_choice_search', 'idx_app_second_choice_search', 'idx_app_third_choice_search' );실제 환경에서는 스크립트 실행 전에 위 쿼리로 기존 인덱스 존재 여부를 사전 확인하기 바랍니다.
# Conflicts: # src/main/resources/secret # src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (1)
34-52: 일관성을 위해 helper 메서드를 사용하는 것을 고려하세요.이 메서드는 Line 48에서
univApplyInfo.termId.eq(termId)로 직접 비교하고 있지만,findAllByFilter(Line 93)와findAllByText(Line 159)는termIdEqhelper 메서드를 사용합니다.인터페이스의 타입이
Long으로 통일되면(이전 리뷰 코멘트 참조) 이 메서드도 nullable termId를 받게 되므로, 다른 메서드들과 동일하게 helper를 사용하는 것이 일관성과 null 처리 측면에서 바람직합니다.다음 diff를 적용하세요:
.where( regionCodeEq(country, regionCode) .and(countryOrUniversityContainsKeyword(country, university, keywords)) - .and(univApplyInfo.termId.eq(termId)) + .and(termIdEq(univApplyInfo, termId)) )src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (1)
82-99: 1. 현재 학기 Term 픽스처 초기화 검토문장 단위로 말씀드릴게요.
현재setUp()에서termFixture.현재_학기("2025-2")가 호출될 때마다 DB에 동일한 학기를 계속 삽입한다면, 학기명에 유니크 제약이 있거나is_current=true가 하나만 허용되는 경우 테스트가 중복 데이터로 실패할 수 있습니다.
테스트 실행마다 기존 학기를 재사용하거나, 픽스처가 중복 생성을 방지하도록 조정해 주세요.src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java (1)
51-146: 1. 현재 학기 조회 로직 중복 제거 제안친절하게 한 문장씩 안내드릴게요.
getApplicants,getApplicantsByUserApplications,validateSiteUserCanViewApplicants세 곳에서 동일한 현재 학기 조회 코드가 반복되고 있어요.
private Term getCurrentTerm()과 같은 헬퍼를 두면 예외 처리와 로깅을 한 곳에서 관리할 수 있어 유지보수성이 높아집니다.
필요하다면 캐싱 정책 변경 시에도 한 군데만 손보면 돼서 위험을 줄일 수 있답니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/example/solidconnection/application/domain/Application.java(6 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java(7 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/mentor/domain/Mentor.java(1 hunks)src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java(1 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java(1 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java(7 hunks)src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java(23 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
- src/main/java/com/example/solidconnection/mentor/domain/Mentor.java
- src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-26T05:07:54.817Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:0-0
Timestamp: 2025-08-26T05:07:54.817Z
Learning: 대학지원정보 텍스트 검색에서 region과 country 간의 INNER JOIN은 데이터 무결성을 위한 의도된 설계이다. country.region_code가 region 테이블에 존재하지 않는 경우는 잘못된 데이터 상태로 간주하여 검색 결과에서 제외한다.
Applied to files:
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
📚 Learning: 2025-08-26T05:00:56.556Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:166-178
Timestamp: 2025-08-26T05:00:56.556Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.
Applied to files:
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.javasrc/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java
📚 Learning: 2025-08-20T13:10:27.822Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 471
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:80-85
Timestamp: 2025-08-20T13:10:27.822Z
Learning: University와 Country 엔티티는 EAGER 연관관계로 설정되어 있어서 fetchJoin() 없이도 자동으로 함께 로딩된다.
Applied to files:
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
⏰ 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 (3)
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (3)
78-111: 잘 구현되었습니다.
termIdEqhelper 메서드를 올바르게 활용하고 있으며, 언어 시험 점수 필터링 로직도 명확하게 분리되어 있습니다.
122-127: null 처리가 적절합니다.Helper 메서드가 wrapper
Long을 받아 null check를 수행하도록 올바르게 구현되었습니다. 이는 termId 기반 필터링이 선택적일 수 있음을 의미론적으로 잘 표현하고 있습니다.
147-186: 텍스트 검색 로직이 잘 구성되었습니다.
termIdEqhelper를 통한 termId 필터링이 올바르게 적용되었고, 기존의 텍스트 매칭 및 우선순위 정렬 로직도 유지되고 있습니다.Based on learnings (텍스트 검색에서 UnivApplyInfo.koreanName 사용 및 region과 country 간 INNER JOIN은 의도된 설계).
| List<UnivApplyInfo> findAllByRegionCodeAndKeywordsAndTermId(String regionCode, List<String> keywords, long term); | ||
|
|
||
| List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, String term, List<String> countryKoreanNames); | ||
| List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, Long termId, List<String> countryKoreanNames); | ||
|
|
||
| List<UnivApplyInfo> findAllByText(String text, String term); | ||
| List<UnivApplyInfo> findAllByText(String text, Long termId); |
There was a problem hiding this comment.
타입과 파라미터명의 일관성을 맞춰주세요.
세 메서드 시그니처 간에 불일치가 있습니다:
- 타입 불일치: Line 9는 primitive
long을, Lines 11과 13은 wrapperLong을 사용합니다. - 파라미터명 불일치: Line 9는
term으로, Lines 11과 13은termId로 명명되어 있습니다.
primitive long은 null을 표현할 수 없어서 구현체의 null check 로직(Line 122-127)과 맞지 않으며, 세 메서드 모두 termId 기반 필터링이라는 동일한 의미를 가지므로 일관된 시그니처를 사용해야 합니다.
다음 diff를 적용하여 일관성을 확보하세요:
- List<UnivApplyInfo> findAllByRegionCodeAndKeywordsAndTermId(String regionCode, List<String> keywords, long term);
+ List<UnivApplyInfo> findAllByRegionCodeAndKeywordsAndTermId(String regionCode, List<String> keywords, Long termId);📝 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.
| List<UnivApplyInfo> findAllByRegionCodeAndKeywordsAndTermId(String regionCode, List<String> keywords, long term); | |
| List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, String term, List<String> countryKoreanNames); | |
| List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, Long termId, List<String> countryKoreanNames); | |
| List<UnivApplyInfo> findAllByText(String text, String term); | |
| List<UnivApplyInfo> findAllByText(String text, Long termId); | |
| List<UnivApplyInfo> findAllByRegionCodeAndKeywordsAndTermId(String regionCode, List<String> keywords, Long termId); | |
| List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, Long termId, List<String> countryKoreanNames); | |
| List<UnivApplyInfo> findAllByText(String text, Long termId); |
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java
around lines 9 to 13, make the method signatures consistent by changing the
first method to use the Long wrapper and the same parameter name as the others:
replace the primitive long term with Long termId so all three methods use Long
termId; update any references in callers/implementations if necessary to the new
parameter name to preserve nullability and match the existing null-check logic
in the implementation (lines ~122-127).
fix: PostRepository & CommentRepository의 일부 메서드의 정렬 순서 오류 해결 (#522) refactor: 이메일 유니크키 제거 refactor: 엔티티가 BaseEntity를 상속하도록 (#524) fix: BaseEntity 관련 마이그레이션 파일 수정 (#530) feat: 멘토 승격 api 구현 (#532) fix: 대학교 중복 오류 수정 (#510) refactor: 불필요한 로그 삭제 (#543) fix: 운영환경 8081 포트 설정 추가 (#542) fix: dev환경 디비명 변경 (#546) refactor: 불필요한 로그 삭제 (#547) refactor: 학기를 테이블로 관리하도록 변경 (#526) refactor: 모의지원 시 지원한 대학 정보 응답 추가 (#539)
3 participants
관련 이슈
작업 내용
변경 사항이 상당합니다.. 작업의 히스토리는 그동안 현재 학기를 yml로 관리해서 학기가 바뀔 때마다 yml 수정해서 secret에 pr날리고 서브모듈 업데이트해서 또 pr을 날리는데 이걸 dev -> release -> master까지 해줘야했습니다.
학기 하나 수정하자고 너무 불필요한 작업들이 많은 거 같아 join을 한 번 더 하더라도 테이블로 관리하자고 이야기가 나왔어서 미루고 미루다 이번에 작업했습니다!
이게 반영되면 추후 관리자 api로 학기 변경하는 기능도 개발하도록 하겠습니다!
특이 사항
flyway 스크립트를 아직 작성하지 못했습니다..
과정이
인데 이 7-8번 과정을 어떻게 해야하는지 잘 모르겠네요.. 그냥 삭제하려하니까 fk 문제도 걸리더라구요..
이걸 어떻게 성공적으로 마이그레이션할지 잘 고민해야할 거 같습니다
리뷰 요구사항 (선택)
변경사항이 상당한 노가다였어서.. 실수한 부분이 존재할 수 있긴 합니다..! 저도 다시 검토해보겠습니다!