From 0eb941ecec4ab91be4d49b267672b6579fcd5e87 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 19 Feb 2014 06:55:53 -0800 Subject: [PATCH] Fixes issue #8: Async methods return faulted tasks now instead of throwing. This is done by adding the 'async' keyword to all their method signatures. The compiler then automatically catches all thrown exceptions and returns a faulted task instead. Fixes issue #9: After argument validation, the next step of any async method that is otherwise synchronously implemented is to call a special method that will switch the method off its caller's thread if the caller was on their Main thread (as detected by the presence of a SynchronizationContext). This way the synchronous I/O available on that platform doesn't block the app's responsiveness, which makes the behavior consistent across all platforms and remains true to the caller's expectations since the method returns a Task. Note we don't have to do this when the platform's I/O stack is already async. As part of testing these changes, I tested passing null into methods that accept strings. I found several methods that didn't throw at all in this invalid case, or that threw the more general ArgumentException. So I added a convenient Requires class and made sure that all methods that accept string arguments call it so that the most appropriate exception type is thrown. Finally, I added tests for the method I use that switches off the caller's main thread. It only works on desktop since it requires actually setting the SynchronizationContext, which smaller framework profiles disallow (grrrr). All tests pass on all platforms. --- .../PCLStorage.Android.csproj | 6 ++ .../DesktopFileSystem.cs | 24 +++--- .../FileSystemFile.cs | 47 ++++------- .../FileSystemFolder.cs | 62 +++++++------- .../PCLStorage.FileSystem.Desktop.csproj | 6 ++ src/PCLStorage.IsoStore.SL/IsoStoreFile.cs | 40 +++------ .../IsoStoreFileSystem.cs | 23 +++--- src/PCLStorage.IsoStore.SL/IsoStoreFolder.cs | 54 +++++++------ .../PCLStorage.IsoStore.SL.csproj | 6 ++ ...Extensions.cs => AwaitExtensions.WinRT.cs} | 3 +- src/PCLStorage.WinRT/PCLStorage.WinRT.csproj | 8 +- src/PCLStorage.WinRT/WinRTFile.cs | 23 ++---- src/PCLStorage.WinRT/WinRTFileSystem.cs | 4 + src/PCLStorage.WinRT/WinRTFolder.cs | 15 ++-- .../PCLStorage.WindowsPhone.csproj | 10 ++- src/PCLStorage.iOS/PCLStorage.iOS.csproj | 6 ++ src/PCLStorage/AwaitExtensions.cs | 72 +++++++++++++++++ src/PCLStorage/PCLStorage.csproj | 2 + src/PCLStorage/Requires.cs | 77 ++++++++++++++++++ .../AwaitExtensionsTest.cs | 81 +++++++++++++++++++ .../PCLStorage.Test.Desktop.csproj | 4 + 21 files changed, 412 insertions(+), 161 deletions(-) rename src/PCLStorage.WinRT/{AwaitExtensions.cs => AwaitExtensions.WinRT.cs} (93%) create mode 100644 src/PCLStorage/AwaitExtensions.cs create mode 100644 src/PCLStorage/Requires.cs create mode 100644 test/PCLStorage.Test.Desktop/AwaitExtensionsTest.cs diff --git a/src/PCLStorage.Android/PCLStorage.Android.csproj b/src/PCLStorage.Android/PCLStorage.Android.csproj index 53b9ff6..126f521 100644 --- a/src/PCLStorage.Android/PCLStorage.Android.csproj +++ b/src/PCLStorage.Android/PCLStorage.Android.csproj @@ -55,6 +55,9 @@ FileSystemFolder.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -64,6 +67,9 @@ PortablePath.cs + + Requires.cs + diff --git a/src/PCLStorage.FileSystem.Desktop/DesktopFileSystem.cs b/src/PCLStorage.FileSystem.Desktop/DesktopFileSystem.cs index 833e5a9..efffdc8 100644 --- a/src/PCLStorage.FileSystem.Desktop/DesktopFileSystem.cs +++ b/src/PCLStorage.FileSystem.Desktop/DesktopFileSystem.cs @@ -58,15 +58,18 @@ public IFolder RoamingStorage /// The path to a file, as returned from the property. /// The cancellation token. /// A file for the given path, or null if it does not exist. - public Task GetFileFromPathAsync(string path, CancellationToken cancellationToken) + public async Task GetFileFromPathAsync(string path, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - IFile ret = null; + Requires.NotNullOrEmpty(path, "path"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + if (File.Exists(path)) { - ret = new FileSystemFile(path); + return new FileSystemFile(path); } - return Task.FromResult(ret); + + return null; } /// @@ -75,16 +78,17 @@ public Task GetFileFromPathAsync(string path, CancellationToken cancellat /// The path to a folder, as returned from the property. /// The cancellation token. /// A folder for the specified path, or null if it does not exist. - public Task GetFolderFromPathAsync(string path, CancellationToken cancellationToken) + public async Task GetFolderFromPathAsync(string path, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - IFolder ret = null; + Requires.NotNullOrEmpty(path, "path"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); if (Directory.Exists(path)) { - ret = new FileSystemFolder(path, true); + return new FileSystemFolder(path, true); } - return Task.FromResult(ret); + return null; } } } diff --git a/src/PCLStorage.FileSystem.Desktop/FileSystemFile.cs b/src/PCLStorage.FileSystem.Desktop/FileSystemFile.cs index 15fc99d..720316e 100644 --- a/src/PCLStorage.FileSystem.Desktop/FileSystemFile.cs +++ b/src/PCLStorage.FileSystem.Desktop/FileSystemFile.cs @@ -51,39 +51,38 @@ public string Path /// Specifies whether the file should be opened in read-only or read/write mode /// The cancellation token. /// A which can be used to read from or write to the file - public Task OpenAsync(FileAccess fileAccess, CancellationToken cancellationToken) + public async Task OpenAsync(FileAccess fileAccess, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - Stream ret; + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + if (fileAccess == FileAccess.Read) { - ret = File.OpenRead(Path); + return File.OpenRead(Path); } else if (fileAccess == FileAccess.ReadAndWrite) { - ret = File.Open(Path, FileMode.Open, System.IO.FileAccess.ReadWrite); + return File.Open(Path, FileMode.Open, System.IO.FileAccess.ReadWrite); } else { throw new ArgumentException("Unrecognized FileAccess value: " + fileAccess); } - return Task.FromResult(ret); } /// /// Deletes the file /// /// A task which will complete after the file is deleted. - public Task DeleteAsync(CancellationToken cancellationToken) + public async Task DeleteAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + if (!File.Exists(Path)) { throw new PCLStorage.Exceptions.FileNotFoundException("File does not exist: " + Path); } + File.Delete(Path); - - return Task.FromResult(true); } /// @@ -95,18 +94,11 @@ public Task DeleteAsync(CancellationToken cancellationToken) /// /// A task which will complete after the file is renamed. /// - public Task RenameAsync(string newName, NameCollisionOption collisionOption, CancellationToken cancellationToken) + public async Task RenameAsync(string newName, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newName == null) - { - throw new ArgumentNullException("newName"); - } - else if (newName.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newName, "newName"); - return MoveAsync(PortablePath.Combine(System.IO.Path.GetDirectoryName(_path), newName), collisionOption, cancellationToken); + await MoveAsync(PortablePath.Combine(System.IO.Path.GetDirectoryName(_path), newName), collisionOption, cancellationToken); } /// @@ -118,16 +110,11 @@ public Task RenameAsync(string newName, NameCollisionOption collisionOption, Can /// /// A task which will complete after the file is moved. /// - public Task MoveAsync(string newPath, NameCollisionOption collisionOption, CancellationToken cancellationToken) + public async Task MoveAsync(string newPath, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newPath == null) - { - throw new ArgumentNullException("newPath"); - } - else if (newPath.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newPath, "newPath"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); string newDirectory = System.IO.Path.GetDirectoryName(newPath); string newName = System.IO.Path.GetFileName(newPath); @@ -165,7 +152,7 @@ public Task MoveAsync(string newPath, NameCollisionOption collisionOption, Cance File.Move(_path, candidatePath); _path = candidatePath; _name = candidateName; - return Task.FromResult(true); + return; } } } diff --git a/src/PCLStorage.FileSystem.Desktop/FileSystemFolder.cs b/src/PCLStorage.FileSystem.Desktop/FileSystemFolder.cs index 9722452..a415262 100644 --- a/src/PCLStorage.FileSystem.Desktop/FileSystemFolder.cs +++ b/src/PCLStorage.FileSystem.Desktop/FileSystemFolder.cs @@ -64,9 +64,11 @@ public string Path /// Specifies how to behave if the specified file already exists /// The cancellation token. /// The newly created file - public Task CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) + public async Task CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(desiredName, "desiredName"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string nameToUse = desiredName; @@ -110,7 +112,7 @@ public Task CreateFileAsync(string desiredName, CreationCollisionOption o } var ret = new FileSystemFile(newPath); - return Task.FromResult(ret); + return ret; } void InternalCreateFile(string path) @@ -127,28 +129,29 @@ void InternalCreateFile(string path) /// The name of the file to get /// The cancellation token. /// The requested file, or null if it does not exist - public Task GetFileAsync(string name, CancellationToken cancellationToken) + public async Task GetFileAsync(string name, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + string path = System.IO.Path.Combine(Path, name); if (!File.Exists(path)) { throw new PCLStorage.Exceptions.FileNotFoundException("File does not exist: " + path); } var ret = new FileSystemFile(path); - return Task.FromResult(ret); + return ret; } /// /// Gets a list of the files in this folder /// /// A list of the files in the folder - public Task> GetFilesAsync(CancellationToken cancellationToken) + public async Task> GetFilesAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); IList ret = Directory.GetFiles(Path).Select(f => new FileSystemFile(f)).ToList().AsReadOnly(); - return Task.FromResult(ret); + return ret; } /// @@ -158,9 +161,10 @@ public Task> GetFilesAsync(CancellationToken cancellationToken) /// Specifies how to behave if the specified folder already exists /// The cancellation token. /// The newly created folder - public Task CreateFolderAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) + public async Task CreateFolderAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(desiredName, "desiredName"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string nameToUse = desiredName; string newPath = System.IO.Path.Combine(Path, nameToUse); @@ -200,7 +204,7 @@ public Task CreateFolderAsync(string desiredName, CreationCollisionOpti } var ret = new FileSystemFolder(newPath, true); - return Task.FromResult(ret); + return ret; } /// @@ -209,28 +213,30 @@ public Task CreateFolderAsync(string desiredName, CreationCollisionOpti /// The name of the folder to get /// The cancellation token. /// The requested folder, or null if it does not exist - public Task GetFolderAsync(string name, CancellationToken cancellationToken) + public async Task GetFolderAsync(string name, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(name, "name"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); string path = System.IO.Path.Combine(Path, name); if (!Directory.Exists(path)) { throw new PCLStorage.Exceptions.DirectoryNotFoundException("Directory does not exist: " + path); } var ret = new FileSystemFolder(path, true); - return Task.FromResult(ret); + return ret; } /// /// Gets a list of subfolders in this folder /// /// A list of subfolders in the folder - public Task> GetFoldersAsync(CancellationToken cancellationToken) + public async Task> GetFoldersAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); IList ret = Directory.GetDirectories(Path).Select(d => new FileSystemFolder(d, true)).ToList().AsReadOnly(); - return Task.FromResult(ret); + return ret; } /// @@ -241,26 +247,23 @@ public Task> GetFoldersAsync(CancellationToken cancellationToken) /// /// A task whose result is the result of the existence check. /// - public Task CheckExistsAsync(string name, CancellationToken cancellationToken) + public async Task CheckExistsAsync(string name, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(name)) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(name, "name"); - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); string checkPath = PortablePath.Combine(this.Path, name); if (File.Exists(checkPath)) { - return Task.FromResult(ExistenceCheckResult.FileExists); + return ExistenceCheckResult.FileExists; } else if (Directory.Exists(checkPath)) { - return Task.FromResult(ExistenceCheckResult.FolderExists); + return ExistenceCheckResult.FolderExists; } else { - return Task.FromResult(ExistenceCheckResult.NotFound); + return ExistenceCheckResult.NotFound; } } @@ -268,16 +271,15 @@ public Task CheckExistsAsync(string name, CancellationToke /// Deletes this folder and all of its contents /// /// A task which will complete after the folder is deleted - public Task DeleteAsync(CancellationToken cancellationToken) + public async Task DeleteAsync(CancellationToken cancellationToken) { if (!_canDelete) { throw new IOException("Cannot delete root storage folder."); } - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); Directory.Delete(Path, true); - return Task.FromResult(true); } void EnsureExists() diff --git a/src/PCLStorage.FileSystem.Desktop/PCLStorage.FileSystem.Desktop.csproj b/src/PCLStorage.FileSystem.Desktop/PCLStorage.FileSystem.Desktop.csproj index 792f1ca..e5186db 100644 --- a/src/PCLStorage.FileSystem.Desktop/PCLStorage.FileSystem.Desktop.csproj +++ b/src/PCLStorage.FileSystem.Desktop/PCLStorage.FileSystem.Desktop.csproj @@ -56,6 +56,9 @@ Properties\CommonAssemblyInfo.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -65,6 +68,9 @@ PortablePath.cs + + Requires.cs + diff --git a/src/PCLStorage.IsoStore.SL/IsoStoreFile.cs b/src/PCLStorage.IsoStore.SL/IsoStoreFile.cs index 67e7b49..9b3d814 100644 --- a/src/PCLStorage.IsoStore.SL/IsoStoreFile.cs +++ b/src/PCLStorage.IsoStore.SL/IsoStoreFile.cs @@ -76,9 +76,9 @@ public string Path /// Specifies whether the file should be opened in read-only or read/write mode /// The cancellation token. /// A which can be used to read from or write to the file - public Task OpenAsync(FileAccess fileAccess, CancellationToken cancellationToken) + public async Task OpenAsync(FileAccess fileAccess, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); System.IO.FileAccess nativeFileAccess; if (fileAccess == FileAccess.Read) { @@ -96,7 +96,7 @@ public Task OpenAsync(FileAccess fileAccess, CancellationToken cancellat try { IsolatedStorageFileStream stream = _root.OpenFile(Path, FileMode.Open, nativeFileAccess, FileShare.Read); - return TaskEx.FromResult(stream); + return stream; } catch (IsolatedStorageException ex) { @@ -125,9 +125,9 @@ public Task OpenAsync(FileAccess fileAccess, CancellationToken cancellat /// Deletes the file /// /// A task which will complete after the file is deleted. - public Task DeleteAsync(CancellationToken cancellationToken) + public async Task DeleteAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); try { #if WINDOWS_PHONE @@ -161,7 +161,6 @@ public Task DeleteAsync(CancellationToken cancellationToken) throw; } } - return TaskEx.FromResult(true); } /// @@ -173,20 +172,13 @@ public Task DeleteAsync(CancellationToken cancellationToken) /// /// A task which will complete after the file is renamed. /// - public Task RenameAsync(string newName, NameCollisionOption collisionOption, CancellationToken cancellationToken) + public async Task RenameAsync(string newName, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newName == null) - { - throw new ArgumentNullException("newName"); - } - else if (newName.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newName, "newName"); - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); string newPath = PortablePath.Combine(System.IO.Path.GetDirectoryName(_path), newName); - return MoveAsync(newPath, collisionOption, cancellationToken); + await MoveAsync(newPath, collisionOption, cancellationToken); } /// @@ -198,17 +190,11 @@ public Task RenameAsync(string newName, NameCollisionOption collisionOption, Can /// /// A task which will complete after the file is moved. /// - public Task MoveAsync(string newPath, NameCollisionOption collisionOption, CancellationToken cancellationToken) + public async Task MoveAsync(string newPath, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newPath == null) - { - throw new ArgumentNullException(); - } - else if (newPath.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newPath, "newPath"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); string newDirectory = System.IO.Path.GetDirectoryName(newPath); string newName = System.IO.Path.GetFileName(newPath); @@ -245,7 +231,7 @@ public Task MoveAsync(string newPath, NameCollisionOption collisionOption, Cance _root.MoveFile(_path, candidatePath); _path = candidatePath; _name = candidateName; - return TaskEx.FromResult(true); + return; } } } diff --git a/src/PCLStorage.IsoStore.SL/IsoStoreFileSystem.cs b/src/PCLStorage.IsoStore.SL/IsoStoreFileSystem.cs index 813bcf7..34a1414 100644 --- a/src/PCLStorage.IsoStore.SL/IsoStoreFileSystem.cs +++ b/src/PCLStorage.IsoStore.SL/IsoStoreFileSystem.cs @@ -53,15 +53,16 @@ public IFolder RoamingStorage /// The path to a file, as returned from the property. /// The cancellation token. /// A file for the given path, or null if it does not exist. - public Task GetFileFromPathAsync(string path, CancellationToken cancellationToken) + public async Task GetFileFromPathAsync(string path, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - IFile ret = null; + Requires.NotNullOrEmpty(path, "path"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + if (Root.FileExists(path)) { - ret = new IsoStoreFile(Root, path); + return new IsoStoreFile(Root, path); } - return TaskEx.FromResult(ret); + return null; } @@ -71,15 +72,17 @@ public Task GetFileFromPathAsync(string path, CancellationToken cancellat /// The path to a folder, as returned from the property. /// The cancellation token. /// A folder for the specified path, or null if it does not exist. - public Task GetFolderFromPathAsync(string path, CancellationToken cancellationToken) + public async Task GetFolderFromPathAsync(string path, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); - IFolder ret = null; + Requires.NotNullOrEmpty(path, "path"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); + if (Root.DirectoryExists(path)) { - ret = new IsoStoreFolder(Root, path); + return new IsoStoreFolder(Root, path); } - return TaskEx.FromResult(ret); + + return null; } } } diff --git a/src/PCLStorage.IsoStore.SL/IsoStoreFolder.cs b/src/PCLStorage.IsoStore.SL/IsoStoreFolder.cs index c8e1e35..8d784ed 100644 --- a/src/PCLStorage.IsoStore.SL/IsoStoreFolder.cs +++ b/src/PCLStorage.IsoStore.SL/IsoStoreFolder.cs @@ -95,9 +95,10 @@ public string Path /// Specifies how to behave if the specified file already exists /// The cancellation token. /// The newly created file - public Task CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) + public async Task CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(desiredName, "desiredName"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string nameToUse = desiredName; string path = System.IO.Path.Combine(Path, nameToUse); @@ -139,7 +140,7 @@ public Task CreateFileAsync(string desiredName, CreationCollisionOption o } var ret = new IsoStoreFile(nameToUse, this); - return TaskEx.FromResult(ret); + return ret; } void InternalCreateFile(string path) @@ -155,9 +156,10 @@ void InternalCreateFile(string path) /// The name of the file to get /// The cancellation token. /// The requested file, or null if it does not exist - public Task GetFileAsync(string name, CancellationToken cancellationToken) + public async Task GetFileAsync(string name, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(name, "name"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string path = System.IO.Path.Combine(Path, name); @@ -166,21 +168,21 @@ public Task GetFileAsync(string name, CancellationToken cancellationToken throw new PCLStorage.Exceptions.FileNotFoundException("File does not exist: " + path); } var ret = new IsoStoreFile(name, this); - return TaskEx.FromResult(ret); + return ret; } /// /// Gets a list of the files in this folder /// /// A list of the files in the folder - public Task> GetFilesAsync(CancellationToken cancellationToken) + public async Task> GetFilesAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string[] fileNames = Root.GetFileNames(System.IO.Path.Combine(Path, "*.*")); IList ret = fileNames.Select(fn => new IsoStoreFile(fn, this)).Cast().ToList().AsReadOnly(); - return TaskEx.FromResult(ret); + return ret; } /// @@ -192,7 +194,9 @@ public Task> GetFilesAsync(CancellationToken cancellationToken) /// The newly created folder public async Task CreateFolderAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(desiredName, "desiredName"); + + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string nameToUse = desiredName; @@ -242,9 +246,10 @@ public async Task CreateFolderAsync(string desiredName, CreationCollisi /// The name of the folder to get /// The cancellation token. /// The requested folder, or null if it does not exist - public Task GetFolderAsync(string name, CancellationToken cancellationToken) + public async Task GetFolderAsync(string name, CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + Requires.NotNullOrEmpty(name, "name"); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string path = System.IO.Path.Combine(Path, name); @@ -253,21 +258,21 @@ public Task GetFolderAsync(string name, CancellationToken cancellationT throw new PCLStorage.Exceptions.DirectoryNotFoundException("Directory does not exist: " + path); } var ret = new IsoStoreFolder(name, this); - return TaskEx.FromResult(ret); + return ret; } /// /// Gets a list of subfolders in this folder /// /// A list of subfolders in the folder - public Task> GetFoldersAsync(CancellationToken cancellationToken) + public async Task> GetFoldersAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); string[] folderNames = Root.GetDirectoryNames(System.IO.Path.Combine(Path, "*")); IList ret = folderNames.Select(fn => new IsoStoreFolder(fn, this)).Cast().ToList().AsReadOnly(); - return TaskEx.FromResult(ret); + return ret; } /// @@ -278,26 +283,23 @@ public Task> GetFoldersAsync(CancellationToken cancellationToken) /// /// A task whose result is the result of the existence check. /// - public Task CheckExistsAsync(string name, CancellationToken cancellationToken) + public async Task CheckExistsAsync(string name, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(name)) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(name, "name"); - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); var checkPath = PortablePath.Combine(Path, name); if (Root.FileExists(checkPath)) { - return TaskEx.FromResult(ExistenceCheckResult.FileExists); + return ExistenceCheckResult.FileExists; } else if (Root.DirectoryExists(checkPath)) { - return TaskEx.FromResult(ExistenceCheckResult.FolderExists); + return ExistenceCheckResult.FolderExists; } else { - return TaskEx.FromResult(ExistenceCheckResult.NotFound); + return ExistenceCheckResult.NotFound; } } @@ -307,7 +309,7 @@ public Task CheckExistsAsync(string name, CancellationToke /// A task which will complete after the folder is deleted public async Task DeleteAsync(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); + await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken); EnsureExists(); if (string.IsNullOrEmpty(Path)) diff --git a/src/PCLStorage.IsoStore.SL/PCLStorage.IsoStore.SL.csproj b/src/PCLStorage.IsoStore.SL/PCLStorage.IsoStore.SL.csproj index c170b22..b39cbaa 100644 --- a/src/PCLStorage.IsoStore.SL/PCLStorage.IsoStore.SL.csproj +++ b/src/PCLStorage.IsoStore.SL/PCLStorage.IsoStore.SL.csproj @@ -97,6 +97,9 @@ Properties\CommonAssemblyInfo.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -106,6 +109,9 @@ PortablePath.cs + + Requires.cs + diff --git a/src/PCLStorage.WinRT/AwaitExtensions.cs b/src/PCLStorage.WinRT/AwaitExtensions.WinRT.cs similarity index 93% rename from src/PCLStorage.WinRT/AwaitExtensions.cs rename to src/PCLStorage.WinRT/AwaitExtensions.WinRT.cs index 6e22716..eff2193 100644 --- a/src/PCLStorage.WinRT/AwaitExtensions.cs +++ b/src/PCLStorage.WinRT/AwaitExtensions.WinRT.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -11,7 +12,7 @@ namespace PCLStorage /// /// Extensions for use internally by PCLStorage for awaiting. /// - internal static class AwaitExtensions + internal static partial class AwaitExtensions { /// /// Returns a task that completes when the async operation completes, diff --git a/src/PCLStorage.WinRT/PCLStorage.WinRT.csproj b/src/PCLStorage.WinRT/PCLStorage.WinRT.csproj index cec24c1..1b617eb 100644 --- a/src/PCLStorage.WinRT/PCLStorage.WinRT.csproj +++ b/src/PCLStorage.WinRT/PCLStorage.WinRT.csproj @@ -106,6 +106,9 @@ Properties\CommonAssemblyInfo.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -115,7 +118,10 @@ PortablePath.cs - + + Requires.cs + + diff --git a/src/PCLStorage.WinRT/WinRTFile.cs b/src/PCLStorage.WinRT/WinRTFile.cs index 3946b65..e93ff9a 100644 --- a/src/PCLStorage.WinRT/WinRTFile.cs +++ b/src/PCLStorage.WinRT/WinRTFile.cs @@ -78,9 +78,10 @@ public async Task OpenAsync(FileAccess fileAccess, CancellationToken can /// Deletes the file /// /// A task which will complete after the file is deleted. - public Task DeleteAsync(CancellationToken cancellationToken) + public async Task DeleteAsync(CancellationToken cancellationToken) { - return _wrappedFile.DeleteAsync().AsTask(cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); + await _wrappedFile.DeleteAsync().AsTask(cancellationToken).ConfigureAwait(false); } /// @@ -94,14 +95,7 @@ public Task DeleteAsync(CancellationToken cancellationToken) /// public async Task RenameAsync(string newName, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newName == null) - { - throw new ArgumentNullException("newName"); - } - else if (newName.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newName, "newName"); try { @@ -129,14 +123,7 @@ public async Task RenameAsync(string newName, NameCollisionOption collisionOptio /// public async Task MoveAsync(string newPath, NameCollisionOption collisionOption, CancellationToken cancellationToken) { - if (newPath == null) - { - throw new ArgumentNullException("newPath"); - } - else if (newPath.Length == 0) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(newPath, "newPath"); var newFolder = await StorageFolder.GetFolderFromPathAsync(System.IO.Path.GetDirectoryName(newPath)).AsTask(cancellationToken).ConfigureAwait(false); string newName = System.IO.Path.GetFileName(newPath); diff --git a/src/PCLStorage.WinRT/WinRTFileSystem.cs b/src/PCLStorage.WinRT/WinRTFileSystem.cs index 7ce1770..7b13532 100644 --- a/src/PCLStorage.WinRT/WinRTFileSystem.cs +++ b/src/PCLStorage.WinRT/WinRTFileSystem.cs @@ -57,6 +57,8 @@ public IFolder RoamingStorage /// A file for the given path, or null if it does not exist. public async Task GetFileFromPathAsync(string path, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(path, "path"); + StorageFile storageFile; try { @@ -78,6 +80,8 @@ public async Task GetFileFromPathAsync(string path, CancellationToken can /// A folder for the specified path, or null if it does not exist. public async Task GetFolderFromPathAsync(string path, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(path, "path"); + StorageFolder storageFolder; try { diff --git a/src/PCLStorage.WinRT/WinRTFolder.cs b/src/PCLStorage.WinRT/WinRTFolder.cs index a812260..52bcb09 100644 --- a/src/PCLStorage.WinRT/WinRTFolder.cs +++ b/src/PCLStorage.WinRT/WinRTFolder.cs @@ -29,9 +29,9 @@ public WinRTFolder(IStorageFolder wrappedFolder) _wrappedFolder = wrappedFolder; if (_wrappedFolder.Path == Windows.Storage.ApplicationData.Current.LocalFolder.Path #if !WINDOWS_PHONE - || _wrappedFolder.Path == Windows.Storage.ApplicationData.Current.RoamingFolder.Path + || _wrappedFolder.Path == Windows.Storage.ApplicationData.Current.RoamingFolder.Path #endif - ) +) { _isRootFolder = true; } @@ -66,6 +66,7 @@ public string Path /// The newly created file public async Task CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(desiredName, "desiredName"); await EnsureExistsAsync(cancellationToken).ConfigureAwait(false); StorageFile wrtFile; try @@ -92,6 +93,8 @@ public async Task CreateFileAsync(string desiredName, CreationCollisionOp /// The requested file, or null if it does not exist public async Task GetFileAsync(string name, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(name, "name"); + await EnsureExistsAsync(cancellationToken).ConfigureAwait(false); try { @@ -125,6 +128,7 @@ public async Task> GetFilesAsync(CancellationToken cancellationToke /// The newly created folder public async Task CreateFolderAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(desiredName, "desiredName"); await EnsureExistsAsync(cancellationToken).ConfigureAwait(false); StorageFolder wrtFolder; try @@ -151,6 +155,8 @@ public async Task CreateFolderAsync(string desiredName, CreationCollisi /// The requested folder, or null if it does not exist public async Task GetFolderAsync(string name, CancellationToken cancellationToken) { + Requires.NotNullOrEmpty(name, "name"); + await EnsureExistsAsync(cancellationToken).ConfigureAwait(false); StorageFolder wrtFolder; try @@ -187,10 +193,7 @@ public async Task> GetFoldersAsync(CancellationToken cancellation /// public async Task CheckExistsAsync(string name, CancellationToken cancellationToken) { - if (string.IsNullOrEmpty(name)) - { - throw new ArgumentException(); - } + Requires.NotNullOrEmpty(name, "name"); // WinRT does not expose an Exists method, so we have to // try accessing the entity to see if it succeeds. diff --git a/src/PCLStorage.WindowsPhone/PCLStorage.WindowsPhone.csproj b/src/PCLStorage.WindowsPhone/PCLStorage.WindowsPhone.csproj index 2d610e7..30eb2b8 100644 --- a/src/PCLStorage.WindowsPhone/PCLStorage.WindowsPhone.csproj +++ b/src/PCLStorage.WindowsPhone/PCLStorage.WindowsPhone.csproj @@ -105,8 +105,8 @@ Properties\CommonAssemblyInfo.cs - - AwaitExtensions.cs + + AwaitExtensions.WinRT.cs WinRTFile.cs @@ -117,6 +117,9 @@ WinRTFolder.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -126,6 +129,9 @@ PortablePath.cs + + Requires.cs + diff --git a/src/PCLStorage.iOS/PCLStorage.iOS.csproj b/src/PCLStorage.iOS/PCLStorage.iOS.csproj index fa69d11..6c7c058 100644 --- a/src/PCLStorage.iOS/PCLStorage.iOS.csproj +++ b/src/PCLStorage.iOS/PCLStorage.iOS.csproj @@ -49,6 +49,9 @@ FileSystemFolder.cs + + AwaitExtensions.cs + Exceptions\PCLStorageExceptions.cs @@ -58,6 +61,9 @@ PortablePath.cs + + Requires.cs + diff --git a/src/PCLStorage/AwaitExtensions.cs b/src/PCLStorage/AwaitExtensions.cs new file mode 100644 index 0000000..186b5cc --- /dev/null +++ b/src/PCLStorage/AwaitExtensions.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace PCLStorage +{ + /// + /// Extensions for use internally by PCLStorage for awaiting. + /// + internal static partial class AwaitExtensions + { + /// + /// Causes the caller who awaits this method to + /// switch off the Main thread. It has no effect if + /// the caller is already off the main thread. + /// + /// The cancellation token. + /// An awaitable that does the thread switching magic. + internal static TaskSchedulerAwaiter SwitchOffMainThreadAsync(CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + return new TaskSchedulerAwaiter( + SynchronizationContext.Current != null ? TaskScheduler.Default : null, + cancellationToken); + } + + internal struct TaskSchedulerAwaiter : INotifyCompletion + { + private TaskScheduler taskScheduler; + private CancellationToken cancellationToken; + + internal TaskSchedulerAwaiter(TaskScheduler taskScheduler, CancellationToken cancellationToken) + { + this.taskScheduler = taskScheduler; + this.cancellationToken = cancellationToken; + } + + internal TaskSchedulerAwaiter GetAwaiter() + { + return this; + } + + public bool IsCompleted + { + get { return this.taskScheduler == null; } + } + + public void OnCompleted(Action continuation) + { + if (this.taskScheduler == null) + { + throw new InvalidOperationException("IsCompleted is true, so this is unexpected."); + } + + Task.Factory.StartNew( + continuation, + CancellationToken.None, + TaskCreationOptions.None, + this.taskScheduler); + } + + public void GetResult() + { + this.cancellationToken.ThrowIfCancellationRequested(); + } + } + } +} diff --git a/src/PCLStorage/PCLStorage.csproj b/src/PCLStorage/PCLStorage.csproj index d1766f7..2336f13 100644 --- a/src/PCLStorage/PCLStorage.csproj +++ b/src/PCLStorage/PCLStorage.csproj @@ -51,10 +51,12 @@ Properties\CommonAssemblyInfo.cs + + diff --git a/src/PCLStorage/Requires.cs b/src/PCLStorage/Requires.cs new file mode 100644 index 0000000..cadde2f --- /dev/null +++ b/src/PCLStorage/Requires.cs @@ -0,0 +1,77 @@ +//----------------------------------------------------------------------- +// +// Copyright (c) Andrew Arnott. All rights reserved. +// +// This file is a derivation of: +// https://github.com/AArnott/Validation/blob/master/src/Validation/Requires.cs +// Which is released under the MS-PL license. +//----------------------------------------------------------------------- + +namespace PCLStorage +{ + using System; + using System.Collections.Generic; + using System.Diagnostics; + using System.Globalization; + + /// + /// Common runtime checks that throw ArgumentExceptions upon failure. + /// + internal static class Requires + { + private const string Argument_EmptyString = "'{0}' cannot be an empty string (\"\") or start with the null character."; + + /// + /// Throws an exception if the specified parameter's value is null. + /// + /// The type of the parameter. + /// The value of the argument. + /// The name of the parameter to include in any thrown exception. + /// The value of the parameter. + /// Thrown if is null + [DebuggerStepThrough] + public static T NotNull(T value, string parameterName) + where T : class // ensures value-types aren't passed to a null checking method + { + if (value == null) + { + throw new ArgumentNullException(parameterName); + } + + return value; + } + + /// + /// Throws an exception if the specified parameter's value is null or empty. + /// + /// The value of the argument. + /// The name of the parameter to include in any thrown exception. + /// Thrown if is null or empty. + [DebuggerStepThrough] + public static void NotNullOrEmpty(string value, string parameterName) + { + // To the guy that is doing random code cleaning: + // Consider the perfomance when changing the code to delegate to NotNull. + // In general do not chain call to another function, check first and return as earlier as possible. + if (value == null) + { + throw new ArgumentNullException(parameterName); + } + + if (value.Length == 0 || value[0] == '\0') + { + throw new ArgumentException(Format(Argument_EmptyString, parameterName), parameterName); + } + } + + /// + /// Helper method that formats string arguments. + /// + /// The formatted string. + private static string Format(string format, params object[] arguments) + { + return string.Format(CultureInfo.CurrentCulture, format, arguments); + } + } +} + diff --git a/test/PCLStorage.Test.Desktop/AwaitExtensionsTest.cs b/test/PCLStorage.Test.Desktop/AwaitExtensionsTest.cs new file mode 100644 index 0000000..39cd549 --- /dev/null +++ b/test/PCLStorage.Test.Desktop/AwaitExtensionsTest.cs @@ -0,0 +1,81 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace PCLStorage.Test +{ + [TestClass] + public class AwaitExtensionsTest + { + [TestMethod] + public async Task SwitchOffMainThreadAsync_OnMainThread() + { + // Make this thread look like the main thread by + // setting up a synchronization context. + var dispatcher = new SynchronizationContext(); + var original = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(dispatcher); + try + { + Thread originalThread = Thread.CurrentThread; + + await AwaitExtensions.SwitchOffMainThreadAsync(CancellationToken.None); + + Assert.AreNotSame(originalThread, Thread.CurrentThread); + Assert.IsNull(SynchronizationContext.Current); + } + finally + { + SynchronizationContext.SetSynchronizationContext(original); + } + } + + [TestMethod] + public void SwitchOffMainThreadAsync_OffMainThread() + { + var awaitable = AwaitExtensions.SwitchOffMainThreadAsync(CancellationToken.None); + var awaiter = awaitable.GetAwaiter(); + Assert.IsTrue(awaiter.IsCompleted); // guarantees the caller wouldn't have switched threads. + } + + [TestMethod] + public void SwitchOffMainThreadAsync_CanceledBeforeSwitch() + { + var cts = new CancellationTokenSource(); + cts.Cancel(); + try + { + AwaitExtensions.SwitchOffMainThreadAsync(cts.Token); + Assert.Fail("Expected OperationCanceledException not thrown."); + } + catch (OperationCanceledException ex) + { + Assert.AreEqual(cts.Token, ex.CancellationToken); + } + } + + [TestMethod] + public void SwitchOffMainThreadAsync_CanceledMidSwitch() + { + var cts = new CancellationTokenSource(); + var awaitable = AwaitExtensions.SwitchOffMainThreadAsync(cts.Token); + var awaiter = awaitable.GetAwaiter(); + + cts.Cancel(); + + try + { + awaiter.GetResult(); + Assert.Fail("Expected OperationCanceledException not thrown."); + } + catch (OperationCanceledException ex) + { + Assert.AreEqual(cts.Token, ex.CancellationToken); + } + } + } +} diff --git a/test/PCLStorage.Test.Desktop/PCLStorage.Test.Desktop.csproj b/test/PCLStorage.Test.Desktop/PCLStorage.Test.Desktop.csproj index ee279d7..e107b55 100644 --- a/test/PCLStorage.Test.Desktop/PCLStorage.Test.Desktop.csproj +++ b/test/PCLStorage.Test.Desktop/PCLStorage.Test.Desktop.csproj @@ -70,6 +70,9 @@ + + AwaitExtensions.cs + FileTestsForDesktop.cs @@ -82,6 +85,7 @@ FolderTests.cs +