Skip to content

Commit

Permalink
Fixes issue dsplaisted#8: Async methods return faulted tasks now inst…
Browse files Browse the repository at this point in the history
…ead 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 dsplaisted#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.
  • Loading branch information
AArnott committed Feb 19, 2014
1 parent 5b1e6b3 commit 0eb941e
Show file tree
Hide file tree
Showing 21 changed files with 412 additions and 161 deletions.
6 changes: 6 additions & 0 deletions src/PCLStorage.Android/PCLStorage.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@
<Compile Include="..\PCLStorage.FileSystem.Desktop\FileSystemFolder.cs">
<Link>FileSystemFolder.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\AwaitExtensions.cs">
<Link>AwaitExtensions.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\Exceptions\PCLStorageExceptions.cs">
<Link>Exceptions\PCLStorageExceptions.cs</Link>
</Compile>
Expand All @@ -64,6 +67,9 @@
<Compile Include="..\PCLStorage\PortablePath.cs">
<Link>PortablePath.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\Requires.cs">
<Link>Requires.cs</Link>
</Compile>
<Compile Include="Resources\Resource.Designer.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
</ItemGroup>
Expand Down
24 changes: 14 additions & 10 deletions src/PCLStorage.FileSystem.Desktop/DesktopFileSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,18 @@ public IFolder RoamingStorage
/// <param name="path">The path to a file, as returned from the <see cref="IFile.Path"/> property.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>A file for the given path, or null if it does not exist.</returns>
public Task<IFile> GetFileFromPathAsync(string path, CancellationToken cancellationToken)
public async Task<IFile> 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;
}

/// <summary>
Expand All @@ -75,16 +78,17 @@ public Task<IFile> GetFileFromPathAsync(string path, CancellationToken cancellat
/// <param name="path">The path to a folder, as returned from the <see cref="IFolder.Path"/> property.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>A folder for the specified path, or null if it does not exist.</returns>
public Task<IFolder> GetFolderFromPathAsync(string path, CancellationToken cancellationToken)
public async Task<IFolder> 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;
}
}
}
47 changes: 17 additions & 30 deletions src/PCLStorage.FileSystem.Desktop/FileSystemFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,39 +51,38 @@ public string Path
/// <param name="fileAccess">Specifies whether the file should be opened in read-only or read/write mode</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>A <see cref="Stream"/> which can be used to read from or write to the file</returns>
public Task<Stream> OpenAsync(FileAccess fileAccess, CancellationToken cancellationToken)
public async Task<Stream> 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);
}

/// <summary>
/// Deletes the file
/// </summary>
/// <returns>A task which will complete after the file is deleted.</returns>
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);
}

/// <summary>
Expand All @@ -95,18 +94,11 @@ public Task DeleteAsync(CancellationToken cancellationToken)
/// <returns>
/// A task which will complete after the file is renamed.
/// </returns>
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);
}

/// <summary>
Expand All @@ -118,16 +110,11 @@ public Task RenameAsync(string newName, NameCollisionOption collisionOption, Can
/// <returns>
/// A task which will complete after the file is moved.
/// </returns>
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);
Expand Down Expand Up @@ -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;
}
}
}
Expand Down
62 changes: 32 additions & 30 deletions src/PCLStorage.FileSystem.Desktop/FileSystemFolder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ public string Path
/// <param name="option">Specifies how to behave if the specified file already exists</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The newly created file</returns>
public Task<IFile> CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken)
public async Task<IFile> CreateFileAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
Requires.NotNullOrEmpty(desiredName, "desiredName");

await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken);
EnsureExists();

string nameToUse = desiredName;
Expand Down Expand Up @@ -110,7 +112,7 @@ public Task<IFile> CreateFileAsync(string desiredName, CreationCollisionOption o
}

var ret = new FileSystemFile(newPath);
return Task.FromResult<IFile>(ret);
return ret;
}

void InternalCreateFile(string path)
Expand All @@ -127,28 +129,29 @@ void InternalCreateFile(string path)
/// <param name="name">The name of the file to get</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The requested file, or null if it does not exist</returns>
public Task<IFile> GetFileAsync(string name, CancellationToken cancellationToken)
public async Task<IFile> 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<IFile>(ret);
return ret;
}

/// <summary>
/// Gets a list of the files in this folder
/// </summary>
/// <returns>A list of the files in the folder</returns>
public Task<IList<IFile>> GetFilesAsync(CancellationToken cancellationToken)
public async Task<IList<IFile>> GetFilesAsync(CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken);
EnsureExists();
IList<IFile> ret = Directory.GetFiles(Path).Select(f => new FileSystemFile(f)).ToList<IFile>().AsReadOnly();
return Task.FromResult(ret);
return ret;
}

/// <summary>
Expand All @@ -158,9 +161,10 @@ public Task<IList<IFile>> GetFilesAsync(CancellationToken cancellationToken)
/// <param name="option">Specifies how to behave if the specified folder already exists</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The newly created folder</returns>
public Task<IFolder> CreateFolderAsync(string desiredName, CreationCollisionOption option, CancellationToken cancellationToken)
public async Task<IFolder> 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);
Expand Down Expand Up @@ -200,7 +204,7 @@ public Task<IFolder> CreateFolderAsync(string desiredName, CreationCollisionOpti
}

var ret = new FileSystemFolder(newPath, true);
return Task.FromResult<IFolder>(ret);
return ret;
}

/// <summary>
Expand All @@ -209,28 +213,30 @@ public Task<IFolder> CreateFolderAsync(string desiredName, CreationCollisionOpti
/// <param name="name">The name of the folder to get</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The requested folder, or null if it does not exist</returns>
public Task<IFolder> GetFolderAsync(string name, CancellationToken cancellationToken)
public async Task<IFolder> 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<IFolder>(ret);
return ret;
}

/// <summary>
/// Gets a list of subfolders in this folder
/// </summary>
/// <returns>A list of subfolders in the folder</returns>
public Task<IList<IFolder>> GetFoldersAsync(CancellationToken cancellationToken)
public async Task<IList<IFolder>> GetFoldersAsync(CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
await AwaitExtensions.SwitchOffMainThreadAsync(cancellationToken);
EnsureExists();
IList<IFolder> ret = Directory.GetDirectories(Path).Select(d => new FileSystemFolder(d, true)).ToList<IFolder>().AsReadOnly();
return Task.FromResult(ret);
return ret;
}

/// <summary>
Expand All @@ -241,43 +247,39 @@ public Task<IList<IFolder>> GetFoldersAsync(CancellationToken cancellationToken)
/// <returns>
/// A task whose result is the result of the existence check.
/// </returns>
public Task<ExistenceCheckResult> CheckExistsAsync(string name, CancellationToken cancellationToken)
public async Task<ExistenceCheckResult> 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;
}
}

/// <summary>
/// Deletes this folder and all of its contents
/// </summary>
/// <returns>A task which will complete after the folder is deleted</returns>
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
<Compile Include="..\..\common\CommonAssemblyInfo.cs">
<Link>Properties\CommonAssemblyInfo.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\AwaitExtensions.cs">
<Link>AwaitExtensions.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\Exceptions\PCLStorageExceptions.cs">
<Link>Exceptions\PCLStorageExceptions.cs</Link>
</Compile>
Expand All @@ -65,6 +68,9 @@
<Compile Include="..\PCLStorage\PortablePath.cs">
<Link>PortablePath.cs</Link>
</Compile>
<Compile Include="..\PCLStorage\Requires.cs">
<Link>Requires.cs</Link>
</Compile>
<Compile Include="DesktopFileSystem.cs" />
<Compile Include="FileSystemFile.cs" />
<Compile Include="FileSystemFolder.cs" />
Expand Down
Loading

0 comments on commit 0eb941e

Please sign in to comment.