Skip to content

Conversation

@nayonsoso
Copy link
Collaborator

관련 이슈

작업 내용

텍스트로 대학지원정보를 검색할 수 있는 API를 구현했습니다.
이슈에 자세한 정책 내용 나와있습니다 🫡

특이 사항

코멘트로 남겨두겠습니다!

@coderabbitai
Copy link

coderabbitai bot commented Aug 25, 2025

Walkthrough

  1. 저장소 인터페이스에 새 메서드 List<UnivApplyInfo> findAllByText(String text, String term)가 추가되었습니다.
  2. 저장소 구현에 findAllByText가 추가되었고, QueryDSL로 University·Country fetch join, Region join, languageRequirements left join을 사용합니다.
  3. 텍스트가 비어있으면 id 오름차순으로 전체 반환합니다.
  4. 텍스트가 제공되면 UnivApplyInfo·Country·Region의 국문명에 OR 매칭을 적용하고 CaseBuilder로 rank(대학=0, 국가=1, 권역=2, 기타=3)를 계산하여 rank, id 순 정렬합니다.
  5. Service와 Controller에서 term 파라미터를 도입해 검색 메서드 시그니처를 변경했고, 텍스트 검색 서비스에 ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}:{1}", cacheManager = "customCacheManager", ttlSec = 86400)를 추가했습니다.
  6. 테스트용 픽스처(UniversityFixture, UnivApplyInfoFixture)와 UnivApplyInfoQueryServiceTest가 term 주입과 텍스트 검색 시나리오(정렬·매칭·캐시)로 확장되었습니다.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • QueryDSL 기반 복합 쿼리와 랭킹 로직, 서비스/컨트롤러 시그니처 변경, 캐싱 키 변경 및 테스트·픽스처 확장 등 이질적 변경들이 섞여 있어 각 영역별 검토가 필요합니다.

Suggested reviewers

  • wibaek
  • whqtker
  • lsy1307
  • Gyuhyeok99

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (11)
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java (1)

13-14: 1) 텍스트 검색 시그니처 추가 타당. 계약(Javadoc)으로 동작 기준을 고정해 주세요.

    1. null/blank 입력 시 전체 반환 및 정렬 기준(예: id 오름차순)을 인터페이스 레벨에서 문서화하면 호출 측 혼선을 줄일 수 있습니다.
    1. 부분 일치/대소문자/공백 트리밍 등 매칭 규칙(특히 한글 여부)을 명확히 남겨 주세요.
    1. term 파라미터의 역할(학기 스코프 고정)도 함께 기술하면 좋습니다.

적용 예시(주석만 추가):

 public interface UnivApplyInfoFilterRepository {

     List<UnivApplyInfo> findAllByRegionCodeAndKeywords(String regionCode, List<String> keywords);

     List<UnivApplyInfo> findAllByFilter(LanguageTestType testType, String testScore, String term, List<String> countryKoreanNames);
+
+    /**
+     * 텍스트 기반 검색.
+     * - 검색 대상: 대학명(국문), 국가명(국문), 권역명(국문)의 부분 일치 OR.
+     * - text == null 또는 blank면 전체를 id 오름차순으로 반환.
+     * - term은 학기 스코프를 의미하며 결과를 해당 학기로 한정.
+     */
     List<UnivApplyInfo> findAllByText(String text, String term);
 }
src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java (1)

42-48: 1) 아칸소주립대학 픽스처 추가는 적절합니다. 한글 표기 일관성만 점검해 주세요.

    1. koreanName "아칸소 주립 대학"의 띄어쓰기/표기(예: ‘주립대학’ vs ‘주립 대학’)가 다른 픽스처들과 혼재합니다. 검색 테스트에 영향은 없으나, 도메인 데이터의 일관성 측면에서 한 번 더 확인을 권장합니다.
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1)

50-58: 2) 입력 text 정규화와 캐시 키 표준화도 고려해 주세요.

    1. null, 공백 only, 앞뒤 공백이 동일 검색을 의미한다면, 동일한 캐시 키로 수렴되도록 표준화가 필요합니다.
    1. SpEL이 가능하다면 키에 trim/lowercase를 적용하고, 아니라면 컨트롤러 레이어에서 정규화된 값으로만 서비스를 호출하는 규칙을 권장합니다.
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (5)

6-6: 1) assertThatCode는 과합니다 — 단순 동일성 비교로 충분합니다.

    1. 예외 발생 여부를 감쌀 필요 없이 리스트 동일성만 검증하면 됩니다.

적용 예시:

-import static org.assertj.core.api.Assertions.assertThatCode;
@@
-            assertThatCode(() -> {
-                List<Long> firstResponseIds = extractIds(firstResponse);
-                List<Long> secondResponseIds = extractIds(secondResponse);
-                assertThat(firstResponseIds).isEqualTo(secondResponseIds);
-            }).doesNotThrowAnyException();
+            List<Long> firstResponseIds = extractIds(firstResponse);
+            List<Long> secondResponseIds = extractIds(secondResponse);
+            assertThat(firstResponseIds).isEqualTo(secondResponseIds);

Also applies to: 268-274


46-47: 2) 테스트 필드 가시성 축소 권장.

    1. term는 테스트 내부에서만 사용되므로 private으로 축소해 캡슐화를 유지하세요.
-    public String term;
+    private String term;

160-174: 3) 빈 문자열/공백 입력 케이스도 보강해 주세요.

    1. null은 커버되지만 "" 또는 " " 입력도 동일 동작(전체 반환 및 정렬)인지 명세 및 테스트로 고정하는 것이 좋습니다.

추가 테스트 예시(참고):

@Test
void 텍스트가_빈문자열이면_null과_동일하게_동작한다() {
    // given
    univApplyInfoFixture.괌대학_A_지원_정보();
    univApplyInfoFixture.메이지대학_지원_정보();

    // when
    var response1 = univApplyInfoQueryService.searchUnivApplyInfoByText(null);
    var response2 = univApplyInfoQueryService.searchUnivApplyInfoByText("");

    // then
    assertThat(response2.univApplyInfoPreviews()).isEqualTo(response1.univApplyInfoPreviews());
}

237-255: 5) 동순위 정렬 안정성(같은 rank 내 id 오름차순)도 한 케이스 추가를 제안합니다.

    1. 현재는 rank 간 우선순위만 검증합니다. 동일 rank 다건 시 id 정렬 보장을 함께 테스트하면 회귀를 방지할 수 있습니다.

추가 테스트 예시(참고):

@Test
void 동일_rank에서는_id_오름차순으로_정렬한다() {
    // given: 동일 권역 '유럽'에 속하는 2개 이상 엔트리 생성
    var a = univApplyInfoFixture.그라츠대학_지원_정보();   // id1
    var b = univApplyInfoFixture.그라츠공과대학_지원_정보(); // id2
    String text = "유럽";

    // when
    var response = univApplyInfoQueryService.searchUnivApplyInfoByText(text);

    // then: 두 결과가 권역 매치(rank 동일)이며 id 오름차순
    assertThat(response.univApplyInfoPreviews())
        .containsExactly(
            UnivApplyInfoPreviewResponse.from(a.getId() < b.getId() ? a : b),
            UnivApplyInfoPreviewResponse.from(a.getId() < b.getId() ? b : a)
        );
}

257-274: 6) 캐시 테스트는 기본 흐름을 잘 보장합니다. term 변화 시 캐시 분리도 검증해 주세요.

    1. 서비스 캐시 키가 text만 포함하면 학기(term) 변경 시 충돌할 수 있습니다. term을 포함하도록 수정 시, 학기 변경된 상황에서도 조회가 저장소로 위임되는지 테스트를 추가해 주세요.

추가 테스트 아이디어(참고, ReflectionTestUtils 사용):

@Test
void 학기가_바뀌면_같은_text라도_캐시가_분리된다() {
    // given
    String text = "Guam";
    univApplyInfoFixture.괌대학_A_지원_정보();

    // when: 1차 호출(현재 term)
    univApplyInfoQueryService.searchUnivApplyInfoByText(text);

    // 학기 변경(예: '2025-FALL' -> '2026-SPRING')
    org.springframework.test.util.ReflectionTestUtils.setField(univApplyInfoQueryService, "term", "2026-SPRING");

    // 2차 호출(변경된 term)
    univApplyInfoQueryService.searchUnivApplyInfoByText(text);

    // then: 저장소가 두 번 호출되어야 함(캐시 키 분리)
    then(univApplyInfoRepository).should(times(2)).findAllByText(text, "2026-SPRING");
}
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (3)

159-167: 4) 검색 UX 개선 — 입력 정규화(trim)와 대소문자 무시를 적용해 주세요.

  • 공백만 다른 동일 검색어가 서로 다른 결과를 내지 않도록 사전 trim을 권장합니다.
  • containsIgnoreCase를 사용하면 영문/로마자 표기 혼합 데이터에도 더 잘 대응합니다.
    1. 제안(diff)
-        if (text == null || text.isBlank()) {
+        text = (text == null) ? null : text.trim();
+        if (text == null || text.isBlank()) {
             return base.orderBy(univApplyInfo.id.asc()).fetch();
         }
 
         // 매칭 조건 (대학명/국가명/지역명 중 하나라도 포함)
-        BooleanExpression universityLike = university.koreanName.contains(text);
-        BooleanExpression countryLike = country.koreanName.contains(text);
-        BooleanExpression regionLike = region.koreanName.contains(text);
+        BooleanExpression universityLike = university.koreanName.containsIgnoreCase(text);
+        BooleanExpression countryLike = country.koreanName.containsIgnoreCase(text);
+        BooleanExpression regionLike = region.koreanName.containsIgnoreCase(text);
  • “아칸소” vs “아 칸 소” 등 공백 차이, 대/소문자 차이에 대한 검색 일치 테스트를 추가해 주세요.

167-171: 5) OR 결합 가독성 개선 — anyOf 사용을 고려해 주세요.

  • BooleanBuilder의 체이닝 대신 anyOf를 쓰면 의도가 더 즉시 드러납니다.
    1. 제안(diff)
-        BooleanBuilder where = new BooleanBuilder()
-                .or(universityLike)
-                .or(countryLike)
-                .or(regionLike);
+        BooleanExpression where = Expressions.anyOf(universityLike, countryLike, regionLike);

145-183: 7) 성능/확장 아키텍처 제안 — LIKE 기반에서의 스케일 고려 포인트입니다.

  • 현재 구현은 like '%text%' 패턴이라, 데이터 증가 시 인덱스 활용이 제한될 수 있습니다.

  • 선택지를 단계적으로 검토해 보세요.

    1. PostgreSQL 사용 시: 한국어용 ngram/pg_trgm + GIN 인덱스를 추가해 부분 일치 속도를 개선합니다.
    2. 다국어 혼합 시: Normalizer(공백/특수문자 제거, 자모 분리/결합 이슈) 적용으로 일치율을 높입니다.
    3. 고도화: 검색 도메인을 검색 전용 테이블/뷰로 정규화하고, Synonym/별칭(국가/지역의 구어체 명칭)을 병행 저장합니다.
    4. 외부 검색엔진: 요구사항이 커지면 OpenSearch/Elasticsearch로 이관하고, Boosting(대학 > 국가 > 지역) 규칙을 Query-time에 전달하세요.
  • 인덱스 전략/마이그레이션 스크립트가 필요하면 초안 PR 패치를 함께 드릴 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3f8c493 and 8865bbc.

📒 Files selected for processing (6)
  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java (1 hunks)
  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2 hunks)
  • src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/fixture/UniversityFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (4 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 (5)
src/test/java/com/example/solidconnection/university/fixture/UniversityFixture.java (1)

35-42: 1) Arkansas State University 픽스처 추가 LGTM.

    1. country/region 설정(미국/영미권) 및 명칭이 도메인 규칙과 일치합니다. 추가 조치 없습니다.
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1)

53-57: 3) 매핑 로직은 간결하고 가독성 좋습니다.

    1. 스트림 체이닝과 DTO 변환이 명확합니다. 추가 액션 없습니다.
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (1)

176-235: 4) 필드별 검색 검증 케이스 구성 훌륭합니다.

    1. 대학/국가/권역 각각의 매치 시나리오가 명확합니다.
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2)

4-17: 1) QueryDSL import 추가 적절 — 그대로 가셔도 됩니다.

  • 필요한 타입(QRegion, BooleanBuilder, CaseBuilder, NumberExpression, JPAQuery)이 정확히 반영되었습니다.
  • 중복/불필요 의존성은 보이지 않습니다.

172-183: 6) 랭크 우선 정렬 설계가 명확합니다 — 주석과 구현이 일치합니다.

  • 대학명(0) > 국가명(1) > 지역명(2) > 기타(3) 순 정렬 로직이 명확하며, tie-breaker로 id ASC를 둔 점이 좋습니다.

  • 1차 조건이 복수로 만족될 때 CASE의 “상단 first match” 규칙으로 의도대로 동작합니다.

  • 동일 랭크 내에서 id ASC가 안정 정렬 역할을 하는지(특히 페이징이 개입될 경우) 서비스 레이어 테스트로 확인해 주세요.

Comment on lines 152 to 161
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.join(region).on(country.regionCode.eq(region.code))
.where(termEq(univApplyInfo, term));

// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}
Copy link

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

2) 빈 텍스트에서 전체 반환 보장 필요 — region INNER JOIN으로 레코드 누락 가능성이 있습니다.

  • 현재 base 쿼리에 region을 INNER JOIN하고, text가 비어 있으면 그대로 fetch합니다.
  • region 테이블에 매핑되지 않은 country.regionCode가 존재하는 경우, 의도(“모두 반환”)와 달리 결과가 줄어듭니다.
  • 필수 수정: LEFT JOIN으로 바꿔 누락을 방지하세요. 선택 개선: region JOIN을 “text가 있을 때만” 추가해 불필요 조인을 피하세요.
    1. 필수 수정(diff)
-                .join(region).on(country.regionCode.eq(region.code))
+                .leftJoin(region).on(country.regionCode.eq(region.code))
2) 선택 개선(아이디어)
    - base에는 region JOIN을 두지 않고, text 분기에서만 region LEFT JOIN을 추가합니다.  
    - 이는 실행 계획 단순화와 성능 개선에 도움이 됩니다.  
  • 데이터에 region 미존재 국가 코드가 포함된 픽스처를 추가하고, text=null/blank인 케이스에서 전체 개수가 유지되는지 테스트해 주세요.

🏁 Script executed:

#!/bin/bash
set -e

# 1) 파일에서 region JOIN 부분 확인 (라인 140–180)
if [ -f src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java ]; then
  sed -n '140,180p' src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
else
  echo "File not found: UnivApplyInfoFilterRepositoryImpl.java"
fi

# 2) join(region) 사용 위치 검색
rg -n "join(region)" src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java || true

# 3) leftJoin 사용 여부 확인
rg -n "leftJoin"  src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java || true

Length of output: 2396


region JOIN을 LEFT JOIN으로 바꿔 누락을 방지하세요.

  1. 변경 1: region을 INNER JOIN에서 LEFT JOIN으로 전환합니다.
    -                .join(region).on(country.regionCode.eq(region.code))
    +                .leftJoin(region).on(country.regionCode.eq(region.code))
  2. 변경 2 (선택): 기본 쿼리(base)에는 region JOIN을 제외하고, text가 있을 때만 LEFT JOIN을 추가해 불필요한 조인을 피할 수 있습니다.
  3. 검증: region이 매핑되지 않은 국가 코드가 포함된 테스트 픽스처를 추가하고, text가 null 또는 blank일 때 전체 개수가 유지되는지 확인하는 단위 테스트를 작성하세요.
📝 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.

Suggested change
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.join(region).on(country.regionCode.eq(region.code))
.where(termEq(univApplyInfo, term));
// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.leftJoin(region).on(country.regionCode.eq(region.code))
.where(termEq(univApplyInfo, term));
// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

region 에 country.region_code 가 존재하지 않는 국가는
"데이터가 잘못된 상태"일 것이므로 검색 결과에 포함하지 않는게 좋을 것 같아.

Copy link

Choose a reason for hiding this comment

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

@nayonsoso 맞습니다! 데이터 무결성 관점에서 INNER JOIN이 올바른 선택이네요.

country.region_code가 region 테이블에 존재하지 않는다면 데이터 정합성 문제이므로, 그런 레코드들을 검색 결과에서 제외하는 것이 합리적입니다. INNER JOIN이 자연스럽게 데이터 검증 필터 역할을 하고 있군요.

제 이전 제안을 철회합니다. 현재 구현이 의도된 설계입니다.

  /|   /|  
 ( :v:  )
o_)-_-(_o

✏️ Learnings added
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.791Z
Learning: 대학지원정보 텍스트 검색에서 region과 country 간의 INNER JOIN은 데이터 무결성을 위한 의도된 설계이다. country.region_code가 region 테이블에 존재하지 않는 경우는 잘못된 데이터 상태로 간주하여 검색 결과에서 제외한다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 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.537Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.

Comment on lines 51 to 52
@ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}", cacheManager = "customCacheManager", ttlSec = 86400)
public UnivApplyInfoPreviewResponses searchUnivApplyInfoByText(String text) {
Copy link

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

1) 캐시 키에 term이 누락되었습니다 — 학기 전환 시 오염된 캐시 응답 위험.

    1. 저장소는 (text, term)로 조회하지만 캐시 키는 text만 반영되어, term 변경 시 이전 학기의 결과가 재사용될 수 있습니다.
    1. 캐시 TTL이 24시간이라 학기 전환 당일 잘못된 결과가 노출될 가능성이 큽니다.

해결 옵션:

  • 옵션 A(ThunderingHerdCaching가 SpEL 지원 시 권장):
-    @ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}", cacheManager = "customCacheManager", ttlSec = 86400)
+    @ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}:{#root.target.term}", cacheManager = "customCacheManager", ttlSec = 86400)
  • 옵션 B(인덱스 플레이스홀더만 지원 시):
-    @ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}", cacheManager = "customCacheManager", ttlSec = 86400)
-    public UnivApplyInfoPreviewResponses searchUnivApplyInfoByText(String text) {
+    @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);
     }

테스트 반영(예시, 주요 호출부만):

-    UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(null);
+    UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(null, term);

캐시 키에 term 파라미터를 반영하지 않아 학기별 데이터 오염 가능성이 있습니다.

  1. 캐시 키에 term이 누락되었습니다.
    저장소는 text와 term을 사용해 데이터를 조회하지만, 캐시 키는 text만 반영되어 term 변경 시 이전 학기 결과가 재사용될 수 있습니다.

  2. TTL 설정으로 인한 리스크가 존재합니다.
    TTL이 24시간(86400초)로 설정되어 있어 학기 전환 당일에도 잘못된 결과가 캐시된 상태로 노출될 수 있습니다.

  3. 해결 옵션을 제안드립니다.
    옵션 A (SpEL 지원 시 권장):
    @ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}:{#root.target.term}", cacheManager = "customCacheManager", ttlSec = 86400)
    옵션 B (인덱스 플레이스홀더만 지원 시):
    메서드 시그니처를 searchUnivApplyInfoByText(String text, String term)로 확장하고
    @ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}:{1}", cacheManager = "customCacheManager", ttlSec = 86400)로 변경하세요.

관련 테스트 코드(searchUnivApplyInfoByText(null, term) 호출 등)도 함께 업데이트해야 합니다.

🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java
around lines 51-52, the cache key only includes the text parameter causing
cross-term cache pollution and the 24h TTL increases risk at term boundaries;
update the caching to include the term parameter (either via SpEL referencing
the target's term or by adding a term parameter to the method signature and
including it in the key) and consider lowering TTL if appropriate; also update
any tests to call the revised method signature or to account for the term-aware
cache key so tests pass.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영 완료

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

@nayonsoso 확인해보니 캐시 키와 메서드 시그니처가 올바르게 수정되었습니다! 👍

1. 캐시 키에 term 파라미터 포함
univApplyInfoTextSearch:{0}:{1} 형태로 text와 term이 모두 반영되어 학기별 캐시 분리가 제대로 구현되었습니다.

2. 메서드 시그니처 확장
searchUnivApplyInfoByText(String text, String term) 형태로 term 파라미터가 추가되어 캐시 키와 일치합니다.

3. 테스트 코드 업데이트
테스트에서도 searchUnivApplyInfoByText(null, term) 형태로 호출하도록 수정되어 일관성이 유지됩니다.

이제 학기 전환 시에도 캐시 오염 없이 정확한 데이터가 제공될 것입니다. 수고하셨습니다!


🏁 Script executed:

#!/bin/bash
# Check the current implementation of the searchUnivApplyInfoByText method
cat src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java | grep -A 10 "searchUnivApplyInfoByText"

Length of output: 515


🏁 Script executed:

#!/bin/bash
# Check if the test file was updated to match any method signature changes
rg -n "searchUnivApplyInfoByText" src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java

Length of output: 1006


@nayonsoso 완벽하게 수정되었습니다! 🎉

확인된 수정사항:

1. 메서드 시그니처 확장 ✅
searchUnivApplyInfoByText(String text, String term) 형태로 term 파라미터가 추가되었습니다.

2. 테스트 코드 전면 업데이트 ✅
모든 테스트 호출부가 searchUnivApplyInfoByText(text, term) 형태로 일관되게 수정되었습니다. (총 7개 호출부 확인)

3. 캐시 키 수정 확인 필요
캐시 어노테이션이 univApplyInfoTextSearch:{0}:{1} 형태로 수정되었는지 마지막으로 확인해보겠습니다.

이제 학기별 캐시 분리가 제대로 작동하여 데이터 오염 문제가 해결되었습니다. 훌륭한 작업이었습니다! 👏


🏁 Script executed:

#!/bin/bash
# Check for the ThunderingHerdCaching annotation on the searchUnivApplyInfoByText method
rg -n -A 3 -B 3 "ThunderingHerdCaching.*univApplyInfoTextSearch" src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java

Length of output: 569


@nayonsoso 완벽합니다! 🎉 모든 수정사항이 정확하게 반영되었습니다!

✅ 완료된 수정사항 전체 확인:

1. 캐시 키 업데이트
univApplyInfoTextSearch:{0}:{1} 형태로 text와 term 모두 포함하여 학기별 캐시 분리 구현

2. 메서드 시그니처 확장
searchUnivApplyInfoByText(String text, String term) 형태로 term 파라미터 추가

3. 테스트 코드 일괄 업데이트
모든 호출부가 새로운 시그니처에 맞게 수정됨 (7개 호출 확인)

이제 학기 전환 시에도 캐시 오염 없이 정확한 데이터가 제공되며, 24시간 TTL로 인한 잘못된 결과 노출 위험도 완전히 해결되었습니다. 훌륭한 작업이었습니다! 👏


🧠 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.537Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Comment on lines 160 to 163
// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}
Copy link
Collaborator Author

@nayonsoso nayonsoso Aug 25, 2025

Choose a reason for hiding this comment

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

만옥님이 요청하신 사항 적용한 부분입니다.
image

Comment on lines 51 to 57
@ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}", cacheManager = "customCacheManager", ttlSec = 86400)
public UnivApplyInfoPreviewResponses searchUnivApplyInfoByText(String text) {
// todo: 구현
return null;
List<UnivApplyInfoPreviewResponse> responses = univApplyInfoRepository.findAllByText(text, term)
.stream()
.map(UnivApplyInfoPreviewResponse::from)
.toList();
return new UnivApplyInfoPreviewResponses(responses);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

지금은 페이지네이션이 아니라 전체 대학을 가져오는 것으로 구현되어있습니다.. 😓
그나마 서버 부하를 최소한으로 줄이기 위해서 캐싱을 적용했습니다.

Comment on lines +237 to +243
@Test
void 대학_국가_권역_일치_순서로_정렬하여_응답한다() {
// given
String text = "아";
UnivApplyInfo 권역_아 = univApplyInfoFixture.메이지대학_지원_정보();
UnivApplyInfo 국가_아 = univApplyInfoFixture.그라츠대학_지원_정보();
UnivApplyInfo 대학지원정보_아 = univApplyInfoFixture.아칸소주립대학_지원_정보();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 테스트코드를 위해서 "아칸소주립대학"을 fixture로 추가했습니다

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (12)
src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java (1)

42-48: 아칸소주립대학 픽스처 추가: 데이터 일관성은 좋고, 표기 통일 한 가지만 점검하면 끝입니다.

    1. UniversityFixture.아칸소_주립_대학()과 한글명 "아칸소 주립 대학"이 일치해서 텍스트 검색 시 혼동이 줄어듭니다.
    1. 다만 기존 픽스처들에 "괌대학(A형)" vs "괌 대학", "네바다주립대학" vs "네바다주립 대학"처럼 공백/표기 불일치가 있습니다. 이번 추가분은 올바르니, 차후 전체 픽스처 네이밍을 한 번에 정리하면 검색/정렬 테스트가 더 견고해집니다.
src/test/java/com/example/solidconnection/university/fixture/UniversityFixture.java (1)

35-42: Arkansas State University 픽스처: 구성 적절, 네이밍 표준화만 가벼운 제안입니다.

    1. country=미국, region=영미권 매핑이 다른 픽스처들과 일관됩니다.
    1. 한글명 "아칸소 주립 대학"은 UnivApplyInfoFixture와 동일해 텍스트 매칭에 유리합니다. 다만 전체 픽스처에서 "주립 대학/주립대학" 공백 표준만 추후 통일해 주세요.
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (3)

156-156: Region 조인은 left join이 더 안전합니다.

    1. 일부 국가가 region 매핑을 갖지 않는다면 inner join으로 인해 결과가 누락될 수 있습니다.
    1. 텍스트 검색에서 지역명이 필수가 아니라면 left join이 의도에 더 부합합니다.
-                .join(region).on(country.regionCode.eq(region.code))
+                .leftJoin(region).on(country.regionCode.eq(region.code))

166-168: contains 대신 containsIgnoreCase 사용으로 검색 관용성을 높여 주세요.

    1. 영문 입력(예: "arkansas") 또는 대소문자 변형에서 매칭 누락을 방지합니다.
    1. 한국어에는 영향이 없고, 혼합 언어 데이터에서도 유리합니다.
-        BooleanExpression univApplyInfoLike = univApplyInfo.koreanName.contains(text);
-        BooleanExpression countryLike = country.koreanName.contains(text);
-        BooleanExpression regionLike = region.koreanName.contains(text);
+        BooleanExpression univApplyInfoLike = univApplyInfo.koreanName.containsIgnoreCase(text);
+        BooleanExpression countryLike = country.koreanName.containsIgnoreCase(text);
+        BooleanExpression regionLike = region.koreanName.containsIgnoreCase(text);

165-173: 부분 일치(contains) 남용은 인덱스 활용이 어려워집니다. 중장기적으로 검색 인프라를 검토해 주세요.

    1. leading wildcard('%text%')는 RDB 인덱스를 거의 쓰지 못해 데이터가 늘면 급격한 성능 저하가 올 수 있습니다.
    1. 대안: 한국어 형태소를 고려한 Full-Text Search(Elasticsearch/OpenSearch), 혹은 DB 측 ngram/GIN(예: PostgreSQL pg_trgm) 인덱스 도입을 제안합니다.
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (7)

6-6: assertThatCode 래핑은 과합니다. 단순 비교로 실패 메시지를 더 명확히 하세요.
테스트 본질은 두 응답이 동일함을 검증하는 것입니다. 불필요한 예외-래핑을 제거하면 가독성과 실패 디버깅이 좋아집니다.
1) assertThatCode(...).doesNotThrowAnyException()는 여기서 의미가 약합니다.
2) 직접 리스트를 비교하면 충분합니다.

아래처럼 정리해 보세요.

-            assertThatCode(() -> {
-                List<Long> firstResponseIds = extractIds(firstResponse);
-                List<Long> secondResponseIds = extractIds(secondResponse);
-                assertThat(firstResponseIds).isEqualTo(secondResponseIds);
-            }).doesNotThrowAnyException();
+            List<Long> firstResponseIds = extractIds(firstResponse);
+            List<Long> secondResponseIds = extractIds(secondResponse);
+            assertThat(firstResponseIds).isEqualTo(secondResponseIds);

27-27: @value 주입은 테스트를 취약하게 합니다. 테스트 프로퍼티 명시 또는 검증 완화를 권장합니다.
프로퍼티가 누락되면 컨텍스트 로딩 단계에서 바로 실패합니다. 목적이 캐시 호출 수 검증이라면 인자 값 동등성까지 강하게 고정하지 않아도 됩니다.
1) 옵션 A: @TestPropertySource로 university.term을 명시해 실행환경 의존을 제거하세요.
2) 옵션 B: 레포 호출 검증을 호출 횟수 중심으로 완화하거나 anyString()/eq(term) 조합으로 유연하게 두세요.
3) 접근 제어자는 public일 이유가 없습니다. private 권장입니다.

아래 최소 변경으로 접근제어자를 완화하세요.

-    @Value("${university.term}")
-    public String term;
+    @Value("${university.term}")
+    private String term;

또는 테스트 클래스 상단에 명시적으로 프로퍼티를 지정하는 방법입니다. (선호)

import org.springframework.test.context.TestPropertySource;

@TestPropertySource(properties = "university.term=TEST_TERM")
class UnivApplyInfoQueryServiceTest {
    // ...
}

Also applies to: 46-48


160-174: null 텍스트 전체 반환 테스트는 데이터 격리에 민감합니다. ID 비교로 안정성을 높이세요.
다른 테스트의 픽스처가 누적되면 containsExactly가 깨질 수 있습니다. ID 리스트로 비교하면 의도가 선명하고 누적 데이터에도 강합니다.
1) extractIds를 재사용해 기대 ID 시퀀스를 직접 비교하세요.
2) 실패 메시지도 더 직관적입니다.

-            assertThat(response.univApplyInfoPreviews())
-                    .containsExactly(
-                            UnivApplyInfoPreviewResponse.from(괌대학_A_지원_정보),
-                            UnivApplyInfoPreviewResponse.from(메이지대학_지원_정보)
-                    );
+            assertThat(extractIds(response))
+                    .containsExactly(
+                            괌대학_A_지원_정보.getId(),
+                            메이지대학_지원_정보.getId()
+                    );

176-196: 대학명 부분일치 테스트 좋습니다. 동률일 때 id 오름차순 타이브레이커도 고정합시다.
정렬 규칙은 회귀가 자주 발생하는 지점입니다. 같은 랭크 안에서 동률이 발생할 때 id 오름차순이 유지되는지 추가 테스트로 못 박아두면 안전합니다.
1) 동일 랭크(대학명 매칭) 두 건 이상을 만들고 id 정렬을 검증하세요.
2) 향후 랭크 산정식 변경에도 타이브레이커 규칙이 보장됩니다.

예시:

@Test
void 동일_랭크_동률이면_id_오름차순으로_정렬한다() {
    // given
    String text = "메";
    UnivApplyInfo a1 = univApplyInfoFixture.메_접두_대학_1(); // 더 작은 ID
    UnivApplyInfo a2 = univApplyInfoFixture.메_접두_대학_2(); // 더 큰 ID

    // when
    UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(text);

    // then
    assertThat(extractIds(response))
            .containsSubsequence(a1.getId(), a2.getId()); // 같은 랭크 내 id 오름차순
}

원하시면 보조 픽스처까지 포함해 드리겠습니다.


237-255: 매칭 우선순위 정렬 테스트 훌륭합니다. 공백/대소문자 케이스를 보강하면 UX 회귀를 막을 수 있습니다.
검색어 전처리(trim, 대소문자 무시)가 서비스에 존재한다면 이를 테스트로 고정해두는 편이 안전합니다.
1) " 아 "처럼 공백이 섞여도 결과가 동일한지 검증하세요.
2) "guam" / "GUAM" 등 대소문자 혼용에서도 결과가 변하지 않음을 고정하세요.

예시:

@Test
void 앞뒤_공백이_있어도_동일한_검색_결과를_반환한다() {
    String text = " 아 ";
    List<Long> base = extractIds(univApplyInfoQueryService.searchUnivApplyInfoByText(text.trim()));
    List<Long> spaced = extractIds(univApplyInfoQueryService.searchUnivApplyInfoByText(text));
    assertThat(spaced).isEqualTo(base);
}

@Test
void 대소문자를_무시하고_검색한다() {
    String lower = "guam";
    String upper = "GUAM";
    assertThat(extractIds(univApplyInfoQueryService.searchUnivApplyInfoByText(lower)))
            .isEqualTo(extractIds(univApplyInfoQueryService.searchUnivApplyInfoByText(upper)));
}

268-274: 캐시 테스트의 목적을 선명하게. 호출 횟수만 검증하도록 인자 매칭을 유연화하세요.
텍스트 정규화가 도입되면 인자 일치 검증이 불필요하게 깨질 수 있습니다. 호출 수(times(1))만 고정하면 캐시 동작을 안정적으로 보장합니다.
1) anyString()/eq(term)로 완화하거나 둘 다 anyString()으로 두어도 충분합니다.
2) 프로퍼티 주입 실패 시에도 테스트가 덜 민감해집니다.

-            then(univApplyInfoRepository).should(times(1)).findAllByText(text, term);
+            then(univApplyInfoRepository).should(times(1))
+                    .findAllByText(
+                            org.mockito.ArgumentMatchers.anyString(),
+                            org.mockito.ArgumentMatchers.eq(term)
+                    );

(더 느슨하게 하려면 두 번째 인자도 anyString()으로 바꿀 수 있습니다.)


276-281: extractIds 유틸은 재사용 가치가 높습니다. 공용 테스트 유틸로 승격을 고려하세요.
여러 프리뷰 응답 비교 테스트에서 반복 사용될 가능성이 큽니다.
1) support 패키지의 TestExtractors 같은 헬퍼 클래스로 옮기면 중복을 줄일 수 있습니다.
2) 제네릭 컬렉션 추출 유틸로 확장하면 더 유용합니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3f8c493 and 29ae322.

📒 Files selected for processing (6)
  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java (1 hunks)
  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2 hunks)
  • src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/fixture/UnivApplyInfoFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/fixture/UniversityFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (2)
src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java (2)
  • Nested (46-68)
  • Nested (70-92)
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (4)
  • Nested (194-241)
  • Nested (243-280)
  • Nested (282-324)
  • Nested (326-415)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (5)
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepository.java (1)

13-13: findAllByText 시그니처 추가 적합.

    1. 서비스/구현과의 정합성이 좋고, 책임 분리가 명확합니다.
    1. 메서드 계약(정렬 우선순위, text null/blank 동작)에 대한 간단한 Javadoc을 추가하면 사용처 가독성이 좋아집니다.
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1)

51-58: 제가 UnivApplyInfoQueryService.java 파일 내 term 변수의 정의 및 @ThunderingHerdCaching 어노테이션 사용 위치를 확인해 보겠습니다. 완료되는 대로 검증 결과를 바탕으로 리뷰 코멘트를 다듬겠습니다.

src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2)

4-17: 새로운 import 추가 구성은 적절합니다.

    1. 순위 계산(CaseBuilder), 동적 where(BooleanBuilder), 정렬(NumberExpression), 커스텀 쿼리(JPAQuery) 모두 필요한 의존입니다.
    1. Region 조인과 랭킹 표현식을 위해 QRegion, CaseBuilder 포함이 타당합니다.

181-184: 랭킹 기반 정렬은 명확하고 확장성 있습니다.

    1. 대학명(0) > 국가명(1) > 지역명(2) 우선순위가 요구사항에 잘 부합합니다.
    1. 이후 다른 필드를 포함한 가중치 확장도 CaseBuilder 체계에서 수월합니다.
src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (1)

198-216: 국가명/권역명 검색 시나리오가 명확합니다. 정렬과 서브셋 검증이 적절합니다.
불필요한 준비 데이터가 결과에 섞이지 않도록 각 테스트가 필요한 픽스처만 준비한 점도 좋습니다.

Also applies to: 217-235

Comment on lines 153 to 164
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.join(region).on(country.regionCode.eq(region.code))
.leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
.where(termEq(univApplyInfo, term));

// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}

Copy link

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

컬렉션 fetch join으로 인해 결과 중복 위험이 있습니다. distinct를 추가해 주세요.

    1. languageRequirements에 대한 left fetch join이 있어 UnivApplyInfo가 다대일 관계에서 중복으로 펼쳐질 수 있습니다.
    1. 기존 다른 메서드(findAllByRegionCodeAndKeywords, findAllByFilter)는 .distinct()를 적용하고 있습니다. 동일하게 맞춰 주세요.

다음 변경으로 베이스 쿼리 단계에서 중복을 제거할 수 있습니다.

-        JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
+        JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
                 .join(univApplyInfo.university, university).fetchJoin()
                 .join(university.country, country).fetchJoin()
-                .join(region).on(country.regionCode.eq(region.code))
+                .join(region).on(country.regionCode.eq(region.code))
                 .leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
-                .where(termEq(univApplyInfo, term));
+                .where(termEq(univApplyInfo, term))
+                .distinct();

또는 반환 지점에서 .distinct()를 추가하는 것도 가능하지만, 베이스에 두는 편이 중복 제거 의도가 명확합니다.

📝 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.

Suggested change
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.join(region).on(country.regionCode.eq(region.code))
.leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
.where(termEq(univApplyInfo, term));
// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}
JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
.join(univApplyInfo.university, university).fetchJoin()
.join(university.country, country).fetchJoin()
.join(region).on(country.regionCode.eq(region.code))
.leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
.where(termEq(univApplyInfo, term))
.distinct();
// text 가 비어있다면 모든 대학지원정보를 id 오름차순으로 정렬하여 반환
if (text == null || text.isBlank()) {
return base.orderBy(univApplyInfo.id.asc()).fetch();
}
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
around lines 153 to 164, the base JPAQuery can return duplicate UnivApplyInfo
rows due to the left fetch join on languageRequirements; add .distinct() to the
base query so duplicates are eliminated at the query source (e.g.,
queryFactory.selectFrom(univApplyInfo).distinct()... ), keeping the existing
fetch joins and where(termEq(...)) intact; ensure you place .distinct() on the
base query before applying orderBy/fetch() so behaviour matches the other
methods that already use .distinct().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

distinct 를 쓰면 range 로 정렬하지 못해.

Copy link

Choose a reason for hiding this comment

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

@nayonsoso 맞습니다! QueryDSL에서 distinct()와 계산된 rank 표현식을 함께 사용할 때 정렬에 문제가 생길 수 있네요.

대신 다음과 같은 대안들을 고려해볼 수 있습니다:

1. Java 스트림으로 중복 제거 (추천)

return base.where(where)
        .orderBy(rank.asc(), univApplyInfo.id.asc())
        .fetch()
        .stream()
        .distinct()
        .collect(toList());

2. 서브쿼리 활용

// 먼저 ID만 조회해서 중복 제거 후, 다시 fetch join으로 조회
List<Long> distinctIds = queryFactory
    .select(univApplyInfo.id)
    .from(univApplyInfo)
    // ... joins and where conditions
    .where(where)
    .orderBy(rank.asc(), univApplyInfo.id.asc())
    .distinct()
    .fetch();

return queryFactory.selectFrom(univApplyInfo)
    // ... fetch joins
    .where(univApplyInfo.id.in(distinctIds))
    .orderBy(rank.asc(), univApplyInfo.id.asc())
    .fetch();

어떤 방식이 더 적절하다고 생각하시나요? 아니면 실제로 중복 데이터가 발생하는지 테스트해보신 결과가 있으신가요?

 (\   /)
( ._.)

o_(")(") curious about the technical details


✏️ Learnings added
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:23:29.514Z
Learning: QueryDSL에서 distinct()와 계산된 rank 표현식(CaseBuilder 등)을 함께 사용하면 정렬(orderBy)에 문제가 발생할 수 있다. 대신 Java 스트림이나 서브쿼리 방식으로 중복을 처리해야 한다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 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.537Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.

}

@Transactional(readOnly = true)
@ThunderingHerdCaching(key = "univApplyInfoTextSearch:{0}", cacheManager = "customCacheManager", ttlSec = 86400)
Copy link
Member

Choose a reason for hiding this comment

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

캐시를 안 써봐서 잘 모르긴 한데 ... tem 을 캐시 키에 포함시켜야 하지 않나요 ?

Copy link
Collaborator Author

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 left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (1)

27-34: 1) 캐시 키 자리수 불일치: {0}:{1} → {0}로 수정 필요합니다.

메서드 인자가 1개인데 키 템플릿은 두 자리({0}:{1})를 요구하고 있어 런타임 포맷 오류 또는 의도치 않은 키 생성 위험이 있습니다. id는 고유 키이므로 단일 자리로 충분합니다.

적용 diff:

-    @ThunderingHerdCaching(key = "univApplyInfo:{0}:{1}", cacheManager = "customCacheManager", ttlSec = 86400)
+    @ThunderingHerdCaching(key = "univApplyInfo:{0}", cacheManager = "customCacheManager", ttlSec = 86400)

추가 메모:

  • 만약 학기(term)에 따라 동일 id가 달라지는 구조라면, 메서드에 term을 인자로 추가하고 컨트롤러에서 전달받아 {0}:{1} 형태로 사용하는 방식으로 일관성 있게 맞추는 것도 가능합니다.
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (1)

153-159: 1) 컬렉션 fetch join으로 인한 중복 방지를 위해 distinct 추가가 필요합니다.

languageRequirements를 left fetch join하고 있어 동일 UnivApplyInfo가 다중 로우로 펼쳐질 수 있습니다. 실제로 다른 메서드(findAllByRegionCodeAndKeywords, findAllByFilter)는 .distinct()를 적용 중입니다. 본 베이스 쿼리에도 동일 적용이 필요합니다.

적용 diff:

-        JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
+        JPAQuery<UnivApplyInfo> base = queryFactory.selectFrom(univApplyInfo)
                 .join(univApplyInfo.university, university).fetchJoin()
                 .join(university.country, country).fetchJoin()
                 .join(region).on(country.regionCode.eq(region.code))
                 .leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
-                .where(termEq(univApplyInfo, term));
+                .where(termEq(univApplyInfo, term))
+                .distinct();

메모:

  • text가 비어 있는 분기(라인 160–163)도 base를 그대로 사용하므로 동일하게 혜택을 받습니다.
  • Preview 응답에 languageRequirements가 필요 없다면 fetchJoin 자체를 제거하는 것도 선택지입니다(아래 선택 개선 참고).
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.java (2)

36-38: 1) term 필드는 public → private로 축소해 주세요.

컨트롤러의 설정 값은 외부로 노출될 필요가 없습니다. 접근 제어를 좁혀 불필요한 가시성과 오용 가능성을 줄이는 편이 안전합니다.

적용 diff:

-    @Value("${university.term}")
-    public String term;
+    @Value("${university.term}")
+    private String term;

104-107: 3) 쿼리 파라미터 이름 변경(value)로 인한 호환성 확인이 필요합니다.

기존 클라이언트가 text 파라미터로 호출하고 있었다면 호환성 이슈가 발생할 수 있어요. 두 이름을 모두 받거나, API 문서/프론트와 동시 반영을 보장해 주세요.

권장 diff(양쪽 파라미터 허용):

-    public ResponseEntity<UnivApplyInfoPreviewResponses> searchUnivApplyInfoByText(
-            @RequestParam(required = false) String value
-    ) {
-        UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(value, term);
+    public ResponseEntity<UnivApplyInfoPreviewResponses> searchUnivApplyInfoByText(
+            @RequestParam(name = "value", required = false) String value,
+            @RequestParam(name = "text", required = false) String text
+    ) {
+        String q = (value != null && !value.isBlank()) ? value : text;
+        UnivApplyInfoPreviewResponses response = univApplyInfoQueryService.searchUnivApplyInfoByText(q, term);
         return ResponseEntity.ok(response);
     }
src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2)

160-163: 2) 빈 텍스트 전체 조회 분기에서도 중복 제거가 보장되어야 합니다.

현재는 base.orderBy(...).fetch()만 호출합니다. 위에서 제안한 base.distinct()가 반영되면 본 분기도 함께 안전해집니다.

검증 가이드:

  • 동일 UnivApplyInfo에 languageRequirements가 두 개 이상인 데이터로 테스트 픽스처를 추가하고, text=null/blank 시 반환 개수가 기대치(고유 UnivApplyInfo 수)와 일치하는지 확인해 주세요.

153-159: 4) 선택 개선: 불필요한 컬렉션 fetch join 제거로 성능/중복 리스크를 추가로 낮출 수 있습니다.

Preview 응답에서 languageRequirements를 소비하지 않는다면, 단순 leftJoin으로 전환해도 됩니다. 이는 중복 리스크와 메모리 사용량을 줄이고 실행 계획을 단순화합니다.

대안 diff:

-                .leftJoin(univApplyInfo.languageRequirements, languageRequirement).fetchJoin()
+                .leftJoin(univApplyInfo.languageRequirements, languageRequirement)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 29ae322 and fc0b5ea.

📒 Files selected for processing (4)
  • src/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.java (3 hunks)
  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (2 hunks)
  • src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (3 hunks)
  • src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/example/solidconnection/university/service/UnivApplyInfoQueryServiceTest.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.537Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.
📚 Learning: 2025-08-26T05:07:54.791Z
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.791Z
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.537Z
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.537Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.

Applied to files:

  • src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java
  • src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.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 (4)
src/main/java/com/example/solidconnection/university/controller/UnivApplyInfoController.java (1)

98-99: 2) term 전달 흐름 좋습니다.

필터 검색에서도 term을 서비스로 전달하도록 맞춘 점이 일관되고 명확합니다.

src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java (1)

165-179: 3) 검색 대상과 랭킹 정책이 의도와 일치합니다.

  • UnivApplyInfo.koreanName, country.koreanName, region.koreanName OR 매칭과 랭킹(대학지원정보명 > 국가명 > 지역명) 적용이 설계 의도와 부합합니다. 이어지는 정렬(rank, id)도 명확합니다.
src/main/java/com/example/solidconnection/university/service/UnivApplyInfoQueryService.java (2)

36-44: 2) 필터 검색의 term 전파 정상 동작 확인했습니다.

서비스 레이어에서도 term을 명시적으로 받아 저장소에 위임하는 흐름이 명확합니다.


47-54: 3) 텍스트 검색 캐시 키에 term 포함되어 오염 위험 해소되었습니다.

키 서픽스에 {1}(term)을 포함시켜 학기 전환 시 캐시 오염 가능성을 제거한 점 좋습니다.

@nayonsoso nayonsoso merged commit a16ce9e into solid-connection:develop Aug 26, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: 대학 텍스트 검색 분리

2 participants