Derive excerpts on every save, and stop 404ing unchanged saves

server/cmd/backfill-excerpts/main.go

@@ -0,0 +1,156 @@
+// Command backfill-excerpts repairs articles whose `excerpt` column was blanked
+// by a save.
+//
+// The excerpt box is optional and the editor sends it on every save, so for a
+// while PUT and PATCH stored the blank verbatim while POST derived one from the
+// body. An article therefore got an excerpt on create and lost it on the first
+// edit afterward. Listings select `excerpt` and never the body, so a blank
+// column is a story that appears on the section pages and the homepage with no
+// summary under it.
+//
+// The write paths now derive an excerpt from the body whenever the field
+// arrives blank; this repairs the rows blanked before that. It derives through
+// exactly the same db.ExcerptOrDerived the server uses, so a repaired row is
+// byte-identical to what a save would write today.
+//
+// It reads DB_* from the environment, like the server, so it can be run inside
+// the backend service's own environment rather than with credentials copied by
+// hand. Dry run by default: pass -apply to write.
+package main
+
+import (
+	"context"
+	"database/sql"
+	"flag"
+	"fmt"
+	"log"
+	"os"
+	"strconv"
+	"strings"
+	"time"
+
+	_ "github.com/go-sql-driver/mysql"
+
+	db "server/internal/database"
+)
+
+func main() {
+	apply := flag.Bool("apply", false, "write the derived excerpts; without it, report what would change")
+	limit := flag.Int("limit", 0, "stop after this many articles (0 = all)")
+	verbose := flag.Bool("verbose", false, "list every article, not just the first few of each kind")
+	flag.Parse()
+
+	ctx := context.Background()
+	conn, err := connect(ctx)
+	if err != nil {
+		log.Fatalf("connect: %v", err)
+	}
+	defer conn.Close()
+
+	// Read every blank row up front rather than streaming: the UPDATEs below run
+	// against the same table the cursor is walking, and the whole set is a few
+	// hundred rows.
+	type article struct {
+		id      int64
+		slug    string
+		content string
+	}
+	query := "SELECT `id`, `slug`, COALESCE(`text`, '') FROM `articles` WHERE TRIM(COALESCE(`excerpt`, '')) = '' ORDER BY `id`"
+	if *limit > 0 {
+		query += " LIMIT " + strconv.Itoa(*limit)
+	}
+	rows, err := conn.QueryContext(ctx, query)
+	if err != nil {
+		log.Fatalf("select blank excerpts: %v", err)
+	}
+	var blank []article
+	for rows.Next() {
+		var a article
+		if err := rows.Scan(&a.id, &a.slug, &a.content); err != nil {
+			rows.Close()
+			log.Fatalf("scan: %v", err)
+		}
+		blank = append(blank, a)
+	}
+	if err := rows.Err(); err != nil {
+		rows.Close()
+		log.Fatalf("iterate: %v", err)
+	}
+	rows.Close()
+
+	fmt.Printf("%d articles have a blank excerpt\n", len(blank))
+
+	var repaired, empty int
+	for _, a := range blank {
+		derived := db.ExcerptOrDerived("", a.content)
+		if strings.TrimSpace(derived) == "" {
+			// Nothing to derive from -- a body that is a bare shortcode, an
+			// image-only post, or a genuinely empty draft. Leave it alone: an
+			// empty excerpt is what it had, and there is no text to improve on.
+			if *verbose || empty < 10 {
+				fmt.Printf("  [skip] %s (body %d bytes: %s)\n", a.slug, len(a.content), truncate(strings.Join(strings.Fields(a.content), " "), 70))
+			}
+			empty++
+			continue
+		}
+		if !*apply {
+			if *verbose || repaired < 10 {
+				fmt.Printf("  %s -> %s\n", a.slug, truncate(derived, 80))
+			}
+			repaired++
+			continue
+		}
+		// mod_date is deliberately left alone. This is a repair of a value the
+		// CMS should have written at the time, not an edit: touching mod_date
+		// would move every repaired article's sitemap lastmod to today and queue
+		// the whole corpus for re-embedding.
+		if _, err := conn.ExecContext(ctx,
+			"UPDATE `articles` SET `excerpt` = ? WHERE `id` = ? AND TRIM(COALESCE(`excerpt`, '')) = ''",
+			derived, a.id,
+		); err != nil {
+			log.Fatalf("update %s: %v", a.slug, err)
+		}
+		repaired++
+	}
+
+	verb := "would repair"
+	if *apply {
+		verb = "repaired"
+	}
+	fmt.Printf("%s %d articles; %d left alone (no text to derive from)\n", verb, repaired, empty)
+}
+
+func connect(ctx context.Context) (*sql.DB, error) {
+	name := strings.TrimSpace(os.Getenv("DB_NAME"))
+	user := strings.TrimSpace(os.Getenv("DB_USER"))
+	password := os.Getenv("DB_PASSWORD")
+	host := strings.TrimSpace(os.Getenv("DB_HOST"))
+	if name == "" || user == "" {
+		return nil, fmt.Errorf("DB_NAME and DB_USER are required")
+	}
+	if host == "" {
+		host = "127.0.0.1"
+	}
+	port := 3306
+	if raw := strings.TrimSpace(os.Getenv("DB_PORT")); raw != "" {
+		parsed, err := strconv.Atoi(raw)
+		if err != nil {
+			return nil, fmt.Errorf("DB_PORT %q: %w", raw, err)
+		}
+		port = parsed
+	}
+	// A one-off maintenance run has no business taking the server's share of a
+	// 200-connection budget, and it is single-threaded anyway.
+	os.Setenv("DB_MAX_OPEN_CONNS", "2")
+
+	timeoutCtx, cancel := context.WithTimeout(ctx, 15*time.Second)
+	defer cancel()
+	return db.InitializeConnection(timeoutCtx, name, user, password, host, port)
+}
+
+func truncate(s string, n int) string {
+	if len(s) <= n {
+		return s
+	}
+	return s[:n] + "..."
+}

server/internal/database/database.go

@@ -13,8 +13,22 @@ import (
 	_ "github.com/go-sql-driver/mysql"
 )
 
+// clientFoundRows makes an UPDATE report the rows it matched rather than the
+// rows whose values it changed.
+//
+// Every handler that reads RowsAffected treats zero as "no such row" and
+// answers 404 -- author update, patch, archive and restore, and both article
+// write paths. Without this flag a save that sets every column to what it
+// already holds matches its row, changes nothing, reports zero, and is answered
+// as though the article had been deleted mid-edit. The editor sends the whole
+// form on every save and autosaves on a timer, so two saves inside the same
+// second -- a double-clicked Save, or an autosave landing on an untouched form
+// after mod_date has already been stamped to the current second -- are enough.
+//
+// No call site uses RowsAffected to ask whether anything changed, so matched
+// rows is what all six of them already mean.
 func buildDSN(dbName, user, password, host string, port int) string {
-	return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?parseTime=true", user, password, host, port, dbName)
+	return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?parseTime=true&clientFoundRows=true", user, password, host, port, dbName)
 }
 
 // defaultMaxOpenConns is the size of the connection pool.

server/internal/database/database_test.go

@@ -1 +1,22 @@
 package database
+
+import (
+	"strings"
+	"testing"
+)
+
+// Every handler that reads RowsAffected treats zero as "no such row" and answers
+// 404. Without clientFoundRows an UPDATE that matches its row but changes none of
+// its values reports zero, so a save that happened to change nothing is answered
+// as though the article had been deleted mid-edit.
+func TestBuildDSN_RequestsFoundRowsSemantics(t *testing.T) {
+	dsn := buildDSN("cms", "user", "pw", "db1.internal", 3306)
+	if !strings.Contains(dsn, "clientFoundRows=true") {
+		t.Errorf("DSN = %q, want clientFoundRows=true", dsn)
+	}
+	// parseTime drives every DATETIME column into time.Time; losing it would turn
+	// every date in the API into a []byte.
+	if !strings.Contains(dsn, "parseTime=true") {
+		t.Errorf("DSN = %q, want parseTime=true kept", dsn)
+	}
+}

server/internal/database/http_models.go

@@ -634,6 +634,21 @@ func ReplaceArticleAuthorsBySlug(ctx context.Context, conn *sql.DB, slug string,
 	return ReplaceArticleAuthors(ctx, conn, articleID, authorIDs)
 }
 
+// GetArticleContentBySlug reads just the body, for a patch that has to derive
+// an excerpt from a body it did not carry. A missing article is "" and no
+// error: the UPDATE that follows reports the 404 on its own row count.
+func GetArticleContentBySlug(ctx context.Context, conn *sql.DB, slug string) (string, error) {
+	var text sql.NullString
+	err := conn.QueryRowContext(ctx, "SELECT `text` FROM `articles` WHERE `slug` = ?", strings.TrimSpace(slug)).Scan(&text)
+	if errors.Is(err, sql.ErrNoRows) {
+		return "", nil
+	}
+	if err != nil {
+		return "", err
+	}
+	return text.String, nil
+}
+
 func ParsePublishedAt(value string) *time.Time {
 	if strings.TrimSpace(value) == "" {
 		return nil
@@ -751,6 +766,19 @@ func deriveExcerpt(content string) string {
 	return strings.Join(words, " ")
 }
 
+// ExcerptOrDerived is what every write that sets the `excerpt` column stores.
+// An excerpt is optional in the editor, so a blank one means "derive it from
+// the body", never "store nothing": listings render `excerpt` and nothing else
+// -- the body is not selected for them -- so a stored empty string is an
+// article that appears on every section page and the homepage with no summary
+// under it.
+func ExcerptOrDerived(excerpt, content string) string {
+	if trimmed := strings.TrimSpace(excerpt); trimmed != "" {
+		return trimmed
+	}
+	return deriveExcerpt(content)
+}
+
 func normalizeSlug(value string) string {
 	s := strings.ToLower(strings.TrimSpace(value))
 	if s == "" {
@@ -792,10 +820,7 @@ func ArticleInputToDBFields(body models.ArticleInput) []any {
 		slug = normalizeSlug(body.Title)
 	}
 
-	excerpt := strings.TrimSpace(body.Excerpt)
-	if excerpt == "" {
-		excerpt = deriveExcerpt(body.Content)
-	}
+	excerpt := ExcerptOrDerived(body.Excerpt, body.Content)
 
 	tags := FormatTags(body.Tags)
 	if tags == "" {
@@ -858,7 +883,7 @@ func ArticleToDBFields(body models.Article) []any {
 	return []any{
 		body.Title,
 		normalizeSlug(body.Slug),
-		body.Excerpt,
+		ExcerptOrDerived(body.Excerpt, body.Content),
 		body.Content,
 		FormatTags(body.Categories),
 		publishedAt,

server/internal/database/http_models_test.go

@@ -182,6 +182,44 @@ func TestArticleToDBFields_StampsModifiedDate(t *testing.T) {
 	}
 }
 
+// The excerpt field is optional in the editor, and a PUT sends it blank when the
+// author never filled it in. Storing that blank left the article with no summary
+// on the section pages and the homepage, which render `excerpt` and never the
+// body -- POST had derived one from the body all along, and a full replacement
+// has the same body to derive from.
+func TestArticleToDBFields_DerivesExcerptFromBodyWhenBlank(t *testing.T) {
+	fields := ArticleToDBFields(models.Article{
+		Title:   "Phillies reinforce team at the trade deadline",
+		Slug:    "phillies-reinforce-team-at-the-trade-deadline",
+		Excerpt: "   ",
+		Content: "<p>The Phillies added two arms before Thursday's deadline.</p>",
+	})
+
+	// Column order is title, slug, excerpt, text.
+	const excerptFieldIndex = 2
+	got, ok := fields[excerptFieldIndex].(string)
+	if !ok {
+		t.Fatalf("excerpt field has type %T, want string", fields[excerptFieldIndex])
+	}
+	if got != "The Phillies added two arms before Thursday's deadline." {
+		t.Errorf("excerpt = %q, want it derived from the body", got)
+	}
+}
+
+func TestArticleToDBFields_KeepsSuppliedExcerpt(t *testing.T) {
+	fields := ArticleToDBFields(models.Article{
+		Title:   "Phillies reinforce team at the trade deadline",
+		Slug:    "phillies-reinforce-team-at-the-trade-deadline",
+		Excerpt: "  An editor's own summary.  ",
+		Content: "<p>The Phillies added two arms before Thursday's deadline.</p>",
+	})
+
+	const excerptFieldIndex = 2
+	if got := fields[excerptFieldIndex]; got != "An editor's own summary." {
+		t.Errorf("excerpt = %q, want the supplied text", got)
+	}
+}
+
 // An article normally opens with its featured image, so an excerpt derived by
 // stripping tags used to start with the photo credit in the caption instead of
 // the story.