golem15/sm-seo-plugin
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: the Open/Resolved and Chains filters apply an exact rule's slash, case and query options (WR-22)

Browse Source 
coveredSQL and chainSQL compared the source literally, so /missing/ or
/missing?utm=x stayed open after /missing answered it, and /a -> /b/ was
no chain next to /b. Both now use one SQL form of the runtime's exact
match (prefiltered on the indexed source_key). Pattern and regex rules
remain with the per-row badge, which the list_not_found schema text now
says.
This commit is contained in:
Jakub Zych
2026-10-27 15:41:18 +01:00
parent ed2d919aee
commit 7677d8d8af
4 changed files with 90 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
controllers/api/notfound.go
View File

@@ -71,7 +71,7 @@ type ListNotFoundInput struct {
Page int `json:"page,omitempty" query:"page" jsonschema:"the page number, from 1"`
PerPage int `json:"per_page,omitempty" query:"per_page" jsonschema:"rows per page, at most 200 (50 by default)"`
Search string `json:"search,omitempty" query:"search" jsonschema:"text the address contains"`
Status string `json:"status,omitempty" query:"status" jsonschema:"open (no redirect answers the address yet) or resolved"`
Status string `json:"status,omitempty" query:"status" jsonschema:"open or resolved: whether an exact redirect or 410 answers the address, with its case, trailing-slash and query options; an address only a pattern or regex rule answers stays open here but has resolved true in its row"`
}
// NotFoundIDInput names one 404 log entry.

8
models/notfound.go
View File

@@ -41,9 +41,11 @@ const (
// FilterScopes lists the 404 log's filter scopes.
func (NotFound) FilterScopes() []string { return []string{FilterNotFoundStatus} }
// coveredSQL is true for an entry an exact redirect or 410 answers now
// (the list filter; the Resolved badge also counts patterns and regex).
const coveredSQL = `EXISTS (SELECT 1 FROM golem15_seo_redirects AS r WHERE r.match = 'exact' AND (r.source = golem15_seo_notfound.url OR (r.ignore_case AND lower(r.source) = lower(golem15_seo_notfound.url))))`
// coveredSQL is true for an entry an exact redirect or 410 answers now,
// with the rule's case, trailing-slash and query options as the runtime
// matcher applies them (WR-22). Pattern and regex rules are left to the
// Resolved badge, which runs the compiled rule set.
var coveredSQL = `EXISTS (SELECT 1 FROM golem15_seo_redirects AS r WHERE ` + coversSQL("r", "golem15_seo_notfound.url") + `)`
// FilterScope narrows the 404 log to open or resolved entries; an unknown
// value matches nothing.

39
models/redirect.go
View File

@@ -224,9 +224,42 @@ func (Redirect) FilterScopes() []string {
return []string{FilterRedirectStatus, FilterRedirectMatch, FilterRedirectOrigin, FilterRedirectChain}
}
// chainSQL is true for a redirect whose target is another redirect's
// source.
const chainSQL = `EXISTS (SELECT 1 FROM golem15_seo_redirects AS next WHERE next.source = golem15_seo_redirects.target AND next.id <> golem15_seo_redirects.id)`
// coversSQL is an SQL condition, true when the exact redirect aliased
// r matches the address addr (an SQL expression of a path with an optional
// query) the way the runtime matcher does (classes.exactMatches): the
// source's path with its ignore_case and trailing_slash options, and, when
// the source has a query, the address's query exactly (WR-22). The
// source_key test (the generated column of classes.exactKey) is a
// necessary condition that lets Postgres use its index; the rest is exact.
// Pattern and regex rules are not covered: their expressions are RE2 and
// only the compiled RedirectSet evaluates them.
func coversSQL(r, addr string) string {
return strings.NewReplacer("R.", r+".", "ADDR", addr).Replace(exactCoversSQL)
}
// exactCoversSQL is coversSQL with the placeholders R. and ADDR.
const exactCoversSQL = `(R.match = 'exact'
AND R.source_key IN (
lower(CASE WHEN length(split_part(ADDR, '?', 1)) > 1 AND right(split_part(ADDR, '?', 1), 1) = '/' THEN left(split_part(ADDR, '?', 1), -1) ELSE split_part(ADDR, '?', 1) END),
lower(CASE WHEN length(split_part(ADDR, '?', 1)) > 1 AND right(split_part(ADDR, '?', 1), 1) = '/' THEN left(split_part(ADDR, '?', 1), -1) ELSE split_part(ADDR, '?', 1) END)
|| '?' || (CASE WHEN strpos(ADDR, '?') > 0 THEN substr(ADDR, strpos(ADDR, '?') + 1) ELSE '' END))
AND (strpos(R.source, '?') = 0
OR substr(R.source, strpos(R.source, '?') + 1) = (CASE WHEN strpos(ADDR, '?') > 0 THEN substr(ADDR, strpos(ADDR, '?') + 1) ELSE '' END))
AND (split_part(R.source, '?', 1) = split_part(ADDR, '?', 1)
OR (R.ignore_case AND lower(split_part(R.source, '?', 1)) = lower(split_part(ADDR, '?', 1)))
OR (R.trailing_slash AND (
(CASE WHEN length(split_part(R.source, '?', 1)) > 1 AND right(split_part(R.source, '?', 1), 1) = '/' THEN left(split_part(R.source, '?', 1), -1) ELSE split_part(R.source, '?', 1) END)
= (CASE WHEN length(split_part(ADDR, '?', 1)) > 1 AND right(split_part(ADDR, '?', 1), 1) = '/' THEN left(split_part(ADDR, '?', 1), -1) ELSE split_part(ADDR, '?', 1) END)
OR (R.ignore_case AND lower(CASE WHEN length(split_part(R.source, '?', 1)) > 1 AND right(split_part(R.source, '?', 1), 1) = '/' THEN left(split_part(R.source, '?', 1), -1) ELSE split_part(R.source, '?', 1) END)
= lower(CASE WHEN length(split_part(ADDR, '?', 1)) > 1 AND right(split_part(ADDR, '?', 1), 1) = '/' THEN left(split_part(ADDR, '?', 1), -1) ELSE split_part(ADDR, '?', 1) END))))))`
// chainSQL is true for a redirect whose target leads into another
// redirect: a path target that another rule's source names literally or
// an exact rule matches with its options (WR-22). Absolute targets on the
// site URL and pattern or regex next hops are only seen by the save-time
// check (the chain notice).
var chainSQL = `EXISTS (SELECT 1 FROM golem15_seo_redirects AS next WHERE next.id <> golem15_seo_redirects.id AND (next.source = golem15_seo_redirects.target OR (golem15_seo_redirects.target LIKE '/%' AND golem15_seo_redirects.target NOT LIKE '//%' AND ` +
coversSQL("next", "golem15_seo_redirects.target") + `)))`
// FilterScope narrows the redirects list; an unknown value matches
// nothing.

48
redirects_test.go
View File

@@ -893,3 +893,51 @@ func TestConcurrentUndo(t *testing.T) {
t.Fatalf("undo history rows = %d (err %v), want 1", undos, err)
}
}
// TestCoverAndChainFilters checks that the 404 log's Open/Resolved filter
// and the redirects list's Chains filter apply an exact rule's
// trailing-slash, case and query options like the runtime matcher (WR-22).
func TestCoverAndChainFilters(t *testing.T) {
env := bootEnv(t)
agent := env.principal(t, env.admin(t, false, "golem15.seo.manage"))
env.mustRedirect(t, "/missing", "exact", "/found", 301)
env.mustRedirect(t, "/Case", "exact", "/found", 301, func(r *models.Redirect) { r.IgnoreCase = true })
env.mustRedirect(t, "/q?id=1", "exact", "/found", 301)
for _, u := range []string{"/missing/", "/missing?utm=x", "/case/", "/q?id=1", "/q?id=2", "/MISSING", "/other"} {
if err := env.db.Exec(`INSERT INTO golem15_seo_notfound (url, hits, first_seen, last_hit) VALUES (?, 1, NOW(), NOW())`, u).Error; err != nil {
t.Fatal(err)
}
}
urls := func(status string) []string {
t.Helper()
got := env.mcpExpect(t, http.StatusOK, http.MethodGet, "/_seo/api/v1/notfound?per_page=200&status="+status, agent, nil)
var out []string
for _, row := range got["data"].([]any) {
out = append(out, row.(map[string]any)["url"].(string))
if resolved := row.(map[string]any)["resolved"].(bool); resolved != (status == "resolved") {
t.Errorf("%s: row %v disagrees with its filter", status, row)
}
}
sort.Strings(out)
return out
}
if got := strings.Join(urls("resolved"), ","); got != "/case/,/missing/,/missing?utm=x,/q?id=1" {
t.Fatalf("resolved = %s", got)
}
if got := strings.Join(urls("open"), ","); got != "/MISSING,/other,/q?id=2" {
t.Fatalf("open = %s", got)
}
env.mustRedirect(t, "/c2", "exact", "/c3", 301)
env.mustRedirect(t, "/c1", "exact", "/c2/", 301)
env.mustRedirect(t, "/d1", "exact", "/C2", 301)
var chained []string
q := (models.Redirect{}).FilterScope(models.FilterRedirectChain, env.db.Model(&models.Redirect{}), models.ChainOnly)
if err := q.Order("source").Pluck("source", &chained).Error; err != nil {
t.Fatal(err)
}
if got := strings.Join(chained, ","); got != "/c1" {
t.Fatalf("chains = %s, want /c1 (/c2/ reaches /c2 with its trailing slash; /C2 does not, /c2 is case-sensitive)", got)
}
}