Conversation
Walkthrough이번 변경에서는 로컬 파일 관리 기능을 제공하는 FileManagerService 모듈이 새롭게 추가되고, 이를 활용하여 버스정류장 JSON 데이터의 버전 관리 및 다운로드 기능이 구현되었습니다. 또한, 관련 의존성 주입 및 서비스 등록, 엔티티/리포지토리/유스케이스 계층이 모두 확장되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant AppCoordinator
participant UpdateBusStationListUseCase
participant BusStationVersionRepository
participant GithubFileDownloadRepository
participant FileManagerService
AppCoordinator->>UpdateBusStationListUseCase: execute()
UpdateBusStationListUseCase->>BusStationVersionRepository: fetchRemoteVersion()
BusStationVersionRepository-->>UpdateBusStationListUseCase: remoteVersion
UpdateBusStationListUseCase->>BusStationVersionRepository: fetchLocalVersion()
BusStationVersionRepository-->>UpdateBusStationListUseCase: localVersion
alt localVersion != remoteVersion
UpdateBusStationListUseCase->>GithubFileDownloadRepository: downloadFile(...)
GithubFileDownloadRepository->>FileManagerService: save(data, directory, fileName)
FileManagerService-->>GithubFileDownloadRepository: 완료
GithubFileDownloadRepository-->>UpdateBusStationListUseCase: 완료
UpdateBusStationListUseCase->>BusStationVersionRepository: save(version)
else localVersion == remoteVersion
UpdateBusStationListUseCase-->>AppCoordinator: 데이터 최신 상태 알림
end
UpdateBusStationListUseCase-->>AppCoordinator: 완료/에러 알림
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
Projects/FileManagerService/Sources/FileManagerService.swift (1)
23-26: 메서드명을 더 자연스럽게 개선해보세요.
isExist메서드명이 문법적으로 부자연스럽습니다.exists로 변경하는 것을 고려해보세요.- func isExist( + func exists( directoryName: String, fileName: String ) -> Observable<Bool>Projects/App/Sources/Coordinator/AppCoordinator.swift (1)
54-54: 의존성 주입 패턴을 일관성 있게 사용해주세요.메서드 내부에서
@Injected를 사용하는 것은 일반적이지 않은 패턴입니다. 보통은 프로퍼티 레벨에서 주입받는 것이 권장됩니다.다음과 같이 프로퍼티 레벨에서 주입받는 것을 고려해보세요:
final class AppCoordinator: Coordinator { // ... 기존 프로퍼티들 ... + @Injected private var updateBusStationListUseCase: UpdateBusStationListUseCase private let disposeBag = DisposeBag() // ... private func checkAndDownloadBusStationList() { - @Injected var useCase: UpdateBusStationListUseCase - - useCase.execute() + updateBusStationListUseCase.execute()Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift (1)
27-30: 더 구체적인 에러 타입 사용을 권장합니다.weak self 패턴은 올바르게 적용되었지만,
RxError.unknown대신 더 구체적인 에러 타입을 사용하는 것이 디버깅에 도움이 됩니다.- return .error(RxError.unknown) // Or a custom error + return .error(FileManagerServiceError.unknown)Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift (1)
18-21: 하드코딩된 값들을 상수로 분리하는 것을 권장합니다.리포지토리 이름과 파일 경로가 하드코딩되어 있어 유지보수성과 테스트 가능성이 떨어집니다.
+ private enum Constants { + static let repositoryName = "BusStationData" + static let versionFileName = "bus_station_version.json" + } + public func fetchRemoteVersion() -> Observable<BusStationVersion> { let endPoint = GithubFileDownloadEndPoint( - repo: "BusStationData", - filePath: "bus_station_version.json" + repo: Constants.repositoryName, + filePath: Constants.versionFileName )Projects/NetworkService/Sources/EndPoint/GithubFileDownloadEndPoint.swift (1)
5-6: 하드코딩된 값들을 설정 가능하게 만드는 것을 고려해보세요.
owner와branch가 하드코딩되어 있어 다른 브랜치나 organization을 사용할 때 유연성이 떨어집니다. 향후 확장성을 위해 이들을 초기화 파라미터로 받는 것을 고려해보세요.public struct GithubFileDownloadEndPoint: EndPoint { - private let owner = "Pepsi-Club" - private let branch = "main" + private let owner: String + private let branch: String private let repo: String private let filePath: String - public init(repo: String, filePath: String) { + public init(owner: String = "Pepsi-Club", repo: String, filePath: String, branch: String = "main") { + self.owner = owner self.repo = repo self.filePath = filePath + self.branch = branch }Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift (2)
22-23: 메모리 안전성을 위한 weak self 사용이 좋습니다.RxError.unknown 대신 더 구체적인 에러 타입 사용을 고려해보세요.
-guard let self = self else { return .error(RxError.unknown) } +guard let self = self else { + return .error(UpdateBusStationListUseCaseError.selfDeallocated) +}
52-59: 클로저에서 self 캡처 확인 필요.
do오퍼레이터의onNext클로저에서self를 강하게 참조하고 있습니다. 메모리 누수 가능성을 검토해보세요.-.do(onNext: { _ in - - self.versionRepository.save(version: newVersion) - print("🚏 로컬에 최신 버스정류장 정보 저장") -}, onError: { error in +.do(onNext: { [weak self] _ in + self?.versionRepository.save(version: newVersion) + print("🚏 로컬에 최신 버스정류장 정보 저장") +}, onError: { error inProjects/FileManagerService/Sources/DefaultFileManagerService.swift (1)
30-30: 프로덕션 코드에서 print 문 제거 고려.파일 경로를 출력하는 print 문이 있습니다. 디버깅 목적이라면 로깅 시스템 사용을 고려해보세요.
-print(fileURL.path) +// Debug: 개발 중에만 활성화하거나 로깅 시스템 사용 고려 +#if DEBUG +print("📁 파일 저장됨: \(fileURL.path)") +#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Projects/App/Project.swift(1 hunks)Projects/App/Sources/AppDelegate+Register.swift(2 hunks)Projects/App/Sources/Coordinator/AppCoordinator.swift(2 hunks)Projects/Core/Sources/Extension/String+.swift(1 hunks)Projects/Data/Project.swift(1 hunks)Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift(1 hunks)Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift(1 hunks)Projects/Domain/Sources/Entity/BusStationVersion.swift(1 hunks)Projects/Domain/Sources/RepositoryInterface/BusStationVersionRepository.swift(1 hunks)Projects/Domain/Sources/RepositoryInterface/GithubFileDownloadRepository.swift(1 hunks)Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift(1 hunks)Projects/FileManagerService/Project.swift(1 hunks)Projects/FileManagerService/Sources/DefaultFileManagerService.swift(1 hunks)Projects/FileManagerService/Sources/FileManagerService.swift(1 hunks)Projects/FileManagerService/Sources/FileManagerServiceError.swift(1 hunks)Projects/NetworkService/Sources/EndPoint/GithubFileDownloadEndPoint.swift(1 hunks)Tuist/ProjectDescriptionHelpers/InfoPlist/SecretInfoPlist.swift(1 hunks)Tuist/ProjectDescriptionHelpers/Module/Local/FileManagerService.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
Projects/App/Sources/Coordinator/AppCoordinator.swift (1)
Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift (1)
execute(15-42)
Projects/Domain/Sources/RepositoryInterface/BusStationVersionRepository.swift (2)
Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift (3)
fetchRemoteVersion(17-24)fetchLocalVersion(26-28)save(30-32)Projects/FileManagerService/Sources/DefaultFileManagerService.swift (1)
save(16-38)
Projects/App/Sources/AppDelegate+Register.swift (6)
Projects/Core/Sources/DIContainer/DIContainer.swift (2)
setLogger(17-19)register(21-28)Projects/Feature/NearMapFeature/Demo/AppDelegate+Resister.swift (1)
register(16-24)Projects/Feature/BusStopFeature/Demo/AppDelegate.swift (1)
register(26-35)Projects/Feature/AlarmFeature/Demo/AppDelegate.swift (1)
register(27-41)Projects/Feature/SearchFeature/Demo/AppDelegate.swift (1)
register(25-29)Projects/Feature/HomeFeature/Demo/AppDelegate.swift (1)
register(25-33)
Projects/Domain/Sources/RepositoryInterface/GithubFileDownloadRepository.swift (1)
Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift (1)
downloadFile(16-37)
Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift (1)
Projects/FileManagerService/Sources/DefaultFileManagerService.swift (1)
save(16-38)
Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift (2)
Projects/Core/Sources/Extension/Data+.swift (1)
decode(18-20)Projects/FileManagerService/Sources/DefaultFileManagerService.swift (1)
save(16-38)
Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift (3)
Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift (3)
fetchRemoteVersion(17-24)fetchLocalVersion(26-28)save(30-32)Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift (1)
downloadFile(16-37)Projects/FileManagerService/Sources/DefaultFileManagerService.swift (1)
save(16-38)
⏰ 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_test
🔇 Additional comments (22)
Projects/FileManagerService/Sources/FileManagerService.swift (1)
5-27: 잘 설계된 리액티브 파일 관리 프로토콜입니다.RxSwift Observable을 활용한 비동기 파일 관리 인터페이스가 잘 정의되어 있습니다. 메서드 시그니처가 일관성 있고 명확하며, 파라미터 구조도 적절합니다.
Tuist/ProjectDescriptionHelpers/InfoPlist/SecretInfoPlist.swift (1)
18-18: GitHub 액세스 토큰 설정이 적절합니다.기존 시크릿 관리 패턴과 일관성 있게 환경 변수 치환 방식을 사용하여 GitHub 액세스 토큰을 안전하게 관리하고 있습니다.
Projects/Data/Project.swift (1)
10-10: Data 레이어에 FileManagerService 추가가 적절합니다.FileManagerService를 Data 프로젝트 종속성에 추가한 것이 아키텍처 관점에서 적절하며, 기존 서비스들과 일관성 있는 패턴을 따르고 있습니다.
Projects/App/Project.swift (1)
9-9: App 프로젝트에 FileManagerService 추가가 적절합니다.앱 레벨에서 FileManagerService를 포함시킨 것이 적절하며, 기존 모듈 구조와 일관성을 유지하고 있습니다.
Projects/FileManagerService/Project.swift (1)
4-9: FileManagerService 프로젝트 설정이 잘 구성되었습니다.새로운 FileManagerService 프레임워크의 프로젝트 설정이 표준 패턴을 따르고 있으며, Domain 종속성과 프레임워크 설정이 적절하게 구성되어 있습니다.
Projects/Domain/Sources/Entity/BusStationVersion.swift (1)
3-5: 엔티티 구조가 명확하고 목적에 적합합니다.
BusStationVersion구조체가 간단하고 명확하게 정의되어 있습니다.Decodable프로토콜을 준수하여 JSON 파싱에 적합하며, 단일 책임 원칙을 잘 따르고 있습니다.Projects/Domain/Sources/RepositoryInterface/GithubFileDownloadRepository.swift (1)
5-12: 잘 설계된 프로토콜입니다.
GithubFileDownloadRepository프로토콜이 단일 책임 원칙을 잘 따르고 있으며, 메서드 시그니처가 GitHub 파일 다운로드에 필요한 모든 파라미터를 포함하고 있습니다.Observable<Void>반환 타입은 비동기 파일 다운로드 작업에 적합합니다.Projects/App/Sources/Coordinator/AppCoordinator.swift (2)
31-31: 앱 시작 시 버스정류장 업데이트 확인이 적절하게 배치되었습니다.
TabBarCoordinator시작 전에 버스정류장 데이터 업데이트를 확인하는 것은 사용자 경험 측면에서 좋은 선택입니다.
57-65: 에러 처리 방식이 적절합니다.버스정류장 업데이트는 앱의 핵심 기능이 아니므로, 실패 시 로깅만 하고 앱 실행을 계속하는 것이 합리적입니다. 사용자 경험을 해치지 않으면서도 문제를 추적할 수 있습니다.
Projects/Domain/Sources/RepositoryInterface/BusStationVersionRepository.swift (1)
5-14: 버전 관리를 위한 프로토콜이 잘 설계되었습니다.
BusStationVersionRepository프로토콜이 버전 관리의 전체 라이프사이클을 명확하게 정의하고 있습니다:
- 원격/로컬 버전 조회의 명확한 분리
- 적절한 반환 타입 (네트워크:
Observable, 로컬: 동기)- 한국어 문서화로 가독성 향상
각 메서드의 책임이 명확하고 일관성 있게 정의되어 있습니다.
Projects/FileManagerService/Sources/FileManagerServiceError.swift (1)
3-18: 잘 설계된 에러 열거형입니다.파일 관리 작업의 모든 주요 실패 시나리오를 포괄하고, 각 케이스에 대해 적절한 연관 값을 제공하여 상세한 에러 컨텍스트를 전달할 수 있습니다. 한국어 주석도 코드의 가독성을 높여줍니다.
Projects/App/Sources/AppDelegate+Register.swift (4)
17-17: 새로운 모듈 의존성이 올바르게 추가되었습니다.FileManagerService 모듈 import가 적절히 추가되어 새로운 서비스 등록을 지원합니다.
24-32: 의존성 등록이 잘 구조화되었습니다.MARK 주석을 사용하여 Service 섹션을 명확히 구분하고, 새로운 FileManagerService와 기존 서비스들을 논리적으로 그룹화했습니다. 등록 패턴도 일관성 있게 적용되었습니다.
33-41: 새로운 Repository 등록이 적절합니다.BusStationVersionRepository와 GithubFileDownloadRepository가 올바른 위치에 등록되었으며, 기존 리포지토리들과 일관된 패턴을 따릅니다.
42-52: UseCase 섹션 구성이 개선되었습니다.새로운 UpdateBusStationListUseCase가 추가되었고, 기존 유스케이스들과 함께 명확히 그룹화되었습니다. FirebaseLogger 등록 위치는 재고가 필요할 수 있지만 기능적으로는 문제없습니다.
Tuist/ProjectDescriptionHelpers/Module/Local/FileManagerService.swift (1)
2-16: 표준적인 Tuist 프레임워크 타겟 정의입니다.FrameworkTarget 프로토콜을 올바르게 준수하고, TargetComponentBuilder 패턴을 적절히 활용하여 의존성과 InfoPlist를 관리합니다. Tuist 프로젝트 설정을 위한 표준 구조를 잘 따르고 있습니다.
Projects/Data/Sources/Repository/DefaultGithubFileDownloadRepository.swift (2)
10-14: 의존성 주입 패턴이 올바르게 적용되었습니다.@injected 프로퍼티 래퍼를 사용하여 NetworkService와 FileManagerService를 깔끔하게 주입받고 있으며, public init()도 적절합니다.
16-37: 반응형 파일 다운로드 구현이 잘 되어 있습니다.네트워크 요청과 파일 저장을 flatMap으로 연결하는 반응형 패턴이 적절히 적용되었고, 전체적인 흐름이 명확합니다. GithubFileDownloadEndPoint 구성도 올바릅니다.
Projects/Data/Sources/Repository/DefaultBusStationVersionRepository.swift (3)
12-13: UserDefaults 기반 로컬 버전 관리가 적절합니다.@UserDefaultsWrapper를 사용하여 로컬 버전을 깔끔하게 관리하고 있습니다. 키 이름도 명확합니다.
22-24: RxSwift decode 메서드 사용이 적절합니다.네트워크 요청 결과를 BusStationVersion 타입으로 직접 디코딩하는 반응형 패턴이 잘 적용되었습니다.
26-32: 로컬 버전 관리 메서드들이 간결하고 명확합니다.fetchLocalVersion()과 save(version:) 메서드 모두 단순하고 명확한 구현으로 UserDefaults 래퍼를 잘 활용하고 있습니다.
Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift (1)
5-7: UseCase 프로토콜 정의가 깔끔합니다.단일 책임 원칙을 잘 따르고 있으며, Observable을 반환하는 비동기 인터페이스가 적절합니다.
| static var githubAccessToken: Self { | ||
| guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"), | ||
| let githubAccessToken = any as? String | ||
| else { | ||
| return "" | ||
| } | ||
| return githubAccessToken | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 방식의 일관성을 개선해주세요.
githubAccessToken은 빈 문자열을 반환하는 반면, serverKey는 fatalError를 발생시킵니다. GitHub 토큰이 없으면 API 인증이 실패할 가능성이 높으므로, 다음 중 하나를 고려해보세요:
serverKey와 같이 필수값으로 처리하여fatalError사용- Optional 타입으로 변경하여 호출부에서 적절히 처리
현재 구현에서는 토큰이 없어도 네트워크 요청이 시도되어 인증 실패가 발생할 수 있습니다:
- static var githubAccessToken: Self {
- guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"),
- let githubAccessToken = any as? String
- else {
- return ""
- }
- return githubAccessToken
- }
+ static var githubAccessToken: Self {
+ guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"),
+ let githubAccessToken = any as? String
+ else { fatalError("GitHub Access Token이 설정되지 않았습니다") }
+ return githubAccessToken
+ }📝 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.
| static var githubAccessToken: Self { | |
| guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"), | |
| let githubAccessToken = any as? String | |
| else { | |
| return "" | |
| } | |
| return githubAccessToken | |
| } | |
| static var githubAccessToken: Self { | |
| guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"), | |
| let githubAccessToken = any as? String | |
| else { fatalError("GitHub Access Token이 설정되지 않았습니다") } | |
| return githubAccessToken | |
| } |
🤖 Prompt for AI Agents
In Projects/Core/Sources/Extension/String+.swift around lines 37 to 44, the
githubAccessToken property returns an empty string when the token is missing,
unlike serverKey which triggers a fatalError. To improve error handling
consistency, either change githubAccessToken to also use fatalError when the
token is missing, treating it as a required value, or modify its return type to
Optional<String> so callers can handle the absence of the token explicitly. This
prevents silent failures during API authentication.
| public var header: [String: String] { | ||
| ["Authorization": "token \(String.githubAccessToken)"] | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
GitHub 토큰의 안전한 사용을 검증해주세요.
Authorization 헤더에서 GitHub 토큰을 사용하고 있습니다. 토큰이 안전하게 관리되고 있는지, 그리고 필요한 권한만 부여되었는지 확인이 필요합니다.
다음 스크립트로 GitHub 토큰 설정을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# GitHub 토큰 설정 확인
echo "GitHub 토큰 관련 설정 확인..."
# String extension에서 토큰 구현 확인
fd "String+" --extension swift --exec grep -l "githubAccessToken" {}
# Info.plist에서 토큰 설정 확인
fd "Info.plist" --exec grep -l -i "github" {} || echo "Info.plist에서 GitHub 관련 설정을 찾을 수 없습니다"
# SecretInfoPlist 설정 확인
fd "SecretInfoPlist.swift" --exec cat {}Length of output: 1031
🏁 Script executed:
#!/bin/bash
# String extension에서 githubAccessToken 구현 확인
echo "Inspecting String+.swift implementation of githubAccessToken..."
fd "String+" --extension swift --exec sed -n '1,200p' {} Length of output: 4347
GitHub 토큰이 앱 번들에 노출됩니다
현재 구현 방식
- SecretInfoPlist.swift
•dictionary에"GITHUB_ACCESS_TOKEN": "$(GITHUB_ACCESS_TOKEN)"를 정의해, 빌드 시 환경 변수값이 Info.plist에 삽입 - String+.swift (
Projects/Core/Sources/Extension/String+.swift)
•static var githubAccessToken이Bundle.main을 통해 Info.plist 키"GITHUB_ACCESS_TOKEN"을 읽어와 헤더에 삽입
문제점
- Info.plist에 실제 토큰이 포함되어 최종 앱에 평문으로 노출되어 보안 리스크 발생
- 토큰 유출 시 GitHub 리포지토리 접근 권한 남용 가능
권장 조치
- 토큰을 앱 번들에 직접 포함하지 않고, 런타임에 Keychain 혹은 안전한 백엔드 서버에서만 가져오도록 변경
- 부여된 토큰 스코프(scope)를 최소 권한 원칙에 맞게 제한
- CI/CD 환경에서만 빌드 타임 주입하도록 검토하고, 로컬 개발용 토큰은 별도 관리
🤖 Prompt for AI Agents
In Projects/NetworkService/Sources/EndPoint/GithubFileDownloadEndPoint.swift
around lines 22 to 24, the GitHub token is currently read from Info.plist,
exposing it in the app bundle and causing a security risk. To fix this, remove
the token retrieval from Info.plist and instead fetch the token securely at
runtime from the Keychain or a secure backend server. Also, ensure the token
scope is limited and consider managing tokens differently for CI/CD and local
development environments to avoid embedding secrets in the app bundle.
| return fileDownloadRepository.downloadFile( | ||
| repo: "BusStationData", | ||
| filePath: "bus_station_list.json", | ||
| directoryName: "jsons", | ||
| fileName: "bus_station_list.json" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 저장소와 파일 정보를 설정 가능하게 만드는 것을 고려해보세요.
repo: "BusStationData"와 파일 경로들이 하드코딩되어 있어 유연성이 떨어집니다.
구성 객체나 의존성 주입을 통해 이 값들을 외부에서 설정할 수 있도록 개선하는 것을 권장합니다:
public struct BusStationConfig {
let repository: String
let filePath: String
let directoryName: String
let fileName: String
}🤖 Prompt for AI Agents
In Projects/Domain/Sources/UseCase/UpdateBusStationListUseCase.swift around
lines 46 to 51, the repository and file path parameters are hardcoded, reducing
flexibility. Refactor the code to accept these values via a configuration object
or dependency injection, such as a BusStationConfig struct containing
repository, filePath, directoryName, and fileName properties, and use this
config to provide the parameters to downloadFile instead of hardcoded strings.
| func getFileURL( | ||
| directoryName: String, | ||
| fileName: String | ||
| ) throws -> URL { | ||
| let directoryURL = fileManager.urls( | ||
| for: .documentDirectory, | ||
| in: .userDomainMask | ||
| )[0].appendingPathComponent(directoryName) | ||
|
|
||
| let fileURL = directoryURL.appendingPathComponent(fileName) | ||
|
|
||
| guard fileManager.fileExists(atPath: fileURL.path) else { | ||
| throw FileManagerServiceError.fileNotFound | ||
| } | ||
|
|
||
| return fileURL | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getFileURL 메소드의 설계 재검토 필요.
이 메소드는 파일이 존재하지 않으면 항상 에러를 던지는데, isExist 메소드에서는 이것이 적절하지 않을 수 있습니다.
파일 존재 여부 확인용 별도 메소드를 만들거나, 옵셔널 파라미터로 존재 확인을 건너뛸 수 있도록 하는 것을 고려해보세요:
func getFileURL(
directoryName: String,
fileName: String,
requiresExistence: Bool = true
) throws -> URL {
let directoryURL = fileManager.urls(
for: .documentDirectory,
in: .userDomainMask
)[0].appendingPathComponent(directoryName)
let fileURL = directoryURL.appendingPathComponent(fileName)
if requiresExistence {
guard fileManager.fileExists(atPath: fileURL.path) else {
throw FileManagerServiceError.fileNotFound
}
}
return fileURL
}🤖 Prompt for AI Agents
In Projects/FileManagerService/Sources/DefaultFileManagerService.swift around
lines 124 to 141, the getFileURL method always throws an error if the file does
not exist, which is not suitable for use cases like isExist that only check for
existence. Modify getFileURL to add an optional Boolean parameter, such as
requiresExistence with a default value of true, and only throw the fileNotFound
error if requiresExistence is true. This allows callers to skip the existence
check when not needed, improving flexibility.
| public func isExist( | ||
| directoryName: String, | ||
| fileName: String | ||
| ) -> Observable<Bool> { | ||
| do { | ||
| let fileURL = try getFileURL( | ||
| directoryName: directoryName, | ||
| fileName: fileName | ||
| ) | ||
| return .just(fileManager.fileExists(atPath: fileURL.path)) | ||
| } catch { | ||
| return .error(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
isExist 메소드의 에러 처리 패턴 불일치.
다른 메소드들은 .create 패턴을 사용하는데, isExist만 다른 패턴을 사용하고 있습니다. 일관성을 위해 동일한 패턴 사용을 고려해보세요.
public func isExist(
directoryName: String,
fileName: String
) -> Observable<Bool> {
- do {
- let fileURL = try getFileURL(
- directoryName: directoryName,
- fileName: fileName
- )
- return .just(fileManager.fileExists(atPath: fileURL.path))
- } catch {
- return .error(error)
- }
+ .create { [weak self] observer in
+ guard let self else {
+ observer.onError(FileManagerServiceError.unknown)
+ return Disposables.create()
+ }
+ do {
+ let directoryURL = try self.getDirectoryURL(with: directoryName)
+ let fileURL = directoryURL.appendingPathComponent(fileName)
+ let exists = self.fileManager.fileExists(atPath: fileURL.path)
+ observer.onNext(exists)
+ observer.onCompleted()
+ } catch {
+ observer.onError(error)
+ }
+ return Disposables.create()
+ }📝 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.
| public func isExist( | |
| directoryName: String, | |
| fileName: String | |
| ) -> Observable<Bool> { | |
| do { | |
| let fileURL = try getFileURL( | |
| directoryName: directoryName, | |
| fileName: fileName | |
| ) | |
| return .just(fileManager.fileExists(atPath: fileURL.path)) | |
| } catch { | |
| return .error(error) | |
| } | |
| } | |
| public func isExist( | |
| directoryName: String, | |
| fileName: String | |
| ) -> Observable<Bool> { | |
| .create { [weak self] observer in | |
| guard let self else { | |
| observer.onError(FileManagerServiceError.unknown) | |
| return Disposables.create() | |
| } | |
| do { | |
| let directoryURL = try self.getDirectoryURL(with: directoryName) | |
| let fileURL = directoryURL.appendingPathComponent(fileName) | |
| let exists = self.fileManager.fileExists(atPath: fileURL.path) | |
| observer.onNext(exists) | |
| observer.onCompleted() | |
| } catch { | |
| observer.onError(error) | |
| } | |
| return Disposables.create() | |
| } | |
| } |
🤖 Prompt for AI Agents
In Projects/FileManagerService/Sources/DefaultFileManagerService.swift around
lines 88 to 101, the isExist method uses .just and .error directly for returning
Observable, which is inconsistent with other methods using the .create pattern.
Refactor isExist to use Observable.create, wrapping the file existence check
inside the create closure and emitting the result or error accordingly to
maintain consistency in error handling and observable creation.
| public var body: [String: Any] { [:] } | ||
| public var method: HTTPMethod { .get } | ||
| public var header: [String: String] { | ||
| ["Authorization": "token \(String.githubAccessToken)"] |
There was a problem hiding this comment.
githubAccessToken이 String의 extension이 아닌 초기화에서 받도록 수정이 필요할 것 같습니다.
githubAccessToken은 Core 모듈의 Bundle에서 가져오는 것이 아닌 App 모듈의 Bundle에서 가져오는 것이기 때문에 Core 모듈이 아닌 App 모듈에 정의가 적합할 것 같습니다.
There was a problem hiding this comment.
#332 PR에 포함된 InfoPlistWrapper가 Bundle 데이터를 관리하기 위해 구현되어서 App 모듈에서 사용해 주입해주시면 될 것 같습니다!
InfoPlistWrapper 작업 커밋
|
|
||
| import RxSwift | ||
|
|
||
| public protocol FileManagerService { |
There was a problem hiding this comment.
Data, Domain 모듈의 객체는 SwiftConcurrency 형태로 수정부탁드립니다!
| public struct GithubFileDownloadEndPoint: EndPoint { | ||
| private let owner = "Pepsi-Club" | ||
| private let branch = "main" | ||
| private let repo: String |
There was a problem hiding this comment.
repo는 고정된 형태로 사용하게 될 것 같은데 EndPoint에서 고정된 path로 관리하는게 어떨까요?
그리고 레포명이 BusStationData가 적합한지는 논의가 필요할 것 같네요. 추후 다른 데이터들을 저장하고 관리한다면 BusStation뿐 아니라 다른 데이터를 관리하는 경우도 생길 것 같아요. 예를들어 현재 Bundle로 사용하고 있는 APIKey 목록을 동적으로 관리할 수 있을 것 같습니다.
|
|
||
| private extension DefaultFileManagerService { | ||
| func getDirectoryURL(with name: String) throws -> URL { | ||
| let directoryURL = fileManager.urls( |
There was a problem hiding this comment.
let directoryURL = fileManager.urls(
for: .documentDirectory,
in: .userDomainMask
)[0]이 구문은 반복되어 사용되기에 연산프로퍼티로 관리할 수 있을 것 같아요.
그리고 [0] 구문은 런타임 크래시가 발생할 수 있어 수정 부탁드립니다.
| public let dependencies: [TargetDependency] | ||
|
|
||
| public init( | ||
| @TargetComponentBuilder dependencies builder: () -> TargetComponentBuilder = { |
There was a problem hiding this comment.
파라미터가 하나이기에 줄바꿈 없이 작성하는건 어떠실까요?
| busStopCoordinator.start() | ||
| } | ||
|
|
||
| private func checkAndDownloadBusStationList() { |
| func execute() -> Observable<Void> | ||
| } | ||
|
|
||
| public final class DefaultUpdateBusStationListUseCase: UpdateBusStationListUseCase { |
There was a problem hiding this comment.
이 객체와 인터페이스도 SwiftConcurrency 형태로 수정 부탁드립니다!
|
|
||
| public init() { } | ||
|
|
||
| public func downloadFile( |
There was a problem hiding this comment.
다운로드 받은 파일을 디바이스에 저장하는 의사결정은 UseCase에서 하는게 적합하지 않을까 생각이 드는데 어떻게 생각하시나요?
BusStationVersionRepository의 fetchRemoteVersion처럼 단방향으로 데이터만 전달해 주는 방향을 생각했습니다!
| func fetchLocalVersion() -> String? | ||
|
|
||
| /// 로컬에 새로운 버스 정류장 데이터의 버전 정보를 저장합니다. | ||
| func save(version: String) |
There was a problem hiding this comment.
함수명이 위의 fetch 메서드들을 보았을 때 불명확한부분이 있는 것 같습니다.
saveLocalVersion(: String)이나 saveLocalVersion( version: String)처럼 어떤 버전을 저장하는지 명확하게 나타낼 필요가 있을 것 같아요!
그리고 Repository의 비동기 메서드 Swift Concurrency 형태로 수정 부탁드립니다!
| static var githubAccessToken: Self { | ||
| guard let any = Bundle.main.object(forInfoDictionaryKey: "GITHUB_ACCESS_TOKEN"), | ||
| let githubAccessToken = any as? String | ||
| else { | ||
| return "" | ||
| } | ||
| return githubAccessToken | ||
| } |
There was a problem hiding this comment.
해당 토큰을 Core에 위치하게 되었을 때 다른 레이어에서도 접근할 수 있게 될 것 같은데, 특정 레이어에서만 필요한 데이터라서 여기에 위치시키는게 맞을지 고민이 되네요
| .flatMap { [weak self] data -> Observable<Void> in | ||
| guard let self = self else { | ||
| return .error(RxError.unknown) // Or a custom error | ||
| } | ||
| return self.fileManagerService.save( | ||
| data: data, | ||
| directoryName: directoryName, | ||
| fileName: fileName | ||
| ) | ||
| } |
There was a problem hiding this comment.
이렇게 한다면 내부에 weak self 해서 작성해야하는 반복 코드를 줄일 수 있을 것 같습니다.
만약 weak self로 한다면? return 시 떨어질 때 로깅을 넣어줘야하지 않을까 싶어요
| public protocol UpdateBusStationListUseCase { | ||
| func execute() -> Observable<Void> | ||
| } |
There was a problem hiding this comment.
인터페이스는 인터페이스 폴더로 따로 작성해주시면 좋을 것 같습니다!
| public func execute() -> Observable<Void> { | ||
| return versionRepository.fetchRemoteVersion() | ||
| .do(onNext: { remoteVersion in | ||
| print("🚏 버스정류장 원격 버전 확인: \(remoteVersion.busStationVersion)") | ||
| }, onError: { error in | ||
| print("🚏 버스정류장 원격 버전 확인 중 에러 발생: \(error)") | ||
| }) | ||
| .flatMap { [weak self] remoteVersion -> Observable<Void> in | ||
| guard let self = self else { return .error(RxError.unknown) } | ||
|
|
||
| let localVersion = self.versionRepository.fetchLocalVersion() | ||
| print("🚏 버스정류장 로컬 버전 확인: \(localVersion ?? "기존 파일 없음")") | ||
|
|
||
| let needsUpdate = localVersion != remoteVersion.busStationVersion | ||
| print("🚏 [버스정류장 버전 비교]") | ||
| print("로컬: \(localVersion ?? "파일 없음")") | ||
| print("원격: \(remoteVersion.busStationVersion)") | ||
|
|
||
| if needsUpdate { | ||
| return self.downloadAndSave( | ||
| newVersion: remoteVersion.busStationVersion | ||
| ) | ||
| } else { | ||
| print("🚏 버스정류장 정보가 이미 최신 버전입니다.") | ||
| return .just(()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
디버깅용 print는 없애거나 형태를 고정하는 걸 같이 의논해보면 어떨까요? 지금도 로깅 자체가 많이 깔리고 있어서 프린트가 많이 찍히는게 고민이 됩니다! 그리고 이전 코멘트랑 동일하게 withUnretained를 사용하면 약한 참조 관련해서 보일러 플레이트가 줄 것 같습니다.
| public init( | ||
| fileManager: FileManager = .default | ||
| ) { | ||
| self.fileManager = fileManager | ||
| } |
There was a problem hiding this comment.
파라미터 하나라서 init을 한줄로 작성해볼 수 있지 않을까요 ?
| } | ||
|
|
||
| func start() { | ||
| checkAndDownloadBusStationList() |
There was a problem hiding this comment.
다운로드를 하고 탭을 시작하게 되어있는데, 다운로드 실패 성공에 따라 시작을 다르게 되어야 원활하게 앱 사용이 될 것 같은데 의견 궁금하고 관련해서는 기획까지 이야기가 되어야할 것 같습니다.
retry를 해줄 수 있는 화면을 만든다던가 안내 화면을 띄운다던가, 혹시 다운로드되는 화면에서 프로그레스 뷰를 보여준다던가..!
|
PR title 확인해주세요~~ |
MUKER-WON
changed the title
3 participants
작업내용
리뷰요청
관련 이슈
close #331
Summary by CodeRabbit
신규 기능
버그 수정
기타