From 51497349cde1ae64ed8cb67b623c610a092d12f7 Mon Sep 17 00:00:00 2001 From: Jari Siikarla Date: Thu, 29 Oct 2026 12:06:44 +0200 Subject: [PATCH] Fix uploading temporary files in blocks and the extension dot Blocks identified by upload IDs: - Send only the data of the last block instead of the whole buffer, which padded the uploaded file with leftover data from the previous block. - Round the number of blocks up. A size that divided evenly by the block size gave one block too many, so MFWS never saw the last block. - Add UploadTemporaryFileBlock[Async] overloads that take the block size, so a shorter last block can be uploaded. Reject offsets that are not at a block boundary and blocks larger than the block size. - Read each block fully from the stream. File names: - Parse the file name once into a title and an extension without the dot (GetFileTitle, GetFileExtension), as M-Files expects. Replaces the separate dot removal in UploadFiles and UploadTemporaryFileBlockBegin, and the dotted extension in UploadTemporaryFileInBlocks. UploadFiles no longer throws for a file without an extension. - UploadTemporaryFileInBlocks returns the title, extension and size with the upload ID, instead of the last block response which had neither a title nor an extension. Tests: close test.txt after creating it, which made the upload tests fail on a clean run. Co-Authored-By: Claude Opus 5.5 --- .../MFWSVaultObjectFileOperations.cs | 47 ++-- .../MFaaP.MFWSClient.Tests.csproj | 1 + MFaaP.MFWSClient.Tests/UploadInBlocks.cs | 243 ++++++++++++++++++ .../MFWSVaultObjectFileOperations.cs | 134 ++++++++-- 4 files changed, 369 insertions(+), 56 deletions(-) create mode 100644 MFaaP.MFWSClient.Tests/UploadInBlocks.cs diff --git a/MFaaP.MFWSClient.Tests/MFWSVaultObjectFileOperations.cs b/MFaaP.MFWSClient.Tests/MFWSVaultObjectFileOperations.cs index 39e2697..690c2b1 100644 --- a/MFaaP.MFWSClient.Tests/MFWSVaultObjectFileOperations.cs +++ b/MFaaP.MFWSClient.Tests/MFWSVaultObjectFileOperations.cs @@ -57,10 +57,7 @@ public async Task UploadFileAsync() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFilesAsync. + tempFile.Create().Dispose(); tempFile = new FileInfo(@"test.txt"); } @@ -99,10 +96,7 @@ public async Task UploadFileAsync_Latest() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFilesAsync. + tempFile.Create().Dispose(); tempFile = new FileInfo(@"test.txt"); } @@ -140,10 +134,7 @@ public async Task UploadFileAsync_Unmanaged() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFilesAsync. + tempFile.Create().Dispose(); tempFile = new FileInfo(@"test.txt"); } @@ -184,10 +175,7 @@ public async Task UploadFilesAsync() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFilesAsync. + tempFile.Create().Dispose(); tempFile.Refresh(); } @@ -224,10 +212,7 @@ public async Task UploadFilesAsync_Multiple() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFilesAsync. + tempFile.Create().Dispose(); tempFile.Refresh(); } @@ -271,10 +256,7 @@ public void UploadFiles() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); - - // NOTE: If test.txt did not exist the first time the test was run then 'file.Length' will throw an exception - // of type 'System.IO.FileNotFoundException' in MFWSVaultObjectFileOperations.UploadFiles. + tempFile.Create().Dispose(); tempFile = new FileInfo(@"test.txt"); } @@ -408,7 +390,10 @@ public async Task AddFilesAsync_CorrectResource() // Create a temporary file. var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) - tempFile.Create(); + { + tempFile.Create().Dispose(); + tempFile.Refresh(); + } // Create our restsharp mock. var mock = new Mock(); @@ -484,7 +469,10 @@ public void AddFiles_CorrectResource() // Create a temporary file. var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) - tempFile.Create(); + { + tempFile.Create().Dispose(); + tempFile.Refresh(); + } // Create our restsharp mock. var mock = new Mock(); @@ -561,7 +549,7 @@ public async Task AddFilesAsync_CorrectMethod() var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) { - tempFile.Create(); + tempFile.Create().Dispose(); tempFile.Refresh(); } @@ -639,7 +627,10 @@ public void AddFiles_CorrectMethod() // Create a temporary file. var tempFile = new FileInfo(@"test.txt"); if (false == tempFile.Exists) - tempFile.Create(); + { + tempFile.Create().Dispose(); + tempFile.Refresh(); + } // Create our restsharp mock. var mock = new Mock(); diff --git a/MFaaP.MFWSClient.Tests/MFaaP.MFWSClient.Tests.csproj b/MFaaP.MFWSClient.Tests/MFaaP.MFWSClient.Tests.csproj index 6f54d9f..2dd1f00 100644 --- a/MFaaP.MFWSClient.Tests/MFaaP.MFWSClient.Tests.csproj +++ b/MFaaP.MFWSClient.Tests/MFaaP.MFWSClient.Tests.csproj @@ -75,6 +75,7 @@ + diff --git a/MFaaP.MFWSClient.Tests/UploadInBlocks.cs b/MFaaP.MFWSClient.Tests/UploadInBlocks.cs new file mode 100644 index 0000000..80bdc0f --- /dev/null +++ b/MFaaP.MFWSClient.Tests/UploadInBlocks.cs @@ -0,0 +1,243 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using RestSharp; + +namespace MFaaP.MFWSClient.Tests +{ + /// + /// Tests for uploading temporary files in blocks identified by upload IDs. + /// + [TestClass] + public class UploadInBlocks + { + /// + /// Gets the value of a querystring parameter of the request, or null. + /// + private static string GetQueryParameter(RestRequest request, string name) + { + return request.Parameters + .FirstOrDefault(p => p.Type == ParameterType.QueryString && p.Name == name)? + .Value as string; + } + + /// + /// Creates a client which records the upload requests and returns the given upload ID. + /// + private static MFaaP.MFWSClient.MFWSClient CreateClient(List requests, int uploadId = 7) + { + var mock = new Mock(); + mock + .Setup(c => c.ExecuteAsync(It.IsAny(), It.IsAny())) + .Callback((RestRequest r, CancellationToken t) => requests.Add(r)) + .Returns((RestRequest r, CancellationToken t) => Task.FromResult(new RestResponse(r) + { + Data = new UploadInfo() { UploadID = uploadId }, + StatusCode = HttpStatusCode.OK, + ResponseUri = new Uri(r.Resource, UriKind.Relative) + })); + return MFWSClient.GetMFWSClient(mock); + } + + /// + /// Uploads the content in blocks and checks the block requests. + /// + private static async Task AssertUploadedInBlocks(int contentLength, int blockSize, int[] expectedBlockSizes) + { + var content = Enumerable.Range(0, contentLength).Select(i => (byte)(i + 1)).ToArray(); + var requests = new List(); + var client = CreateClient(requests); + + // Execute. + using (var stream = new MemoryStream(content)) + { + await client.ObjectFileOperations.UploadTemporaryFileInBlocksAsync("test.txt", stream, blockSize: blockSize); + } + + // The upload is started, then the blocks are uploaded. + Assert.AreEqual(expectedBlockSizes.Length + 1, requests.Count); + Assert.AreEqual("/REST/files/upload.aspx", requests[0].Resource); + var offset = 0; + for (var i = 0; i < expectedBlockSizes.Length; i++) + { + var request = requests[i + 1]; + Assert.AreEqual("/REST/files/upload/7.aspx", request.Resource); + Assert.AreEqual((i + 1).ToString(), GetQueryParameter(request, "resumableChunkNumber")); + Assert.AreEqual(expectedBlockSizes.Length.ToString(), GetQueryParameter(request, "resumableTotalChunks")); + // A file smaller than the block size is one block of its own size. + Assert.AreEqual(Math.Min(blockSize, contentLength).ToString(), GetQueryParameter(request, "resumableChunkSize")); + + // The block contains only its own data. + var body = (byte[])request.Parameters.First(p => p.Type == ParameterType.RequestBody).Value; + CollectionAssert.AreEqual(content.Skip(offset).Take(expectedBlockSizes[i]).ToArray(), body); + offset += expectedBlockSizes[i]; + } + } + + /// + /// Ensures that the last block is sent with only its own data, not the whole buffer. + /// + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_ShortLastBlock() + { + await AssertUploadedInBlocks(10, 4, new[] { 4, 4, 2 }); + } + + /// + /// Ensures that the number of blocks is correct when the size divides evenly by the block size. + /// + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_EvenlyDivisible() + { + await AssertUploadedInBlocks(8, 4, new[] { 4, 4 }); + } + + /// + /// Ensures that a file smaller than the block size is uploaded in one block. + /// + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_SmallerThanBlock() + { + await AssertUploadedInBlocks(3, 4, new[] { 3 }); + } + + /// + /// Ensures that the file name is split into a title and an extension without the dot, which are sent when the + /// upload is started and returned with the upload ID and size. + /// + private static async Task AssertFileNameSplit(string fileName, string expectedTitle, string expectedExtension) + { + var requests = new List(); + var client = CreateClient(requests); + + // Execute. + UploadInfo upload; + using (var stream = new MemoryStream(new byte[10])) + { + upload = await client.ObjectFileOperations.UploadTemporaryFileInBlocksAsync(fileName, stream, blockSize: 4); + } + + // The start sends the title and the extension without the dot. + var started = (UploadInfo)requests[0].Parameters.First(p => p.Type == ParameterType.RequestBody).Value; + Assert.AreEqual(expectedTitle, started.Title); + Assert.AreEqual(expectedExtension, started.Extension); + + // The result can be used to create an object: it has the upload ID, the file name and the size. + Assert.AreEqual(7, upload.UploadID); + Assert.AreEqual(expectedTitle, upload.Title); + Assert.AreEqual(expectedExtension, upload.Extension); + Assert.AreEqual(10, upload.Size); + } + + /// + /// Ensures that + /// sends the extension without the dot, also for a file without an extension. + /// + private static async Task AssertFileInfoExtension(string fileName, string expectedExtension) + { + var requests = new List(); + var client = CreateClient(requests); + var directory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); + try + { + var file = new FileInfo(Path.Combine(directory.FullName, fileName)); + File.WriteAllBytes(file.FullName, new byte[3]); + + // Execute. + await client.ObjectFileOperations.UploadTemporaryFileBlockBeginAsync(file); + + // Verify. + var started = (UploadInfo)requests[0].Parameters.First(p => p.Type == ParameterType.RequestBody).Value; + Assert.AreEqual(expectedExtension, started.Extension); + } + finally + { + directory.Delete(true); + } + } + + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_FileName_WithExtension() + { + await AssertFileNameSplit("report.pdf", "report", "pdf"); + } + + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_FileName_SeveralDots() + { + await AssertFileNameSplit("archive.tar.gz", "archive.tar", "gz"); + } + + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_FileName_NoExtension() + { + await AssertFileNameSplit("README", "README", ""); + } + + [TestMethod] + public async Task UploadTemporaryFileInBlocksAsync_FileName_OnlyExtension() + { + await AssertFileNameSplit(".gitignore", "", "gitignore"); + } + + [TestMethod] + public async Task UploadTemporaryFileBlockBeginAsync_FileInfo_WithExtension() + { + await AssertFileInfoExtension("upload.txt", "txt"); + } + + [TestMethod] + public async Task UploadTemporaryFileBlockBeginAsync_FileInfo_NoExtension() + { + await AssertFileInfoExtension("upload", ""); + } + + /// + /// Ensures that the overload that takes the block size from the block counts the blocks correctly + /// when the size divides evenly by the block size. + /// + [TestMethod] + public async Task UploadTemporaryFileBlockAsync_BlockSizeFromBlock() + { + var requests = new List(); + var client = CreateClient(requests); + + // Execute. + await client.ObjectFileOperations.UploadTemporaryFileBlockAsync(7, 8, 4, new byte[4]); + + // Verify. + Assert.AreEqual(1, requests.Count); + Assert.AreEqual("2", GetQueryParameter(requests[0], "resumableChunkNumber")); + Assert.AreEqual("2", GetQueryParameter(requests[0], "resumableTotalChunks")); + Assert.AreEqual("4", GetQueryParameter(requests[0], "resumableChunkSize")); + } + + /// + /// Ensures that an offset which is not at a block boundary is rejected. + /// + [TestMethod] + [ExpectedException(typeof(ArgumentOutOfRangeException))] + public async Task UploadTemporaryFileBlockAsync_OffsetNotAtBlockBoundary_Throws() + { + var client = CreateClient(new List()); + await client.ObjectFileOperations.UploadTemporaryFileBlockAsync(7, 10, 6, new byte[4], 4); + } + + /// + /// Ensures that a block larger than the block size is rejected. + /// + [TestMethod] + [ExpectedException(typeof(ArgumentException))] + public async Task UploadTemporaryFileBlockAsync_BlockLargerThanBlockSize_Throws() + { + var client = CreateClient(new List()); + await client.ObjectFileOperations.UploadTemporaryFileBlockAsync(7, 10, 0, new byte[5], 4); + } + } +} diff --git a/MFaaP.MFWSClient/MFWSVaultObjectFileOperations.cs b/MFaaP.MFWSClient/MFWSVaultObjectFileOperations.cs index 7727ae3..eab0bbc 100644 --- a/MFaaP.MFWSClient/MFWSVaultObjectFileOperations.cs +++ b/MFaaP.MFWSClient/MFWSVaultObjectFileOperations.cs @@ -20,7 +20,30 @@ internal MFWSVaultObjectFileOperations(MFWSClientBase client) : base(client) { } - + + /// + /// Gets the title of a file name: the part before its last dot, or the whole name if it has no dot. + /// + /// The file name, e.g. "report.pdf". + /// The title, e.g. "report". + internal static string GetFileTitle(string fileName) + { + var dot = fileName?.LastIndexOf('.') ?? -1; + return dot < 0 ? fileName ?? "" : fileName.Substring(0, dot); + } + + /// + /// Gets the extension of a file name without the dot, as M-Files expects it: the part after its last dot, + /// or an empty string if it has no dot. + /// + /// The file name, e.g. "report.pdf". + /// The extension, e.g. "pdf". + internal static string GetFileExtension(string fileName) + { + var dot = fileName?.LastIndexOf('.') ?? -1; + return dot < 0 ? "" : fileName.Substring(dot + 1); + } + #region Downloading files /// @@ -438,7 +461,7 @@ public async Task UploadFilesAsync(CancellationToken token, params var uploadInfo = data[i]; var file = files[i]; uploadInfo.Title = file.Name; - uploadInfo.Extension = file.Extension.Substring(1); // Remove the dot. + uploadInfo.Extension = GetFileExtension(file.Name); uploadInfo.Size = file.Length; } @@ -906,9 +929,7 @@ public Task UploadTemporaryFileBlockBeginAsync(FileInfo fileInfo, Ca // Add the request body. var uploadInfo = new UploadInfo() { - Extension = fileInfo.Extension.StartsWith(".") - ? fileInfo.Extension.Substring(1) // Remove the dot. - : fileInfo.Extension, + Extension = GetFileExtension(fileInfo.Name), Size = fileInfo.Length, Title = fileInfo.Name }; @@ -974,28 +995,62 @@ public UploadInfo UploadTemporaryFileBlockBegin(FileInfo fileInfo, CancellationT /// The binary data for the chunk. /// A cancellation token for the request. /// Information on the upload. + /// + /// The block size is taken from the length of , so every block must be of the same size. + /// To upload a shorter last block, use the overload that takes the block size. + /// THIS METHOD IS NOT DOCUMENTED SO DO NOT USE IN PRODUCTION ENVIRONMENTS. + /// + public Task UploadTemporaryFileBlockAsync(int uploadId, long totalSizeInBytes, long offset, byte[] block, + CancellationToken token = default) + { + // Sanity. + if (null == block) + throw new ArgumentNullException(nameof(block)); + + // Use the other overload. + return this.UploadTemporaryFileBlockAsync(uploadId, totalSizeInBytes, offset, block, block.Length, token); + } + + /// + /// Uploads a block of data to a temporary file. + /// + /// The ID of the temporary upload returned from . + /// The total size of the file (in bytes). + /// The offset (in bytes) to start writing this chunk. Must be a multiple of . + /// The binary data for the chunk. Only the last block may be shorter than . + /// The size (in bytes) of every block but the last. + /// A cancellation token for the request. + /// Information on the upload. /// THIS METHOD IS NOT DOCUMENTED SO DO NOT USE IN PRODUCTION ENVIRONMENTS. public async Task UploadTemporaryFileBlockAsync(int uploadId, long totalSizeInBytes, long offset, byte[] block, - CancellationToken token = default) + int blockSize, CancellationToken token = default) { + // Sanity. + if (null == block) + throw new ArgumentNullException(nameof(block)); + if (blockSize <= 0) + throw new ArgumentOutOfRangeException(nameof(blockSize), "The block size must be greater than zero."); + if (block.Length > blockSize) + throw new ArgumentException("The block cannot be larger than the block size.", nameof(block)); + if (offset < 0 || offset >= totalSizeInBytes || offset % blockSize != 0) + throw new ArgumentOutOfRangeException(nameof(offset), "The offset must be a multiple of the block size within the file."); + // Create the request. var request = new RestRequest($"/REST/files/upload/{uploadId}.aspx"); - // Calculate the block data. - var chunkSize = block.Length; // All chunks must be the same size, aside from the last one. - var resumableChunkNumber = offset / chunkSize + 1; // Chunks start at 1. - var totalChunks = (totalSizeInBytes / chunkSize) + 1; // Chunks start at 1. + // Calculate the block data. All chunks must be the same size, aside from the last one. + var resumableChunkNumber = offset / blockSize + 1; // Chunks start at 1. + var totalChunks = (totalSizeInBytes + blockSize - 1) / blockSize; // The last chunk may be shorter. // Add the binary data to the request body. request.AddParameter("application/octet-stream", block, ParameterType.RequestBody); // Add the querystring arguments. + // The server calculates the offset of the chunk from its number and the chunk size, + // and commits the upload after the last chunk. request.AddQueryParameter("resumableChunkNumber", resumableChunkNumber.ToString()); request.AddQueryParameter("resumableTotalChunks", totalChunks.ToString()); - if (resumableChunkNumber > 0) - { - request.AddQueryParameter("resumableChunkSize", chunkSize.ToString()); - } + request.AddQueryParameter("resumableChunkSize", blockSize.ToString()); // Execute the request. var response = await this.MFWSClient.Post(request, token) @@ -1027,6 +1082,27 @@ public UploadInfo UploadTemporaryFileBlock(int uploadId, long totalSizeInBytes, } + /// + /// Uploads a block of data to a temporary file. + /// + /// The ID of the temporary upload returned from . + /// The total size of the file (in bytes). + /// The offset (in bytes) to start writing this chunk. Must be a multiple of . + /// The binary data for the chunk. Only the last block may be shorter than . + /// The size (in bytes) of every block but the last. + /// A cancellation token for the request. + /// Information on the upload. + /// THIS METHOD IS NOT DOCUMENTED SO DO NOT USE IN PRODUCTION ENVIRONMENTS. + public UploadInfo UploadTemporaryFileBlock(int uploadId, long totalSizeInBytes, long offset, byte[] block, + int blockSize, CancellationToken token = default) + { + // Execute the async method. + return this.UploadTemporaryFileBlockAsync(uploadId, totalSizeInBytes, offset, block, blockSize, token) + .ConfigureAwait(false) + .GetAwaiter() + .GetResult(); + } + #endregion #region Complete/commit temporary file @@ -1109,15 +1185,11 @@ public async Task UploadTemporaryFileInBlocksAsync( // Start the upload. var uploadInfo = new UploadInfo() { - Title = newFileName.Contains(".") - ? newFileName.Substring(0, newFileName.LastIndexOf(".")) - : newFileName, - Extension = newFileName.Contains(".") - ? newFileName.Substring(newFileName.LastIndexOf(".")) - : "", + Title = GetFileTitle(newFileName), + Extension = GetFileExtension(newFileName), Size = fileToUpload.Length }; - uploadInfo = await this.UploadTemporaryFileBlockBeginAsync(uploadInfo, token); + uploadInfo.UploadID = (await this.UploadTemporaryFileBlockBeginAsync(uploadInfo, token)).UploadID; // Iterate over the file blocks and upload them. long offset = 0; @@ -1125,18 +1197,24 @@ public async Task UploadTemporaryFileInBlocksAsync( if (blockSize > totalSizeInBytes) blockSize = (int)totalSizeInBytes; - var buffer = new byte[blockSize]; do { - // Read the data from the stream. - var bytesRead = await fileToUpload.ReadAsync(buffer, 0, blockSize, token); + // Read the block from the stream. Only the last block may be shorter. + var block = new byte[(int)Math.Min(blockSize, totalSizeInBytes - offset)]; + var read = 0; + while (read < block.Length) + { + var readNow = await fileToUpload.ReadAsync(block, read, block.Length - read, token); + if (readNow == 0) + throw new EndOfStreamException("The stream ended before its length."); + read += readNow; + } - // Upload the block. - var ui = await this.UploadTemporaryFileBlockAsync(uploadInfo.UploadID, totalSizeInBytes, offset, buffer, token); - uploadInfo = ui; + // Upload the block. The response has no title or extension, so the upload info is kept. + await this.UploadTemporaryFileBlockAsync(uploadInfo.UploadID, totalSizeInBytes, offset, block, blockSize, token); // Move onwards. - offset += bytesRead; + offset += block.Length; } while (offset < totalSizeInBytes); // Commit the file.