refactor: UploadFileUseCase 리팩토링#373
Merged
hgjwilly-koreatech merged 1 commit intodevelopfrom Mar 14, 2026
Merged
Conversation
Core 도메인으로 분리
📝 WalkthroughWalkthroughThis PR migrates file upload functionality from the Shop domain to the Core domain by: (1) removing uploadFiles methods from ShopService/ShopRepository/ShopAPI; (2) adding uploadFiles methods to CoreService/CoreRepository/CoreAPI with domain parameter support; (3) introducing UploadDomain enum for domain categorization; (4) creating UploadFileUseCase in Core domain; (5) updating presentation layer to use core-based uploads with appropriate domains. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Koin/Presentation/LostItem/EditLostItem/EditLostItemViewModel.swift (1)
32-32: Same DI consideration asPostLostItemViewModel.Inline instantiation reduces testability. If addressing this, consider making
uploadFileUseCaseinjectable via the initializer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/LostItem/EditLostItem/EditLostItemViewModel.swift` at line 32, The field uploadFileUseCase is being instantiated inline which harms testability; make it an injectable dependency by adding an initializer parameter (e.g., uploadFileUseCase: UploadFileUseCase) to EditLostItemViewModel and assign it to the uploadFileUseCase property instead of creating DefaultUploadFileUseCase(DefaultCoreRepository(DefaultCoreService())) inside the class; optionally provide a default parameter value that constructs DefaultUploadFileUseCase(...) to preserve existing call sites while enabling tests to inject a mock implementation.Koin/Data/Service/Network/API/CoreAPI.swift (1)
42-49: Consider removing explicitContent-Typeheader for multipart uploads.When using Alamofire's
AF.upload(multipartFormData:...), the framework automatically sets theContent-Typeheader with the correct boundary parameter. Manually settingContent-Type: multipart/form-datawithout the boundary may cause server-side parsing issues.♻️ Suggested fix
public var headers: [String: String] { - var baseHeaders: [String: String] = [:] switch self { - case .checkVersion, .fetchBanner, .fetchClubCategories, .fetchHotClubs: return [:] - case .uploadFiles: - baseHeaders["Content-Type"] = "multipart/form-data" + case .checkVersion, .fetchBanner, .fetchClubCategories, .fetchHotClubs, .uploadFiles: + return [:] } - return baseHeaders }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Data/Service/Network/API/CoreAPI.swift` around lines 42 - 49, The headers getter currently sets "Content-Type" for the .uploadFiles case which can interfere with AF.upload's boundary handling; remove the manual assignment in the public var headers (specifically the .uploadFiles branch) so it returns baseHeaders (empty or only other required headers) and allow Alamofire's AF.upload(multipartFormData:...) to set the Content-Type with boundary automatically; update any tests or callers that assumed a preset Content-Type in headers for uploadFiles.Koin/Presentation/LostItem/PostLostItem/PostLostItemViewModel.swift (1)
35-35: Consider dependency injection for better testability.The inline instantiation of
DefaultUploadFileUseCase(coreRepository: DefaultCoreRepository(service: DefaultCoreService()))makes this view model harder to test. Consider injecting the use case through the initializer, similar to howupdateLostItemUseCaseis handled inEditLostItemViewModel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Presentation/LostItem/PostLostItem/PostLostItemViewModel.swift` at line 35, The inline creation of uploadFileUseCase using DefaultUploadFileUseCase(DefaultCoreRepository(DefaultCoreService())) hinders testing; change PostLostItemViewModel to accept an UploadFileUseCase via its initializer (similar to how updateLostItemUseCase is injected in EditLostItemViewModel), add a stored property for uploadFileUseCase, and provide a default initializer value that constructs DefaultUploadFileUseCase(coreRepository: DefaultCoreRepository(service: DefaultCoreService())) so existing callers are unaffected while tests can inject mocks.Koin/Domain/UseCase/Core/UploadFileUseCase.swift (1)
23-25: Prefer keepingUploadDomaintyped beyond use case boundary.Line 24 converts to
Stringearly, which makes downstream contracts stringly-typed. Consider passingUploadDomainthrough repository/service and mapping torawValueonly inCoreAPIrequest construction.♻️ Suggested direction
- func uploadFiles(files: [Data], domain: String) -> AnyPublisher<FileUploadResponse, ErrorResponse> + func uploadFiles(files: [Data], domain: UploadDomain) -> AnyPublisher<FileUploadResponse, ErrorResponse>- return coreRepository.uploadFiles(files: files, domain: domain.rawValue) + return coreRepository.uploadFiles(files: files, domain: domain)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Koin/Domain/UseCase/Core/UploadFileUseCase.swift` around lines 23 - 25, The execute method is converting UploadDomain to String too early; update the use case and downstream API to keep UploadDomain typed until the API layer by changing the signature of UploadFileUseCase.execute(files:domain:) to accept UploadDomain (not String) and adjust coreRepository.uploadFiles to accept an UploadDomain parameter (or overload it), then perform domain.rawValue only inside the CoreAPI request construction (e.g., in the class/method that builds the HTTP request), leaving execute(files:domain:), UploadDomain, and coreRepository.uploadFiles with the strong-typed enum across the use-case boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Koin/Presentation/Chat/Chat/ChatViewModel.swift`:
- Around line 77-79: ChatViewModel currently hardcodes the upload domain to
.lostItem in uploadFiles, causing uploads from NoticeDataViewController and
ChatListTableViewController to hit the wrong backend; update ChatViewModel to
accept a domain parameter (similar to articleId/chatRoomId) or compute domain
from the originating context and use that when calling
uploadFileUseCase.execute(files:domain:), ensuring uploadFiles (and any
callers/initializers) pass the correct domain instead of .lostItem.
---
Nitpick comments:
In `@Koin/Data/Service/Network/API/CoreAPI.swift`:
- Around line 42-49: The headers getter currently sets "Content-Type" for the
.uploadFiles case which can interfere with AF.upload's boundary handling; remove
the manual assignment in the public var headers (specifically the .uploadFiles
branch) so it returns baseHeaders (empty or only other required headers) and
allow Alamofire's AF.upload(multipartFormData:...) to set the Content-Type with
boundary automatically; update any tests or callers that assumed a preset
Content-Type in headers for uploadFiles.
In `@Koin/Domain/UseCase/Core/UploadFileUseCase.swift`:
- Around line 23-25: The execute method is converting UploadDomain to String too
early; update the use case and downstream API to keep UploadDomain typed until
the API layer by changing the signature of
UploadFileUseCase.execute(files:domain:) to accept UploadDomain (not String) and
adjust coreRepository.uploadFiles to accept an UploadDomain parameter (or
overload it), then perform domain.rawValue only inside the CoreAPI request
construction (e.g., in the class/method that builds the HTTP request), leaving
execute(files:domain:), UploadDomain, and coreRepository.uploadFiles with the
strong-typed enum across the use-case boundary.
In `@Koin/Presentation/LostItem/EditLostItem/EditLostItemViewModel.swift`:
- Line 32: The field uploadFileUseCase is being instantiated inline which harms
testability; make it an injectable dependency by adding an initializer parameter
(e.g., uploadFileUseCase: UploadFileUseCase) to EditLostItemViewModel and assign
it to the uploadFileUseCase property instead of creating
DefaultUploadFileUseCase(DefaultCoreRepository(DefaultCoreService())) inside the
class; optionally provide a default parameter value that constructs
DefaultUploadFileUseCase(...) to preserve existing call sites while enabling
tests to inject a mock implementation.
In `@Koin/Presentation/LostItem/PostLostItem/PostLostItemViewModel.swift`:
- Line 35: The inline creation of uploadFileUseCase using
DefaultUploadFileUseCase(DefaultCoreRepository(DefaultCoreService())) hinders
testing; change PostLostItemViewModel to accept an UploadFileUseCase via its
initializer (similar to how updateLostItemUseCase is injected in
EditLostItemViewModel), add a stored property for uploadFileUseCase, and provide
a default initializer value that constructs
DefaultUploadFileUseCase(coreRepository: DefaultCoreRepository(service:
DefaultCoreService())) so existing callers are unaffected while tests can inject
mocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a966ad0d-1af4-48d0-b3f7-ef92caf1145f
📒 Files selected for processing (18)
Koin/Data/Repository/DefaultCoreRepository.swiftKoin/Data/Repository/DefaultShopRepository.swiftKoin/Data/Service/CoreService.swiftKoin/Data/Service/Network/API/CoreAPI.swiftKoin/Data/Service/Network/API/ShopAPI.swiftKoin/Data/Service/Network/NetworkService.swiftKoin/Data/Service/ShopService.swiftKoin/Domain/Model/UploadDomain.swiftKoin/Domain/Repository/CoreRepository.swiftKoin/Domain/Repository/ShopRepository.swiftKoin/Domain/UseCase/Core/UploadFileUseCase.swiftKoin/Domain/UseCase/Shop/UploadFileUseCase.swiftKoin/Presentation/Chat/Chat/ChatViewModel.swiftKoin/Presentation/LostItem/EditLostItem/EditLostItemViewModel.swiftKoin/Presentation/LostItem/PostLostItem/PostLostItemViewModel.swiftKoin/Presentation/Shop/ShopReview/ShopReviewViewModel.swiftKoin/Presentation/Shop/ShopReviewList/ReviewListViewController.swiftkoin.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (3)
- Koin/Domain/Repository/ShopRepository.swift
- Koin/Data/Repository/DefaultShopRepository.swift
- Koin/Domain/UseCase/Shop/UploadFileUseCase.swift
Comment on lines
77
to
79
| private func uploadFiles(files: [Data]) { | ||
| uploadFileUseCase.execute(files: files).sink { [weak self] completion in | ||
| uploadFileUseCase.execute(files: files, domain: .lostItem).sink { [weak self] completion in | ||
| if case let .failure(error) = completion { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== UploadDomain enum values =="
fd 'UploadDomain.swift$' -x sed -n '1,140p' {}
echo
echo "== All execute(files:domain:) call sites =="
rg -n --type=swift -C2 'uploadFileUseCase\.execute\(files:\s*[^,]+,\s*domain:\s*\.[A-Za-z]+' Koin
echo
echo "== Chat-related viewmodels using upload =="
rg -n --type=swift -C3 'final class .*ChatViewModel|case uploadFile\(\[Data\]\)|sendMessage\(message:.*isImage' KoinRepository: BCSDLab/KOIN_iOS
Length of output: 8132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChatViewModel instantiation points =="
rg -n --type=swift 'ChatViewModel\(' Koin | head -20
echo
echo "== Views/ViewControllers that use ChatViewModel =="
rg -n --type=swift '@StateObject.*ChatViewModel|@ObservedObject.*ChatViewModel|viewModel.*ChatViewModel|ChatViewModel()' Koin | head -20
echo
echo "== Chat module structure =="
fd 'Chat' Koin/Presentation -type d
echo
echo "== CallVan chat domain usage =="
rg -n --type=swift 'callVanChat' Koin
echo
echo "== Full ChatViewModel input/transform to see domain parameter flow =="
sed -n '1,130p' Koin/Presentation/Chat/Chat/ChatViewModel.swiftRepository: BCSDLab/KOIN_iOS
Length of output: 2195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChatViewModel init signature and full source =="
sed -n '1,50p' Koin/Presentation/Chat/Chat/ChatViewModel.swift
echo
echo "== LostItemDataViewController ChatViewModel instantiation context =="
sed -n '270,280p' Koin/Presentation/LostItem/LostItemData/LostItemDataViewController.swift
echo
echo "== NoticeDataViewController ChatViewModel instantiation context =="
sed -n '285,295p' Koin/Presentation/Notice/NoticeData/NoticeDataViewController.swift
echo
echo "== Check if any viewModel receives domain parameter =="
rg -n --type=swift 'domain.*:' Koin/Presentation/Chat/Chat/ChatViewModel.swift
echo
echo "== Look for article/request types to infer domain =="
rg -n --type=swift 'articleId|chatRoomId' Koin/Presentation/Chat/Chat/ChatViewModel.swift | head -10Repository: BCSDLab/KOIN_iOS
Length of output: 4082
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== callVanChat and callVanReport domain usage =="
rg -n --type=swift 'callVanChat|callVanReport' Koin
echo
echo "== Backend domain path construction (if exposed in client) =="
rg -n --type=swift 'domain.*upload|upload.*domain' Koin | head -10
echo
echo "== UploadFileUseCase to understand domain usage =="
fd 'UploadFileUseCase.swift' -o 'DefaultUploadFileUseCase.swift' -x cat {}Repository: BCSDLab/KOIN_iOS
Length of output: 2019
ChatViewModel hardcodes .lostItem upload domain for all chat contexts.
Line 78 always routes uploads to /LOST_ITEMS/upload/files regardless of origin. ChatViewModel is instantiated from three distinct features:
- LostItemDataViewController (lost item context — correct)
- NoticeDataViewController (notice context — incorrect)
- ChatListTableViewController (generic chat context — incorrect)
Uploads from Notice and ChatList contexts will be routed to the wrong backend domain. Consider passing domain as an init parameter (similar to how articleId and chatRoomId are passed) or determining it from context at upload time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Koin/Presentation/Chat/Chat/ChatViewModel.swift` around lines 77 - 79,
ChatViewModel currently hardcodes the upload domain to .lostItem in uploadFiles,
causing uploads from NoticeDataViewController and ChatListTableViewController to
hit the wrong backend; update ChatViewModel to accept a domain parameter
(similar to articleId/chatRoomId) or compute domain from the originating context
and use that when calling uploadFileUseCase.execute(files:domain:), ensuring
uploadFiles (and any callers/initializers) pass the correct domain instead of
.lostItem.
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.
#️⃣연관된 이슈
📝작업 내용
기존 UploadFileUseCase는 Shop 도메인에 결합되어, domain이 항상 SHOP 입니다.
Core 도메인으로 변경하여, 사용하는 도메인마다 SHOP, LOST-ITEM 등을 domain 값으로 전달하도록 변경합니다.
스크린샷 (선택)
💬리뷰 요구사항(선택)
Summary by CodeRabbit