@@ -98,48 +117,50 @@ export default function App() {
return (
-
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
- } />
-
-
-
- )}
- />
-
-
-
- )}
- />
- } />
-
-
-
- )}
- />
-
+ }>
+
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+ } />
+
+
+
+ )}
+ />
+
+
+
+ )}
+ />
+ } />
+
+
+
+ )}
+ />
+
+
)
}
diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts
index f9ad34b..005c1c7 100644
--- a/frontend/vite.config.ts
+++ b/frontend/vite.config.ts
@@ -18,6 +18,33 @@ export default defineConfig({
url: path.resolve(__dirname, "node_modules/url/url.js"),
},
},
+ build: {
+ rollupOptions: {
+ output: {
+ // Vendor code changes only when a dependency is upgraded, while app
+ // code changes every deploy. Splitting them means a routine deploy
+ // invalidates the app chunk and leaves React and the UI kit in the
+ // browser cache, which is what the immutable caching in nginx.conf is
+ // there to exploit.
+ manualChunks: {
+ react: ["react", "react-dom", "react-router-dom"],
+ // Radix and the two icon sets: large, stable, and pulled in by nearly
+ // every route, so they belong in neither the app chunk nor a route's.
+ ui: [
+ "@radix-ui/react-avatar",
+ "@radix-ui/react-checkbox",
+ "@radix-ui/react-dropdown-menu",
+ "@radix-ui/react-label",
+ "@radix-ui/react-popover",
+ "@radix-ui/react-separator",
+ "@radix-ui/react-slot",
+ "@phosphor-icons/react",
+ "lucide-react",
+ ],
+ },
+ },
+ },
+ },
test: {
environment: "jsdom",
globals: true,
diff --git a/server/internal/database/article_categories.go b/server/internal/database/article_categories.go
new file mode 100644
index 0000000..ab37d1f
--- /dev/null
+++ b/server/internal/database/article_categories.go
@@ -0,0 +1,235 @@
+package database
+
+import (
+ "context"
+ "database/sql"
+ "encoding/json"
+ "strings"
+)
+
+// article_categories is a derived index of `articles`.`categories`: one row per
+// (article, category title), so "which articles are in this section" is an index
+// lookup instead of a scan.
+//
+// It replaces a `categories LIKE '%"news"%'` predicate. That predicate could
+// never use an index -- a leading wildcard on a LONGTEXT column rules it out --
+// so every section page, every homepage block and every taxonomy count read the
+// whole articles table, twice per request once the paging COUNT(*) is included.
+//
+// The table is strictly derived, never authoritative. `articles`.`categories`
+// remains the source of truth, this is rebuilt from it at every startup, and
+// article writes keep it in step in between. That combination is deliberate:
+// the corpus is periodically reloaded wholesale by the WordPress ETL, which
+// knows nothing about this table, and a derived index that silently survives a
+// reseed would serve empty section pages with no indication why.
+
+// maxCategoryLength matches the VARCHAR in schema/article_categories.sql. A
+// longer title is skipped rather than truncated, because a truncated key would
+// match a section it does not belong to.
+const maxCategoryLength = 191
+
+// A note for the next person who looks at the listing's paging COUNT(*), which
+// evaluates the artifact filter (see handlers.articleQueryFilters) over every
+// published row and is the largest remaining cost on /v1/articles.
+//
+// Two rewrites were measured against this corpus and both rejected:
+//
+// - Two EXISTS over articles_authors and this table. Correct on today's data
+// -- zero disagreements across all 9,371 development and 10,113 production
+// rows -- and worth 13.1ms -> 9.4ms on the COUNT, but it also costs 0.1ms
+// on the listing query, which materializes both subqueries to return 21
+// rows. Under 4ms net, in exchange for moving the definition of "filed"
+// onto indexes that match the columns by data rather than by construction.
+// - A PERSISTENT generated column with an index, which would make the COUNT
+// index-only. Adding one rewrites `articles`, and rewriting `articles`
+// re-tokenizes the bodies into the two FULLTEXT indexes: 8m53s for a single
+// ALTER here, during which the CMS's own startup migration sat in "Waiting
+// for table metadata lock". In production that is the newsroom's writes
+// queued behind a container that is not yet serving traffic.
+//
+// The cheap wins are already taken (see EnsureArticlesPublishedIndex and
+// EnsureArticleAuthorsIndex). What is left needs either a schema the ETL owns
+// or an approximate count, and neither is worth four milliseconds.
+
+func EnsureArticleCategoriesTable(ctx context.Context, conn *sql.DB) error {
+ _, err := conn.ExecContext(ctx, TableSchema("article_categories"))
+ return err
+}
+
+// normalizeCategoryValues turns the stored `categories` JSON into the keys this
+// table is indexed on.
+//
+// Only a well-formed JSON array counts. parseStringListField falls back to
+// splitting on commas for rows that are not JSON, but the LIKE predicate this
+// replaces anchored on JSON quotes and so never matched those rows either;
+// indexing them here would quietly add articles to sections that have never
+// shown them.
+func normalizeCategoryValues(raw string) []string {
+ trimmed := strings.TrimSpace(raw)
+ if trimmed == "" {
+ return nil
+ }
+ var parsed []string
+ if err := json.Unmarshal([]byte(trimmed), &parsed); err != nil {
+ return nil
+ }
+ return normalizeCategoryList(parsed)
+}
+
+// normalizeCategoryList lowercases, trims and dedupes category titles. The
+// lowercasing mirrors the LOWER() the old predicate applied to the column, and
+// is what lets CategoryMatchValues compare against these rows directly.
+func normalizeCategoryList(categories []string) []string {
+ out := make([]string, 0, len(categories))
+ for _, category := range categories {
+ value := strings.ToLower(strings.TrimSpace(category))
+ if value == "" || len(value) > maxCategoryLength {
+ continue
+ }
+ duplicate := false
+ for _, existing := range out {
+ if existing == value {
+ duplicate = true
+ break
+ }
+ }
+ if !duplicate {
+ out = append(out, value)
+ }
+ }
+ return out
+}
+
+// execer is satisfied by both *sql.DB and *sql.Tx, so the write-path helpers can
+// join a caller's transaction.
+type execer interface {
+ ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error)
+}
+
+// ReplaceArticleCategories rewrites one article's rows. Delete-then-insert
+// rather than a diff: an article carries a handful of categories, and the
+// simpler operation cannot leave a stale row behind.
+func ReplaceArticleCategories(ctx context.Context, conn execer, articleID int64, categories []string) error {
+ if _, err := conn.ExecContext(ctx, "DELETE FROM `article_categories` WHERE `article_id` = ?", articleID); err != nil {
+ return err
+ }
+ values := normalizeCategoryList(categories)
+ if len(values) == 0 {
+ return nil
+ }
+
+ placeholders := make([]string, 0, len(values))
+ args := make([]any, 0, len(values)*2)
+ for _, value := range values {
+ placeholders = append(placeholders, "(?, ?)")
+ args = append(args, articleID, value)
+ }
+ _, err := conn.ExecContext(ctx,
+ "INSERT INTO `article_categories` (`article_id`, `category`) VALUES "+strings.Join(placeholders, ", "),
+ args...,
+ )
+ return err
+}
+
+// ReplaceArticleCategoriesBySlug is ReplaceArticleCategories for the update
+// paths, which address an article by slug. A slug that matches nothing is not an
+// error here: the caller's own UPDATE already reported that as a 404, and
+// failing again would turn one missing article into a 500.
+func ReplaceArticleCategoriesBySlug(ctx context.Context, conn *sql.DB, slug string, categories []string) error {
+ var articleID int64
+ switch err := conn.QueryRowContext(ctx,
+ "SELECT `id` FROM `articles` WHERE `slug` = ? LIMIT 1", strings.TrimSpace(slug),
+ ).Scan(&articleID); err {
+ case nil:
+ case sql.ErrNoRows:
+ return nil
+ default:
+ return err
+ }
+ return ReplaceArticleCategories(ctx, conn, articleID, categories)
+}
+
+// RebuildArticleCategories rebuilds the whole table from `articles`.
+//
+// Run at every startup. It is cheap -- the corpus is under ten thousand rows and
+// two or three categories each -- and running it unconditionally is what makes
+// the table safe to depend on: after a WordPress ETL reseed, which replaces
+// `articles` wholesale and renumbers its ids, the previous contents are not
+// merely stale but point at the wrong articles.
+func RebuildArticleCategories(ctx context.Context, conn *sql.DB) error {
+ rows, err := conn.QueryContext(ctx, "SELECT `id`, `categories` FROM `articles`")
+ if err != nil {
+ return err
+ }
+ defer rows.Close()
+
+ type articleCategories struct {
+ id int64
+ categories []string
+ }
+ var pending []articleCategories
+ for rows.Next() {
+ var id int64
+ var raw sql.NullString
+ if err := rows.Scan(&id, &raw); err != nil {
+ return err
+ }
+ values := normalizeCategoryValues(raw.String)
+ if len(values) == 0 {
+ continue
+ }
+ pending = append(pending, articleCategories{id: id, categories: values})
+ }
+ if err := rows.Err(); err != nil {
+ return err
+ }
+ // The scan is finished before the writes start: holding a result set open
+ // across them would need a second connection from the pool, and the pool is
+ // the resource this whole change is trying to spend less of.
+
+ tx, err := conn.BeginTx(ctx, nil)
+ if err != nil {
+ return err
+ }
+ defer func() { _ = tx.Rollback() }()
+
+ // TRUNCATE rather than DELETE: this is a full replacement, and it leaves no
+ // undo log to write for rows that are about to be re-inserted anyway.
+ if _, err := tx.ExecContext(ctx, "TRUNCATE TABLE `article_categories`"); err != nil {
+ return err
+ }
+
+ // Batched, because one INSERT per article is ten thousand round trips on
+ // every boot.
+ const batchSize = 500
+ placeholders := make([]string, 0, batchSize)
+ args := make([]any, 0, batchSize*2)
+ flush := func() error {
+ if len(placeholders) == 0 {
+ return nil
+ }
+ _, err := tx.ExecContext(ctx,
+ "INSERT INTO `article_categories` (`article_id`, `category`) VALUES "+strings.Join(placeholders, ", "),
+ args...,
+ )
+ placeholders = placeholders[:0]
+ args = args[:0]
+ return err
+ }
+
+ for _, article := range pending {
+ for _, category := range article.categories {
+ placeholders = append(placeholders, "(?, ?)")
+ args = append(args, article.id, category)
+ if len(placeholders) >= batchSize {
+ if err := flush(); err != nil {
+ return err
+ }
+ }
+ }
+ }
+ if err := flush(); err != nil {
+ return err
+ }
+ return tx.Commit()
+}
diff --git a/server/internal/database/article_categories_test.go b/server/internal/database/article_categories_test.go
new file mode 100644
index 0000000..ac5faf6
--- /dev/null
+++ b/server/internal/database/article_categories_test.go
@@ -0,0 +1,71 @@
+package database
+
+import (
+ "reflect"
+ "strings"
+ "testing"
+)
+
+func TestNormalizeCategoryValues(t *testing.T) {
+ cases := map[string]struct {
+ raw string
+ want []string
+ }{
+ "json array": {`["News","Campus"]`, []string{"news", "campus"}},
+ // The escape encoding/json used to emit for "&". Decoding resolves it,
+ // which is what makes the SQL-side REPLACE unnecessary.
+ "escaped ampersand": {`["Comics & Puzzles"]`, []string{"comics & puzzles"}},
+ "whitespace": {`[" News "]`, []string{"news"}},
+ "duplicates": {`["News","news"]`, []string{"news"}},
+ "empty members": {`["News",""]`, []string{"news"}},
+ "empty array": {`[]`, nil},
+ "blank": {" ", nil},
+ // A row that is not a JSON array never matched the LIKE predicate this
+ // replaced, so indexing it would add articles to sections that have
+ // never shown them.
+ "not json": {`News,Campus`, nil},
+ }
+
+ for name, tc := range cases {
+ t.Run(name, func(t *testing.T) {
+ got := normalizeCategoryValues(tc.raw)
+ if len(got) == 0 && len(tc.want) == 0 {
+ return
+ }
+ if !reflect.DeepEqual(got, tc.want) {
+ t.Errorf("normalizeCategoryValues(%q) = %v, want %v", tc.raw, got, tc.want)
+ }
+ })
+ }
+}
+
+// A title longer than the column is skipped rather than truncated: a truncated
+// key would compare equal to a section it does not belong to.
+func TestNormalizeCategoryListSkipsOverlongTitles(t *testing.T) {
+ long := strings.Repeat("a", maxCategoryLength+1)
+ got := normalizeCategoryList([]string{"News", long})
+ if !reflect.DeepEqual(got, []string{"news"}) {
+ t.Errorf("got %v, want just [news]", got)
+ }
+}
+
+// The index stores what CategoryMatchValues asks for. If the two normalizations
+// ever diverge, every section page silently empties.
+func TestIndexedCategoriesMatchTheValuesQueriedFor(t *testing.T) {
+ withCategoryAliases(t, map[string][]string{"entertainment": {"Arts & Entertainment"}})
+
+ indexed := normalizeCategoryValues(`["Arts & Entertainment"]`)
+ wanted := CategoryMatchValues("entertainment")
+
+ found := false
+ for _, have := range indexed {
+ for _, want := range wanted {
+ if have == want {
+ found = true
+ }
+ }
+ }
+ if !found {
+ t.Errorf("indexed %v matches none of the queried values %v", indexed, wanted)
+ }
+}
diff --git a/server/internal/database/article_search.go b/server/internal/database/article_search.go
index 3c60869..11312f5 100644
--- a/server/internal/database/article_search.go
+++ b/server/internal/database/article_search.go
@@ -18,7 +18,11 @@ import (
// not soft-deleted. Archived rows were previously reachable here.
const searchLiveArticlesClause = "`pub_date` IS NOT NULL AND `pub_date` <= UTC_TIMESTAMP() AND `archived_at` IS NULL"
-const searchSelectColumns = "SELECT `id`, `title`, `slug`, `description`, `text`, `excerpt`, `tags`, `categories`, `pub_date`, `mod_date`, `priority`, `breaking_news`, `comment_status`, `photo_url`, `focus_keyword`, `meta_description`, `seo_title`, `creation_date`, `scheduled_pub_date`, `canonical_url`, `noindex`, `photo_alt` FROM `articles` "
+// searchSelectColumns omits `text`, like every other listing read: search
+// results are rendered as excerpts. The body is still matched and ranked on --
+// it just appears in WHERE and ORDER BY rather than in the SELECT list, which
+// costs nothing to read.
+var searchSelectColumns = "SELECT " + articleSelectList(false, "") + " FROM `articles` "
// fulltextMinTokenSize mirrors innodb_ft_min_token_size. InnoDB never indexes
// tokens shorter than this, and a required term (`+ab`) that the index cannot
diff --git a/server/internal/database/http_models.go b/server/internal/database/http_models.go
index b728185..e396c54 100644
--- a/server/internal/database/http_models.go
+++ b/server/internal/database/http_models.go
@@ -36,19 +36,135 @@ var ArticleSortByColumn = map[string]string{
var AuthorColumns = []string{"id", "display_name", "first_name", "last_name", "email", "login", "archived_at"}
-// ArticleColumns, articleSelectColumnsQualified, searchSelectColumns
-// (article_search.go) and the inline SELECT in handlers.ListArticles are all
-// consumed by the one positional ScanArticle below. Adding a column means adding
-// it to all four -- in the same place -- and to ScanArticle.
-var ArticleColumns = []string{
- "id", "title", "slug", "description", "text", "excerpt", "tags", "categories",
- "pub_date", "mod_date", "priority", "breaking_news",
- "comment_status", "photo_url",
- "focus_keyword", "meta_description", "seo_title", "creation_date", "scheduled_pub_date",
- "canonical_url", "noindex", "photo_alt",
+// articleRow is the landing zone for a scanned article: every column at its
+// nullable database type, before toArticle turns it into the model.
+//
+// It exists so that the column list and the scan targets come from one place.
+// They used to be four hand-maintained lists that a positional Scan had to
+// agree with, which meant adding a column in the wrong position silently
+// shifted every value after it into the wrong field.
+type articleRow struct {
+ id int64
+ title string
+ slug sql.NullString
+ description sql.NullString
+ text sql.NullString
+ excerpt sql.NullString
+ tags sql.NullString
+ categories sql.NullString
+ pubDate sql.NullTime
+ modDate sql.NullTime
+ priority sql.NullBool
+ breakingNews sql.NullBool
+ commentStatus sql.NullString
+ photoURL sql.NullString
+ focusKeyword sql.NullString
+ metaDescription sql.NullString
+ seoTitle sql.NullString
+ creationDate sql.NullTime
+ scheduledDate sql.NullTime
+ canonicalURL sql.NullString
+ noIndex sql.NullBool
+ photoAlt sql.NullString
+}
+
+// articleColumn pairs a column name with the field it scans into, so a query's
+// SELECT list and its Scan targets are generated from the same ordered slice and
+// cannot drift out of step.
+type articleColumn struct {
+ name string
+ target func(*articleRow) any
+}
+
+// articleColumnSet is the full set, in the order every article query selects
+// them. Adding a column here is the only edit needed: the lists below and the
+// scanners all derive from it.
+var articleColumnSet = []articleColumn{
+ {"id", func(r *articleRow) any { return &r.id }},
+ {"title", func(r *articleRow) any { return &r.title }},
+ {"slug", func(r *articleRow) any { return &r.slug }},
+ {"description", func(r *articleRow) any { return &r.description }},
+ {"text", func(r *articleRow) any { return &r.text }},
+ {"excerpt", func(r *articleRow) any { return &r.excerpt }},
+ {"tags", func(r *articleRow) any { return &r.tags }},
+ {"categories", func(r *articleRow) any { return &r.categories }},
+ {"pub_date", func(r *articleRow) any { return &r.pubDate }},
+ {"mod_date", func(r *articleRow) any { return &r.modDate }},
+ {"priority", func(r *articleRow) any { return &r.priority }},
+ {"breaking_news", func(r *articleRow) any { return &r.breakingNews }},
+ {"comment_status", func(r *articleRow) any { return &r.commentStatus }},
+ {"photo_url", func(r *articleRow) any { return &r.photoURL }},
+ {"focus_keyword", func(r *articleRow) any { return &r.focusKeyword }},
+ {"meta_description", func(r *articleRow) any { return &r.metaDescription }},
+ {"seo_title", func(r *articleRow) any { return &r.seoTitle }},
+ {"creation_date", func(r *articleRow) any { return &r.creationDate }},
+ {"scheduled_pub_date", func(r *articleRow) any { return &r.scheduledDate }},
+ {"canonical_url", func(r *articleRow) any { return &r.canonicalURL }},
+ {"noindex", func(r *articleRow) any { return &r.noIndex }},
+ {"photo_alt", func(r *articleRow) any { return &r.photoAlt }},
+}
+
+// summaryOmittedColumns are the columns a listing does not read.
+//
+// `text` is the whole article body, averaging ~5KB across the migrated corpus.
+// A twenty-item listing was fetching around 95KB of it per request and using
+// none: list responses carry an excerpt, which comes from `excerpt` (falling
+// back to `description`), and is derived from the body only at write time. On
+// the homepage, which builds six section blocks, the same waste was paid six
+// times over.
+var summaryOmittedColumns = map[string]bool{"text": true}
+
+func articleColumnNames(full bool) []string {
+ names := make([]string, 0, len(articleColumnSet))
+ for _, col := range articleColumnSet {
+ if !full && summaryOmittedColumns[col.name] {
+ continue
+ }
+ names = append(names, col.name)
+ }
+ return names
+}
+
+// articleSelectList renders the SELECT list. qualifier prefixes each column for
+// queries that join (""=unqualified, "a"=`a`.`col`).
+func articleSelectList(full bool, qualifier string) string {
+ prefix := ""
+ if qualifier != "" {
+ prefix = qualifier + "."
+ }
+ var b strings.Builder
+ for i, name := range articleColumnNames(full) {
+ if i > 0 {
+ b.WriteString(", ")
+ }
+ b.WriteString(prefix + "`" + name + "`")
+ }
+ return b.String()
+}
+
+func (r *articleRow) scanTargets(full bool) []any {
+ targets := make([]any, 0, len(articleColumnSet))
+ for _, col := range articleColumnSet {
+ if !full && summaryOmittedColumns[col.name] {
+ continue
+ }
+ targets = append(targets, col.target(r))
+ }
+ return targets
}
-const articleSelectColumnsQualified = "a.`id`, a.`title`, a.`slug`, a.`description`, a.`text`, a.`excerpt`, a.`tags`, a.`categories`, a.`pub_date`, a.`mod_date`, a.`priority`, a.`breaking_news`, a.`comment_status`, a.`photo_url`, a.`focus_keyword`, a.`meta_description`, a.`seo_title`, a.`creation_date`, a.`scheduled_pub_date`, a.`canonical_url`, a.`noindex`, a.`photo_alt`"
+// ArticleColumns is the full column list, for the article detail endpoint --
+// the one read that actually renders the body.
+var ArticleColumns = articleColumnNames(true)
+
+// ArticleSummaryColumns is the listing column list. Everything that produces
+// models.ArticleListItem uses it.
+var ArticleSummaryColumns = articleColumnNames(false)
+
+// ArticleSummarySelectList is ArticleSummaryColumns rendered as a quoted SELECT
+// list, for the handlers that assemble their query as a string rather than
+// through Select.
+var ArticleSummarySelectList = articleSelectList(false, "")
// Image/photo URLs are canonicalized upstream in the WordPress ETL (see
// wordpress-etl Utils/MediaURL) so `photo_url` and inline body images are stored
@@ -133,104 +249,98 @@ func ScanAuthorOverview(rows *sql.Rows) (models.AuthorOverview, error) {
return a, nil
}
+// ScanArticle reads a row selected with ArticleColumns -- the full set,
+// including the article body.
func ScanArticle(rows *sql.Rows) (models.Article, error) {
- var (
- a models.Article
- slug sql.NullString
- description sql.NullString
- text sql.NullString
- excerpt sql.NullString
- tags sql.NullString
- categories sql.NullString
- pubDate sql.NullTime
- priority sql.NullBool
- breakingNews sql.NullBool
- commentStatus sql.NullString
- photoURL sql.NullString
- modDate sql.NullTime
- focusKeyword sql.NullString
- metaDescription sql.NullString
- seoTitle sql.NullString
- creationDate sql.NullTime
- scheduledDate sql.NullTime
- canonicalURL sql.NullString
- noIndex sql.NullBool
- photoAlt sql.NullString
- )
- err := rows.Scan(
- &a.ID, &a.Title, &slug, &description, &text, &excerpt, &tags, &categories,
- &pubDate, &modDate, &priority, &breakingNews,
- &commentStatus, &photoURL,
- &focusKeyword, &metaDescription, &seoTitle, &creationDate, &scheduledDate,
- &canonicalURL, &noIndex, &photoAlt,
- )
- if err != nil {
+ return scanArticleRow(rows, true)
+}
+
+// ScanArticleSummary reads a row selected with ArticleSummaryColumns. The
+// returned Article has an empty Content, which is correct for every caller that
+// renders a listing: none of them read the body.
+func ScanArticleSummary(rows *sql.Rows) (models.Article, error) {
+ return scanArticleRow(rows, false)
+}
+
+func scanArticleRow(rows *sql.Rows, full bool) (models.Article, error) {
+ var row articleRow
+ if err := rows.Scan(row.scanTargets(full)...); err != nil {
return models.Article{}, err
}
- if modDate.Valid {
- t := modDate.Time
+ return row.toArticle(), nil
+}
+
+// toArticle maps a scanned row onto the model. A column the query did not
+// select is simply invalid here and leaves its field at the zero value, which is
+// what makes a summary scan a strict subset of a full one rather than a
+// different shape.
+func (row articleRow) toArticle() models.Article {
+ a := models.Article{ID: row.id, Title: row.title}
+
+ if row.modDate.Valid {
+ t := row.modDate.Time
a.ModifiedAt = &t
}
- if canonicalURL.Valid {
- a.CanonicalURL = strings.TrimSpace(canonicalURL.String)
+ if row.canonicalURL.Valid {
+ a.CanonicalURL = strings.TrimSpace(row.canonicalURL.String)
}
- if noIndex.Valid {
- a.NoIndex = noIndex.Bool
+ if row.noIndex.Valid {
+ a.NoIndex = row.noIndex.Bool
}
- if focusKeyword.Valid {
- a.FocusKeyword = focusKeyword.String
+ if row.focusKeyword.Valid {
+ a.FocusKeyword = row.focusKeyword.String
}
- if metaDescription.Valid {
- a.MetaDescription = metaDescription.String
+ if row.metaDescription.Valid {
+ a.MetaDescription = row.metaDescription.String
}
- if seoTitle.Valid {
- a.SEOTitle = seoTitle.String
+ if row.seoTitle.Valid {
+ a.SEOTitle = row.seoTitle.String
}
- if text.Valid {
- a.Content = text.String
+ if row.text.Valid {
+ a.Content = row.text.String
}
- if excerpt.Valid {
- a.Excerpt = excerpt.String
- } else if description.Valid {
- a.Excerpt = description.String
+ if row.excerpt.Valid {
+ a.Excerpt = row.excerpt.String
+ } else if row.description.Valid {
+ a.Excerpt = row.description.String
}
- if slug.Valid {
- a.Slug = slug.String
+ if row.slug.Valid {
+ a.Slug = row.slug.String
}
- a.Tags = parseStringListField(tags)
- a.Categories = parseStringListField(categories)
- if creationDate.Valid {
- t := creationDate.Time
+ a.Tags = parseStringListField(row.tags)
+ a.Categories = parseStringListField(row.categories)
+ if row.creationDate.Valid {
+ t := row.creationDate.Time
a.CreatedAt = &t
}
- if pubDate.Valid {
- t := pubDate.Time
+ if row.pubDate.Valid {
+ t := row.pubDate.Time
a.PublishedAt = &t
a.Status = models.ArticleStatusPublished
- } else if scheduledDate.Valid {
- t := scheduledDate.Time
+ } else if row.scheduledDate.Valid {
+ t := row.scheduledDate.Time
a.PublishedAt = &t
a.ScheduledAt = &t
a.Status = models.ArticleStatusScheduled
} else {
a.Status = models.ArticleStatusDraft
}
- if priority.Valid {
- a.IsFeatured = priority.Bool
+ if row.priority.Valid {
+ a.IsFeatured = row.priority.Bool
}
- if breakingNews.Valid {
- a.BreakingNews = breakingNews.Bool
+ if row.breakingNews.Valid {
+ a.BreakingNews = row.breakingNews.Bool
}
- if commentStatus.Valid {
- a.CommentStatus = normalizeCommentStatus(commentStatus.String)
+ if row.commentStatus.Valid {
+ a.CommentStatus = normalizeCommentStatus(row.commentStatus.String)
}
- if photoURL.Valid {
- a.PhotoURL = photoURL.String
+ if row.photoURL.Valid {
+ a.PhotoURL = row.photoURL.String
}
- if photoAlt.Valid {
- a.PhotoAlt = photoAlt.String
+ if row.photoAlt.Valid {
+ a.PhotoAlt = row.photoAlt.String
}
- return a, nil
+ return a
}
// NormalizeCanonicalURL trims a canonical-URL override and reports whether it is
@@ -268,10 +378,16 @@ func parseStringListField(value sql.NullString) []string {
return parsed
}
+// CollectArticles drains rows selected with ArticleSummaryColumns.
+//
+// It is the listing path, and there is deliberately no full-column counterpart:
+// every caller that collects more than one article is building a list of
+// excerpts, and the single read that needs the body -- the article detail
+// endpoint -- scans its one row with ScanArticle directly.
func CollectArticles(rows *sql.Rows) ([]models.Article, error) {
var articles []models.Article
for rows.Next() {
- a, err := ScanArticle(rows)
+ a, err := ScanArticleSummary(rows)
if err != nil {
return nil, err
}
@@ -379,7 +495,7 @@ func GetRelatedArticlesBySlug(ctx context.Context, conn *sql.DB, slug string, k
return nil, fmt.Errorf("k must be greater than 0")
}
- query := "SELECT " + articleSelectColumnsQualified + " " +
+ query := "SELECT " + articleSelectList(false, "a") + " " +
"FROM articles AS src " +
"JOIN article_embeddings AS src_vec ON src_vec.article_id = src.id " +
"JOIN article_embeddings AS cand_vec ON cand_vec.article_id <> src.id " +
diff --git a/server/internal/database/http_models_test.go b/server/internal/database/http_models_test.go
index 6b7ff0f..89a009c 100644
--- a/server/internal/database/http_models_test.go
+++ b/server/internal/database/http_models_test.go
@@ -255,3 +255,60 @@ func TestDeriveExcerpt_KeepsEditorialBrackets(t *testing.T) {
t.Errorf("deriveExcerpt = %q, want %q", got, want)
}
}
+
+// The listing column set must be the full set minus exactly the body. If a new
+// column is added to articleColumnSet and quietly lands in summaryOmittedColumns
+// too, listings stop returning it and the public site renders a blank field.
+func TestArticleSummaryColumns_OmitOnlyTheBody(t *testing.T) {
+ full := map[string]bool{}
+ for _, name := range ArticleColumns {
+ full[name] = true
+ }
+ summary := map[string]bool{}
+ for _, name := range ArticleSummaryColumns {
+ summary[name] = true
+ }
+
+ for name := range full {
+ if name == "text" {
+ continue
+ }
+ if !summary[name] {
+ t.Errorf("column %q is missing from ArticleSummaryColumns", name)
+ }
+ }
+ if summary["text"] {
+ t.Error("ArticleSummaryColumns still selects `text`; listings do not read the body")
+ }
+ if len(ArticleSummaryColumns) != len(ArticleColumns)-1 {
+ t.Errorf("ArticleSummaryColumns has %d columns, want %d", len(ArticleSummaryColumns), len(ArticleColumns)-1)
+ }
+}
+
+// scanTargets is what makes the column list and the Scan positional agreement
+// one fact rather than two. If they can disagree, every value after the
+// mismatch lands in the wrong field.
+func TestArticleScanTargets_MatchColumnCounts(t *testing.T) {
+ var row articleRow
+ if got, want := len(row.scanTargets(true)), len(ArticleColumns); got != want {
+ t.Errorf("full scan targets = %d, want %d", got, want)
+ }
+ if got, want := len(row.scanTargets(false)), len(ArticleSummaryColumns); got != want {
+ t.Errorf("summary scan targets = %d, want %d", got, want)
+ }
+}
+
+func TestArticleSelectList_QualifiesForJoins(t *testing.T) {
+ if got := articleSelectList(false, "a"); !strings.HasPrefix(got, "a.`id`, a.`title`") {
+ t.Errorf("qualified select list = %q, want it to start with a.`id`, a.`title`", got)
+ }
+ if got := articleSelectList(false, ""); !strings.HasPrefix(got, "`id`, `title`") {
+ t.Errorf("unqualified select list = %q, want it to start with `id`, `title`", got)
+ }
+ if strings.Contains(articleSelectList(false, "a"), "`text`") {
+ t.Error("summary select list names `text`")
+ }
+ if !strings.Contains(articleSelectList(true, "a"), "`text`") {
+ t.Error("full select list is missing `text`")
+ }
+}
diff --git a/server/internal/database/schema/article_categories.sql b/server/internal/database/schema/article_categories.sql
new file mode 100644
index 0000000..dd9e2e2
--- /dev/null
+++ b/server/internal/database/schema/article_categories.sql
@@ -0,0 +1,11 @@
+CREATE TABLE IF NOT EXISTS article_categories (
+ article_id BIGINT NOT NULL,
+ -- The category title as it appears in `articles`.`categories`, lowercased and
+ -- trimmed. 191 is the longest utf8mb4 VARCHAR that fits a legacy 767-byte
+ -- index prefix; the longest category in the corpus is under 30.
+ category VARCHAR(191) NOT NULL,
+ PRIMARY KEY (article_id, category),
+ -- The section-page lookup: given a handful of category titles, find the
+ -- articles. article_id rides along so the index covers the whole subquery.
+ KEY idx_article_categories_category (category, article_id)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4
diff --git a/server/internal/database/taxonomy.go b/server/internal/database/taxonomy.go
index f61e33d..04483e2 100644
--- a/server/internal/database/taxonomy.go
+++ b/server/internal/database/taxonomy.go
@@ -290,19 +290,9 @@ func categoryAliasesFor(slug string) []string {
return categoryAliasBySlug[slug]
}
-// CategoryColumnExpr is the SQL expression every category match runs against.
-//
-// `articles`.`categories` is a JSON array of category titles, but it is written
-// by two producers that escape it differently: the ETL emits a plain "&", while
-// Go's encoding/json HTML-escapes it to a backslash-u escape (see FormatTags),
-// so an article edited in the CMS stops matching the very section it is filed
-// under. Folding that escape away here means callers only ever reason about one
-// spelling. FormatTags no longer produces it, but rows written before that fix
-// still carry it.
-const CategoryColumnExpr = "REPLACE(LOWER(`categories`), '\\\\u0026', '&')"
-
-// CategoryMatchPatterns returns the CategoryColumnExpr LIKE patterns that
-// identify articles filed under a taxonomy slug.
+// CategoryMatchValues returns the category titles that identify articles filed
+// under a taxonomy slug, normalized the same way article_categories stores
+// them: lowercased and trimmed.
//
// WordPress category text does not match our slugs literally, so a slug stands
// in for several spellings: "comics-puzzles" has to find "Comics & Puzzles".
@@ -310,35 +300,40 @@ const CategoryColumnExpr = "REPLACE(LOWER(`categories`), '\\\\u0026', '&')"
// article listing and the count rebuild call it, because when they disagreed a
// section could list 2545 articles while reporting 8.
//
-// Every pattern is anchored on the JSON quotes around a member, so a slug
-// matches a WHOLE category and never a fragment of a longer one. Unanchored
-// patterns silently merged sibling taxonomies: `%puzzles%` matched the parent
+// Every value is a WHOLE category title, compared for equality, so a slug never
+// matches a fragment of a longer one. This used to be a LIKE pattern anchored on
+// the JSON quotes around an array member, for the same reason: unanchored
+// patterns silently merged sibling taxonomies -- `%puzzles%` matched the parent
// title "Comics & Puzzles" and so pulled all 219 comics into the Puzzles
// subsection, and `%men's basketball%` is a substring of "Women's Basketball",
// which folded the women's team into the men's. Both read as plausible-but-wrong
-// pages rather than as errors, so anchoring is load-bearing, not cosmetic.
+// pages rather than as errors, so exactness is load-bearing, not cosmetic. An
+// equality join gives it for free, and can use an index besides.
//
// Because the match is exact, a slug that is not the canonicalized category
// string resolves to nothing on its own -- it needs an alias row. Those come
// from the cache, so callers must have run RefreshCategoryAliases first;
// EnsureTaxonomyTable does it at startup.
-func CategoryMatchPatterns(slug string) []string {
+//
+// Ampersand escaping used to be folded in SQL here too: the ETL emits a plain
+// "&" while Go's encoding/json HTML-escapes it to & (see FormatTags), so an
+// article edited in the CMS stopped matching its own section. That fold is now
+// implicit -- article_categories is built by JSON-decoding the column, which
+// resolves the escape to the character it stands for.
+func CategoryMatchValues(slug string) []string {
normalized := strings.ToLower(strings.TrimSpace(slug))
if normalized == "" {
return nil
}
- patterns := make([]string, 0, 4)
+ values := make([]string, 0, 4)
add := func(value string) {
- // The JSON quotes are what make the match exact; without them a
- // pattern matches any category containing the phrase.
- pattern := `%"` + value + `"%`
- for _, existing := range patterns {
- if existing == pattern {
+ for _, existing := range values {
+ if existing == value {
return
}
}
- patterns = append(patterns, pattern)
+ values = append(values, value)
}
add(normalized)
@@ -353,7 +348,7 @@ func CategoryMatchPatterns(slug string) []string {
for _, alias := range categoryAliasesFor(normalized) {
add(strings.ToLower(strings.TrimSpace(alias)))
}
- return patterns
+ return values
}
// possessiveStems are the words a slug can only have lost an apostrophe from.
@@ -438,19 +433,46 @@ func taxonomyMatchSlugs(ctx context.Context, conn *sql.DB) (map[string][]string,
// TaxonomyCountCondition builds the WHERE fragment matching articles in any of
// the given slugs, along with its arguments.
+//
+// It reads the article_categories index rather than the `categories` column.
+// The predicate it replaced was a chain of `LOWER(categories) LIKE '%"news"%'`
+// -- a leading wildcard, so no index could ever serve it, and every section page
+// scanned the whole table to answer it (twice, since the listing pages with a
+// COUNT(*) alongside).
+//
+// EXISTS rather than IN: callers negate this fragment (see
+// ReportOrphanedArticles), and NOT IN against a subquery that can yield NULL
+// evaluates to UNKNOWN and quietly returns nothing. NOT EXISTS has no such
+// edge.
+//
+// The correlation name is the bare `articles` table, which every caller selects
+// from unaliased.
func TaxonomyCountCondition(slugs []string) (string, []any) {
- var clauses []string
- var args []any
+ var values []string
+ seen := make(map[string]bool)
for _, slug := range slugs {
- for _, pattern := range CategoryMatchPatterns(slug) {
- clauses = append(clauses, CategoryColumnExpr+" LIKE ?")
- args = append(args, pattern)
+ for _, value := range CategoryMatchValues(slug) {
+ if seen[value] {
+ continue
+ }
+ seen[value] = true
+ values = append(values, value)
}
}
- if len(clauses) == 0 {
+ if len(values) == 0 {
return "", nil
}
- return "(" + strings.Join(clauses, " OR ") + ")", args
+
+ placeholders := make([]string, len(values))
+ args := make([]any, len(values))
+ for i, value := range values {
+ placeholders[i] = "?"
+ args[i] = value
+ }
+
+ condition := "EXISTS (SELECT 1 FROM `article_categories` `ac` WHERE `ac`.`article_id` = `articles`.`id` " +
+ "AND `ac`.`category` IN (" + strings.Join(placeholders, ", ") + "))"
+ return condition, args
}
// countArticlesForSlugs counts the articles a taxonomy row matches, over the
diff --git a/server/internal/database/taxonomy_integration_test.go b/server/internal/database/taxonomy_integration_test.go
index 9561961..6acbb2f 100644
--- a/server/internal/database/taxonomy_integration_test.go
+++ b/server/internal/database/taxonomy_integration_test.go
@@ -91,11 +91,11 @@ func TestTaxonomyAliasSeedAndCacheRoundTrip(t *testing.T) {
// defaultCategoryAliases rather than a literal list: the map grows every
// time another orphaned category is filed, and a test that pins the
// contents fails on the filing rather than on anything being broken.
- patterns := CategoryMatchPatterns("entertainment")
+ values := CategoryMatchValues("entertainment")
for _, alias := range defaultCategoryAliases["entertainment"] {
- want := `%"` + strings.ToLower(alias) + `"%`
- if !containsPattern(patterns, want) {
- t.Errorf("entertainment patterns %v missing the seeded alias %s", patterns, want)
+ want := strings.ToLower(alias)
+ if !containsValue(values, want) {
+ t.Errorf("entertainment values %v missing the seeded alias %s", values, want)
}
}
// The aliases must not be HTML-escaped in the column, or one holding an
@@ -118,8 +118,8 @@ func TestTaxonomyAliasSeedAndCacheRoundTrip(t *testing.T) {
}
// A slug with no default keeps no aliases.
- if got := CategoryMatchPatterns(unseededSlug); len(got) != 1 {
- t.Errorf("%s patterns = %v, want just its own", unseededSlug, got)
+ if got := CategoryMatchValues(unseededSlug); len(got) != 1 {
+ t.Errorf("%s values = %v, want just its own", unseededSlug, got)
}
}
@@ -169,8 +169,8 @@ func TestTaxonomyAliasRefreshPicksUpWrites(t *testing.T) {
if err := RefreshCategoryAliases(ctx, conn); err != nil {
t.Fatalf("refresh: %v", err)
}
- if got := CategoryMatchPatterns("movies"); len(got) != 1 {
- t.Fatalf("movies patterns = %v, want just its own", got)
+ if got := CategoryMatchValues("movies"); len(got) != 1 {
+ t.Fatalf("movies values = %v, want just its own", got)
}
if _, err := conn.ExecContext(ctx,
@@ -180,20 +180,20 @@ func TestTaxonomyAliasRefreshPicksUpWrites(t *testing.T) {
}
// Still stale until the cache is told, which is why every write path calls
// RefreshCategoryAliases.
- if got := CategoryMatchPatterns("movies"); len(got) != 1 {
- t.Errorf("patterns changed before a refresh: %v", got)
+ if got := CategoryMatchValues("movies"); len(got) != 1 {
+ t.Errorf("values changed before a refresh: %v", got)
}
if err := RefreshCategoryAliases(ctx, conn); err != nil {
t.Fatalf("refresh after write: %v", err)
}
- if got := CategoryMatchPatterns("movies"); !containsPattern(got, `%"movies i've seen"%`) {
- t.Errorf("movies patterns %v missing the written alias", got)
+ if got := CategoryMatchValues("movies"); !containsValue(got, "movies i've seen") {
+ t.Errorf("movies values %v missing the written alias", got)
}
}
-func containsPattern(patterns []string, want string) bool {
- for _, pattern := range patterns {
- if pattern == want {
+func containsValue(values []string, want string) bool {
+ for _, value := range values {
+ if value == want {
return true
}
}
diff --git a/server/internal/database/taxonomy_test.go b/server/internal/database/taxonomy_test.go
index 3765cc8..ba8e970 100644
--- a/server/internal/database/taxonomy_test.go
+++ b/server/internal/database/taxonomy_test.go
@@ -7,49 +7,49 @@ import (
func assertPatterns(t *testing.T, slug string, want []string) {
t.Helper()
- got := CategoryMatchPatterns(slug)
+ got := CategoryMatchValues(slug)
if len(got) != len(want) {
- t.Fatalf("%s: got %d patterns %v, want %d %v", slug, len(got), got, len(want), want)
+ t.Fatalf("%s: got %d values %v, want %d %v", slug, len(got), got, len(want), want)
}
for i := range want {
if got[i] != want[i] {
- t.Errorf("%s: pattern %d = %q, want %q", slug, i, got[i], want[i])
+ t.Errorf("%s: value %d = %q, want %q", slug, i, got[i], want[i])
}
}
}
-func TestCategoryMatchPatternsExpandsDashedSlugs(t *testing.T) {
+func TestCategoryMatchValuesExpandsDashedSlugs(t *testing.T) {
// A dashed slug has to find the WordPress spelling, which uses spaces and
// often an ampersand: "comics-puzzles" must match "Comics & Puzzles".
assertPatterns(t, "comics-puzzles", []string{
- `%"comics-puzzles"%`, `%"comics puzzles"%`, `%"comics & puzzles"%`,
+ "comics-puzzles", "comics puzzles", "comics & puzzles",
})
}
-func TestCategoryMatchPatternsRestoresPossessiveApostrophe(t *testing.T) {
+func TestCategoryMatchValuesRestoresPossessiveApostrophe(t *testing.T) {
// The category text is "Men's Basketball"; no slug can carry the
// apostrophe, so without this pattern the subsection matched nothing.
assertPatterns(t, "mens-basketball", []string{
- `%"mens-basketball"%`, `%"mens basketball"%`, `%"mens & basketball"%`, `%"men's basketball"%`,
+ "mens-basketball", "mens basketball", "mens & basketball", "men's basketball",
})
}
-func TestCategoryMatchPatternsLeavesPluralsAlone(t *testing.T) {
+func TestCategoryMatchValuesLeavesPluralsAlone(t *testing.T) {
// "philly-sports" is a plural, not a possessive: guessing "philly sport's"
// would add a pattern that matches nothing.
- for _, pattern := range CategoryMatchPatterns("philly-sports") {
- if strings.Contains(pattern, "'") {
- t.Fatalf("unexpected possessive pattern %q", pattern)
+ for _, value := range CategoryMatchValues("philly-sports") {
+ if strings.Contains(value, "'") {
+ t.Fatalf("unexpected possessive value %q", value)
}
}
}
-func TestCategoryMatchPatternsSingleWordSlug(t *testing.T) {
- assertPatterns(t, "comics", []string{`%"comics"%`})
+func TestCategoryMatchValuesSingleWordSlug(t *testing.T) {
+ assertPatterns(t, "comics", []string{"comics"})
}
-func TestCategoryMatchPatternsEmpty(t *testing.T) {
- if got := CategoryMatchPatterns(" "); got != nil {
+func TestCategoryMatchValuesEmpty(t *testing.T) {
+ if got := CategoryMatchValues(" "); got != nil {
t.Fatalf("got %v, want nil for a blank slug", got)
}
}
@@ -75,14 +75,17 @@ func withCategoryAliases(t *testing.T, aliases map[string][]string) {
// editor or a copy-paste, which would make the tests below silently vacuous.
var escapedAmp = `\` + "u0026"
-// matchesCategories reports whether any pattern for slug would match a
-// `categories` JSON array, mirroring what CategoryColumnExpr + LIKE does in
-// SQL: lowercase the column, unescape the ampersand, then substring-match.
+// matchesCategories reports whether slug matches a `categories` JSON array. It
+// composes the two halves of the real lookup -- the rows RebuildArticleCategories
+// would index for the article, and the values TaxonomyCountCondition would ask
+// for -- so these assertions describe what the database will actually answer.
func matchesCategories(slug, categoriesJSON string) bool {
- column := strings.ReplaceAll(strings.ToLower(categoriesJSON), escapedAmp, "&")
- for _, pattern := range CategoryMatchPatterns(slug) {
- if strings.Contains(column, strings.Trim(pattern, "%")) {
- return true
+ indexed := normalizeCategoryValues(categoriesJSON)
+ for _, want := range CategoryMatchValues(slug) {
+ for _, have := range indexed {
+ if have == want {
+ return true
+ }
}
}
return false
@@ -165,24 +168,22 @@ func TestCategoryMatchToleratesEscapedAmpersand(t *testing.T) {
}
}
-func TestTaxonomyCountConditionORsEverySlug(t *testing.T) {
+func TestTaxonomyCountConditionMatchesEverySlug(t *testing.T) {
// A section matches its own slug OR any child's, so a container section
// with no category of its own still resolves to its subsections' articles.
+ // Every spelling lands in one IN list, which is that OR.
condition, args := TaxonomyCountCondition([]string{"special-editions", "welcome-week"})
if condition == "" {
t.Fatal("expected a condition")
}
- if strings.Contains(condition, " AND ") {
- t.Errorf("condition must OR its slugs, not AND them: %s", condition)
- }
- // 3 patterns per dashed slug, two slugs.
- if got := strings.Count(condition, "LIKE ?"); got != 6 {
+ // 3 spellings per dashed slug, two slugs.
+ if got := strings.Count(condition, "?"); got != 6 {
t.Errorf("got %d placeholders, want 6: %s", got, condition)
}
if len(args) != 6 {
t.Errorf("got %d args, want 6: %v", len(args), args)
}
- for _, want := range []string{`%"welcome week"%`, `%"special editions"%`} {
+ for _, want := range []string{"welcome week", "special editions"} {
found := false
for _, arg := range args {
if arg == want {
@@ -190,16 +191,31 @@ func TestTaxonomyCountConditionORsEverySlug(t *testing.T) {
}
}
if !found {
- t.Errorf("missing pattern %q in %v", want, args)
+ t.Errorf("missing category %q in %v", want, args)
}
}
}
-func TestTaxonomyCountConditionMatchesOnTheNormalizedColumn(t *testing.T) {
- // The escape-folding has to be in the SQL, not just in the patterns.
+// The predicate has to be correlated on the outer articles row, and it has to
+// be EXISTS: callers negate it, and NOT IN over a subquery that can produce
+// NULL evaluates to UNKNOWN and silently matches nothing.
+func TestTaxonomyCountConditionIsCorrelatedExists(t *testing.T) {
condition, _ := TaxonomyCountCondition([]string{"comics"})
- if !strings.Contains(condition, CategoryColumnExpr) {
- t.Errorf("condition must match on %s, got %s", CategoryColumnExpr, condition)
+ if !strings.HasPrefix(condition, "EXISTS (") {
+ t.Errorf("condition must be an EXISTS, got %s", condition)
+ }
+ if !strings.Contains(condition, "`ac`.`article_id` = `articles`.`id`") {
+ t.Errorf("condition must correlate on the outer articles row, got %s", condition)
+ }
+}
+
+// Slugs resolving to the same spelling must not each contribute a placeholder;
+// a section and its identically-named subsection would otherwise double the
+// argument list on every section page.
+func TestTaxonomyCountConditionDeduplicatesValues(t *testing.T) {
+ _, args := TaxonomyCountCondition([]string{"comics", "comics"})
+ if len(args) != 1 {
+ t.Errorf("got %d args, want 1: %v", len(args), args)
}
}
@@ -281,11 +297,11 @@ func TestCategoryAliasesAreCaseInsensitiveOnTheSlug(t *testing.T) {
}
}
-func TestCategoryMatchPatternsDeduplicatesAliases(t *testing.T) {
+func TestCategoryMatchValuesDeduplicatesAliases(t *testing.T) {
// An alias that merely restates a derived pattern must not double the
// placeholders in every query that uses the slug.
withCategoryAliases(t, map[string][]string{"comics": {"Comics"}})
- assertPatterns(t, "comics", []string{`%"comics"%`})
+ assertPatterns(t, "comics", []string{"comics"})
}
func TestDefaultCategoryAliasesCoverTheKnownMismatches(t *testing.T) {
diff --git a/server/internal/database/users.go b/server/internal/database/users.go
index c4645f4..8550378 100644
--- a/server/internal/database/users.go
+++ b/server/internal/database/users.go
@@ -45,6 +45,56 @@ func EnsureArticlesPublishedIndex(ctx context.Context, conn *sql.DB) error {
return err
}
+// EnsureArticlesSlugIndex indexes the column every article is addressed by:
+// the detail endpoint, the comment thread, the permalink check, the
+// featured-article write. Without it each of those is a full scan of the
+// migrated corpus -- ten thousand rows read to return one.
+//
+// It is a PREFIX index, and that detail is the whole design. The ETL ships
+// `slug` as LONGTEXT, which cannot be indexed whole; the obvious fix is to
+// narrow the column to VARCHAR first. Do not. Retyping a column rewrites the
+// table, and rewriting `articles` means re-tokenizing 44MB of article bodies
+// into the two FULLTEXT indexes -- measured at over six minutes on a corpus
+// this size, holding a lock the newsroom's writes would queue behind, and on a
+// blue/green deploy the container doing it is not even the one serving. Adding
+// a secondary index is an in-place operation that does none of that.
+//
+// 191 characters is a prefix no slug in the corpus reaches (the longest is 154)
+// and stays under the 767-byte limit older row formats impose, so it is exact
+// in practice and safe on any table layout. Even where it were not, a prefix
+// index only narrows the candidates -- InnoDB rechecks the full value -- so
+// this can cost an extra row read but can never return a wrong article.
+//
+// The index is deliberately not UNIQUE. Uniqueness is a property of the data,
+// and the corpus belongs to the ETL, not the CMS: production already carries
+// one duplicated slug from an archived pair, which a UNIQUE index would turn
+// into a CMS that will not start.
+func EnsureArticlesSlugIndex(ctx context.Context, conn *sql.DB) error {
+ _, err := conn.ExecContext(ctx, `
+ ALTER TABLE articles
+ ADD INDEX IF NOT EXISTS idx_articles_slug (`+"`slug`"+`(191))
+ `)
+ return err
+}
+
+// EnsureArticleAuthorsIndex indexes the join column that every listing reads.
+//
+// LoadAuthorsByArticleIDs resolves a page of articles' bylines with one
+// `WHERE articles_id IN (...)`, but the ETL creates `articles_authors` with
+// only a primary key on its own id, so that lookup scanned the entire join
+// table on every listing request -- including each of the homepage's six
+// section blocks. author_id rides along so the index covers the join.
+//
+// Cheap and safe to run at boot, unlike anything touching `articles`: the table
+// is a few hundred kilobytes and carries no FULLTEXT index to rebuild.
+func EnsureArticleAuthorsIndex(ctx context.Context, conn *sql.DB) error {
+ _, err := conn.ExecContext(ctx, `
+ ALTER TABLE articles_authors
+ ADD INDEX IF NOT EXISTS idx_articles_authors_article (`+"`articles_id`, `author_id`"+`)
+ `)
+ return err
+}
+
// EnsureArticlesSearchIndex adds the FULLTEXT indexes public search ranks on.
//
// Two indexes, not one: the combined index answers "does this article match at
diff --git a/server/internal/handlers/article_default_order_integration_test.go b/server/internal/handlers/article_default_order_integration_test.go
index f83dd83..a204ec7 100644
--- a/server/internal/handlers/article_default_order_integration_test.go
+++ b/server/internal/handlers/article_default_order_integration_test.go
@@ -33,9 +33,13 @@ func seedDefaultOrderArticles(t *testing.T, conn *sql.DB) {
{10020, "an-archive-story", "2011-04-08 15:04:51"},
}
for _, row := range rows {
+ // `categories` is a JSON array, which is what both producers write --
+ // the ETL json.dumps its list, and the CMS goes through FormatTags. The
+ // bare string this fixture used before matched no shape in the corpus
+ // and quietly exempted itself from the category index.
if _, err := conn.ExecContext(ctx,
"INSERT INTO articles (id, title, slug, `text`, authors, categories, pub_date, creation_date) VALUES (?, ?, ?, ?, ?, ?, ?, UTC_TIMESTAMP())",
- row.id, row.slug, row.slug, "Body", `["Rui Zhao"]`, "News", row.pub,
+ row.id, row.slug, row.slug, "Body", `["Rui Zhao"]`, `["News"]`, row.pub,
); err != nil {
t.Fatalf("seed %s: %v", row.slug, err)
}
diff --git a/server/internal/handlers/article_patch_integration_test.go b/server/internal/handlers/article_patch_integration_test.go
index 2adbbcc..1f5282a 100644
--- a/server/internal/handlers/article_patch_integration_test.go
+++ b/server/internal/handlers/article_patch_integration_test.go
@@ -107,6 +107,14 @@ func articlePatchTestDB(t *testing.T) *sql.DB {
if err := db.EnsureTaxonomyTable(ctx, conn); err != nil {
t.Fatalf("ensure taxonomy table: %v", err)
}
+ // The write handlers keep this index in step with `articles`.`categories`,
+ // so every save touches it.
+ if _, err := conn.ExecContext(ctx, "DROP TABLE IF EXISTS article_categories"); err != nil {
+ t.Fatalf("drop article_categories: %v", err)
+ }
+ if err := db.EnsureArticleCategoriesTable(ctx, conn); err != nil {
+ t.Fatalf("ensure article_categories: %v", err)
+ }
return conn
}
diff --git a/server/internal/handlers/featured_article_integration_test.go b/server/internal/handlers/featured_article_integration_test.go
index f9342a4..d73ae40 100644
--- a/server/internal/handlers/featured_article_integration_test.go
+++ b/server/internal/handlers/featured_article_integration_test.go
@@ -147,6 +147,10 @@ func TestFeaturedArticleHTTP_HomepageLeadsWithTheFeaturedArticle(t *testing.T) {
); err != nil {
t.Fatalf("seed articles: %v", err)
}
+ // The homepage blocks are section-filtered, which reads the derived
+ // category index; these rows were inserted straight into `articles`, so
+ // nothing has indexed them yet.
+ indexArticleCategories(t, conn)
homepageNewsSlugs := func() []string {
rec := httptest.NewRecorder()
diff --git a/server/internal/handlers/handlers.go b/server/internal/handlers/handlers.go
index 9b91a04..26f8915 100644
--- a/server/internal/handlers/handlers.go
+++ b/server/internal/handlers/handlers.go
@@ -1227,7 +1227,7 @@ type ArticleParams struct {
func queryArticles(r *http.Request, conn *sql.DB, params ArticleParams, limit, offset int) (*sql.Rows, error) {
q := r.URL.Query()
conditions, args := articleQueryFilters(r, params)
- query := "SELECT `id`, `title`, `slug`, `description`, `text`, `excerpt`, `tags`, `categories`, `pub_date`, `mod_date`, `priority`, `breaking_news`, `comment_status`, `photo_url`, `focus_keyword`, `meta_description`, `seo_title`, `creation_date`, `scheduled_pub_date`, `canonical_url`, `noindex`, `photo_alt` FROM `articles`"
+ query := "SELECT " + db.ArticleSummarySelectList + " FROM `articles`"
if len(conditions) > 0 {
query += " WHERE " + strings.Join(conditions, " AND ")
}
@@ -1316,6 +1316,15 @@ func articleQueryFilters(r *http.Request, params ArticleParams) ([]string, []any
// A brand-new CMS draft looks exactly like one of those until an author or a
// category is attached, so editors keep unpublished rows regardless — without
// the exemption a draft is invisible in the CMS listing until it is filed.
+ //
+ // Kept as a predicate over the columns rather than over the derived
+ // article_categories / articles_authors indexes. Rewriting it as two EXISTS
+ // was measured: it saves 3.8ms on the paging COUNT and costs 0.1ms on the
+ // listing itself, and in exchange the definition of "filed" would move from
+ // the columns to two indexes that agree with them by data rather than by
+ // construction. Four milliseconds does not buy that -- the failure mode is
+ // articles silently vanishing from every listing while still resolving by
+ // direct link.
artifactFilter := "((TRIM(COALESCE(`authors`, '')) <> '' AND TRIM(`authors`) <> '[]') OR (TRIM(COALESCE(`categories`, '')) <> '' AND TRIM(`categories`) <> '[]'))"
if isEditor {
artifactFilter = "(" + artifactFilter + " OR `pub_date` IS NULL)"
@@ -2136,6 +2145,10 @@ func PostArticles(conn *sql.DB) http.HandlerFunc {
writeError(w, http.StatusInternalServerError, err.Error())
return
}
+ if err := db.ReplaceArticleCategories(r.Context(), conn, articleID, body.Categories); err != nil {
+ writeError(w, http.StatusInternalServerError, err.Error())
+ return
+ }
if err := incrementTaxonomyArticleCounts(r.Context(), conn, body.Categories); err != nil {
writeError(w, http.StatusInternalServerError, err.Error())
return
@@ -2228,6 +2241,15 @@ func PutArticle(conn *sql.DB) http.HandlerFunc {
writeError(w, http.StatusInternalServerError, err.Error())
return
}
+ // Keyed on the new slug, because the UPDATE above may have renamed it.
+ // Unconditional, unlike the taxonomy counts: the index describes where
+ // the article is filed, which is true of archived rows too -- they are
+ // excluded by the listing's own archived_at predicate, not by being
+ // missing from here.
+ if err := db.ReplaceArticleCategoriesBySlug(r.Context(), conn, body.Slug, body.Categories); err != nil {
+ writeError(w, http.StatusInternalServerError, err.Error())
+ return
+ }
if isActiveArticle {
if err := reconcileTaxonomyArticleCounts(r.Context(), conn, oldCategories, body.Categories); err != nil {
writeError(w, http.StatusInternalServerError, err.Error())
@@ -2495,6 +2517,13 @@ func PatchArticle(conn *sql.DB) http.HandlerFunc {
return
}
}
+ // nextCategories is oldCategories unless this patch carried a categories
+ // field, so an unrelated patch rewrites the index to what it already
+ // held rather than clearing it.
+ if err := db.ReplaceArticleCategoriesBySlug(r.Context(), conn, targetSlug, nextCategories); err != nil {
+ writeError(w, http.StatusInternalServerError, err.Error())
+ return
+ }
if isActiveArticle {
if err := reconcileTaxonomyArticleCounts(r.Context(), conn, oldCategories, nextCategories); err != nil {
writeError(w, http.StatusInternalServerError, err.Error())
diff --git a/server/internal/handlers/handlers_test.go b/server/internal/handlers/handlers_test.go
index 77a98c2..cb190db 100644
--- a/server/internal/handlers/handlers_test.go
+++ b/server/internal/handlers/handlers_test.go
@@ -118,13 +118,13 @@ func TestAppendCategorySlugCondition(t *testing.T) {
t.Fatalf("expected 3 args, got %d", len(args))
}
- // Patterns are anchored on the JSON quotes so a slug matches a whole
- // category, never a fragment of a longer one -- see
- // db.CategoryMatchPatterns.
+ // Whole category titles, compared for equality against the
+ // article_categories index -- so a slug matches a whole category and never
+ // a fragment of a longer one. See db.CategoryMatchValues.
wantArgs := []string{
- `%"comics-puzzles"%`,
- `%"comics puzzles"%`,
- `%"comics & puzzles"%`,
+ "comics-puzzles",
+ "comics puzzles",
+ "comics & puzzles",
}
for i, want := range wantArgs {
got, ok := args[i].(string)
diff --git a/server/internal/handlers/taxonomy_integration_test.go b/server/internal/handlers/taxonomy_integration_test.go
index 5856cff..834cb6b 100644
--- a/server/internal/handlers/taxonomy_integration_test.go
+++ b/server/internal/handlers/taxonomy_integration_test.go
@@ -72,6 +72,16 @@ func taxonomyHTTPTestDB(t *testing.T) *sql.DB {
t.Fatalf("create articles: %v", err)
}
t.Cleanup(func() { _, _ = conn.ExecContext(context.Background(), "DROP TABLE IF EXISTS articles") })
+
+ // Section matching reads this index rather than the categories column, so a
+ // harness without it answers every count with an error.
+ if _, err := conn.ExecContext(ctx, "DROP TABLE IF EXISTS article_categories"); err != nil {
+ t.Fatalf("drop article_categories: %v", err)
+ }
+ if err := db.EnsureArticleCategoriesTable(ctx, conn); err != nil {
+ t.Fatalf("ensure article_categories: %v", err)
+ }
+ t.Cleanup(func() { _, _ = conn.ExecContext(context.Background(), "DROP TABLE IF EXISTS article_categories") })
return conn
}
@@ -146,8 +156,8 @@ func TestTaxonomyHTTPAliasFixesAnEmptySection(t *testing.T) {
}
// Nothing matches "Food" yet, because the articles say "Restaurant Reviews".
- if got := db.CategoryMatchPatterns("food"); len(got) != 1 {
- t.Fatalf("patterns = %v, want only the slug's own", got)
+ if got := db.CategoryMatchValues("food"); len(got) != 1 {
+ t.Fatalf("values = %v, want only the slug's own", got)
}
// The editor saves the real category name. This is the exact payload the
@@ -164,16 +174,16 @@ func TestTaxonomyHTTPAliasFixesAnEmptySection(t *testing.T) {
// The matcher must see it immediately -- no restart. This is what the
// cache refresh on write buys.
- patterns := db.CategoryMatchPatterns("food")
- for _, want := range []string{`%"restaurant reviews"%`, `%"beer reviews"%`} {
+ values := db.CategoryMatchValues("food")
+ for _, want := range []string{"restaurant reviews", "beer reviews"} {
found := false
- for _, pattern := range patterns {
- if pattern == want {
+ for _, value := range values {
+ if value == want {
found = true
}
}
if !found {
- t.Errorf("patterns %v missing %q right after the save", patterns, want)
+ t.Errorf("values %v missing %q right after the save", values, want)
}
}
@@ -243,8 +253,8 @@ func TestTaxonomyHTTPExplicitEmptyClearsAliases(t *testing.T) {
if len(item.CategoryAliases) != 0 {
t.Errorf("aliases = %v, want cleared", item.CategoryAliases)
}
- if got := db.CategoryMatchPatterns("food"); len(got) != 1 {
- t.Errorf("patterns = %v, want the alias gone from matching too", got)
+ if got := db.CategoryMatchValues("food"); len(got) != 1 {
+ t.Errorf("values = %v, want the alias gone from matching too", got)
}
}
@@ -309,6 +319,7 @@ func TestTaxonomyHTTPDeleteRefusesWhenArticlesExist(t *testing.T) {
); err != nil {
t.Fatalf("insert article: %v", err)
}
+ indexArticleCategories(t, conn)
var storedCount int64
if err := conn.QueryRowContext(ctx,
@@ -372,6 +383,7 @@ func TestTaxonomyHTTPAliasSaveUpdatesTheDisplayedCount(t *testing.T) {
); err != nil {
t.Fatalf("insert articles: %v", err)
}
+ indexArticleCategories(t, conn)
if code := taxonomyRequest(t, PostTaxonomy(conn), http.MethodPost, "/v1/taxonomy", map[string]any{
"type": "section",
@@ -416,6 +428,7 @@ func TestTaxonomyHTTPSubsectionSaveRecountsItsParent(t *testing.T) {
); err != nil {
t.Fatalf("insert articles: %v", err)
}
+ indexArticleCategories(t, conn)
for _, body := range []map[string]any{
{"type": "section", "slug": "food", "canonical_title": "Food"},
@@ -468,6 +481,7 @@ func TestTaxonomyHTTPDeleteRecountsTheParent(t *testing.T) {
); err != nil {
t.Fatalf("insert article: %v", err)
}
+ indexArticleCategories(t, conn)
if _, err := conn.ExecContext(ctx,
"UPDATE site_taxonomy SET article_count = 99 WHERE slug = 'food'",
); err != nil {
@@ -482,3 +496,13 @@ func TestTaxonomyHTTPDeleteRecountsTheParent(t *testing.T) {
t.Errorf("parent count after deleting a child = %d, want 1 -- the parent was not recounted", got)
}
}
+
+// indexArticleCategories rebuilds the derived category index, standing in for
+// the startup rebuild. These tests seed `articles` with raw INSERTs rather than
+// through the write handlers, so nothing else would populate it.
+func indexArticleCategories(t *testing.T, conn *sql.DB) {
+ t.Helper()
+ if err := db.RebuildArticleCategories(context.Background(), conn); err != nil {
+ t.Fatalf("rebuild article categories: %v", err)
+ }
+}
diff --git a/server/internal/middleware/compression.go b/server/internal/middleware/compression.go
new file mode 100644
index 0000000..5868575
--- /dev/null
+++ b/server/internal/middleware/compression.go
@@ -0,0 +1,203 @@
+package middleware
+
+import (
+ "compress/gzip"
+ "net/http"
+ "strings"
+ "sync"
+)
+
+// gzipWriterPool reuses the compressor state across requests. A gzip.Writer
+// carries a 32KB window plus its Huffman tables, so allocating one per response
+// is the kind of per-request garbage that shows up as GC pressure long before it
+// shows up as latency.
+var gzipWriterPool = sync.Pool{
+ New: func() any {
+ // BestSpeed rather than the default: article JSON is highly redundant
+ // (repeated keys, HTML markup) and already compresses ~5-8x at level 1.
+ // The extra CPU of the default level buys single-digit percent on this
+ // payload shape and is paid on every request.
+ w, _ := gzip.NewWriterLevel(nil, gzip.BestSpeed)
+ return w
+ },
+}
+
+// compressibleTypes are the media types worth compressing. Everything else --
+// JPEG and PNG from the media library above all -- is already compressed, and
+// running it through gzip spends CPU to make the response marginally larger.
+var compressibleTypes = []string{
+ "application/json",
+ "application/javascript",
+ "application/xml",
+ "image/svg+xml",
+ "text/",
+}
+
+func compressible(contentType string) bool {
+ // Content-Type carries parameters ("application/json; charset=utf-8"), so
+ // match on the prefix before them.
+ mediaType := strings.ToLower(strings.TrimSpace(strings.Split(contentType, ";")[0]))
+ for _, candidate := range compressibleTypes {
+ if strings.HasPrefix(mediaType, candidate) {
+ return true
+ }
+ }
+ return false
+}
+
+// minCompressSize is the payload below which gzip is not worth it. The gzip
+// header and trailer alone are 18 bytes, and a small JSON error body routinely
+// comes out larger compressed than raw.
+const minCompressSize = 512
+
+// gzipResponseWriter defers the decision to compress until the first Write,
+// because that is the earliest point at which Content-Type is known -- handlers
+// set it just before writing the body, and some let net/http sniff it.
+type gzipResponseWriter struct {
+ http.ResponseWriter
+
+ gz *gzip.Writer
+
+ // buf holds the first writes until there is either enough of the body to
+ // judge it worth compressing or the handler is done. Without it a response
+ // shorter than minCompressSize could not be passed through uncompressed,
+ // since the headers would already be on the wire.
+ buf []byte
+
+ decided bool
+ compress bool
+ status int
+ wroteHead bool
+}
+
+func (g *gzipResponseWriter) WriteHeader(code int) {
+ if g.wroteHead {
+ return
+ }
+ g.status = code
+ // Deliberately not forwarded yet: the choice to compress rewrites headers,
+ // and that has to happen before the status line goes out. flush() sends it.
+ g.wroteHead = true
+}
+
+func (g *gzipResponseWriter) Write(b []byte) (int, error) {
+ if !g.wroteHead {
+ g.WriteHeader(http.StatusOK)
+ }
+ if g.decided {
+ if g.compress {
+ return g.gz.Write(b)
+ }
+ return g.ResponseWriter.Write(b)
+ }
+
+ g.buf = append(g.buf, b...)
+ if len(g.buf) < minCompressSize {
+ // Not enough body yet to know which way this goes.
+ return len(b), nil
+ }
+ if err := g.decide(true); err != nil {
+ return 0, err
+ }
+ return len(b), nil
+}
+
+// decide commits to compressing or not, emits the headers and drains the
+// buffer. large reports whether the body has already exceeded minCompressSize;
+// a short body that ends at flush time is never compressed.
+func (g *gzipResponseWriter) decide(large bool) error {
+ g.decided = true
+
+ header := g.Header()
+ // A handler that encoded the body itself (or explicitly opted out with an
+ // identity encoding) owns the wire format; never double-encode it.
+ alreadyEncoded := header.Get("Content-Encoding") != ""
+ g.compress = large && !alreadyEncoded && compressible(header.Get("Content-Type"))
+
+ if g.compress {
+ header.Set("Content-Encoding", "gzip")
+ // Content-Length describes the uncompressed body and is now wrong.
+ // Leaving it makes the client truncate the response at that many bytes.
+ header.Del("Content-Length")
+ g.ResponseWriter.WriteHeader(g.status)
+
+ g.gz = gzipWriterPool.Get().(*gzip.Writer)
+ g.gz.Reset(g.ResponseWriter)
+ _, err := g.gz.Write(g.buf)
+ g.buf = nil
+ return err
+ }
+
+ g.ResponseWriter.WriteHeader(g.status)
+ if len(g.buf) > 0 {
+ _, err := g.ResponseWriter.Write(g.buf)
+ g.buf = nil
+ return err
+ }
+ return nil
+}
+
+// flush ends the response: it settles any still-undecided short body and closes
+// the compressor so the gzip trailer is written.
+func (g *gzipResponseWriter) flush() {
+ if !g.decided {
+ if !g.wroteHead {
+ // A handler that wrote nothing at all -- 204s, and the HEAD-like
+ // paths. Fall through to the standard 200 net/http would send.
+ g.status = http.StatusOK
+ }
+ _ = g.decide(false)
+ }
+ if g.gz != nil {
+ _ = g.gz.Close()
+ gzipWriterPool.Put(g.gz)
+ g.gz = nil
+ }
+}
+
+// Flush forwards explicit flushes, so a streaming handler is not silently
+// buffered forever by this middleware.
+func (g *gzipResponseWriter) Flush() {
+ if !g.decided {
+ // A handler that flushes is streaming: it may never reach
+ // minCompressSize, and holding its bytes back would stall the client.
+ _ = g.decide(len(g.buf) >= minCompressSize)
+ }
+ if g.gz != nil {
+ _ = g.gz.Flush()
+ }
+ if f, ok := g.ResponseWriter.(http.Flusher); ok {
+ f.Flush()
+ }
+}
+
+// Compression gzips responses for clients that accept it.
+//
+// Article JSON is the reason this exists: a listing page is tens of kilobytes of
+// highly redundant text, and the CMS previously shipped all of it raw, both to
+// the editor SPA and through Cloudflare to the public site.
+//
+// Placement matters. This must sit *outside* Recovery, so that the 500 body
+// Recovery writes after a panic goes through the same encoder as everything
+// else -- inside it, the gzip stream would be closed during unwinding and the
+// plain-text error would be appended to a finished stream, producing a response
+// no client can decode. The consequence is that Logging and Metrics, which are
+// outside this, keep counting uncompressed bytes; that is the same number they
+// reported before compression existed, so the dashboards stay comparable.
+func Compression(next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ // Vary goes on unconditionally, including for clients that did not ask
+ // for gzip: caches keyed on the URL alone would otherwise serve a
+ // compressed body to a client that cannot read it.
+ w.Header().Add("Vary", "Accept-Encoding")
+
+ if !strings.Contains(strings.ToLower(r.Header.Get("Accept-Encoding")), "gzip") {
+ next.ServeHTTP(w, r)
+ return
+ }
+
+ gw := &gzipResponseWriter{ResponseWriter: w, status: http.StatusOK}
+ defer gw.flush()
+ next.ServeHTTP(gw, r)
+ })
+}
diff --git a/server/internal/middleware/compression_test.go b/server/internal/middleware/compression_test.go
new file mode 100644
index 0000000..c54de27
--- /dev/null
+++ b/server/internal/middleware/compression_test.go
@@ -0,0 +1,180 @@
+package middleware
+
+import (
+ "bytes"
+ "compress/gzip"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+// jsonBody is comfortably over minCompressSize and as redundant as real article
+// JSON, so a test that asserts "smaller than the original" is not asserting a
+// coin flip.
+func jsonBody(n int) string {
+ return `{"articles":[` + strings.Repeat(`{"title":"Dragons win again","excerpt":"The Drexel Dragons"},`, n) + `]}`
+}
+
+func handlerWriting(contentType, body string) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", contentType)
+ _, _ = io.WriteString(w, body)
+ })
+}
+
+func serve(t *testing.T, h http.Handler, acceptEncoding string) *http.Response {
+ t.Helper()
+ req := httptest.NewRequest(http.MethodGet, "/v1/articles", nil)
+ if acceptEncoding != "" {
+ req.Header.Set("Accept-Encoding", acceptEncoding)
+ }
+ rec := httptest.NewRecorder()
+ Compression(h).ServeHTTP(rec, req)
+ return rec.Result()
+}
+
+func TestCompressionGzipsJSONForAcceptingClients(t *testing.T) {
+ body := jsonBody(50)
+ res := serve(t, handlerWriting("application/json", body), "gzip, deflate, br")
+
+ if got := res.Header.Get("Content-Encoding"); got != "gzip" {
+ t.Fatalf("Content-Encoding = %q, want gzip", got)
+ }
+ if got := res.Header.Get("Vary"); !strings.Contains(got, "Accept-Encoding") {
+ t.Errorf("Vary = %q, want it to contain Accept-Encoding", got)
+ }
+
+ raw, err := io.ReadAll(res.Body)
+ if err != nil {
+ t.Fatalf("read body: %v", err)
+ }
+ if len(raw) >= len(body) {
+ t.Errorf("compressed body is %d bytes, not smaller than the %d-byte original", len(raw), len(body))
+ }
+
+ zr, err := gzip.NewReader(bytes.NewReader(raw))
+ if err != nil {
+ t.Fatalf("open gzip reader: %v", err)
+ }
+ decoded, err := io.ReadAll(zr)
+ if err != nil {
+ t.Fatalf("decompress: %v", err)
+ }
+ if string(decoded) != body {
+ t.Errorf("round-tripped body does not match the original")
+ }
+}
+
+func TestCompressionSkipsClientsThatDidNotAskForIt(t *testing.T) {
+ body := jsonBody(50)
+ res := serve(t, handlerWriting("application/json", body), "")
+
+ if got := res.Header.Get("Content-Encoding"); got != "" {
+ t.Fatalf("Content-Encoding = %q, want empty", got)
+ }
+ raw, _ := io.ReadAll(res.Body)
+ if string(raw) != body {
+ t.Errorf("body was altered for a client that did not accept gzip")
+ }
+ // The header still has to be there, or a shared cache keyed on the URL
+ // alone will hand this identity response to a gzip client and vice versa.
+ if got := res.Header.Get("Vary"); !strings.Contains(got, "Accept-Encoding") {
+ t.Errorf("Vary = %q, want it to contain Accept-Encoding", got)
+ }
+}
+
+func TestCompressionSkipsAlreadyCompressedContentTypes(t *testing.T) {
+ // Bytes that do not compress, standing in for a JPEG out of the media
+ // library: the point is that the middleware never looks at them.
+ body := strings.Repeat("\x89PNG\r\n\x1a\n\xde\xad\xbe\xef", 200)
+ res := serve(t, handlerWriting("image/jpeg", body), "gzip")
+
+ if got := res.Header.Get("Content-Encoding"); got != "" {
+ t.Fatalf("Content-Encoding = %q, want empty for image/jpeg", got)
+ }
+ raw, _ := io.ReadAll(res.Body)
+ if string(raw) != body {
+ t.Errorf("image body was altered")
+ }
+}
+
+func TestCompressionSkipsShortBodies(t *testing.T) {
+ body := `{"error":"not found"}`
+ res := serve(t, handlerWriting("application/json", body), "gzip")
+
+ if got := res.Header.Get("Content-Encoding"); got != "" {
+ t.Fatalf("Content-Encoding = %q, want empty for a body under minCompressSize", got)
+ }
+ raw, _ := io.ReadAll(res.Body)
+ if string(raw) != body {
+ t.Errorf("short body = %q, want %q", raw, body)
+ }
+}
+
+func TestCompressionPreservesStatusCode(t *testing.T) {
+ body := jsonBody(50)
+ h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ w.WriteHeader(http.StatusCreated)
+ _, _ = io.WriteString(w, body)
+ })
+ res := serve(t, h, "gzip")
+
+ if res.StatusCode != http.StatusCreated {
+ t.Fatalf("status = %d, want %d", res.StatusCode, http.StatusCreated)
+ }
+ if got := res.Header.Get("Content-Encoding"); got != "gzip" {
+ t.Errorf("Content-Encoding = %q, want gzip", got)
+ }
+}
+
+// A stale Content-Length is the difference between a whole response and one the
+// client truncates at the uncompressed length.
+func TestCompressionDropsContentLength(t *testing.T) {
+ body := jsonBody(50)
+ h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ w.Header().Set("Content-Length", "999999")
+ _, _ = io.WriteString(w, body)
+ })
+ res := serve(t, h, "gzip")
+
+ if got := res.Header.Get("Content-Length"); got != "" {
+ t.Errorf("Content-Length = %q, want it removed once the body is gzipped", got)
+ }
+}
+
+func TestCompressionLeavesHandlerEncodedBodiesAlone(t *testing.T) {
+ body := jsonBody(50)
+ h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ w.Header().Set("Content-Encoding", "identity")
+ _, _ = io.WriteString(w, body)
+ })
+ res := serve(t, h, "gzip")
+
+ if got := res.Header.Get("Content-Encoding"); got != "identity" {
+ t.Fatalf("Content-Encoding = %q, want the handler's own identity to survive", got)
+ }
+ raw, _ := io.ReadAll(res.Body)
+ if string(raw) != body {
+ t.Errorf("body was double-encoded")
+ }
+}
+
+func TestCompressionHandlesEmptyResponses(t *testing.T) {
+ h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusNoContent)
+ })
+ res := serve(t, h, "gzip")
+
+ if res.StatusCode != http.StatusNoContent {
+ t.Fatalf("status = %d, want %d", res.StatusCode, http.StatusNoContent)
+ }
+ raw, _ := io.ReadAll(res.Body)
+ if len(raw) != 0 {
+ t.Errorf("body = %q, want empty", raw)
+ }
+}
diff --git a/server/main.go b/server/main.go
index 663dee4..8ebfbe1 100644
--- a/server/main.go
+++ b/server/main.go
@@ -130,6 +130,15 @@ func main() {
slog.Error("failed to build article pub_date index; public listings filesort", "error", err)
}
+ // Non-fatal for the same reason as the pub_date index: the lookups are
+ // correct without it, only slower.
+ if err := database.EnsureArticlesSlugIndex(context.Background(), db); err != nil {
+ slog.Error("failed to index article slugs; article lookups scan the table", "error", err)
+ }
+ if err := database.EnsureArticleAuthorsIndex(context.Background(), db); err != nil {
+ slog.Error("failed to index article authors; byline lookups scan the join table", "error", err)
+ }
+
// Deliberately not fatal, and deliberately after the column migration: the
// first FULLTEXT index on `articles` rebuilds the table, which on the
// migrated corpus is slow enough that failing the boot over it would trade a
@@ -202,6 +211,27 @@ func main() {
slog.Error("failed to create classifieds table", "error", err)
os.Exit(1)
}
+
+ // Fatal, unlike the index migrations above, because this one is not an
+ // optimization the queries can do without: every section page, homepage
+ // block and taxonomy count now reads article_categories, and a CMS that
+ // booted without it would serve empty sections rather than slow ones.
+ //
+ // Rebuilt unconditionally, and deliberately not behind
+ // CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP. The table is derived from
+ // `articles`, which the WordPress ETL replaces wholesale and renumbers; a
+ // rebuild that an operator has to remember to ask for is one that will be
+ // forgotten on the reseed that needs it most. It costs a single scan of a
+ // ten-thousand-row table.
+ if err := database.EnsureArticleCategoriesTable(context.Background(), db); err != nil {
+ slog.Error("failed to create article categories index", "error", err)
+ os.Exit(1)
+ }
+ if err := database.RebuildArticleCategories(context.Background(), db); err != nil {
+ slog.Error("failed to rebuild article categories index", "error", err)
+ os.Exit(1)
+ }
+
if strings.EqualFold(strings.TrimSpace(os.Getenv("CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP")), "true") {
if err := database.RebuildTaxonomyArticleCounts(context.Background(), db); err != nil {
slog.Error("failed to rebuild taxonomy article counts", "error", err)
@@ -347,8 +377,10 @@ func newDefaultServer(cert *tls.Certificate, mux *http.ServeMux, logger *slog.Lo
Addr: ":8080",
// Metrics is outermost: Chain applies in reverse, so it wraps
// Recovery and therefore records the 500 a panic turns into rather
- // than losing the request entirely.
- Handler: middleware.Chain(mux, middleware.Metrics, middleware.Logging, middleware.Recovery),
+ // than losing the request entirely. Compression sits just inside
+ // Logging and just outside Recovery -- see middleware.Compression
+ // for why that position is the only correct one.
+ Handler: middleware.Chain(mux, middleware.Metrics, middleware.Logging, middleware.Compression, middleware.Recovery),
TLSConfig: tlsConfig,
ErrorLog: slog.NewLogLogger(logger.Handler(), slog.LevelError),
},