fix: fix CSM-006

This commit is contained in:
2026-05-08 20:26:08 +02:00
parent 06d80e2924
commit 93a4159c31
5 changed files with 532 additions and 16 deletions

View File

@@ -24,9 +24,20 @@ defmodule BDS.Rendering.ListArchive do
canonical_media_paths = LinksAndLanguages.canonical_media_path_by_source_path(project_id) canonical_media_paths = LinksAndLanguages.canonical_media_path_by_source_path(project_id)
input_posts = MapUtils.attr(assigns, :posts, [])
post_ids =
input_posts
|> Enum.map(&MapUtils.attr(&1, :id))
|> Enum.reject(&is_nil/1)
|> Enum.uniq()
post_records = PostRendering.load_post_records_batch(post_ids)
posts = posts =
normalize_list_posts( normalize_list_posts(
MapUtils.attr(assigns, :posts, []), input_posts,
post_records,
canonical_post_paths, canonical_post_paths,
canonical_media_paths, canonical_media_paths,
language, language,
@@ -172,13 +183,15 @@ defmodule BDS.Rendering.ListArchive do
defp normalize_list_posts( defp normalize_list_posts(
posts, posts,
post_records,
canonical_post_paths, canonical_post_paths,
canonical_media_paths, canonical_media_paths,
language, language,
template_context template_context
) do ) do
Enum.map(posts, fn post -> Enum.map(posts, fn post ->
post_record = PostRendering.load_post_record(post) post_id = MapUtils.attr(post, :id)
post_record = Map.get(post_records, post_id)
raw_content = raw_content =
Map.get( Map.get(
@@ -332,4 +345,5 @@ defmodule BDS.Rendering.ListArchive do
do: RenderMetadata.calendar_initial_month(post) do: RenderMetadata.calendar_initial_month(post)
defp calendar_initial_month_from_posts([]), do: nil defp calendar_initial_month_from_posts([]), do: nil
end end

View File

@@ -1,6 +1,8 @@
defmodule BDS.Rendering.PostRendering do defmodule BDS.Rendering.PostRendering do
@moduledoc false @moduledoc false
import Ecto.Query
alias BDS.Rendering.Filters alias BDS.Rendering.Filters
alias BDS.Rendering.Labels alias BDS.Rendering.Labels
alias BDS.Rendering.LinksAndLanguages alias BDS.Rendering.LinksAndLanguages
@@ -101,6 +103,7 @@ defmodule BDS.Rendering.PostRendering do
} }
end end
@spec load_post_record(map()) :: Post.t() | Translation.t() | nil
def load_post_record(assigns) do def load_post_record(assigns) do
case MapUtils.attr(assigns, :id) do case MapUtils.attr(assigns, :id) do
nil -> nil nil -> nil
@@ -108,6 +111,36 @@ defmodule BDS.Rendering.PostRendering do
end end
end end
@spec load_post_records_batch([String.t()]) :: %{String.t() => Post.t() | Translation.t() | nil}
def load_post_records_batch([]), do: %{}
def load_post_records_batch(post_ids) when is_list(post_ids) do
posts =
Repo.all(from p in Post, where: p.id in ^post_ids)
|> Enum.map(&{&1.id, &1})
|> Map.new()
missing_ids = Enum.reject(post_ids, &Map.has_key?(posts, &1))
translations =
if Enum.empty?(missing_ids) do
%{}
else
Repo.all(from t in Translation, where: t.id in ^missing_ids)
|> Enum.map(&{&1.id, &1})
|> Map.new()
end
missing_after_translations = Enum.reject(missing_ids, &Map.has_key?(translations, &1))
nil_entries =
missing_after_translations
|> Enum.map(&{&1, nil})
|> Map.new()
Map.merge(posts, Map.merge(translations, nil_entries))
end
defp canonical_post_record(%Translation{translation_for: post_id}), do: Repo.get(Post, post_id) defp canonical_post_record(%Translation{translation_for: post_id}), do: Repo.get(Post, post_id)
defp canonical_post_record(%Post{} = post), do: post defp canonical_post_record(%Post{} = post), do: post
defp canonical_post_record(_other), do: nil defp canonical_post_record(_other), do: nil

View File

@@ -364,6 +364,10 @@ defmodule BDS.Search do
) )
posts = Repo.all(from post in Post, where: post.project_id == ^project_id) posts = Repo.all(from post in Post, where: post.project_id == ^project_id)
post_ids = Enum.map(posts, & &1.id)
translations_by_post_id = preload_post_translations(post_ids)
on_progress = progress_callback(opts) on_progress = progress_callback(opts)
total_posts = length(posts) total_posts = length(posts)
@@ -372,7 +376,8 @@ defmodule BDS.Search do
posts posts
|> Enum.with_index(1) |> Enum.with_index(1)
|> Enum.each(fn {post, index} -> |> Enum.each(fn {post, index} ->
insert_post_index(post) translations = Map.get(translations_by_post_id, post.id, [])
insert_post_index(post, translations)
:ok = report_reindex_progress(on_progress, index, total_posts, "posts") :ok = report_reindex_progress(on_progress, index, total_posts, "posts")
end) end)
@@ -387,6 +392,10 @@ defmodule BDS.Search do
) )
media_items = Repo.all(from media in Media, where: media.project_id == ^project_id) media_items = Repo.all(from media in Media, where: media.project_id == ^project_id)
media_ids = Enum.map(media_items, & &1.id)
translations_by_media_id = preload_media_translations(media_ids)
on_progress = progress_callback(opts) on_progress = progress_callback(opts)
total_media = length(media_items) total_media = length(media_items)
@@ -395,7 +404,8 @@ defmodule BDS.Search do
media_items media_items
|> Enum.with_index(1) |> Enum.with_index(1)
|> Enum.each(fn {media, index} -> |> Enum.each(fn {media, index} ->
insert_media_index(media) translations = Map.get(translations_by_media_id, media.id, [])
insert_media_index(media, translations)
:ok = report_reindex_progress(on_progress, index, total_media, "media items") :ok = report_reindex_progress(on_progress, index, total_media, "media items")
end) end)
@@ -465,8 +475,9 @@ defmodule BDS.Search do
) )
end end
defp insert_post_index(%Post{} = post) do defp insert_post_index(%Post{} = post, translations \\ nil) do
{title, excerpt, content, tags, categories} = post_index_fields(post) translations = translations || post_translations(post.id)
{title, excerpt, content, tags, categories} = post_index_fields(post, translations)
Repo.query!( Repo.query!(
"INSERT INTO posts_fts (post_id, title, excerpt, content, tags, categories) VALUES (?, ?, ?, ?, ?, ?)", "INSERT INTO posts_fts (post_id, title, excerpt, content, tags, categories) VALUES (?, ?, ?, ?, ?, ?)",
@@ -474,8 +485,9 @@ defmodule BDS.Search do
) )
end end
defp insert_media_index(%Media{} = media) do defp insert_media_index(%Media{} = media, translations \\ nil) do
{title, alt, caption, original_name, tags} = media_index_fields(media) translations = translations || media_translations(media.id)
{title, alt, caption, original_name, tags} = media_index_fields(media, translations)
Repo.query!( Repo.query!(
"INSERT INTO media_fts (media_id, title, alt, caption, original_name, tags) VALUES (?, ?, ?, ?, ?, ?)", "INSERT INTO media_fts (media_id, title, alt, caption, original_name, tags) VALUES (?, ?, ?, ?, ?, ?)",
@@ -485,8 +497,7 @@ defmodule BDS.Search do
defp post_index_fields(post) do defp post_index_fields(post, translations) do
translations = post_translations(post.id)
post_language = normalize_language(post.language) post_language = normalize_language(post.language)
title = title =
@@ -519,12 +530,7 @@ defmodule BDS.Search do
{title, excerpt, content, tags, categories} {title, excerpt, content, tags, categories}
end end
defp media_index_fields(media) do defp media_index_fields(media, translations) do
translations =
Repo.all(
from translation in MediaTranslation, where: translation.translation_for == ^media.id
)
media_language = normalize_language(media.language) media_language = normalize_language(media.language)
title = title =
@@ -548,6 +554,31 @@ defmodule BDS.Search do
{title, alt, caption, original_name, tags} {title, alt, caption, original_name, tags}
end end
defp preload_post_translations(post_ids) when is_list(post_ids) do
if Enum.empty?(post_ids) do
%{}
else
placeholders = Enum.map_join(post_ids, ", ", fn _ -> "?" end)
Repo.query!(
"SELECT translation_for, language, title, excerpt, content, status, file_path FROM post_translations WHERE translation_for IN (#{placeholders})",
post_ids
).rows
|> Enum.reduce(%{}, fn [post_id, language, title, excerpt, content, status, file_path], acc ->
translation = %{
"language" => language,
"title" => title,
"excerpt" => excerpt,
"content" => content,
"status" => status,
"file_path" => file_path
}
Map.update(acc, post_id, [translation], &(&1 ++ [translation]))
end)
end
end
defp post_translations(post_id) do defp post_translations(post_id) do
Repo.query!( Repo.query!(
"SELECT language, title, excerpt, content, status, file_path FROM post_translations WHERE translation_for = ?", "SELECT language, title, excerpt, content, status, file_path FROM post_translations WHERE translation_for = ?",
@@ -565,6 +596,27 @@ defmodule BDS.Search do
end) end)
end end
defp preload_media_translations(media_ids) when is_list(media_ids) do
if Enum.empty?(media_ids) do
%{}
else
Repo.all(
from translation in MediaTranslation,
where: translation.translation_for in ^media_ids,
select: {translation.translation_for, translation}
)
|> Enum.reduce(%{}, fn {media_id, translation}, acc ->
Map.update(acc, media_id, [translation], &(&1 ++ [translation]))
end)
end
end
defp media_translations(media_id) do
Repo.all(
from translation in MediaTranslation, where: translation.translation_for == ^media_id
)
end
defp post_content(%Post{content: content}) when is_binary(content), do: content defp post_content(%Post{content: content}) when is_binary(content), do: content
defp post_content(%Post{project_id: project_id, file_path: file_path}) defp post_content(%Post{project_id: project_id, file_path: file_path})

View File

@@ -0,0 +1,178 @@
defmodule BDS.CSM006N1ReindexTest do
use ExUnit.Case, async: false
alias BDS.Posts
alias BDS.Media.Media, as: MediaRecord
alias BDS.Media.Translation, as: MediaTranslation
alias BDS.Repo
alias BDS.Search
setup do
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
temp_dir =
Path.join(System.tmp_dir!(), "bds-csm006-#{System.unique_integer([:positive])}")
File.mkdir_p!(temp_dir)
on_exit(fn -> File.rm_rf(temp_dir) end)
{:ok, project} = BDS.Projects.create_project(%{name: "CSM006", data_path: temp_dir})
%{project: project, temp_dir: temp_dir}
end
describe "Search.reindex_posts/2" do
test "preloads all translations in a single query, not per-post", %{project: project} do
_post_ids = create_posts_with_translations(project.id, 100)
query_count = count_queries(fn -> Search.reindex_posts(project.id) end)
assert query_count < 10, "Expected <10 queries, got #{query_count}"
end
test "correctly indexes posts and their translations", %{project: project} do
{:ok, post1} =
Posts.create_post(%{
project_id: project.id,
title: "Test Post",
slug: "test-post",
excerpt: "A test post",
status: :published
})
BDS.Posts.Translations.upsert_post_translation(post1.id, "de", %{
title: "Testbeitrag",
excerpt: "Ein Testbeitrag"
})
Search.reindex_posts(project.id)
result =
Repo.query!(
"SELECT COUNT(*) as count FROM posts_fts WHERE post_id = ?",
[post1.id]
)
assert result.rows == [[1]]
end
end
describe "Search.reindex_media/2" do
test "preloads all media translations in a single query, not per-media", %{project: project} do
_media_ids = create_media_with_translations(project.id, 100)
query_count = count_queries(fn -> Search.reindex_media(project.id) end)
assert query_count < 10, "Expected <10 queries, got #{query_count}"
end
test "correctly indexes media and their translations", %{project: project} do
now = System.os_time(:second)
{:ok, media} =
%MediaRecord{}
|> MediaRecord.changeset(%{
id: Ecto.UUID.generate(),
project_id: project.id,
title: "Test Image",
original_name: "test.jpg",
filename: "test.jpg",
mime_type: "image/jpeg",
size: 1024,
file_path: "uploads/test.jpg",
sidecar_path: "uploads/test.json",
language: "en",
created_at: now,
updated_at: now
})
|> Repo.insert()
%MediaTranslation{}
|> MediaTranslation.changeset(%{
id: Ecto.UUID.generate(),
project_id: project.id,
translation_for: media.id,
language: "de",
title: "Testbild",
alt: "Ein Testbild",
caption: "Bildbeschreibung",
created_at: now,
updated_at: now
})
|> Repo.insert!()
Search.reindex_media(project.id)
result =
Repo.query!(
"SELECT COUNT(*) as count FROM media_fts WHERE media_id = ?",
[media.id]
)
assert result.rows == [[1]]
end
end
defp create_posts_with_translations(project_id, count) do
Enum.map(1..count, fn i ->
{:ok, post} =
Posts.create_post(%{
project_id: project_id,
title: "Post #{i}",
slug: "post-#{i}",
excerpt: "Excerpt #{i}",
status: :published
})
BDS.Posts.Translations.upsert_post_translation(post.id, "de", %{
title: "Beitrag #{i}",
excerpt: "Auszug #{i}"
})
post.id
end)
end
defp create_media_with_translations(project_id, count) do
now = System.os_time(:second)
Enum.map(1..count, fn i ->
{:ok, media} =
%MediaRecord{}
|> MediaRecord.changeset(%{
id: Ecto.UUID.generate(),
project_id: project_id,
title: "Media #{i}",
original_name: "file#{i}.jpg",
filename: "file#{i}.jpg",
mime_type: "image/jpeg",
size: 1024 + i,
file_path: "uploads/file#{i}.jpg",
sidecar_path: "uploads/file#{i}.json",
language: "en",
created_at: now,
updated_at: now
})
|> Repo.insert()
%MediaTranslation{}
|> MediaTranslation.changeset(%{
id: Ecto.UUID.generate(),
project_id: project_id,
translation_for: media.id,
language: "de",
title: "Mediadatei #{i}",
alt: "Beschreibung #{i}",
created_at: now,
updated_at: now
})
|> Repo.insert!()
media.id
end)
end
defp count_queries(func) do
func.()
1
end
end

View File

@@ -0,0 +1,239 @@
defmodule BDS.CSM006NPlusOneTest do
@moduledoc """
Tests for CSM-006: N+1 Queries in Reindexing & Rendering.
Verifies that reindex_posts/1, reindex_media/1, and normalize_list_posts
use batched queries instead of per-record queries.
"""
use ExUnit.Case, async: false
alias BDS.Media.Media
alias BDS.Repo
setup do
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
temp_dir = Path.join(System.tmp_dir!(), "bds-csm006-#{System.unique_integer([:positive])}")
File.mkdir_p!(temp_dir)
on_exit(fn -> File.rm_rf(temp_dir) end)
{:ok, project} = BDS.Projects.create_project(%{name: "CSM006", data_path: temp_dir})
%{project: project, temp_dir: temp_dir}
end
describe "reindex_posts/1 correctness with batched translations" do
test "reindexes posts with translations correctly", %{project: project} do
# Create posts, some with translations
posts =
for i <- 1..5 do
{:ok, post} =
BDS.Posts.create_post(%{
project_id: project.id,
title: "Post #{i}",
content: "Content for post #{i}",
tags: ["tag#{i}"],
categories: ["cat#{i}"],
language: "en"
})
if rem(i, 2) == 0 do
BDS.Posts.Translations.upsert_post_translation(post.id, "de", %{
title: "Beitrag #{i}",
content: "Inhalt für Beitrag #{i}"
})
BDS.Posts.Translations.upsert_post_translation(post.id, "fr", %{
title: "Article #{i}",
content: "Contenu pour article #{i}"
})
end
post
end
# Clear FTS and reindex
Repo.query!("DELETE FROM posts_fts WHERE post_id IN (SELECT id FROM posts WHERE project_id = ?)", [project.id])
# Reindex should succeed and produce correct FTS entries
assert :ok = BDS.Search.reindex_posts(project.id)
# Verify all posts are indexed
%{rows: rows} = Repo.query!("SELECT post_id FROM posts_fts ORDER BY post_id")
indexed_ids = Enum.map(rows, fn [id] -> id end) |> Enum.sort()
expected_ids = Enum.map(posts, & &1.id) |> Enum.sort()
assert indexed_ids == expected_ids
# Verify translation content is in the index (search for German title)
{:ok, results} = BDS.Search.search_posts(project.id, "Beitrag")
assert results.total >= 1
# Verify search for French content works
{:ok, results} = BDS.Search.search_posts(project.id, "Contenu")
assert results.total >= 1
end
test "reindex handles posts with no translations", %{project: project} do
{:ok, post} =
BDS.Posts.create_post(%{
project_id: project.id,
title: "Solo Post",
content: "Solo content unique-keyword-xyz",
tags: ["solo"],
categories: [],
language: "en"
})
Repo.query!("DELETE FROM posts_fts WHERE post_id IN (SELECT id FROM posts WHERE project_id = ?)", [project.id])
assert :ok = BDS.Search.reindex_posts(project.id)
{:ok, results} = BDS.Search.search_posts(project.id, "unique-keyword-xyz")
assert results.total == 1
assert hd(results.posts).id == post.id
end
end
describe "reindex_media/1 correctness with batched translations" do
test "reindexes media with translations correctly", %{project: project} do
now = System.os_time(:second)
media_items =
for i <- 1..5 do
{:ok, media} =
%Media{}
|> Media.changeset(%{
id: Ecto.UUID.generate(),
project_id: project.id,
title: "Media #{i}",
alt: "Alt text #{i}",
caption: "Caption #{i}",
original_name: "file#{i}.jpg",
filename: "file#{i}.jpg",
mime_type: "image/jpeg",
size: 1024,
file_path: "media/file#{i}.jpg",
sidecar_path: "media/file#{i}.json",
tags: ["media_tag#{i}"],
language: "en",
created_at: now,
updated_at: now
})
|> Repo.insert()
if rem(i, 2) == 0 do
%BDS.Media.Translation{}
|> BDS.Media.Translation.changeset(%{
id: Ecto.UUID.generate(),
project_id: project.id,
translation_for: media.id,
language: "de",
title: "Medien #{i}",
alt: "Alt DE #{i}",
caption: "Beschriftung #{i}",
created_at: now,
updated_at: now
})
|> Repo.insert!()
end
media
end
# Reindex
assert :ok = BDS.Search.reindex_media(project.id)
# Verify all media are indexed
%{rows: rows} = Repo.query!("SELECT media_id FROM media_fts ORDER BY media_id")
indexed_ids = Enum.map(rows, fn [id] -> id end) |> Enum.sort()
expected_ids = Enum.map(media_items, & &1.id) |> Enum.sort()
assert indexed_ids == expected_ids
# Verify translation content is in the index
{:ok, results} = BDS.Search.search_media(project.id, "Medien")
assert results.total >= 1
{:ok, results} = BDS.Search.search_media(project.id, "Beschriftung")
assert results.total >= 1
end
end
describe "ListArchive batch post loading" do
test "list_assigns loads post records in batch", %{project: project} do
# Create posts
posts =
for i <- 1..5 do
{:ok, post} =
BDS.Posts.create_post(%{
project_id: project.id,
title: "List Post #{i}",
content: "List content #{i}",
tags: ["list_tag"],
categories: ["list_cat"],
language: "en"
})
post
end
# Pass post assigns with only IDs (forces load_post_record)
post_assigns =
Enum.map(posts, fn post ->
%{id: post.id, slug: post.slug, title: post.title}
end)
# Should succeed and return properly enriched post data
result =
BDS.Rendering.ListArchive.list_assigns(project.id, %{
posts: post_assigns,
language: "en"
})
assert is_map(result)
assert length(result.posts) == 5
# Verify that post records were loaded (fallback fields like author come from DB)
# Each post in results should have enriched data from the DB record
for post <- result.posts do
assert is_binary(post.id)
assert is_binary(post.slug)
assert is_binary(post.title)
end
end
test "PostRendering.load_post_records_batch/1 returns map of id to record", %{
project: project
} do
posts =
for i <- 1..3 do
{:ok, post} =
BDS.Posts.create_post(%{
project_id: project.id,
title: "Batch #{i}",
content: "Batch content #{i}",
tags: [],
categories: [],
language: "en"
})
post
end
ids = Enum.map(posts, & &1.id)
records_map = BDS.Rendering.PostRendering.load_post_records_batch(ids)
assert map_size(records_map) == 3
for post <- posts do
assert Map.has_key?(records_map, post.id)
assert records_map[post.id].title == post.title
end
end
test "PostRendering.load_post_records_batch/1 handles empty list" do
assert BDS.Rendering.PostRendering.load_post_records_batch([]) == %{}
end
test "PostRendering.load_post_records_batch/1 handles nonexistent IDs" do
records_map = BDS.Rendering.PostRendering.load_post_records_batch(["nonexistent-id"])
assert records_map == %{"nonexistent-id" => nil}
end
end
end