-
Notifications
You must be signed in to change notification settings - Fork 1
Fixed Directory Enumeration #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Aligns directory enumeration with WebDAV semantics and NextcloudKit conventions, and refactors metadata conversion to reliably identify the enumerated directory.
- Replace toDirectoryReadMetadatas with toSendableDirectoryMetadata and introduce isDirectoryToRead to correctly identify the target directory among NKFile entries.
- Normalize root handling to use NextcloudKit.shared.nkCommonInstance.rootFileName and switch tests to davFilesUrl.
- Update mock interfaces and tests to reflect the new behavior and expectations.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift | Adds isDirectoryToRead and toSendableDirectoryMetadata; adjusts root fix-up in toItemMetadata; introduces an actor container for concurrent conversion. |
| Sources/NextcloudFileProviderKit/Enumeration/Enumerator+SyncEngine.swift | Switches to new conversion API and improves logging; ensures directory metadata is inserted for DB updates. |
| Sources/NextcloudFileProviderKit/Item/Item+Create.swift | Uses toSendableDirectoryMetadata during item creation; updates logging and variables. |
| Tests/NextcloudFileProviderKitTests/NKFileExtensionTests.swift | Updates and adds tests for root handling, directory identification, and new conversion API. |
| Tests/Interface/MockRemoteInterfaceTests.swift | Aligns expectations with NextcloudKit root naming and WebDAV path. |
| Tests/Interface/MockRemoteItem.swift | Adjusts root name and NKFile mapping; potential regression in serverUrl mapping for non-root items. |
| Tests/InterfaceTests/MockRemoteInterfaceTests.swift | Imports NextcloudKit and updates assertions for root file naming. |
| Tests/NextcloudFileProviderKitTests/RemoteChangeObserverTests.swift | Uses davFilesUrl in test setup and formats a closure for readability. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift
Outdated
Show resolved
Hide resolved
Sources/NextcloudFileProviderKit/Extensions/NKFile+Extensions.swift
Outdated
Show resolved
Hide resolved
- Our corporate instance was a real world use case in which the retrieved metadata of a directory and its content is no longer fulfilling the previous assumption that the first item is related to the queried directory itself. Instead, it can be anywhere in the array. Hence the implementation had to be changed to detect it by other means while taking the special `__NC_ROOT__` case into consideration. - While reading, I also applied code style changes based on conventional Swift code style. - Further, I hope to improve clarity by renaming some symbols like `DirectoryReadConversionActor` to `DirectoryMetadataContainer` which hints its purpose a lot better in my opinion. - Also replaced some occurrences of empty strings or `__NC_ROOT__` literals with the symbol reference to NextcloudKit. Signed-off-by: Iva Horn <142165879+i2h3@users.noreply.github.com>
41b603d to
a6d75d3
Compare
__NC_ROOT__case into consideration.DirectoryReadConversionActortoDirectoryMetadataContainerwhich hints its purpose a lot better in my opinion.__NC_ROOT__literals with the symbol reference to NextcloudKit.