chore: replace repository dependencies with use cases in Files view models - WPB-26913#4986
chore: replace repository dependencies with use cases in Files view models - WPB-26913#4986jullianm wants to merge 4 commits into
Conversation
jullianm
changed the title
Test Results315 tests 315 ✅ 1m 47s ⏱️ Results for commit 12e3159. ♻️ This comment has been updated with latest results. Summary: workflow run #29003514028 |
|
| return fileURL | ||
| } | ||
|
|
||
| @MainActor |
There was a problem hiding this comment.
question: why does this need to be main actor?
| package import Combine | ||
| package import Foundation | ||
|
|
||
| @MainActor |
There was a problem hiding this comment.
question: also why does this need to be main actor? I would expect that the view model is main actor but use cases are concurrent.
samwyndham
left a comment
samwyndham
left a comment
There was a problem hiding this comment.
Just a few questions before approving.
| /// Fetches nodes for a particular configuration, and mutating the injected WireDriveNodesCollection. | ||
| package final class WireDriveFetchNodesUseCase: Sendable { | ||
| @MainActor | ||
| package final class WireDriveFetchNodesUseCase { |
There was a problem hiding this comment.
question: It is a shame to remove the Sendable here. Isn't there a way we can keep it as sendable?
| } | ||
|
|
||
| package func clearNodes() { | ||
| state = WireDriveNodesCollection() |
There was a problem hiding this comment.
question: Could we clear the nodes from the collection instead of resetting state here? That way state can remain a let.
3 participants
Issue
FilesViewModel,FilesItemViewModel, and theMoveToFolderview models held direct references toWireDriveNodesRepositoryProtocol/WireDriveLocalAssetRepositoryProtocolalongside their use cases, and the use case wiring forFilesViewContainer,RecycleBinContainer, andWireMessagingFactorywas duplicated verbatim across all three call sites. This PR removes the repository dependencies from the view models so they only talk to use cases, and centralizes use case construction.Technical approach:
FilesViewModel.DependenciesandFilesViewModel.makeUseCases(dependencies:)to build the fullUseCasesstruct in one place, replacing the copy-pasted initializer previously duplicated inFilesViewContainer,RecycleBinContainer, andWireMessagingFactory.localAssetRepositoryandnodesRepositoryfromFilesViewModel; it now depends solely onUseCases.WireDriveFetchNodesPageUseCasenow receives itsconfigurationper call (invoke(configuration:...)) instead of at init, sinceFilesViewModelcomputes the configuration dynamically fromisBrowsing/isRecycleBin/navigationPath.WireDriveFetchNodesUseCaseis now@MainActor, exposes anodesCombine publisher (backed by its internalWireDriveNodesCollection), and adds aclearNodes()method to reset and restart observation — used byMoveToFolderPageViewModelinstead of owning its ownWireDriveNodesCollection.WireDriveObserveAssetUseCaseand a synchronousWireDriveGetAssetUseCase.asset(nodeID:), replacing directlocalAssetRepository.observeAsset/assetcalls inFilesItemViewModel.MoveToFolderViewModel/MoveToFolderPageViewModelnow take an injectedmoveNodeUseCase/fetchNodesUseCaseinstead of building their own repository-backed instances per navigation step.#if DEBUGdeleteAsset()helper inFilesItemViewModelthat reached into the repository directly.No behavior change intended; this is a dependency-direction cleanup ahead of further refactoring.
Testing
WireDriveFetchNodesPageUseCaseTests,FilesViewModelTests,FilesItemViewModelTests,FilesBrowserViewTests,FilesViewTests).WireMessagingtest target and manually exercise Files browsing, recycle bin, and Move to Folder flows to confirm listing, search, and offline-asset state still behave as before.Checklist
[WPB-XXX].UI accessibility checklist
No UI changes in this PR — internal dependency refactor only.