Skip to content

Commit a3e3c0b

Browse files
committed
fix: resolve CI failures (format, credo, dialyzer)
- Run mix format on all files - Fix Credo alias ordering in index.ex - Remove unreachable V1 build_backup clause (Dialyzer pattern_match_cov) - Extract unwrap_command_result/evaluate_condition_then_cas helpers to reduce put_if/delete_if cyclomatic complexity below Credo threshold - Fix ra_system.fetch pattern match in application.ex (returns map, not tuple)
1 parent 1dddc0e commit a3e3c0b

File tree

9 files changed

+81
-87
lines changed

9 files changed

+81
-87
lines changed

lib/concord.ex

Lines changed: 42 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -205,40 +205,16 @@ defmodule Concord do
205205
result =
206206
cond do
207207
expected != nil ->
208-
# Direct CAS — no functions in the command
209-
case command({:put_if, key, compressed_value, expires_at, expected}, timeout) do
210-
{:ok, :ok, _} -> :ok
211-
{:ok, {:error, reason}, _} -> {:error, reason}
212-
{:timeout, _} -> {:error, :timeout}
213-
{:error, :noproc} -> {:error, :cluster_not_ready}
214-
{:error, reason} -> {:error, reason}
215-
end
208+
unwrap_command_result({:put_if, key, compressed_value, expires_at, expected}, timeout)
216209

217210
condition_fn != nil ->
218-
# Evaluate condition pre-consensus, then CAS
219-
case get(key, Keyword.take(opts, [:token, :timeout, :consistency])) do
220-
{:ok, current_value} ->
221-
if condition_fn.(current_value) do
222-
case command(
223-
{:put_if, key, compressed_value, expires_at, current_value},
224-
timeout
225-
) do
226-
{:ok, :ok, _} -> :ok
227-
{:ok, {:error, reason}, _} -> {:error, reason}
228-
{:timeout, _} -> {:error, :timeout}
229-
{:error, :noproc} -> {:error, :cluster_not_ready}
230-
{:error, reason} -> {:error, reason}
231-
end
232-
else
233-
{:error, :condition_failed}
234-
end
235-
236-
{:error, :not_found} ->
237-
{:error, :not_found}
238-
239-
{:error, reason} ->
240-
{:error, reason}
241-
end
211+
evaluate_condition_then_cas(
212+
key,
213+
fn current -> {:put_if, key, compressed_value, expires_at, current} end,
214+
condition_fn,
215+
opts,
216+
timeout
217+
)
242218

243219
true ->
244220
{:error, :missing_condition}
@@ -288,39 +264,19 @@ defmodule Concord do
288264

289265
start_time = System.monotonic_time()
290266

291-
# Evaluate condition functions pre-consensus (same pattern as put_if)
292267
result =
293268
cond do
294269
expected != nil ->
295-
case command({:delete_if, key, expected, nil}, timeout) do
296-
{:ok, :ok, _} -> :ok
297-
{:ok, {:error, reason}, _} -> {:error, reason}
298-
{:timeout, _} -> {:error, :timeout}
299-
{:error, :noproc} -> {:error, :cluster_not_ready}
300-
{:error, reason} -> {:error, reason}
301-
end
270+
unwrap_command_result({:delete_if, key, expected, nil}, timeout)
302271

303272
condition_fn != nil ->
304-
case get(key, Keyword.take(opts, [:token, :timeout, :consistency])) do
305-
{:ok, current_value} ->
306-
if condition_fn.(current_value) do
307-
case command({:delete_if, key, current_value, nil}, timeout) do
308-
{:ok, :ok, _} -> :ok
309-
{:ok, {:error, reason}, _} -> {:error, reason}
310-
{:timeout, _} -> {:error, :timeout}
311-
{:error, :noproc} -> {:error, :cluster_not_ready}
312-
{:error, reason} -> {:error, reason}
313-
end
314-
else
315-
{:error, :condition_failed}
316-
end
317-
318-
{:error, :not_found} ->
319-
{:error, :not_found}
320-
321-
{:error, reason} ->
322-
{:error, reason}
323-
end
273+
evaluate_condition_then_cas(
274+
key,
275+
fn current -> {:delete_if, key, current, nil} end,
276+
condition_fn,
277+
opts,
278+
timeout
279+
)
324280

325281
true ->
326282
{:error, :missing_condition}
@@ -806,6 +762,32 @@ defmodule Concord do
806762
:ra.process_command(server_id(), cmd, timeout)
807763
end
808764

765+
defp unwrap_command_result(cmd, timeout) do
766+
case command(cmd, timeout) do
767+
{:ok, :ok, _} -> :ok
768+
{:ok, {:error, reason}, _} -> {:error, reason}
769+
{:timeout, _} -> {:error, :timeout}
770+
{:error, :noproc} -> {:error, :cluster_not_ready}
771+
{:error, reason} -> {:error, reason}
772+
end
773+
end
774+
775+
# Evaluate a condition function pre-consensus, then issue a CAS command.
776+
# Keeps anonymous functions out of the Raft log.
777+
defp evaluate_condition_then_cas(key, build_cmd, condition_fn, opts, timeout) do
778+
case get(key, Keyword.take(opts, [:token, :timeout, :consistency])) do
779+
{:ok, current_value} ->
780+
if condition_fn.(current_value) do
781+
unwrap_command_result(build_cmd.(current_value), timeout)
782+
else
783+
{:error, :condition_failed}
784+
end
785+
786+
{:error, reason} ->
787+
{:error, reason}
788+
end
789+
end
790+
809791
defp query(query, timeout, consistency) do
810792
query_fun = fun(query)
811793

lib/concord/application.ex

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,10 @@ defmodule Concord.Application do
188188

189189
defp wait_for_ra_system(attempts) do
190190
case :ra_system.fetch(:default) do
191-
{:ok, _} ->
191+
%{} ->
192192
:ok
193193

194-
other when other in [:error, :undefined] ->
194+
_ ->
195195
Process.sleep(200)
196196
wait_for_ra_system(attempts - 1)
197197
end

lib/concord/backup.ex

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -332,21 +332,6 @@ defmodule Concord.Backup do
332332
{:ok, backup}
333333
end
334334

335-
# V1 backward compatibility: bare list of KV tuples
336-
defp build_backup(snapshot_data) when is_list(snapshot_data) do
337-
metadata = %{
338-
timestamp: DateTime.utc_now(),
339-
node: node(),
340-
cluster_name: Application.get_env(:concord, :cluster_name, :concord_cluster),
341-
entry_count: length(snapshot_data),
342-
memory_bytes: :erlang.external_size(snapshot_data),
343-
version: Application.spec(:concord, :vsn) |> to_string(),
344-
checksum: compute_checksum(snapshot_data)
345-
}
346-
347-
{:ok, %{metadata: metadata, data: snapshot_data}}
348-
end
349-
350335
defp write_backup(backup_dir, backup_data, compress) do
351336
timestamp = DateTime.utc_now() |> DateTime.to_iso8601(:basic) |> String.replace(":", "")
352337
filename = "concord_backup_#{timestamp}#{@backup_extension}"

lib/concord/index.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ defmodule Concord.Index do
3232
:ok = Concord.Index.create("by_email", fn u -> u.email end)
3333
"""
3434

35-
alias Concord.StateMachine
3635
alias Concord.Index.Extractor
36+
alias Concord.StateMachine
3737

3838
@timeout 5_000
3939
@cluster_name :concord_cluster

lib/concord/index/extractor.ex

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ defmodule Concord.Index.Extractor do
5252
def extract({:map_get, key}, value) when is_map(value), do: Map.get(value, key)
5353
def extract({:nested, keys}, value) when is_map(value), do: get_in(value, keys)
5454
def extract({:identity}, value), do: value
55-
def extract({:element, n}, value) when is_tuple(value) and tuple_size(value) > n, do: elem(value, n)
55+
56+
def extract({:element, n}, value) when is_tuple(value) and tuple_size(value) > n,
57+
do: elem(value, n)
58+
5659
# Backward compatibility: evaluate anonymous functions during migration
5760
def extract(extractor, value) when is_function(extractor, 1) do
5861
extractor.(value)

lib/concord/rbac.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,9 @@ defmodule Concord.RBAC do
285285

286286
defp fallback_create_role(role, permissions) do
287287
case get_role(role) do
288-
{:ok, _} -> {:error, :role_exists}
288+
{:ok, _} ->
289+
{:error, :role_exists}
290+
289291
{:error, :not_found} ->
290292
:ets.insert(:concord_roles, {role, permissions})
291293
:ok

test/concord/backup_test.exs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ defmodule Concord.BackupTest do
1414

1515
backup_state = %{
1616
version: 2,
17-
kv_data: [{"key1", %{value: "val1", expires_at: nil}}, {"key2", %{value: "val2", expires_at: nil}}],
17+
kv_data: [
18+
{"key1", %{value: "val1", expires_at: nil}},
19+
{"key2", %{value: "val2", expires_at: nil}}
20+
],
1821
tokens: %{"tok1" => %{permissions: [:read, :write]}},
1922
roles: %{admin: %{permissions: [:read, :write, :admin]}},
2023
role_grants: %{"tok1" => [:admin]},
@@ -39,7 +42,10 @@ defmodule Concord.BackupTest do
3942

4043
# Verify RBAC roles in state
4144
assert data.roles == %{admin: %{permissions: [:read, :write, :admin]}}
42-
assert :ets.lookup(:concord_roles, :admin) == [{:admin, %{permissions: [:read, :write, :admin]}}]
45+
46+
assert :ets.lookup(:concord_roles, :admin) == [
47+
{:admin, %{permissions: [:read, :write, :admin]}}
48+
]
4349

4450
# Verify role grants
4551
assert data.role_grants == %{"tok1" => [:admin]}
@@ -51,7 +57,10 @@ defmodule Concord.BackupTest do
5157

5258
# Verify tenants
5359
assert data.tenants == %{"tenant1" => %{id: "tenant1", namespace: "t1:*"}}
54-
assert :ets.lookup(:concord_tenants, "tenant1") == [{"tenant1", %{id: "tenant1", namespace: "t1:*"}}]
60+
61+
assert :ets.lookup(:concord_tenants, "tenant1") == [
62+
{"tenant1", %{id: "tenant1", namespace: "t1:*"}}
63+
]
5564
end
5665

5766
test "V2 restore_backup with indexes rebuilds index ETS", %{state: state} do
@@ -75,7 +84,8 @@ defmodule Concord.BackupTest do
7584
backup_state = %{
7685
version: 2,
7786
kv_data: [
78-
{"user:1", %{value: :erlang.term_to_binary(%{email: "alice@test.com"}), expires_at: nil}},
87+
{"user:1",
88+
%{value: :erlang.term_to_binary(%{email: "alice@test.com"}), expires_at: nil}},
7989
{"user:2", %{value: :erlang.term_to_binary(%{email: "bob@test.com"}), expires_at: nil}}
8090
],
8191
tokens: %{},
@@ -135,7 +145,9 @@ defmodule Concord.BackupTest do
135145
# Old data should be gone
136146
assert :ets.lookup(:concord_store, "old_key") == []
137147
# New data should be present
138-
assert :ets.lookup(:concord_store, "new_key") == [{"new_key", %{value: "new_val", expires_at: nil}}]
148+
assert :ets.lookup(:concord_store, "new_key") == [
149+
{"new_key", %{value: "new_val", expires_at: nil}}
150+
]
139151
end
140152
end
141153
end

test/concord/determinism_test.exs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ defmodule Concord.DeterminismTest do
2121
defp clear_ets do
2222
:ets.delete_all_objects(:concord_store)
2323

24-
for table <- [:concord_tokens, :concord_roles, :concord_role_grants, :concord_acls, :concord_tenants] do
24+
for table <- [
25+
:concord_tokens,
26+
:concord_roles,
27+
:concord_role_grants,
28+
:concord_acls,
29+
:concord_tenants
30+
] do
2531
if :ets.whereis(table) != :undefined, do: :ets.delete_all_objects(table)
2632
end
2733

test/concord/snapshot_test.exs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,11 @@ defmodule Concord.SnapshotTest do
224224

225225
# 6. ACL
226226
{s7, _, _} =
227-
StateMachine.apply_command(%{index: 7}, {:rbac_create_acl, "snap:*", :snaprole, [:read]}, s6)
227+
StateMachine.apply_command(
228+
%{index: 7},
229+
{:rbac_create_acl, "snap:*", :snaprole, [:read]},
230+
s6
231+
)
228232

229233
# 7. Tenant
230234
{s8, _, _} =

0 commit comments

Comments
 (0)