Skip to content

refactor: 모의지원 시 지원한 대학 정보 응답 추가 #539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Gyuhyeok99 merged 3 commits into from
Nov 3, 2025
Merged

refactor: 모의지원 시 지원한 대학 정보 응답 추가 #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53d8cec
refactor: 지원서 제출 시 지원 대학명 응답 추가
Gyuhyeok99 Oct 30, 2025
16f4614
test: 지원서 제출 시 지원 대학명 응답 관련 테스트 수정
Gyuhyeok99 Oct 30, 2025
6055836
fix: 충돌 해결
Gyuhyeok99 Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/main/java/com/example/solidconnection/application/dto/ApplicationSubmissionResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
package com.example.solidconnection.application.dto;

import com.example.solidconnection.application.domain.Application;
import com.example.solidconnection.university.domain.UnivApplyInfo;
import java.util.List;

public record ApplicationSubmissionResponse(
int applyCount
int totalApplyCount,
int applyCount,
UnivApplyInfoResponse appliedUniversities
) {

public static ApplicationSubmissionResponse from(Application application) {
return new ApplicationSubmissionResponse(application.getUpdateCount());
public static ApplicationSubmissionResponse of(int totalApplyCount, Application application, List<UnivApplyInfo> uniApplyInfos) {
return new ApplicationSubmissionResponse(
totalApplyCount,
application.getUpdateCount(),
UnivApplyInfoResponse.of(application, uniApplyInfos)
);
}
}
41 changes: 41 additions & 0 deletions src/main/java/com/example/solidconnection/application/dto/UnivApplyInfoResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.example.solidconnection.application.dto;

import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;

import com.example.solidconnection.application.domain.Application;
import com.example.solidconnection.university.domain.UnivApplyInfo;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public record UnivApplyInfoResponse(

@JsonProperty("firstChoiceUniversity")
String firstChoiceUnivApplyInfo,

@JsonProperty("secondChoiceUniversity")
@JsonInclude(NON_NULL)
String secondChoiceUnivApplyInfo,

@JsonProperty("thirdChoiceUniversity")
@JsonInclude(NON_NULL)
String thirdChoiceUnivApplyInfo) {

public static UnivApplyInfoResponse of(Application application, List<UnivApplyInfo> univApplyInfos) {
Map<Long, String> univApplyInfoMap = univApplyInfos.stream()
.collect(Collectors.toMap(
UnivApplyInfo::getId,
UnivApplyInfo::getKoreanName
));

return new UnivApplyInfoResponse(
univApplyInfoMap.get(application.getFirstChoiceUnivApplyInfoId()),
application.getSecondChoiceUnivApplyInfoId() != null
? univApplyInfoMap.get(application.getSecondChoiceUnivApplyInfoId()) : null,
application.getThirdChoiceUnivApplyInfoId() != null
? univApplyInfoMap.get(application.getThirdChoiceUnivApplyInfoId()) : null
);
}
}
20 changes: 18 additions & 2 deletions ...in/java/com/example/solidconnection/application/service/ApplicationSubmissionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@
import com.example.solidconnection.siteuser.repository.SiteUserRepository;
import com.example.solidconnection.term.domain.Term;
import com.example.solidconnection.term.repository.TermRepository;
import com.example.solidconnection.university.domain.UnivApplyInfo;
import com.example.solidconnection.university.repository.UnivApplyInfoRepository;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -39,6 +44,7 @@ public class ApplicationSubmissionService {
private final LanguageTestScoreRepository languageTestScoreRepository;
private final SiteUserRepository siteUserRepository;
private final TermRepository termRepository;
private final UnivApplyInfoRepository univApplyInfoRepository;

// 학점 및 어학성적이 모두 유효한 경우에만 지원서 등록이 가능하다.
// 기존에 있던 status field 우선 APRROVED로 입력시킨다.
Expand All @@ -52,7 +58,7 @@ public ApplicationSubmissionResponse apply(long siteUserId, ApplyRequest applyRe
Term term = termRepository.findByIsCurrentTrue()
.orElseThrow(() -> new CustomException(CURRENT_TERM_NOT_FOUND));

long firstChoiceUnivApplyInfoId = univApplyInfoChoiceRequest.firstChoiceUnivApplyInfoId();
Long firstChoiceUnivApplyInfoId = univApplyInfoChoiceRequest.firstChoiceUnivApplyInfoId();
Long secondChoiceUnivApplyInfoId = univApplyInfoChoiceRequest.secondChoiceUnivApplyInfoId();
Long thirdChoiceUnivApplyInfoId = univApplyInfoChoiceRequest.thirdChoiceUnivApplyInfoId();

Expand Down Expand Up @@ -80,7 +86,17 @@ public ApplicationSubmissionResponse apply(long siteUserId, ApplyRequest applyRe
newApplication.setVerifyStatus(VerifyStatus.APPROVED);
applicationRepository.save(newApplication);

return ApplicationSubmissionResponse.from(newApplication);
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);
}
Comment on lines +89 to 100
Copy link

@coderabbitai coderabbitai bot Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

필수 대학 정보 누락 시 즉시 검증 필요
findAllByIds가 전달한 ID를 모두 돌려주지 못하면 firstChoiceUnivApplyInfoId가 map에 존재하지 않아 응답의 firstChoiceUniversity가 null로 직행합니다. 이번 변경으로 해당 필드는 클라이언트에 반드시 내려가야 하는 값인데, 관리자 삭제·동시성 등 현실적인 상황에서 바로 계약이 깨지고 프런트는 NPE나 렌더링 오류를 겪게 됩니다. 저장 전·후 중 한 곳에서라도 조회 결과에 firstChoiceUnivApplyInfoId가 포함돼 있는지 확인하고, 누락 시 CustomException으로 즉시 중단해 주세요.

Copy link
Contributor Author

@Gyuhyeok99 Gyuhyeok99 Oct 30, 2025

Choose a reason for hiding this comment

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

@ coderabbitai
firstChoiceUnivApplyInfoId 이값은 무조건 존재한다는 가정인데 검증해야해?

Copy link

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

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

🧩 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

@Gyuhyeok99

네, 요청 단계에서는 ValidUnivApplyInfoChoiceValidator가 firstChoiceUnivApplyInfoId의 null 여부를 검증하고 있습니다.

하지만 제가 우려한 부분은 조금 다른 시나리오입니다:

현재 상황:

  1. 요청 검증 통과 (firstChoiceUnivApplyInfoId = 123L, null 아님) ✅
  2. Application 저장 ✅
  3. findAllByIds로 UnivApplyInfo 조회
  4. 만약 ID=123 레코드가 DB에 없다면? → map에서 get(123L) = null
  5. firstChoiceUniversity = null → API 응답에서 필수 필드가 null

기술적 격차:

  • Application 엔티티가 UnivApplyInfo ID를 Long 필드로만 저장 (외래키 제약 없음)
  • DB 레벨 참조 무결성이 없어서 UnivApplyInfo 레코드 삭제 가능
  • firstChoiceUniversity는 @JsonInclude(NON_NULL) 없이 필수 응답 필드

선택지:

  1. 런타임 검증 추가 - 조회 후 firstChoice ID가 결과에 포함됐는지 확인하고 누락 시 예외 발생
  2. FK 제약 추가 - Application 엔티티에 @ManyToOne 관계 추가해 DB 레벨 보장
  3. 현상 유지 - 비즈니스 레이어에서 삭제하지 않는다는 가정으로 진행

어떤 방향이 좋을까요? 프로덕션 환경에서 관리자 실수나 데이터 정합성 이슈를 고려하면 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을 검색 대상으로 사용하는 것이 의도된 설계이다.

Copy link
Contributor Author

@Gyuhyeok99 Gyuhyeok99 Oct 30, 2025

Choose a reason for hiding this comment

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

다른 분들 의견 듣고 판단하겠습니다!

Copy link

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

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

@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을 검색 대상으로 사용하는 것이 의도된 설계이다.

Copy link
Contributor

@sukangpunch sukangpunch Oct 31, 2025

Choose a reason for hiding this comment

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

다른 분들 의견 듣고 판단하겠습니다!

저도 해당 id는 무조건 존재하는 상황에서 요청이 온다 생각하기 때문에 추가적인 검증이 필요하지 않다고 생각이 듭니다.
그러나 혹시라도 매우 낮은 확률로 서비스 운영 중에 db 데이터가 잘못되었거나(예: 다른 작업중 db 오류 및 요청 중 해당 학교가 교환 학생 지원이 유효하지 않은 상태가 되어 db에 반영했을 때 -- 사실 그럴일이 없을 것 같긴 합니다.) 지원 자체가 안되는 상황을 유도하기 위해 검증하는 것도 좋다고 생각이 듭니다!

Copy link

@coderabbitai coderabbitai bot Oct 31, 2025

Choose a reason for hiding this comment

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

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!

Copy link
Contributor

@Hexeong Hexeong Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

대학 ID의 경우, 사용자가 직접 값을 입력하는 변수는 아니다 보니 저도 해당 검증은 오버헤드라는 생각이 드네요! 방어적 프로그래밍 관점에서도 대학 ID는 사용자에 의해 값이 관리되지 않기 때문에 저는 굳이라는 생각이 듭니다~


private GpaScore getValidGpaScore(SiteUser siteUser, Long gpaScoreId) {
Expand Down
8 changes: 8 additions & 0 deletions ...ava/com/example/solidconnection/application/service/ApplicationSubmissionServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,16 @@ void setUp() {
// then
Application savedApplication = applicationRepository.findBySiteUserIdAndTermId(user.getId(), term.getId()).orElseThrow();
assertAll(
() -> assertThat(response.totalApplyCount())
.isEqualTo(APPLICATION_UPDATE_COUNT_LIMIT),
() -> assertThat(response.applyCount())
.isEqualTo(savedApplication.getUpdateCount()),
() -> assertThat(response.appliedUniversities().firstChoiceUnivApplyInfo())
.isEqualTo(괌대학_A_지원_정보.getKoreanName()),
() -> assertThat(response.appliedUniversities().secondChoiceUnivApplyInfo())
.isEqualTo(괌대학_B_지원_정보.getKoreanName()),
() -> assertThat(response.appliedUniversities().thirdChoiceUnivApplyInfo())
.isEqualTo(서던덴마크대학교_지원_정보.getKoreanName()),
() -> assertThat(savedApplication.getVerifyStatus())
.isEqualTo(VerifyStatus.APPROVED),
() -> assertThat(savedApplication.isDelete())
Expand Down
Loading