[Refactor] 사진 업로드 병렬화 및 테스트 인프라 정비#31
Merged
Merged
Conversation
업로드 경로 - PhotoRepository.uploadPhotos를 TaskGroup으로 변경하여 S3 업로드 병렬화 - 강제 언래핑(URL(string:)!)과 try? silent skip 제거 → 한 건 실패 시 전체 throw로 표면화 - PhotoUploadItem.imageUrl 타입을 String에서 URL로 변경 - PhotoMetadataExtractor의 임시 파일 쓰기 실패를 명시적 nil 반환으로 처리 테스트 - PhotoRepository 단위 테스트 3개 추가 (빈 입력, 전체 성공, 일부 실패) - StubURLProtocol, MockTokenStore 등 재사용 가능한 테스트 인프라 추가 - 기존 테스트의 깨진 DTO 참조 정리 (TagRequestDTO → UpdateTagRequestDTO, SearchResponseDto → SearchResponseDTO 등) - MockDataFactory를 현재 DTO 스키마에 맞춰 갱신 (isSensitive 필드, ISO8601 타임존)
|
Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughPhotoRepository의 uploadPhotos를 withThrowingTaskGroup로 병렬화하고, PhotoUploadItem.imageUrl을 URL로 변경했으며, 임시 파일 쓰기 실패 처리를 명시적으로 바꾸고, 테스트 헬퍼(StubURLProtocol 등)와 통합 테스트·성능 테스트 참조를 정정했습니다. 변경사항사진 업로드 병렬화 및 테스트 인프라
Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 `@Rephoto_iOS/Features/Home/Data/Repositories/PhotoRepository.swift`:
- Around line 33-40: The child tasks in withThrowingTaskGroup capture
self.adapter which must be Sendable; either make MoyaNetworkAdapter conform to
Sendable (or annotate as `@unchecked` Sendable if safe) so adapter can be captured
(update the MoyaNetworkAdapter type declaration), or avoid capturing adapter by
extracting the needed request operation out of the task (e.g., create a local,
Sendable-safe request function or copy any required data before group.addTask)
and call that inside the `@Sendable` closure; update references in PhotoRepository
where adapter is used inside the group (adapter, withThrowingTaskGroup, addTask)
accordingly.
In `@Rephoto_iOSTests/MemoryPerformanceTests.swift`:
- Around line 38-39: The test currently swallows decode failures with `try?`
which can make the memory benchmark measure a nil path; replace the `try?
JSONDecoder().decode(SearchResponseDTO.self, from: data)` usage in this test
(the `retainedSearchResult` assignment) with an explicit do-catch around
`JSONDecoder().decode`, on success assign to `retainedSearchResult`, and on
failure call `XCTFail` with the decoding error so the test fails instead of
measuring a no-op path (keep the subsequent
`XCTAssertEqual(retainedSearchResult?.searchResults.count, 500)` intact).
🪄 Autofix (Beta)
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
Run ID: 69664a4f-6be0-4433-a67b-51af38b3d7c0
📒 Files selected for processing (9)
Rephoto_iOS/Features/Home/Data/Repositories/PhotoRepository.swiftRephoto_iOS/Features/Home/Domain/Models/PhotoUploadItem.swiftRephoto_iOS/Features/Home/Domain/Services/PhotoMetadataExtractor.swiftRephoto_iOSTests/DecodingPerformanceTests.swiftRephoto_iOSTests/MemoryPerformanceTests.swiftRephoto_iOSTests/MockDataFactory.swiftRephoto_iOSTests/PhotoInfoPerformanceTests.swiftRephoto_iOSTests/PhotoRepositoryTests.swiftRephoto_iOSTests/TestHelpers.swift
- 디코딩 실패가 silent로 nil 경로를 측정하던 문제 수정 - 실패 시 XCTFail로 명시 → 다른 성능 테스트와 패턴 통일
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.
✨ PR 유형
어떤 변경 사항이 있나요??
🛠️ 작업내용
업로드 경로 개선
PhotoRepository.uploadPhotos를 순차for루프에서withThrowingTaskGroup으로 변경하여 S3 업로드 병렬화URL(string: item.imageUrl)!강제 언래핑 제거 —PhotoUploadItem.imageUrl타입을String에서URL로 변경하여 타입 시스템 차원에서 보장try? Data(contentsOf:)silent skip 제거 — 한 건이라도 실패하면 TaskGroup이 자동 cancel 후 throw, 사용자에게 정상적으로 노출됨PhotoMetadataExtractor의 임시 파일 쓰기 실패를try?→ 명시적do/catch + return nil로 처리테스트 추가
PhotoRepositoryTests— 3개 케이스test_uploadPhotos_emptyItems_makesNoRequeststest_uploadPhotos_allSuccess_uploadsEachItemThenCallsBatchSavetest_uploadPhotos_anyS3Failure_throwsAndSkipsBatchSaveTestHelpers— 재사용 가능한 인프라StubURLProtocol—URLSession응답 stub + 요청 기록MockTokenStore— 메모리 기반TokenStoreactorMockTokenRefreshService기존 테스트 빌드 복구
PhotoInfoPerformanceTests— 존재하지 않는TagRequestDTO→UpdateTagRequestDTODecodingPerformanceTests— album 테스트의[SearchResponseDTO]→[AlbumResponseDTO]MemoryPerformanceTests—SearchResponseDto→SearchResponseDTOMockDataFactory— 옛 필드명private→isSensitive,createdAt에 ISO8601 타임존(Z) 추가📋 추후 진행 상황
📌 리뷰 포인트
PhotoRepository.uploadPhotos의 TaskGroup 내부에서self.adapter를 로컬로 캡처한 부분, 그리고JSONDecoder()를 각 task 내에서 새로 만든 점이 의도된 선택인지 확인 부탁드립니다 (Sendable 안전성 우선)PhotoUploadItem.imageUrl타입 변경의 파급은PhotoMetadataExtractor와PhotoRepository두 호출처에 한정됩니다StubURLProtocol은 다른 Repository 테스트에서도 그대로 재사용 가능하도록 작성했습니다✅ Checklist
PR이 다음 요구 사항을 충족하는지 확인해주세요!!!
Closes #30
Summary by CodeRabbit
새로운 기능
버그 수정
테스트