fix: 페이지네이션 page 파라미터가 1 미만이면 예외를 발생시키도록 - #830
Open
whqtker wants to merge 3 commits into
Open
Conversation
GET /admin/host-universities 등에서 page=0/1이 모두 내부 오프셋 0으로 클램프되어 첫 페이지가 중복 조회되고 마지막 페이지가 영구 누락되는 문제(#829)의 근본 원인은 fetch join이 아니라, CustomPageableHandlerMethod ArgumentResolver의 1-indexed 계약을 0-indexed로 오사용한 클라이언트였다. page<1 요청을 조용히 기본값으로 클램프하는 대신 400으로 명시적으로 거부해 향후 동일한 오사용이 재발해도 즉시 드러나도록 한다. ingest_universities.py의 임시 keyword 우회 방식도 1-indexed 벌크 fetch로 복원. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WTCqRi8V2TJ1ZqHpfiQ1vy
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WTCqRi8V2TJ1ZqHpfiQ1vy
whqtker
requested review from
Gyuhyeok99,
Hexeong,
lsy1307 and
wibaek
as code owners
August 11, 2026 05:41
Walkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WTCqRi8V2TJ1ZqHpfiQ1vy
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/load-universities/scripts/ingest_universities.py:
- Around line 383-392: Update the by_name construction in the university
ingestion flow to detect when different records produce the same trimmed name
key and raise IngestionError before overwriting the existing mapping. Apply this
collision check to the stripped aliases for koreanName, englishName, and
formatName while preserving valid exact-name mappings and allowing the same
record to reuse a key.
In
`@src/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.java`:
- Around line 46-52: Update the page validation in
CustomPageableHandlerMethodArgumentResolver to parse and compare out-of-range
negative values without allowing NumberFormatException to bypass
INVALID_PAGE_PARAMETER; use BigInteger or equivalent arbitrary-precision
validation against MIN_ONE_INDEXED_PAGE, and add boundary tests covering values
below Integer.MIN_VALUE.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a79d696-6675-47bf-b1ef-a9724bf45b8a
📒 Files selected for processing (4)
.claude/skills/load-universities/scripts/ingest_universities.pysrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.javasrc/test/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolverTest.java
Comment on lines
383
to
+392
| for name_key in ("koreanName", "englishName", "formatName"): | ||
| name = item.get(name_key) | ||
| if name: | ||
| by_name[name] = item | ||
| # parse_rows() always strips field values via clean(), so a DB | ||
| # record whose name has stray whitespace (a data-entry artifact) | ||
| # would otherwise never match row.host_korean_name/english_name. | ||
| stripped = name.strip() | ||
| if stripped and stripped != name: | ||
| by_name[stripped] = item |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 호스트 대학 이름의 정규화 후 유일성과 중복 검출을 확인합니다.
rg -n -C 6 \
'host-universities|HostUniversity|koreanName|englishName|formatName|unique|Unique|duplicate' \
--glob '*.java' \
--glob '*.py' \
--glob '*.sql' .Repository: solid-connection/solid-connect-server
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- HostUniversity definitions and admin API ---'
rg -n -C 12 \
'class HostUniversity|record AdminHostUniversityResponse|host-universities|HostUniversityRepository|findAll|Pageable|koreanName|englishName|formatName' \
src/main/java/com/example/solidconnection/university \
src/main/java/com/example/solidconnection/admin \
--glob '*.java' | head -n 1200
printf '%s\n' '--- Host-university schema and seed data ---'
rg -n -C 8 \
'host_university|host-universit|korean_name|english_name|format_name' \
src/main/resources .claude/skills/load-universities \
--glob '*.sql' --glob '*.csv' --glob '*.json' --glob '*.py' 2>/dev/null | head -n 1200Repository: solid-connection/solid-connect-server
Length of output: 50394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Candidate files ---'
fd -t f 'HostUniversity|host.*univers|Admin.*Univers|University.*Controller|University.*Service|University.*Repository' src/main/java .claude/skills/load-universities
printf '%s\n' '--- Exact host-university references ---'
rg -l 'HostUniversity|host-universities' src/main/java --glob '*.java'
printf '%s\n' '--- Entity constraints and update paths ---'
rg -n -C 10 'class HostUniversity|`@Column`|unique|update|create|save|findAll' \
src/main/java/com/example/solidconnection/university/domain/HostUniversity.java \
src/main/java/com/example/solidconnection/university/repository/HostUniversityRepository.java \
src/main/java/com/example/solidconnection/admin \
--glob '*.java' | rg -C 4 'HostUniversity|host-universities|koreanName|englishName|formatName|unique|Unique|update|create|findAll'Repository: solid-connection/solid-connect-server
Length of output: 50393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
src/main/java/com/example/solidconnection/admin/university/controller/AdminHostUniversityController.java \
src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java \
src/main/java/com/example/solidconnection/university/repository/HostUniversityRepository.java \
src/main/java/com/example/solidconnection/university/repository/custom/HostUniversityFilterRepository.java \
src/main/java/com/example/solidconnection/university/repository/custom/HostUniversityFilterRepositoryImpl.java \
src/main/java/com/example/solidconnection/admin/university/dto/AdminHostUniversityCreateRequest.java \
src/main/java/com/example/solidconnection/admin/university/dto/AdminHostUniversityUpdateRequest.java \
src/main/java/com/example/solidconnection/university/domain/HostUniversity.java
do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '\n--- Relevant error codes ---\n'
rg -n -C 3 'HOST_UNIVERSITY|UNIVERSITY_ALREADY|DUPLICAT' \
src/main/java --glob '*.java' | head -n 300Repository: solid-connection/solid-connect-server
Length of output: 50393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Ingestion call graph and name lookup ---'
cat -n .claude/skills/load-universities/scripts/ingest_universities.py | sed -n '330,415p;590,645p'
printf '%s\n' '--- Read-only behavioral verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
entity = Path("src/main/java/com/example/solidconnection/university/domain/HostUniversity.java").read_text()
service = Path("src/main/java/com/example/solidconnection/admin/university/service/AdminHostUniversityService.java").read_text()
query = Path("src/main/java/com/example/solidconnection/university/repository/custom/HostUniversityFilterRepositoryImpl.java").read_text()
ingest = Path(".claude/skills/load-universities/scripts/ingest_universities.py").read_text()
fields = ("koreanName", "englishName", "formatName")
print("entity_unique_annotations:")
for field in fields:
match = re.search(
rf'`@Column`\([^)]*\)\s*private\s+String\s+{field}\s*;',
entity,
re.S,
)
declaration = match.group(0) if match else "<missing>"
print(f" {field}: {'unique = true' in declaration} | {declaration.replace(chr(10), ' ')}")
print("service_duplicate_checks:")
for field in fields:
count = len(re.findall(rf'findBy{field[0].upper() + field[1:]}\s*\(', service))
print(f" {field}: repository lookup count={count}")
print("query_order:")
order = re.search(r'\.orderBy\(([^)]*)\)', query)
print(f" {order.group(1).strip() if order else '<missing>'}")
print("index_collision_simulation:")
records = [
{"id": 101, "koreanName": "한국대", "englishName": " A", "formatName": "표시A"},
{"id": 202, "koreanName": "다른대", "englishName": "A", "formatName": "표시B"},
]
by_name = {}
for item in records:
for key in fields:
name = item.get(key)
if name:
by_name[name] = item
stripped = name.strip()
if stripped and stripped != name:
by_name[stripped] = item
print(f" lookup['A'] -> id={by_name['A']['id']}")
print(" collision: distinct ids 101 and 202 normalize to the same lookup key")
print("ingestion_error_symbol:")
print(f" IngestionError referenced: {'IngestionError' in ingest}")
PYRepository: solid-connection/solid-connect-server
Length of output: 8598
trim() 키 충돌을 감지하고 IngestionError로 중단해 주세요.
HostUniversity는 koreanName만 정확한 값에 대해 유일성을 보장합니다. englishName과 formatName에는 유일성 제약과 중복 검증이 없습니다. 따라서 서로 다른 레코드가 trim 후 같은 키를 만들면 후속 레코드가 by_name 값을 덮어쓰고 잘못된 host["id"]를 선택합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/load-universities/scripts/ingest_universities.py around lines
383 - 392, Update the by_name construction in the university ingestion flow to
detect when different records produce the same trimmed name key and raise
IngestionError before overwriting the existing mapping. Apply this collision
check to the stripped aliases for koreanName, englishName, and formatName while
preserving valid exact-name mappings and allowing the same record to reuse a
key.
Comment on lines
+46
to
+52
| try { | ||
| if (Integer.parseInt(pageParameter) < MIN_ONE_INDEXED_PAGE) { | ||
| throw new CustomException(INVALID_PAGE_PARAMETER); | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| // 숫자로 파싱할 수 없는 값은 기존과 동일하게 상위 리졸버가 기본값으로 대체한다. | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.java --items all
rg -n -C 4 'Integer\.parseInt|NumberFormatException|INVALID_PAGE_PARAMETER|provideOutOfRangePageParameters' \
src/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.java \
src/test/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolverTest.javaRepository: solid-connection/solid-connect-server
Length of output: 7972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- resolver ---'
cat -n src/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.java
printf '%s\n' '--- tests (relevant sections) ---'
sed -n '1,155p' src/test/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolverTest.java
printf '%s\n' '--- numeric behavior ---'
python3 - <<'PY'
values = ["-1", "0", "1", "-2147483648", "-2147483649", "2147483647", "2147483648", "invalid"]
for value in values:
try:
n = int(value)
result = "INVALID_PAGE_PARAMETER" if n < 1 else "accepted by validation"
except ValueError:
result = "NumberFormatException path"
print(f"{value}: {result}")
PY
printf '%s\n' '--- resolver configuration and usages ---'
rg -n -C 5 'CustomPageableHandlerMethodArgumentResolver|setOneIndexedParameters|resolveArgument\\(' src/main src/testRepository: solid-connection/solid-connect-server
Length of output: 8881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Java parseInt boundary model ---'
python3 - <<'PY'
def java_parse_int(value):
if not value:
raise ValueError("NumberFormatException")
sign = -1 if value[0] == "-" else 1
digits = value[1:] if value[0] in "+-" else value
if not digits or not digits.isdecimal():
raise ValueError("NumberFormatException")
result = sign * int(digits)
if result < -(2**31) or result > 2**31 - 1:
raise ValueError("NumberFormatException")
return result
for value in ["-1", "0", "-2147483648", "-2147483649", "2147483647", "2147483648"]:
try:
parsed = java_parse_int(value)
print(f"{value}: parsed={parsed}, below_one={parsed < 1}")
except ValueError:
print(f"{value}: NumberFormatException")
PY
printf '%s\n' '--- error mapping ---'
rg -n -C 4 'INVALID_PAGE_PARAMETER|enum ErrorCode|CustomException' \
src/main/java src/test/java \
--glob '*.java' | head -160Repository: solid-connection/solid-connect-server
Length of output: 19526
1. 범위를 벗어난 음수 page를 거부하세요.
Integer.parseInt는 -2147483649를 NumberFormatException으로 처리합니다. 현재 코드는 이 예외를 무시하므로 INVALID_PAGE_PARAMETER가 발생하지 않습니다. BigInteger 등으로 값을 비교하고 경계값 테스트를 추가하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/example/solidconnection/common/resolver/CustomPageableHandlerMethodArgumentResolver.java`
around lines 46 - 52, Update the page validation in
CustomPageableHandlerMethodArgumentResolver to parse and compare out-of-range
negative values without allowing NumberFormatException to bypass
INVALID_PAGE_PARAMETER; use BigInteger or equivalent arbitrary-precision
validation against MIN_ONE_INDEXED_PAGE, and add boundary tests covering values
below Integer.MIN_VALUE.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d460611cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+379
to
+381
| page = 1 | ||
| while True: | ||
| response = api.request_json("GET", "/admin/host-universities", query={"page": page, "size": 100}) | ||
| response = api.request_json("GET", "/admin/host-universities", query={"page": page, "size": 50}) |
There was a problem hiding this comment.
When the university-import workflow is run through the tracked Codex skill, .codex/skills/load-universities/scripts/ingest_universities.py:379-381 still initializes page = 0 and sends it to /admin/host-universities. The new resolver rejects that first request, so both preflight and apply abort with HTTP 400 before loading any hosts; this commit updates only the .claude copy. Update the .codex copy as well, or deduplicate the helpers before enforcing the new minimum.
Useful? React with 👍 / 👎.
whqtker
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)