fix(file-provider): Ensure path comparisons are done on equal normalization

[claucambra]

Resolves

#10087

(though the deletion issue is unrelated and I see no way in which it is related)

Summary

macOS generally uses NFD normalisation, whereas the server uses NFC normalisation. This causes issues when performing path comparisons involving special characters (e.g. acute accents) or in certain languages (e.g. Korean).

This PR ensures we normalise server URLs and filenames consistently

Checklist

• Sign-off message is added to all commits.
• The commit history is clean with no merge commits.
  • How to rebase your commits
  • How to squash commits with rebase
• Uploaded screenshots from before and after for UI changes.
• Test(s) added to branch, with before/after results included in PR description, for performance improvements where applicable.
• Documentation has been updated or is not required.
• Backports requested where applicable (critical bugfixes).
• Labels added where applicable (bug/enhancement).
• Milestone added for target version.

AI (if applicable)

• The content of this PR was partly or fully generated using AI
  • I have read the guidelines for AI-assisted contributions.
  • I have read Nextcloud's AI Contribution Policy.

[claucambra]

/backport to stable-34.0 please

[i2h3]

I vaguely remember the problem of different Unicode representations from many years ago. 😅 I had to deal with that in exactly this context as well.

In general, the account no longer is a discriminator and does not need to be considered in code. I intended to remove it over time from internal APIs. Every file provider domain, process and Realm has its own scope restricted to a single account.

[i2h3]

I will let Claude review this as well and post the results once they arrived.

[Copilot]

Pull request overview

This PR addresses Unicode normalization mismatches (NFC vs NFD) in the macOS File Provider extension by introducing canonicalized comparison keys for remote locations and updating Realm queries/comparisons to use canonical-equivalent matching, with added regression tests and documentation.

Changes:

• Add persisted canonical (NFC) location keys (normalizedServerUrl, normalizedFileName) to RealmItemMetadata, including schema migration and centralized query helpers.
• Update FilesDatabaseManager queries and location comparisons to use canonical-equivalent matching (via normalized keys and ItemMetadata.hasSameLocation(as:)).
• Add regression tests and DocC design notes covering canonical-equivalence behavior and deduplication semantics.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/RemoteChangePropagationTests.swift	Updates test metadata setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/PathBoundaryPrefixTests.swift	Refactors test setup to use updateLocation(...) for consistent normalized fields.
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/MoveSafeDeletionTests.swift	Refactors test metadata setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/LockTokenInvalidationTests.swift	Refactors test metadata setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/KeepDownloadedRecursiveTests.swift	Refactors test metadata setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemPropertyTests.swift	Refactors test metadata setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemMetadataTests.swift	Adds canonical-equivalence comparison test for hasSameLocation(as:).
shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/FilesDatabaseManagerTests.swift	Adds canonical-path lookup and deduplication behavior tests; refactors setup to use updateLocation(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/RealmItemMetadata.swift	Adds persisted normalized location keys and updateLocation(...); updates equality and init to use canonical location comparison.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Metadata/ItemMetadata.swift	Adds hasSameLocation(as:) helper for canonical-equivalent in-memory comparisons.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Documentation.docc/UnicodePathNormalization.md	Adds DocC design note documenting Unicode normalization model and usage boundaries.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Documentation.docc/Documentation.md	Links the new Unicode normalization design note from package docs.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/SchemaVersion.swift	Bumps Realm schema version for canonical location keys.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/RealmItemMetadata+Queries.swift	Introduces centralized Realm query helpers for canonical-equivalent location/serverUrl matching.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Trash.swift	Updates trash queries to use canonical-aware hasServerUrl(...).
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Directories.swift	Updates directory/descendant queries and rename propagation to use canonical-aware helpers and keep normalized fields in sync.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager+Deduplication.swift	Updates logical-duplicate eviction and startup cleanup to use canonical-aware location keys.
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Database/FilesDatabaseManager.swift	Updates schema version, performs migration backfill of normalized fields, and switches remote-path lookup/comparisons to canonical-aware logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[i2h3]

Note: the review below was produced with Claude Code (Opus 4.8) as an extra machine-assisted pass and is shared by me as input for manual review. I've read through it and it looks sound, but the analysis and wording are Claude's, not mine — please treat it as a second opinion to sanity-check rather than a hand-written review.

Summary

Nicely scoped, well-documented fix. The approach is the right one: keep the raw serverUrl/fileName for all server I/O and store separate NFC comparison keys (normalizedServerUrl/normalizedFileName), then compare identity on the normalized keys. Using canonical mapping (precomposedStringWithCanonicalMapping, i.e. NFC) rather than compatibility mapping is the correct choice — it won't fold genuinely-distinct names together. The updateLocation() funnel, the DocC design note, and the migration+dedup handling are all careful work.

Two things to flag up front:

• CI: the File Provider test job passes, but Lint (swiftformat) is failing and blocks merge — see Client side encryption v2 #1.
• One pre-existing normalization gap in the working-set scan (Update README #2) is left untouched and is worth closing here or as a fast-follow, since it's the same NFD/NFC hazard this PR is about.

What's good

• Raw values preserved for server requests; normalized keys used only for local identity.
• normalizedServerUrl is indexed, so the migrated queries are actually faster than the old raw serverUrl comparisons (raw serverUrl was never indexed).
• Migration 202→203 backfills all rows before cleanupPreexistingLogicalDuplicates() runs.
• Dedup carefully protects in-flight rows and keeps the newest settled row; raw spelling of the survivor is preserved.
• Genuinely good UnicodePathNormalization.md design note.

---

🔴 Blocking

1. swiftformat indent failure — merge-blocking
Database/FilesDatabaseManager.swift:360-368 (the let existingMetadatas = database / .objects / .where chain in updateItemMetadatas). The chained calls sit at 12 spaces (aligned with let) while the closing brace is at 16; swiftformat's indent rule wants the chain at +4. This is the single file failing Lint. Fix: run swiftformat on the file (formatting only, no logic change).

---

🟠 Worth addressing

2. Working-set scan still compares raw paths across a normalization boundary — medium
Enumeration/Enumerator+WorkingSetScan.swift:181-185

let childHasMaterialisedDescendant = materialisedItems.contains {
    $0.ocId != childDirectory.ocId
        && ($0.serverUrl == childPath || $0.serverUrl.hasPrefix(childPath + "/"))
}

Here childPath = childDirectory.remotePath() is server-derived (NFC), while materialisedItems[].serverUrl is the DB-stored raw value (as received — possibly NFD). This is exactly the cross-boundary mismatch the PR fixes for DB queries, but this in-memory check was not converted. If a materialised descendant is persisted NFD and the changed child directory's path is NFC, childHasMaterialisedDescendant is false, the changed directory is not enqueued, and its changed descendants are never scanned — a dropped remote change. Suggest canonicalizing both operands (mirror ItemMetadata.hasSameLocation). The sibling check at lines ~202-205 has the same raw pattern but is DB-vs-DB and benign in direction — worth fixing for consistency only. (This file isn't in the diff, so it's reasonable to treat as a fast-follow — but it's the same bug class.)

3. The 202→203 backfill migration is untested — medium
Database/FilesDatabaseManager.swift:90-100. There is no test that opens a pre-203 DB and asserts normalizedServerUrl/normalizedFileName are populated. A test that pins a config to schema 202, inserts a row with NFD raw fields and empty normalized fields, reopens under 203, and asserts the NFC backfill would cover the version gate, the guard-let extraction, and the exact subscript key strings.

---

🟡 Low / consider

4. cleanupPreexistingLogicalDuplicates() runs on every DB open, not just after migration — low
FilesDatabaseManager.swift:111 calls it unconditionally after every try Realm(). It enumerates all non-deleted/non-lock/non-root rows and builds a full bucket dictionary (three fresh strings per row) before the collisions.isEmpty early-return — so the O(n) scan runs every launch even on long-clean DBs; only the write transaction is skipped. The doc comment calls it a "one-shot startup pass," but it's every-open. Inside the memory-constrained extension with large accounts this is avoidable — consider gating on "migration actually occurred" (thread oldSchemaVersion out, or persist a done-marker).

5. Distinct NFC/NFD server files get collapsed locally on upgrade — low (document as known limitation)
Nextcloud on Linux is byte-preserving and does not enforce NFC, so a folder can legitimately contain two canonically-equivalent-but-byte-different names. On upgrade, cleanupPreexistingLogicalDuplicates() buckets by normalized key and soft-deletes the loser. Two mitigations make this acceptable: (a) it's a local tombstone only — no server-side delete is issued, and (b) APFS/macOS can't represent both siblings anyway (that's the " 2"-suffix problem this PR fixes). Worth a one-line comment near the bucketing so future readers know the collapse is intentional and server files are untouched.

6. Test coverage gaps beyond the migration one (#3) — low

• Startup cleanup is only tested on byte-identical rows; the NFC/NFD case only exercises the evictLogicalDuplicates path, not cleanupPreexistingLogicalDuplicates (different code path). Add a startup test seeding the same file NFC vs NFD via updateLocation (no manual normalized override) and assert the older row is soft-deleted.
• No NFD coverage for trash / directory-descendant / rename-propagation queries (all use ASCII paths).
• RealmItemMetadata.isEqual now folds in serverUrl, but the change is untested; hasSameLocation has only one positive case (fileName-only). Add a negative case and a serverUrl-normalization case.

7. Empty-normalized raw fallback in the query helpers — low
Database/RealmItemMetadata+Queries.swift:18,20,34-35,39. Given the invariant that normalizedX == "" ⇔ rawX == "" (updateLocation + migration always populate), the (normalizedX == "" && rawX == query) branch only fires for empty-serverUrl rows, where branch 1 already matches — so it's largely redundant, and for a hypothetical un-normalized NFD row it compares raw(NFD)==query(NFC) and wouldn't match anyway. It does still provide defense-in-depth for rows built without updateLocation (the test suite does exactly this), so it's not strictly dead — but a short comment clarifying its scope would help.

---

🔵 Nits

• account is compared raw in LogicalKey / hasLocation while paths are normalized. Safe direction (a mismatch causes a missed merge, never a false merge); a one-line comment on the intentional asymmetry would be enough.
• isEqual overridden (now including serverUrl) without a matching hash override — pre-existing and harmless (the type isn't used as a hashed key), but worth a comment, or override hash over the same fields if it's ever put in a Set.
• Several startup-cleanup tests call updateLocation(...) and then redundantly re-assign normalizedServerUrl/normalizedFileName to the identical precomposed values (FilesDatabaseManagerTests.swift ~1766-1767, 1775-1776, 1809-1810, 1818-1819, 1849-1850, 1857-1858, 1866-1867, 1896-1897, 1906-1907). updateLocation already sets these — the extra lines can be dropped.

---

Checked and found safe (no action needed)

To save re-review, these raw comparisons were examined and are not normalization gaps — both sides are machine-constructed or server/sentinel values with no user-supplied path component: FilesDatabaseManager.parentItemIdentifierFromMetadata root/trash checks (:683-685), Enumerator+SyncEngine root detection (:188-190), NKFile+Extensions root fixup (:31,132), and Upload.swift chunk-id comparison (:225).

[claucambra]

I believe the points for Claude's comments are addressed

[i2h3]

Have you run swiftformat . in shell_integration/MacOSX/NextcloudFileProviderKit? That should fix the failing lint action.

[claucambra]

Have you run swiftformat . in shell_integration/MacOSX/NextcloudFileProviderKit? That should fix the failing lint action.

thanks :) now done

[github-actions]

Artifact containing the AppImage: nextcloud-appimage-pr-10443.zip

Digest: sha256:a2babd32950b54c6c8fdadc02f64f5502b5fa0c102fc787c7472fc3ce141ae15

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud