From 33be0e5039636c2ea8989ab0d180ae8ff82d882d Mon Sep 17 00:00:00 2001 From: Diego Santo Date: Tue, 7 Oct 2025 22:00:54 -0300 Subject: [PATCH 1/2] fix: implement in-memory caching for resumable uploads, optimize error handling, and refactor upload progress reporting. --- Storage/Extensions/HttpClientProgress.cs | 104 ++++++++++++++----- Storage/UploadMemoryCache.cs | 123 +++++++++++++++++++++++ StorageTests/StorageFileTests.cs | 5 +- 3 files changed, 205 insertions(+), 27 deletions(-) create mode 100644 Storage/UploadMemoryCache.cs diff --git a/Storage/Extensions/HttpClientProgress.cs b/Storage/Extensions/HttpClientProgress.cs index 6831352..44bce5a 100644 --- a/Storage/Extensions/HttpClientProgress.cs +++ b/Storage/Extensions/HttpClientProgress.cs @@ -6,6 +6,8 @@ using System.Threading.Tasks; using BirdMessenger; using BirdMessenger.Collections; +using BirdMessenger.Delegates; +using BirdMessenger.Infrastructure; using Newtonsoft.Json; using Supabase.Storage.Exceptions; @@ -175,7 +177,7 @@ public static async Task UploadAsync( } } - var response = await client.PostAsync(uri, content, cancellationToken); + var response = await client.PostAsync(uri, content, cancellationToken); if (!response.IsSuccessStatusCode) { @@ -266,31 +268,46 @@ private static async Task ResumableUploadAsync( } } - var createOption = new TusCreateRequestOption() + string? cacheKey = null; + if (metadata != null) + cacheKey = + $"{metadata["bucketName"]}/{metadata["objectName"]}/{metadata["contentType"]}"; + + UploadMemoryCache.TryGet(cacheKey, out var upload); + Uri? fileLocation = null; + if (upload == null) { - Endpoint = uri, - Metadata = metadata, - UploadLength = fileStream.Length, - }; + var createOption = new TusCreateRequestOption() + { + Endpoint = uri, + Metadata = metadata, + UploadLength = fileStream.Length, + }; + + TusCreateResponse responseCreate; + try + { + responseCreate = await client.TusCreateAsync(createOption, cancellationToken); + + fileLocation = responseCreate.FileLocation; + UploadMemoryCache.Set(cacheKey, fileLocation.ToString()); + } + catch (TusException error) + { + throw await HandleResponseError(error); + } + } - var responseCreate = await client.TusCreateAsync(createOption, cancellationToken); + if (upload != null) + fileLocation = new Uri(upload); var patchOption = new TusPatchRequestOption { - FileLocation = responseCreate.FileLocation, + FileLocation = fileLocation, Stream = fileStream, UploadBufferSize = 6 * 1024 * 1024, UploadType = UploadType.Chunk, - OnProgressAsync = x => - { - if (progress == null) - return Task.CompletedTask; - - var uploadedProgress = (float)x.UploadedSize / x.TotalSize * 100f; - progress.Report(uploadedProgress); - - return Task.CompletedTask; - }, + OnProgressAsync = x => ReportProgressAsync(progress, x), OnCompletedAsync = _ => Task.CompletedTask, OnFailedAsync = _ => Task.CompletedTask, }; @@ -300,19 +317,54 @@ private static async Task ResumableUploadAsync( if (responsePatch.OriginResponseMessage.IsSuccessStatusCode) return responsePatch.OriginResponseMessage; - var httpContent = await responsePatch.OriginResponseMessage.Content.ReadAsStringAsync(); + throw await HandleResponseError(responsePatch.OriginResponseMessage); + } + + private static Task ReportProgressAsync( + IProgress? progress, + UploadProgressEvent progressInfo + ) + { + if (progress == null) + return Task.CompletedTask; + + var uploadedProgress = (float)progressInfo.UploadedSize / progressInfo.TotalSize * 100f; + progress.Report(uploadedProgress); + + return Task.CompletedTask; + } + + private static async Task HandleResponseError( + HttpResponseMessage response + ) + { + var httpContent = await response.Content.ReadAsStringAsync(); var errorResponse = JsonConvert.DeserializeObject(httpContent); - var e = new SupabaseStorageException(errorResponse?.Message ?? httpContent) + var error = new SupabaseStorageException(errorResponse?.Message ?? httpContent) + { + Content = httpContent, + Response = response, + StatusCode = errorResponse?.StatusCode ?? (int)response.StatusCode, + }; + error.AddReason(); + + return error; + } + + private static async Task HandleResponseError( + TusException response + ) + { + var httpContent = await response.OriginHttpResponse.Content.ReadAsStringAsync(); + var error = new SupabaseStorageException(httpContent) { Content = httpContent, - Response = responsePatch.OriginResponseMessage, - StatusCode = - errorResponse?.StatusCode - ?? (int)responsePatch.OriginResponseMessage.StatusCode, + Response = response.OriginHttpResponse, + StatusCode = (int)response.OriginHttpResponse.StatusCode, }; + error.AddReason(); - e.AddReason(); - throw e; + return error; } } } diff --git a/Storage/UploadMemoryCache.cs b/Storage/UploadMemoryCache.cs new file mode 100644 index 0000000..f8a89b6 --- /dev/null +++ b/Storage/UploadMemoryCache.cs @@ -0,0 +1,123 @@ +using System; +using System.Collections.Concurrent; +using System.Threading; + +namespace Supabase.Storage; + +public class UploadMemoryCache +{ + // Thread-safe in-memory cache for resumable upload URLs keyed by an identifier (e.g., file path or upload id). + // Uses simple sliding expiration. + private static readonly ConcurrentDictionary _cache = new(); + + // Default sliding expiration for cached URLs. + private static TimeSpan _defaultTtl = TimeSpan.FromMinutes(60); + + private static long _version; // helps with testing/observability if needed + + private sealed class CacheEntry + { + public string Url { get; } + public DateTimeOffset Expiration { get; private set; } + public TimeSpan Ttl { get; } + + public CacheEntry(string url, TimeSpan ttl) + { + Url = url; + Ttl = ttl <= TimeSpan.Zero ? TimeSpan.FromMinutes(5) : ttl; + Touch(); + } + + public void Touch() + { + Expiration = DateTimeOffset.UtcNow.Add(Ttl); + } + + public bool IsExpired() => DateTimeOffset.UtcNow >= Expiration; + } + + // Sets the default time-to-live for future cache entries. + public static void SetDefaultTtl(TimeSpan ttl) + { + _defaultTtl = ttl <= TimeSpan.Zero ? TimeSpan.FromMinutes(5) : ttl; + } + + // Store or update the resumable upload URL for the provided key. + public static void Set(string key, string url, TimeSpan? ttl = null) + { + if (string.IsNullOrWhiteSpace(key)) + throw new ArgumentException("Key must be provided.", nameof(key)); + if (string.IsNullOrWhiteSpace(url)) + throw new ArgumentException("Url must be provided.", nameof(url)); + + var entryTtl = ttl.GetValueOrDefault(_defaultTtl); + _cache.AddOrUpdate( + key, + _ => new CacheEntry(url, entryTtl), + (_, existing) => new CacheEntry(url, entryTtl) + ); + + Interlocked.Increment(ref _version); + CleanupIfNeeded(); + } + + // Try to get a cached URL. Refreshes sliding expiration on successful hit. + public static bool TryGet(string key, out string? url) + { + url = null; + if (string.IsNullOrWhiteSpace(key)) + return false; + + if (_cache.TryGetValue(key, out var entry)) + { + if (entry.IsExpired()) + { + // Evict expired entry + _cache.TryRemove(key, out _); + return false; + } + + // Sliding expiration + entry.Touch(); + url = entry.Url; + return true; + } + + return false; + } + + // Remove a cached URL. + public static bool Remove(string key) + { + if (string.IsNullOrWhiteSpace(key)) + return false; + + var removed = _cache.TryRemove(key, out _); + if (removed) + Interlocked.Increment(ref _version); + return removed; + } + + // Clear all cached URLs. + public static void Clear() + { + _cache.Clear(); + Interlocked.Increment(ref _version); + } + + // Optionally expose count for diagnostics. + public static int Count => _cache.Count; + + // Simple opportunistic cleanup to remove expired entries. + private static void CleanupIfNeeded() + { + // Cheap scan for expired entries. No need for strict guarantees. + foreach (var kvp in _cache) + { + if (kvp.Value.IsExpired()) + { + _cache.TryRemove(kvp.Key, out _); + } + } + } +} \ No newline at end of file diff --git a/StorageTests/StorageFileTests.cs b/StorageTests/StorageFileTests.cs index f2d2f82..3a80175 100644 --- a/StorageTests/StorageFileTests.cs +++ b/StorageTests/StorageFileTests.cs @@ -270,7 +270,7 @@ public async Task UploadOrResumeByteWithInterruptionAndResume() var options = new FileOptions { Duplex = "duplex", Metadata = metadata }; - using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(300)); + using var cts = new CancellationTokenSource(); try { @@ -280,6 +280,9 @@ await _bucket.UploadOrResume( options, (_, progress) => { + if (progress > 20) + cts.Cancel(); + Console.WriteLine($"First upload progress: {progress}"); firstUploadProgressTriggered.TrySetResult(true); }, From 3c1f7cd2b0bdcbcb8dcaa4af7a2e84da633ece2a Mon Sep 17 00:00:00 2001 From: Diego Santo Date: Sat, 18 Oct 2025 18:05:48 -0300 Subject: [PATCH 2/2] fix: add remove from cache after complete --- Storage/Extensions/HttpClientProgress.cs | 32 ++++++------ Storage/StorageFileApi.cs | 4 +- Storage/UploadMemoryCache.cs | 64 +++++++++++++++--------- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/Storage/Extensions/HttpClientProgress.cs b/Storage/Extensions/HttpClientProgress.cs index 44bce5a..6b768bb 100644 --- a/Storage/Extensions/HttpClientProgress.cs +++ b/Storage/Extensions/HttpClientProgress.cs @@ -201,8 +201,8 @@ public static Task UploadOrContinueFileAsync( this HttpClient client, Uri uri, string filePath, + MetadataCollection metadata, Dictionary? headers = null, - MetadataCollection? metadata = null, Progress? progress = null, CancellationToken cancellationToken = default ) @@ -212,8 +212,8 @@ public static Task UploadOrContinueFileAsync( client, uri, fileStream, - headers, metadata, + headers, progress, cancellationToken ); @@ -223,8 +223,8 @@ public static Task UploadOrContinueByteAsync( this HttpClient client, Uri uri, byte[] data, + MetadataCollection metadata, Dictionary? headers = null, - MetadataCollection? metadata = null, Progress? progress = null, CancellationToken cancellationToken = default ) @@ -234,8 +234,8 @@ public static Task UploadOrContinueByteAsync( client, uri, stream, - headers, metadata, + headers, progress, cancellationToken ); @@ -245,8 +245,8 @@ private static async Task ResumableUploadAsync( this HttpClient client, Uri uri, Stream fileStream, + MetadataCollection metadata, Dictionary? headers = null, - MetadataCollection? metadata = null, IProgress? progress = null, CancellationToken cancellationToken = default ) @@ -268,10 +268,8 @@ private static async Task ResumableUploadAsync( } } - string? cacheKey = null; - if (metadata != null) - cacheKey = - $"{metadata["bucketName"]}/{metadata["objectName"]}/{metadata["contentType"]}"; + var cacheKey = + $"{metadata["bucketName"]}/{metadata["objectName"]}/{metadata["contentType"]}"; UploadMemoryCache.TryGet(cacheKey, out var upload); Uri? fileLocation = null; @@ -284,13 +282,15 @@ private static async Task ResumableUploadAsync( UploadLength = fileStream.Length, }; - TusCreateResponse responseCreate; try { - responseCreate = await client.TusCreateAsync(createOption, cancellationToken); - + var responseCreate = await client.TusCreateAsync( + createOption, + cancellationToken + ); + fileLocation = responseCreate.FileLocation; - UploadMemoryCache.Set(cacheKey, fileLocation.ToString()); + UploadMemoryCache.Set(cacheKey, fileLocation.ToString()); } catch (TusException error) { @@ -308,7 +308,11 @@ private static async Task ResumableUploadAsync( UploadBufferSize = 6 * 1024 * 1024, UploadType = UploadType.Chunk, OnProgressAsync = x => ReportProgressAsync(progress, x), - OnCompletedAsync = _ => Task.CompletedTask, + OnCompletedAsync = _ => + { + UploadMemoryCache.Remove(cacheKey); + return Task.CompletedTask; + }, OnFailedAsync = _ => Task.CompletedTask, }; diff --git a/Storage/StorageFileApi.cs b/Storage/StorageFileApi.cs index 1c26ab2..a8b9f5c 100644 --- a/Storage/StorageFileApi.cs +++ b/Storage/StorageFileApi.cs @@ -744,8 +744,8 @@ private async Task UploadOrContinue( await Helpers.HttpUploadClient!.UploadOrContinueFileAsync( uri, localPath, - headers, metadata, + headers, progress, cancellationToken ); @@ -792,8 +792,8 @@ private async Task UploadOrContinue( await Helpers.HttpUploadClient!.UploadOrContinueByteAsync( uri, data, - headers, metadata, + headers, progress, cancellationToken ); diff --git a/Storage/UploadMemoryCache.cs b/Storage/UploadMemoryCache.cs index f8a89b6..6829246 100644 --- a/Storage/UploadMemoryCache.cs +++ b/Storage/UploadMemoryCache.cs @@ -4,13 +4,13 @@ namespace Supabase.Storage; +/// +/// Provides thread-safe in-memory caching for resumable upload URLs with sliding expiration. +/// public class UploadMemoryCache { - // Thread-safe in-memory cache for resumable upload URLs keyed by an identifier (e.g., file path or upload id). - // Uses simple sliding expiration. private static readonly ConcurrentDictionary _cache = new(); - // Default sliding expiration for cached URLs. private static TimeSpan _defaultTtl = TimeSpan.FromMinutes(60); private static long _version; // helps with testing/observability if needed @@ -36,13 +36,23 @@ public void Touch() public bool IsExpired() => DateTimeOffset.UtcNow >= Expiration; } - // Sets the default time-to-live for future cache entries. + /// + /// Sets the default time-to-live duration for future cache entries. + /// + /// The time-to-live duration. If less than or equal to zero, defaults to 5 minutes. public static void SetDefaultTtl(TimeSpan ttl) { _defaultTtl = ttl <= TimeSpan.Zero ? TimeSpan.FromMinutes(5) : ttl; } - // Store or update the resumable upload URL for the provided key. + // Store or upate the resumable upload URL for the provided key. + /// + /// Stores or updates a resumable upload URL in the cache for the specified key. + /// + /// The unique identifier for the cached URL. + /// The resumable upload URL to cache. + /// Optional time-to-live duration. If not specified, uses the default TTL. + /// Thrown when key or url is null, empty, or whitespace. public static void Set(string key, string url, TimeSpan? ttl = null) { if (string.IsNullOrWhiteSpace(key)) @@ -61,32 +71,36 @@ public static void Set(string key, string url, TimeSpan? ttl = null) CleanupIfNeeded(); } - // Try to get a cached URL. Refreshes sliding expiration on successful hit. + /// + /// Attempts to retrieve a cached URL by its key. Updates the sliding expiration on successful retrieval. + /// + /// The unique identifier for the cached URL. + /// When this method returns, contains the cached URL if found; otherwise, null. + /// True if the URL was found in the cache; otherwise, false. public static bool TryGet(string key, out string? url) { url = null; if (string.IsNullOrWhiteSpace(key)) return false; - if (_cache.TryGetValue(key, out var entry)) + if (!_cache.TryGetValue(key, out var entry)) return false; + if (entry.IsExpired()) { - if (entry.IsExpired()) - { - // Evict expired entry - _cache.TryRemove(key, out _); - return false; - } - - // Sliding expiration - entry.Touch(); - url = entry.Url; - return true; + _cache.TryRemove(key, out _); + return false; } - return false; + entry.Touch(); + url = entry.Url; + return true; + } - // Remove a cached URL. + /// + /// Removes a cached URL by its key. + /// + /// The unique identifier for the cached URL to remove. + /// True if the URL was successfully removed; otherwise, false. public static bool Remove(string key) { if (string.IsNullOrWhiteSpace(key)) @@ -98,20 +112,22 @@ public static bool Remove(string key) return removed; } - // Clear all cached URLs. + /// + /// Removes all cached URLs from the cache. + /// public static void Clear() { _cache.Clear(); Interlocked.Increment(ref _version); } - // Optionally expose count for diagnostics. + /// + /// Gets the current number of entries in the cache. + /// public static int Count => _cache.Count; - // Simple opportunistic cleanup to remove expired entries. private static void CleanupIfNeeded() { - // Cheap scan for expired entries. No need for strict guarantees. foreach (var kvp in _cache) { if (kvp.Value.IsExpired())