fix: fix CSM-016 for real (previous commit was 015)
This commit is contained in:
18
CODESMELL.md
18
CODESMELL.md
@@ -270,15 +270,15 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### CSM-016 — String Concatenation for Paths
|
### ~~CSM-016 — String Concatenation for Paths~~ ✅ FIXED
|
||||||
- **Files:**
|
- **Fixed:** 2026-05-09
|
||||||
- `lib/bds/rendering/metadata.ex:43` — `"/#{slug}/"`
|
- **What was done:**
|
||||||
- `lib/bds/rendering/metadata.ex:112` — `prefix <> "/"`
|
- **`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/publishing.ex:284` — `String.trim_trailing(path, "/") <> "/"`
|
- **`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/file_system.ex:29` — `normalized_path <> ".liquid"`
|
- **`lib/bds/rendering/metadata.ex`** — Added `menu_items_from_raw/1` public function for testability.
|
||||||
- `lib/bds/rendering/links_and_languages.ex` — path construction with `<>`
|
- **`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`.
|
||||||
- **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.
|
- **`lib/bds/publishing.ex`** — `ensure_trailing_slash/1` made public for testability (implementation already correct).
|
||||||
- **Test:** Test paths with trailing slashes, empty segments, and special characters.
|
- 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).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ config :bds,
|
|||||||
config :bds, BDS.Repo,
|
config :bds, BDS.Repo,
|
||||||
database: Path.expand("../priv/data/bds_dev.db", __DIR__),
|
database: Path.expand("../priv/data/bds_dev.db", __DIR__),
|
||||||
pool_size: 5,
|
pool_size: 5,
|
||||||
|
journal_mode: :wal,
|
||||||
busy_timeout: 15_000,
|
busy_timeout: 15_000,
|
||||||
log: false,
|
log: false,
|
||||||
stacktrace: true,
|
stacktrace: true,
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ config :bds, BDS.Repo,
|
|||||||
database: Path.expand("../priv/data/bds_test.db", __DIR__),
|
database: Path.expand("../priv/data/bds_test.db", __DIR__),
|
||||||
pool: Ecto.Adapters.SQL.Sandbox,
|
pool: Ecto.Adapters.SQL.Sandbox,
|
||||||
pool_size: 5,
|
pool_size: 5,
|
||||||
|
journal_mode: :wal,
|
||||||
busy_timeout: 15_000
|
busy_timeout: 15_000
|
||||||
|
|
||||||
config :logger, level: :warning
|
config :logger, level: :warning
|
||||||
|
|||||||
@@ -283,7 +283,7 @@ defmodule BDS.Publishing do
|
|||||||
defp rsync_excludes(%{kind: :media}), do: ["--exclude=*.meta"]
|
defp rsync_excludes(%{kind: :media}), do: ["--exclude=*.meta"]
|
||||||
defp rsync_excludes(_target), do: []
|
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
|
defp remote_dir_spec(credentials, remote_dir) do
|
||||||
remote_base(credentials) <> ":" <> ensure_trailing_slash(remote_dir)
|
remote_base(credentials) <> ":" <> ensure_trailing_slash(remote_dir)
|
||||||
|
|||||||
@@ -25,18 +25,24 @@ defmodule BDS.Rendering.FileSystem do
|
|||||||
raise Liquex.Error, message: "Illegal template path '#{template_path}'"
|
raise Liquex.Error, message: "Illegal template path '#{template_path}'"
|
||||||
|
|
||||||
true ->
|
true ->
|
||||||
|
filename = ensure_liquid_ext(normalized_path)
|
||||||
|
|
||||||
root_paths
|
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)
|
|> Enum.find(&File.regular?/1)
|
||||||
|> case do
|
|> case do
|
||||||
nil ->
|
nil ->
|
||||||
Path.expand(Path.join(List.first(root_paths) || ".", normalized_path <> ".liquid"))
|
Path.expand(Path.join(List.first(root_paths) || ".", filename))
|
||||||
|
|
||||||
path ->
|
path ->
|
||||||
path
|
path
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp ensure_liquid_ext(path) do
|
||||||
|
if Path.extname(path) == ".liquid", do: path, else: path <> ".liquid"
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defimpl Liquex.FileSystem, for: BDS.Rendering.FileSystem do
|
defimpl Liquex.FileSystem, for: BDS.Rendering.FileSystem do
|
||||||
|
|||||||
@@ -50,13 +50,13 @@ defmodule BDS.Rendering.LinksAndLanguages do
|
|||||||
])
|
])
|
||||||
|> String.downcase()
|
|> String.downcase()
|
||||||
|
|
||||||
Map.put(acc, source_key, "/" <> media.file_path)
|
Map.put(acc, source_key, Path.join("/", media.file_path))
|
||||||
end)
|
end)
|
||||||
end
|
end
|
||||||
|
|
||||||
def post_path(post, language_prefix)
|
def post_path(post, language_prefix)
|
||||||
when is_binary(language_prefix) and language_prefix != "" do
|
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
|
end
|
||||||
|
|
||||||
def post_path(post, ""), do: post_path(post, nil)
|
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)
|
datetime = Persistence.from_unix_ms!(post.created_at)
|
||||||
|
|
||||||
Path.join([
|
Path.join([
|
||||||
|
"/",
|
||||||
Integer.to_string(datetime.year),
|
Integer.to_string(datetime.year),
|
||||||
String.pad_leading(Integer.to_string(datetime.month), 2, "0"),
|
String.pad_leading(Integer.to_string(datetime.month), 2, "0"),
|
||||||
String.pad_leading(Integer.to_string(datetime.day), 2, "0"),
|
String.pad_leading(Integer.to_string(datetime.day), 2, "0"),
|
||||||
post.slug,
|
post.slug
|
||||||
"index.html"
|
]) <> "/"
|
||||||
])
|
|
||||||
|> then(&("/" <> String.trim_trailing(&1, "index.html")))
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def post_path(post, language, main_language) do
|
def post_path(post, language, main_language) do
|
||||||
|
|||||||
@@ -24,6 +24,10 @@ defmodule BDS.Rendering.Metadata do
|
|||||||
Enum.map(items, &to_template_menu_item/1)
|
Enum.map(items, &to_template_menu_item/1)
|
||||||
end
|
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
|
defp to_template_menu_item(item) do
|
||||||
kind = Map.get(item, :kind)
|
kind = Map.get(item, :kind)
|
||||||
children = Enum.map(Map.get(item, :children, []), &to_template_menu_item/1)
|
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: :home}), do: "/"
|
||||||
|
|
||||||
defp menu_item_href(%{kind: :page, slug: slug}) when is_binary(slug) and slug != "",
|
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 != "",
|
defp menu_item_href(%{kind: :category_archive, slug: slug}) when is_binary(slug) and slug != "",
|
||||||
do: "/category/#{URI.encode(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 default_pico_stylesheet_href(theme), do: PreviewAssets.stylesheet_href(theme)
|
||||||
|
|
||||||
def href_for_language(""), do: "/"
|
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),
|
def calendar_initial_year(%{created_at: created_at}) when is_integer(created_at),
|
||||||
do: Persistence.from_unix_ms!(created_at).year
|
do: Persistence.from_unix_ms!(created_at).year
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ defmodule BDS.CSM007ReloadShellTest do
|
|||||||
|
|
||||||
setup do
|
setup do
|
||||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||||
|
Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
|
||||||
|
|
||||||
temp_dir =
|
temp_dir =
|
||||||
Path.join(System.tmp_dir!(), "bds-csm007-#{System.unique_integer([:positive])}")
|
Path.join(System.tmp_dir!(), "bds-csm007-#{System.unique_integer([:positive])}")
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ defmodule BDS.CSM008RenderPathTest do
|
|||||||
|
|
||||||
setup do
|
setup do
|
||||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||||
|
Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
|
||||||
|
|
||||||
temp_dir =
|
temp_dir =
|
||||||
Path.join(System.tmp_dir!(), "bds-csm008-#{System.unique_integer([:positive])}")
|
Path.join(System.tmp_dir!(), "bds-csm008-#{System.unique_integer([:positive])}")
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ defmodule BDS.CSM011UrlStateTest do
|
|||||||
|
|
||||||
setup do
|
setup do
|
||||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||||
|
Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
|
||||||
|
|
||||||
prev = System.get_env("BDS_DESKTOP_AUTOMATION")
|
prev = System.get_env("BDS_DESKTOP_AUTOMATION")
|
||||||
System.put_env("BDS_DESKTOP_AUTOMATION", "1")
|
System.put_env("BDS_DESKTOP_AUTOMATION", "1")
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ defmodule BDS.CSM012FilePickerAsyncTest do
|
|||||||
|
|
||||||
setup do
|
setup do
|
||||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||||
|
Ecto.Adapters.SQL.Sandbox.mode(BDS.Repo, {:shared, self()})
|
||||||
|
|
||||||
prev = System.get_env("BDS_DESKTOP_AUTOMATION")
|
prev = System.get_env("BDS_DESKTOP_AUTOMATION")
|
||||||
System.put_env("BDS_DESKTOP_AUTOMATION", "1")
|
System.put_env("BDS_DESKTOP_AUTOMATION", "1")
|
||||||
|
|||||||
Reference in New Issue
Block a user