Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ListDirectoryAsync return IAsyncEnumerable #1126

Merged
merged 9 commits into from
Aug 30, 2023
Merged

ListDirectoryAsync return IAsyncEnumerable #1126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
14f4838
ListDirectoryAsync return IAsyncEnumerable
WojciechNagorski May 23, 2023
1b0732d
Fix documentation
WojciechNagorski May 23, 2023
ef915c7
Merge develop
WojciechNagorski May 24, 2023
eafa871
Update README.md
WojciechNagorski May 24, 2023
582d6a3
Fix
WojciechNagorski May 24, 2023
aaf8790
Add Sftp ListDirectoryAsync test
WojciechNagorski May 24, 2023
a1b3dd6
Revert
WojciechNagorski May 24, 2023
5fbaba6
Merge master
WojciechNagorski Aug 30, 2023
17e7abe
Integration tests for ListDirectoryAsync with IAsyncEnumerable
WojciechNagorski Aug 30, 2023
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Private keys can be encrypted using one of the following cipher methods:
## Framework Support
**SSH.NET** supports the following target frameworks:
* .NETFramework 4.6.2 (and higher)
* .NET Standard 2.0
* .NET Standard 2.0 and 2.1
* .NET 6 (and higher)

## Usage
Expand Down
4 changes: 4 additions & 0 deletions build/build.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
<OutputDirectory>Renci.SshNet\bin\$(Configuration)\netstandard2.0</OutputDirectory>
<Moniker>netstandard2.0</Moniker>
</TargetFrameworkModern>
<TargetFrameworkModern Include=".NETStandard 2.1">
WojciechNagorski marked this conversation as resolved.
Show resolved Hide resolved
<OutputDirectory>Renci.SshNet\bin\$(Configuration)\netstandard2.1</OutputDirectory>
<Moniker>netstandard2.1</Moniker>
</TargetFrameworkModern>
<TargetFrameworkModern Include=".NET 6.0">
<OutputDirectory>Renci.SshNet\bin\$(Configuration)\net6.0</OutputDirectory>
<Moniker>net6.0</Moniker>
Expand Down
3 changes: 3 additions & 0 deletions build/nuget/SSH.NET.nuspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
<group targetFramework="net462" />
<group targetFramework="netstandard2.0">
<dependency id="SshNet.Security.Cryptography" version="[1.3.0]" />
</group>
<group targetFramework="netstandard2.1">
<dependency id="SshNet.Security.Cryptography" version="[1.3.0]" />
</group>
<group targetFramework="net6.0">
<dependency id="SshNet.Security.Cryptography" version="[1.3.0]" />
Expand Down
10 changes: 5 additions & 5 deletions src/Renci.SshNet.IntegrationTests/SftpClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,17 @@ public async Task Create_directory_with_contents_and_list_it_async()
Assert.IsTrue(_sftpClient.Exists(testFilePath));

// Check if ListDirectory works
var files = await _sftpClient.ListDirectoryAsync(testDirectory, CancellationToken.None);

_sftpClient.DeleteFile(testFilePath);
_sftpClient.DeleteDirectory(testDirectory);
var files = _sftpClient.ListDirectoryAsync(testDirectory, CancellationToken.None);

var builder = new StringBuilder();
foreach (var file in files)
await foreach (var file in files)
{
builder.AppendLine($"{file.FullName} {file.IsRegularFile} {file.IsDirectory}");
}

_sftpClient.DeleteFile(testFilePath);
_sftpClient.DeleteDirectory(testDirectory);

Assert.AreEqual(@"/home/sshnet/sshnet-test/. False True
/home/sshnet/sshnet-test/.. False True
/home/sshnet/sshnet-test/test-file.txt True False
Expand Down
30 changes: 29 additions & 1 deletion src/Renci.SshNet.Tests/Classes/SftpClientTest.ListDirectory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
using System;
using System.Diagnostics;
using System.Linq;
#if NET6_0_OR_GREATER
using System.Threading;
using System.Threading.Tasks;
#endif

namespace Renci.SshNet.Tests.Classes
{
Expand Down Expand Up @@ -89,6 +93,30 @@ public void Test_Sftp_ListDirectory_Current()
}
}

#if NET6_0_OR_GREATER
[TestMethod]
[TestCategory("Sftp")]
[TestCategory("integration")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this test unnecessary now that there is an integration tests project?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I plan to rewrite all integration tests in the new approach, so this will be removed.

public async Task Test_Sftp_ListDirectoryAsync_Current()
{
using (var sftp = new SftpClient(Resources.HOST, Resources.USERNAME, Resources.PASSWORD))
{
sftp.Connect();
var cts = new CancellationTokenSource();
cts.CancelAfter(TimeSpan.FromMinutes(1));
var count = 0;
await foreach(var file in sftp.ListDirectoryAsync(".", cts.Token))
{
count++;
Debug.WriteLine(file.FullName);
}

Assert.IsTrue(count > 0);

sftp.Disconnect();
}
}
#endif
[TestMethod]
[TestCategory("Sftp")]
[TestCategory("integration")]
Expand Down Expand Up @@ -265,4 +293,4 @@ public void Test_Sftp_Call_EndListDirectory_Twice()
}
}
}
}
}
12 changes: 7 additions & 5 deletions src/Renci.SshNet/ISftpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public interface ISftpClient : IBaseClient, IDisposable
/// SSH_FXP_DATA protocol fields.
/// </para>
/// <para>
/// The size of the each indivual SSH_FXP_DATA message is limited to the
/// The size of the each individual SSH_FXP_DATA message is limited to the
/// local maximum packet size of the channel, which is set to <c>64 KB</c>
/// for SSH.NET. However, the peer can limit this even further.
/// </para>
Expand Down Expand Up @@ -699,21 +699,23 @@ public interface ISftpClient : IBaseClient, IDisposable
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallback = null);

#if FEATURE_ASYNC_ENUMERABLE
/// <summary>
/// Asynchronously retrieves list of files in remote directory.
/// Asynchronously enumerates the files in remote directory.
/// </summary>
/// <param name="path">The path.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
/// <returns>
/// A <see cref="Task{IEnumerable}"/> that represents the asynchronous list operation.
/// The task result contains an enumerable collection of <see cref="SftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// An <see cref="IAsyncEnumerable{T}"/> of <see cref="ISftpFile"/> that represents the asynchronous enumeration operation.
/// The enumeration contains an async stream of <see cref="ISftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
/// <exception cref="SshConnectionException">Client is not connected.</exception>
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
Task<IEnumerable<ISftpFile>> ListDirectoryAsync(string path, CancellationToken cancellationToken);
IAsyncEnumerable<ISftpFile> ListDirectoryAsync(string path, CancellationToken cancellationToken);
#endif //FEATURE_ASYNC_ENUMERABLE

/// <summary>
/// Opens a <see cref="SftpFileStream"/> on the specified path with read/write access.
Expand Down
10 changes: 7 additions & 3 deletions src/Renci.SshNet/Renci.SshNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@
<PropertyGroup>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<AssemblyName>Renci.SshNet</AssemblyName>
<TargetFrameworks>net462;netstandard2.0;net6.0;net7.0</TargetFrameworks>
<TargetFrameworks>net462;netstandard2.0;netstandard2.1;net6.0;net7.0</TargetFrameworks>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the motivation to add a netstandard2.1 target, given that net6.0 is the lowest supported platform that can use it?

Should the test projects have a net5.0 (or earlier) target in order to test the netstandard2.1 target? Is it worth it for a new feature?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-1#select-net-standard-version
.NET Standard 2.1 is broader than just modern .NET. For instance, Unity supports .NET Standard 2.1.

I am open to discussing this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not think of that. But given any platform supporting netstandard2.1 would also support netstandard2.0, the only advantage is the extra APIs available e.g. IAsyncEnumerable.

And if we are adding new features using those APIs, at some point the implementation may differ from the .NET implementation, and we then probably ought to have test coverage by using an older target.

That doesn't apply here, so while I would vote to drop netstandard2.1, I think it's fine for now.

</PropertyGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'net462' ">
<DefineConstants>FEATURE_BINARY_SERIALIZATION;FEATURE_SOCKET_EAP;FEATURE_SOCKET_APM;FEATURE_DNS_SYNC;FEATURE_HASH_RIPEMD160_CREATE;FEATURE_HMAC_RIPEMD160</DefineConstants>
</PropertyGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0' ">
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netstandard2.1' or '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0' ">
<PackageReference Include="SshNet.Security.Cryptography" Version="[1.3.0]" />
</ItemGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0' ">
<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netstandard2.1' or '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0' ">
<DefineConstants>FEATURE_SOCKET_TAP;FEATURE_SOCKET_APM;FEATURE_SOCKET_EAP;FEATURE_DNS_SYNC;FEATURE_DNS_APM;FEATURE_DNS_TAP</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition=" '$(TargetFramework)' == 'netstandard2.1' or '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'net7.0' ">
<DefineConstants>$(DefineConstants);FEATURE_ASYNC_ENUMERABLE</DefineConstants>
</PropertyGroup>
</Project>
21 changes: 11 additions & 10 deletions src/Renci.SshNet/SftpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
using Renci.SshNet.Common;
using Renci.SshNet.Sftp;
using System.Threading.Tasks;
#if FEATURE_ASYNC_ENUMERABLE
using System.Runtime.CompilerServices;
#endif

namespace Renci.SshNet
{
Expand Down Expand Up @@ -92,7 +95,7 @@ public TimeSpan OperationTimeout
/// SSH_FXP_DATA protocol fields.
/// </para>
/// <para>
/// The size of the each indivual SSH_FXP_DATA message is limited to the
/// The size of the each individual SSH_FXP_DATA message is limited to the
/// local maximum packet size of the channel, which is set to <c>64 KB</c>
/// for SSH.NET. However, the peer can limit this even further.
/// </para>
Expand Down Expand Up @@ -584,21 +587,22 @@ public IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallbac
return InternalListDirectory(path, listCallback);
}

#if FEATURE_ASYNC_ENUMERABLE
/// <summary>
/// Asynchronously retrieves list of files in remote directory.
/// Asynchronously enumerates the files in remote directory.
/// </summary>
/// <param name="path">The path.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
/// <returns>
/// A <see cref="Task{IEnumerable}"/> that represents the asynchronous list operation.
/// The task result contains an enumerable collection of <see cref="SftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// An <see cref="IAsyncEnumerable{T}"/> of <see cref="ISftpFile"/> that represents the asynchronous enumeration operation.
/// The enumeration contains an async stream of <see cref="ISftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
/// <exception cref="SshConnectionException">Client is not connected.</exception>
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
public async Task<IEnumerable<ISftpFile>> ListDirectoryAsync(string path, CancellationToken cancellationToken)
public async IAsyncEnumerable<ISftpFile> ListDirectoryAsync(string path, [EnumeratorCancellation] CancellationToken cancellationToken)
{
CheckDisposed();

Expand All @@ -616,7 +620,6 @@ public async Task<IEnumerable<ISftpFile>> ListDirectoryAsync(string path, Cancel

var fullPath = await _sftpSession.GetCanonicalPathAsync(path, cancellationToken).ConfigureAwait(false);

var result = new List<SftpFile>();
var handle = await _sftpSession.RequestOpenDirAsync(fullPath, cancellationToken).ConfigureAwait(false);
try
{
Expand All @@ -634,18 +637,16 @@ public async Task<IEnumerable<ISftpFile>> ListDirectoryAsync(string path, Cancel

foreach (var file in files)
{
result.Add(new SftpFile(_sftpSession, basePath + file.Key, file.Value));
yield return new SftpFile(_sftpSession, basePath + file.Key, file.Value);
}
}

}
finally
{
await _sftpSession.RequestCloseAsync(handle, cancellationToken).ConfigureAwait(false);
}

return result;
}
#endif //FEATURE_ASYNC_ENUMERABLE

/// <summary>
/// Begins an asynchronous operation of retrieving list of files in remote directory.
Expand Down