feat: crawler checks for status, sitemaps, canonical, titles and headings
- page_status, sitemap_error, sitemap_noindex, sitemap_blocked and sitemap_missing against the fetched sitemap files and robots.txt - canonical_missing and canonical_error, missing and duplicate titles and descriptions over the crawl - h1_missing, h1_multiple, heading_skipped, heading_empty and h1_title_mismatch; content headings route to the record's tool - the staging lock's noindex is never counted (Pitfall 12) - TestCrawlerChecks on the planted site with a clean control page
This commit is contained in:
Jakub Zych
@@ -1,8 +1,13 @@
|
||||
package crawl
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"git.golem15.com/golem15/sm-seo-plugin/models"
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"github.com/jimsmart/grobotstxt"
|
||||
)
|
||||
|
||||
// The crawl kinds of UI-SPEC C23.
|
||||
@@ -107,10 +112,16 @@ func Kinds() []string {
|
||||
}
|
||||
|
||||
// Env is what the checks know besides the crawl: the record behind an
|
||||
// address.
|
||||
// address, whether the site runs outside production (the staging lock's
|
||||
// noindex is never reported, RESEARCH Pitfall 12) and the generic link
|
||||
// texts.
|
||||
type Env struct {
|
||||
// Lookup returns the CMS record of an address, nil when none.
|
||||
Lookup func(key string) *Record
|
||||
// Staging is true outside production.
|
||||
Staging bool
|
||||
// GenericAnchors are the generic link texts (any case).
|
||||
GenericAnchors []string
|
||||
}
|
||||
|
||||
func (e *Env) record(key string) *Record {
|
||||
@@ -120,32 +131,312 @@ func (e *Env) record(key string) *Record {
|
||||
return e.Lookup(key)
|
||||
}
|
||||
|
||||
// Check runs the per-page checks over a crawl's pages.
|
||||
// locale is the issue locale of a page: its record's.
|
||||
func locale(rec *Record) string {
|
||||
if rec == nil {
|
||||
return ""
|
||||
}
|
||||
return rec.Locale
|
||||
}
|
||||
|
||||
// noindex reports whether a page asks not to be indexed (meta robots or
|
||||
// X-Robots-Tag). On a non-production environment every page carries the
|
||||
// staging lock's noindex, so it is never counted there.
|
||||
func (e *Env) noindex(p *Page) bool {
|
||||
if e != nil && e.Staging {
|
||||
return false
|
||||
}
|
||||
if p.Facts != nil && p.Facts.Noindex() {
|
||||
return true
|
||||
}
|
||||
return p.Resp != nil && hasNoindex(p.Resp.XRobots)
|
||||
}
|
||||
|
||||
// selfCanonical reports whether a page's canonical is missing or names
|
||||
// the page itself.
|
||||
func selfCanonical(res *Result, p *Page) bool {
|
||||
if p.Facts == nil || p.Facts.Canonical == "" || res.fetcher == nil {
|
||||
return true
|
||||
}
|
||||
u, err := url.Parse(p.Facts.Canonical)
|
||||
if err != nil {
|
||||
return true
|
||||
}
|
||||
return res.fetcher.Key(u) == p.Key
|
||||
}
|
||||
|
||||
// indexable reports whether a page answers 200 with HTML, may be indexed
|
||||
// and is its own canonical.
|
||||
func (e *Env) indexable(res *Result, p *Page) bool {
|
||||
return p.OK() && !e.noindex(p) && selfCanonical(res, p)
|
||||
}
|
||||
|
||||
// Check runs every check over a crawl's pages: the per-page checks of
|
||||
// UI-SPEC C23 and the site-wide ones (sitemaps, statuses, duplicates).
|
||||
func Check(res *Result, env *Env) []Finding {
|
||||
if env == nil {
|
||||
env = &Env{}
|
||||
}
|
||||
var out []Finding
|
||||
for _, key := range res.Order {
|
||||
p := res.Pages[key]
|
||||
rec := env.record(key)
|
||||
out = append(out, checkStatus(res, p, rec)...)
|
||||
if !p.OK() {
|
||||
continue
|
||||
}
|
||||
out = append(out, checkHeadings(p, env.record(key))...)
|
||||
out = append(out, checkHead(res, p, rec)...)
|
||||
out = append(out, checkHeadings(p, rec)...)
|
||||
}
|
||||
out = append(out, checkSitemaps(res, env)...)
|
||||
out = append(out, checkDuplicates(res, env)...)
|
||||
return out
|
||||
}
|
||||
|
||||
// checkStatus reports page_status for an address that answers an error
|
||||
// (other than 404 and 410, which are broken links when linked) or cannot
|
||||
// be fetched.
|
||||
func checkStatus(res *Result, p *Page, rec *Record) []Finding {
|
||||
r := p.Resp
|
||||
if r == nil {
|
||||
return nil
|
||||
}
|
||||
switch {
|
||||
case r.Err != "" && r.Err != "canceled":
|
||||
return []Finding{{Kind: KindPageStatus, URL: p.Key, Locale: locale(rec), Record: rec, Tool: ToolCreateRedirect, Details: map[string]any{"status": 0, "error": r.Err}}}
|
||||
case r.Status >= 400 && r.Status != 404 && r.Status != 410:
|
||||
return []Finding{{Kind: KindPageStatus, URL: p.Key, Locale: locale(rec), Record: rec, Tool: ToolCreateRedirect, Details: map[string]any{"status": r.Status}}}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// checkHead reports a missing title, description or canonical and a
|
||||
// canonical pointing to an error page.
|
||||
func checkHead(res *Result, p *Page, rec *Record) []Finding {
|
||||
var out []Finding
|
||||
f := p.Facts
|
||||
loc := locale(rec)
|
||||
if strings.TrimSpace(f.Title) == "" {
|
||||
out = append(out, Finding{Kind: KindMissingTitle, URL: p.Key, Locale: loc, Record: rec, Tool: ToolSetURLSEO})
|
||||
}
|
||||
if strings.TrimSpace(f.Description) == "" {
|
||||
out = append(out, Finding{Kind: KindMissingDesc, URL: p.Key, Locale: loc, Record: rec, Tool: ToolSetURLSEO})
|
||||
}
|
||||
if f.Canonical == "" {
|
||||
out = append(out, Finding{Kind: KindCanonicalMissing, URL: p.Key, Locale: loc, Record: rec})
|
||||
} else if u, err := url.Parse(f.Canonical); err == nil && res.fetcher != nil {
|
||||
if key := res.fetcher.Key(u); key != "" && key != p.Key {
|
||||
if t := res.Page(key); t != nil && t.Resp != nil && (t.Resp.Err != "" || t.Resp.Status >= 400) {
|
||||
out = append(out, Finding{Kind: KindCanonicalError, URL: p.Key, Locale: loc, Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"url": key, "status": t.Resp.Status}})
|
||||
}
|
||||
}
|
||||
}
|
||||
if h1 := f.H1(); len(h1) > 0 && strings.TrimSpace(f.Title) != "" && strings.TrimSpace(h1[0]) != "" {
|
||||
a, b := normal(h1[0]), normal(f.Title)
|
||||
if !strings.Contains(b, a) && !strings.Contains(a, b) {
|
||||
out = append(out, Finding{Kind: KindH1TitleMismatch, URL: p.Key, Locale: loc, Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"h1": h1[0], "title": f.Title}})
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// checkHeadings reports h1_missing and h1_multiple. An H1 inside the
|
||||
// record's own content is fixed through the record's headings tool.
|
||||
// checkHeadings reports h1_missing, h1_multiple, heading_skipped and
|
||||
// heading_empty. A heading inside the record's own content is fixed
|
||||
// through the record's tool: H1 findings prefer Tools[headings], the
|
||||
// others use Tools[edit] (P33-38).
|
||||
func checkHeadings(p *Page, rec *Record) []Finding {
|
||||
var out []Finding
|
||||
loc := locale(rec)
|
||||
h1 := p.Facts.H1()
|
||||
switch {
|
||||
case len(h1) == 0:
|
||||
return []Finding{{Kind: KindH1Missing, URL: p.Key, Record: rec}}
|
||||
out = append(out, Finding{Kind: KindH1Missing, URL: p.Key, Locale: loc, Record: rec})
|
||||
case len(h1) > 1:
|
||||
f := Finding{Kind: KindH1Multiple, URL: p.Key, Record: rec, Details: map[string]any{"count": len(h1), "h1": h1}}
|
||||
f := Finding{Kind: KindH1Multiple, URL: p.Key, Locale: loc, Record: rec, Details: map[string]any{"count": len(h1), "h1": h1}}
|
||||
if rec.hasHeading(1, "") {
|
||||
f.Content = []string{pact.ContentToolHeadings, pact.ContentToolEdit}
|
||||
}
|
||||
return []Finding{f}
|
||||
out = append(out, f)
|
||||
}
|
||||
return nil
|
||||
var skips []map[string]any
|
||||
var first *Finding
|
||||
prev := 0
|
||||
for _, h := range p.Facts.Headings {
|
||||
if prev > 0 && h.Level > prev+1 {
|
||||
skips = append(skips, map[string]any{"from": prev, "to": h.Level, "text": h.Text})
|
||||
if first == nil {
|
||||
first = &Finding{Kind: KindHeadingSkipped, URL: p.Key, Locale: loc, Record: rec, Details: map[string]any{"from": prev, "to": h.Level, "text": h.Text}}
|
||||
if rec.hasHeading(h.Level, h.Text) {
|
||||
first.Content = []string{pact.ContentToolEdit}
|
||||
}
|
||||
}
|
||||
}
|
||||
prev = h.Level
|
||||
}
|
||||
if first != nil {
|
||||
first.Details["skips"] = skips
|
||||
first.Details["count"] = len(skips)
|
||||
out = append(out, *first)
|
||||
}
|
||||
empty := 0
|
||||
var firstEmpty *Finding
|
||||
for _, h := range p.Facts.Headings {
|
||||
if strings.TrimSpace(h.Text) != "" {
|
||||
continue
|
||||
}
|
||||
empty++
|
||||
if firstEmpty == nil {
|
||||
firstEmpty = &Finding{Kind: KindHeadingEmpty, URL: p.Key, Locale: loc, Record: rec, Details: map[string]any{"level": h.Level}}
|
||||
if hasEmptyHeading(rec, h.Level) {
|
||||
if h.Level == 1 {
|
||||
firstEmpty.Content = []string{pact.ContentToolHeadings, pact.ContentToolEdit}
|
||||
} else {
|
||||
firstEmpty.Content = []string{pact.ContentToolEdit}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if firstEmpty != nil {
|
||||
firstEmpty.Details["count"] = empty
|
||||
out = append(out, *firstEmpty)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// hasEmptyHeading reports whether the record's own headings hold an empty
|
||||
// one of level.
|
||||
func hasEmptyHeading(rec *Record, level int) bool {
|
||||
if rec == nil {
|
||||
return false
|
||||
}
|
||||
for _, h := range rec.Headings {
|
||||
if h.Level == level && strings.TrimSpace(h.Text) == "" {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// checkSitemaps compares the sitemaps with what the crawl found:
|
||||
// sitemap_error for a listed address that does not answer 200,
|
||||
// sitemap_noindex for a listed noindex page, sitemap_blocked for a listed
|
||||
// address robots.txt keeps Googlebot from, and sitemap_missing for an
|
||||
// indexable page found only through links.
|
||||
func checkSitemaps(res *Result, env *Env) []Finding {
|
||||
var out []Finding
|
||||
var keys []string
|
||||
for key := range res.Listed {
|
||||
keys = append(keys, key)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
robots := res.Robots
|
||||
for _, key := range keys {
|
||||
file := res.Listed[key]
|
||||
rec := env.record(key)
|
||||
if p := res.Page(key); p != nil && p.Resp != nil {
|
||||
switch {
|
||||
case p.Resp.Err == "canceled":
|
||||
case p.Resp.Err != "" || p.Resp.Status != 200:
|
||||
out = append(out, Finding{Kind: KindSitemapError, URL: key, Locale: locale(rec), Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"status": p.Resp.Status, "file": file}})
|
||||
case env.noindex(p):
|
||||
out = append(out, Finding{Kind: KindSitemapNoindex, URL: key, Locale: locale(rec), Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"file": file}})
|
||||
}
|
||||
}
|
||||
if robots != "" && res.fetcher != nil {
|
||||
m := grobotstxt.NewRobotsMatcher()
|
||||
if !m.AgentAllowed(robots, "Googlebot", res.fetcher.Absolute(key)) {
|
||||
line := m.MatchingLine()
|
||||
rule := ""
|
||||
if lines := strings.Split(robots, "\n"); line > 0 && line <= len(lines) {
|
||||
rule = strings.TrimSpace(lines[line-1])
|
||||
}
|
||||
out = append(out, Finding{Kind: KindSitemapBlocked, URL: key, Locale: locale(rec), Record: rec, Tool: ToolUpdateRobots, Details: map[string]any{"line": line, "rule": rule, "file": file}})
|
||||
}
|
||||
}
|
||||
}
|
||||
links := inbound(res)
|
||||
for _, key := range res.Order {
|
||||
p := res.Pages[key]
|
||||
if _, listed := res.Listed[key]; listed || !env.indexable(res, p) {
|
||||
continue
|
||||
}
|
||||
rec := env.record(key)
|
||||
f := Finding{Kind: KindSitemapMissing, URL: key, Locale: locale(rec), Record: rec, Tool: ToolCreatePage, Details: map[string]any{"count": len(links[key])}}
|
||||
if rec != nil {
|
||||
f.Tool = ToolSetURLSEO
|
||||
}
|
||||
out = append(out, f)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// inbound maps each address to the pages linking to it (each once, a page
|
||||
// linking to itself left out), in crawl order.
|
||||
func inbound(res *Result) map[string][]string {
|
||||
out := map[string][]string{}
|
||||
if res.fetcher == nil {
|
||||
return out
|
||||
}
|
||||
for _, from := range res.Order {
|
||||
p := res.Pages[from]
|
||||
if p.Facts == nil {
|
||||
continue
|
||||
}
|
||||
seen := map[string]bool{}
|
||||
for _, l := range p.Facts.Links {
|
||||
to := res.fetcher.Key(l.URL)
|
||||
if to == "" || to == from || seen[to] {
|
||||
continue
|
||||
}
|
||||
seen[to] = true
|
||||
out[to] = append(out[to], from)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// checkDuplicates reports titles and descriptions shared by indexable
|
||||
// pages of the crawl (compared without case and extra spaces).
|
||||
func checkDuplicates(res *Result, env *Env) []Finding {
|
||||
titles := map[string][]string{}
|
||||
descs := map[string][]string{}
|
||||
for _, key := range res.Order {
|
||||
p := res.Pages[key]
|
||||
if !env.indexable(res, p) {
|
||||
continue
|
||||
}
|
||||
if t := normal(p.Facts.Title); t != "" {
|
||||
titles[t] = append(titles[t], key)
|
||||
}
|
||||
if d := normal(p.Facts.Description); d != "" {
|
||||
descs[d] = append(descs[d], key)
|
||||
}
|
||||
}
|
||||
var out []Finding
|
||||
emit := func(groups map[string][]string, kind string) {
|
||||
var values []string
|
||||
for v := range groups {
|
||||
values = append(values, v)
|
||||
}
|
||||
sort.Strings(values)
|
||||
for _, v := range values {
|
||||
list := groups[v]
|
||||
if len(list) < 2 {
|
||||
continue
|
||||
}
|
||||
for _, key := range list {
|
||||
others := make([]string, 0, len(list)-1)
|
||||
for _, o := range list {
|
||||
if o != key {
|
||||
others = append(others, o)
|
||||
}
|
||||
}
|
||||
rec := env.record(key)
|
||||
out = append(out, Finding{Kind: kind, URL: key, Locale: locale(rec), Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"value": v, "count": len(others), "also": others}})
|
||||
}
|
||||
}
|
||||
}
|
||||
emit(titles, KindDuplicateTitle)
|
||||
emit(descs, KindDuplicateDesc)
|
||||
return out
|
||||
}
|
||||
|
||||
@@ -221,7 +221,7 @@ func finish(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.C
|
||||
if err := SavePages(tx, row.ID, res); err != nil {
|
||||
return err
|
||||
}
|
||||
findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx)})
|
||||
findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx), Staging: svc.Staging()})
|
||||
cms, err := svc.Issues(classes.WithTx(ctx, tx))
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
Reference in New Issue
Block a user