Skip to content

test: BannerService/S3Service 단위 테스트 추가 + 이미지 업로드 크래시 버그 2건 수정 - #224

Merged
unam98 merged 1 commit into
devfrom
tests/banner-s3-service-unit-tests
Aug 5, 2026
Merged

test: BannerService/S3Service 단위 테스트 추가 + 이미지 업로드 크래시 버그 2건 수정#224
unam98 merged 1 commit into
devfrom
tests/banner-s3-service-unit-tests

Conversation

@unam98

@unam98 unam98 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

작업 배경

  • 테스트 커버리지 확장 마지막 단계. 남아있던 BannerService, S3Service에 단위 테스트 추가. S3ServiceAmazonS3가 생성자 주입돼있어 (Auth 계열과 달리) 순수 단위 테스트가 가능했음.

변경 사항

영역 내용
S3Service.getFileExtension 파일명 null 체크 추가, 확장자(".") 없는 파일명 처리 추가
BannerServiceTest, S3ServiceTest 단위 테스트 9개 신규

발견해서 수정한 버그

S3Service.uploadImage가 업로드 파일명 검증 단계에서 두 가지 케이스에 500을 냈음:

  1. 파일명이 null이면 fileName.length()에서 NPE
  2. 파일명에 "."가 없으면 lastIndexOf가 -1을 반환하고 그대로 substring(-1)을 호출해 StringIndexOutOfBoundsException

둘 다 원래 "잘못된 이미지 파일입니다"(400/404)로 처리됐어야 할 케이스가 서버 에러로 새어나가고 있었음.

⚠️ 확인만 하고 고치지 않은 것

S3Service.deleteFile()imageUrl.substring(49)는 폴더명/버킷명 길이에 따라 항상 값이 달라지는데 49로 하드코딩돼있어 구조적으로 항상 맞을 수 없음. 다만 코드베이스 전체에서 이 메서드를 실제로 호출하는 곳이 없어(dead code) 지금 당장 영향은 없음. 실제 S3 URL 포맷 확인 없이 숫자만 바꾸는 건 오히려 위험해서 이번엔 손대지 않음 — 나중에 deleteFile을 실제로 연결해서 쓰기 전에 반드시 점검 필요.

영향 범위

  • 이미지 업로드 API에서 파일명이 없거나(null/빈값) 확장자가 없는 잘못된 요청을 보내는 극단적 케이스가 500 대신 400/404로 정상 처리됨. 정상적인 파일 업로드 흐름에는 영향 없음.
  • 런타임 영향 없음.

검증 매트릭스

영향 범위 테스트 코드
배너 목록 조회 - 순서/인덱싱 활성_배너를_정렬된_순서_그대로_0부터_인덱싱해서_반환한다
활성_배너가_없으면_빈_목록을_반환한다
이미지 업로드 - 정상/확장자 검증 정상_업로드
대문자_확장자
지원하지_않는_확장자
스트림_읽기_실패
이미지 업로드 크래시 버그 수정 검증 파일명이_null
파일명이_빈문자열
확장자가_없는_파일명

Test Plan

  • 로컬에서 신규 테스트 9개 전부 통과
  • 전체 테스트(188개) 함께 실행해도 간섭 없음 확인
  • 로컬 DB/Redis 띄우고 ./gradlew build 전체(ServerApplicationTests 포함) 통과 확인

🤖 Generated with Claude Code

BannerService 2개, S3Service.uploadImage 7개, 총 9개 신규 테스트.

테스트 작성 중 발견해서 수정한 버그 (S3Service.getFileExtension):
1. 파일명이 null이면 length() 호출에서 NPE → 500. null/빈 문자열
   체크 추가.
2. 파일명에 "."가 없으면 lastIndexOf가 -1을 반환하고 그 값 그대로
   substring(-1)을 호출해 StringIndexOutOfBoundsException → 500.
   "."가 없는 경우를 먼저 걸러내도록 수정.
둘 다 원래는 "잘못된 이미지 파일입니다"(400/404)로 처리됐어야 할
케이스가 서버 에러로 새어나가고 있었음.

## 별도로 확인만 하고 고치지 않은 것
S3Service.deleteFile()의 `imageUrl.substring(49)`는 폴더명/버킷명
길이에 따라 늘 달라지는 값이라 하드코딩된 49는 구조적으로 항상
맞을 수 없음. 다만 이 메서드를 실제로 호출하는 곳이 코드베이스
전체에 없어(dead code) 지금 당장 영향은 없음. 실제 URL 포맷을
확인 없이 숫자만 바꾸는 건 오히려 위험해서, 이번엔 고치지 않고
플래그만 해둠 — deleteFile을 실제로 쓰기 전에 반드시 점검 필요.
@unam98 unam98 self-assigned this Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@unam98, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 39 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad587a10-a267-4975-96ce-5759fb760f52

📥 Commits

Reviewing files that changed from the base of the PR and between 75b8524 and 3a2b8f2.

📒 Files selected for processing (3)
  • src/main/java/org/runnect/server/external/aws/S3Service.java
  • src/test/java/org/runnect/server/banner/service/BannerServiceTest.java
  • src/test/java/org/runnect/server/external/aws/S3ServiceTest.java

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

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

@unam98
unam98 merged commit c75f765 into dev Aug 5, 2026
2 checks passed
@unam98
unam98 deleted the tests/banner-s3-service-unit-tests branch August 5, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants