Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/Foundatio/Caching/ScopedCacheClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,22 @@ namespace Foundatio.Caching;

public class ScopedHybridCacheClient : ScopedCacheClient, IHybridCacheClient
{
public ScopedHybridCacheClient(IHybridCacheClient client, string scope = null) : base(client, scope) { }
public ScopedHybridCacheClient(IHybridCacheClient client, string scope = null, bool shouldDispose = false) : base(client, scope, shouldDispose) { }
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ScopedHybridCacheClient constructor maintains the default value for scope parameter while the base ScopedCacheClient constructor removes it. This inconsistency could lead to confusion and compilation errors when the base constructor is called.

Copilot uses AI. Check for mistakes.
}

public class ScopedCacheClient : ICacheClient, IHaveLogger, IHaveLoggerFactory, IHaveTimeProvider, IHaveResiliencePolicyProvider
{
private string _keyPrefix;
private bool _isLocked;
private readonly object _lock = new();
private readonly bool _shouldDispose;

Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor signature change removes the default value for the scope parameter. This is a breaking change that could affect existing code that relied on new ScopedCacheClient(client) without specifying a scope. Consider adding an overload that preserves the original signature: public ScopedCacheClient(ICacheClient client, string scope = null, bool shouldDispose = false)

Suggested change
public ScopedCacheClient(ICacheClient client, string scope = null, bool shouldDispose = false)
: this(client, scope, shouldDispose)
{
}

Copilot uses AI. Check for mistakes.
public ScopedCacheClient(ICacheClient client, string scope = null)
public ScopedCacheClient(ICacheClient client, string scope, bool shouldDispose = false)
{
UnscopedCache = client ?? new NullCacheClient();
_isLocked = scope != null;
Scope = !String.IsNullOrWhiteSpace(scope) ? scope.Trim() : null;
_shouldDispose = shouldDispose;

_keyPrefix = Scope != null ? String.Concat(Scope, ":") : String.Empty;
}
Expand Down Expand Up @@ -253,5 +255,9 @@ public Task<CacheValue<ICollection<T>>> GetListAsync<T>(string key, int? page =
return UnscopedCache.GetListAsync<T>(GetUnscopedCacheKey(key), page, pageSize);
}

public void Dispose() { }
public void Dispose()
{
if (_shouldDispose)
UnscopedCache.Dispose();
}
}
10 changes: 8 additions & 2 deletions src/Foundatio/Storage/ScopedFileStorage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ namespace Foundatio.Storage;
public class ScopedFileStorage : IFileStorage, IHaveLogger, IHaveLoggerFactory, IHaveTimeProvider, IHaveResiliencePolicyProvider
{
private readonly string _pathPrefix;
private readonly bool _shouldDispose;

public ScopedFileStorage(IFileStorage storage, string scope)
public ScopedFileStorage(IFileStorage storage, string scope, bool shouldDispose = false)
{
UnscopedStorage = storage ?? throw new ArgumentNullException(nameof(storage));
Scope = !String.IsNullOrWhiteSpace(scope) ? scope.Trim().NormalizePath() : null;
_pathPrefix = Scope != null ? String.Concat(Scope, "/") : String.Empty;
_shouldDispose = shouldDispose;

// NOTE: we can't really check reliably using Path.GetInvalidPathChars() because each storage implementation and platform could be different.
if (Scope is not null && Scope.Contains("*"))
Expand Down Expand Up @@ -142,5 +144,9 @@ private async Task<NextPageResult> NextPage(PagedFileListResult result)
};
}

public void Dispose() { }
public void Dispose()
{
if (_shouldDispose)
UnscopedStorage.Dispose();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
using System;
using Foundatio.Caching;
using Xunit;
using Xunit.Abstractions;

namespace Foundatio.Tests.Caching;
public class ScopedCacheClientShouldDisposeTests : IDisposable
{
private readonly ITestOutputHelper _output;

public ScopedCacheClientShouldDisposeTests(ITestOutputHelper output)
{
_output = output;
}

[Fact]
public void Dispose_DefaultBehavior_DoesNotDisposeUnderlyingCache()
{
// Arrange
var innerCache = new TrackableDisposableCacheClient();
var scopedCache = new ScopedCacheClient(innerCache, "test");

// Act
scopedCache.Dispose();

// Assert
Assert.False(innerCache.WasDisposed);
}

[Fact]
public void Dispose_ShouldDisposeTrue_DisposesUnderlyingCache()
{
// Arrange
var innerCache = new TrackableDisposableCacheClient();
var scopedCache = new ScopedCacheClient(innerCache, "test", shouldDispose: true);

// Act
scopedCache.Dispose();

// Assert
Assert.True(innerCache.WasDisposed);
}

[Fact]
public void Dispose_WithUsingStatementAndShouldDisposeTrue_DisposesUnderlyingCache()
{
// Arrange
var innerCache = new TrackableDisposableCacheClient();

// Act
using (var scopedCache = new ScopedCacheClient(innerCache, "test", shouldDispose: true))
{
// No operations needed
}

// Assert
Assert.True(innerCache.WasDisposed);
}

[Fact]
public void Dispose_WithUsingStatementAndShouldDisposeFalse_DoesNotDisposeUnderlyingCache()
{
// Arrange
var innerCache = new TrackableDisposableCacheClient();

// Act
using (var scopedCache = new ScopedCacheClient(innerCache, "test", shouldDispose: false))
{
// No operations needed
}

// Assert
Assert.False(innerCache.WasDisposed);
}

public void Dispose()
{
// Cleanup if needed
}

private class TrackableDisposableCacheClient : InMemoryCacheClient
{
public bool WasDisposed { get; private set; }

public new void Dispose()
{
WasDisposed = true;
base.Dispose();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
using System;
using Foundatio.Storage;
using Xunit;
using Xunit.Abstractions;

namespace Foundatio.Tests.Storage;
public class ScopedFileStorageShouldDisposeTests : IDisposable
{
private readonly ITestOutputHelper _output;

public ScopedFileStorageShouldDisposeTests(ITestOutputHelper output)
{
_output = output;
}

[Fact]
public void Dispose_DefaultBehavior_DoesNotDisposeUnderlyingStorage()
{
// Arrange
var innerStorage = new TrackableDisposableFileStorage();
var scopedStorage = new ScopedFileStorage(innerStorage, "test");

// Act
scopedStorage.Dispose();

// Assert
Assert.False(innerStorage.WasDisposed);
}

[Fact]
public void Dispose_ShouldDisposeTrue_DisposesUnderlyingStorage()
{
// Arrange
var innerStorage = new TrackableDisposableFileStorage();
var scopedStorage = new ScopedFileStorage(innerStorage, "test", shouldDispose: true);

// Act
scopedStorage.Dispose();

// Assert
Assert.True(innerStorage.WasDisposed);
}

[Fact]
public void Dispose_WithUsingStatementAndShouldDisposeTrue_DisposesUnderlyingStorage()
{
// Arrange
var innerStorage = new TrackableDisposableFileStorage();

// Act
using (var scopedStorage = new ScopedFileStorage(innerStorage, "test", shouldDispose: true))
{
// No operations needed
}

// Assert
Assert.True(innerStorage.WasDisposed);
}

[Fact]
public void Dispose_WithUsingStatementAndShouldDisposeFalse_DoesNotDisposeUnderlyingStorage()
{
// Arrange
var innerStorage = new TrackableDisposableFileStorage();

// Act
using (var scopedStorage = new ScopedFileStorage(innerStorage, "test", shouldDispose: false))
{
// No operations needed
}

// Assert
Assert.False(innerStorage.WasDisposed);
}

public void Dispose()
{
// Cleanup if needed
}

private class TrackableDisposableFileStorage : InMemoryFileStorage
{
public bool WasDisposed { get; private set; }

public new void Dispose()
{
WasDisposed = true;
base.Dispose();
}
}
}
Loading