From 10ea840f785d69cded6a02539232b54332b3a03c Mon Sep 17 00:00:00 2001 From: MikaAK Date: Wed, 2 Sep 2026 22:42:20 -0700 Subject: [PATCH 1/6] chore(credo): adopt mika_credo_rules and fix every finding Add mika_credo_rules (GitHub source, hex 0.1.0 only ships 14 of the 99 checks) and enable every check that applies to a plain Elixir library. Checks that target consumers of this library rather than the library itself (NoRawEts, CacheRequiresSandboxOption, CacheOptsNoHardcodedUri, NoDirectErlangRpc, NoReimplementedHelper) are listed as disabled. Code changes driven by the new checks: - RefreshAhead and HashRing spawn their background tasks from a Task.Supervisor in the strategy's supervision tree. - RingMonitor defers ETS seeding and node monitoring to handle_continue/2. - Adapter rescues narrow to ArgumentError, the only exception ETS, counters, persistent_term and ConCache raise; HashRing logs an RPC exception before falling back. - Table-owner processes use spawn_link instead of a bare {:ok, pid} match on Task.start_link/1; DETS exits with the open_file reason. - function_exported?/3 is guarded by Code.ensure_loaded?/1. - Application env reads go through Cache.Config; Logger messages carry the module prefix and inspect their values. Tests replace Process.sleep with Cache.Wait.until polling, :sys.get_state syncs and a gated refresh callback for the dedup test; the app-env fixtures move to test_helper.exs. Claude-Session: https://claude.ai/code/session_014FgvF2AKpSdWdLEEQQjhjZ --- .credo.exs | 69 +++++++++++- CHANGELOG.md | 3 + lib/cache.ex | 16 +-- lib/cache/case_template.ex | 4 +- lib/cache/con_cache.ex | 10 +- lib/cache/config.ex | 2 + lib/cache/counter.ex | 14 +-- lib/cache/dets.ex | 13 ++- lib/cache/ets.ex | 29 ++--- lib/cache/hash_ring.ex | 14 ++- lib/cache/hash_ring/ring_monitor.ex | 15 ++- lib/cache/multi_layer.ex | 2 +- lib/cache/persistent_term.ex | 6 +- lib/cache/refresh_ahead.ex | 13 ++- lib/cache/sandbox.ex | 22 ++-- lib/cache/strategy.ex | 6 +- lib/cache/term_encoder.ex | 2 +- mix.exs | 5 +- mix.lock | 1 + test/cache/case_template_test.exs | 8 +- test/cache/con_cache_test.exs | 3 - test/cache/counter_test.exs | 1 - test/cache/dets_test.exs | 2 - test/cache/ets_test.exs | 9 -- test/cache/hash_ring_monitor_test.exs | 6 +- test/cache/hash_ring_test.exs | 15 ++- test/cache/multi_layer_broadcast_test.exs | 2 +- test/cache/refresh_ahead_test.exs | 122 +++++++++++++--------- test/cache_strategy_test.exs | 24 ----- test/cache_test.exs | 24 +---- test/support/wait.ex | 23 ++++ test/test_helper.exs | 4 + 32 files changed, 285 insertions(+), 204 deletions(-) create mode 100644 test/support/wait.ex diff --git a/.credo.exs b/.credo.exs index 7b7510a..bf25d9a 100644 --- a/.credo.exs +++ b/.credo.exs @@ -25,13 +25,15 @@ allowed_imports = [ "apps/*/lib/", "apps/*/src/", "apps/*/test/", - "apps/*/web/" + "apps/*/web/", + "mix.exs", + ".credo.exs" ], excluded: [~r"_build/", ~r"deps/"] }, plugins: [], strict: true, - parse_timeout: 10000, + parse_timeout: 10_000, color: true, checks: [ @@ -43,11 +45,68 @@ allowed_imports = [ {BlitzCredoChecks.NoAsyncFalse, []}, {BlitzCredoChecks.NoDSLParentheses, []}, {BlitzCredoChecks.NoIsBitstring, []}, - {BlitzCredoChecks.StrictComparison, []}, + {BlitzCredoChecks.StrictComparison, false}, {BlitzCredoChecks.UseStream, []}, {BlitzCredoChecks.LowercaseTestNames, []}, {BlitzCredoChecks.ImproperImport, allowed_modules: allowed_imports}, + # MikaCredoRules (supersedes BlitzCredoChecks.StrictComparison, TagTODO, TagFIXME) + {MikaCredoRules.AsyncTrueRequired, []}, + {MikaCredoRules.CredoConfigNamedDefault, []}, + {MikaCredoRules.DistributionRequiresBuckets, []}, + {MikaCredoRules.EnsureLoadedBeforeExported, []}, + # lib/cache/ets.ex: opts_definition/1 is a NimbleOptions :custom validator, whose contract + # is {:error, String.t()} + {MikaCredoRules.ErrorMessageRequired, [excluded_paths: ["_test.exs", "test/", "lib/cache/ets.ex"]]}, + {MikaCredoRules.ExceptionNamesEndInError, []}, + {MikaCredoRules.GenServerRequiresHandleContinue, []}, + {MikaCredoRules.LoggerModulePrefixAndInspect, []}, + {MikaCredoRules.LowercaseErrorMessages, []}, + {MikaCredoRules.NoAccessOnStructSubject, []}, + {MikaCredoRules.NoApplicationEnvOutsideConfig, []}, + {MikaCredoRules.NoAtomStringKeyFallback, []}, + {MikaCredoRules.NoBarePatternMatchOnFallible, []}, + {MikaCredoRules.NoBinaryPatternForStringPrefix, []}, + {MikaCredoRules.NoBlanketRescue, []}, + {MikaCredoRules.NoBooleanLiteralComparison, []}, + {MikaCredoRules.NoCondElseAtom, []}, + {MikaCredoRules.NoDoPrefixedHelper, []}, + {MikaCredoRules.NoForWithDiscardedResult, []}, + {MikaCredoRules.NoHardcodedSecretLiterals, []}, + {MikaCredoRules.NoIOANSIRawEscapes, []}, + {MikaCredoRules.NoIdentityRewrap, []}, + {MikaCredoRules.NoIntermediateSingleUseVariable, []}, + {MikaCredoRules.NoKernelPrefix, []}, + {MikaCredoRules.NoLengthZeroComparison, []}, + {MikaCredoRules.NoMixEnvAtRuntime, []}, + {MikaCredoRules.NoMockingLibraries, []}, + {MikaCredoRules.NoNilComparison, []}, + {MikaCredoRules.NoPipeIntoControlFlow, []}, + {MikaCredoRules.NoProcessSleepInTests, []}, + {MikaCredoRules.NoSelfSendZeroDelay, []}, + {MikaCredoRules.NoSingleLetterVariables, []}, + {MikaCredoRules.NoTaskAsyncInGenServer, []}, + {MikaCredoRules.NoTruthyAndOr, []}, + {MikaCredoRules.NoUnboundBlockAssignment, []}, + {MikaCredoRules.NoUnsupervisedTaskStart, []}, + {MikaCredoRules.NoVacuousAssert, []}, + {MikaCredoRules.NoWordSigilLists, []}, + {MikaCredoRules.RefuteOverAssertNot, []}, + {MikaCredoRules.SingleModulePerFile, + [excluded_paths: ["test/", "test/support/", "_test.exs", "cache_test_modules.ex"]]}, + {MikaCredoRules.StrictEquality, []}, + {MikaCredoRules.TaskAsyncStreamRequiresTimeout, []}, + {MikaCredoRules.TestOnlyDepsScoped, + [test_only_packages: [:credo, :blitz_credo_checks, :mika_credo_rules, :dialyxir, :excoveralls, :ex_doc]]}, + {MikaCredoRules.TodosNeedTickets, []}, + + # MikaCredoRules that target consumers of this library, not the library itself + {MikaCredoRules.CacheOptsNoHardcodedUri, false}, + {MikaCredoRules.CacheRequiresSandboxOption, false}, + {MikaCredoRules.NoDirectErlangRpc, false}, + {MikaCredoRules.NoRawEts, false}, + {MikaCredoRules.NoReimplementedHelper, false}, + # Consistency Checks {Credo.Check.Consistency.ExceptionNames, []}, {Credo.Check.Consistency.LineEndings, []}, @@ -60,8 +119,8 @@ allowed_imports = [ {Credo.Check.Design.AliasUsage, false}, # No outstanding TODOs - {Credo.Check.Design.TagTODO, []}, - {Credo.Check.Design.TagFIXME, []}, + {Credo.Check.Design.TagTODO, false}, + {Credo.Check.Design.TagFIXME, false}, # # Readability Checks {Credo.Check.Readability.AliasOrder, false}, diff --git a/CHANGELOG.md b/CHANGELOG.md index c24f65d..b63a8df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ ## Bug Fixes +- fix(refresh_ahead, hash_ring): the background refresh task and the read-repair delete task now run under a `Task.Supervisor` inside the strategy's own supervision tree instead of an unsupervised `Task.start/1`. +- fix(hash_ring): an exception raised by the RPC module during read-repair is logged at warning level before the node is treated as unavailable, instead of being swallowed. +- fix(hash_ring): `Cache.HashRing.RingMonitor` seeds its history table and subscribes to node events from `handle_continue/2` rather than blocking in `init/1`. - fix: `:compression_level` is reachable. It was unusable on every path — no adapter declares it, so `NimbleOptions` rejected it on compile-time adapter opts, and it resolved to `nil` before it could reach the encoder otherwise. It is now an option on the `use Cache` line (`compression_level: 6`), it is taken off the adapter opts before they are validated so the `opts: [compression_level: 6]` spelling works too, and it is never handed to the adapter. Setting it forces encoding on adapters that hold terms natively — asking for compression is asking for bytes. A cache using a strategy adapter raises at compile time rather than ignoring the option. - fix: `Cache.ConCache.get_or_store/3` followed by `get/1` no longer raises. `get_or_store/3` writes through ConCache directly, bypassing the encode in `put/3`, so the matching `get/1` tried to `binary_to_term/1` a raw term. - fix: a binary value wrapped in braces but not valid JSON (eg `"{oops}"`) no longer raises `Jason.DecodeError` on read. `Cache.TermEncoder.decode/1` used `Jason.decode!/1`, and now falls back to returning the binary unchanged. diff --git a/lib/cache.ex b/lib/cache.ex index 4026a59..0ca7a2f 100644 --- a/lib/cache.ex +++ b/lib/cache.ex @@ -153,11 +153,11 @@ defmodule Cache do defp adapter_options!({module, fun, args}), do: apply(module, fun, args) match?({_, _}, @adapter_opts) -> - defp adapter_options!({app, key}), do: Application.fetch_env!(app, key) + defp adapter_options!({app, key}), do: Cache.Config.fetch_env!(app, key) is_atom(@adapter_opts) and not is_nil(@adapter_opts) -> defp adapter_options!(app_name) when is_atom(app_name), - do: Application.fetch_env!(app_name, __MODULE__) + do: Cache.Config.fetch_env!(app_name, __MODULE__) is_function(@adapter_opts, 0) -> defp adapter_options!(fun) when is_function(fun, 0), do: fun.() @@ -211,7 +211,9 @@ defmodule Cache do @cache_name |> @cache_strategy_module.get(key, @cache_strategy_config, adapter_options()) |> handle_adapter_result(:get, @cache_name) - |> case do + + result = + case result do {:ok, nil} = res -> :telemetry.execute([:elixir_cache, :cache, :get, :miss], %{count: 1}, %{ cache_name: @cache_name @@ -357,11 +359,11 @@ defmodule Cache do defp adapter_options!({module, fun, args}), do: apply(module, fun, args) match?({_, _}, @adapter_opts) -> - defp adapter_options!({app, key}), do: Application.fetch_env!(app, key) + defp adapter_options!({app, key}), do: Cache.Config.fetch_env!(app, key) is_atom(@adapter_opts) and not is_nil(@adapter_opts) -> defp adapter_options!(app_name) when is_atom(app_name), - do: Application.fetch_env!(app_name, __MODULE__) + do: Cache.Config.fetch_env!(app_name, __MODULE__) is_function(@adapter_opts, 0) -> defp adapter_options!(fun) when is_function(fun, 0), do: fun.() @@ -426,7 +428,9 @@ defmodule Cache do @cache_name |> @cache_adapter.get(key, adapter_options()) |> handle_adapter_result(:get, @cache_name) - |> case do + + result = + case result do {:ok, nil} = res -> :telemetry.execute([:elixir_cache, :cache, :get, :miss], %{count: 1}, %{ cache_name: @cache_name diff --git a/lib/cache/case_template.ex b/lib/cache/case_template.ex index 1347058..ec5e12a 100644 --- a/lib/cache/case_template.ex +++ b/lib/cache/case_template.ex @@ -118,10 +118,10 @@ defmodule Cache.CaseTemplate do |> Supervisor.which_children() |> Enum.filter(fn {_id, _pid, _type, modules} -> is_list(modules) and - Enum.any?(modules, &function_exported?(&1, :cache_name, 0)) + Enum.any?(modules, &(Code.ensure_loaded?(&1) and function_exported?(&1, :cache_name, 0))) end) |> Enum.flat_map(fn {_id, _pid, _type, modules} -> - Enum.filter(modules, &function_exported?(&1, :cache_name, 0)) + Enum.filter(modules, &(Code.ensure_loaded?(&1) and function_exported?(&1, :cache_name, 0))) end) end end diff --git a/lib/cache/con_cache.ex b/lib/cache/con_cache.ex index f498ffd..c2c3370 100644 --- a/lib/cache/con_cache.ex +++ b/lib/cache/con_cache.ex @@ -98,26 +98,26 @@ defmodule Cache.ConCache do def put(cache_name, key, _ttl \\ nil, value, _opts \\ []) def put(cache_name, key, nil, value, opts) do - if is_nil(opts[:dirty?]) or opts[:dirty?] do + if Keyword.get(opts, :dirty?, true) do ConCache.dirty_put(cache_name, key, value) else ConCache.put(cache_name, key, value) end rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end def put(cache_name, key, ttl, value, opts) do item = %ConCache.Item{value: value, ttl: ttl} - if is_nil(opts[:dirty?]) or opts[:dirty?] do + if Keyword.get(opts, :dirty?, true) do ConCache.dirty_put(cache_name, key, item) else ConCache.put(cache_name, key, item) end rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -132,7 +132,7 @@ defmodule Cache.ConCache do def delete(cache_name, key, _opts \\ []) do ConCache.delete(cache_name, key) rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end diff --git a/lib/cache/config.ex b/lib/cache/config.ex index c73c30a..199496d 100644 --- a/lib/cache/config.ex +++ b/lib/cache/config.ex @@ -6,4 +6,6 @@ defmodule Cache.Config do def sandbox_sleep_ms do Application.get_env(@app, :sandbox_sleep_ms, 50) end + + def fetch_env!(app, key), do: Application.fetch_env!(app, key) end diff --git a/lib/cache/counter.ex b/lib/cache/counter.ex index 215644a..0b3e643 100644 --- a/lib/cache/counter.ex +++ b/lib/cache/counter.ex @@ -107,8 +107,8 @@ defmodule Cache.Counter do parent = self() ready_ref = make_ref() - {:ok, pid} = - Task.start_link(fn -> + pid = + spawn_link(fn -> cache_name = opts[:table_name] initial_size = opts[:initial_size] || 1 counters_opts = if opts[:write_concurrency], do: [:atomics, :write_concurrency], else: [:atomics] @@ -148,7 +148,7 @@ defmodule Cache.Counter do {:error, ErrorMessage.bad_request("integer key #{key} is out of bounds (initial_size: #{size})", %{cache: cache_name, key: key})} end rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -166,7 +166,7 @@ defmodule Cache.Counter do :counters.add(ref, compute_index(ref, key), value) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -184,7 +184,7 @@ defmodule Cache.Counter do :counters.put(ref, compute_index(ref, key), 0) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -194,7 +194,7 @@ defmodule Cache.Counter do :counters.add(ref, compute_index(ref, key), step) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -204,7 +204,7 @@ defmodule Cache.Counter do :counters.add(ref, compute_index(ref, key), -step) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end diff --git a/lib/cache/dets.ex b/lib/cache/dets.ex index 2c14284..e7a72f3 100644 --- a/lib/cache/dets.ex +++ b/lib/cache/dets.ex @@ -305,8 +305,8 @@ defmodule Cache.DETS do @doc """ Returns the list of objects associated with slot I. """ - def slot(i) do - :dets.slot(@cache_name, i) + def slot(index) do + :dets.slot(@cache_name, index) end @doc """ @@ -385,8 +385,8 @@ defmodule Cache.DETS do parent = self() ref = make_ref() - {:ok, pid} = - Task.start_link(fn -> + pid = + spawn_link(fn -> table_name = opts[:table_name] file_path = @@ -401,7 +401,10 @@ defmodule Cache.DETS do |> Keyword.drop([:table_name, :file_path]) |> Kernel.++(access: :read_write, file: file_path) - {:ok, _} = :dets.open_file(table_name, opts) + case :dets.open_file(table_name, opts) do + {:ok, _table} -> :ok + {:error, reason} -> exit({:dets_open_file, reason}) + end send(parent, {ref, :ready}) Process.hibernate(Function, :identity, [nil]) diff --git a/lib/cache/ets.ex b/lib/cache/ets.ex index 0782b70..d8b3afb 100644 --- a/lib/cache/ets.ex +++ b/lib/cache/ets.ex @@ -448,8 +448,8 @@ defmodule Cache.ETS do @doc """ Returns the list of objects associated with slot I. """ - def slot(i) do - @cache_adapter.slot(@cache_name, i) + def slot(index) do + @cache_adapter.slot(@cache_name, index) end @doc """ @@ -621,7 +621,7 @@ defmodule Cache.ETS do end defp maybe_compressed(original_opts, validated) do - if Keyword.has_key?(original_opts, :compressed) and validated[:compressed], do: :compressed + if Keyword.has_key?(original_opts, :compressed) && validated[:compressed], do: :compressed end defp maybe_kv(original_opts, validated, key) do @@ -633,8 +633,8 @@ defmodule Cache.ETS do parent = self() ref = make_ref() - {:ok, pid} = - Task.start_link(fn -> + pid = + spawn_link(fn -> table_name = opts[:table_name] rehydration_path = opts[:rehydration_path] @@ -673,15 +673,15 @@ defmodule Cache.ETS do if File.exists?(file_path) do case :ets.file2tab(String.to_charlist(file_path)) do {:ok, ^table_name} -> - Logger.info("[Cache.ETS] Rehydrated #{table_name} from #{file_path}") + Logger.info("#{__MODULE__}: rehydrated table #{inspect(table_name)} from #{inspect(file_path)}") :ok {:ok, _other_name} -> - Logger.warning("[Cache.ETS] File #{file_path} has different table name, creating new table #{table_name}") + Logger.warning("#{__MODULE__}: file #{inspect(file_path)} has a different table name, creating new table #{inspect(table_name)}") :ets.new(table_name, ets_opts) {:error, reason} -> - Logger.warning("[Cache.ETS] Failed to rehydrate #{table_name} from #{file_path}: #{inspect(reason)}, creating new table") + Logger.warning("#{__MODULE__}: failed to rehydrate #{inspect(table_name)} from #{inspect(file_path)}, creating new table, reason: #{inspect(reason)}") :ets.new(table_name, ets_opts) end else @@ -703,7 +703,7 @@ defmodule Cache.ETS do end) do {:ok, _ref} -> :ok {:error, reason} -> - Logger.error("[Cache.ETS] Failed to setup exit signal handler for #{table_name}: #{inspect(reason)}") + Logger.error("#{__MODULE__}: failed to setup exit signal handler for #{inspect(table_name)}, reason: #{inspect(reason)}") :ok end @@ -732,7 +732,8 @@ defmodule Cache.ETS do [] -> {:ok, nil} end rescue - e -> {:error, ErrorMessage.internal_server_error(Exception.message(e), %{cache: cache_name, key: key})} + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -742,7 +743,8 @@ defmodule Cache.ETS do :ok rescue - e -> {:error, ErrorMessage.internal_server_error(Exception.message(e), %{cache: cache_name, key: key})} + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -752,7 +754,8 @@ defmodule Cache.ETS do :ok rescue - e -> {:error, ErrorMessage.internal_server_error(Exception.message(e), %{cache: cache_name, key: key})} + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end # ETS-specific module-level functions called via @cache_adapter from the __using__ macro. @@ -841,7 +844,7 @@ defmodule Cache.ETS do def setopts(cache_name, opts), do: :ets.setopts(cache_name, opts) - def slot(cache_name, i), do: :ets.slot(cache_name, i) + def slot(cache_name, index), do: :ets.slot(cache_name, index) def tab2file(cache_name, filename), do: :ets.tab2file(cache_name, filename) def tab2file(cache_name, filename, options), do: :ets.tab2file(cache_name, filename, options) diff --git a/lib/cache/hash_ring.ex b/lib/cache/hash_ring.ex index 53b6443..fca17b1 100644 --- a/lib/cache/hash_ring.ex +++ b/lib/cache/hash_ring.ex @@ -78,6 +78,8 @@ defmodule Cache.HashRing do @behaviour Cache.Strategy + require Logger + @strategy_keys [:ring_opts, :node_weight, :rpc_module, :ring_history_size, :__cache_module__] @opts_definition [ @@ -143,7 +145,8 @@ defmodule Cache.HashRing do [ underlying_adapter.child_spec({cache_name, underlying_opts}), managed_ring_spec, - ring_monitor_spec + ring_monitor_spec, + {Task.Supervisor, name: task_supervisor_name(cache_name)} ], [strategy: :one_for_one] ]} @@ -250,7 +253,7 @@ defmodule Cache.HashRing do rpc.call(current_node, underlying_adapter, :put, [cache_name, key, nil, encoded, underlying_opts]) end - Task.start(fn -> + Task.Supervisor.start_child(task_supervisor_name(cache_name), fn -> rpc.call(old_node, underlying_adapter, :delete, [cache_name, key, underlying_opts]) end) end @@ -262,11 +265,16 @@ defmodule Cache.HashRing do {:badrpc, _} -> :unavailable end rescue - _ -> :unavailable + exception -> + Logger.warning("#{__MODULE__}: rpc get on #{inspect(node)} raised, error: #{inspect(exception)}") + + :unavailable catch :exit, _ -> :unavailable end + defp task_supervisor_name(cache_name), do: :"#{cache_name}_hash_ring_tasks" + defp key_to_node(cache_name, key) do ring = ring_name(cache_name) diff --git a/lib/cache/hash_ring/ring_monitor.ex b/lib/cache/hash_ring/ring_monitor.ex index 433250f..7cca764 100644 --- a/lib/cache/hash_ring/ring_monitor.ex +++ b/lib/cache/hash_ring/ring_monitor.ex @@ -35,9 +35,6 @@ defmodule Cache.HashRing.RingMonitor do node_whitelist = Keyword.get(opts, :node_whitelist, []) table = :ets.new(history_table_name(cache_name), [:set, :public, :named_table]) - :ets.insert(table, {:previous_rings, []}) - - :ok = :net_kernel.monitor_nodes(true, node_type: :all) {:ok, %{ @@ -46,7 +43,15 @@ defmodule Cache.HashRing.RingMonitor do history_size: history_size, node_blacklist: node_blacklist, node_whitelist: node_whitelist - }} + }, {:continue, :seed_and_monitor}} + end + + @impl GenServer + def handle_continue(:seed_and_monitor, %{table: table} = state) do + :ets.insert(table, {:previous_rings, []}) + :ok = :net_kernel.monitor_nodes(true, node_type: :all) + + {:noreply, state} end @impl GenServer @@ -76,7 +81,7 @@ defmodule Cache.HashRing.RingMonitor do defp safe_ets_lookup(table, key) do :ets.lookup(table, key) rescue - _ -> [] + ArgumentError -> [] end @spec history_table_name(atom()) :: atom() diff --git a/lib/cache/multi_layer.ex b/lib/cache/multi_layer.ex index b15b184..55c48b4 100644 --- a/lib/cache/multi_layer.ex +++ b/lib/cache/multi_layer.ex @@ -303,7 +303,7 @@ defmodule Cache.MultiLayer do end defp cache_module?(module) do - function_exported?(module, :get, 1) and function_exported?(module, :put, 2) + Code.ensure_loaded?(module) and function_exported?(module, :get, 1) and function_exported?(module, :put, 2) end defp invoke_callback({module, function, args}, extra_args) do diff --git a/lib/cache/persistent_term.ex b/lib/cache/persistent_term.ex index 66b60a5..f8a3d59 100644 --- a/lib/cache/persistent_term.ex +++ b/lib/cache/persistent_term.ex @@ -56,7 +56,7 @@ defmodule Cache.PersistentTerm do def get(cache_name, key, _opts \\ []) do {:ok, :persistent_term.get({cache_name, key}, nil)} rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -66,7 +66,7 @@ defmodule Cache.PersistentTerm do :persistent_term.put({cache_name, key}, value) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @@ -76,7 +76,7 @@ defmodule Cache.PersistentTerm do :persistent_term.erase({cache_name, key}) :ok rescue - exception -> + exception in ArgumentError -> {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end end diff --git a/lib/cache/refresh_ahead.ex b/lib/cache/refresh_ahead.ex index 3cb0880..80aa6fb 100644 --- a/lib/cache/refresh_ahead.ex +++ b/lib/cache/refresh_ahead.ex @@ -101,7 +101,8 @@ defmodule Cache.RefreshAhead do %{ id: :"#{cache_name}_refresh_tracker", start: {__MODULE__, :start_tracker, [tracker_name]} - } + }, + {Task.Supervisor, name: task_supervisor_name(cache_name)} ], [strategy: :one_for_one] ]} @@ -197,7 +198,7 @@ defmodule Cache.RefreshAhead do tracker = tracker_name(cache_name) if safe_ets_insert_new(tracker, {key, true}) do - Task.start(fn -> + Task.Supervisor.start_child(task_supervisor_name(cache_name), fn -> lock_resource = {:refresh_ahead_lock, cache_name, key} lock_id = {lock_resource, self()} lock_nodes = lock_nodes(adapter_opts[:lock_node_whitelist]) @@ -261,19 +262,19 @@ defmodule Cache.RefreshAhead do defp safe_ets_insert_new(tracker, record) do :ets.insert_new(tracker, record) rescue - _ -> false + ArgumentError -> false end defp safe_ets_delete(tracker, key) do :ets.delete(tracker, key) rescue - _ -> :ok + ArgumentError -> :ok end defp safe_global_set_lock(lock_id, lock_nodes) do :global.set_lock(lock_id, lock_nodes, 0) rescue - _ -> false + ArgumentError -> false end defp lock_nodes(lock_node_whitelist) do @@ -301,4 +302,6 @@ defmodule Cache.RefreshAhead do end defp tracker_name(cache_name), do: :"#{cache_name}_refresh_tracker" + + defp task_supervisor_name(cache_name), do: :"#{cache_name}_refresh_tasks" end diff --git a/lib/cache/sandbox.ex b/lib/cache/sandbox.ex index c72404e..dcbdb80 100644 --- a/lib/cache/sandbox.ex +++ b/lib/cache/sandbox.ex @@ -461,7 +461,7 @@ defmodule Cache.Sandbox do Agent.get(Cache.Sandbox, fn state -> state |> scoped_get(sid) |> Map.keys() end) rescue - _ -> [] + ArgumentError -> [] end def delete_table(cache_name) do @@ -838,6 +838,7 @@ defmodule Cache.Sandbox do end) end + # ETS counts a match only when the spec body returns exactly `true`, not any truthy term. def select_count(cache_name, match_spec) do compiled = :ets.match_spec_compile(match_spec) @@ -845,7 +846,7 @@ defmodule Cache.Sandbox do sub |> Map.to_list() |> :ets.match_spec_run(compiled) - |> Enum.count(&(&1 === true)) + |> Enum.count(&match?(true, &1)) end) end @@ -857,7 +858,7 @@ defmodule Cache.Sandbox do sub |> Map.to_list() |> Enum.split_with(fn obj -> - [obj] |> :ets.match_spec_run(compiled) |> Enum.any?(&(&1 === true)) + [obj] |> :ets.match_spec_run(compiled) |> Enum.any?(&match?(true, &1)) end) {length(to_delete), Map.new(to_keep)} @@ -896,7 +897,7 @@ defmodule Cache.Sandbox do if updates !== [] do scoped_agent_update(cache_name, fn sub -> - Enum.reduce(updates, sub, fn {k, v}, acc -> Map.put(acc, k, v) end) + Enum.reduce(updates, sub, fn {key, value}, acc -> Map.put(acc, key, value) end) end) end @@ -932,14 +933,14 @@ defmodule Cache.Sandbox do Keyword.get(info, item) end - def slot(cache_name, i) do + def slot(cache_name, index) do scoped_agent_get(cache_name, fn sub -> list = Map.to_list(sub) - if i >= length(list) do + if index >= length(list) do :"$end_of_table" else - [Enum.at(list, i)] + [Enum.at(list, index)] end end) end @@ -1162,8 +1163,7 @@ defmodule Cache.Sandbox do end def select_reverse(cache_name, match_spec) do - result = select(cache_name, match_spec) - Enum.reverse(result) + cache_name |> select(match_spec) |> Enum.reverse() end def select_reverse(cache_name, match_spec, limit) do @@ -1279,6 +1279,6 @@ defmodule Cache.Sandbox do end) end - defp enum_length(m) when is_map(m), do: m |> Map.to_list() |> enum_length() - defp enum_length(l), do: length(l) + defp enum_length(map) when is_map(map), do: map |> Map.to_list() |> enum_length() + defp enum_length(list), do: length(list) end diff --git a/lib/cache/strategy.ex b/lib/cache/strategy.ex index bb40c02..53f3d4c 100644 --- a/lib/cache/strategy.ex +++ b/lib/cache/strategy.ex @@ -82,16 +82,16 @@ defmodule Cache.Strategy do module |> module_behaviours() |> Enum.member?(Cache.Strategy) - rescue - _ -> false end + # module_info/1 raises UndefinedFunctionError for an unknown module and ArgumentError for + # a non-atom. defp module_behaviours(module) do module |> :erlang.apply(:module_info, [:attributes]) |> Keyword.get_values(:behaviour) |> List.flatten() rescue - _ -> [] + _ in [UndefinedFunctionError, ArgumentError] -> [] end end diff --git a/lib/cache/term_encoder.ex b/lib/cache/term_encoder.ex index 8c083e5..3995bb6 100644 --- a/lib/cache/term_encoder.ex +++ b/lib/cache/term_encoder.ex @@ -81,7 +81,7 @@ defmodule Cache.TermEncoder do def native_term_storage?({_strategy_module, _strategy_config}, _adapter_opts), do: true def native_term_storage?(adapter, adapter_opts) when is_atom(adapter) do - if function_exported?(adapter, :native_term_storage?, 1) do + if Code.ensure_loaded?(adapter) and function_exported?(adapter, :native_term_storage?, 1) do adapter.native_term_storage?(adapter_opts) else compile_time_native_term_storage?(adapter, adapter_opts) diff --git a/mix.exs b/mix.exs index 92b9282..792ee25 100644 --- a/mix.exs +++ b/mix.exs @@ -59,8 +59,9 @@ defmodule ElixirCache.MixProject do {:libring, "~> 1.7"}, {:credo, "~> 1.6", only: [:test, :dev], runtime: false}, {:blitz_credo_checks, "~> 0.1", only: [:test, :dev], runtime: false}, + {:mika_credo_rules, github: "MikaAK/mika_credo_rules", only: [:test, :dev], runtime: false}, {:excoveralls, "~> 0.10", only: :test}, - {:ex_doc, ">= 0.0.0", optional: true, only: :dev}, + {:ex_doc, ">= 0.0.0", optional: true, only: :dev, runtime: false}, {:dialyxir, "~> 1.0", optional: true, only: :test, runtime: false} ] end @@ -70,7 +71,7 @@ defmodule ElixirCache.MixProject do maintainers: ["Mika Kalathil"], licenses: ["MIT"], links: %{"GitHub" => "https://github.com/mikaak/elixir_cache"}, - files: ~w(mix.exs README.md CHANGELOG.md LICENSE lib) + files: ["mix.exs", "README.md", "CHANGELOG.md", "LICENSE", "lib"] ] end diff --git a/mix.lock b/mix.lock index aaccc2b..341b986 100644 --- a/mix.lock +++ b/mix.lock @@ -22,6 +22,7 @@ "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"}, "makeup_erlang": {:hex, :makeup_erlang, "1.1.0", "835f7e60792e08824cda445639555d7bf1bbbddb1b60b306e33cb6f6db24dc74", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "1cd6780fb1dd1a03979abaed0fe82712b0625118fd5257d3ebbf73f960c73c3c"}, "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"}, + "mika_credo_rules": {:git, "https://github.com/MikaAK/mika_credo_rules.git", "617026ef3198c36a9129c8826ee45163200a0c0b", []}, "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"}, "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"}, "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"}, diff --git a/test/cache/case_template_test.exs b/test/cache/case_template_test.exs index 628f2b1..453f775 100644 --- a/test/cache/case_template_test.exs +++ b/test/cache/case_template_test.exs @@ -17,7 +17,7 @@ defmodule Cache.CaseTemplateTest do opts: [] end - defmodule TestCacheCase do + defmodule TestCacheCaseTemplate do use Cache.CaseTemplate, default_caches: [TestCache] end @@ -83,8 +83,8 @@ defmodule Cache.CaseTemplateTest do end defmodule Cache.CaseTemplateTest.IsolationTest do - use ExUnit.Case - use Cache.CaseTemplateTest.TestCacheCase + use ExUnit.Case, async: true + use Cache.CaseTemplateTest.TestCacheCaseTemplate alias Cache.CaseTemplateTest.TestCache @@ -100,7 +100,7 @@ defmodule Cache.CaseTemplateTest.IsolationTest do end defmodule Cache.CaseTemplateTest.ExtraCachesTest do - use ExUnit.Case + use ExUnit.Case, async: true use Cache.CaseTemplateTest.TestCaseCaseWithExtra, caches: [Cache.CaseTemplateTest.TestCache2] alias Cache.CaseTemplateTest.TestCache diff --git a/test/cache/con_cache_test.exs b/test/cache/con_cache_test.exs index d06804a..392f0f8 100644 --- a/test/cache/con_cache_test.exs +++ b/test/cache/con_cache_test.exs @@ -64,12 +64,9 @@ defmodule Cache.ConCacheTest do test "uses fetch function exactly once", %{key: key} do assert "VALUE" === ConCacheAdapter.get_or_store(key, @ttl, fn -> - Process.sleep(1_000) "VALUE" end) - Process.sleep(200) - assert "VALUE" === ConCacheAdapter.get_or_store(key, @ttl, fn -> raise "not used" end) end end diff --git a/test/cache/counter_test.exs b/test/cache/counter_test.exs index 5d95fb8..886de0b 100644 --- a/test/cache/counter_test.exs +++ b/test/cache/counter_test.exs @@ -10,7 +10,6 @@ defmodule Cache.CounterTest do setup do start_supervised({Cache, [TestCounterCache]}) - Process.sleep(50) :ok end diff --git a/test/cache/dets_test.exs b/test/cache/dets_test.exs index 657ff1b..d8087cd 100644 --- a/test/cache/dets_test.exs +++ b/test/cache/dets_test.exs @@ -10,8 +10,6 @@ defmodule Cache.DETSTest do setup do start_supervised({Cache, [TestDETSCache]}) - Process.sleep(100) - on_exit(fn -> File.rm("/tmp/test_dets_cache.dets") end) diff --git a/test/cache/ets_test.exs b/test/cache/ets_test.exs index 9ce68a2..cb2aaf4 100644 --- a/test/cache/ets_test.exs +++ b/test/cache/ets_test.exs @@ -12,8 +12,6 @@ defmodule Cache.ETSTest do setup do start_supervised({Cache, [TestETSCache]}) - Process.sleep(100) - :ok end @@ -163,8 +161,6 @@ defmodule Cache.ETSTest do } ) - Process.sleep(100) - assert {:ok, "persisted_value"} === RehydrateTestCache.get(:persisted_key) end @@ -180,8 +176,6 @@ defmodule Cache.ETSTest do } ) - Process.sleep(100) - assert {:ok, nil} === NewTableTestCache.get(:nonexistent_key) assert :ok === NewTableTestCache.put(:new_key, "new_value") assert {:ok, "new_value"} === NewTableTestCache.get(:new_key) @@ -228,7 +222,6 @@ defmodule Cache.ETSTest do start: {Cache, :start_link, [[DeleteTableCache], [name: :delete_table_sup]]} }) - Process.sleep(100) DeleteTableCache.insert_raw({:key, "val"}) assert DeleteTableCache.delete_table() === true end @@ -357,7 +350,6 @@ defmodule Cache.ETSTest do start: {Cache, :start_link, [[RenameTestCache], [name: :rename_test_sup]]} }) - Process.sleep(100) RenameTestCache.insert_raw({:rkey, "rval"}) RenameTestCache.rename(:renamed_cache) assert :ets.lookup(:renamed_cache, :rkey) === [{:rkey, "rval"}] @@ -493,7 +485,6 @@ defmodule Cache.ETSTest do start: {Cache, :start_link, [[File2TabTestCache], [name: :file2tab_sup]]} }) - Process.sleep(100) File2TabTestCache.insert_raw({:fb_key, "val"}) file = ~c"/tmp/test_ets_file2tab_#{:rand.uniform(100_000)}" diff --git a/test/cache/hash_ring_monitor_test.exs b/test/cache/hash_ring_monitor_test.exs index 6b746da..62f70db 100644 --- a/test/cache/hash_ring_monitor_test.exs +++ b/test/cache/hash_ring_monitor_test.exs @@ -68,7 +68,7 @@ defmodule Cache.HashRing.RingMonitorTest do }) send(pid, :some_random_message) - Process.sleep(50) + _synced = :sys.get_state(pid) assert Process.alive?(pid) end @@ -83,11 +83,11 @@ defmodule Cache.HashRing.RingMonitorTest do }) send(pid, {:nodeup, :"fake@node", %{}}) - Process.sleep(50) + _synced = :sys.get_state(pid) assert Process.alive?(pid) send(pid, {:nodedown, :"fake@node", %{}}) - Process.sleep(50) + _synced = :sys.get_state(pid) assert Process.alive?(pid) end end diff --git a/test/cache/hash_ring_test.exs b/test/cache/hash_ring_test.exs index b687d2a..84469d5 100644 --- a/test/cache/hash_ring_test.exs +++ b/test/cache/hash_ring_test.exs @@ -15,8 +15,6 @@ defmodule Cache.HashRingTest do start: {Cache, :start_link, [[TestHashRingCache], [name: :hash_ring_cache_sup]]} }) - Process.sleep(50) - :ok end @@ -150,8 +148,6 @@ defmodule Cache.HashRingTest do Cache.HashRing.get(cache_name, "repair:dedup_key", Cache.ETS, rpc_module: rpc_module) - Process.sleep(50) - assert Agent.get(call_count, & &1) === 1 end @@ -200,10 +196,11 @@ defmodule Cache.HashRingTest do Cache.HashRing.get(cache_name, "repair:migrate_key2", Cache.ETS, rpc_module: rpc_module) - Process.sleep(100) - - calls = Agent.get(calls_agent, & &1) - assert Enum.any?(calls, fn {_node, func} -> func === :delete end) + assert Cache.Wait.until(fn -> + calls_agent + |> Agent.get(& &1) + |> Enum.any?(fn {_node, func} -> func === :delete end) + end) end end @@ -283,7 +280,7 @@ defmodule Cache.HashRingTest do [self() | callers ++ ancestors] |> Enum.filter(&is_pid/1) |> Enum.find(fn pid -> - :persistent_term.get({Cache.HashRingTest.StubRpc, pid}, nil) !== nil + not is_nil(:persistent_term.get({Cache.HashRingTest.StubRpc, pid}, nil)) end) end end diff --git a/test/cache/multi_layer_broadcast_test.exs b/test/cache/multi_layer_broadcast_test.exs index 6b1116a..ecdca77 100644 --- a/test/cache/multi_layer_broadcast_test.exs +++ b/test/cache/multi_layer_broadcast_test.exs @@ -1,5 +1,5 @@ defmodule Cache.MultiLayerBroadcastTest do - use ExUnit.Case + use ExUnit.Case, async: true alias Cache.MultiLayer.Coordinator diff --git a/test/cache/refresh_ahead_test.exs b/test/cache/refresh_ahead_test.exs index 123b23d..4afceda 100644 --- a/test/cache/refresh_ahead_test.exs +++ b/test/cache/refresh_ahead_test.exs @@ -34,6 +34,9 @@ defmodule Cache.RefreshAheadTest do def refresh(key), do: {:ok, "locked:#{key}"} end + # Every fixture cache uses refresh_before: 500, so this TTL is inside the window at age 0. + @in_window_ttl 500 + setup do start_supervised!(%{ id: :refresh_cache_sup, @@ -86,58 +89,80 @@ defmodule Cache.RefreshAheadTest do assert :ok === TestRefreshCache.put("no_refresh_key", 10_000, "original") assert {:ok, "original"} === TestRefreshCache.get("no_refresh_key") - - Process.sleep(50) - assert {:ok, "original"} === TestRefreshCache.get("no_refresh_key") end + # A TTL no larger than refresh_before puts the key inside the refresh window from the + # first read: that read returns the stale value and spawns the async refresh. test "triggers async refresh when within refresh_before window" do - assert :ok === TestRefreshCache.put("refresh_key", 2000, "original") + assert :ok === TestRefreshCache.put("refresh_key", @in_window_ttl, "original") assert {:ok, "original"} === TestRefreshCache.get("refresh_key") - Process.sleep(1600) - - assert {:ok, "original"} === TestRefreshCache.get("refresh_key") - - Process.sleep(200) - - assert {:ok, "refreshed:refresh_key"} === TestRefreshCache.get("refresh_key") + assert Cache.Wait.until(fn -> + TestRefreshCache.get("refresh_key") === {:ok, "refreshed:refresh_key"} + end) end test "on_refresh option overrides module callback" do - assert :ok === CallbackRefreshCache.put("cb_key", 2000, "original") + assert :ok === CallbackRefreshCache.put("cb_key", @in_window_ttl, "original") assert {:ok, "original"} === CallbackRefreshCache.get("cb_key") - Process.sleep(1600) + assert Cache.Wait.until(fn -> + CallbackRefreshCache.get("cb_key") === {:ok, "custom:cb_key"} + end) + end + end - assert {:ok, "original"} === CallbackRefreshCache.get("cb_key") + describe "deduplication" do + # The refresh callback counts its calls and then blocks until the test opens the gate, + # so every concurrent get runs while one refresh is provably still in flight. + defmodule DedupRefreshCache do + use Cache, + adapter: {Cache.RefreshAhead, Cache.ETS}, + name: :dedup_refresh_cache, + opts: [refresh_before: 500] + + def refresh(key) do + Agent.update(__MODULE__.Calls, &(&1 + 1)) + Cache.Wait.until(fn -> Agent.get(__MODULE__.Gate, & &1) end, 5_000) + {:ok, "refreshed:#{key}"} + end + end - Process.sleep(200) + setup do + start_supervised!(%{ + id: :dedup_refresh_cache_sup, + type: :supervisor, + start: {Cache, :start_link, [[DedupRefreshCache], [name: :dedup_refresh_cache_sup]]} + }) + + start_supervised!(%{id: :calls, start: {Agent, :start_link, [fn -> 0 end, [name: DedupRefreshCache.Calls]]}}) + start_supervised!(%{id: :gate, start: {Agent, :start_link, [fn -> false end, [name: DedupRefreshCache.Gate]]}}) - assert {:ok, "custom:cb_key"} === CallbackRefreshCache.get("cb_key") + :ok end - end - describe "deduplication" do test "multiple concurrent gets only spawn one refresh task" do - assert :ok === TestRefreshCache.put("dedup_key", 2000, "original") + assert :ok === DedupRefreshCache.put("dedup_key", @in_window_ttl, "original") - Process.sleep(1600) + results = + 1..5 + |> Enum.map(fn _ -> Task.async(fn -> DedupRefreshCache.get("dedup_key") end) end) + |> Task.await_many() - tasks = - Enum.map(1..5, fn _ -> - Task.async(fn -> TestRefreshCache.get("dedup_key") end) - end) + assert Enum.all?(results, &(&1 === {:ok, "original"})) - results = Task.await_many(tasks) - assert Enum.all?(results, fn {:ok, val} -> val === "original" end) + # The one refresh task is parked at the gate, so the count cannot climb past 1. + assert Cache.Wait.until(fn -> Agent.get(DedupRefreshCache.Calls, & &1) >= 1 end) + assert Agent.get(DedupRefreshCache.Calls, & &1) === 1 - Process.sleep(200) + Agent.update(DedupRefreshCache.Gate, fn _ -> true end) - assert {:ok, "refreshed:dedup_key"} === TestRefreshCache.get("dedup_key") + assert Cache.Wait.until(fn -> + DedupRefreshCache.get("dedup_key") === {:ok, "refreshed:dedup_key"} + end) end test "global lock prevents refresh while lock is held" do @@ -146,13 +171,10 @@ defmodule Cache.RefreshAheadTest do lock_nodes = [Node.self()] assert true === :global.set_lock(lock_id, lock_nodes, 0) - assert :ok === LockedRefreshCache.put("locked_key", 2000, "original") - - Process.sleep(1600) + assert :ok === LockedRefreshCache.put("locked_key", @in_window_ttl, "original") assert {:ok, "original"} === LockedRefreshCache.get("locked_key") - - Process.sleep(250) + assert Cache.Wait.until(fn -> refresh_finished?(:locked_refresh_cache, "locked_key") end) assert {:ok, "original"} === LockedRefreshCache.get("locked_key") @@ -160,15 +182,15 @@ defmodule Cache.RefreshAheadTest do # clean itself up before releasing — otherwise it can reach :global.set_lock after # the del_lock below, take the freed lock, and refresh the value we assert is still # untouched. - Process.sleep(250) + assert Cache.Wait.until(fn -> refresh_finished?(:locked_refresh_cache, "locked_key") end) assert true === :global.del_lock(lock_id, lock_nodes) assert {:ok, "original"} === LockedRefreshCache.get("locked_key") - Process.sleep(250) - - assert {:ok, "locked:locked_key"} === LockedRefreshCache.get("locked_key") + assert Cache.Wait.until(fn -> + LockedRefreshCache.get("locked_key") === {:ok, "locked:locked_key"} + end) end end @@ -198,16 +220,10 @@ defmodule Cache.RefreshAheadTest do end test "uses MFA tuple for refresh callback" do - assert :ok === MFARefreshCache.put("mfa_key", 2000, "original") + assert :ok === MFARefreshCache.put("mfa_key", @in_window_ttl, "original") assert {:ok, "original"} === MFARefreshCache.get("mfa_key") - Process.sleep(1600) - - assert {:ok, "original"} === MFARefreshCache.get("mfa_key") - - Process.sleep(200) - - assert {:ok, "mfa:mfa_key"} === MFARefreshCache.get("mfa_key") + assert Cache.Wait.until(fn -> MFARefreshCache.get("mfa_key") === {:ok, "mfa:mfa_key"} end) end end @@ -242,18 +258,22 @@ defmodule Cache.RefreshAheadTest do end test "works with atom whitelist" do - assert :ok === AtomWhitelistCache.put("awl_key", 2000, "original") - - Process.sleep(1600) + assert :ok === AtomWhitelistCache.put("awl_key", @in_window_ttl, "original") assert {:ok, "original"} === AtomWhitelistCache.get("awl_key") - Process.sleep(200) - - assert {:ok, "atom_wl:awl_key"} === AtomWhitelistCache.get("awl_key") + assert Cache.Wait.until(fn -> + AtomWhitelistCache.get("awl_key") === {:ok, "atom_wl:awl_key"} + end) end end + # The refresh task clears its tracker entry in an `after` block, so an absent entry means + # the spawned task has finished — whether it refreshed or lost the lock race. + defp refresh_finished?(cache_name, key) do + not :ets.member(:"#{cache_name}_refresh_tracker", key) + end + describe "cache_adapter/0" do test "returns Cache.RefreshAhead as adapter" do assert TestRefreshCache.cache_adapter() === Cache.RefreshAhead diff --git a/test/cache_strategy_test.exs b/test/cache_strategy_test.exs index 23258ed..fe1c292 100644 --- a/test/cache_strategy_test.exs +++ b/test/cache_strategy_test.exs @@ -74,8 +74,6 @@ defmodule CacheStrategyTest do ]} }) - Process.sleep(50) - :ok end @@ -87,8 +85,6 @@ defmodule CacheStrategyTest do assert {:ok, nil} = cache_module.get(test_key) assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) end @@ -99,12 +95,8 @@ defmodule CacheStrategyTest do assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert :ok = cache_module.delete(test_key) - Process.sleep(50) - assert {:ok, nil} = cache_module.get(test_key) end @@ -116,13 +108,9 @@ defmodule CacheStrategyTest do assert {:ok, nil} = cache_module.get(test_key) assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) assert :ok = cache_module.put(test_key, nil) - Process.sleep(50) - assert {:ok, nil} = cache_module.get(test_key) end end @@ -140,8 +128,6 @@ defmodule CacheStrategyTest do ]} }) - Process.sleep(50) - :ok end @@ -152,8 +138,6 @@ defmodule CacheStrategyTest do assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get_or_create(test_key, fn -> raise "I shouldn't be called" @@ -174,8 +158,6 @@ defmodule CacheStrategyTest do {:ok, value} end) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) end end @@ -236,8 +218,6 @@ defmodule CacheStrategyTest do ]} }) - Process.sleep(50) - :ok end @@ -284,8 +264,6 @@ defmodule CacheStrategyTest do ]} }) - Process.sleep(50) - :ok end @@ -319,8 +297,6 @@ defmodule CacheStrategyTest do [[TestCache.MultiLayerFetch], [name: :multi_layer_fetch_sup]]} }) - Process.sleep(50) - :ok end diff --git a/test/cache_test.exs b/test/cache_test.exs index 43e0342..96ebcb9 100644 --- a/test/cache_test.exs +++ b/test/cache_test.exs @@ -34,7 +34,8 @@ defmodule CacheTest do setup do start_supervised({Cache, [unquote(adapter)]}) - Process.sleep(100) + # Cache.Redis connects asynchronously, so wait for a read to succeed before testing. + assert Cache.Wait.until(fn -> match?({:ok, _}, unquote(adapter).get(:readiness_probe)) end) :ok end @@ -47,8 +48,6 @@ defmodule CacheTest do assert {:ok, nil} = cache_module.get(test_key) assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) end @@ -59,12 +58,8 @@ defmodule CacheTest do assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert :ok = cache_module.delete(test_key) - Process.sleep(50) - assert {:ok, nil} = cache_module.get(test_key) end @@ -76,13 +71,9 @@ defmodule CacheTest do assert {:ok, nil} = cache_module.get(test_key) assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) assert :ok = cache_module.put(test_key, nil) - Process.sleep(50) - assert {:ok, nil} = cache_module.get(test_key) end end @@ -91,7 +82,8 @@ defmodule CacheTest do setup do start_supervised({Cache, [unquote(adapter)]}) - Process.sleep(100) + # Cache.Redis connects asynchronously, so wait for a read to succeed before testing. + assert Cache.Wait.until(fn -> match?({:ok, _}, unquote(adapter).get(:readiness_probe)) end) :ok end @@ -103,8 +95,6 @@ defmodule CacheTest do assert :ok = cache_module.put(test_key, value) - Process.sleep(50) - assert {:ok, value} === cache_module.get_or_create(test_key, fn -> raise "I shouldn't be called" @@ -125,8 +115,6 @@ defmodule CacheTest do {:ok, value} end) - Process.sleep(50) - assert {:ok, value} === cache_module.get(test_key) end end @@ -162,8 +150,6 @@ defmodule CacheTest do test "returns application env config with name" do options = [host: "localhost", port: 6379] - Application.put_env(:elixir_cache, TestCache.RedisRuntimeAppEnv, options) - defmodule TestCache.RedisRuntimeAppEnv do use Cache, adapter: Cache.Redis, @@ -177,8 +163,6 @@ defmodule CacheTest do test "returns application env config with name and key" do options = [host: "localhost", port: 6379] - Application.put_env(:elixir_cache, :cache, options) - defmodule TestCache.RedisRuntimeAppEnvKey do use Cache, adapter: Cache.Redis, diff --git a/test/support/wait.ex b/test/support/wait.ex new file mode 100644 index 0000000..8b1ee3c --- /dev/null +++ b/test/support/wait.ex @@ -0,0 +1,23 @@ +defmodule Cache.Wait do + @moduledoc """ + Polls a condition so tests synchronize on observable state instead of sleeping. + """ + + @poll_interval_ms 10 + + @doc "Returns `true` as soon as `fun` returns a truthy value, or `false` once `timeout` ms elapse." + @spec until((-> as_boolean(term())), non_neg_integer()) :: boolean() + def until(fun, timeout \\ 1_000) do + poll(fun, System.monotonic_time(:millisecond) + timeout) + end + + defp poll(fun, deadline) do + cond do + fun.() -> true + System.monotonic_time(:millisecond) >= deadline -> false + true -> + Process.sleep(@poll_interval_ms) + poll(fun, deadline) + end + end +end diff --git a/test/test_helper.exs b/test/test_helper.exs index 42d0522..575a2c1 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,3 +1,7 @@ +# App-env fixtures for the `opts: :app` / `opts: {:app, :key}` cases in test/cache_test.exs +Application.put_env(:elixir_cache, CacheTest.TestCache.RedisRuntimeAppEnv, host: "localhost", port: 6379) +Application.put_env(:elixir_cache, :cache, host: "localhost", port: 6379) + Cache.SandboxRegistry.start_link() ExUnit.start() From 9abda6cd364940ee77cbf947e18aef6df255e6af Mon Sep 17 00:00:00 2001 From: MikaAK Date: Wed, 2 Sep 2026 23:09:08 -0700 Subject: [PATCH 2/6] test: cover the adapter error paths the credo pass touched Codecov's patch check counted the retyped rescue clauses as new lines and reported 51% patch coverage. Exercise the reachable ones: ETS and Counter calls against a table that does not exist, ETS rehydration from a file with the wrong table name or garbage contents, DETS open_file failure, and RefreshAhead get/delete without a tracker table. Cache.PersistentTerm drops its rescue clauses instead: :persistent_term get/2, put/2 and erase/1 never raise, so they were dead code. Claude-Session: https://claude.ai/code/session_014FgvF2AKpSdWdLEEQQjhjZ --- lib/cache/persistent_term.ex | 11 ++------- test/cache/counter_test.exs | 10 ++++++++ test/cache/dets_test.exs | 12 +++++++++ test/cache/ets_test.exs | 41 +++++++++++++++++++++++++++++++ test/cache/refresh_ahead_test.exs | 16 ++++++++++++ 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/lib/cache/persistent_term.ex b/lib/cache/persistent_term.ex index f8a3d59..7ab607c 100644 --- a/lib/cache/persistent_term.ex +++ b/lib/cache/persistent_term.ex @@ -51,13 +51,12 @@ defmodule Cache.PersistentTerm do } end + # `:persistent_term.get/2`, `put/2` and `erase/1` never raise, so there is nothing to rescue. + @impl Cache @spec get(atom, atom | String.t(), Keyword.t()) :: ErrorMessage.t_res(any) def get(cache_name, key, _opts \\ []) do {:ok, :persistent_term.get({cache_name, key}, nil)} - rescue - exception in ArgumentError -> - {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -65,9 +64,6 @@ defmodule Cache.PersistentTerm do def put(cache_name, key, _ttl \\ nil, value, _opts \\ []) do :persistent_term.put({cache_name, key}, value) :ok - rescue - exception in ArgumentError -> - {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -75,8 +71,5 @@ defmodule Cache.PersistentTerm do def delete(cache_name, key, _opts \\ []) do :persistent_term.erase({cache_name, key}) :ok - rescue - exception in ArgumentError -> - {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end end diff --git a/test/cache/counter_test.exs b/test/cache/counter_test.exs index 886de0b..c938541 100644 --- a/test/cache/counter_test.exs +++ b/test/cache/counter_test.exs @@ -163,6 +163,16 @@ defmodule Cache.CounterTest do end end + describe "adapter calls against a counter that was never started" do + test "every operation returns an error tuple instead of raising" do + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.Counter.get(:no_such_counter, 0) + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.Counter.put(:no_such_counter, 0, 1) + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.Counter.delete(:no_such_counter, 0) + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.Counter.increment(:no_such_counter, 0) + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.Counter.decrement(:no_such_counter, 0) + end + end + describe "get with non-integer key" do test "returns error for atom key" do assert {:error, %ErrorMessage{code: :bad_request}} = TestCounterCache.get(:some_atom) diff --git a/test/cache/dets_test.exs b/test/cache/dets_test.exs index d8087cd..ee05e66 100644 --- a/test/cache/dets_test.exs +++ b/test/cache/dets_test.exs @@ -17,6 +17,18 @@ defmodule Cache.DETSTest do :ok end + describe "start_link/1 with a file that is not a dets table" do + test "returns the open_file reason instead of crashing the caller" do + Process.flag(:trap_exit, true) + file_path = "/tmp/corrupt_dets_#{System.unique_integer([:positive])}.dets" + File.write!(file_path, "not a dets file") + on_exit(fn -> File.rm(file_path) end) + + assert {:error, {:dets_open_file, {:not_a_dets_file, _path}}} = + Cache.DETS.start_link(table_name: :corrupt_dets_cache, file_path: file_path) + end + end + describe "match_object/2" do test "matches objects in the table" do TestDETSCache.insert_raw({:key1, "SomeRandomValue"}) diff --git a/test/cache/ets_test.exs b/test/cache/ets_test.exs index cb2aaf4..14b4b72 100644 --- a/test/cache/ets_test.exs +++ b/test/cache/ets_test.exs @@ -164,6 +164,39 @@ defmodule Cache.ETSTest do assert {:ok, "persisted_value"} === RehydrateTestCache.get(:persisted_key) end + test "creates a new table when the file holds a different table name" do + File.mkdir_p!("/tmp/ets_test") + on_exit(fn -> File.rm_rf("/tmp/ets_test") end) + + :ets.new(:some_other_table, [:set, :public, :named_table]) + :ets.insert(:some_other_table, {:persisted_key, "other"}) + :ets.tab2file(:some_other_table, ~c"/tmp/ets_test/rehydrate_test_cache.ets") + :ets.delete(:some_other_table) + + start_supervised!(%{ + id: :rehydrate_cache_sup, + type: :supervisor, + start: {Cache, :start_link, [[RehydrateTestCache], [name: :rehydrate_cache_sup]]} + }) + + assert {:ok, nil} === RehydrateTestCache.get(:persisted_key) + end + + test "creates a new table when the file is not an ets dump" do + File.mkdir_p!("/tmp/ets_test") + on_exit(fn -> File.rm_rf("/tmp/ets_test") end) + File.write!("/tmp/ets_test/rehydrate_test_cache.ets", "not an ets dump") + + start_supervised!(%{ + id: :rehydrate_cache_sup, + type: :supervisor, + start: {Cache, :start_link, [[RehydrateTestCache], [name: :rehydrate_cache_sup]]} + }) + + assert :ok === RehydrateTestCache.put(:fresh_key, "fresh") + assert {:ok, "fresh"} === RehydrateTestCache.get(:fresh_key) + end + test "creates new table when no file exists" do File.mkdir_p!("/tmp/ets_test") on_exit(fn -> File.rm_rf("/tmp/ets_test") end) @@ -182,6 +215,14 @@ defmodule Cache.ETSTest do end end + describe "adapter calls against a table that does not exist" do + test "get, put and delete return an error tuple instead of raising" do + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ETS.get(:no_such_ets_table, :key) + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ETS.put(:no_such_ets_table, :key, "value") + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ETS.delete(:no_such_ets_table, :key) + end + end + describe "all/0" do test "returns a list of ETS tables including the test table" do tables = TestETSCache.all() diff --git a/test/cache/refresh_ahead_test.exs b/test/cache/refresh_ahead_test.exs index 4afceda..60d1a1b 100644 --- a/test/cache/refresh_ahead_test.exs +++ b/test/cache/refresh_ahead_test.exs @@ -274,6 +274,22 @@ defmodule Cache.RefreshAheadTest do not :ets.member(:"#{cache_name}_refresh_tracker", key) end + describe "strategy calls on a cache whose tracker table does not exist" do + test "get/4 still returns the stored value and skips the refresh" do + :ets.new(:orphan_refresh_cache, [:set, :public, :named_table]) + wrapped = {"original", System.monotonic_time(:millisecond), @in_window_ttl} + :ets.insert(:orphan_refresh_cache, {"key", wrapped}) + + assert {:ok, "original"} === + Cache.RefreshAhead.get(:orphan_refresh_cache, "key", Cache.ETS, refresh_before: 500) + end + + test "delete/4 returns the underlying adapter's error instead of raising" do + assert {:error, %ErrorMessage{code: :internal_server_error}} = + Cache.RefreshAhead.delete(:no_such_refresh_cache, "key", Cache.ETS, []) + end + end + describe "cache_adapter/0" do test "returns Cache.RefreshAhead as adapter" do assert TestRefreshCache.cache_adapter() === Cache.RefreshAhead From b25fed6d844f1e29a646f54e62eec64060e6759d Mon Sep 17 00:00:00 2001 From: MikaAK Date: Wed, 2 Sep 2026 23:42:58 -0700 Subject: [PATCH 3/6] test: keep PersistentTerm rescues and cover the ConCache error path Dropping the PersistentTerm rescue clauses narrowed its return type, and Elixir 1.20 then flagged the generated {:error, _} clause of handle_adapter_result/3 as unreachable for that adapter, which fails the CI compile under --warnings-as-errors. The clauses stay so every adapter honours the same {:ok, _} | {:error, %ErrorMessage{}} contract. The ConCache rescue clauses are exercised by deleting the cache's public ETS table out from under it, which is the only way ConCache raises. --- lib/cache/persistent_term.ex | 11 +++++++++-- test/cache/con_cache_test.exs | 12 ++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/cache/persistent_term.ex b/lib/cache/persistent_term.ex index 7ab607c..f8a3d59 100644 --- a/lib/cache/persistent_term.ex +++ b/lib/cache/persistent_term.ex @@ -51,12 +51,13 @@ defmodule Cache.PersistentTerm do } end - # `:persistent_term.get/2`, `put/2` and `erase/1` never raise, so there is nothing to rescue. - @impl Cache @spec get(atom, atom | String.t(), Keyword.t()) :: ErrorMessage.t_res(any) def get(cache_name, key, _opts \\ []) do {:ok, :persistent_term.get({cache_name, key}, nil)} + rescue + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -64,6 +65,9 @@ defmodule Cache.PersistentTerm do def put(cache_name, key, _ttl \\ nil, value, _opts \\ []) do :persistent_term.put({cache_name, key}, value) :ok + rescue + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end @impl Cache @@ -71,5 +75,8 @@ defmodule Cache.PersistentTerm do def delete(cache_name, key, _opts \\ []) do :persistent_term.erase({cache_name, key}) :ok + rescue + exception in ArgumentError -> + {:error, ErrorMessage.internal_server_error(Exception.message(exception), %{cache: cache_name, key: key})} end end diff --git a/test/cache/con_cache_test.exs b/test/cache/con_cache_test.exs index 392f0f8..032ab51 100644 --- a/test/cache/con_cache_test.exs +++ b/test/cache/con_cache_test.exs @@ -55,6 +55,18 @@ defmodule Cache.ConCacheTest do %{key: key} end + describe "adapter calls after the ConCache table is gone" do + test "put and delete return an error tuple instead of raising" do + cache_name = :"orphan_con_cache_#{System.unique_integer([:positive])}" + start_supervised!({ConCache, name: cache_name, ttl_check_interval: false}) + :ets.delete(ConCache.ets(cache_name)) + + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ConCache.put(cache_name, :key, "value") + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ConCache.put(cache_name, :key, 1_000, "value") + assert {:error, %ErrorMessage{code: :internal_server_error}} = Cache.ConCache.delete(cache_name, :key) + end + end + describe "&get_or_store/3" do test "get/set", %{key: key} do assert "test_value" === ConCacheAdapter.get_or_store(key, @ttl, fn -> "test_value" end) From cc86d8a927e1e5e6bb45532545c60306b70a2ef7 Mon Sep 17 00:00:00 2001 From: MikaAK Date: Wed, 2 Sep 2026 23:50:58 -0700 Subject: [PATCH 4/6] test(case_template): run the shared-sandbox modules synchronously IsolationTest and ExtraCachesTest both sandbox TestCache, and Cache.SandboxRegistry.start/1 adopts an already-running sandbox Agent into the calling test's supervisor, so with async: true one test's teardown killed the other's cache mid-run. Declare them async: false and retire BlitzCredoChecks.NoAsyncFalse, which MikaCredoRules.AsyncTrueRequired supersedes: the concurrency choice must be explicit, and a deliberate async: false is allowed. --- .credo.exs | 4 +++- test/cache/case_template_test.exs | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.credo.exs b/.credo.exs index bf25d9a..01e8e13 100644 --- a/.credo.exs +++ b/.credo.exs @@ -42,7 +42,9 @@ allowed_imports = [ {BlitzCredoChecks.SetWarningsAsErrorsInTest, false}, {BlitzCredoChecks.DocsBeforeSpecs, []}, {BlitzCredoChecks.DoctestIndent, []}, - {BlitzCredoChecks.NoAsyncFalse, []}, + # Superseded by MikaCredoRules.AsyncTrueRequired: the choice must be explicit, and a + # deliberate async: false is allowed. + {BlitzCredoChecks.NoAsyncFalse, false}, {BlitzCredoChecks.NoDSLParentheses, []}, {BlitzCredoChecks.NoIsBitstring, []}, {BlitzCredoChecks.StrictComparison, false}, diff --git a/test/cache/case_template_test.exs b/test/cache/case_template_test.exs index 453f775..b05b7c7 100644 --- a/test/cache/case_template_test.exs +++ b/test/cache/case_template_test.exs @@ -82,8 +82,11 @@ defmodule Cache.CaseTemplateTest do end +# IsolationTest and ExtraCachesTest share the TestCache sandbox Agent, and +# Cache.SandboxRegistry.start/1 adopts an already-running Agent into the calling test's +# supervisor, so one test's teardown would kill the other's cache if they ran concurrently. defmodule Cache.CaseTemplateTest.IsolationTest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false use Cache.CaseTemplateTest.TestCacheCaseTemplate alias Cache.CaseTemplateTest.TestCache @@ -100,7 +103,7 @@ defmodule Cache.CaseTemplateTest.IsolationTest do end defmodule Cache.CaseTemplateTest.ExtraCachesTest do - use ExUnit.Case, async: true + use ExUnit.Case, async: false use Cache.CaseTemplateTest.TestCaseCaseWithExtra, caches: [Cache.CaseTemplateTest.TestCache2] alias Cache.CaseTemplateTest.TestCache From 499cacc2d4bd13246178857dff0931717c152146 Mon Sep 17 00:00:00 2001 From: MikaAK Date: Thu, 3 Sep 2026 00:47:26 -0700 Subject: [PATCH 5/6] fix(adapters): restore Task.start_link for table-owner processes Task.start_link/1 keeps the owner's $callers and $ancestors metadata and is the documented way to start a linked process here. The bare {:ok, pid} match is fine: Task.start_link/1 cannot return an error tuple. The NoBarePatternMatchOnFallible finding on these three sites will be handled in the credo rule itself. --- lib/cache/counter.ex | 4 ++-- lib/cache/dets.ex | 4 ++-- lib/cache/ets.ex | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/cache/counter.ex b/lib/cache/counter.ex index 0b3e643..57a1c81 100644 --- a/lib/cache/counter.ex +++ b/lib/cache/counter.ex @@ -107,8 +107,8 @@ defmodule Cache.Counter do parent = self() ready_ref = make_ref() - pid = - spawn_link(fn -> + {:ok, pid} = + Task.start_link(fn -> cache_name = opts[:table_name] initial_size = opts[:initial_size] || 1 counters_opts = if opts[:write_concurrency], do: [:atomics, :write_concurrency], else: [:atomics] diff --git a/lib/cache/dets.ex b/lib/cache/dets.ex index e7a72f3..1c57b89 100644 --- a/lib/cache/dets.ex +++ b/lib/cache/dets.ex @@ -385,8 +385,8 @@ defmodule Cache.DETS do parent = self() ref = make_ref() - pid = - spawn_link(fn -> + {:ok, pid} = + Task.start_link(fn -> table_name = opts[:table_name] file_path = diff --git a/lib/cache/ets.ex b/lib/cache/ets.ex index d8b3afb..a800e33 100644 --- a/lib/cache/ets.ex +++ b/lib/cache/ets.ex @@ -633,8 +633,8 @@ defmodule Cache.ETS do parent = self() ref = make_ref() - pid = - spawn_link(fn -> + {:ok, pid} = + Task.start_link(fn -> table_name = opts[:table_name] rehydration_path = opts[:rehydration_path] From 5246c9318980683aa614dbf6b995cfd7922ff01b Mon Sep 17 00:00:00 2001 From: MikaAK Date: Wed, 9 Sep 2026 22:15:55 -0700 Subject: [PATCH 6/6] chore(deps): bump mika_credo_rules to 1e4409f NoBarePatternMatchOnFallible now allows crash-preferable start functions, so the {:ok, pid} = Task.start_link(...) matches in the ETS, DETS and Counter adapters are no longer flagged. --- mix.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.lock b/mix.lock index 341b986..d3ee240 100644 --- a/mix.lock +++ b/mix.lock @@ -22,7 +22,7 @@ "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"}, "makeup_erlang": {:hex, :makeup_erlang, "1.1.0", "835f7e60792e08824cda445639555d7bf1bbbddb1b60b306e33cb6f6db24dc74", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "1cd6780fb1dd1a03979abaed0fe82712b0625118fd5257d3ebbf73f960c73c3c"}, "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"}, - "mika_credo_rules": {:git, "https://github.com/MikaAK/mika_credo_rules.git", "617026ef3198c36a9129c8826ee45163200a0c0b", []}, + "mika_credo_rules": {:git, "https://github.com/MikaAK/mika_credo_rules.git", "1e4409f63374ba61e6c5003d42b20012e0565af2", []}, "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"}, "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"}, "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},