hugo/bDS2
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: fix CSM-010

Browse Source
This commit is contained in:
Chili Palmer
2026-05-09 14:31:44 +02:00
parent 14dfbd8829
commit f1445120fc
5 changed files with 185 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
CODESMELL.md
View File

@@ -186,13 +186,15 @@
---
### CSM-010 — `rescue` for Control Flow in Data Layer
- **File:** `lib/bds/desktop/shell_data.ex:64-74, 81-103, 152-182`
- **What:** `rescue Exqlite.Error`, `rescue DBConnection.OwnershipError` to return empty defaults in `project_snapshot/0`, `dashboard/1`, `sidebar_view/3`, and `git_badge_count/2`.
- **Why it's bad:** Exceptions are for exceptional conditions. The rescue here handles the boot-phase case where the DB table doesn't exist yet — this is an expected state during startup.
- **Nuance:** The code does check `Exception.message(error)` for "no such table" and re-raises everything else, which limits the damage. But it's still control-flow-via-exception.
- **Fix:** Add an explicit `DB.ready?/0` check (or `Ecto.Adapters.SQL.query!/2` with a lightweight probe) before calling the data functions. Return `{:ok, result}` / `{:error, :not_ready}` tuples from the lowest-level callers and handle them upstream. Only `rescue` for truly unexpected DB errors.
- **Test:** Call `project_snapshot/0` with no DB connection; assert it returns `{:error, :not_ready}` instead of a default map.
### ~~CSM-010 — `rescue` for Control Flow in Data Layer~~ ✅ FIXED
- **Fixed:** 2026-05-09
- **What was done:**
- Added `BDS.Repo.ready?/0` — a lightweight probe that queries `sqlite_master` (parameterized) to check if core tables exist, without raising exceptions.
- Replaced all 4 `rescue` blocks in `ShellData` (`project_snapshot/0`, `dashboard/1`, `sidebar_view/3`, `git_badge_count/2`) with upfront `Repo.ready?()` checks.
- All four functions now return `{:ok, result}` / `{:error, :not_ready}` tuples instead of silently returning defaults via rescue.
- Updated callers in `ShellLive.refresh_content/2` and `ShellLive.refresh_sidebar/2` to pattern-match the new tuples and fall back to empty defaults only on `{:error, :not_ready}`.
- Made `default_project_snapshot/0` public for use by callers handling the not-ready case.
- Added 10 tests in `test/bds/csm010_rescue_control_flow_test.exs`: `Repo.ready?` returns true when DB is available, each of the 4 functions returns `{:ok, _}` when DB is ready and `{:error, :not_ready}` when the Repo is stopped.
---
@@ -430,11 +432,12 @@
- CSM-001: Fixed. All `String.to_atom` on dynamic data replaced with `MapUtils.safe_atomize_key/keys` or `String.to_existing_atom`.
- CSM-002: Fixed. Search now pushes all filtering and pagination into SQL via Ecto queries and CTEs.
- CSM-004: Fixed. `attach_runner` moved to `handle_continue`, `terminate/2` added for cleanup, `restart: :temporary` set, JobStore `detach_runner` bug fixed.
- [ ] All high-severity items (CSM-006 to CSM-010) have been addressed.
- [x] All high-severity items (CSM-006 to CSM-010) have been addressed.
- CSM-006: Fixed. Batch INSERT for reindexing, preloaded post records for rendering.
- CSM-007: Fixed. Decomposed into refresh_layout, refresh_sidebar, refresh_content, reload_shell.
- CSM-008: Fixed. Panel data pre-computed in event handlers, tab meta skips DB for complete entries.
- CSM-009: Fixed. All bang Image/File variants replaced with error-tuple handling, `ensure_thumbnails` returns `{:error, _}` instead of crashing.
- CSM-010: Fixed. Replaced rescue blocks with `Repo.ready?/0` probe and `{:ok, _}`/`{:error, :not_ready}` tuples.
- [x] CSM-001 fix covers ALL 6 affected files, not just `import_definitions.ex`.
- [x] CSM-003 fix covers ALL `Repo.delete!` call sites (posts, tags, scripts, media, projects, templates, translations).
- [x] CSM-007 decomposition is the prerequisite for fixing CSM-008 (render-path queries).

56
lib/bds/desktop/shell_data.ex
View File

@@ -6,6 +6,7 @@ defmodule BDS.Desktop.ShellData do
alias BDS.Git
alias BDS.I18n
alias BDS.Projects
alias BDS.Repo
alias BDS.UI.Dashboard
alias BDS.UI.Sidebar
alias BDS.UI.Workbench
@@ -62,15 +63,11 @@ defmodule BDS.Desktop.ShellData do
end
def project_snapshot do
Projects.shell_snapshot()
rescue
error in [Exqlite.Error, DBConnection.OwnershipError] ->
if match?(%Exqlite.Error{}, error) and
not String.contains?(Exception.message(error), "no such table: projects") do
reraise error, __STACKTRACE__
if Repo.ready?() do
{:ok, Projects.shell_snapshot()}
else
{:error, :not_ready}
end
default_project_snapshot()
end
def current_project(projects_snapshot) do
@@ -79,27 +76,19 @@ defmodule BDS.Desktop.ShellData do
end
def dashboard(project_id) do
Dashboard.snapshot(project_id)
rescue
error in [Exqlite.Error, DBConnection.OwnershipError] ->
if match?(%Exqlite.Error{}, error) and
not String.contains?(Exception.message(error), "no such table") do
reraise error, __STACKTRACE__
if Repo.ready?() do
{:ok, Dashboard.snapshot(project_id)}
else
{:error, :not_ready}
end
Dashboard.empty_snapshot()
end
def sidebar_view(project_id, view_id, params \\ %{}) do
Sidebar.view(project_id, view_id, params)
rescue
error in [Exqlite.Error, DBConnection.OwnershipError] ->
if match?(%Exqlite.Error{}, error) and
not String.contains?(Exception.message(error), "no such table") do
reraise error, __STACKTRACE__
if Repo.ready?() do
{:ok, Sidebar.view(project_id, view_id, params)}
else
{:error, :not_ready}
end
Sidebar.view(nil, view_id, params)
end
def assistant_cards do
@@ -146,14 +135,16 @@ defmodule BDS.Desktop.ShellData do
def git_badge_count(project_id, opts \\ [])
def git_badge_count(nil, _opts), do: 0
def git_badge_count("default", _opts), do: 0
def git_badge_count(nil, _opts), do: {:ok, 0}
def git_badge_count("default", _opts), do: {:ok, 0}
def git_badge_count(project_id, opts) when is_binary(project_id) do
if not Repo.ready?() do
{:error, :not_ready}
else
provider = Keyword.get(opts, :provider, git_remote_state_provider())
custom_provider? = provider != (&BDS.Git.remote_state/2)
try do
has_git =
custom_provider? ||
case BDS.Projects.get_project(project_id) do
@@ -161,6 +152,7 @@ defmodule BDS.Desktop.ShellData do
project -> File.dir?(Path.join(BDS.Projects.project_data_dir(project), ".git"))
end
count =
if has_git do
case provider.(project_id, []) do
{:ok, %{behind: behind}} when is_integer(behind) and behind > 0 -> behind
@@ -170,14 +162,8 @@ defmodule BDS.Desktop.ShellData do
else
0
end
rescue
error in [DBConnection.OwnershipError, Exqlite.Error] ->
if match?(%Exqlite.Error{}, error) and
not String.contains?(Exception.message(error), "no such table") do
reraise error, __STACKTRACE__
end
0
{:ok, count}
end
end
@@ -303,7 +289,7 @@ defmodule BDS.Desktop.ShellData do
defp maybe_add_panel_tab(tabs, _route, _tab), do: tabs
defp default_project_snapshot do
def default_project_snapshot do
%{
active_project_id: "default",
projects: [

35
lib/bds/desktop/shell_live.ex
View File

@@ -633,11 +633,14 @@ defmodule BDS.Desktop.ShellLive do
active_view_id = Atom.to_string(workbench.active_view)
sidebar_data =
ShellData.sidebar_view(
case ShellData.sidebar_view(
project_id,
active_view_id,
ShellSidebarState.current_filters(socket, active_view_id)
)
) do
{:ok, data} -> data
{:error, :not_ready} -> BDS.UI.Sidebar.view(nil, active_view_id, %{})
end
sidebar_data = ShellSidebarState.merge_ui_state(socket, active_view_id, sidebar_data)
@@ -647,17 +650,35 @@ defmodule BDS.Desktop.ShellLive do
end
defp refresh_content(socket, workbench) do
projects = ShellData.project_snapshot()
dashboard = ShellData.dashboard(projects.active_project_id)
git_badge_count = ShellData.git_badge_count(projects.active_project_id)
projects =
case ShellData.project_snapshot() do
{:ok, data} -> data
{:error, :not_ready} -> ShellData.default_project_snapshot()
end
dashboard =
case ShellData.dashboard(projects.active_project_id) do
{:ok, data} -> data
{:error, :not_ready} -> BDS.UI.Dashboard.empty_snapshot()
end
git_badge_count =
case ShellData.git_badge_count(projects.active_project_id) do
{:ok, count} -> count
{:error, :not_ready} -> 0
end
active_view_id = Atom.to_string(workbench.active_view)
sidebar_data =
ShellData.sidebar_view(
case ShellData.sidebar_view(
projects.active_project_id,
active_view_id,
ShellSidebarState.current_filters(socket, active_view_id)
)
) do
{:ok, data} -> data
{:error, :not_ready} -> BDS.UI.Sidebar.view(nil, active_view_id, %{})
end
sidebar_data = ShellSidebarState.merge_ui_state(socket, active_view_id, sidebar_data)

18
lib/bds/repo.ex
View File

@@ -2,4 +2,22 @@ defmodule BDS.Repo do
use Ecto.Repo,
otp_app: :bds,
adapter: Ecto.Adapters.SQLite3
@doc """
Returns true if the database is connected and core tables exist.
Used to guard data access during startup before migrations have run.
"""
def ready? do
case Ecto.Adapters.SQL.query(
__MODULE__,
"SELECT 1 FROM sqlite_master WHERE type = ?1 AND name = ?2 LIMIT 1",
["table", "projects"]
) do
{:ok, %{num_rows: 1}} -> true
{:ok, _} -> false
{:error, _} -> false
end
rescue
_ -> false
end
end

88
test/bds/csm010_rescue_control_flow_test.exs Normal file
View File

@@ -0,0 +1,88 @@
defmodule BDS.CSM010RescueControlFlowTest do
use ExUnit.Case, async: false
alias BDS.Desktop.ShellData
alias BDS.Repo
setup do
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
:ok
end
describe "Repo.ready?/0" do
test "returns true when the database is available" do
assert Repo.ready?()
end
end
describe "project_snapshot/0" do
test "returns {:ok, snapshot} when DB is ready" do
assert {:ok, snapshot} = ShellData.project_snapshot()
assert is_map(snapshot)
assert Map.has_key?(snapshot, :active_project_id)
assert Map.has_key?(snapshot, :projects)
end
test "returns {:error, :not_ready} when DB is unavailable" do
Ecto.Adapters.SQL.Sandbox.checkin(BDS.Repo)
assert {:error, :not_ready} = with_stopped_repo(&ShellData.project_snapshot/0)
end
end
describe "dashboard/1" do
test "returns {:ok, snapshot} when DB is ready" do
assert {:ok, dashboard} = ShellData.dashboard("default")
assert is_map(dashboard)
assert Map.has_key?(dashboard, :post_stats)
end
test "returns {:error, :not_ready} when DB is unavailable" do
Ecto.Adapters.SQL.Sandbox.checkin(BDS.Repo)
assert {:error, :not_ready} = with_stopped_repo(fn -> ShellData.dashboard("default") end)
end
end
describe "sidebar_view/3" do
test "returns {:ok, view} when DB is ready" do
assert {:ok, view} = ShellData.sidebar_view("default", "posts")
assert is_map(view)
end
test "returns {:error, :not_ready} when DB is unavailable" do
Ecto.Adapters.SQL.Sandbox.checkin(BDS.Repo)
assert {:error, :not_ready} =
with_stopped_repo(fn -> ShellData.sidebar_view("default", "posts") end)
end
end
describe "git_badge_count/2" do
test "returns {:ok, count} when DB is ready" do
assert {:ok, count} = ShellData.git_badge_count("default")
assert is_integer(count)
end
test "returns {:ok, 0} for nil project_id" do
assert {:ok, 0} = ShellData.git_badge_count(nil)
end
test "returns {:error, :not_ready} when DB is unavailable" do
Ecto.Adapters.SQL.Sandbox.checkin(BDS.Repo)
assert {:error, :not_ready} =
with_stopped_repo(fn -> ShellData.git_badge_count("some-project") end)
end
end
defp with_stopped_repo(fun) do
Supervisor.terminate_child(BDS.Supervisor, BDS.Repo)
try do
fun.()
after
Supervisor.restart_child(BDS.Supervisor, BDS.Repo)
end
end
end