Skip to content

Ensure directory URL enumerator error handler block is invoked with non-nil URL #5491

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

Merged
merged 2 commits into from
Oct 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apinotes/Foundation.apinotes
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,9 @@ Classes:
- Selector: 'URLForPublishingUbiquitousItemAtURL:expirationDate:error:'
SwiftName: url(forPublishingUbiquitousItemAt:expiration:)
MethodKind: Instance
- Selector: 'enumeratorAtURL:includingPropertiesForKeys:options:errorHandler:'
MethodKind: Instance
SwiftPrivate: true
- Name: NSFileVersion
Methods:
- Selector: 'versionOfItemAtURL:forPersistentIdentifier:'
Expand Down
19 changes: 19 additions & 0 deletions stdlib/public/SDK/Foundation/FileManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ internal func NS_Swift_NSFileManager_replaceItemAtURL_withItemAtURL_backupItemNa
_ options: FileManager.ItemReplacementOptions,
_ error: NSErrorPointer) -> NSURL?

@_silgen_name("NS_Swift_NSFileManager_enumeratorAt_includingPropertiesForKeys_options_errorHandler")
internal func NS_Swift_NSFileManager_enumeratorAt_includingPropertiesForKeys_options_errorHandler(
_ self_ : AnyObject,
_ url: AnyObject,
_ keys: NSArray?,
_ options: FileManager.DirectoryEnumerationOptions,
_ errorHandler: @escaping @convention(block) (NSURL, NSError) -> Bool) -> FileManager.DirectoryEnumerator?

extension FileManager {
/*
Expand All @@ -45,4 +52,16 @@ extension FileManager {
}
throw error!
}

@available(OSX 10.6, iOS 4.0, *)
public func enumerator(at url: URL, includingPropertiesForKeys keys: [URLResourceKey]?, options mask: FileManager.DirectoryEnumerationOptions = [], errorHandler handler: ((URL, Error) -> Bool)? = nil) -> FileManager.DirectoryEnumerator? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, this should probably be marked final and @nonobjc (per discussion with @parkera).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to tackle that later (it would be a source breaking change at this point I believe).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we at least get it right on new methods?

return NS_Swift_NSFileManager_enumeratorAt_includingPropertiesForKeys_options_errorHandler(self, url as NSURL, keys as NSArray?, mask, { (url, error) in
var errorResult = true;
if let h = handler {
errorResult = h(url as URL, error)
}
return errorResult
})
}

}
28 changes: 28 additions & 0 deletions stdlib/public/SDK/Foundation/FileManagerThunks.m
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,31 @@
[backupItemName release];
return success ? [result retain] : nil;
}

extern /*"C"*/ NS_RETURNS_RETAINED id
NS_Swift_NSFileManager_enumeratorAt_includingPropertiesForKeys_options_errorHandler(
NSFileManager *NS_RELEASES_ARGUMENT _Nonnull self_,
NSURL *NS_RELEASES_ARGUMENT _Nonnull url,
NSArray *NS_RELEASES_ARGUMENT _Nullable keys,
NSUInteger options,
BOOL (^_Nonnull errorHandler)(NSURL * _Nonnull url, NSError * _Nonnull error) ) {

NSDirectoryEnumerator *result = [self_ enumeratorAtURL:url
includingPropertiesForKeys:keys
options:(NSDirectoryEnumerationOptions)options
errorHandler:^(NSURL * url, NSError *error) {

NSURL *realURL = url ?: error.userInfo[NSURLErrorKey];
if (!realURL) {
NSString *path = error.userInfo[NSFilePathErrorKey];
realURL = [NSURL fileURLWithPath:path];
}
return errorHandler(realURL, error);

}];
[self_ release];
[url release];
[keys release];
[errorHandler release];
return [result retain];
}
56 changes: 56 additions & 0 deletions test/stdlib/TestFileManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,68 @@ class TestFileManager : TestFileManagerSuper {
expectTrue(threw, "Should have thrown")

}

func testDirectoryEnumerator_error() {
let fm = FileManager.default
let nonexistantURL = URL(fileURLWithPath: "\(NSTemporaryDirectory())/nonexistant")

var invoked = false
let e = fm.enumerator(at: nonexistantURL, includingPropertiesForKeys: []) { (url, err) in
invoked = true
expectEqual(nonexistantURL, url)
expectEqual((err as NSError).code, NSFileReadNoSuchFileError)
return true
}

let url = e?.nextObject()
expectTrue(invoked)
expectTrue(url == nil)

}

func testDirectoryEnumerator_error_noHandler() {
let fm = FileManager.default
let nonexistantURL = URL(fileURLWithPath: "\(NSTemporaryDirectory())/nonexistant")

let e = fm.enumerator(at: nonexistantURL, includingPropertiesForKeys: [])
let url = e?.nextObject()
expectTrue(url == nil)

}

func testDirectoryEnumerator_simple() {
let fm = FileManager.default
let dirPath = (NSTemporaryDirectory() as NSString).appendingPathComponent(NSUUID().uuidString)
try! fm.createDirectory(atPath: dirPath, withIntermediateDirectories: true, attributes: nil)
defer { try! FileManager.default.removeItem(atPath: dirPath) }

let item1 = URL(fileURLWithPath: "\(dirPath)/1", isDirectory: false)
let item2 = URL(fileURLWithPath: "\(dirPath)/2", isDirectory: false)

try! Data().write(to: item1)
try! Data().write(to: item2)

let e = fm.enumerator(at: URL(fileURLWithPath: dirPath, isDirectory: true), includingPropertiesForKeys: [])
let result1 = e?.nextObject()
let result2 = e?.nextObject()
let result3 = e?.nextObject()

// Avoid potential symlink discrepancy between the result and the original URL
expectEqual((result1! as! URL).lastPathComponent, item1.lastPathComponent)
expectEqual((result2! as! URL).lastPathComponent, item2.lastPathComponent)
expectTrue(result3 == nil)

}

}

#if !FOUNDATION_XCTEST
var FMTests = TestSuite("TestFileManager")
FMTests.test("testReplaceItem") { TestFileManager().testReplaceItem() }
FMTests.test("testReplaceItem_error") { TestFileManager().testReplaceItem_error() }
FMTests.test("testDirectoryEnumerator_error") { TestFileManager().testDirectoryEnumerator_error() }
FMTests.test("testDirectoryEnumerator_error_noHandler") { TestFileManager().testDirectoryEnumerator_error_noHandler() }
FMTests.test("testDirectoryEnumerator_simple") { TestFileManager().testDirectoryEnumerator_simple() }

runAllTests()
#endif
Expand Down