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

fix: decompose monolithic functions into focused helpers (CSM-023)

Browse Source 
Break up SRP violations in Templates.do_update_template/2 and
Capabilities.for_project/2 by extracting domain-specific builder
functions and single-responsibility private helpers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Chili Palmer
2026-05-11 09:15:48 +02:00
parent 09df925e9b
commit 7b383d31ab
4 changed files with 432 additions and 280 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
CODESMELL.md
View File

@@ -369,11 +369,20 @@
---
### CSM-023 — SRP Violations
- **Files:**
- `lib/bds/templates.ex:86-163` — `update_template/2` does slug changes, content changes, status transitions, file paths, transactions, cascades, and filesystem sync.
- `lib/bds/scripting/capabilities.ex:22-248` — `for_project/2` returns a 200+ line map literal.
- **Fix:** Decompose into smaller private pipelines or domain-specific builder functions.
### ~~CSM-023 — SRP Violations~~ ✅ FIXED
- **Fixed:** 2026-05-11
- **What was done:**
- **`lib/bds/templates.ex`** — `do_update_template/2`:
- Extracted `resolve_next_slug/2` — determines slug from attrs or keeps current.
- Extracted `content_changed?/2` — checks if content attr differs from effective content.
- Extracted `resolve_next_status/2` — pattern-matched function heads for status transition (published + content change → draft).
- Extracted `build_update_attrs/5` — assembles the changeset map from resolved values.
- Extracted `commit_update_transaction/4` — runs the Repo transaction with cascade logic.
- `do_update_template/2` is now a concise pipeline: resolve → build → commit → sync.
- **`lib/bds/scripting/capabilities.ex`** — `for_project/2`:
- Extracted 13 domain-specific builder functions: `app_capabilities/2`, `project_capabilities/1`, `meta_capabilities/1`, `post_capabilities/1`, `media_capabilities/1`, `script_capabilities/1`, `template_capabilities/1`, `tag_capabilities/1`, `task_capabilities/0`, `sync_capabilities/2`, `publish_capabilities/2`, `chat_capabilities/1`, `embedding_capabilities/1`.
- `for_project/2` is now a 15-line dispatch map.
- Added 5 tests in `test/bds/csm023_srp_violations_test.exs`: source-level assertions for helper extraction in templates, delegation in do_update_template, builder function presence in capabilities, concise for_project body (≤20 lines), no inline capability definitions in for_project.
---

104
lib/bds/scripting/capabilities.ex
View File

@@ -25,7 +25,24 @@ defmodule BDS.Scripting.Capabilities do
@spec for_project(String.t(), keyword()) :: map()
def for_project(project_id, opts \\ []) when is_binary(project_id) and is_list(opts) do
%{
app: %{
app: app_capabilities(project_id, opts),
projects: project_capabilities(project_id),
meta: meta_capabilities(project_id),
posts: post_capabilities(project_id),
media: media_capabilities(project_id),
scripts: script_capabilities(project_id),
templates: template_capabilities(project_id),
tags: tag_capabilities(project_id),
tasks: task_capabilities(),
sync: sync_capabilities(project_id, opts),
publish: publish_capabilities(project_id, opts),
chat: chat_capabilities(opts),
embeddings: embedding_capabilities(project_id)
}
end
defp app_capabilities(project_id, opts) do
%{
copy_to_clipboard: one_arg(fn text -> copy_to_clipboard(text, opts) end),
get_data_paths: zero_or_one_arg(fn _args -> data_paths(project_id) end),
get_blogmark_bookmarklet: zero_or_one_arg(fn _args -> blogmark_bookmarklet() end),
@@ -39,8 +56,11 @@ defmodule BDS.Scripting.Capabilities do
set_preview_post_target: one_arg(fn post_id -> set_preview_post_target(post_id) end),
show_item_in_folder: one_arg(fn item_path -> show_item_in_folder(item_path, opts) end),
trigger_menu_action: one_arg(fn action -> trigger_menu_action(action, opts) end)
},
projects: %{
}
end
defp project_capabilities(project_id) do
%{
create: zero_or_one_arg(fn attrs -> create_project(attrs) end),
delete: one_arg(fn project_id_to_delete -> delete_project(project_id_to_delete) end),
delete_with_data:
@@ -54,8 +74,11 @@ defmodule BDS.Scripting.Capabilities do
two_arg(fn project_id_to_update, attrs ->
update_project(project_id_to_update, attrs)
end)
},
meta: %{
}
end
defp meta_capabilities(project_id) do
%{
get_project_metadata: zero_or_one_arg(fn _args -> load_metadata(project_id) end),
update_project_metadata:
one_arg(fn attrs -> update_project_metadata(project_id, attrs) end),
@@ -73,8 +96,11 @@ defmodule BDS.Scripting.Capabilities do
clear_publishing_preferences:
zero_or_one_arg(fn _args -> clear_publishing_preferences(project_id) end),
sync_on_startup: zero_or_one_arg(fn _args -> sync_meta_on_startup(project_id) end)
},
posts: %{
}
end
defp post_capabilities(project_id) do
%{
create: one_arg(fn attrs -> create_post(project_id, attrs) end),
discard: one_arg(fn post_id -> discard_post(project_id, post_id) end),
filter: one_arg(fn filters -> filter_posts(project_id, filters) end),
@@ -120,8 +146,11 @@ defmodule BDS.Scripting.Capabilities do
rebuild_links: zero_or_one_arg(fn _args -> rebuild_post_links(project_id) end),
reindex_text: zero_or_one_arg(fn _args -> reindex_project_search(project_id) end),
search: one_arg(fn query -> search_posts(project_id, query) end)
},
media: %{
}
end
defp media_capabilities(project_id) do
%{
delete_translation:
two_arg(fn media_id, language ->
delete_media_translation(project_id, media_id, language)
@@ -161,8 +190,11 @@ defmodule BDS.Scripting.Capabilities do
three_arg(fn media_id, language, attrs ->
upsert_media_translation(project_id, media_id, language, attrs)
end)
},
scripts: %{
}
end
defp script_capabilities(project_id) do
%{
create: one_arg(fn attrs -> create_script(project_id, attrs) end),
update: two_arg(fn script_id, attrs -> update_script(project_id, script_id, attrs) end),
delete: one_arg(fn script_id -> delete_script(project_id, script_id) end),
@@ -171,8 +203,11 @@ defmodule BDS.Scripting.Capabilities do
publish: one_arg(fn script_id -> publish_script(project_id, script_id) end),
rebuild_from_files:
zero_or_one_arg(fn _args -> rebuild_scripts_from_files(project_id) end)
},
templates: %{
}
end
defp template_capabilities(project_id) do
%{
create: one_arg(fn attrs -> create_template(project_id, attrs) end),
update:
two_arg(fn template_id, attrs -> update_template(project_id, template_id, attrs) end),
@@ -184,8 +219,11 @@ defmodule BDS.Scripting.Capabilities do
rebuild_from_files:
zero_or_one_arg(fn _args -> rebuild_templates_from_files(project_id) end),
validate: one_arg(fn source -> validate_template_source(source) end)
},
tags: %{
}
end
defp tag_capabilities(project_id) do
%{
create: one_arg(fn attrs -> create_tag(project_id, attrs) end),
update: two_arg(fn tag_id, attrs -> update_tag(project_id, tag_id, attrs) end),
delete: one_arg(fn tag_id -> delete_tag(project_id, tag_id) end),
@@ -200,16 +238,22 @@ defmodule BDS.Scripting.Capabilities do
end),
rename: two_arg(fn tag_id, new_name -> rename_tag(project_id, tag_id, new_name) end),
sync_from_posts: zero_or_one_arg(fn _args -> sync_tags_from_posts(project_id) end)
},
tasks: %{
}
end
defp task_capabilities do
%{
get: one_arg(fn task_id -> load_task(task_id) end),
status_snapshot: zero_or_one_arg(fn _args -> sanitize(Tasks.status_snapshot()) end),
cancel: one_arg(fn task_id -> cancel_task(task_id) end),
get_all: zero_or_one_arg(fn _args -> list_all_tasks() end),
get_running: zero_or_one_arg(fn _args -> list_running_tasks() end),
clear_completed: zero_or_one_arg(fn _args -> clear_completed_tasks() end)
},
sync: %{
}
end
defp sync_capabilities(project_id, opts) do
%{
check_availability: zero_or_one_arg(fn _args -> sync_available?() end),
get_repo_state: zero_or_one_arg(fn _args -> repo_state(project_id, opts) end),
get_status: zero_or_one_arg(fn _args -> repo_status(project_id, opts) end),
@@ -219,11 +263,17 @@ defmodule BDS.Scripting.Capabilities do
pull: zero_or_one_arg(fn _args -> repo_pull(project_id, opts) end),
push: zero_or_one_arg(fn _args -> repo_push(project_id, opts) end),
commit_all: one_arg(fn message -> repo_commit_all(project_id, message, opts) end)
},
publish: %{
}
end
defp publish_capabilities(project_id, opts) do
%{
upload_site: one_arg(fn credentials -> upload_site(project_id, credentials, opts) end)
},
chat: %{
}
end
defp chat_capabilities(opts) do
%{
detect_post_language:
two_arg(fn title, content -> detect_post_language(title, content, opts) end),
analyze_post: one_arg(fn post_id -> analyze_post(post_id, opts) end),
@@ -236,8 +286,11 @@ defmodule BDS.Scripting.Capabilities do
end),
translate_media_metadata:
two_arg(fn media_id, language -> translate_media_metadata(media_id, language, opts) end)
},
embeddings: %{
}
end
defp embedding_capabilities(project_id) do
%{
get_progress: zero_or_one_arg(fn _args -> embedding_progress(project_id) end),
find_similar: two_arg(fn post_id, limit -> find_similar(post_id, limit) end),
compute_similarities:
@@ -248,6 +301,5 @@ defmodule BDS.Scripting.Capabilities do
dismiss_pair: two_arg(fn post_id_a, post_id_b -> dismiss_pair(post_id_a, post_id_b) end),
index_unindexed_posts: zero_or_one_arg(fn _args -> index_unindexed_posts(project_id) end)
}
}
end
end

56
lib/bds/templates.ex
View File

@@ -95,7 +95,27 @@ defmodule BDS.Templates do
end
defp do_update_template(template, attrs) do
next_slug =
now = Persistence.now_ms()
next_slug = resolve_next_slug(template, attrs)
slug_changed? = next_slug != template.slug
content_changed? = content_changed?(template, attrs)
next_status = resolve_next_status(template, content_changed?)
updates = build_update_attrs(template, attrs, next_slug, next_status, now)
with {:ok, {updated_template, affected_posts}} <-
commit_update_transaction(template, updates, slug_changed?, now),
:ok <-
sync_template_update_side_effects(
template,
updated_template,
affected_posts,
slug_changed?
) do
{:ok, updated_template}
end
end
defp resolve_next_slug(template, attrs) do
if has_attr?(attrs, :slug) do
unique_slug(
template.project_id,
@@ -106,35 +126,30 @@ defmodule BDS.Templates do
else
template.slug
end
end
content_changed? =
defp content_changed?(template, attrs) do
has_attr?(attrs, :content) and
attr(attrs, :content) != effective_template_content(template)
end
slug_changed? = next_slug != template.slug
now = Persistence.now_ms()
defp resolve_next_status(%Template{status: :published}, true = _content_changed?), do: :draft
defp resolve_next_status(%Template{status: status}, _content_changed?), do: status
next_status =
if(template.status == :published and content_changed?,
do: :draft,
else: template.status
)
next_file_path = next_template_file_path(template, next_slug)
updates =
defp build_update_attrs(template, attrs, next_slug, next_status, now) do
%{}
|> maybe_put(:title, attr(attrs, :title))
|> maybe_put(:kind, attr(attrs, :kind))
|> maybe_put(:enabled, attr(attrs, :enabled))
|> maybe_put(:content, attr(attrs, :content))
|> Map.put(:file_path, next_file_path)
|> Map.put(:file_path, next_template_file_path(template, next_slug))
|> Map.put(:slug, next_slug)
|> Map.put(:version, template.version + 1)
|> Map.put(:updated_at, now)
|> Map.put(:status, next_status)
end
with {:ok, {updated_template, affected_posts}} <-
defp commit_update_transaction(template, updates, slug_changed?, now) do
Repo.transaction(fn ->
updated_template =
template
@@ -149,16 +164,7 @@ defmodule BDS.Templates do
end
{updated_template, affected_posts}
end),
:ok <-
sync_template_update_side_effects(
template,
updated_template,
affected_posts,
slug_changed?
) do
{:ok, updated_template}
end
end)
end
@spec rebuild_templates_from_files(String.t(), keyword()) :: {:ok, [Template.t()]}

85
test/bds/csm023_srp_violations_test.exs Normal file
View File

@@ -0,0 +1,85 @@
defmodule BDS.CSM023SrpViolationsTest do
use ExUnit.Case, async: true
describe "Templates.do_update_template/2 decomposition" do
test "update_template pipeline is decomposed into focused private functions" do
source = File.read!("lib/bds/templates.ex")
assert source =~ "defp resolve_next_slug("
assert source =~ "defp content_changed?("
assert source =~ "defp resolve_next_status("
assert source =~ "defp build_update_attrs("
assert source =~ "defp commit_update_transaction("
refute source =~ "defp do_update_template(template, attrs) do\n next_slug =",
"do_update_template should delegate to focused helpers, not inline all logic"
end
test "do_update_template delegates to helpers instead of inlining" do
source = File.read!("lib/bds/templates.ex")
[do_update_body] =
Regex.scan(
~r/defp do_update_template\(template, attrs\) do\n(.*?)(?=\n defp )/s,
source,
capture: :all_but_first
)
|> Enum.map(&hd/1)
assert do_update_body =~ "resolve_next_slug(template, attrs)"
assert do_update_body =~ "content_changed?(template, attrs)"
assert do_update_body =~ "resolve_next_status(template, content_changed?)"
assert do_update_body =~ "build_update_attrs(template, attrs, next_slug, next_status, now)"
assert do_update_body =~ "commit_update_transaction(template, updates, slug_changed?, now)"
end
end
describe "Capabilities.for_project/2 decomposition" do
test "for_project delegates to domain-specific builder functions" do
source = File.read!("lib/bds/scripting/capabilities.ex")
assert source =~ "defp app_capabilities("
assert source =~ "defp project_capabilities("
assert source =~ "defp meta_capabilities("
assert source =~ "defp post_capabilities("
assert source =~ "defp media_capabilities("
assert source =~ "defp script_capabilities("
assert source =~ "defp template_capabilities("
assert source =~ "defp tag_capabilities("
assert source =~ "defp task_capabilities"
assert source =~ "defp sync_capabilities("
assert source =~ "defp publish_capabilities("
assert source =~ "defp chat_capabilities("
assert source =~ "defp embedding_capabilities("
end
test "for_project body is a concise map of builder calls" do
source = File.read!("lib/bds/scripting/capabilities.ex")
for_project_body = extract_for_project_body(source)
line_count = for_project_body |> String.split("\n") |> length()
assert line_count <= 20,
"for_project body should be a concise dispatch map, got #{line_count} lines"
end
test "no domain-specific capability entries remain in for_project body" do
source = File.read!("lib/bds/scripting/capabilities.ex")
for_project_body = extract_for_project_body(source)
refute for_project_body =~ "one_arg(",
"for_project body should not contain one_arg calls directly"
refute for_project_body =~ "zero_or_one_arg(",
"for_project body should not contain zero_or_one_arg calls directly"
end
defp extract_for_project_body(source) do
lines = String.split(source, "\n")
start_idx = Enum.find_index(lines, &String.contains?(&1, "def for_project("))
remaining = Enum.drop(lines, start_idx + 1)
end_idx = Enum.find_index(remaining, &(&1 == " end"))
remaining |> Enum.take(end_idx) |> Enum.join("\n")
end
end
end