Support for functions that can set TTL in Decorator similar to Match

[ks0m1c]

Proposed Behaviour

Suppose we have a certificate that expires in time_to_expiry_of_cert seconds

@decorate cacheable(cache: Cache,
    key: {ExternalApi.Cert, :get_cert},
    match: &cert_legit/1,
    opts: [ttl: &cert_expiry/1])
# returns %{:ok, %{exp: time_to_expiry_of_cert}}
  defp get_cert(), do: ExternalApi.Cert.get_Cert()


  defp cert_legit({:ok, %{exp: _exp}}), do: true
  defp cert_legit(_), do: false

  defp cert_expiry({:ok, %{exp: exp}}) when is_integer(exp), do: exp
  defp cert_expiry(_), do: 0

This acts similarly to the current that decides whether the code-block evaluation result is cached or not

:match - Match function (term -> boolean | {true, term}) (optional). This function is for matching and decide whether the code-block evaluation result is cached or not. If true the code-block evaluation result is cached as it is (the default). If {true, value} is returned, then the value is what is cached (useful to control what is meant to be cached). Returning false will cause that nothing is stored in the cache.

Current Behaviour

(ArgumentError) expected ttl: to be a valid timeout, got: #Function<0.110703989/1 in cert_expiry>
(nebulex 2.4.2) lib/nebulex/helpers.ex:15: Nebulex.Helpers.get_option/5
        (nebulex 2.4.2) lib/nebulex/cache/entry.ex:244: anonymous fn/6 in Nebulex.Cache.Entry.do_put/5
        (nebulex 2.4.2) lib/nebulex/cache/entry.ex:42: Nebulex.Cache.Entry.put/4
        (nebulex 2.4.2) lib/nebulex/caching.ex:792: Nebulex.Caching.eval_match/5

[cabol]

This is a nice suggestion/feature, and doesn't seem hard to implement, will give it a priority to have it available soon. Thanks!

[cabol]

Hey 👋 ! I pushed a solution for this, but I changed the proposal a bit, like this:

The match function can return:

  * `true` - the code-block evaluation result is cached as it is (the default).
  * `{true, value}` - `value` is cached. This is useful to set what exactly must be cached.
  * `{true, value, opts}` - `value` is cached with the options given by `opts`. This return allows us to set the value to be cached, as well as the runtime options for storing it (e.g.: the `ttl`).
  * `false` - Nothing is cached.

The idea was to be able to define the storing options at runtime, not just the ttl, and also reuse the match option (you can see the details HERE). So, taking the example in the proposal it will look like this:

@decorate cacheable(cache: Cache,
    key: {ExternalApi.Cert, :get_cert},
    match: &cert_legit/1)
# returns %{:ok, %{exp: time_to_expiry_of_cert}}
  defp get_cert(), do: ExternalApi.Cert.get_Cert()


  defp cert_legit({:ok, %{exp: exp}} = ok), do: {true, ok, [ttl: exp]}
  defp cert_legit(_), do: false

Please let me know if that works for you, thanks!

[cabol]

Closing this issue, but feel free to reopen it if you come across issues.

[ks0m1c]

Hey Carlos,

This is clearly a road to a much more elegant and extensible way to solve the problem, was planning to play around with it in the weekend and couldn't get back to you earlier.

Since the cache value is now dynamic should the first return before the cache key is formed also follow that transformation

Currently the subsequent value returns as per that transformation to which its stored to the cache.

Here is an example of such a process

defmodule CacheTest do
  use Nebulex.Caching

@decorate cacheable(cache: Phos.Cache,
    key: {CacheTest, :get_cert},
    match: &time_legit/1)
# returns %{:ok, %{exp: time_to_expiry_of_cert}}
  def get_time() do
    host = "http://127.0.0.1:4040"
    with {:ok, %{body: body}} <- HTTPoison.get(host <> "/ping",[{"Content-Type","application/json"}]) do
      {:ok, %{body: body, exp: 10000}}
      end
  end


  defp time_legit({:ok, %{exp: exp, body: body}} = ok), do: {true, body, [ttl: exp]}
  defp time_legit(_), do: false
end

Upon calling CacheTest.get_time() the first call returns {:ok, %{body: body, exp: 10000}} while the subsequent calls returns merely the body as expected till 10s is over and the cycle repeats itself

Maybe this behavior can be explicitly toggled on or off in case current behavior is preferred

[cabol]

Right, when returning the tuple {true, value_to_cache} or {true, value_to_cache, opts}, value_to_cache is what is stored in the cache. If you want to cache {:ok, %{body: body, exp: 10000}}, then you have to change the match function to return {true, ok, [ttl: exp]} (instead of {true, body, [ttl: exp]}), like in the example I put in my previous comment, like so:

@decorate cacheable(cache: Phos.Cache,
    key: {CacheTest, :get_cert},
    match: &time_legit/1)
# returns %{:ok, %{exp: time_to_expiry_of_cert}}
  def get_time() do
    host = "http://127.0.0.1:4040"
    with {:ok, %{body: body}} <- HTTPoison.get(host <> "/ping",[{"Content-Type","application/json"}]) do
      {:ok, %{body: body, exp: 10000}}
      end
  end


  defp time_legit({:ok, %{exp: exp, body: body}} = ok), do: {true, ok, [ttl: exp]}
  defp time_legit(_), do: false

I'm not sure if that is what you meant, please let me know, stay tuned, thanks !!!

[ks0m1c]

That is indeed the best way to use the currently exposed functionality for the subsequent calls to cache to follow the shape of the data returned by your own function, but in terms of ergonomics of the API if someone wishes to transform the data before storing in cache differently this does force one to create wrapper function to standardise the shape of the data.

As an example if one does not wish you have the exp key to be exposed to functions calling CacheTest.get_time() you will have a helper function dropping the :exp key. Since eval_match() currently acts like a side-effect I can understand why you exposed the functionality the way you did

      quote do
        result = unquote(block)

        unquote(__MODULE__).run_cmd(
          unquote(__MODULE__),
          :eval_match,
          [result, match, cache, unquote(key), opts],
          on_error,
          result
        )

        result

Might be an abuse of match to expose match.(result)'s value here to override result

[cabol]

What you have described is totally correct. However, if you have a proposal or idea about how we can improve it, please let me know, it would be great to discuss other alternatives and/or ideas. And BTW, thanks a lot for the TTL proposal, I think this has been a nice feature 👍

[ks0m1c]

Hey Cabol, I have a simple proof of concept that might be to your liking that I implemented for the @Cacheable decorator that returns the desired cached value for the initial call as well.

I broke it into two commits
One that implements the case do that can return the value that is to be stored to cache:
ks0m1c@4a55812#diff-f67eaf83762cfb8e75c9d985b827e807a1aea41e5edb3ab461e6e69036ed4d60L1038-L1047

and another that adds a :return_cached flag that keeps to the current default behavior that you can flip if you feel the alternative is saner
ks0m1c@6230793#diff-f67eaf83762cfb8e75c9d985b827e807a1aea41e5edb3ab461e6e69036ed4d60

A sample program to test the behavior

Application.put_env(:test, Test.Cache, gc_interval: 86_400_000)

defmodule Test.Cache do
  use Nebulex.Cache,
    otp_app: :test,
    adapter: Nebulex.Adapters.Local
end

defmodule Test.Grain do
  use Nebulex.Caching

  @decorate cacheable(
              cache: Test.Cache,
              key: {Grain, :get},
              match: &grain_legit/1
            )
  # returns weight of grain everytime in integer
  def get() do
    with {:ok, %{grain: weight}} <- {:ok, %{grain: 10000}} do
      {:ok, %{grain: weight}}
    end
  end

  defp grain_legit({:ok, %{grain: weight}}), do: {true, weight, [ttl: floor(weight/10), return_cached: true]}
  defp grain_legit(_), do: false
end

If you feel this is up to standard I can extend the behaviour for the other decorators that utilize eval_match() as well

[cabol]

Hey @KosmonautX 👋 !!

Apologies for the lateness here, I've been quite busy with other stuff, including Nebulex v3, but let's get into it.

IMHO, I don't see in what scenarios the : return_cached option may be useful, but let me explain why. It may break the function contract. In your example, the function returns a tuple {:ok, %{grain: weight}}, but now with the option return_cached: true it will return something different. This is ambiguous and if it is not handled properly it may cause issues. Besides, because of this option, the function will never return what is supposed to, which is the {:ok, %{grain: weight}} tuple, it will always return the cached value weight, hence, what is the point of returning the tuple in the first place? In general, the match function must be consistent with the decorated function contract. Based on the example, you could actually write it like this without the :return_cached and it will be the same:

defmodule Test.Grain do
  use Nebulex.Caching

  @decorate cacheable(
              cache: Test.Cache,
              key: {Grain, :get},
              match: &grain_legit/1
            )
  # returns weight of grain everytime in integer
  def get() do
    with {:ok, %{grain: weight}} <- {:ok, %{grain: 10000}} do
      weight
    end
  end

  # You can match the success like this, and otherwise return the false
  defp grain_legit(weight) when is_integer(weight), do: {true, weight, [ttl: floor(weight/10)]}
  defp grain_legit(_), do: false
  
  # You could also match the error case(s) first and ensure returning false, and otherwise the true
  # defp grain_legit({:error, _}), do: false
  # defp grain_legit(weight), do: {true, weight, [ttl: floor(weight/10)]}
end

In this case, the match is consistent with the function contract as it should be. I think there are several ways to write, but of course, I'd need maybe more details on what you are trying to achieve. There may be several use cases for the match option, but the main idea is to help the cacheable and cache_put decorators work together, or when you want to pass a custom cache options (like in your case with the :ttl). For example:

@decorate cacheable(key: id)
def get_book(id) do
  # This returns the Book schema from the DB or nil
  Repo.get(Book, id)
end

@decorate cache_put(key: book.id, match: &match_fun/1)
def update_book(%Book{} = book, attrs) do
  # On the other hand this returns an Ok/Error tuple, so we need the match
  # to cached only the value to be consistent with the cacheable decorator
  book
  |> Book.changeset(attrs)
  |> Repo.update()
end

defp match_fun({:ok, usr}), do: {true, usr}
defp match_fun({:error, _}), do: false

But overall, if you can write your code in such a way you don't need to worry about the match function would be the best, but of course, that is not always possible, that's the reason for the match option. But, taking the same example above, one could write it like this as well:

@decorate cacheable(key: id)
def fetch_book(id) do
  # Wrapper for returning the Ok/Error tuple
  if book = Repo.get(Book, id) do
    # This is the cached result which is consistent with the cache_put
    {:ok, book}
  else
    {:error, :not_found}
  end
end

@decorate cache_put(key: book.id)
def update_book(%Book{} = book, attrs) do
  # Returns an Ok/Error tuple
  book
  |> Book.changeset(attrs)
  |> Repo.update()
end

Anywho, please let me know your thoughts, if I'm missing something, etc. Stay tuned, thanks 🙇 !!!