[#177] Improve default match function in decorators to cover more sce…

…narios

lib/nebulex/caching.ex

@@ -241,7 +241,21 @@ if Code.ensure_loaded?(Decorator.Define) do
         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.
+        cache. The default match function looks like this:
+
+        ```elixir
+        fn
+          {:error, _} -> false
+          :error -> false
+          nil -> false
+          _ -> true
+        end
+        ```
+
+        By default, if the code-block evaluation returns any of the following
+        terms/values `nil`, `:error`, `{:error, term}`, the default match
+        function returns `false` (the returned result is not cached),
+        otherwise, `true` is returned (the returned result is cached).
 
       * `:key_generator` - The custom key-generator to be used (optional).
         If present, this option overrides the default key generator provided
@@ -683,17 +697,17 @@ if Code.ensure_loaded?(Decorator.Define) do
     ## Private Functions
 
     defp caching_action(action, attrs, block, context) do
-      _cache = attrs[:cache] || raise ArgumentError, "expected cache: to be given as argument"
+      cache = attrs[:cache] || raise ArgumentError, "expected cache: to be given as argument"
       opts_var = attrs[:opts] || []
       on_error_var = on_error_opt(attrs)
-      match_var = attrs[:match] || quote(do: fn _ -> true end)
+      match_var = attrs[:match] || default_match_fun()
 
       args =
         context.args
         |> Enum.reduce([], &walk/2)
         |> Enum.reverse()
 
-      cache_block = cache_block(attrs, args, context)
+      cache_block = cache_block(cache, args, context)
       keygen_block = keygen_block(attrs, args, context)
       action_block = action_block(action, block, attrs, keygen_block)
 
@@ -707,6 +721,17 @@ if Code.ensure_loaded?(Decorator.Define) do
       end
     end
 
+    defp default_match_fun do
+      quote do
+        fn
+          {:error, _} -> false
+          :error -> false
+          nil -> false
+          _ -> true
+        end
+      end
+    end
+
     defp walk({:\\, _, [ast, _]}, acc) do
       walk(ast, acc)
     end
@@ -726,10 +751,21 @@ if Code.ensure_loaded?(Decorator.Define) do
       acc
     end
 
-    defp cache_block(attrs, args, ctx) do
-      attrs
-      |> Keyword.get(:cache)
-      |> cache_call(ctx, args)
+    # MFA cache: `{module, function, args}`
+    defp cache_block({:{}, _, [mod, fun, cache_args]}, args, ctx) do
+      quote do
+        unquote(mod).unquote(fun)(
+          unquote(ctx.module),
+          unquote(ctx.name),
+          unquote(args),
+          unquote_splicing(cache_args)
+        )
+      end
+    end
+
+    # Module implementing the cache behaviour (default)
+    defp cache_block({_, _, _} = cache, _args, _ctx) do
+      quote(do: unquote(cache))
     end
 
     defp keygen_block(attrs, args, ctx) do
@@ -751,23 +787,6 @@ if Code.ensure_loaded?(Decorator.Define) do
       end
     end
 
-    # MFA cache: `{module, function, args}`
-    defp cache_call({:{}, _, [mod, fun, cache_args]}, ctx, args) do
-      quote do
-        unquote(mod).unquote(fun)(
-          unquote(ctx.module),
-          unquote(ctx.name),
-          unquote(args),
-          unquote_splicing(cache_args)
-        )
-      end
-    end
-
-    # Module implementing the cache behaviour (default)
-    defp cache_call({_, _, _} = cache, _ctx, _args) do
-      quote(do: unquote(cache))
-    end
-
     # MFA key-generator: `{module, function, args}`
     defp keygen_call({:{}, _, [mod, fun, keygen_args]}, _ctx, _args) do
       quote do
@@ -931,8 +950,8 @@ if Code.ensure_loaded?(Decorator.Define) do
           result = block.()
 
           referenced_key =
-            with link_fun when is_function(link_fun, 1) <- references do
-              link_fun.(result)
+            with ref_fun when is_function(ref_fun, 1) <- references do
+              ref_fun.(result)
             end
 
           with true <-

test/nebulex/caching_test.exs

@@ -100,18 +100,33 @@ defmodule Nebulex.CachingTest do
   describe "cacheable" do
     test "with default opts" do
       refute Cache.get("x")
-      assert get_by_x("x") == {"x", "y"}
-      assert Cache.get("x") == {"x", "y"}
-      refute get_by_x("nil")
-      refute Cache.get("nil")
+      assert get_by_x("x") == nil
+      refute Cache.get("x")
+
+      assert get_by_x(1, 11) == 11
+      assert Cache.get(1) == 11
+
+      assert get_by_x(2, {:ok, 22}) == {:ok, 22}
+      assert Cache.get(2) == {:ok, 22}
+
+      assert get_by_x(3, :error) == :error
+      refute Cache.get(3)
 
-      refute Cache.get({2, 2})
-      assert get_by_xy(2, 2) == {2, 4}
-      assert Cache.get({2, 2}) == {2, 4}
+      assert get_by_x(4, {:error, 4}) == {:error, 4}
+      refute Cache.get(4)
+
+      refute Cache.get({:xy, 2})
+      assert get_by_xy(:xy, 2) == {:xy, 4}
+      assert Cache.get({:xy, 2}) == {:xy, 4}
 
       :ok = Process.sleep(1100)
-      assert Cache.get("x") == {"x", "y"}
-      assert Cache.get({2, 2}) == {2, 4}
+
+      refute Cache.get("x")
+      assert Cache.get(1) == 11
+      assert Cache.get(2) == {:ok, 22}
+      refute Cache.get(3)
+      refute Cache.get(4)
+      assert Cache.get({:xy, 2}) == {:xy, 4}
     end
 
     test "with opts" do
@@ -264,16 +279,28 @@ defmodule Nebulex.CachingTest do
 
   describe "cache_put" do
     test "with default opts" do
+      assert update_fun(1) == nil
+      refute Cache.get(1)
+
+      assert update_fun(1, :error) == :error
+      refute Cache.get(1)
+
+      assert update_fun(1, {:error, :error}) == {:error, :error}
+      refute Cache.get(1)
+
       assert set_keys(x: 1, y: 2, z: 3) == :ok
-      assert update_fun(:x) == :x
-      assert update_fun(:y) == :y
-      assert Cache.get(:x) == :x
-      assert Cache.get(:y) == :y
+
+      assert update_fun(:x, 2) == 2
+      assert update_fun(:y, {:ok, 4}) == {:ok, 4}
+
+      assert Cache.get(:x) == 2
+      assert Cache.get(:y) == {:ok, 4}
       assert Cache.get(:z) == 3
 
       :ok = Process.sleep(1100)
-      assert Cache.get(:x) == :x
-      assert Cache.get(:y) == :y
+
+      assert Cache.get(:x) == 2
+      assert Cache.get(:y) == {:ok, 4}
       assert Cache.get(:z) == 3
     end
 
@@ -576,13 +603,18 @@ defmodule Nebulex.CachingTest do
   def get_without_args, do: "hello"
 
   @decorate cacheable(cache: Cache, key: x)
-  def get_by_x(x, y \\ "y") do
-    case x do
-      "nil" -> nil
-      _ -> {x, y}
+  def get_by_x(x, y \\ nil) do
+    with _ when not is_nil(x) <- x,
+         _ when not is_nil(y) <- y do
+      y
     end
   end
 
+  @decorate cacheable(cache: Cache, key: {x, y})
+  def get_by_xy(x, y) do
+    {x, y * 2}
+  end
+
   @decorate cacheable(cache: Cache, opts: [ttl: 1000])
   def get_with_opts(x) do
     x
@@ -606,11 +638,6 @@ defmodule Nebulex.CachingTest do
     _ -> :error
   end
 
-  @decorate cacheable(cache: Cache, key: {x, y})
-  def get_by_xy(x, y) do
-    {x, y * 2}
-  end
-
   @decorate cacheable(cache: Cache)
   def get_with_default_key(x, y) do
     _ = {x, y}
@@ -631,8 +658,11 @@ defmodule Nebulex.CachingTest do
   def update_without_args, do: "hello"
 
   @decorate cache_put(cache: Cache, key: x)
-  def update_fun(x) do
-    x
+  def update_fun(x, y \\ nil) do
+    with _ when not is_nil(x) <- x,
+         _ when not is_nil(y) <- y do
+      y
+    end
   end
 
   @decorate cache_put(cache: Cache, key: x, opts: [ttl: 1000])