From a83634da36a58638bd06193551578439a93dd035 Mon Sep 17 00:00:00 2001 From: Firehose Bot Date: Thu, 7 May 2026 13:24:11 +0100 Subject: [PATCH] refactor(Blogex.LinkValidator): simplify dead code and naming - Drop redundant :ok return in _validate_links/2 (blog.ex) - Remove dead HTML link regex from extract_links/1 (body is raw markdown) - Rename slug_slug_end/1 to slug_end/1 - Simplify parse_blog_link/1 to return {blog_id, slug}, removing parse_query_fragment/1 and dead case branches --- blogex/lib/blogex/blog.ex | 6 +- blogex/lib/blogex/link_validator.ex | 66 +++++----------------- blogex/test/blogex/link_validator_test.exs | 22 ++++---- 3 files changed, 27 insertions(+), 67 deletions(-) diff --git a/blogex/lib/blogex/blog.ex b/blogex/lib/blogex/blog.ex index 7926dbc..7741d70 100644 --- a/blogex/lib/blogex/blog.ex +++ b/blogex/lib/blogex/blog.ex @@ -153,9 +153,7 @@ defmodule Blogex.Blog do def _validate_links(posts, blog_id) do Enum.each(posts, fn post -> case Blogex.LinkValidator.validate_body(post.body, blog_id, post_id: post.id) do - :ok -> - :ok - + :ok -> :ok {:error, errors} -> raise Blogex.LinkError, blog: blog_id, @@ -163,7 +161,5 @@ defmodule Blogex.Blog do errors: errors end end) - - :ok end end diff --git a/blogex/lib/blogex/link_validator.ex b/blogex/lib/blogex/link_validator.ex index bb84416..48c0779 100644 --- a/blogex/lib/blogex/link_validator.ex +++ b/blogex/lib/blogex/link_validator.ex @@ -50,39 +50,27 @@ defmodule Blogex.LinkValidator do `/blog/{engineering|releases}/{slug}`. External links and non-blog internal links are ignored. - Handles both markdown link syntax `[text](url)` and HTML ``. + Handles markdown link syntax `[text](url)`. ## Examples - iex> extract_links("

[link](/blog/engineering/post)

") + iex> extract_links("[link](/blog/engineering/post)") ["/blog/engineering/post"] - iex> extract_links("

link

") - ["/blog/engineering/post"] - - iex> extract_links("

See [GitHub](https://github.com)

") + iex> extract_links("See [GitHub](https://github.com)") [] """ @spec extract_links(String.t()) :: [String.t()] def extract_links(body) when is_binary(body) do - markdown_links = - ~r/\[([^\]]+)\]\(([^)]+)\)/ - |> Regex.scan(body) - |> Enum.map(fn [_, _, path] -> path end) - - html_links = - ~r/ Regex.scan(body) - |> Enum.map(fn [_, path] -> path end) - - (markdown_links ++ html_links) - |> Enum.uniq() + ~r/\[([^\]]+)\]\(([^)]+)\)/ + |> Regex.scan(body) + |> Enum.map(fn [_, _, path] -> path end) |> Enum.filter(&internal_blog_link?/1) end defp internal_blog_link?(path) do case parse_blog_link(path) do - {_blog_id_str, _slug, _query, _fragment} -> true + {_, _} -> true nil -> false end end @@ -109,7 +97,7 @@ defmodule Blogex.LinkValidator do nil -> {:error, "not a blog link: #{link}"} - {blog_id_str, slug_part, _query, _fragment} -> + {blog_id_str, slug_part} -> case Map.fetch(@valid_blog_ids, blog_id_str) do {:ok, _blog_atom} -> validate_slug(slug_part) :error -> {:error, "unknown blog ID: #{blog_id_str}"} @@ -186,53 +174,29 @@ defmodule Blogex.LinkValidator do end end + # --- Private helpers --- @doc false - @spec parse_blog_link(String.t()) :: - {String.t(), String.t(), String.t() | nil, String.t() | nil} | nil + @spec parse_blog_link(String.t()) :: {String.t(), String.t()} | nil def parse_blog_link(path) do - # Parse /blog/{id}/{slug} with optional query string and/or fragment - with ["", "blog", blog_id, rest] <- String.split(path, "/", parts: 4), - {slug, query_fragment} <- String.split_at(rest, slug_slug_end(rest)), - {query, fragment} <- parse_query_fragment(query_fragment) do - case Map.fetch(@valid_blog_ids, blog_id) do - {:ok, _blog_atom} -> {blog_id, slug, query, fragment} - :error -> {blog_id, slug, query, fragment} - end + with ["", "blog", blog_id, rest] <- String.split(path, "/", parts: 4) do + slug = String.slice(rest, 0, slug_end(rest)) + {blog_id, slug} else _ -> nil end end @doc false - @spec slug_slug_end(String.t()) :: integer() - defp slug_slug_end(str) do + @spec slug_end(String.t()) :: integer() + defp slug_end(str) do case String.split(str, ["?", "#"], parts: 2) do [slug | _] -> String.length(slug) _ -> String.length(str) end end - @doc false - @spec parse_query_fragment(String.t()) :: {String.t() | nil, String.t() | nil} - defp parse_query_fragment("") do - {nil, nil} - end - - defp parse_query_fragment("?" <> query) do - case String.split(query, "#", parts: 2) do - [q, f] -> {q, f} - [q] -> {q, nil} - end - end - - defp parse_query_fragment("#" <> fragment) do - {nil, fragment} - end - - defp parse_query_fragment(_), do: {nil, nil} - @doc false @spec validate_slug(String.t()) :: :ok | {:error, String.t()} defp validate_slug(slug) when slug == "" do diff --git a/blogex/test/blogex/link_validator_test.exs b/blogex/test/blogex/link_validator_test.exs index 5b72413..a68017c 100644 --- a/blogex/test/blogex/link_validator_test.exs +++ b/blogex/test/blogex/link_validator_test.exs @@ -5,7 +5,7 @@ defmodule Blogex.LinkValidatorTest do describe "extract_links/1" do test "extracts internal blog links from markdown body" do body = - "

Check out [hello world](/blog/engineering/hello-world) and [release v1](/blog/releases/v1-0-0).

" + "Check out [hello world](/blog/engineering/hello-world) and [release v1](/blog/releases/v1-0-0)." assert LinkValidator.extract_links(body) == [ "/blog/engineering/hello-world", @@ -14,25 +14,25 @@ defmodule Blogex.LinkValidatorTest do end test "ignores external links" do - body = "

See [GitHub](https://github.com) and [internal](/blog/engineering/post).

" + body = "See [GitHub](https://github.com) and [internal](/blog/engineering/post)." assert LinkValidator.extract_links(body) == ["/blog/engineering/post"] end test "ignores non-blog internal links" do - body = "

See [/about](/about) and [/blog/engineering/post](/blog/engineering/post).

" + body = "See [/about](/about) and [/blog/engineering/post](/blog/engineering/post)." assert LinkValidator.extract_links(body) == ["/blog/engineering/post"] end test "returns empty list when no internal blog links" do - body = "

Just external links: [GitHub](https://github.com).

" + body = "Just external links: [GitHub](https://github.com)." assert LinkValidator.extract_links(body) == [] end test "handles multiple links on one line" do - body = "

[a](/blog/engineering/a) [b](/blog/releases/b) [c](/blog/engineering/c)

" + body = "[a](/blog/engineering/a) [b](/blog/releases/b) [c](/blog/engineering/c)" assert LinkValidator.extract_links(body) == [ "/blog/engineering/a", @@ -42,13 +42,13 @@ defmodule Blogex.LinkValidatorTest do end test "handles links with query strings" do - body = "

[link](/blog/engineering/post?foo=bar)

" + body = "[link](/blog/engineering/post?foo=bar)" assert LinkValidator.extract_links(body) == ["/blog/engineering/post?foo=bar"] end test "handles links with anchor fragments" do - body = "

[link](/blog/engineering/post#section)

" + body = "[link](/blog/engineering/post#section)" assert LinkValidator.extract_links(body) == ["/blog/engineering/post#section"] end @@ -190,19 +190,19 @@ defmodule Blogex.LinkValidatorTest do describe "validate_body/2" do test "returns :ok when body has no internal blog links" do - body = "

Just text, no links.

" + body = "Just text, no links." assert LinkValidator.validate_body(body, :engineering) == :ok end test "returns :ok when all links are valid" do - body = "

[link](/blog/engineering/post)

" + body = "[link](/blog/engineering/post)" assert LinkValidator.validate_body(body, :engineering) == :ok end test "returns errors with post context" do - body = "

[link](/blog/unknown/post)

" + body = "[link](/blog/unknown/post)" assert LinkValidator.validate_body(body, :engineering) == { :error, @@ -218,7 +218,7 @@ defmodule Blogex.LinkValidatorTest do end test "includes post_id in error tuples when provided" do - body = "

[link](/blog/unknown/post)

" + body = "[link](/blog/unknown/post)" assert LinkValidator.validate_body(body, :engineering, post_id: "test-post") == { :error,