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.

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

[API Proposal]: Provide a configurable overload of MemoryCacheEntryOptions for the commonly used method GetOrCreate in MemoryCacheExtension #92101

Closed
git102347501 opened this issue Sep 15, 2023 · 8 comments · Fixed by #94335
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Caching help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@git102347501
Copy link

git102347501 commented Sep 15, 2023

Background and motivation

In the MemoryCache extension class, The commonly used GetOrCreateAsync() should have an overloaded method for setting the MemoryCacheEntryOptions property

When we use the caching function. It is usually necessary to set MemoryCacheEntryOptions to determine the cache timeout and other configuration properties. If GetOrCreate does not provide an overload of valid MemoryCacheEntryOptions, this method will become unavailable in the scenario where MemoryCacheEntryOptions need to be configured. Users can only go back and select Get and call the Set method to configure.

API Proposal

namespace System.Collections.Generic;

public class MemoryCacheExtensions
{

        /// <summary>
        /// Gets the value associated with this key if it exists, or generates a new entry using the provided key and a value from the given factory if the key is not found.
        /// </summary>
        /// <typeparam name="TItem">The type of the object to get.</typeparam>
        /// <param name="cache">The <see cref="IMemoryCache"/> instance this method extends.</param>
        /// <param name="key">The key of the entry to look for or create.</param>
        /// <param name="factory">The factory that creates the value associated with this key if the key does not exist in the cache.</param>
        /// <param name="options">The existing <see cref="MemoryCacheEntryOptions"/> instance to apply to the new entry.</param>
        /// <returns>The value associated with this key.</returns>
        public static TItem? GetOrCreate<TItem>(this IMemoryCache cache, object key, Func<ICacheEntry, TItem> factory, MemoryCacheEntryOptions? options)
        {
            if (!cache.TryGetValue(key, out object? result))
            {
                using ICacheEntry entry = cache.CreateEntry(key);
                if (options != null) {
                    entry.SetOptions(options);
                }
                result = factory(entry);
                entry.Value = result;
            }

            return (TItem?)result;
        }

        /// <summary>
        /// Asynchronously gets the value associated with this key if it exists, or generates a new entry using the provided key and a value from the given factory if the key is not found.
        /// </summary>
        /// <typeparam name="TItem">The type of the object to get.</typeparam>
        /// <param name="cache">The <see cref="IMemoryCache"/> instance this method extends.</param>
        /// <param name="key">The key of the entry to look for or create.</param>
        /// <param name="factory">The factory task that creates the value associated with this key if the key does not exist in the cache.</param>
        /// <param name="options">The existing <see cref="MemoryCacheEntryOptions"/> instance to apply to the new entry.</param>
        /// <returns>The task object representing the asynchronous operation.</returns>
        public static async Task<TItem?> GetOrCreateAsync<TItem>(this IMemoryCache cache, object key, Func<ICacheEntry, Task<TItem>> factory, MemoryCacheEntryOptions? options)
        {
            if (!cache.TryGetValue(key, out object? result))
            {
                using ICacheEntry entry = cache.CreateEntry(key);
                if (options != null) {
                    entry.SetOptions(options);
                }
                result = await factory(entry).ConfigureAwait(false);
                entry.Value = result;
            }

            return (TItem?)result;
        }


}

API Usage

// GetOrCreateAsync And Set MemoryCacheEntryOptions

  var key = _cache.GetOrCreate("yj-key", c => "key-value", new MemoryCacheEntryOptions()
  {
      AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(1)
  });

  var key = await _cache.GetOrCreateAsync("yj-key", c => "key-value", new MemoryCacheEntryOptions()
  {
      AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(1)
  });

Alternative Designs

You can change the existing GetOrCreate method, add parameters, and set default values to avoid destructive updates, but relatively speaking, overloading a new method is more suitable

Risks

The code generated by the change is all existing code within the extension method, and no new logical code has been generated, resulting in a low risk factor.

But we still need to consider whether the execution order of SetOptions within the method in the above example code will have an impact on the functionality.

@git102347501 git102347501 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 15, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 15, 2023
@ghost
Copy link

ghost commented Sep 15, 2023

Tagging subscribers to this area: @dotnet/area-extensions-caching
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

In the MemoryCache extension class, The commonly used GetOrCreateAsync() should have an overloaded method for setting the MemoryCacheEntryOptions property

API Proposal

namespace System.Collections.Generic;

public class MemoryCacheExtensions
{

        /// <summary>
        /// Gets the value associated with this key if it exists, or generates a new entry using the provided key and a value from the given factory if the key is not found.
        /// </summary>
        /// <typeparam name="TItem">The type of the object to get.</typeparam>
        /// <param name="cache">The <see cref="IMemoryCache"/> instance this method extends.</param>
        /// <param name="key">The key of the entry to look for or create.</param>
        /// <param name="factory">The factory that creates the value associated with this key if the key does not exist in the cache.</param>
        /// <param name="options">The existing <see cref="MemoryCacheEntryOptions"/> instance to apply to the new entry.</param>
        /// <returns>The value associated with this key.</returns>
        public static TItem? GetOrCreate<TItem>(this IMemoryCache cache, object key, Func<ICacheEntry, TItem> factory, MemoryCacheEntryOptions? options)
        {
            if (!cache.TryGetValue(key, out object? result))
            {
                using ICacheEntry entry = cache.CreateEntry(key);
                if (options != null) {
                    entry.SetOptions(options);
                }
                result = factory(entry);
                entry.Value = result;
            }

            return (TItem?)result;
        }

        /// <summary>
        /// Asynchronously gets the value associated with this key if it exists, or generates a new entry using the provided key and a value from the given factory if the key is not found.
        /// </summary>
        /// <typeparam name="TItem">The type of the object to get.</typeparam>
        /// <param name="cache">The <see cref="IMemoryCache"/> instance this method extends.</param>
        /// <param name="key">The key of the entry to look for or create.</param>
        /// <param name="factory">The factory task that creates the value associated with this key if the key does not exist in the cache.</param>
        /// <param name="options">The existing <see cref="MemoryCacheEntryOptions"/> instance to apply to the new entry.</param>
        /// <returns>The task object representing the asynchronous operation.</returns>
        public static async Task<TItem?> GetOrCreateAsync<TItem>(this IMemoryCache cache, object key, Func<ICacheEntry, Task<TItem>> factory, MemoryCacheEntryOptions? options)
        {
            if (!cache.TryGetValue(key, out object? result))
            {
                using ICacheEntry entry = cache.CreateEntry(key);
                if (options != null) {
                    entry.SetOptions(options);
                }
                result = await factory(entry).ConfigureAwait(false);
                entry.Value = result;
            }

            return (TItem?)result;
        }


}

API Usage

// GetOrCreateAsync And Set MemoryCacheEntryOptions

  var key = _cache.GetOrCreate("yj-key", c => "key-value", new MemoryCacheEntryOptions()
  {
      AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(1)
  });

  var key = await _cache.GetOrCreateAsync("yj-key", c => "key-value", new MemoryCacheEntryOptions()
  {
      AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(1)
  });

Alternative Designs

You can change the existing GetOrCreate method, add parameters, and set default values to avoid destructive updates, but relatively speaking, overloading a new method is more suitable

Risks

The code generated by the change is all existing code within the extension method, and no new logical code has been generated, resulting in a low risk factor.

But we still need to consider whether the execution order of SetOptions within the method in the above example code will have an impact on the functionality.

Author: git102347501
Assignees: -
Labels:

api-suggestion, area-Extensions-Caching

Milestone: -

@git102347501 git102347501 changed the title [API Proposal]: [API Proposal]: Provide a configurable overload of MemoryCacheEntryOptions for the commonly used method GetOrCreate in MemoryCacheExtension Sep 15, 2023
@adamsitnik adamsitnik added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Sep 22, 2023
@adamsitnik adamsitnik added this to the 9.0.0 milestone Sep 22, 2023
@adamsitnik
Copy link
Member

It sounds reasonable, I am supportive of this proposal. Let's discuss it at API Review meeting.

@bartonjs
Copy link
Member

bartonjs commented Oct 31, 2023

Video

  • We changed the options parameter to createOptions to clue that they're only utilized on the "OrCreate" path.
namespace Microsoft.Extensions.Caching.Memory;

public partial class CacheExtensions
{
        public static TItem? GetOrCreate<TItem>(
            this IMemoryCache cache,
            object key,
            Func<ICacheEntry, TItem> factory,
            MemoryCacheEntryOptions? createOptions);

        public static async Task<TItem?> GetOrCreateAsync<TItem>(
            this IMemoryCache cache,
            object key,
            Func<ICacheEntry, Task<TItem>> factory,
            MemoryCacheEntryOptions? createOptions);
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 31, 2023
@adamsitnik adamsitnik added the help wanted [up-for-grabs] Good issue for external contributors label Nov 2, 2023
@adamsitnik
Copy link
Member

I've applied "help wanted" label, which means that we would be more than happy to review a community PR that adds these two new methods.

Some hints:

  • before you start working on the issue, please leave a comment here so there is only one person working on it
  • the new methods needs to be added to a ref assembly in their approved form here
  • the implementation needs to be added here
  • the tests should be added here
  • once you build the whole repo by following the steps described in here, you can rebuild only specific parts of it when re-running the tests:
.\dotnet.cmd build .\src\libraries\Microsoft.Extensions.Caching.Abstractions\Microsoft.Extensions.Caching.Abstractions.sln
&& .\dotnet.cmd build .\src\libraries\Microsoft.Extensions.Caching.Memory\tests\Microsoft.Extensions.Caching.Memory.Tests.csproj /t:Test

@WeihanLi
Copy link
Contributor

WeihanLi commented Nov 2, 2023

@adamsitnik could I take this one, and thank you very much for the detailed hints

@adamsitnik
Copy link
Member

could I take this one, and thank you very much for the detailed hints

Yes, of course! I've assigned you, please let me know if you need any help.

@WeihanLi
Copy link
Contributor

WeihanLi commented Nov 2, 2023

@git102347501 would you like to take this since you had drafted a PR before?

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 3, 2023
@git102347501
Copy link
Author

@adamsitnik could I take this one, and thank you very much for the detailed hints

No problem,Thank

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 6, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Caching help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants