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
This commit is contained in:
parent
1f0423841a
commit
a83634da36
@ -153,9 +153,7 @@ defmodule Blogex.Blog do
|
|||||||
def _validate_links(posts, blog_id) do
|
def _validate_links(posts, blog_id) do
|
||||||
Enum.each(posts, fn post ->
|
Enum.each(posts, fn post ->
|
||||||
case Blogex.LinkValidator.validate_body(post.body, blog_id, post_id: post.id) do
|
case Blogex.LinkValidator.validate_body(post.body, blog_id, post_id: post.id) do
|
||||||
:ok ->
|
:ok -> :ok
|
||||||
:ok
|
|
||||||
|
|
||||||
{:error, errors} ->
|
{:error, errors} ->
|
||||||
raise Blogex.LinkError,
|
raise Blogex.LinkError,
|
||||||
blog: blog_id,
|
blog: blog_id,
|
||||||
@ -163,7 +161,5 @@ defmodule Blogex.Blog do
|
|||||||
errors: errors
|
errors: errors
|
||||||
end
|
end
|
||||||
end)
|
end)
|
||||||
|
|
||||||
:ok
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -50,39 +50,27 @@ defmodule Blogex.LinkValidator do
|
|||||||
`/blog/{engineering|releases}/{slug}`. External links and non-blog
|
`/blog/{engineering|releases}/{slug}`. External links and non-blog
|
||||||
internal links are ignored.
|
internal links are ignored.
|
||||||
|
|
||||||
Handles both markdown link syntax `[text](url)` and HTML `<a href="url">`.
|
Handles markdown link syntax `[text](url)`.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
iex> extract_links("<p>[link](/blog/engineering/post)</p>")
|
iex> extract_links("[link](/blog/engineering/post)")
|
||||||
["/blog/engineering/post"]
|
["/blog/engineering/post"]
|
||||||
|
|
||||||
iex> extract_links("<p><a href=\"/blog/engineering/post\">link</a></p>")
|
iex> extract_links("See [GitHub](https://github.com)")
|
||||||
["/blog/engineering/post"]
|
|
||||||
|
|
||||||
iex> extract_links("<p>See [GitHub](https://github.com)</p>")
|
|
||||||
[]
|
[]
|
||||||
"""
|
"""
|
||||||
@spec extract_links(String.t()) :: [String.t()]
|
@spec extract_links(String.t()) :: [String.t()]
|
||||||
def extract_links(body) when is_binary(body) do
|
def extract_links(body) when is_binary(body) do
|
||||||
markdown_links =
|
~r/\[([^\]]+)\]\(([^)]+)\)/
|
||||||
~r/\[([^\]]+)\]\(([^)]+)\)/
|
|> Regex.scan(body)
|
||||||
|> Regex.scan(body)
|
|> Enum.map(fn [_, _, path] -> path end)
|
||||||
|> Enum.map(fn [_, _, path] -> path end)
|
|
||||||
|
|
||||||
html_links =
|
|
||||||
~r/<a\s+href=["']([^"']*)["']/i
|
|
||||||
|> Regex.scan(body)
|
|
||||||
|> Enum.map(fn [_, path] -> path end)
|
|
||||||
|
|
||||||
(markdown_links ++ html_links)
|
|
||||||
|> Enum.uniq()
|
|
||||||
|> Enum.filter(&internal_blog_link?/1)
|
|> Enum.filter(&internal_blog_link?/1)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp internal_blog_link?(path) do
|
defp internal_blog_link?(path) do
|
||||||
case parse_blog_link(path) do
|
case parse_blog_link(path) do
|
||||||
{_blog_id_str, _slug, _query, _fragment} -> true
|
{_, _} -> true
|
||||||
nil -> false
|
nil -> false
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@ -109,7 +97,7 @@ defmodule Blogex.LinkValidator do
|
|||||||
nil ->
|
nil ->
|
||||||
{:error, "not a blog link: #{link}"}
|
{: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
|
case Map.fetch(@valid_blog_ids, blog_id_str) do
|
||||||
{:ok, _blog_atom} -> validate_slug(slug_part)
|
{:ok, _blog_atom} -> validate_slug(slug_part)
|
||||||
:error -> {:error, "unknown blog ID: #{blog_id_str}"}
|
:error -> {:error, "unknown blog ID: #{blog_id_str}"}
|
||||||
@ -186,53 +174,29 @@ defmodule Blogex.LinkValidator do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
# --- Private helpers ---
|
# --- Private helpers ---
|
||||||
|
|
||||||
@doc false
|
@doc false
|
||||||
@spec parse_blog_link(String.t()) ::
|
@spec parse_blog_link(String.t()) :: {String.t(), String.t()} | nil
|
||||||
{String.t(), String.t(), String.t() | nil, String.t() | nil} | nil
|
|
||||||
def parse_blog_link(path) do
|
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) do
|
||||||
with ["", "blog", blog_id, rest] <- String.split(path, "/", parts: 4),
|
slug = String.slice(rest, 0, slug_end(rest))
|
||||||
{slug, query_fragment} <- String.split_at(rest, slug_slug_end(rest)),
|
{blog_id, slug}
|
||||||
{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
|
|
||||||
else
|
else
|
||||||
_ -> nil
|
_ -> nil
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc false
|
@doc false
|
||||||
@spec slug_slug_end(String.t()) :: integer()
|
@spec slug_end(String.t()) :: integer()
|
||||||
defp slug_slug_end(str) do
|
defp slug_end(str) do
|
||||||
case String.split(str, ["?", "#"], parts: 2) do
|
case String.split(str, ["?", "#"], parts: 2) do
|
||||||
[slug | _] -> String.length(slug)
|
[slug | _] -> String.length(slug)
|
||||||
_ -> String.length(str)
|
_ -> String.length(str)
|
||||||
end
|
end
|
||||||
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
|
@doc false
|
||||||
@spec validate_slug(String.t()) :: :ok | {:error, String.t()}
|
@spec validate_slug(String.t()) :: :ok | {:error, String.t()}
|
||||||
defp validate_slug(slug) when slug == "" do
|
defp validate_slug(slug) when slug == "" do
|
||||||
|
|||||||
@ -5,7 +5,7 @@ defmodule Blogex.LinkValidatorTest do
|
|||||||
describe "extract_links/1" do
|
describe "extract_links/1" do
|
||||||
test "extracts internal blog links from markdown body" do
|
test "extracts internal blog links from markdown body" do
|
||||||
body =
|
body =
|
||||||
"<p>Check out [hello world](/blog/engineering/hello-world) and [release v1](/blog/releases/v1-0-0).</p>"
|
"Check out [hello world](/blog/engineering/hello-world) and [release v1](/blog/releases/v1-0-0)."
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == [
|
assert LinkValidator.extract_links(body) == [
|
||||||
"/blog/engineering/hello-world",
|
"/blog/engineering/hello-world",
|
||||||
@ -14,25 +14,25 @@ defmodule Blogex.LinkValidatorTest do
|
|||||||
end
|
end
|
||||||
|
|
||||||
test "ignores external links" do
|
test "ignores external links" do
|
||||||
body = "<p>See [GitHub](https://github.com) and [internal](/blog/engineering/post).</p>"
|
body = "See [GitHub](https://github.com) and [internal](/blog/engineering/post)."
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == ["/blog/engineering/post"]
|
assert LinkValidator.extract_links(body) == ["/blog/engineering/post"]
|
||||||
end
|
end
|
||||||
|
|
||||||
test "ignores non-blog internal links" do
|
test "ignores non-blog internal links" do
|
||||||
body = "<p>See [/about](/about) and [/blog/engineering/post](/blog/engineering/post).</p>"
|
body = "See [/about](/about) and [/blog/engineering/post](/blog/engineering/post)."
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == ["/blog/engineering/post"]
|
assert LinkValidator.extract_links(body) == ["/blog/engineering/post"]
|
||||||
end
|
end
|
||||||
|
|
||||||
test "returns empty list when no internal blog links" do
|
test "returns empty list when no internal blog links" do
|
||||||
body = "<p>Just external links: [GitHub](https://github.com).</p>"
|
body = "Just external links: [GitHub](https://github.com)."
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == []
|
assert LinkValidator.extract_links(body) == []
|
||||||
end
|
end
|
||||||
|
|
||||||
test "handles multiple links on one line" do
|
test "handles multiple links on one line" do
|
||||||
body = "<p>[a](/blog/engineering/a) [b](/blog/releases/b) [c](/blog/engineering/c)</p>"
|
body = "[a](/blog/engineering/a) [b](/blog/releases/b) [c](/blog/engineering/c)"
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == [
|
assert LinkValidator.extract_links(body) == [
|
||||||
"/blog/engineering/a",
|
"/blog/engineering/a",
|
||||||
@ -42,13 +42,13 @@ defmodule Blogex.LinkValidatorTest do
|
|||||||
end
|
end
|
||||||
|
|
||||||
test "handles links with query strings" do
|
test "handles links with query strings" do
|
||||||
body = "<p>[link](/blog/engineering/post?foo=bar)</p>"
|
body = "[link](/blog/engineering/post?foo=bar)"
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == ["/blog/engineering/post?foo=bar"]
|
assert LinkValidator.extract_links(body) == ["/blog/engineering/post?foo=bar"]
|
||||||
end
|
end
|
||||||
|
|
||||||
test "handles links with anchor fragments" do
|
test "handles links with anchor fragments" do
|
||||||
body = "<p>[link](/blog/engineering/post#section)</p>"
|
body = "[link](/blog/engineering/post#section)"
|
||||||
|
|
||||||
assert LinkValidator.extract_links(body) == ["/blog/engineering/post#section"]
|
assert LinkValidator.extract_links(body) == ["/blog/engineering/post#section"]
|
||||||
end
|
end
|
||||||
@ -190,19 +190,19 @@ defmodule Blogex.LinkValidatorTest do
|
|||||||
|
|
||||||
describe "validate_body/2" do
|
describe "validate_body/2" do
|
||||||
test "returns :ok when body has no internal blog links" do
|
test "returns :ok when body has no internal blog links" do
|
||||||
body = "<p>Just text, no links.</p>"
|
body = "Just text, no links."
|
||||||
|
|
||||||
assert LinkValidator.validate_body(body, :engineering) == :ok
|
assert LinkValidator.validate_body(body, :engineering) == :ok
|
||||||
end
|
end
|
||||||
|
|
||||||
test "returns :ok when all links are valid" do
|
test "returns :ok when all links are valid" do
|
||||||
body = "<p>[link](/blog/engineering/post)</p>"
|
body = "[link](/blog/engineering/post)"
|
||||||
|
|
||||||
assert LinkValidator.validate_body(body, :engineering) == :ok
|
assert LinkValidator.validate_body(body, :engineering) == :ok
|
||||||
end
|
end
|
||||||
|
|
||||||
test "returns errors with post context" do
|
test "returns errors with post context" do
|
||||||
body = "<p>[link](/blog/unknown/post)</p>"
|
body = "[link](/blog/unknown/post)"
|
||||||
|
|
||||||
assert LinkValidator.validate_body(body, :engineering) == {
|
assert LinkValidator.validate_body(body, :engineering) == {
|
||||||
:error,
|
:error,
|
||||||
@ -218,7 +218,7 @@ defmodule Blogex.LinkValidatorTest do
|
|||||||
end
|
end
|
||||||
|
|
||||||
test "includes post_id in error tuples when provided" do
|
test "includes post_id in error tuples when provided" do
|
||||||
body = "<p>[link](/blog/unknown/post)</p>"
|
body = "[link](/blog/unknown/post)"
|
||||||
|
|
||||||
assert LinkValidator.validate_body(body, :engineering, post_id: "test-post") == {
|
assert LinkValidator.validate_body(body, :engineering, post_id: "test-post") == {
|
||||||
:error,
|
:error,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user