refactor: University 도메인 리팩터링 #338
Conversation
whqtker
requested review from
Gyuhyeok99,
lsy1307,
nayonsoso and
wibaek
as code owners
|
""" Walkthrough
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (35)
💤 Files with no reviewable changes (4)
🚧 Files skipped from review as they are similar to previous changes (31)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java (1)
79-83: 🛠️ Refactor suggestion① EAGER 로딩은 N+1 위험·과도한 메모리 사용을 부릅니다
한 문장:@OneToMany와@ManyToOne에서FetchType.EAGER를 사용하면 조회 시 관계 전체가 즉시 로드되어 성능이 급락하고 N+1 쿼리가 발생할 수 있습니다.- @OneToMany(mappedBy = "univApplyInfo", fetch = FetchType.EAGER) + @OneToMany(mappedBy = "univApplyInfo", fetch = FetchType.LAZY) - @ManyToOne(fetch = FetchType.EAGER) + @ManyToOne(fetch = FetchType.LAZY)src/main/java/com/example/solidconnection/university/service/UniversityLikeService.java (1)
38-52: 🛠️ Refactor suggestion① 동시에 눌렀을 때 중복 예외가 DB에서만 터질 수 있습니다
단락: 선행 중복 체크 후 저장-로직은 트랜잭션 경합 시 무력화되어DataIntegrityViolationException이 발생할 수 있습니다. 서비스 단에서 예외를 캐치하여 동일 응답을 내려주면 UX 일관성이 유지됩니다.try { likedUniversityRepository.save(likedUniversity); } catch (DataIntegrityViolationException e) { throw new CustomException(ALREADY_LIKED_UNIVERSITY); }src/main/java/com/example/solidconnection/university/repository/custom/UniversityFilterRepositoryImpl.java (1)
98-109:⚠️ Potential issue3️⃣
testType이 null-일 때 NPE 발생
testScore는 존재하지만testType이 null인 경우compareMyTestScoreToMinPassScore에서testType.compare(...)호출 시 NPE가 발생합니다. 안전 가드를 하나 더 추가해 주세요.- return filteredUnivApplyInfo.stream() - .filter(uifa -> compareMyTestScoreToMinPassScore(uifa, testType, testScore) >= 0) - .toList(); + if (testType == null) { + return Collections.emptyList(); // 또는 필터링 없이 반환 + } + return filteredUnivApplyInfo.stream() + .filter(uifa -> compareMyTestScoreToMinPassScore(uifa, testType, testScore) >= 0) + .toList();src/main/java/com/example/solidconnection/university/service/UniversityRecommendService.java (1)
55-60: 🛠️ Refactor suggestion4️⃣ 추천 리스트 길이 검증 누락으로
IndexOutOfBoundsException위험
generalRecommend.subList(0, RECOMMEND_UNIVERSITY_NUM - alreadyPicked.size())호출 전에generalRecommend의 크기를 확인하지 않으면 후보 수가 부족할 때 런타임 예외가 발생합니다.- return generalRecommend.subList(0, RECOMMEND_UNIVERSITY_NUM - alreadyPicked.size()); + int need = RECOMMEND_UNIVERSITY_NUM - alreadyPicked.size(); + return generalRecommend.subList(0, Math.min(need, generalRecommend.size()));
🧹 Nitpick comments (23)
src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (1)
74-76: 2️⃣ 필드 타입 변경은 OK, 하지만 픽스처 명칭이 어색합니다.
UnivApplyInfo로 필드 타입을 교체한 점은 일관성이 확보되어 좋습니다.
다만 여전히UniversityInfoForApplyFixture클래스를 사용하고 있어, 타입/클래스 명이 서로 달라 혼동될 수 있습니다.
테스트 유지보수를 위해 픽스처 이름도UnivApplyInfoFixture등으로 맞춰 두는 편이 가독성에 더 도움이 됩니다.src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java (1)
49-49: 2️⃣ 변수 제네릭 타입 변경 OK, 추가 검증 권장
List<UnivApplyInfo>로 타입을 맞춘 점은 문제없습니다.
다만 추천 로직에서 N ≥ 데이터 수일 때 빈 리스트가 반환되지 않는지 단위 테스트를 하나 더 추가하면 안전합니다.src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java (1)
58-65: 2️⃣ 변수명 일관성 GOOD, 레포지토리·메서드 명 불일치변수 타입을
UnivApplyInfo로 맞춘 점은 적절합니다.
하지만UniversityInfoForApplyRepository#getUniversityInfoForApplyByIdAndTerm메서드 시그니처는 옛 이름을 그대로 사용합니다.
장기적으로는UnivApplyInfoRepository#getUnivApplyInfoByIdAndTerm처럼 레포지토리·메서드 명도 동일한 도메인 용어로 통일하는 편이 혼란을 줄입니다.
src/main/java/com/example/solidconnection/university/service/GeneralUniversityRecommendService.java (1)
25-26: 2️⃣ 필드 타입 일치 OK, 레포지토리 이름은 구 버전
recommendUniversities컬렉션 타입 변경은 문제없으나, 아직UniversityInfoForApplyRepository라는 구명칭 레포지토리를 주입받고 있습니다.
향후 리팩터링 시 레포지토리 인터페이스도 새로운 도메인명으로 변경하여 명확성을 높여 주세요.src/test/java/com/example/solidconnection/university/service/UniversityQueryServiceTest.java (1)
49-56: 가독성을 높이기 위한 변수명 영문화 제안
- 테스트 코드 내 한글 변수명이 지나치게 길어 IDE-자동접힘·diff 확인 시 불편함이 큽니다.
- 예)
괌대학_A_지원_정보→guamUnivAApplyInfo등.- 국제 협업 시 언어 혼용으로 리뷰 속도가 저하될 수 있습니다.
- 전체 테스트에서 동일 패턴이 반복되므로 빌더에서 별칭을 반환하거나, 변수명만 영문화해도 효과가 큽니다.
Also applies to: 61-69, 88-109, 137-149, 155-167, 173-187, 192-205
src/test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java (1)
55-61: Builder‧필드명 일관성 유지 필요
- 한글 + 긴 변수명은 앞선 테스트와 동일한 가독성 이슈가 있습니다.
- 추천 로직 자체는 훌륭하므로, 필드명을 영문화해 본질적 코드 품질만 끌어올리면 좋겠습니다.
src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java (1)
85-87: ② 파라미터명 혼동 주의 – 복수형 맞추기
addLanguageRequirements(LanguageRequirement languageRequirements)의 파라미터는 단수 객체인데 이름은 복수라 가독성이 떨어집니다.- public void addLanguageRequirements(LanguageRequirement languageRequirements) { - this.languageRequirements.add(languageRequirements); + public void addLanguageRequirement(LanguageRequirement languageRequirement) { + this.languageRequirements.add(languageRequirement); }src/main/java/com/example/solidconnection/application/dto/UniversityApplicantsResponse.java (1)
14-20: ① 매개변수명 오타 – 리스트는 복수형이 자연스럽습니다
한 문장:applicant→applicants로 변경하면 의미가 명확해집니다.- public static UniversityApplicantsResponse of(UnivApplyInfo univApplyInfo, List<ApplicantResponse> applicant) { + public static UniversityApplicantsResponse of(UnivApplyInfo univApplyInfo, List<ApplicantResponse> applicants) { return new UniversityApplicantsResponse( univApplyInfo.getKoreanName(), univApplyInfo.getStudentCapacity(), univApplyInfo.getUniversity().getRegion().getKoreanName(), univApplyInfo.getUniversity().getCountry().getKoreanName(), - applicant); + applicants); }src/main/java/com/example/solidconnection/university/service/UniversityLikeService.java (1)
58-67: ② 메서드명·파라미터명 레거시 잔재
한 문장:universityInfoForApplyId는 과거 타입명을 노출합니다;univApplyInfoId로 교체하면 일관성이 생깁니다.src/main/java/com/example/solidconnection/university/service/UniversityQueryService.java (1)
52-60: ② 반환 DTO 명칭의 과거 이름 유지
Sentence: 메서드 시그니처는UniversityInfoForApplyPreviewResponses를 그대로 사용하고 있어 신규 네이밍과 혼재됩니다. 전체 DTO 리네이밍을 한 묶음으로 처리하면 혼동이 사라집니다.src/main/java/com/example/solidconnection/university/dto/UniversityInfoForApplyPreviewResponse.java (2)
19-25: 정렬 로직 간결화 및 불변 리스트 보장 제안
- 스트림에서 바로
sorted()→toList()로 마무리하면ArrayList생성·Collections.sort호출을 제거해 가독성과 성능을 모두 챙길 수 있습니다.toList()는 불변 리스트를 반환하기 때문에 DTO 내 컬렉션 불변성을 자연스럽게 확보할 수 있습니다.- List<LanguageRequirementResponse> languageRequirementResponses = new java.util.ArrayList<>( - univApplyInfo.getLanguageRequirements().stream() - .map(LanguageRequirementResponse::from) - .toList()); - Collections.sort(languageRequirementResponses); + List<LanguageRequirementResponse> languageRequirementResponses = + univApplyInfo.getLanguageRequirements().stream() + .map(LanguageRequirementResponse::from) + .sorted() + .toList();
8-8: DTO 이름이 도메인 변경과 불일치엔티티가
UnivApplyInfo로 교체된 만큼 DTO 명도UnivApplyInfoPreviewResponse등으로 맞춰 두면 팀원이 한눈에 매핑 관계를 파악하기 쉽습니다. 대대적 리네이밍 부담이 없다면 검토 부탁드립니다.src/test/java/com/example/solidconnection/university/fixture/LanguageRequirementFixtureBuilder.java (1)
30-33: 메서드 시그니처와 필드명 불일치메서드명이 여전히
universityInfoForApply로 남아 있어 빌더 사용 시 혼동을 유발합니다.univApplyInfo로 맞춰 주세요.- public LanguageRequirementFixtureBuilder universityInfoForApply(UnivApplyInfo univApplyInfo) { + public LanguageRequirementFixtureBuilder univApplyInfo(UnivApplyInfo univApplyInfo) {src/test/java/com/example/solidconnection/university/repository/UniversityLikeRepositoryTest.java (1)
36-49: 제약조건 검증 시점 확실히 하기
save만 호출하면 트랜잭션 커밋 시점까지 INSERT 가 지연되어DataIntegrityViolationException이 잡히지 않을 수 있습니다. 즉시 예외를 확인하려면saveAndFlush또는flush호출을 추가해 주세요.- assertThatCode(() -> likedUniversityRepository.save(secondLike)) + assertThatCode(() -> likedUniversityRepository.saveAndFlush(secondLike)) .isInstanceOf(DataIntegrityViolationException.class);src/main/java/com/example/solidconnection/university/dto/UniversityDetailResponse.java (1)
54-56: 언어 요구사항 리스트 정렬·불변화 적용 고려
UniversityInfoForApplyPreviewResponse와 동일한 패턴입니다. 정렬된 불변 리스트를 반환하면 응답 DTO 신뢰도가 높아집니다.- univApplyInfo.getLanguageRequirements().stream() - .map(LanguageRequirementResponse::from) - .toList(), + univApplyInfo.getLanguageRequirements().stream() + .map(LanguageRequirementResponse::from) + .sorted() + .toList(),src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java (1)
123-132: 2️⃣ 람다 변수명·레포지토리 네이밍 일관성 제안
1. 람다 내부 변수명이universityInfoForApply로 남아 있어 가독성이 떨어집니다.univApplyInfo로 바꿔주세요.
2. 주입받는UniversityInfoForApplyRepository역시 레거시 명칭이라 혼동의 여지가 있습니다. 후속 PR에서UnivApplyInfoRepository로 리네이밍하면 팀원 이해도가 높아질 것 같습니다.- .map(universityInfoForApply -> UniversityApplicantsResponse.of( - universityInfoForApply, - findApplicationsByChoice.apply(universityInfoForApply) ... + .map(univApplyInfo -> UniversityApplicantsResponse.of( + univApplyInfo, + findApplicationsByChoice.apply(univApplyInfo) ... )src/main/java/com/example/solidconnection/university/domain/LikedUniversity.java (1)
34-39: 지연(LAZY) 로딩 옵션으로 N+1 가능성 줄이기
현재@ManyToOne기본값(EAGER)이라 조회마다 대학·사용자 엔티티가 즉시 로딩됩니다. 좋아요 리스트를 한꺼번에 읽을 때 N+1이 재현될 수 있으니fetch = FetchType.LAZY를 권장드립니다.- @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "university_info_for_apply_id") - private UnivApplyInfo univApplyInfo; + private UnivApplyInfo univApplyInfo; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) private SiteUser siteUser;src/test/java/com/example/solidconnection/support/integration/BaseIntegrationTest.java (1)
479-490: 2️⃣ 중복save호출로 인한 성능·일관성 저하 우려
univApplyInfo는 이미 영속 상태이므로 컬렉션에 요소를 추가한 뒤save(univApplyInfo)를 다시 호출할 필요가 없습니다. JPAflush시점에 연관 객체가 자동으로 영속화됩니다. 중복 호출을 제거하면 쿼리 1회를 줄여 성능과 가독성을 개선할 수 있습니다.- univApplyInfo.addLanguageRequirements(languageRequirement); - universityInfoForApplyRepository.save(univApplyInfo); - languageRequirementRepository.save(languageRequirement); + univApplyInfo.addLanguageRequirements(languageRequirement); + languageRequirementRepository.save(languageRequirement); // cascade 적용 시 이 줄도 생략 가능src/test/java/com/example/solidconnection/application/fixture/ApplicationFixtureBuilder.java (1)
21-55: 5️⃣ 빌더 체인에서 객체 새로 생성 – 의도 확인 필요
application()메서드가new ApplicationFixtureBuilder(...)를 반환하기 때문에, 앞서 설정한 필드들이 초기화된 빌더 인스턴스가 사라집니다. 체이닝 사용 시 혼란을 줄 수 있으니return this;로 바꾸거나 메서드명을newBuilder()등으로 명확히 해 주세요.src/test/java/com/example/solidconnection/university/fixture/UniversityInfoForApplyFixture.java (1)
3-12: 1. Fixture 클래스 네이밍 불일치
- 파일·클래스 이름은 여전히UniversityInfoForApplyFixture로 남아 있어 새 엔티티명과 어긋납니다.
- 후속 PR에서 클래스/메서드 이름을UnivApplyInfoFixture등으로 통일하면 가독성이 올라갑니다.src/main/java/com/example/solidconnection/university/repository/UniversityInfoForApplyRepository.java (2)
25-47: 2. 메서드·인터페이스 네이밍 레거시
- 인터페이스명 및 일부 메서드명(예:findUniversityInfoForApplies...)이 기존 명칭을 유지합니다.
- 기능에는 영향 없으나, 이후 일괄 리네임을 권장합니다.
56-64: 3. default 메서드 내부 예외 코드 상수 유지
- 예외 코드 상수는 기존 명칭(UNIVERSITY_INFO_FOR_APPLY_NOT_FOUND)을 그대로 사용해도 문제없으나, 통일성을 위해 상수도 교체할지 검토해 주세요.src/main/java/com/example/solidconnection/application/domain/Application.java (1)
88-112: 2. 생성자·업데이트 메서드 파라미터 순서 가독성
- 동일한 타입이 연속으로 세 번 등장하면서 실수로 순서를 바꿔 넘길 위험이 있습니다.
1) 레코드/DTO 도입
2) 빌더 패턴 활용
- 중·장기적으로 고려해 보시면 유지보수성이 좋아집니다.Also applies to: 129-133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
src/main/java/com/example/solidconnection/application/domain/Application.java(5 hunks)src/main/java/com/example/solidconnection/application/dto/UniversityApplicantsResponse.java(2 hunks)src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java(2 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java(3 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java(2 hunks)src/main/java/com/example/solidconnection/siteuser/repository/LikedUniversityRepository.java(2 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(1 hunks)src/main/java/com/example/solidconnection/university/domain/LanguageRequirement.java(2 hunks)src/main/java/com/example/solidconnection/university/domain/LikedUniversity.java(2 hunks)src/main/java/com/example/solidconnection/university/domain/UnivApplyInfo.java(3 hunks)src/main/java/com/example/solidconnection/university/dto/UniversityDetailResponse.java(3 hunks)src/main/java/com/example/solidconnection/university/dto/UniversityInfoForApplyPreviewResponse.java(2 hunks)src/main/java/com/example/solidconnection/university/repository/LanguageRequirementRepository.java(2 hunks)src/main/java/com/example/solidconnection/university/repository/UniversityInfoForApplyRepository.java(3 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UniversityFilterRepository.java(1 hunks)src/main/java/com/example/solidconnection/university/repository/custom/UniversityFilterRepositoryImpl.java(2 hunks)src/main/java/com/example/solidconnection/university/service/GeneralUniversityRecommendService.java(2 hunks)src/main/java/com/example/solidconnection/university/service/UniversityLikeService.java(4 hunks)src/main/java/com/example/solidconnection/university/service/UniversityQueryService.java(2 hunks)src/main/java/com/example/solidconnection/university/service/UniversityRecommendService.java(4 hunks)src/main/resources/db/migration/V14__add_unique_constraint_to_liked_university.sql(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(2 hunks)src/test/java/com/example/solidconnection/application/service/ApplicationSubmissionServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/support/integration/BaseIntegrationTest.java(4 hunks)src/test/java/com/example/solidconnection/university/fixture/LanguageRequirementFixture.java(2 hunks)src/test/java/com/example/solidconnection/university/fixture/LanguageRequirementFixtureBuilder.java(4 hunks)src/test/java/com/example/solidconnection/university/fixture/UniversityInfoForApplyFixture.java(2 hunks)src/test/java/com/example/solidconnection/university/fixture/UniversityInfoForApplyFixtureBuilder.java(2 hunks)src/test/java/com/example/solidconnection/university/repository/UniversityLikeRepositoryTest.java(1 hunks)src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/university/service/UniversityLikeServiceTest.java(5 hunks)src/test/java/com/example/solidconnection/university/service/UniversityQueryServiceTest.java(8 hunks)src/test/java/com/example/solidconnection/university/service/UniversityRecommendServiceTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (21)
src/test/java/com/example/solidconnection/application/service/ApplicationQueryServiceTest.java (1)
18-18: 1️⃣ 도메인 타입 교체가 잘 반영되었습니다.
UnivApplyInfo로의 import 수정이 정확합니다. 👍src/test/java/com/example/solidconnection/university/service/GeneralUniversityRecommendServiceTest.java (1)
4-4: 1️⃣ 도메인 import 교체 완료 확인.
UnivApplyInfoimport 적용이 올바르게 반영되었습니다.src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java (1)
16-16: 1️⃣ 새 도메인 타입 import 확인
UnivApplyInfoimport가 정상적으로 반영되었습니다.src/main/java/com/example/solidconnection/university/service/GeneralUniversityRecommendService.java (1)
3-3: 1️⃣ 도메인 import 교체 완료
UnivApplyInfoimport 교체가 정확합니다.src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
95-98: 👍 메서드명 교체가 일관성 있게 반영되었습니다기존 도메인 리네이밍에 맞춰
getUnivApplyInfo()로 정상 교체된 것을 확인했습니다. 스트림 처리·DTO 매핑 모두 문제 없어 보입니다.src/main/java/com/example/solidconnection/university/domain/LanguageRequirement.java (1)
36-37: 엔티티 매핑 수정 확인 완료
@JoinColumn추가로 FK 명시가 명확해졌고, 타입 변경도 일관성 있게 반영되었습니다. 좋은 수정입니다.src/main/java/com/example/solidconnection/siteuser/repository/LikedUniversityRepository.java (1)
17-18: 👍 변경 사항 일관성 확인 완료
메서드명·타입 모두 새로운 엔티티명으로 잘 맞춰졌습니다.src/main/java/com/example/solidconnection/university/service/UniversityQueryService.java (1)
35-41: ① 캐싱 키 충돌 가능성 검토
Sentence: ID만으로 키를 구성할 때 동일 ID 다른 term 데이터가 캐싱되면 오동작할 수 있습니다. term 을 키에 포함하는 방안을 고려해 보세요.src/test/java/com/example/solidconnection/application/fixture/ApplicationFixture.java (1)
17-26: 타입 교체 반영 완료 – LGTM파라미터 타입을 모두
UnivApplyInfo로 치환하여 일관성이 확보되었습니다. 추가 이슈 없습니다.src/main/java/com/example/solidconnection/university/repository/custom/UniversityFilterRepository.java (2)
5-5: 👍 새 엔티티로의 import 교체 완료!
UnivApplyInfoimport가 잘 반영되었습니다. 추가 조정 필요 없어 보입니다.
13-14: 리턴 타입 변경 후 구현체·호출부 점검 필요
- 구현체(
UniversityFilterRepositoryImpl)의 반환 타입도List<UnivApplyInfo>로 맞춰졌는지 확인해주세요.- 서비스·컨트롤러 등 호출부에서 제네릭 불일치로 컴파일 경고가 발생하지 않는지 한 번 돌려보시면 안전합니다.
src/main/java/com/example/solidconnection/application/service/ApplicationQueryService.java (1)
67-76: 1️⃣map(UnivApplyInfo::getUniversity)적용 확인 완료
새 엔티티에 맞춘 메서드 참조가 잘 동작할 것으로 보입니다. 특별한 이슈 없이 컴파일될 것입니다.src/main/java/com/example/solidconnection/university/domain/LikedUniversity.java (1)
22-27: ✅ 복합 Unique 제약 조건 추가, 굿!
동일 유저-대학교 중복 좋아요를 DB 레벨에서 차단하도록 설정되었습니다. 예상되는 경합 상황을 근본적으로 방지해 주네요.src/test/java/com/example/solidconnection/university/service/UniversityLikeServiceTest.java (2)
45-52: 테스트 픽스처 타입 교체 확인
UnivApplyInfo로의 필드 타입 변경이 일관되게 반영되었습니다. 테스트 시나리오 영향은 없겠습니다.
64-67: 리포지토리 메서드명 변경까지 깔끔하게 처리
findBySiteUserAndUnivApplyInfo로 호출부가 맞춰졌습니다. 레포지토리 인터페이스에도 동일한 시그니처가 있는지 한번 더 컴파일 체크만 해주시면 됩니다.src/test/java/com/example/solidconnection/application/service/ApplicationSubmissionServiceTest.java (1)
58-62: 엔티티 이름 교체 후 테스트 변수 싱크 맞춤 OK
세 곳의 대학 지원 정보 필드가 모두UnivApplyInfo로 교체되었습니다. 테스트 로직 변경 사항 없이 통과될 것으로 예상됩니다.src/main/java/com/example/solidconnection/application/repository/ApplicationRepository.java (1)
7-31: 1. 타입 변경 반영 — 컴파일 경로 이상 없음
-UniversityInfoForApply → UnivApplyInfo로 타입이 일관되게 교체된 것을 확인했습니다.
- 메서드 시그니처·파라미터까지 모두 반영되어 런타임 오류 가능성은 낮아 보입니다.src/test/java/com/example/solidconnection/university/fixture/UniversityInfoForApplyFixture.java (1)
18-24: 2. 빌더 메서드 명칭 레거시 유지
-universityInfoForApply()빌더 메서드가 계속 사용됩니다. 빌더 내부도 타입 교체가 완료됐는지 확인 부탁드립니다.
- 내부 시그니처가 변경되지 않았다면 컴파일 오류가 날 수 있으니 검증이 필요합니다.Also applies to: 26-32, 34-40, 42-48, 50-56, 58-64, 66-72, 74-80, 82-88, 90-96
src/test/java/com/example/solidconnection/university/fixture/LanguageRequirementFixture.java (1)
5-45: 1. 빌더 파라미터 타입 일치 여부 확인
-languageRequirementFixtureBuilder.universityInfoForApply(universityInfo)호출부가 여전히 옛 이름을 쓰고 있습니다.
- 빌더가UnivApplyInfo타입을 받도록 변경됐는지 컴파일 확인이 필요합니다.src/main/java/com/example/solidconnection/university/repository/UniversityInfoForApplyRepository.java (1)
18-27: 1. Repository 시그니처 정상 교체 완료
- 제네릭·메서드 리턴 타입 모두UnivApplyInfo로 교체된 부분 확인했습니다.src/main/java/com/example/solidconnection/application/domain/Application.java (1)
57-65: 1. 필드 타입 교체 — 정상 반영
- 세 개의 choice 필드 모두UnivApplyInfo로 문제없이 교체되었습니다.
src/main/resources/db/migration/V14__add_unique_constraint_to_liked_university.sql
Outdated
Show resolved
Hide resolved
...ava/com/example/solidconnection/university/fixture/UniversityInfoForApplyFixtureBuilder.java
Show resolved
Hide resolved
...in/java/com/example/solidconnection/university/repository/LanguageRequirementRepository.java
Outdated
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생하셨습니다! 혹시 UniversityInfoForApplyFixture과 UniversityInfoForApplyFixtureBuilder도 변경된 이름에 맞게 수정해주실 수 있을까요?
네 변경하겠습니다~! |
There was a problem hiding this comment.
PR 디스크립션과 커밋을 잘 작성해주셔서 빠르게 읽을 수 있었습니다
이전에 University 엔티티를 조회할 때 N+1 문제가 발생하였는데, 다시 확인해보니 N+1 문제가 발생하지 않았습니다.
무슨 일인지는 모르겠지만 좋은.. 거겠죠..? 😳
엔티티만
UniversityInfoForApply를UnivApplyInfo로 변경하였습니다. 코드 내에 존재하는 모든UniversityInfoForApply를UnivApplyInfo로 변경하기에는 많은 수정이 있을 것 같고, 머지 시 충돌이 발생할 것 같아 필요 시 별도의 작업으로 분리하는 것이 좋을 거 같습니다.
적절한 판단이었다 생각합니다😁
엔티티명은
UnivApplyInfo이지만 테이블명은 기존 이름인university_info_for_apply를 그대로 유지하도록 하였습니다. 이 부분도 건드리면 일이 엄청 커질 거 같더라구요 .. 따라서@Table,@JoinColumn을 통해 테이블 및 컬럼명은university_info_for_apply형태를 유지하도록 작성하였습니다.
제 기억으로는 회의에서 "테이블 명은 축약하지 말고 엔티티 클래스 이름만 축약형으로 하자"고 했던 것 같아요!
@JoinColumn 은 이 변경에서 놓치기 쉬운 부분이었을 것 같은데 꼼꼼하게 잘 적용하셨습니다👏
그리고 conflict 해소하실 때는 merge 하는 방법 / rebase 하는 방법이 있을텐데, 성혁님이 선호하시는 방법으로 선택하셔도 괜찮습니다~
어떤 팀에서는 선형적인 히스토리를 위해 conflict는 rebase로만 해결하라는 규정도 있더라고요 🤔
그런데 저는 conflict를 rebase로 해결하면 원본 변경이 날라간다고 생각해서 merge도 괜찮다고 생각합니다!
편한 방법으로 선택해주세요~
...est/java/com/example/solidconnection/university/repository/UniversityLikeRepositoryTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/university/domain/LikedUniversity.java
Outdated
Show resolved
Hide resolved
...in/java/com/example/solidconnection/university/repository/LanguageRequirementRepository.java
Outdated
Show resolved
Hide resolved
* refactor: rename UniversityInfoForApply to UnivApplyInfo - 엔티티명만 변경, 이에 따른 변수명 변경 * fix: 동일한 사용자가 동일한 대학 좋아요가 가능했던 문제 수정 - 복합 unique 제약 조건 추가, 관련 테스트 코드 추가 * refactor: liked_university 테이블에 복합 unique 제약 조건을 설정하는 sql 작성 * refactor: 파일 및 변수명 변경 * chore: 미사용 함수 제거 * chore: UK 제약 조건 이름 변경 * chore: 테스트 코드에서 불필요한 검증 제거 * chore: 마이그레이션 파일 버전 수정 V14 -> V15
whqtker
force-pushed
the
refactor/334-university-refactoring
branch
from
58b25ec to
ee7ac39
Compare
* refactor: rename UniversityInfoForApply to UnivApplyInfo - 엔티티명만 변경, 이에 따른 변수명 변경 * fix: 동일한 사용자가 동일한 대학 좋아요가 가능했던 문제 수정 - 복합 unique 제약 조건 추가, 관련 테스트 코드 추가 * refactor: liked_university 테이블에 복합 unique 제약 조건을 설정하는 sql 작성 * refactor: 파일 및 변수명 변경 * chore: 미사용 함수 제거 * chore: UK 제약 조건 이름 변경 * chore: 테스트 코드에서 불필요한 검증 제거 * chore: 마이그레이션 파일 버전 수정 V14 -> V15
whqtker
force-pushed
the
refactor/334-university-refactoring
branch
from
ee7ac39 to
fc8391b
Compare
3 participants
관련 이슈
작업 내용
UniversityInfoForApply엔티티명을UnivApplyInfo로 변경하였습니다.UniversityInfoForApplyFixture,UniversityInfoForApplyFixtureBuilder또한 동일한 규칙으로 이름을 변경하였습니다.특이 사항
University엔티티를 조회할 때 N+1 문제가 발생하였는데, 다시 확인해보니 N+1 문제가 발생하지 않았습니다. 아래 쿼리는/universities/recommend에 접근 시 발생하는 쿼리입니다. N+1 문제가 발생되어야 했던 부분입니다.여러 커밋으로 되돌아가서 테스트를 진행해도 N+1 문제는 발생하지 않았습니다 .. 따라서 원래 Fetch Join을 통해 해결할 계획이었으나, 기존 코드 그대로 유지하였습니다.
UniversityInfoForApply를UnivApplyInfo로 변경하였습니다. 코드 내에 존재하는 모든UniversityInfoForApply를UnivApplyInfo로 변경하기에는 많은 수정이 있을 것 같고, 머지 시 충돌이 발생할 것 같아 필요 시 별도의 작업으로 분리하는 것이 좋을 거 같습니다.UnivApplyInfo이지만 테이블명은 기존 이름인university_info_for_apply를 그대로 유지하도록 하였습니다. 이 부분도 건드리면 일이 엄청 커질 거 같더라구요 .. 따라서@Table,@JoinColumn을 통해 테이블 및 컬럼명은university_info_for_apply형태를 유지하도록 작성하였습니다.리뷰 요구사항 (선택)