Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update CachingService.cs for RemoveAll() method #113

Closed
wants to merge 2 commits into from
Closed

Update CachingService.cs for RemoveAll() method #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f3b5af
Update CachingService.cs
ArnaudB88 Mar 31, 2020
682b314
Update IAppCache.cs
ArnaudB88 Mar 31, 2020
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
11 changes: 9 additions & 2 deletions LazyCache/CachingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace LazyCache
// ReSharper disable once ClassWithVirtualMembersNeverInherited.Global
public class CachingService : IAppCache
{
private readonly Lazy<ICacheProvider> cacheProvider;
private Lazy<ICacheProvider> cacheProvider;

private readonly SemaphoreSlim locker = new SemaphoreSlim(1, 1);

Expand Down Expand Up @@ -141,6 +141,13 @@ public virtual void Remove(string key)
ValidateKey(key);
CacheProvider.Remove(key);
}

public virtual void RemoveAll()
{
CacheProvider.Dispose();
cacheProvider = new Lazy<ICacheProvider>(() =>
Copy link
Contributor

@jnyrup jnyrup Mar 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Initializing a new cacheProvider instead of removing the keys from the existing cacheProvider gives inconsistent behavior to how Remove(key) works.
E.g. if the CachingService was instantiated using any of the constructors except CachingService(), then calling RemoveAll will change the underlying cacheProvider to a DefaultCacheProvider.

Copy link
Author

Choose a reason for hiding this comment

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

You are right.
I made this PR because I use dependency injection to create a CachingService, and the PR #91 doesn't work correctly with DI.
I think it's best to abandon this PR, and edit PR #91 so dependency injection code (asp.net core, ninject) works correctly.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah this sort of change will need to incorporate DI so will close in favour of #91

new MemoryCacheProvider(new MemoryCache(new MemoryCacheOptions())));
}

public virtual ICacheProvider CacheProvider => cacheProvider.Value;

Expand Down Expand Up @@ -299,4 +306,4 @@ protected virtual void ValidateKey(string key)
throw new ArgumentOutOfRangeException(nameof(key), "Cache keys cannot be empty or whitespace");
}
}
}
}
4 changes: 3 additions & 1 deletion LazyCache/IAppCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ public interface IAppCache
Task<T> GetOrAddAsync<T>(string key, Func<ICacheEntry, Task<T>> addItemFactory);

void Remove(string key);

void RemoveAll();
}
}
}