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.

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

DownloadDirectoryAsync not downloading relativly but always with full file name #852

Closed
JanKotschenreuther opened this issue Mar 7, 2022 · 3 comments

Comments

@JanKotschenreuther
Copy link

FTP OS: Windows Server
FTP Server: Windows NT
Computer OS: Windows 11 Pro
FluentFTP Version: 37.0.2 on .NET 6


Problem

When using DownloadDirectoryAsync files are always downloaded with their full file names.

var remoteHostUrl = "ftp://ftp.example.com/";
var credentials = new System.Net.NetworkCredential("ftpuser", "mySecurePw");
using (var client = new FtpClient(remoteHostUrl, credentials))
{
    var localFolder = @"C:\Local_Folder";
    var remoteFolder = "Remote_Folder/Sub_Folder";

    await client.ConnectAsync().ConfigureAwait(false);

    var result = await client.DownloadDirectoryAsync(
        localFolder,
        remoteFolder,
        mode: FtpFolderSyncMode.Update,
        progress: progress
        ).ConfigureAwait(false);

    await client.DisconnectAsync().ConfigureAwait(false);
}

The used FTP-Server got the username as root directory, in this example /ftpuser, not sure if that is anything usual.
The remote file /ftpuser/Remote_Folder/Sub_Folder/Sub_Folder_2/example.json will be downloaded to the local file C:\Local_Folder\ftpuser\Remote_Folder\Sub_Folder\Sub_Folder_2\example.json.
I expected the file to be downloaded to the local file C:\Local_Folder\Sub_Folder_2\example.json.

Because i want the download to be relative, i tried changing the working directory, which did not change the behavior.

    await client.SetWorkingDirectoryAsync("ftpuser/Remote_Folder/Sub_Folder").ConfigureAwait(false);

Because preceding did not work as I expected, I tried passing var remoteFolder = "ftpuser/Remote_Folder/Sub_Folder" to the DownloadDirectoryAsync method, which ran into an exception.


Problem Location

Reviewing the FluentFTP code for a solution, I found the method FtpClient_FolderDownload.cs - GetFilesToDownload on line 175 - code which calculates pathes on lines 182 and 183.

  1. RemovePrefix(remoteFolder) only works if the user passed remoteFolder with a preceding "/".
  2. RemovePrefix(remoteFolder) never applies if the remoteFile.FullName contains the user-root-directory "ftpuser".
  3. Because passing the remoteFolder "ftpuser/Remote_Folder/Sub_Folder" to the Download always resolves to an exception, there is no chance that the code will be hit and correctly calculate the relative path.

Possible Solutions

I reproduced the code on my machine and changed the mentioned lines as following:

// calculate the local path
var relativePath = remoteFile.FullName
    .EnsurePrefix("/").RemovePrefix(Credentials.UserName.EnsurePrefix("/")) //Ensure that the UserName is prefixed and remove the UserName as prefix from the file full name.
    .EnsurePrefix("/").RemovePrefix(remoteFolder.EnsurePrefix("/")) //Ensure that the remoteFolder is prefixed.
    .Replace('/', Path.DirectorySeparatorChar);
var localFile = localFolder.CombineLocalPath(relativePath);

Would be nice if the solution for the UserName will also be picked up, but I am not sure if that is an appropriate solution as i do not know if that is a usual or an unusual case.


Suggestions

To approache this problem, I thought it would be a nice idea to inherit FtpClient.
Sadly, a lot of useful code is private and not even protected, so i had to search the FluentFTP codebase a while to find everything i need and copy it to my inherited FtpClient.
Maybe some methods could be made public to make more options available.
For example:

  • List<FtpResult> GetFilesToDownload(bunch of parameters){...}
  • bool FilePassesRules(FtpResult result, List<FtpRule>? rule, more parameters){...}
  • bool IsBlank(IEnumerable? value)
  • Some public API that can pick up List<FtpResult> as parameter for a download.
@robinrodricks
Copy link
Owner

This is highly dependent on your use case : Credentials.UserName.EnsurePrefix("/")

I think the issue here is that FluentFTP does not know how to remove the prefix from the paths correctly. Can you suggest a more generic way that would work for all servers?

@JanKotschenreuther
Copy link
Author

A weird thing that I was able to observe was that for an Upload I was able to do everything with the out-of-the-box FluentFTP-APIs without running into any issues. I just passed the full remote path that contains the name of the ftpuser.

I solved my problem by the following steps:

  1. Calling the public method GetListingAsync which returns a Task<FtpListItem>.
  2. Reimplementing GetFilesToDownload which takes Task<FtpListItem> and calculates pathes just the way I need them to be calculated and build + return List<FtpResult>.
  3. Reimplementing DownloadServerFilesAsync which takes List<FtpResult> and does the job of downloading. I could have copy-pasted everything needed but that was cumbersome because a lot of methods are private, so I decided to iterate through List<FtpResult> and call DownloadFileAsync for each file + some additional code to make notifications with IProgress<FtpProgress> where FtpProgress.FileIndex and FtpProgress.FileCount is set correctly.

Other Devs can of course come up with a similar solution if needed, but maybe FluentFTP can provide some nice APIs for that.
I am not familiar with FluentFTP, as I made my first steps with it.
So at the moment I can only come up with 2 ideas that could solve the problem generically.


Suggestion 1:

Currently DownloadDirectoryAsync works the following way:

  1. The public method GetListingAsync returns a Task<FtpListItem>.
  2. The private method GetFilesToDownload takes that list to determine the list of files to be downloaded and returns those as List<FtpResult>.
  3. The private method DownloadServerFilesAsync takes that List<FtpResult> and does the job of downloading the files to the location as given by the type FtpResult.

Solution 1:

  1. Because GetListingAsync is already public, a Dev can use that result and create List<FtpResult> and calculate pathes by himself.
  2. But there is no public method that takes a List<FtpResult>.
  3. The private method DownloadServerFilesAsync could be made public or at least protected, so that a Dev is able to make use of the nice working and existing implementation. Or if you think this could be problematic, you maybe want another method which serves as public API and takes List<FtpResult> and other parameters.

Suggestion 2:

Maybe a parameter pathPrefix could be added to DownloadDirectoryAsync, which can be used for the path calculations when passed.
But I got the feeling that this solution is not as nice, not sure about it.

@robinrodricks
Copy link
Owner

Added to the bucket list. We will pick this up as and when we have free time. Comment on this issue if you want us to prioritize it. Thanks!

@robinrodricks robinrodricks closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants