From ce73e0f1f60e5bbe4e09853ea3d1b783c8412bec Mon Sep 17 00:00:00 2001 From: "Marcelo C." Date: Wed, 4 Dec 2024 09:29:14 -0300 Subject: [PATCH] Implementation of fixes suggested on code review Post changes tests: - Folder creation and discovery checked. - Backup and restore operations checked. - Tests with backend tests performed. --- .../Library/Backend/pCloud/pCloudBackend.cs | 192 ++++++++++-------- 1 file changed, 103 insertions(+), 89 deletions(-) diff --git a/Duplicati/Library/Backend/pCloud/pCloudBackend.cs b/Duplicati/Library/Backend/pCloud/pCloudBackend.cs index 8447d8e16..8c6c2261a 100644 --- a/Duplicati/Library/Backend/pCloud/pCloudBackend.cs +++ b/Duplicati/Library/Backend/pCloud/pCloudBackend.cs @@ -92,7 +92,7 @@ public class pCloudBackend : IStreamingBackend /// requests /// private ulong? _CachedFolderID; - + /// /// Name of the authentication parameter/option /// @@ -139,7 +139,7 @@ public class pCloudBackend : IStreamingBackend if (string.IsNullOrWhiteSpace(uri.Host)) throw new UserInformationException(Strings.pCloudBackend.NoServerSpecified, "NopCloudServerSpecified"); - + // Ensure that the path is in the correct format, without starting or tailing slashes _Path = uri.Path.TrimStart(PATH_SEPARATORS).TrimEnd(PATH_SEPARATORS).Trim(); _ServerUrl = uri.Host; @@ -166,43 +166,47 @@ public class pCloudBackend : IStreamingBackend /// Implementation of interface method for listing remote folder contents. /// The root parameter is used to list the root folder, as the pCloud API /// when using oauth tokens creates an isolated folder Applications/ApplicationName - /// + /// /// /// The folder ID to consider as root + /// Cancellation Token /// - private IEnumerable List(ulong folderId) + private async Task> List(ulong folderId, CancellationToken cancellationToken) { - return ListWithMetadata(folderId) - .Select(item => new pCloudFileEntry - { - IsFolder = item.isfolder, - Name = item.name, - Size = item.size ?? 0, - LastAccess = DateTime.Parse(item.created), - LastModification = DateTime.Parse(item.modified) - }) + var result = await ListWithMetadata(folderId, cancellationToken).ConfigureAwait(false); + return result.Select(item => new pCloudFileEntry + { + IsFolder = item.isfolder, + Name = item.name, + Size = item.size ?? 0, + LastAccess = DateTime.Parse(item.created), + LastModification = DateTime.Parse(item.modified) + }) .ToList(); } - + /// /// Lists folders with pCloud metadata, necessary to obtain the folder IDs /// /// FolderId to be used as root. + /// Cancellation Token /// /// - private IEnumerable ListWithMetadata(ulong folderId) + private async Task> ListWithMetadata(ulong folderId, CancellationToken cancellationToken) { - var timeoutToken = new CancellationTokenSource(); + using var timeoutToken = new CancellationTokenSource(); timeoutToken.CancelAfter(TimeSpan.FromSeconds(SHORT_OPERATION_TIMEOUT_SECONDS)); + using var combinedTokens = + CancellationTokenSource.CreateLinkedTokenSource(timeoutToken.Token, cancellationToken); using var requestResources = CreateRequest($"/listfolder?folderid={folderId}", HttpMethod.Get); - using var response = requestResources.HttpClient + using var response = await requestResources.HttpClient .SendAsync(requestResources.RequestMessage, HttpCompletionOption.ResponseContentRead, - timeoutToken.Token).Await(); + timeoutToken.Token).ConfigureAwait(false); response.EnsureSuccessStatusCode(); - var content = response.Content.ReadAsStringAsync(timeoutToken.Token).Result; + var content = await response.Content.ReadAsStringAsync(timeoutToken.Token).ConfigureAwait(false); var listFolderResponse = JsonSerializer.Deserialize(content); if (pCloudErrorList.ErrorMessages.TryGetValue(listFolderResponse.result, out var message)) @@ -218,13 +222,19 @@ public class pCloudBackend : IStreamingBackend /// Implementation of interface method for listing remote folder contents /// /// List of IFileEntry with directory listing result - public IEnumerable List() + public async Task> ListAsync(CancellationToken cancellationToken) { - _CachedFolderID ??= GetFolderId().Await(); + _CachedFolderID ??= await GetFolderId(cancellationToken).ConfigureAwait(false); - return List(_CachedFolderID.Value); + return await List(_CachedFolderID.Value, cancellationToken).ConfigureAwait(false); } + /// + /// Wrapper method of legacy non async call to list files in the remote folder + /// + /// + public IEnumerable List() => ListAsync(CancellationToken.None).Await(); + /// /// Upload files to remote location /// @@ -250,8 +260,8 @@ public class pCloudBackend : IStreamingBackend /// Exceptions arising from either code execution public async Task PutAsync(string remotename, Stream input, CancellationToken cancellationToken) { - _CachedFolderID ??= GetFolderId().Await(); - + _CachedFolderID ??= await GetFolderId(cancellationToken).ConfigureAwait(false); + using var timeoutToken = new CancellationTokenSource(); timeoutToken.CancelAfter(TimeSpan.FromSeconds(LONG_OPERATION_TIMEOUT_SECONDS)); using var combinedTokens = @@ -262,7 +272,7 @@ public class pCloudBackend : IStreamingBackend using var requestResources = CreateRequest($"/uploadfile?folderid={_CachedFolderID}&filename={encodedPath}&nopartial=1", HttpMethod.Post); - + requestResources.RequestMessage.Content = new StreamContent(input); requestResources.RequestMessage.Content.Headers.ContentLength = input.Length; requestResources.RequestMessage.Content.Headers.ContentType = @@ -289,7 +299,7 @@ public class pCloudBackend : IStreamingBackend /// Destination stream to write to /// CancellationToken that is combined with internal timeout token /// FileMissingException when file is not found - /// Exceptions arising from either code execution + /// Exceptions arising from either code execution or FileMissingException public async Task GetAsync(string remotename, string localname, CancellationToken cancellationToken) { await using var fs = File.Open(localname, @@ -303,23 +313,27 @@ public class pCloudBackend : IStreamingBackend /// /// /// Filename at remote, path is automatically concatenated if needed + /// Cancellation Token /// /// FileMissingException when file is not found /// Exceptions arising from either code execution - private async Task GetFileLink(string filename) + private async Task GetFileLink(string filename, CancellationToken cancellationToken) { - using var requestResources = CreateRequest($"/getfilelink?fileid={GetFileId(filename).Await()}", HttpMethod.Get); + using var timeoutToken = new CancellationTokenSource(); + timeoutToken.CancelAfter(TimeSpan.FromSeconds(SHORT_OPERATION_TIMEOUT_SECONDS)); + using var combinedTokens = + CancellationTokenSource.CreateLinkedTokenSource(timeoutToken.Token, cancellationToken); + + using var requestResources = CreateRequest($"/getfilelink?fileid={await GetFileId(filename, cancellationToken).ConfigureAwait(false)}", HttpMethod.Get); using var response = await requestResources.HttpClient.SendAsync( requestResources.RequestMessage, - HttpCompletionOption.ResponseContentRead).ConfigureAwait(false); + HttpCompletionOption.ResponseContentRead, combinedTokens.Token).ConfigureAwait(false); if (!response.IsSuccessStatusCode) - { throw new Exception($"Failed to get download link. Status: {response.StatusCode}"); - } - var content = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + var content = await response.Content.ReadAsStringAsync(combinedTokens.Token).ConfigureAwait(false); var getFileIdResponse = JsonSerializer.Deserialize(content); if (pCloudErrorList.ErrorMessages.TryGetValue(getFileIdResponse.result, out var message)) @@ -339,7 +353,7 @@ public class pCloudBackend : IStreamingBackend /// Destination stream to write to /// CancellationToken that is combined with internal timeout token /// FileMissingException when file is not found - /// Exceptions arising from either code execution + /// Exceptions arising from either code execution or FileMissingException public async Task GetAsync(string remotename, Stream output, CancellationToken cancellationToken) { try @@ -351,10 +365,10 @@ public class pCloudBackend : IStreamingBackend using var requestResources = CreateRequest(string.Empty, HttpMethod.Get); - requestResources.RequestMessage.RequestUri = new Uri(GetFileLink(remotename).Await()); + requestResources.RequestMessage.RequestUri = new Uri(await GetFileLink(remotename, cancellationToken).ConfigureAwait(false)); await requestResources.HttpClient.DownloadFile(requestResources.RequestMessage, output, null, - timeoutToken.Token).ConfigureAwait(false); + combinedTokens.Token).ConfigureAwait(false); } catch (HttpRequestException wex) { @@ -385,22 +399,22 @@ public class pCloudBackend : IStreamingBackend /// /// FileMissingException when file is not found /// Exceptions arising from either code execution or business logic when return code from pcloud indicates an error. - public Task DeleteAsync(string remotename, CancellationToken cancellationToken) + public async Task DeleteAsync(string remotename, CancellationToken cancellationToken) { using var timeoutToken = new CancellationTokenSource(); timeoutToken.CancelAfter(TimeSpan.FromSeconds(SHORT_OPERATION_TIMEOUT_SECONDS)); using var combinedTokens = CancellationTokenSource.CreateLinkedTokenSource(timeoutToken.Token, cancellationToken); - using var requestResources = CreateRequest($"/deletefile?fileid={GetFileId(remotename).Await()}", HttpMethod.Get); + using var requestResources = CreateRequest($"/deletefile?fileid={await GetFileId(remotename, cancellationToken).ConfigureAwait(false)}", HttpMethod.Get); - using var response = requestResources.HttpClient.SendAsync( + using var response = await requestResources.HttpClient.SendAsync( requestResources.RequestMessage, - HttpCompletionOption.ResponseContentRead, combinedTokens.Token).Await(); + HttpCompletionOption.ResponseContentRead, combinedTokens.Token).ConfigureAwait(false); response.EnsureSuccessStatusCode(); - var content = response.Content.ReadAsStringAsync(combinedTokens.Token).Await(); + var content = await response.Content.ReadAsStringAsync(combinedTokens.Token).ConfigureAwait(false); var deleteFileResponse = JsonSerializer.Deserialize(content); // If no error code is matched, result was == 0 so it successfully created the folder @@ -414,7 +428,6 @@ public class pCloudBackend : IStreamingBackend throw new Exception( Strings.pCloudBackend.FailedWithUnexpectedErrorCode("delete", deleteFileResponse.result)); - return Task.CompletedTask; } /// @@ -429,21 +442,14 @@ public class pCloudBackend : IStreamingBackend /// /// The cancellation token (not used) /// Thrown when configured path does not exist - public Task TestAsync(CancellationToken cancellationToken) + public async Task TestAsync(CancellationToken cancellationToken) { - if (string.IsNullOrWhiteSpace(_Path)) - return Task.CompletedTask; + if (string.IsNullOrWhiteSpace(_Path) || _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries).Length == 0) + return; - var segments = _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries); - if (segments.Length == 0) - return Task.CompletedTask; + // This method will search for the folderId recursively and throw an exception if the folder is not found + await GetFolderId(cancellationToken).ConfigureAwait(false); - _ = segments.Aggregate(0UL, (parentId, folder) => - ListWithMetadata(parentId) - .FirstOrDefault(x => x.isfolder && x.name == folder) - ?.folderid ?? throw new FolderMissingException()); - - return Task.CompletedTask; } /// @@ -452,20 +458,21 @@ public class pCloudBackend : IStreamingBackend /// CancellationToken that will be combined with internal timeout token /// /// - public Task CreateFolderAsync(CancellationToken cancellationToken) + public async Task CreateFolderAsync(CancellationToken cancellationToken) { - if (string.IsNullOrWhiteSpace(_Path)) + if (string.IsNullOrWhiteSpace(_Path) || _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries).Length == 0) + { _CachedFolderID = 0; - else - _CachedFolderID = _Path.IndexOfAny(PATH_SEPARATORS) == -1 - ? CreateFolder(cancellationToken, 0, _Path).Await() - : _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries) - .Aggregate(0UL, (currentId, folder) => - CreateFolder(cancellationToken, currentId, folder).Await()); - - return Task.CompletedTask; + return; + } + + var currentId = 0UL; + foreach (var folder in _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries)) + currentId = await CreateFolder(cancellationToken, currentId, folder).ConfigureAwait(false); + + _CachedFolderID = currentId; } - + /// /// Create remote folder in relation to the parent folder /// @@ -473,7 +480,7 @@ public class pCloudBackend : IStreamingBackend /// Parent Folder ID /// Folder name /// The folderID of the newly created folder - private Task CreateFolder(CancellationToken cancellationToken, ulong parentFolderId, string folderName) + private async Task CreateFolder(CancellationToken cancellationToken, ulong parentFolderId, string folderName) { using var timeoutToken = new CancellationTokenSource(); timeoutToken.CancelAfter(TimeSpan.FromSeconds(SHORT_OPERATION_TIMEOUT_SECONDS)); @@ -482,59 +489,66 @@ public class pCloudBackend : IStreamingBackend using var requestResources = CreateRequest($"/createfolderifnotexists?folderid={parentFolderId}&name={folderName}", HttpMethod.Get); - using var response = requestResources.HttpClient.SendAsync( + using var response = await requestResources.HttpClient.SendAsync( requestResources.RequestMessage, - HttpCompletionOption.ResponseContentRead, combinedTokens.Token).Await(); + HttpCompletionOption.ResponseContentRead, combinedTokens.Token).ConfigureAwait(false); response.EnsureSuccessStatusCode(); - var content = response.Content.ReadAsStringAsync(combinedTokens.Token).Await(); + var content = await response.Content.ReadAsStringAsync(combinedTokens.Token).ConfigureAwait(false); var createFolderResponse = JsonSerializer.Deserialize(content); if (pCloudErrorList.ErrorMessages.TryGetValue(createFolderResponse.result, out var message)) throw new Exception(message); - + return createFolderResponse is { result: 0, metadata.folderid: var id } - ? Task.FromResult(id) + ? id : throw new Exception(Strings.pCloudBackend.FailedWithUnexpectedErrorCode("createfolder", createFolderResponse.result)); } /// /// Returns the fileID by listing the folder and searching for the filename & metadata. - /// + /// /// For operations such as delete/getfilelink using the fileID is more reliable than using direct path/filename concatenation. /// /// The filename + /// Cancellation Token /// /// - private Task GetFileId(string name) + private async Task GetFileId(string name, CancellationToken cancellationToken) { - _CachedFolderID ??= GetFolderId().Await(); - - return Task.FromResult(ListWithMetadata((ulong)_CachedFolderID) - .FirstOrDefault(x => !x.isfolder && x.name == name) - ?.fileid ?? throw new FileNotFoundException(name)); + _CachedFolderID ??= await GetFolderId(cancellationToken).ConfigureAwait(false); + + var result = await ListWithMetadata((ulong)_CachedFolderID, cancellationToken).ConfigureAwait(false); + + return result.FirstOrDefault(x => !x.isfolder && x.name == name)?.fileid ?? throw new FileNotFoundException(name); + } - + /// /// Returns the folder ID for the configured path, regardless of the depth /// /// - private Task GetFolderId() + private async Task GetFolderId(CancellationToken cancellationToken) { - if (string.IsNullOrWhiteSpace(_Path)) - return Task.FromResult(0UL); + if (string.IsNullOrWhiteSpace(_Path) || _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries).Length == 0) + return 0UL; - var segments = _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries); - if (segments.Length == 0) - return Task.FromResult(0UL); + var currentFolderId = 0UL; + foreach (var folder in _Path.Split(PATH_SEPARATORS, StringSplitOptions.RemoveEmptyEntries)) + { + var folderContent = await ListWithMetadata(currentFolderId, cancellationToken).ConfigureAwait(false); + var matchingFolder = folderContent.FirstOrDefault(x => x.isfolder && x.name == folder); - return Task.FromResult(segments.Aggregate(0UL, (parentId, folder) => - ListWithMetadata(parentId) - .FirstOrDefault(x => x.isfolder && x.name == folder) - ?.folderid ?? throw new FolderMissingException())); + if (matchingFolder?.folderid == null) + throw new FolderMissingException(); + + currentFolderId = matchingFolder.folderid; + } + + return currentFolderId; } - + /// /// Wrapper for the tupple of HttpClient and HttpRequestMessage used in web requests. ///