diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift index cb677534fa1c9..656151ce4856a 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Create.swift @@ -205,8 +205,13 @@ public extension Item { account: account.ncKitAccount, classFile: "", // Placeholder as not set in original code contentType: contentType, - creationDate: Date(), // Default as not set in original code - date: date ?? Date(), + // Prefer the locally-known dates the system handed us (and which we + // just sent to the server) over the second-precision values echoed + // back in the PUT response. Aligning these with what's on disk keeps + // NSDocument-style editors from seeing the file as "changed by + // another application" right after they save it. + creationDate: (itemTemplate.creationDate as? Date) ?? date ?? Date(), + date: (itemTemplate.contentModificationDate as? Date) ?? date ?? Date(), directory: false, e2eEncrypted: false, // Default as not set in original code etag: etag ?? "", diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift index 68f10bd343ad2..c6ccabed76b31 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Item/Item+Modify.swift @@ -200,7 +200,12 @@ public extension Item { var newMetadata = dbManager.setStatusForItemMetadata(updatedMetadata, status: .normal) ?? SendableItemMetadata(value: updatedMetadata) - newMetadata.date = date ?? Date() + // Prefer the mtime we just handed to the server (and which is on disk) + // over the truncated, second-precision value the PUT response carries + // in `Last-Modified`. Aligning `metadata.date` with what's on disk keeps + // NSDocument-style editors (Xcode, TextEdit, …) from seeing the file as + // "changed by another application" right after they save it. + newMetadata.date = newContentModificationDate ?? date ?? metadata.date newMetadata.etag = etag ?? metadata.etag newMetadata.ocId = ocId newMetadata.size = size ?? 0 diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift index 0da2d92b3bd72..1c83397b82380 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/Interface/MockRemoteInterface.swift @@ -575,6 +575,18 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable { /// Track the number of read operations performed public var readOperationCount: Int = 0 + /// When non-nil, the upload mock truncates the modification date returned to + /// the caller to this precision (in seconds) — simulating Nextcloud's + /// `Last-Modified` IMF-fixdate, which is second-precision even when we sent + /// `X-OC-Mtime` with sub-second precision. The stored server-side mtime + /// stays precise; only the response value is degraded. `nil` keeps the + /// echo-back behaviour every existing test relies on. + public var uploadResponseMtimeTruncation: TimeInterval? + + /// When `true`, the upload mock returns `nil` for the response date, + /// exercising the locally-known-mtime fallback in the production code. + public var uploadReturnsNilDate: Bool = false + /// Handler to track enumerate calls public var enumerateCallHandler: ((String, EnumerateDepth, Bool, [String], Data?, Account, NKRequestOptions, @escaping (URLSessionTask) -> Void) -> Void)? @@ -795,11 +807,23 @@ public class MockRemoteInterface: RemoteInterface, @unchecked Sendable { print("Created item \(item.name)") } + let responseDate: NSDate? = { + if uploadReturnsNilDate { + return nil + } + if let truncation = uploadResponseMtimeTruncation, truncation > 0 { + let raw = item.modificationDate.timeIntervalSince1970 + let truncated = (raw / truncation).rounded(.down) * truncation + return Date(timeIntervalSince1970: truncated) as NSDate + } + return item.modificationDate as NSDate + }() + return ( account.ncKitAccount, item.identifier, item.versionIdentifier, - item.modificationDate as NSDate, + responseDate, item.size, nil, .success diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift index 91db6777fef5d..9f3e8b9540938 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemCreateTests.swift @@ -136,6 +136,118 @@ final class ItemCreateTests: NextcloudFileProviderKitTestCase { XCTAssertTrue(createdItem.isUploaded) } + /// Regression test for the same root cause as + /// `testModifyFilePreservesLocalContentModificationDateWhenServerTruncates`, + /// but on the create path: when the server truncates the upload-response + /// mtime to 1 s precision, the returned `Item.creationDate` and + /// `Item.contentModificationDate` must reflect the template's sub-second + /// values rather than the truncated server response. + func testCreateFilePreservesLocalDatesWhenServerTruncates() async throws { + let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + remoteInterface.uploadResponseMtimeTruncation = 1.0 + + let preciseCreationDate = Date(timeIntervalSince1970: 1_747_564_300.123456) + let preciseMtime = Date(timeIntervalSince1970: 1_747_564_337.456789) + + var fileItemMetadata = SendableItemMetadata( + ocId: "file-precise-dates", fileName: "file-precise-dates", account: Self.account + ) + fileItemMetadata.classFile = NKTypeClassFile.document.rawValue + fileItemMetadata.creationDate = preciseCreationDate + fileItemMetadata.date = preciseMtime + + let tempUrl = FileManager.default.temporaryDirectory + .appendingPathComponent("file-precise-dates") + try Data("Hello, precise dates!".utf8).write(to: tempUrl) + + let fileItemTemplate = Item( + metadata: fileItemMetadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + let (createdItemMaybe, error) = await Item.create( + basedOn: fileItemTemplate, + contents: tempUrl, + account: Self.account, + remoteInterface: remoteInterface, + progress: Progress(), + dbManager: Self.dbManager, + log: FileProviderLogMock() + ) + XCTAssertNil(error) + let createdItem = try XCTUnwrap(createdItemMaybe) + + XCTAssertEqual(createdItem.creationDate, preciseCreationDate) + XCTAssertEqual(createdItem.contentModificationDate, preciseMtime) + + let truncatedMtime = Date( + timeIntervalSince1970: preciseMtime.timeIntervalSince1970.rounded(.down) + ) + XCTAssertNotEqual(truncatedMtime, preciseMtime) + XCTAssertNotEqual(createdItem.contentModificationDate, truncatedMtime) + + let dbItem = try XCTUnwrap( + Self.dbManager.itemMetadata(ocId: createdItem.itemIdentifier.rawValue) + ) + XCTAssertEqual(dbItem.creationDate, preciseCreationDate) + XCTAssertEqual(dbItem.date, preciseMtime) + } + + /// Defensive coverage for the secondary fallback: if the system passes a + /// template whose `creationDate` / `contentModificationDate` are nil (an + /// `NSFileProviderItem` not built from one of our `Item` instances), the + /// fix must fall through to the server's response date rather than + /// silently substituting `Date()` (the previous behaviour, which would + /// anchor the new metadata to the current wall clock for no good reason). + func testCreateFileFallsBackToServerDateWhenTemplateHasNilDates() async throws { + let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + + let template: NSFileProviderItem = MockFileProviderItem( + identifier: .init("file-nil-dates"), + filename: "file-nil-dates", + isUploaded: false + ) + // MockFileProviderItem doesn't declare creationDate / contentModificationDate, so + // both flow through the @objc-optional protocol's default-nil. That's exactly the + // shape this test wants: an item from outside our `Item` type where the system + // simply didn't furnish those fields. + XCTAssertNil(template.creationDate ?? nil, "Test setup expects template.creationDate to be nil") + XCTAssertNil(template.contentModificationDate ?? nil, "Test setup expects template.contentModificationDate to be nil") + + let tempUrl = FileManager.default.temporaryDirectory + .appendingPathComponent("file-nil-dates") + try Data("Hello, nil dates!".utf8).write(to: tempUrl) + + let beforeUpload = Date() + let (createdItemMaybe, error) = await Item.create( + basedOn: template, + contents: tempUrl, + account: Self.account, + remoteInterface: remoteInterface, + progress: Progress(), + dbManager: Self.dbManager, + log: FileProviderLogMock() + ) + let afterUpload = Date() + XCTAssertNil(error) + let createdItem = try XCTUnwrap(createdItemMaybe) + + // The mock echoes the (defaulted-to-now) mtime back as the response, + // so the resulting dates should equal the server-derived value rather + // than a second invocation of `Date()` taken after the response. We + // can't pin to an exact value (the mock initialises with `Date()` when + // the template hands it nil), but it must land within the upload + // window — and `creationDate` and `contentModificationDate` must agree + // with each other. + let creationDate = try XCTUnwrap(createdItem.creationDate) + let modificationDate = try XCTUnwrap(createdItem.contentModificationDate) + XCTAssertEqual(creationDate, modificationDate, "Both dates should derive from the same server response value") + XCTAssertGreaterThanOrEqual(creationDate, beforeUpload) + XCTAssertLessThanOrEqual(creationDate, afterUpload) + } + func testCreateFileIntoFolder() async throws { let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) diff --git a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift index 5d49fb182c220..6239c89865713 100644 --- a/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift +++ b/shell_integration/MacOSX/NextcloudFileProviderKit/Tests/NextcloudFileProviderKitTests/ItemModifyTests.swift @@ -156,6 +156,75 @@ final class ItemModifyTests: NextcloudFileProviderKitTestCase { ) } + /// Real Nextcloud servers echo the upload's `X-OC-Mtime` back via a + /// `Last-Modified` IMF-fixdate, which collapses to 1 s precision. APFS stores + /// sub-second mtimes, so if the file provider trusted the server's response + /// the returned `Item.contentModificationDate` would drift down by hundreds + /// of milliseconds — and AppKit/NSDocument-based editors (Xcode, TextEdit) + /// would interpret that drift as "the item has been changed on disk by + /// another application" right after their own save. + /// Regression test for that scenario: configure the mock to truncate the + /// upload response date to 1 s precision, save a sub-second mtime, and + /// verify the resulting `Item` and the persisted metadata both preserve the + /// exact local value rather than the truncated one. + func testModifyFilePreservesLocalContentModificationDateWhenServerTruncates() async throws { + let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem) + remoteInterface.uploadResponseMtimeTruncation = 1.0 + + let itemMetadata = remoteItem.toItemMetadata(account: Self.account) + Self.dbManager.addItemMetadata(itemMetadata) + + let newContents = "Hello, New World!".data(using: .utf8) + let newContentsUrl = FileManager.default.temporaryDirectory + .appendingPathComponent("modify-precise-mtime") + try newContents?.write(to: newContentsUrl) + + // Pick a sub-second value the mock will truncate down to `…17.000`. + let preciseLocalMtime = Date(timeIntervalSince1970: 1_747_564_337.456789) + var targetItemMetadata = SendableItemMetadata(value: itemMetadata) + targetItemMetadata.date = preciseLocalMtime + targetItemMetadata.size = try Int64(XCTUnwrap(newContents?.count)) + + let item = Item( + metadata: itemMetadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + + let targetItem = Item( + metadata: targetItemMetadata, + parentItemIdentifier: .rootContainer, + account: Self.account, + remoteInterface: remoteInterface, + dbManager: Self.dbManager + ) + + let (modifiedItemMaybe, error) = await item.modify( + itemTarget: targetItem, + changedFields: [.contents, .contentModificationDate], + contents: newContentsUrl, + dbManager: Self.dbManager + ) + XCTAssertNil(error) + let modifiedItem = try XCTUnwrap(modifiedItemMaybe) + + XCTAssertEqual(modifiedItem.contentModificationDate, preciseLocalMtime) + + // Belt-and-braces: also confirm we did *not* take the mock server's + // truncated value. If this is ever equal to the precise value the test + // is moot, so guard against that too. + let truncatedResponse = Date( + timeIntervalSince1970: preciseLocalMtime.timeIntervalSince1970.rounded(.down) + ) + XCTAssertNotEqual(truncatedResponse, preciseLocalMtime) + XCTAssertNotEqual(modifiedItem.contentModificationDate, truncatedResponse) + + let persisted = try XCTUnwrap(Self.dbManager.itemMetadata(ocId: itemMetadata.ocId)) + XCTAssertEqual(persisted.date, preciseLocalMtime) + } + func testModifyFolder() async throws { let remoteInterface = MockRemoteInterface(account: Self.account, rootItem: rootItem)