[Feat] Share Extension을 통한 외부 앱 네컷사진 저장 기능 추가#187
Conversation
…into feat/#182-share-extension # Conflicts: # Neki-iOS.xcodeproj/project.pbxproj
Remaked-Swain
added
Add ✚
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
Walkthrough앱 그룹 기반 Share Extension 타겟과 관련 리포지토리·유틸리티·UI·엔타이틀먼트가 추가되고, 딥링크로 메인 앱을 트리거해 공유 이미지들을 병렬 업로드하고 정리하는 흐름이 구현되었습니다. Changes
시퀀스 다이어그램sequenceDiagram
participant User as 사용자(소스 앱)
participant ShareExt as Share Extension
participant Container as App Group 컨테이너
participant MainApp as 메인 앱
participant Archive as Archive 기능
User->>ShareExt: 이미지 공유 요청
activate ShareExt
ShareExt->>ShareExt: NSItemProvider에서 이미지 추출/검증
ShareExt->>Container: 파일 저장 (SharedImages)
Container-->>ShareExt: 저장 완료
ShareExt->>MainApp: Deep Link 호출 (neki://shareExtension?appGroupID=...)
deactivate ShareExt
activate MainApp
MainApp->>MainApp: onOpenURL 수신 → pendingShareAppGroupID 설정
MainApp->>Archive: addPhotoFromShareExtension(appGroupID)
activate Archive
Archive->>Container: shared image URLs 조회
Container-->>Archive: [URL,...]
Archive->>Archive: uploadConcurrentlyFromURLs (병렬 업로드, batch size 3)
Archive->>Container: 업로드 후 shared images 정리
Container-->>Archive: 정리 완료
Archive-->>MainApp: uploadSharedImagesResponse (성공/실패)
deactivate Archive
MainApp->>MainApp: 사용자에게 결과 표시
deactivate MainApp
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Neki-iOS/APP/Sources/Application/AppCoordinator.swift (1)
196-203:⚠️ Potential issue | 🟠 Major
.userSessionStatusChanged(.signedIn)경로에서 pending share extension 실행이 누락되었습니다.
.userSessionStatusChanged액션으로 사용자가 signedIn 상태로 전환될 때, 라우트가.mainTab으로 설정되지만executePendingShareExtensionIfNeeded가 호출되지 않습니다. 같은 signedIn 상태로의 라우팅을 처리하는 다른 경로들(.route(.auth(.delegate(.moveToMainTab)))226번 줄,navigateToNextScreen함수 259번 줄)에서는 모두 이 함수를 명시적으로 호출하고 있어, 일관성이 필요합니다.공유 확장 기능이 사용자 세션 변경 시나리오에서 실행되지 않을 수 있으므로, 이 경로에서도
.send(.executePendingShareExtensionIfNeeded)를 반환하도록 수정하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/APP/Sources/Application/AppCoordinator.swift` around lines 196 - 203, In the .userSessionStatusChanged(.signedIn) branch, after setting state.route = .mainTab(.init(user: user)) ensure you also return a .send(.executePendingShareExtensionIfNeeded) so the pending share extension runs; locate the switch handling .userSessionStatusChanged(.signedIn) in AppCoordinator (the case let .signedIn(user) block that mutates state.route) and change the return from .none to .send(.executePendingShareExtensionIfNeeded) (matching other paths like .route(.auth(.delegate(.moveToMainTab))) and navigateToNextScreen).
🧹 Nitpick comments (11)
Neki-iOS/APP/Sources/Application/Neki_iOSApp.swift (1)
25-28: 커스텀 스킴은 최소 화이트리스트 검증 후 디스패치하는 것을 권장합니다.Line 25에서 scheme만 확인하고 전달하면 의도치 않은
neki://...입력도 동일 경로로 들어옵니다.host == "shareExtension"와appGroupID존재를 먼저 확인하면 노이즈 액션/예외 경로를 줄일 수 있습니다.예시 수정안
private func handleIncomingURL(_ url: URL) { - if url.scheme == "neki" { + if url.scheme?.lowercased() == "neki" { + let components = URLComponents(url: url, resolvingAgainstBaseURL: false) + let hasAppGroupID = components?.queryItems?.contains { + $0.name == "appGroupID" && !($0.value ?? "").isEmpty + } == true + guard url.host == "shareExtension", hasAppGroupID else { return } + store.send(.onOpenURL(url)) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/APP/Sources/Application/Neki_iOSApp.swift` around lines 25 - 28, Validate the incoming URL more strictly before dispatching: instead of only checking url.scheme == "neki" in Neki_iOSApp.swift, also verify url.host == "shareExtension" and that the required appGroupID exists (e.g., check AppConfig.appGroupID or similar) before calling store.send(.onOpenURL(url)); if any check fails, do not dispatch and optionally handle/log the unexpected URL. This change targets the url handling branch where store.send(.onOpenURL(url)) is invoked to reduce noise and unintended activations.Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swift (1)
10-14:appGroupID를 값 객체로 감싸는 리팩터를 권장합니다.현재처럼
String을 직접 받으면 호출부 오타가 런타임 실패로만 드러납니다. 타입을 분리하면 API 안정성이 올라갑니다.예시 수정안
import Foundation +public struct AppGroupID: RawRepresentable, Hashable, Sendable { + public let rawValue: String + public init(rawValue: String) { self.rawValue = rawValue } +} + public protocol SharedImageRepository { - func fetchSharedImages(appGroupID: String) async throws -> [ImageUploadEntity] - func fetchSharedImageURLs(appGroupID: String) async throws -> [URL] - func clearSharedImages(appGroupID: String) async throws + func fetchSharedImages(appGroupID: AppGroupID) async throws -> [ImageUploadEntity] + func fetchSharedImageURLs(appGroupID: AppGroupID) async throws -> [URL] + func clearSharedImages(appGroupID: AppGroupID) async throws }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swift` around lines 10 - 14, 리포지토리 메서드들이 모든 곳에서 문자열 오타로 인한 런타임 버그에 취약하므로 appGroupID 파라미터를 String에서 값 객체로 교체하세요: 새 Value Object 타입(예: AppGroupID) 를 정의하고 SharedImageRepository의 메서드 시그니처(fetchSharedImages, fetchSharedImageURLs, clearSharedImages)의 appGroupID 타입을 해당 값 객체로 변경한 뒤, 모든 호출부를 찾아 문자열 대신 AppGroupID 인스턴스를 생성하여 전달하도록 수정하고, 필요하면 값 객체에 유효성 검사/표현자(예: rawValue 또는 urlEncoded())를 추가해 사용성을 보장하세요.Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift (1)
14-26:Sendable적합성 확인이 필요합니다.
FileManagerSharedImageRepository가async메서드를 가지고 있고DependencyKey로 등록되어 concurrent context에서 사용되지만,Sendable적합성이 선언되지 않았습니다.
FileManager.default는 thread-safe하므로@unchecked Sendable을 추가하는 것이 좋습니다 (Share Extension의FileManagerExtensionImageRepository와 동일하게).♻️ Sendable 적합성 추가
-public struct FileManagerSharedImageRepository { +public struct FileManagerSharedImageRepository: `@unchecked` Sendable {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift` around lines 14 - 26, The struct FileManagerSharedImageRepository lacks Sendable conformance but is used concurrently; update its definition to declare Sendable conformance (use `@unchecked` Sendable) and ensure the private property fileManager (initialized with FileManager.default in init) is the reason for unchecked conformance; match the approach used in FileManagerExtensionImageRepository so the type can be used safely in concurrent contexts.NekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swift (2)
10-12:ShareExtensionConfigurationError가 정의되었지만 사용되지 않습니다.이 에러 enum이 선언되었으나 현재 코드에서 throw되는 곳이 없습니다. 향후 사용 예정이라면 TODO 주석을 추가하거나, 불필요하다면 제거를 고려하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swift` around lines 10 - 12, The enum ShareExtensionConfigurationError is declared but never used; either remove the unused enum to eliminate dead code or keep it and add a clear TODO comment explaining intended future use and where it will be thrown (e.g., during validation in ShareExtensionConfiguration initializers or parsing methods). Locate the symbol ShareExtensionConfigurationError and either delete its declaration or annotate it with a TODO explaining the planned error cases and where to implement throws (for example in configuration parsing/validation functions like init(from:) or validateConfiguration()).
18-28: 초기화 파라미터bundle이 사용되지 않습니다.
init(bundle: Bundle = .main)파라미터가 선언되었으나 실제로 사용되지 않습니다. 향후 확장성을 위해 남겨둔 것이라면_ bundle또는 주석으로 의도를 명시하세요.App Group ID 설정은 entitlements 파일과 정확히 일치합니다. ✓
♻️ 미사용 파라미터 제거 제안
- init(bundle: Bundle = .main) { + init() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swift` around lines 18 - 28, The init(bundle: Bundle = .main) parameter in ShareExtensionConfiguration.init(bundle:) is declared but never used; either remove the unused parameter or mark it as intentionally unused by renaming it to _bundle (or prefix with _ as _ bundle) and add a short comment explaining it's reserved for future extension; update the initializer signature and ensure references to appGroupID and deepLinkURL remain unchanged.Neki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swift (1)
27-54: 업로드된 미디어 ID 순서가 입력 URL 순서와 다를 수 있습니다.
withThrowingTaskGroup의for try await루프는 완료 순서대로 결과를 수집하므로, 반환되는uploadedMediaIDs배열의 순서가 입력fileURLs순서와 일치하지 않을 수 있습니다.현재 사용처(
ArchiveFeature)에서 순서가 중요하지 않다면 문제없지만, 순서가 필요하다면 인덱스와 함께 결과를 수집해야 합니다.♻️ 순서 보장이 필요한 경우 제안
uploadConcurrentlyFromURLs: { fileURLs, mediaType in - return try await withThrowingTaskGroup(of: [Int].self) { group in - var uploadedMediaIDs: [Int] = [] + return try await withThrowingTaskGroup(of: (Int, [Int]).self) { group in + var uploadedMediaIDs: [(Int, [Int])] = [] let maxConcurrentTasks = 3 - var urlIterator = fileURLs.makeIterator() + var urlIterator = fileURLs.enumerated().makeIterator() for _ in 0..<maxConcurrentTasks { - guard let url = urlIterator.next() else { break } + guard let (index, url) = urlIterator.next() else { break } group.addTask { let data = try Data(contentsOf: url) let entity = ImageUploadEntity(data: data, format: data.detectedImageFormat) - return try await repository.upload(items: [entity], mediaType: mediaType) + return (index, try await repository.upload(items: [entity], mediaType: mediaType)) } } - for try await resultIDs in group { - uploadedMediaIDs.append(contentsOf: resultIDs) + for try await result in group { + uploadedMediaIDs.append(result) - guard let nextURL = urlIterator.next() else { continue } + guard let (nextIndex, nextURL) = urlIterator.next() else { continue } group.addTask { let data = try Data(contentsOf: nextURL) let entity = ImageUploadEntity(data: data, format: data.detectedImageFormat) - return try await repository.upload(items: [entity], mediaType: mediaType) + return (nextIndex, try await repository.upload(items: [entity], mediaType: mediaType)) } } - return uploadedMediaIDs + return uploadedMediaIDs.sorted(by: { $0.0 < $1.0 }).flatMap { $0.1 } } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swift` around lines 27 - 54, The task group currently gathers results in completion order so uploadedMediaIDs may not match the input fileURLs order; modify uploadConcurrentlyFromURLs to preserve order by capturing each URL's index when you add a task (use the iterator index or enumerate fileURLs), have each group task return a tuple like (index: Int, ids: [Int]) from repository.upload, collect all tuples from the withThrowingTaskGroup, sort them by index, then flatten the ids into uploadedMediaIDs in the original fileURLs order before returning; update any references to uploadedMediaIDs handling accordingly (functions: uploadConcurrentlyFromURLs, ImageUploadEntity creation, repository.upload).Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swift (1)
296-299: 빈 이미지 목록 시 토스트가 두 번 표시될 수 있습니다.Line 297에서
showToast를 직접 호출하고, Line 298에서uploadSharedImagesResponse(.failure(...))를 보내면 Line 367-369에서 다시 에러 토스트가 표시됩니다.하나의 에러 경로만 사용하는 것이 좋습니다.
♻️ 중복 토스트 제거 제안
guard fileURLs.isEmpty == false else { - await send(.delegate(.showToast(NekiToastItem("가져올 수 있는 이미지가 없어요.", style: .error)))) await send(.uploadSharedImagesResponse(.failure(UploadError.uploadFailed))) return }그리고
uploadSharedImagesResponse(.failure)핸들러에서 에러 타입에 따라 적절한 메시지를 표시하도록 수정하거나, 현재처럼 일반 메시지를 사용합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swift` around lines 296 - 299, The guard in ArchiveFeature (the block that calls send(.delegate(.showToast(...))) then send(.uploadSharedImagesResponse(.failure(...)))) causes duplicate toasts because the uploadSharedImagesResponse failure is also handled later to show a toast; change this to a single error path by removing the direct send(.delegate(.showToast(...))) in that guard and only send the failure via send(.uploadSharedImagesResponse(.failure(...))), or alternatively update the uploadSharedImagesResponse failure handler to accept and show the message while removing the inline toast; locate the guard around fileURLs and the uploadSharedImagesResponse handler in ArchiveFeature and apply one of these two fixes so only one toast is produced.NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift (1)
166-170: 이미지 초과 시 badge 색상 가독성이 낮을 수 있습니다.Line 97에서
backgroundColor가.systemBlue로 설정되고, 초과 시textColor만.systemRed로 변경됩니다. 파란 배경에 빨간 텍스트는 가독성이 낮을 수 있습니다.♻️ 초과 시 배경색도 함께 변경
let isOverLimit = count > maxImageCount countBadgeLabel.text = isOverLimit ? "10+" : "\(count)" -countBadgeLabel.textColor = isOverLimit ? .systemRed.withAlphaComponent(0.9) : .label +countBadgeLabel.textColor = isOverLimit ? .white : .label +countBadgeLabel.backgroundColor = isOverLimit ? .systemRed.withAlphaComponent(0.9) : .systemBlue.withAlphaComponent(0.9) countBadgeLabel.isHidden = count < 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift` around lines 166 - 170, The badge becomes hard to read when countBadgeLabel has a .systemBlue background but only its textColor is switched to .systemRed when over the limit; update the badge styling so that when isOverLimit you change countBadgeLabel.backgroundColor to a high-contrast color (e.g., .systemRed.withAlphaComponent(0.15) or a darker/red-tinted color) and adjust countBadgeLabel.textColor to a readable foreground (e.g., .systemRed or .white) and when not over limit restore the original background (.systemBlue) and textColor (.label); modify the code around countBadgeLabel (where backgroundColor is set) so both backgroundColor and textColor are toggled together when computing isOverLimit, keeping navigationBar.items?.first?.rightBarButtonItem?.isEnabled logic unchanged.NekiShareExtension/Sources/Domain/Sources/Utilities/ImageShareProcessor.swift (2)
11-16:invalidProvider에러 케이스가 사용되지 않습니다.
ImageShareProcessorError.invalidProvider가 선언되었지만 현재 구현에서 사용되지 않습니다. 의도적으로 향후 사용을 위해 예약한 것이라면 주석을 추가하거나, 그렇지 않다면 제거를 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NekiShareExtension/Sources/Domain/Sources/Utilities/ImageShareProcessor.swift` around lines 11 - 16, The ImageShareProcessorError enum declares a case invalidProvider that is never used; either remove the unused case from ImageShareProcessorError or, if it's intentionally reserved for future logic, add a clear comment above invalidProvider in the enum stating it’s reserved for future use (e.g., "reserved for unsupported provider errors") so reviewers know it’s intentional; update any related tests or code paths that might expect invalidProvider if you choose removal.
68-78: Share Extension 메모리 제약 고려.
withThrowingTaskGroup으로 모든 이미지를 동시에 처리합니다. 파일 복사 방식이므로 메모리 사용량은 낮겠지만, Share Extension의 메모리 제한(~120MB)을 고려하여 대용량 이미지 10장 동시 처리 시 안정성을 테스트해 주세요.필요 시 순차 처리로 변경하거나 동시 실행 수를 제한할 수 있습니다:
♻️ 동시 실행 수 제한 옵션
func share(providers: [NSItemProvider]) async throws { guard providers.isEmpty == false else { throw ImageShareProcessorError.noImageFound } - try await withThrowingTaskGroup(of: Void.self) { group in - for provider in providers { - group.addTask { try await processAndSaveImage(from: provider) } - } - - for try await _ in group {} + // 순차 처리 방식 + for provider in providers { + try await processAndSaveImage(from: provider) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NekiShareExtension/Sources/Domain/Sources/Utilities/ImageShareProcessor.swift` around lines 68 - 78, The current share(providers:) spawns a task for every NSItemProvider concurrently which can blow the Share Extension memory; change to either sequential processing or bounded concurrency: replace the withThrowingTaskGroup-all-at-once approach and instead call processAndSaveImage(from:) in a simple for-await loop to process one-by-one, or implement batching/limited parallelism (e.g., process providers in chunks of size N or use a counting semaphore/limited TaskGroup) so at most N concurrent tasks run; update the implementation inside share(providers:) and keep processAndSaveImage(from:) unchanged, exposing a configurable concurrency limit constant if needed.Neki-iOS.xcodeproj/project.pbxproj (1)
355-427: Debug와 Release 간SWIFT_APPROACHABLE_CONCURRENCY설정 불일치.Debug 설정(Line 386)에는
SWIFT_APPROACHABLE_CONCURRENCY = YES가 있지만 Release 설정(Lines 394-427)에는 없습니다. 이로 인해 Debug와 Release 빌드 간 동시성 동작이 달라질 수 있습니다.일관성을 위해 Release 설정에도 동일한 값을 추가하는 것을 권장합니다.
♻️ Release 설정에 추가
59235C5C2F64245A00198051 /* Release */ = { isa = XCBuildConfiguration; buildSettings = { ... SWIFT_EMIT_LOC_STRINGS = NO; + SWIFT_APPROACHABLE_CONCURRENCY = YES; SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; ... }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS.xcodeproj/project.pbxproj` around lines 355 - 427, The Release build configuration (the XCBuildConfiguration block with name = Release / id 59235C5C2F64245A00198051) is missing the SWIFT_APPROACHABLE_CONCURRENCY build setting present in Debug; add SWIFT_APPROACHABLE_CONCURRENCY = YES inside the Release buildSettings to match Debug so concurrency behavior is consistent across builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift`:
- Around line 189-198: The openMainApp(with:) implementation uses the responder
chain to find UIApplication (which is unsupported in extensions) so replace that
logic by calling NSExtensionContext's open(_:completionHandler:) on
self.extensionContext to open the URL; use the completion handler to call
self.extensionContext?.completeRequest(returningItems: []) (or handle failures
accordingly) and remove the responder-chain loop — update the function named
openMainApp(with:) to use extensionContext.open instead of searching for
UIApplication.
---
Outside diff comments:
In `@Neki-iOS/APP/Sources/Application/AppCoordinator.swift`:
- Around line 196-203: In the .userSessionStatusChanged(.signedIn) branch, after
setting state.route = .mainTab(.init(user: user)) ensure you also return a
.send(.executePendingShareExtensionIfNeeded) so the pending share extension
runs; locate the switch handling .userSessionStatusChanged(.signedIn) in
AppCoordinator (the case let .signedIn(user) block that mutates state.route) and
change the return from .none to .send(.executePendingShareExtensionIfNeeded)
(matching other paths like .route(.auth(.delegate(.moveToMainTab))) and
navigateToNextScreen).
---
Nitpick comments:
In `@Neki-iOS.xcodeproj/project.pbxproj`:
- Around line 355-427: The Release build configuration (the XCBuildConfiguration
block with name = Release / id 59235C5C2F64245A00198051) is missing the
SWIFT_APPROACHABLE_CONCURRENCY build setting present in Debug; add
SWIFT_APPROACHABLE_CONCURRENCY = YES inside the Release buildSettings to match
Debug so concurrency behavior is consistent across builds.
In `@Neki-iOS/APP/Sources/Application/Neki_iOSApp.swift`:
- Around line 25-28: Validate the incoming URL more strictly before dispatching:
instead of only checking url.scheme == "neki" in Neki_iOSApp.swift, also verify
url.host == "shareExtension" and that the required appGroupID exists (e.g.,
check AppConfig.appGroupID or similar) before calling
store.send(.onOpenURL(url)); if any check fails, do not dispatch and optionally
handle/log the unexpected URL. This change targets the url handling branch where
store.send(.onOpenURL(url)) is invoked to reduce noise and unintended
activations.
In `@Neki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swift`:
- Around line 27-54: The task group currently gathers results in completion
order so uploadedMediaIDs may not match the input fileURLs order; modify
uploadConcurrentlyFromURLs to preserve order by capturing each URL's index when
you add a task (use the iterator index or enumerate fileURLs), have each group
task return a tuple like (index: Int, ids: [Int]) from repository.upload,
collect all tuples from the withThrowingTaskGroup, sort them by index, then
flatten the ids into uploadedMediaIDs in the original fileURLs order before
returning; update any references to uploadedMediaIDs handling accordingly
(functions: uploadConcurrentlyFromURLs, ImageUploadEntity creation,
repository.upload).
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift`:
- Around line 14-26: The struct FileManagerSharedImageRepository lacks Sendable
conformance but is used concurrently; update its definition to declare Sendable
conformance (use `@unchecked` Sendable) and ensure the private property
fileManager (initialized with FileManager.default in init) is the reason for
unchecked conformance; match the approach used in
FileManagerExtensionImageRepository so the type can be used safely in concurrent
contexts.
In
`@Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swift`:
- Around line 10-14: 리포지토리 메서드들이 모든 곳에서 문자열 오타로 인한 런타임 버그에 취약하므로 appGroupID
파라미터를 String에서 값 객체로 교체하세요: 새 Value Object 타입(예: AppGroupID) 를 정의하고
SharedImageRepository의 메서드 시그니처(fetchSharedImages, fetchSharedImageURLs,
clearSharedImages)의 appGroupID 타입을 해당 값 객체로 변경한 뒤, 모든 호출부를 찾아 문자열 대신 AppGroupID
인스턴스를 생성하여 전달하도록 수정하고, 필요하면 값 객체에 유효성 검사/표현자(예: rawValue 또는 urlEncoded())를 추가해
사용성을 보장하세요.
In
`@Neki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swift`:
- Around line 296-299: The guard in ArchiveFeature (the block that calls
send(.delegate(.showToast(...))) then
send(.uploadSharedImagesResponse(.failure(...)))) causes duplicate toasts
because the uploadSharedImagesResponse failure is also handled later to show a
toast; change this to a single error path by removing the direct
send(.delegate(.showToast(...))) in that guard and only send the failure via
send(.uploadSharedImagesResponse(.failure(...))), or alternatively update the
uploadSharedImagesResponse failure handler to accept and show the message while
removing the inline toast; locate the guard around fileURLs and the
uploadSharedImagesResponse handler in ArchiveFeature and apply one of these two
fixes so only one toast is produced.
In
`@NekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swift`:
- Around line 10-12: The enum ShareExtensionConfigurationError is declared but
never used; either remove the unused enum to eliminate dead code or keep it and
add a clear TODO comment explaining intended future use and where it will be
thrown (e.g., during validation in ShareExtensionConfiguration initializers or
parsing methods). Locate the symbol ShareExtensionConfigurationError and either
delete its declaration or annotate it with a TODO explaining the planned error
cases and where to implement throws (for example in configuration
parsing/validation functions like init(from:) or validateConfiguration()).
- Around line 18-28: The init(bundle: Bundle = .main) parameter in
ShareExtensionConfiguration.init(bundle:) is declared but never used; either
remove the unused parameter or mark it as intentionally unused by renaming it to
_bundle (or prefix with _ as _ bundle) and add a short comment explaining it's
reserved for future extension; update the initializer signature and ensure
references to appGroupID and deepLinkURL remain unchanged.
In
`@NekiShareExtension/Sources/Domain/Sources/Utilities/ImageShareProcessor.swift`:
- Around line 11-16: The ImageShareProcessorError enum declares a case
invalidProvider that is never used; either remove the unused case from
ImageShareProcessorError or, if it's intentionally reserved for future logic,
add a clear comment above invalidProvider in the enum stating it’s reserved for
future use (e.g., "reserved for unsupported provider errors") so reviewers know
it’s intentional; update any related tests or code paths that might expect
invalidProvider if you choose removal.
- Around line 68-78: The current share(providers:) spawns a task for every
NSItemProvider concurrently which can blow the Share Extension memory; change to
either sequential processing or bounded concurrency: replace the
withThrowingTaskGroup-all-at-once approach and instead call
processAndSaveImage(from:) in a simple for-await loop to process one-by-one, or
implement batching/limited parallelism (e.g., process providers in chunks of
size N or use a counting semaphore/limited TaskGroup) so at most N concurrent
tasks run; update the implementation inside share(providers:) and keep
processAndSaveImage(from:) unchanged, exposing a configurable concurrency limit
constant if needed.
In `@NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift`:
- Around line 166-170: The badge becomes hard to read when countBadgeLabel has a
.systemBlue background but only its textColor is switched to .systemRed when
over the limit; update the badge styling so that when isOverLimit you change
countBadgeLabel.backgroundColor to a high-contrast color (e.g.,
.systemRed.withAlphaComponent(0.15) or a darker/red-tinted color) and adjust
countBadgeLabel.textColor to a readable foreground (e.g., .systemRed or .white)
and when not over limit restore the original background (.systemBlue) and
textColor (.label); modify the code around countBadgeLabel (where
backgroundColor is set) so both backgroundColor and textColor are toggled
together when computing isOverLimit, keeping
navigationBar.items?.first?.rightBarButtonItem?.isEnabled logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 330536e7-b941-4a69-90e0-b6aed59b4bca
📒 Files selected for processing (18)
Neki-iOS.xcodeproj/project.pbxprojNeki-iOS/APP/Sources/Application/AppCoordinator.swiftNeki-iOS/APP/Sources/Application/Neki_iOSApp.swiftNeki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Client/SharedImageClient.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swiftNeki-iOS/Features/Archive/Sources/Presentation/Sources/Feature/ArchiveFeature.swiftNeki-iOS/Neki-iOS-Debug.entitlementsNeki-iOS/Neki-iOS-Release.entitlementsNekiShareExtension/NekiShareExtension-Debug.entitlementsNekiShareExtension/NekiShareExtension-Release.entitlementsNekiShareExtension/Resources/Info.plistNekiShareExtension/Sources/Data/Sources/FileManagerExtensionImageRepository.swiftNekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swiftNekiShareExtension/Sources/Domain/Sources/Interfaces/ExtensionImageRepository.swiftNekiShareExtension/Sources/Domain/Sources/Utilities/ImageShareProcessor.swiftNekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift
| func openMainApp(with url: URL) { | ||
| var responder: UIResponder? = self | ||
| while responder != nil { | ||
| if let application = responder as? UIApplication { | ||
| application.open(url) | ||
| break | ||
| } | ||
| responder = responder?.next | ||
| } | ||
| self.extensionContext?.completeRequest(returningItems: []) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 다른 프로젝트에서 share extension URL 열기 패턴 확인
rg -n "openURL" --type swift -C 3Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 15969
🏁 Script executed:
# Check for Share Extension code and ShareViewController
find . -type f -name "*ShareViewController*" -o -name "*Extension*" | grep -i share | head -20Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 548
🏁 Script executed:
# Look for existing extension implementations
fd -t f "ShareViewController" -x cat {}Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 8471
🏁 Script executed:
# Check the actual file structure
git ls-files | grep -i "extension" | head -20Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 858
🏁 Script executed:
# Look for any extension URL handling patterns
rg -l "extension" --type swift | head -10Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 787
🏁 Script executed:
# Check the entitlements for app groups
cat NekiShareExtension/NekiShareExtension-Debug.entitlementsRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 385
🏁 Script executed:
# Check the Info.plist for URL schemes
cat NekiShareExtension/Resources/Info.plist | head -50Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 874
🏁 Script executed:
# Look for the deep link URL configuration
cat NekiShareExtension/Sources/Domain/Sources/Entities/ShareExtensionConfiguration.swiftRepository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 809
🏁 Script executed:
# Check how the main app handles the deep link
rg -n "neki://shareExtension" -A 5 -B 5Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 1335
🏁 Script executed:
# Look for any URL scheme handling in the main app
rg -n "openURL\|handleURL\|URLScheme" --type swift -B 3 -A 3 | head -100Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🏁 Script executed:
# Check if there's any existing pattern for opening URLs from extensions in main app
rg -n "extensionContext\|NSExtensionItem\|complete.*Request" --type swift | head -20Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🏁 Script executed:
# Check how the main app handles the deep link for share extension
rg -n "shareExtension" --type swift -B 5 -A 5Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 2577
🏁 Script executed:
# Look for URL scheme handling in AppCoordinator or main app initialization
rg -n "scene.*didConnect\|application.*open.*url\|onOpenURL" --type swift -B 3 -A 5Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
🌐 Web query:
iOS Share Extension UIApplication access responder chain limitations
💡 Result:
In iOS Share Extensions (and app extensions generally), direct access to UIApplication.shared (formerly sharedApplication) is unavailable by design. Apple's official App Extension Programming Guide explicitly states: "An app extension cannot: Access a sharedApplication object, and so cannot use any of the methods on that object." This limitation exists because extensions run in a restricted environment hosted by another app, without their own full UIApplication instance. The responder chain in extensions follows the standard UIKit pattern (view -> superview -> view controller -> window -> UIApplication), but the UIApplication at the top is the host app's instance, not the extension's containing app's. Developers commonly traverse this chain starting from a view controller (e.g., self in ShareViewController) to reach UIApplication indirectly: @objc func openURL(_ url: URL) -> Bool { var responder: UIResponder? = self while responder != nil { if let application = responder as? UIApplication { return application.perform(#selector(openURL(:)), with: url) != nil } responder = responder?.next } return false } This hack works in practice for actions like openURL because it uses the host app's UIApplication to open URLs (e.g., custom schemes to launch the containing app). However, it violates Apple's documented restrictions, risks App Store rejection, and may fail in future iOS versions or specific hosts (e.g., deprecated openURL in iOS 18 logs warnings). Recent sources (2024) confirm it still functions despite deprecations by using updated open(:options:completionHandler:). Recommended alternatives: - Use NSExtensionContext.open(_:completionHandler:) to request URL opening via the host/system. - Share data via App Groups (UserDefaults or files) without launching the app. - Handle all logic within the extension, as intended by Apple. This policy has remained consistent since iOS 8, with no changes as of 2026.
Citations:
- 1: https://developer.apple.com/library/archive/documentation/General/Conceptual/ExtensibilityPG/ExtensionOverview.html
- 2: https://developer.apple.com/library/archive/documentation/General/Conceptual/ExtensibilityPG/index.html
- 3: https://developer.apple.com/documentation/uikit/using-responders-and-the-responder-chain-to-handle-events
- 4: https://stackoverflow.com/questions/27506413/share-extension-to-open-containing-app/27517554
- 5: https://stackoverflow.com/questions/79077018/unable-to-open-main-app-from-action-extension-in-ios-18-previously-working-met
- 6: https://developer.apple.com/documentation/foundation/nsextensioncontext
🏁 Script executed:
# Check if NSExtensionContext.open is available/used
rg -n "NSExtensionContext\|extensionContext.*open" --type swift -B 2 -A 2Repository: YAPP-Github/27th-App-Team-2-iOS
Length of output: 57
Share Extension에서 responder chain을 통한 URL 열기는 신뢰할 수 없습니다.
Share Extension에서는 responder chain을 통해 호스트 앱의 UIApplication에 접근할 수 없습니다. Apple의 공식 문서에서 명시하듯이 extension은 직접 UIApplication에 접근할 수 없으며, 현재 코드는 실제로 main app을 열지 못할 수 있습니다. 제안된 selector 기반 방식도 마찬가지로 미지원 API에 의존하므로 문제가 있습니다.
공식적으로 권장되는 방식은 NSExtensionContext의 open(_:completionHandler:) 메서드를 사용하는 것입니다:
🔧 권장 수정안
func openMainApp(with url: URL) {
- var responder: UIResponder? = self
- while responder != nil {
- if let application = responder as? UIApplication {
- application.open(url)
- break
- }
- responder = responder?.next
- }
+ self.extensionContext?.open(url, completionHandler: nil)
self.extensionContext?.completeRequest(returningItems: [])
}📝 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.
| func openMainApp(with url: URL) { | |
| var responder: UIResponder? = self | |
| while responder != nil { | |
| if let application = responder as? UIApplication { | |
| application.open(url) | |
| break | |
| } | |
| responder = responder?.next | |
| } | |
| self.extensionContext?.completeRequest(returningItems: []) | |
| func openMainApp(with url: URL) { | |
| self.extensionContext?.open(url, completionHandler: nil) | |
| self.extensionContext?.completeRequest(returningItems: []) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift`
around lines 189 - 198, The openMainApp(with:) implementation uses the responder
chain to find UIApplication (which is unsupported in extensions) so replace that
logic by calling NSExtensionContext's open(_:completionHandler:) on
self.extensionContext to open the URL; use the completion handler to call
self.extensionContext?.completeRequest(returningItems: []) (or handle failures
accordingly) and remove the responder-chain loop — update the function named
openMainApp(with:) to use extensionContext.open instead of searching for
UIApplication.
There was a problem hiding this comment.
PR 내용에 작성해두었듯이, iMessage, Today 위젯이 아닌 이상 NSExtensionContext의 open(_:completionHandler:) 메서드를 사용할 수 없습니다. 이는 공식문서에서도 확인할 수 있는 내용이며, 리스폰더 체인을 통한 DeepLink 열기 방식 밖에 사용할 수 없는 상황입니다. 따라서 이 리뷰는 잘못되었습니다.
There was a problem hiding this comment.
🧩 Analysis chain
@Remaked-Swain, 지적해 주셔서 감사합니다. 제 리뷰가 잘못되었습니다.
Apple의 공식 문서에 명시되어 있듯이, openURL:completionHandler: 메서드는 **Today 위젯(및 다른 extension 타입은 해당 없음)**만 호스팅 앱을 열기 위해 사용할 수 있습니다. 따라서 Share Extension에서는 NSExtensionContext.open(_:completionHandler:)를 사용할 수 없으며, 현재 구현하신 responder chain을 통한 방식이 Share Extension에서 컨테이닝 앱을 실행하는 올바른 접근 방식입니다. 제 이전 리뷰는 철회합니다.
🌐 Web query:
NSExtensionContext open(_:completionHandler:) share extension availability iOS
💡 Result:
NSExtensionContext.open(_:completionHandler:) exists, but it’s not generally available to Share extensions in the sense of “will successfully open your containing app.”
- Apple’s App Extension Programming Guide states that only a Today widget can ask the system to open its containing app via
NSExtensionContext.openURL:completionHandler:. [1] - Apple’s Today widget docs reiterate that widgets can open the containing app by calling
openURL:completionHandler:onNSExtensionContext. [2] - Apple Developer Forums (quoting the
NSExtensionContext.openAPI docs) notes that on iOS the extension points that support this are Today and iMessage, and iMessage has additional limits (can only open its parent app, and only under certain conditions). Share/Action extensions therefore typically getsuccess == false. [3]
So, for a Share extension, calling extensionContext.open(url) is expected to fail; the supported pattern is to share data (e.g., App Group / file / pasteboard) and have the user open the app (or use other UX like a notification/deep link the user taps).
Sources: [1] [2] [3]
@Remaked-Swain, 검색 결과로도 확인했습니다. 지적이 맞습니다.
iOS에서 NSExtensionContext.open(_:completionHandler:)를 지원하는 extension point는 Today 위젯과 iMessage뿐이며, Share Extension에서 이를 호출하면 success == false가 반환됩니다. 따라서 현재 구현하신 responder chain을 통해 UIApplication을 찾아 URL을 여는 방식이 Share Extension에서 사용할 수 있는 올바른 접근법입니다. 잘못된 리뷰를 드려 죄송합니다.
✏️ Learnings added
Learnt from: Remaked-Swain
Repo: YAPP-Github/27th-App-Team-2-iOS PR: 187
File: NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift:189-198
Timestamp: 2026-03-23T11:56:52.440Z
Learning: In iOS Share Extensions (`com.apple.share-services`), `NSExtensionContext.open(_:completionHandler:)` is NOT available. According to Apple's official App Extension Programming Guide, only Today widgets (not any other app extension type) can use `openURL:completionHandler:` on NSExtensionContext to open the containing app. The correct workaround for Share Extensions to open the containing app via a deep link/URL scheme is to traverse the UIResponder chain to find the UIApplication instance and call `open(_:)` on it. This is the standard accepted approach.
Learnt from: Remaked-Swain
Repo: YAPP-Github/27th-App-Team-2-iOS PR: 187
File: NekiShareExtension/Sources/Presentation/Sources/ShareViewController.swift:189-198
Timestamp: 2026-03-23T11:56:52.440Z
Learning: In iOS Share Extensions (`com.apple.share-services`), `NSExtensionContext.open(_:completionHandler:)` is NOT available and will return `success == false`. According to Apple's official App Extension Programming Guide and developer forums, only Today widgets and iMessage extensions support `NSExtensionContext.open(_:completionHandler:)` to open the containing app. The correct workaround for Share Extensions to open the containing app via a deep link/URL scheme is to traverse the UIResponder chain (starting from `self` in `ShareViewController`) to find the `UIApplication` instance and call `open(_:)` on it. Do NOT suggest `NSExtensionContext.open(_:completionHandler:)` for Share Extensions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…into feat/#182-share-extension # Conflicts: # Neki-iOS.xcodeproj/project.pbxproj # Neki-iOS/APP/Sources/Application/Neki_iOSApp.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Neki-iOS.xcodeproj/project.pbxproj`:
- Around line 43-54: The PBXCopyFilesBuildPhase entry named "Embed Foundation
Extensions" (identifier 59235C5A2F64245A00198051) is defined but not referenced
by the Neki-iOS target's buildPhases; add the phase's reference into the
Neki-iOS target object's buildPhases array so the NekiShareExtension.appex
copy-files phase is executed and the .appex is embedded into the app bundle
PlugIns directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 41c79585-08b2-443e-81f1-46bc349ae299
📒 Files selected for processing (2)
Neki-iOS.xcodeproj/project.pbxprojNeki-iOS/APP/Sources/Application/Neki_iOSApp.swift
OneTen19
left a comment
OneTen19
left a comment
There was a problem hiding this comment.
수고하셨습니다!!!
이미지 공유 과정 중, 로그인 전이라면 로그인을 수행한 뒤에 공유 과정을 이어서 진행하도록 했습니다. 이러한 엣지케이스들을 더 발견하고 대응하여 안정성을 높여야 합니다. 놓친 부분이 있다거나 추천할만한 아이디어가 있다면 말씀해주세요!
테스트코드 작성해보는 거 어떰!?
| SWIFT_APPROACHABLE_CONCURRENCY = YES; | ||
| SWIFT_EMIT_LOC_STRINGS = YES; | ||
| SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES; | ||
| SWIFT_VERSION = 5.0; |
| ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon_Dev; | ||
| ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; | ||
| CODE_SIGN_ENTITLEMENTS = "Neki-iOS/Neki-iOS.entitlements"; | ||
| CODE_SIGN_ENTITLEMENTS = "Neki-iOS/Neki-iOS-Debug.entitlements"; |
There was a problem hiding this comment.
p1
어 이거 파이어베이스 설정떄문에 debug, release 이름 동일하게 해둔건데 혹시 build phase 쪽 체크 되셨을까요!?
There was a problem hiding this comment.
이건 xcconfig아니고 entitlements인데다가 아까 잘되는거 확인받았습니다!
| "@executable_path/Frameworks", | ||
| ); | ||
| MARKETING_VERSION = 1.1.7; | ||
| MARKETING_VERSION = 1.2.0; |
There was a problem hiding this comment.
p3
아 우리 스프린트 끝나면 1.2.0 으로 출시할라캤는디 2.0.0 으로 해야겠군
There was a problem hiding this comment.
이거 아직 테플이든 빌드제출 안했으니까 1.2.0인 상태로 그대로 내비뒀다가 올리면 1.2.0으로 출ㄹ시된다 아님?
| private func handleIncomingURL(_ url: URL) { | ||
| if url.scheme == "neki" { | ||
| store.send(.onOpenURL(url)) | ||
| return | ||
| } | ||
|
|
||
| if url.scheme == "https" || url.scheme == "http" { | ||
| guard let host = url.host(), host == "neki.suitestudy.com" else { return } | ||
| return | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
p3
어 이거 익스텐션으로 분리해뒀었는데 다시 돌아왔다. 작동상 차이가 있나요??
There was a problem hiding this comment.
무슨 마ㄹ인지 모루겟소용
NekiApp의 익스텐션으로 메서드를 정리해뒀었다는 얘기인가요?
There was a problem hiding this comment.
정리했던 버전의 코드는 못봤는데 그래서 되돌린 적도 없긴함ㄴ다
| uploadConcurrentlyFromURLs: { fileURLs, mediaType in | ||
| return try await withThrowingTaskGroup(of: [Int].self) { group in | ||
| var uploadedMediaIDs: [Int] = [] | ||
| let maxConcurrentTasks = 3 | ||
| var urlIterator = fileURLs.makeIterator() | ||
|
|
||
| for _ in 0..<maxConcurrentTasks { | ||
| guard let url = urlIterator.next() else { break } | ||
| group.addTask { | ||
| let data = try Data(contentsOf: url) | ||
| let entity = ImageUploadEntity(data: data, format: data.detectedImageFormat) | ||
| return try await repository.upload(items: [entity], mediaType: mediaType) | ||
| } | ||
| } | ||
|
|
||
| for try await resultIDs in group { | ||
| uploadedMediaIDs.append(contentsOf: resultIDs) | ||
|
|
||
| guard let nextURL = urlIterator.next() else { continue } | ||
| group.addTask { | ||
| let data = try Data(contentsOf: nextURL) | ||
| let entity = ImageUploadEntity(data: data, format: data.detectedImageFormat) | ||
| return try await repository.upload(items: [entity], mediaType: mediaType) | ||
| } | ||
| } | ||
|
|
||
| return uploadedMediaIDs | ||
| } | ||
| }, | ||
|
|
There was a problem hiding this comment.
p1
제 해석이 틀린 부분 있다면 알려주세요!!
메모리 혹은 여타 성능적인 최적화를 위해 한 번에 실행하는 작업의 개수를 3개로 제한해두고 반복자 패턴으로 작업을 추가해주면서 진행되는 것으로 보입니다. 반복자 패턴의 쓰임새를 눈으로 확인할 수 있어서 매우 좋았는데요, 한 가지 뭔가 걸리는 부분은 매 작업마다 레포지토리의 upload를 호출하고 있는 게 조금 의아하게 느껴집니다!
repository.upload 는 한 번에 여러 개의 ImageUploadEntity를 받아서 한 번에 PresignedURL들을 발급받아 S3 업로드를 진행하게 설계했습니다!! 즉, 10개의 이미지를 업로드 한다고 하면 10개의 엔티티를 upload 요청 1번으로 수행할 수 있는데요, 지금 구조는 매 작업마다 PresignedURL을 발급받고 S3 업로드 하는 과정을 반복하고 있습니다. 그래서 이미지 10개를 업로드 하려면 PresignedURL 발급 요청을 10번 별개로 보내고 있는 것 같아 보여요.
그래서 아래의 convert 케이스처럼 ImageUploadEntity 들을 배열로 지닌 다음 한 번의 repository.upload 요청을 보내는 구조로 바꾸는 건 어떻게 생각하시는지 궁금합니다!
이런 식으로요!
uploadConcurrentlyFromURLs: { fileURLs, mediaType in
try await withThrowingTaskGroup(of: ImageUploadEntity?.self) { group in
var entities: [ImageUploadEntity] = []
let maxConcurrentTasks = 3
var urlIterator = fileURLs.makeIterator()
for _ in 0..<maxConcurrentTasks {
guard let url = urlIterator.next() else { break }
group.addTask {
let data = try Data(contentsOf: url)
return ImageUploadEntity(data: data, format: data.detectedImageFormat)
}
}
for try await entity in group {
guard let entity else { continue }
entities.append(entity)
guard let nextURL = urlIterator.next() else { continue }
group.addTask {
let data = try Data(contentsOf: nextURL)
return ImageUploadEntity(data: data, format: data.detectedImageFormat)
}
}
return try await repository.upload(items: entities, mediaType: mediaType)
}
},
또한 repository.upload 메서드의 반환 타입이 우리 서버로 업로드 된 이미지의 id 리스트이기 때문에
uploadedMediaIDs 를 별도로 들고 리턴에 사용할 필요 없이 바로 repository.upload 를 리턴으로 반환해도 될 것 같습니다!!
There was a problem hiding this comment.
이거 코딩하다가 로직이 의도한 바와 달라져서 조금 잘못됐네요.
최대 10장의 이미지를 한번에 Data로 불러왔을 때, 최악의 경우 메모리 부족으로 앱크래시 날까봐 3장씩 쪼개서 업로드를 하고, 업로드가 끝난 하나의 작업은 즉시 메모리에서 해제시켜서 메모리 부족 상황이 오지 않도록 하려는 거였어요.
쉽게 말하면 3장, 3장, 3장, 1장으로 쪼개서, 4개 그룹에 대한 이미지들의 처리는 Repository에서 병렬적으로 업로드 수행하되, 그룹끼리는 순차적으로 처리입니다. 이렇게 최대 호출횟수 4회 안에 업로드를 따로따로 진행하는게 목표였습니다.
원래 의도에 맞도록 로직을 살짝 수정하겠습니다.
그와 별개로 위의 OOM으로 인한 크래시를 막기 위한 방어코드를 쓰면서도 결과를 한번에 반환하기 위해서는 uploadedMediaIDs 로컬 변수에 누적해야합니다.
There was a problem hiding this comment.
아 10장의 데이터를 메모리에서 쥐고 있지 않는 걸 의도하신거군요
이해했습니다!
ㅇㄴ 그럼 기존 업로드 쪽이랑은 작동방식이 달라지는 군. 나중에 통일하는 게 좋으려나
| import UniformTypeIdentifiers | ||
| import os | ||
|
|
||
| public struct FileManagerSharedImageRepository { |
There was a problem hiding this comment.
p3
우리 컨벤션인 Default 접두사보다
FileManager를 사용하는 클라이언트이니까 FileManagerSharedImageRepository 가 더 명확하다고 판단해서 요렇게 네이밍 하신 걸까요?!
There was a problem hiding this comment.
정확합니다. 다른 Repository는 구현기반이 변경될 예정이 없었는데, 이 친구는 FileManager 외의 다른 영속성 모델로도 구현될 수도 있을 것 같다고 판단해서 네이밍했습니다.
그러한 구현계획이 현재로선 없으므로 Default 접두사를 붙여 이름을 다시 지을까요?
| private func sharedImageURLs(for appGroupID: String) throws -> [URL] { | ||
| guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: appGroupID) else { return [] } | ||
| let sharedDirectoryURL = groupURL.appendingPathComponent("SharedImages", conformingTo: .directory) | ||
|
|
||
| guard fileManager.fileExists(atPath: sharedDirectoryURL.path()) else { return [] } | ||
| return try fileManager.contentsOfDirectory(at: sharedDirectoryURL, includingPropertiesForKeys: nil) | ||
| } |
| protocol ImageShareUseCase { | ||
| func extractImageProviders(from extensionItems: [NSExtensionItem]) -> [NSItemProvider] | ||
| func fetchPreviewData(from provider: NSItemProvider) async throws -> Data | ||
| func share(providers: [NSItemProvider]) async throws | ||
| } |
There was a problem hiding this comment.
p3
client가 아닌 usecase 형태를 채택하셨는데, 이유가 있을까요???
아직 클라이언트, 유즈케이스에 대한 이해가 부족해서 사용 이유와 판단법 및 근거 등등 알고 싶음...
그리고 Processor 라는 구조도 종종 사용하시던데 이런 거 정하는 기준 같은 게 따로 있으실까요??? 난 오로지 유즈케이스, 클라이언트밖에 몰라
There was a problem hiding this comment.
개념 자체는 다 똑같다고 생각합니다.
그래서 그걸 특정한 고정형이 있는 것으로 인식하면 더 어려워질 것 같다는 생각이 드네요... (제가 그랬듯이요)
비즈니스 로직을 표현하는 타입이라는 점에서 모두 동일하구요. 네이밍 자체에 대한 나름의 기준이 있냐고 물으시면, 음... 여기 공유 확장 타겟에는 TCA가 없으니까 Client는 더 헷갈리게 만들거같아서 제외, Processor라고 하기에는 저수준 프레임워크에 의존하거나 복잡한 로직은 없음(이미지 다운샘플링 프로세서를 예시로 들게요!) 그래서 제외... 그러다가 문득 유즈케이스라는 네이밍이 번뜩 들어서 걍 그걸로 지엇음요
| } | ||
|
|
||
| func setupLayout() { | ||
| NSLayoutConstraint.activate([ |
There was a problem hiding this comment.
p3
와 맨날 스냅킷만 써서 NSLayoutConstraint 되게 오랜만에 보네
There was a problem hiding this comment.
스냅킷 붐따
나약한자들의 라이브러리 흥칫
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift (1)
36-46: 정리 작업 결과 요약 로깅 추가 고려현재 구현은 개별 파일 삭제 실패 시에도 계속 진행하는데, 이는 정리 로직으로서 적절한 접근입니다. 다만 디버깅과 모니터링을 위해 정리 작업의 전체 결과(성공/실패 개수)를 요약 로깅하면 운영 시 유용할 수 있습니다.
📊 정리 결과 요약 로깅 제안
public func clearSharedImages(appGroupID: String) async throws { let fileURLs = try sharedImageURLs(for: appGroupID) + var failedCount = 0 for url in fileURLs { do { try fileManager.removeItem(at: url) } catch { + failedCount += 1 Logger.data.error("파일 정리 실패: \(url.lastPathComponent) - \(error.localizedDescription)") } } + + if failedCount > 0 { + Logger.data.warning("공유 이미지 정리 완료: \(fileURLs.count - failedCount)/\(fileURLs.count) 성공") + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift` around lines 36 - 46, Enhance clearSharedImages(appGroupID:) to record and log a summary of the cleanup outcome: keep counters for total files, successful deletions, and failures while iterating over URLs returned by sharedImageURLs(for:), incrementing successes in the try block and failures in the catch (which should still log per-file errors as currently done), then after the loop emit a single summary log via Logger.data (e.g., include appGroupID, total count, successes, failures) so operators can quickly see overall cleanup results; keep existing per-file error logging unchanged and reference the clearSharedImages method, sharedImageURLs(for:), fileManager.removeItem(at:), and Logger.data in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift`:
- Around line 19-25: In sharedImageURLs(for:) don’t silently return [] when
fileManager.containerURL(forSecurityApplicationGroupIdentifier:) is nil—replace
the silent failure with a thrown/descriptive error (or at least a logged error)
so App Group misconfiguration is distinguishable from “no images”; also remove
the fileExists(atPath:) pre-check to avoid TOCTOU and instead call
fileManager.contentsOfDirectory(at:includingPropertiesForKeys:) directly and
handle errors: if the error is “directory not found” return [] otherwise rethrow
or propagate the error. Use the function name sharedImageURLs, and the calls
containerURL(forSecurityApplicationGroupIdentifier:), fileExists(atPath:), and
contentsOfDirectory(at:includingPropertiesForKeys:) to locate where to change
behavior.
---
Nitpick comments:
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift`:
- Around line 36-46: Enhance clearSharedImages(appGroupID:) to record and log a
summary of the cleanup outcome: keep counters for total files, successful
deletions, and failures while iterating over URLs returned by
sharedImageURLs(for:), incrementing successes in the try block and failures in
the catch (which should still log per-file errors as currently done), then after
the loop emit a single summary log via Logger.data (e.g., include appGroupID,
total count, successes, failures) so operators can quickly see overall cleanup
results; keep existing per-file error logging unchanged and reference the
clearSharedImages method, sharedImageURLs(for:), fileManager.removeItem(at:),
and Logger.data in your changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b40ac03f-91e4-4706-8846-6cefe16ebcfe
📒 Files selected for processing (3)
Neki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swiftNeki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swiftNeki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swift
✅ Files skipped from review due to trivial changes (1)
- Neki-iOS/Features/Archive/Sources/Domain/Sources/Interfaces/Repositories/SharedImageRepository.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Neki-iOS/Core/Sources/ImagePicker/Domain/ImageUploadClient.swift
| private func sharedImageURLs(for appGroupID: String) throws -> [URL] { | ||
| guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: appGroupID) else { return [] } | ||
| let sharedDirectoryURL = groupURL.appendingPathComponent("SharedImages", conformingTo: .directory) | ||
|
|
||
| guard fileManager.fileExists(atPath: sharedDirectoryURL.path()) else { return [] } | ||
| return try fileManager.contentsOfDirectory(at: sharedDirectoryURL, includingPropertiesForKeys: nil) | ||
| } |
There was a problem hiding this comment.
App Group 컨테이너 접근 실패 시 조용한 실패 처리
Line 20에서 containerURL이 nil인 경우 빈 배열을 반환하고 있습니다. App Group이 잘못 구성되었거나 entitlements 설정 오류가 있을 때 디버깅이 어려워질 수 있습니다. 실제 구성 오류와 단순히 공유된 이미지가 없는 상황을 구분하기 어렵습니다.
또한 Line 23-24에서 fileExists 체크와 contentsOfDirectory 호출 사이에 TOCTOU(Time-of-Check to Time-of-Use) 경쟁 조건이 발생할 수 있습니다. 디렉토리가 존재하지 않을 때 contentsOfDirectory가 던지는 에러를 직접 처리하는 것이 더 안전합니다.
🛡️ 제안하는 개선안
private func sharedImageURLs(for appGroupID: String) throws -> [URL] {
- guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: appGroupID) else { return [] }
+ guard let groupURL = fileManager.containerURL(forSecurityApplicationGroupIdentifier: appGroupID) else {
+ Logger.data.error("App Group 컨테이너 접근 실패: \(appGroupID)")
+ return []
+ }
let sharedDirectoryURL = groupURL.appendingPathComponent("SharedImages", conformingTo: .directory)
- guard fileManager.fileExists(atPath: sharedDirectoryURL.path()) else { return [] }
- return try fileManager.contentsOfDirectory(at: sharedDirectoryURL, includingPropertiesForKeys: nil)
+ do {
+ return try fileManager.contentsOfDirectory(at: sharedDirectoryURL, includingPropertiesForKeys: nil)
+ } catch CocoaError.fileReadNoSuchFile, CocoaError.fileNoSuchFile {
+ return []
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Neki-iOS/Features/Archive/Sources/Data/Sources/FileManagerSharedImageRepository.swift`
around lines 19 - 25, In sharedImageURLs(for:) don’t silently return [] when
fileManager.containerURL(forSecurityApplicationGroupIdentifier:) is nil—replace
the silent failure with a thrown/descriptive error (or at least a logged error)
so App Group misconfiguration is distinguishable from “no images”; also remove
the fileExists(atPath:) pre-check to avoid TOCTOU and instead call
fileManager.contentsOfDirectory(at:includingPropertiesForKeys:) directly and
handle errors: if the error is “directory not found” return [] otherwise rethrow
or propagate the error. Use the function name sharedImageURLs, and the calls
containerURL(forSecurityApplicationGroupIdentifier:), fileExists(atPath:), and
contentsOfDirectory(at:includingPropertiesForKeys:) to locate where to change
behavior.
2 participants
🌴 작업한 브랜치
✅ 작업한 내용
개요
Share Extension을 활용하여 외부 앱(Photos 등)에서 Neki 메인 앱으로 네컷사진을 안전하게 공유하고 S3에 업로드하는 기능을 추가합니다.
이 기능은 Clean Architecture에 기반한 관심사 분리 원칙 등에 의거 작성되었으며, 한 번에 저장할 수 있는 최대 이미지 개수 10장 등 주요 비즈니스 규칙을 동일하게 적용하여 개발되었습니다.
메인 앱의 다양한 생명주기(Terminated, Suspended 등)와 로그인 상태 등 엣지 케이스를 고려하여 DeepLink를 통해 안정적으로 이미지 공유 시퀀스를 제공합니다.
1. 타겟 및 컨테이너 설정
Share Extension 타겟 추가와 App Group Capabilities를 추가하고 애플 디벨로퍼스에 적용한 주요 식별자(Debug, Release 각각)를 똑같이 적용했습니다. 단 공유 확장과 메인 앱은 두 개의 스키마로 분리가 되어있기 때문에 Capabilities 추가에 따른 Entitlements도 분리해주어야 합니다. 다음의 스크린샷처럼 각 타겟에서 스키마별 Entitlements 파일 위치를 알려주고, 실제 Entitlements 파일 각각에 대해 식별자를 작성해주면 됩니다.
2. Share Extension 주요 구현부
Share Extension 및 App Group을 사용해 샌드박스를 넘어 외부 앱으로부터 Neki 메인 앱으로 이미지를 공유받기 위해서는 중간 공유저장소가 필요합니다. 그 이유는 단순 문자열이나 가벼운 자료라면 DeepLink 자체에 내용을 싣거나 해도 무방하지만 이미지 자료는 때때로 아주 무거울 수 있기 때문입니다. 따라서 UserDefaults는 무거운 자료를 담기에 부적절하고 CoreData는 과하다고 판단, FileManager를 공유저장소로 채택했습니다.
공유 대상은 오직 '이미지'로 제한되어야 하므로 Share-Extension의 Info.plist를 꼭 설정하여야 합니다!
NSExtensionActivationRule또한 스토리보드 없이 코드베이스로 UI를 구현했으므로, 스토리보드 관련 내용을 제거하고 ShareViewController를 사용하도록 운영체제에 알려주어야 합니다.
NSExtensionPrincipleClass이미지 공유 시퀀스는 간단하게 아래와 같이 진행됩니다.
주요 타입
이미지 원본 자체(Data)를 복사하는 것은 비효율적이며 메모리 스파이크로 인한 크래시를 유발할 수 있으므로, URL을 저장하는 방식을 택했습니다. 왜냐하면 이미지 축소, 병렬처리 등의 안전장치는 메인 앱에서 이미 제공하고 있기 떄문에 공유 시트 자체는 URL만을 다루고 실제로 메모리가 우려되는 부분은 메인 앱에서 이미 검증된 다운샘플링 로직에 해결을 기대하는 방식입니다.
메인 앱 라우팅 및 사용자 경험 처리
SLComposeServiceViewController를 상속받아 공유 시트를 구현할 경우, 개발 리소스는 가장 적을 수 있으나 불필요한 UI 요소(텍스트필드 등)가 있어UIViewController를 상속해 직접 커스터마이징 하였습니다. 오랜만에 오토레이아웃을 잡으면서 왜 제가 SwiftUI를 주력기로 삼게 되었는지 다시 느낄 수 있던 시간이었습니다.❗️PR Point
📸 스크린샷
톡방에 올린 시연자료로 대체합니다!
📟 관련 이슈
Summary by CodeRabbit
New Features
Chores