Improves Files.App.Storage #10834
Improves Files.App.Storage #10834
Conversation
| public Task<bool> IsFileSystemAccessibleAsync(CancellationToken cancellationToken = default) | ||
| { |
|
|
||
| private static async Task<AsyncFtpClient> GetClient(string path, CancellationToken cancellationToken = default) | ||
| { | ||
| using AsyncFtpClient client = path.GetFtpClient(); |
There was a problem hiding this comment.
using statement here will dispose the client after the method exits. Remove it from here
There was a problem hiding this comment.
Besides, you could refactor the method in FtpStorable.GetFtpClientAsync() - to use a string path parameter, and remove this method
| } | ||
|
|
||
| public static Task EnsureConnectedAsync(this AsyncFtpClient ftpClient, CancellationToken cancellationToken = default) | ||
| public static string GetFtpPath(this string path) |
There was a problem hiding this comment.
Don't make GetFtpPath an extension method on string!!!!
| @@ -5,7 +5,7 @@ namespace Files.Shared.Helpers | |||
| { | |||
| public static class PathHelpers | |||
| { | |||
| public static string GetParentDir(string path) | |||
| public static string GetParentDir(this string path) | |||
| @@ -14,7 +14,7 @@ public static string GetParentDir(string path) | |||
| return path.Substring(0, index != -1 ? index : path.Length); | |||
| } | |||
|
|
|||
| public static string Combine(string folder, string name) | |||
| public static string Combine(this string folder, string name) | |||
| { | ||
| return Task.FromResult<ILocatableFolder?>(null); | ||
| } | ||
| => Task.FromResult<ILocatableFolder?>(null); |
| public abstract class FtpStorable : ILocatableStorable | ||
| { | ||
| /// <inheritdoc/> | ||
| public string Path { get; protected set; } | ||
| public string Id => string.Empty; |
There was a problem hiding this comment.
fyi, I wouldn't do anything to this for now, as Id will be implemented here. (And in other places as well)
|
|
||
| protected AsyncFtpClient GetFtpClient() | ||
| protected async Task<AsyncFtpClient> GetFtpClient(CancellationToken cancellationToken = default) |
| : base(path, name) | ||
| { | ||
| } | ||
| public FtpStorageFile(string path, string name) : base(path, name) {} |
There was a problem hiding this comment.
Revert this change :) (And in other places too)
|
@d2dyno1 @yaira2 @QuaintMako This pr has a lot of cancellation requests and conflicts with #10477. I am closing it because it is no longer needed. |
Resolved / Related Issues
Clean Storage code.
Validation
How did you test these changes?