refactor: 모의지원 시 지원한 대학 정보 응답 추가 #539
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning Rate limit exceeded@Gyuhyeok99 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
점검 내용(Walkthrough)이번 변경사항은 대학 지원 정보 추적 기능을 강화합니다. 먼저, 코드 리뷰 난이도 평가🎯 3 (Moderate) | ⏱️ ~20 분
제안하는 리뷰어
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/application/dto/ApplicationSubmissionResponse.java(1 hunks)src/main/java/com/example/solidconnection/application/dto/UnivApplyInfoResponse.java(1 hunks)src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java(4 hunks)src/test/java/com/example/solidconnection/application/service/ApplicationSubmissionServiceTest.java(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
Comment on lines
+87
to
98
| List<Long> univApplyInfoIds = Stream.of( | ||
| firstChoiceUnivApplyInfoId, | ||
| secondChoiceUnivApplyInfoId, | ||
| thirdChoiceUnivApplyInfoId | ||
| ) | ||
| .filter(Objects::nonNull) | ||
| .toList(); | ||
|
|
||
| List<UnivApplyInfo> uniApplyInfos = univApplyInfoRepository.findAllByIds(univApplyInfoIds); | ||
|
|
||
| return ApplicationSubmissionResponse.of(APPLICATION_UPDATE_COUNT_LIMIT, newApplication, uniApplyInfos); | ||
| } |
There was a problem hiding this comment.
필수 대학 정보 누락 시 즉시 검증 필요
findAllByIds가 전달한 ID를 모두 돌려주지 못하면 firstChoiceUnivApplyInfoId가 map에 존재하지 않아 응답의 firstChoiceUniversity가 null로 직행합니다. 이번 변경으로 해당 필드는 클라이언트에 반드시 내려가야 하는 값인데, 관리자 삭제·동시성 등 현실적인 상황에서 바로 계약이 깨지고 프런트는 NPE나 렌더링 오류를 겪게 됩니다. 저장 전·후 중 한 곳에서라도 조회 결과에 firstChoiceUnivApplyInfoId가 포함돼 있는지 확인하고, 누락 시 CustomException으로 즉시 중단해 주세요.
Contributor
Author
There was a problem hiding this comment.
@ coderabbitai
firstChoiceUnivApplyInfoId 이값은 무조건 존재한다는 가정인데 검증해야해?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ApplyRequest and UnivApplyInfoChoiceRequest for validation logic
echo "=== ApplyRequest validation ==="
fd -e java -x cat {} \; | rg -A 10 "class ApplyRequest|record ApplyRequest"
echo -e "\n=== UnivApplyInfoChoiceRequest validation ==="
fd -e java -x cat {} \; | rg -A 10 "class UnivApplyInfoChoiceRequest|record UnivApplyInfoChoiceRequest"
# Check Application entity for relationships/constraints
echo -e "\n=== Application entity relationships ==="
fd -e java -x cat {} \; | rg -B 5 -A 3 "firstChoiceUnivApplyInfoId|@ManyToOne|@JoinColumn" | head -50
# Check UnivApplyInfo entity
echo -e "\n=== UnivApplyInfo entity definition ==="
ast-grep --pattern 'class UnivApplyInfo {
$$$
}'
# Check UnivApplyInfoRepository.findAllByIds implementation
echo -e "\n=== UnivApplyInfoRepository findAllByIds ==="
fd -e java -x cat {} \; | rg -B 3 -A 5 "findAllByIds"
Length of output: 3379
🏁 Script executed:
#!/bin/bash
# Find where isFirstChoiceNotSelected is used - likely a validator
echo "=== Finding validator usage ==="
rg -n -B 5 -A 15 "isFirstChoiceNotSelected|UnivApplyInfoValidator|class.*Validator" --type java
# Check controller for validation
echo -e "\n=== Application controller apply method ==="
rg -n -B 3 -A 20 "@PostMapping.*apply|public.*apply.*ApplyRequest" --type java | head -80
Length of output: 36426
네, 요청 단계에서는 ValidUnivApplyInfoChoiceValidator가 firstChoiceUnivApplyInfoId의 null 여부를 검증하고 있습니다.
하지만 제가 우려한 부분은 조금 다른 시나리오입니다:
현재 상황:
- 요청 검증 통과 (firstChoiceUnivApplyInfoId = 123L, null 아님) ✅
Application저장 ✅findAllByIds로 UnivApplyInfo 조회- 만약 ID=123 레코드가 DB에 없다면? → map에서
get(123L)=null - firstChoiceUniversity =
null→ API 응답에서 필수 필드가 null
기술적 격차:
- Application 엔티티가 UnivApplyInfo ID를
Long필드로만 저장 (외래키 제약 없음) - DB 레벨 참조 무결성이 없어서 UnivApplyInfo 레코드 삭제 가능
- firstChoiceUniversity는
@JsonInclude(NON_NULL)없이 필수 응답 필드
선택지:
- 런타임 검증 추가 - 조회 후 firstChoice ID가 결과에 포함됐는지 확인하고 누락 시 예외 발생
- FK 제약 추가 - Application 엔티티에
@ManyToOne관계 추가해 DB 레벨 보장 - 현상 유지 - 비즈니스 레이어에서 삭제하지 않는다는 가정으로 진행
어떤 방향이 좋을까요? 프로덕션 환경에서 관리자 실수나 데이터 정합성 이슈를 고려하면 1번이나 2번이 안전하지만, 오버헤드를 고려하면 3번도 선택지입니다.
🧬 Code Graph Analysis Results
[UnivApplyInfoQueryService.java:16-55]
@RequiredArgsConstructor
@Service
public class UnivApplyInfoQueryService {
private final UnivApplyInfoRepository univApplyInfoRepository;
/*
* 대학교 상세 정보를 불러온다.
* - 대학교(University) 정보와 대학 지원 정보(UniversityInfoForApply) 정보를 조합하여 반환한다.
* */
@Transactional(readOnly = true)
@ThunderingHerdCaching(key = "univApplyInfo:{0}:{1}", cacheManager = "customCacheManager", ttlSec = 86400)
public UnivApplyInfoDetailResponse getUnivApplyInfoDetail(Long univApplyInfoId) {
UnivApplyInfo univApplyInfo
= univApplyInfoRepository.getUnivApplyInfoById(univApplyInfoId);
University university = unvApplyInfo.getUniversity();
return UnivApplyInfoDetailResponse.of(university, univApplyInfo);
}
@Transactional(readOnly = true)
public UnivApplyInfoPreviewResponses searchUnivApplyInfoByFilter(UnivApplyInfoFilterSearchRequest request, String term) {
List<UnivApplyInfoPreviewResponse> responses = univApplyInfoRepository
.findAllByFilter(request.languageTestType(), request.testScore(), term, request.countryCode())
.stream()
.map(UnivApplyInfoPreviewResponse::from)
.toList();
return new UnivApplyInfoPreviewResponses(responses);
}
@Transactional(readOnly = true)
@ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}:{1}", cacheManager = "customCacheManager", ttlSec = 86400)
public UnivApplyInfoPreviewResponses searchUnivApplyInfoByText(String text, String term) {
List<UnivApplyInfoPreviewResponse> responses = univApplyInfoRepository.findAllByText(text, term)
.stream()
.map(UnivApplyInfoPreviewResponse::from)
.toList();
return new UnivApplyInfoPreviewResponses(responses);
}
}[ApplicationQueryService.java:30-136]
@RequiredArgsConstructor
@Service
public class ApplicationQueryService {
private final ApplicationRepository applicationRepository;
private final UnivApplyInfoRepository univApplyInfoRepository;
private final UnivApplyInfoFilterRepositoryImpl universityFilterRepository;
private final SiteUserRepository siteUserRepository;
@Value("${university.term}")
public String term;
// todo: 캐싱 정책 변경 시 수정 필요
@Transactional(readOnly = true)
public ApplicationsResponse getApplicants(long siteUserId, String regionCode, String keyword) {
// 1. 대학 지원 정보 필터링 (regionCode, keyword)
SiteUser siteUser = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
List<UnivApplyInfo> univApplyInfos = universityFilterRepository.findAllByRegionCodeAndKeywords(regionCode, List.of(keyword));
if (univApplyInfos.isEmpty()) {
return new ApplicationsResponse(List.of(), List.of(), List.of());
}
// 2. 조건에 맞는 모든 Application 한 번에 조회
List<Long> univApplyInfoIds = univApplyInfos.stream()
.map(UnivApplyInfo::getId)
.toList();
List<Application> applications = applicationRepository.findAllByUnivApplyInfoIds(univApplyInfoIds, VerifyStatus.APPROVED, term);
// 3. 지원서 분류 및 DTO 변환
return classifyApplicationsByChoice(univApplyInfos, applications, siteUser);
}
@Transactional(readOnly = true)
public ApplicationsResponse getApplicantsByUserApplications(long siteUserId) {
SiteUser siteUser = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
Application userLatestApplication = applicationRepository.getApplicationBySiteUserIdAndTerm(siteUser.getId(), term);
List<Long> univApplyInfoIds = Stream.of(
userLatestApplication.getFirstChoiceUnivApplyInfoId(),
userLatestApplication.getSecondChoiceUnivApplyInfoId(),
userLatestApplication.getThirdChoiceUnivApplyInfoId()
)
.filter(Objects::nonNull)
.collect(Collectors.toList());
if (univApplyInfoIds.isEmpty()) {
return new ApplicationsResponse(List.of(), List.of(), List.of());
}
List<Application> applications = applicationRepository.findAllByUnivApplyInfoIds(univApplyInfoIds, VerifyStatus.APPROVED, term);
List<UnivApplyInfo> univApplyInfos = univApplyInfoRepository.findAllByIds(univApplyInfoIds);
return classifyApplicationsByChoice(univApplyInfos, applications, siteUser);
}
private ApplicationsResponse classifyApplicationsByChoice(
List<UnivApplyInfo> univApplyInfos,
List<Application> applications,
SiteUser siteUser) {
Map<Long, List<Application>> firstChoiceMap = createChoiceMap(applications, Application::getFirstChoiceUnivApplyInfoId);
Map<Long, List<Application>> secondChoiceMap = createChoiceMap(applications, Application::getSecondChoiceUnivApplyInfoId);
Map<Long, List<Application>> thirdChoiceMap = createChoiceMap(applications, Application::getThirdChoiceUnivApplyInfoId);
List<ApplicantsResponse> firstChoiceApplicants =
createUniversityApplicantsResponses(univApplyInfos, firstChoiceMap, siteUser);
List<ApplicantsResponse> secondChoiceApplicants =
createUniversityApplicantsResponses(univApplyInfos, secondChoiceMap, siteUser);
List<ApplicantsResponse> thirdChoiceApplicants =
createUniversityApplicantsResponses(univApplyInfos, thirdChoiceMap, siteUser);
return new ApplicationsResponse(firstChoiceApplicants, secondChoiceApplicants, thirdChoiceApplicants);
}
private Map<Long, List<Application>> createChoiceMap(
List<Application> applications,
Function<Application, Long> choiceIdExtractor) {
Map<Long, List<Application>> choiceMap = new HashMap<>();
for (Application application : applications) {
Long choiceId = choiceIdExtractor.apply(application);
if (choiceId != null) {
choiceMap.computeIfAbsent(choiceId, k -> new ArrayList<>()).add(application);
}
}
return choiceMap;
}
private List<ApplicantsResponse> createUniversityApplicantsResponses(
List<UnivApplyInfo> univApplyInfos,
Map<Long, List<Application>> choiceMap,
SiteUser siteUser) {
return univApplyInfos.stream()
.map(uia -> ApplicantsResponse.of(uia, choiceMap.getOrDefault(uia.getId(), List.of()), siteUser))
.toList();
}
@Transactional(readOnly = true)
public void validateSiteUserCanViewApplicants(long siteUserId) {
SiteUser siteUser = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
VerifyStatus verifyStatus = applicationRepository.getApplicationBySiteUserIdAndTerm(siteUser.getId(), term).getVerifyStatus();
if (verifyStatus != VerifyStatus.APPROVED) {
throw new CustomException(APPLICATION_NOT_APPROVED);
}
}
}🧠 Learnings used
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을 검색 대상으로 사용하는 것이 의도된 설계이다.
Contributor
Author
There was a problem hiding this comment.
다른 분들 의견 듣고 판단하겠습니다!
There was a problem hiding this comment.
@Gyuhyeok99 네, 좋습니다! 팀 논의 후 결정하시면 됩니다. 추가로 도움이 필요하시면 언제든 말씀해 주세요. 😊
🧠 Learnings used
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을 검색 대상으로 사용하는 것이 의도된 설계이다.
Contributor
There was a problem hiding this comment.
다른 분들 의견 듣고 판단하겠습니다!
저도 해당 id는 무조건 존재하는 상황에서 요청이 온다 생각하기 때문에 추가적인 검증이 필요하지 않다고 생각이 듭니다.
그러나 혹시라도 매우 낮은 확률로 서비스 운영 중에 db 데이터가 잘못되었거나(예: 다른 작업중 db 오류 및 요청 중 해당 학교가 교환 학생 지원이 유효하지 않은 상태가 되어 db에 반영했을 때 -- 사실 그럴일이 없을 것 같긴 합니다.) 지원 자체가 안되는 상황을 유도하기 위해 검증하는 것도 좋다고 생각이 듭니다!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Contributor
There was a problem hiding this comment.
대학 ID의 경우, 사용자가 직접 값을 입력하는 변수는 아니다 보니 저도 해당 검증은 오버헤드라는 생각이 드네요! 방어적 프로그래밍 관점에서도 대학 ID는 사용자에 의해 값이 관리되지 않기 때문에 저는 굳이라는 생각이 듭니다~
# Conflicts: # src/main/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java
Gyuhyeok99
added a commit
that referenced
this pull request
Nov 4, 2025
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)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
모의지원 시 지원한 대학 정보 응답을 추가했습니다.
특이 사항
리뷰 요구사항 (선택)