diff --git a/CODESMELL.md b/CODESMELL.md index b95ca0b..e6c64a7 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -270,15 +270,15 @@ --- -### CSM-016 — String Concatenation for Paths -- **Files:** - - `lib/bds/rendering/metadata.ex:43` — `"/#{slug}/"` - - `lib/bds/rendering/metadata.ex:112` — `prefix <> "/"` - - `lib/bds/publishing.ex:284` — `String.trim_trailing(path, "/") <> "/"` - - `lib/bds/rendering/file_system.ex:29` — `normalized_path <> ".liquid"` - - `lib/bds/rendering/links_and_languages.ex` — path construction with `<>` -- **Fix:** Use `Path.join/1-2` and `Path.extname` / `Path.rootname`. For `"/#{slug}/"`, use `Path.join(["/", slug])` or `"/" <> slug <> "/"` → `URI.encode(slug)` is already used elsewhere. -- **Test:** Test paths with trailing slashes, empty segments, and special characters. +### ~~CSM-016 — String Concatenation for Paths~~ ✅ FIXED +- **Fixed:** 2026-05-09 +- **What was done:** + - **`lib/bds/rendering/file_system.ex`** — Extracted `ensure_liquid_ext/1` using `Path.extname/1` to check before appending `.liquid`, preventing double-extension bugs (e.g. `"header.liquid.liquid"`). + - **`lib/bds/rendering/metadata.ex`** — `menu_item_href` for `:page` kind now applies `URI.encode/1` to the slug (matching the existing `:category_archive` pattern). `href_for_language/1` now uses `String.trim_trailing(prefix, "/")` before appending `/` to prevent double trailing slashes. + - **`lib/bds/rendering/metadata.ex`** — Added `menu_items_from_raw/1` public function for testability. + - **`lib/bds/rendering/links_and_languages.ex`** — `post_path/2` for `nil` language now uses `Path.join(["/", year, month, day, slug]) <> "/"` instead of building with `index.html` then stripping it. Language-prefix clause uses `String.trim_trailing/2` to prevent double slashes. `canonical_media_path_by_source_path/1` uses `Path.join("/", media.file_path)` instead of `"/" <> file_path`. + - **`lib/bds/publishing.ex`** — `ensure_trailing_slash/1` made public for testability (implementation already correct). + - Added 17 tests in `test/bds/csm016_path_concatenation_test.exs`: FileSystem extension handling (bare name, double extension, nested paths), `href_for_language` (empty, with/without trailing slash), menu item href encoding (special chars, plain slugs, category slugs), post_path construction (leading/trailing slashes, no double slashes, language prefix), `language_prefix` (same/nil/different language), `ensure_trailing_slash` (without/with trailing slash, empty string). --- diff --git a/config/config.exs b/config/config.exs index f39b1e0..e0f4897 100644 --- a/config/config.exs +++ b/config/config.exs @@ -6,6 +6,7 @@ config :bds, config :bds, BDS.Repo, database: Path.expand("../priv/data/bds_dev.db", __DIR__), pool_size: 5, + journal_mode: :wal, busy_timeout: 15_000, log: false, stacktrace: true, diff --git a/config/test.exs b/config/test.exs index a31767d..c32ea51 100644 --- a/config/test.exs +++ b/config/test.exs @@ -4,6 +4,7 @@ config :bds, BDS.Repo, database: Path.expand("../priv/data/bds_test.db", __DIR__), pool: Ecto.Adapters.SQL.Sandbox, pool_size: 5, + journal_mode: :wal, busy_timeout: 15_000 config :logger, level: :warning diff --git a/lib/bds/publishing.ex b/lib/bds/publishing.ex index a510684..2fc4e76 100644 --- a/lib/bds/publishing.ex +++ b/lib/bds/publishing.ex @@ -283,7 +283,7 @@ defmodule BDS.Publishing do defp rsync_excludes(%{kind: :media}), do: ["--exclude=*.meta"] defp rsync_excludes(_target), do: [] - defp ensure_trailing_slash(path), do: String.trim_trailing(path, "/") <> "/" + def ensure_trailing_slash(path), do: String.trim_trailing(path, "/") <> "/" defp remote_dir_spec(credentials, remote_dir) do remote_base(credentials) <> ":" <> ensure_trailing_slash(remote_dir) diff --git a/lib/bds/rendering/file_system.ex b/lib/bds/rendering/file_system.ex index d826aed..3d8b06b 100644 --- a/lib/bds/rendering/file_system.ex +++ b/lib/bds/rendering/file_system.ex @@ -25,18 +25,24 @@ defmodule BDS.Rendering.FileSystem do raise Liquex.Error, message: "Illegal template path '#{template_path}'" true -> + filename = ensure_liquid_ext(normalized_path) + root_paths - |> Enum.map(&Path.expand(Path.join(&1, normalized_path <> ".liquid"))) + |> Enum.map(&Path.expand(Path.join(&1, filename))) |> Enum.find(&File.regular?/1) |> case do nil -> - Path.expand(Path.join(List.first(root_paths) || ".", normalized_path <> ".liquid")) + Path.expand(Path.join(List.first(root_paths) || ".", filename)) path -> path end end end + + defp ensure_liquid_ext(path) do + if Path.extname(path) == ".liquid", do: path, else: path <> ".liquid" + end end defimpl Liquex.FileSystem, for: BDS.Rendering.FileSystem do diff --git a/lib/bds/rendering/links_and_languages.ex b/lib/bds/rendering/links_and_languages.ex index bc6211a..62efe8b 100644 --- a/lib/bds/rendering/links_and_languages.ex +++ b/lib/bds/rendering/links_and_languages.ex @@ -50,13 +50,13 @@ defmodule BDS.Rendering.LinksAndLanguages do ]) |> String.downcase() - Map.put(acc, source_key, "/" <> media.file_path) + Map.put(acc, source_key, Path.join("/", media.file_path)) end) end def post_path(post, language_prefix) when is_binary(language_prefix) and language_prefix != "" do - language_prefix <> post_path(post, nil) + String.trim_trailing(language_prefix, "/") <> post_path(post, nil) end def post_path(post, ""), do: post_path(post, nil) @@ -65,13 +65,12 @@ defmodule BDS.Rendering.LinksAndLanguages do datetime = Persistence.from_unix_ms!(post.created_at) Path.join([ + "/", Integer.to_string(datetime.year), String.pad_leading(Integer.to_string(datetime.month), 2, "0"), String.pad_leading(Integer.to_string(datetime.day), 2, "0"), - post.slug, - "index.html" - ]) - |> then(&("/" <> String.trim_trailing(&1, "index.html"))) + post.slug + ]) <> "/" end def post_path(post, language, main_language) do diff --git a/lib/bds/rendering/metadata.ex b/lib/bds/rendering/metadata.ex index bbe1f4f..15046c7 100644 --- a/lib/bds/rendering/metadata.ex +++ b/lib/bds/rendering/metadata.ex @@ -24,6 +24,10 @@ defmodule BDS.Rendering.Metadata do Enum.map(items, &to_template_menu_item/1) end + def menu_items_from_raw(items) when is_list(items) do + Enum.map(items, &to_template_menu_item/1) + end + defp to_template_menu_item(item) do kind = Map.get(item, :kind) children = Enum.map(Map.get(item, :children, []), &to_template_menu_item/1) @@ -40,7 +44,7 @@ defmodule BDS.Rendering.Metadata do defp menu_item_href(%{kind: :home}), do: "/" defp menu_item_href(%{kind: :page, slug: slug}) when is_binary(slug) and slug != "", - do: "/#{slug}/" + do: "/#{URI.encode(slug)}/" defp menu_item_href(%{kind: :category_archive, slug: slug}) when is_binary(slug) and slug != "", do: "/category/#{URI.encode(slug)}/" @@ -109,7 +113,7 @@ defmodule BDS.Rendering.Metadata do def default_pico_stylesheet_href(theme), do: PreviewAssets.stylesheet_href(theme) def href_for_language(""), do: "/" - def href_for_language(prefix), do: prefix <> "/" + def href_for_language(prefix), do: String.trim_trailing(prefix, "/") <> "/" def calendar_initial_year(%{created_at: created_at}) when is_integer(created_at), do: Persistence.from_unix_ms!(created_at).year diff --git a/test/bds/csm007_reload_shell_test.exs b/test/bds/csm007_reload_shell_test.exs index 65fd0c3..a832819 100644 --- a/test/bds/csm007_reload_shell_test.exs +++ b/test/bds/csm007_reload_shell_test.exs @@ -8,6 +8,7 @@ defmodule BDS.CSM007ReloadShellTest do setup do :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo) + Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()}) temp_dir = Path.join(System.tmp_dir!(), "bds-csm007-#{System.unique_integer([:positive])}") diff --git a/test/bds/csm008_render_path_test.exs b/test/bds/csm008_render_path_test.exs index 18233a5..84c4420 100644 --- a/test/bds/csm008_render_path_test.exs +++ b/test/bds/csm008_render_path_test.exs @@ -8,6 +8,7 @@ defmodule BDS.CSM008RenderPathTest do setup do :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo) + Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()}) temp_dir = Path.join(System.tmp_dir!(), "bds-csm008-#{System.unique_integer([:positive])}") diff --git a/test/bds/csm011_url_state_test.exs b/test/bds/csm011_url_state_test.exs index 12a318c..1851c42 100644 --- a/test/bds/csm011_url_state_test.exs +++ b/test/bds/csm011_url_state_test.exs @@ -8,6 +8,7 @@ defmodule BDS.CSM011UrlStateTest do setup do :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo) + Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()}) prev = System.get_env("BDS_DESKTOP_AUTOMATION") System.put_env("BDS_DESKTOP_AUTOMATION", "1") diff --git a/test/bds/csm012_file_picker_async_test.exs b/test/bds/csm012_file_picker_async_test.exs index cbe2943..61e4c45 100644 --- a/test/bds/csm012_file_picker_async_test.exs +++ b/test/bds/csm012_file_picker_async_test.exs @@ -8,6 +8,7 @@ defmodule BDS.CSM012FilePickerAsyncTest do setup do :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo) + Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()}) prev = System.get_env("BDS_DESKTOP_AUTOMATION") System.put_env("BDS_DESKTOP_AUTOMATION", "1")