diff --git a/classes/csv_test.go b/classes/csv_test.go new file mode 100644 index 0000000..2deca90 --- /dev/null +++ b/classes/csv_test.go @@ -0,0 +1,118 @@ +package classes + +import ( + "errors" + "reflect" + "strings" + "testing" +) + +// TestCSV: the export writes a BOM, the delimiter and escaped formula +// cells; the import strips the BOM, detects or takes the delimiter, reads +// quoting, unescapes formula cells and refuses what is not CSV. +func TestCSV(t *testing.T) { + header := []string{"source", "target", "note"} + rows := [][]string{ + {"/a", "/b", "plain"}, + {"/ł", "/ż", "zażółć; gęślą, jaźń"}, + {"/q", "/r", `she said "hi"` + "\nnext line"}, + {"/f", "/g", "=HYPERLINK(\"x\")"}, + {"/p", "/s", "+1"}, + {"/m", "/n", "-1"}, + {"/at", "/x", "@SUM(A1)"}, + {"/tab", "/x", "\tlead"}, + {"/cr", "/x", "\rlead"}, + {"/ok", "/x", "a=b"}, + } + + t.Run("round trip with each delimiter", func(t *testing.T) { + for _, delim := range []string{",", ";", ""} { + out, err := WriteCSV(header, rows, delim) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(out, BOM) { + t.Fatalf("%q: no BOM", delim) + } + sep := "," + if delim == ";" { + sep = ";" + } + if !strings.HasPrefix(strings.TrimPrefix(out, BOM), strings.Join(header, sep)+"\n") { + t.Fatalf("%q: header line %q", delim, strings.SplitN(out, "\n", 2)[0]) + } + gotHeader, got, err := ReadCSV(out, delim) + if err != nil || !reflect.DeepEqual(gotHeader, header) || len(got) != len(rows) { + t.Fatalf("%q: ReadCSV = %v, %d rows, %v", delim, gotHeader, len(got), err) + } + for i, row := range rows { + // Escaped cells keep their leading tab or CR: the apostrophe + // protects them from the import's trimming. + for j, col := range header { + if want := row[j]; got[i].Fields[col] != want { + t.Fatalf("%q row %d %s = %q, want %q", delim, i, col, got[i].Fields[col], want) + } + } + } + } + }) + + t.Run("formula cells are escaped on export", func(t *testing.T) { + out, _ := WriteCSV([]string{"note"}, [][]string{{"=1+1"}, {"+x"}, {"-x"}, {"@x"}, {"a=b"}, {""}}, ",") + lines := strings.Split(strings.TrimPrefix(out, BOM), "\n") + if lines[1] != "'=1+1" || lines[2] != "'+x" || lines[3] != "'-x" || lines[4] != "'@x" || lines[5] != "a=b" { + t.Fatalf("export = %q", lines) + } + for in, want := range map[string]string{"'=x": "=x", "'+x": "+x", "'abc": "'abc", "''=x": "''=x", "plain": "plain", "'": "'"} { + if got := unescapeCell(in); got != want { + t.Errorf("unescapeCell(%q) = %q, want %q", in, got, want) + } + } + }) + + t.Run("delimiter detection and the setting", func(t *testing.T) { + if CSVDelimiter(";") != ';' || CSVDelimiter(" ; ") != ';' || CSVDelimiter(",") != ',' || CSVDelimiter("") != ',' || CSVDelimiter("|") != ',' { + t.Fatal("CSVDelimiter") + } + h, rows, err := ReadCSV("Source;Target\n/a;/b\n", "") + if err != nil || !reflect.DeepEqual(h, []string{"source", "target"}) || rows[0].Fields["target"] != "/b" { + t.Fatalf("detected ; = %v %+v %v", h, rows, err) + } + h, rows, err = ReadCSV(BOM+" SOURCE , Target \n/a,/b\n", "") + if err != nil || !reflect.DeepEqual(h, []string{"source", "target"}) || rows[0].Line != 2 { + t.Fatalf("detected , = %v %+v %v", h, rows, err) + } + }) + + t.Run("quoting, blank rows and extra cells", func(t *testing.T) { + data := "source,target,note\n\"/a,1\",\"/b\"\"x\",\"two\nlines\"\n,,\n\n/c,/d,n,extra\n" + _, rows, err := ReadCSV(data, ",") + if err != nil || len(rows) != 2 { + t.Fatalf("rows = %+v, %v", rows, err) + } + if rows[0].Fields["source"] != "/a,1" || rows[0].Fields["target"] != `/b"x` || rows[0].Fields["note"] != "two\nlines" || rows[0].Line != 2 { + t.Fatalf("quoted row = %+v", rows[0]) + } + if rows[1].Fields["source"] != "/c" || len(rows[1].Fields) != 3 || rows[1].Line != 6 { + t.Fatalf("row with an extra cell = %+v", rows[1]) + } + }) + + t.Run("unreadable files", func(t *testing.T) { + for name, data := range map[string]string{ + "empty": "", + "a BOM only": BOM, + "blank": " \n\t\n", + "a bare quote": "source,target\n\"/a,/b\n", + "the wrong delimiter": "source;target\n/a;/b\n", + } { + delim := "" + if name == "the wrong delimiter" { + delim = "," + } + if _, _, err := ReadCSV(data, delim); !errors.Is(err, ErrUnreadable) { + t.Errorf("%s: err = %v", name, err) + } + } + }) +} diff --git a/classes/helpers_test.go b/classes/helpers_test.go new file mode 100644 index 0000000..4a2976d --- /dev/null +++ b/classes/helpers_test.go @@ -0,0 +1,134 @@ +package classes + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/compass" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/golem15/summercms/modules/signpost" +) + +// testService is a service over an application config without a database: +// env is SUMMER_ENV, site the golem15.seo.site_url ("" leaves it unset), +// environ more SUMMER_ variables. +func testService(t *testing.T, env, site string, environ ...string) *Service { + t.Helper() + svc, _ := testApp(t, env, site, environ...) + return svc +} + +func testApp(t *testing.T, env, site string, environ ...string) (*Service, *backpack.App) { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("name: seo-unit\nlocale: en\nfallback_locale: en\n"), 0o644); err != nil { + t.Fatal(err) + } + vars := append([]string{"SUMMER_ENV=" + env}, environ...) + if site != "" { + vars = append(vars, "SUMMER_GOLEM15__SEO__SITE_URL="+site) + } + cfg, err := compass.Open(compass.Options{Dir: dir, Environ: vars}) + if err != nil { + t.Fatal(err) + } + app := backpack.New(cfg) + svc, err := NewService(app) + if err != nil { + t.Fatal(err) + } + return svc, app +} + +// testPage is a record of the pages content type (/pages/:slug): a path +// may carry a /pl prefix; a published record is matched exactly on its own +// slug in the path's locale and inexactly on another of its slugs; a draft +// is never matched. +type testPage struct { + id string + slugs map[string]string // locale -> slug + published bool +} + +type typesPlugin struct{ types []pact.MenuItemType } + +func (typesPlugin) ID() string { return "acme.pages" } +func (p typesPlugin) MenuItemTypes() []pact.MenuItemType { return p.types } + +func testPagesType(pages ...testPage) pact.MenuItemType { + find := func(slug string) (testPage, string, bool) { + for _, p := range pages { + for locale, s := range p.slugs { + if s == slug { + return p, locale, true + } + } + } + return testPage{}, "", false + } + return pact.MenuItemType{ + Key: "acme-page", DefaultPattern: "/pages/:slug", + Resolve: func(context.Context, pact.MenuItemResolve) (*pact.ResolvedMenuItem, error) { return nil, nil }, + Content: &pact.ContentDescriptor{ + Model: "acme_pages", Controller: "acme.pages.pages", + Describe: func(_ context.Context, in pact.ContentDescribe) (*pact.ContentRecord, error) { + for _, p := range pages { + if p.id == in.ID { + rec := &pact.ContentRecord{ID: p.id, Published: p.published} + for locale, slug := range p.slugs { + rec.Locales = append(rec.Locales, pact.ContentLocale{Locale: locale, URL: "/pages/" + slug, Title: "Page " + p.id}) + } + return rec, nil + } + } + return nil, nil + }, + Match: func(_ context.Context, in pact.ContentMatch) (*pact.ContentMatchResult, error) { + locale, path := "en", in.Path + if rest, ok := strings.CutPrefix(path, "/pl/"); ok { + locale, path = "pl", "/"+rest + } + values, ok := signpost.MatchPattern(in.Pattern, path) + if !ok { + return nil, nil + } + p, own, ok := find(values["slug"]) + if !ok || !p.published { + return nil, nil + } + return &pact.ContentMatchResult{ID: p.id, Locale: locale, Exact: own == locale}, nil + }, + }, + } +} + +// testServiceWithPages is testService with the pages content type +// registered in signpost. +func testServiceWithPages(t *testing.T, site string, pages ...testPage) *Service { + t.Helper() + svc, app := testApp(t, "production", site) + if err := signpost.Activate(app, []typesPlugin{{types: []pact.MenuItemType{testPagesType(pages...)}}}); err != nil { + t.Fatal(err) + } + return svc +} + +// dryDB is a Postgres-dialect GORM handle that never connects: every query +// is built and not run, so reads find no rows. +func dryDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(postgres.New(postgres.Config{DSN: "host=127.0.0.1 port=1 user=seo dbname=seo sslmode=disable"}), + &gorm.Config{DryRun: true, DisableAutomaticPing: true, Logger: logger.Discard}) + if err != nil { + t.Fatal(err) + } + return db +} diff --git a/classes/issues_test.go b/classes/issues_test.go new file mode 100644 index 0000000..10e88cf --- /dev/null +++ b/classes/issues_test.go @@ -0,0 +1,149 @@ +package classes + +import ( + "context" + "errors" + "fmt" + "reflect" + "testing" + + "git.golem15.com/golem15/sm-seo-plugin/models" +) + +// TestIssues: each kind of the CMS-data issue report from loaded data +// (the database reads of Issues are covered by TestIssues in the plugin +// package). +func TestIssues(t *testing.T) { + svc := testServiceWithPages(t, "https://example.test", + testPage{id: "1", slugs: map[string]string{"en": "a"}, published: true}, + testPage{id: "2", slugs: map[string]string{"en": "b"}, published: true}, + ) + entry := func(url, locale, title, desc string) models.Entry { + return models.Entry{URL: url, Locale: locale, EffectiveTitle: title, EffectiveDescription: desc} + } + canonicals := map[string][2]string{ + "/noindexed": {IssueCanonicalNoindex, ""}, + "/moved": {IssueCanonicalRedirected, "/pages/a"}, + } + canonical := func(_ context.Context, value string) (string, string, error) { + if value == "/boom" { + return "", "", errors.New("resolver down") + } + got := canonicals[value] + return got[0], got[1], nil + } + data := issueData{ + locale: "en", + entries: []models.Entry{ + func() models.Entry { + e := entry("/pages/a", "en", "Same Title", "First") + e.Canonical = "/moved" + return e + }(), + entry("/pages/b", "en", "same title ", ""), + entry("/pages/c", "en", "", "Shared description"), + entry("/pages/c", "pl", "Same Title", "Shared description"), + entry("/pages/d", "en", "Unique", "shared DESCRIPTION"), + }, + rules: []models.Rule{ + {ID: 7, URL: "/pages/a", Match: models.MatchExact, Robots: "noindex"}, + {ID: 8, URL: "/landing", Match: models.MatchExact, Title: "Unique", Canonical: "/noindexed"}, + {ID: 9, URL: "/pages/*", Match: models.MatchPattern, Title: "Same Title"}, + {ID: 10, URL: "/pages/b?x=1", Match: models.MatchExact, Title: "Query rule"}, + {ID: 11, URL: "/pages/b", Match: models.MatchExact}, + }, + redirects: []models.Redirect{rd(1, "/old1", "/old2"), rd(2, "/old2", "/pages/a"), rd(3, "/direct", "/pages/a")}, + robots: models.Robots{Content: "User-agent: *\nDisallow: /pages/a\n"}, + noindex: []string{"/pages/a", "/landing"}, + } + issues, err := svc.issuesOf(t.Context(), data, canonical) + if err != nil { + t.Fatal(err) + } + got := map[string]Issue{} + var order []string + for _, is := range issues { + key := is.Kind + " " + is.URL + " " + is.Locale + if _, dup := got[key]; dup { + t.Fatalf("issue listed twice: %s", key) + } + got[key] = is + order = append(order, key) + } + want := []string{ + "canonical_noindex /landing ", + "canonical_redirected /pages/a en", + "duplicate_description /pages/c en", + "duplicate_description /pages/d en", + "duplicate_title /landing ", + "duplicate_title /pages/a en", + "duplicate_title /pages/b en", + "duplicate_title /pages/d en", + "missing_description /pages/b en", + "missing_title /pages/c en", + "noindex_blocked /pages/a ", + "redirect_chain /old1 ", + "rule_overrides_record /pages/a en", + } + if !reflect.DeepEqual(order, want) { + t.Fatalf("issues (sorted by kind, url, locale):\n%v\nwant\n%v", order, want) + } + tools := map[string]string{ + "canonical_noindex /landing ": "seo_update_rule", "canonical_redirected /pages/a en": "seo_set_url_seo", + "missing_title /pages/c en": "seo_set_url_seo", "noindex_blocked /pages/a ": "seo_update_robots", + "redirect_chain /old1 ": "seo_update_redirect", "rule_overrides_record /pages/a en": "seo_update_rule", + } + for key, tool := range tools { + if got[key].Tool != tool { + t.Errorf("%s tool = %s, want %s", key, got[key].Tool, tool) + } + } + if d := got["duplicate_title /pages/a en"].Details; d["value"] != "same title" || fmt.Sprint(d["also"]) != "[/pages/b]" { + t.Errorf("duplicate_title details = %v", d) + } + if d := got["duplicate_title /landing "].Details; fmt.Sprint(d["also"]) != "[/pages/d]" { + t.Errorf("a rule's title joins the default locale's duplicates: %v", d) + } + if d := got["rule_overrides_record /pages/a en"].Details; d["rule_id"] != uint(7) || d["type"] != "acme-page" || d["id"] != "1" || fmt.Sprint(d["fields"]) != "[robots]" { + t.Errorf("rule_overrides_record details = %v", d) + } + if d := got["redirect_chain /old1 "].Details; d["hops"] != 2 || d["final"] != "/pages/a" || fmt.Sprint(d["path"]) != "[/old1 /old2 /pages/a]" { + t.Errorf("redirect_chain details = %v", d) + } + if d := got["canonical_redirected /pages/a en"].Details; d["canonical"] != "/moved" || d["target"] != "/pages/a" { + t.Errorf("canonical_redirected details = %v", d) + } + if d := got["canonical_noindex /landing "].Details; d["rule_id"] != uint(8) || d["canonical"] != "/noindexed" { + t.Errorf("canonical_noindex details = %v", d) + } + + t.Run("a clean site has no issues", func(t *testing.T) { + clean := issueData{locale: "en", entries: []models.Entry{entry("/pages/a", "en", "A", "About A"), entry("/pages/b", "en", "B", "About B")}} + if issues, err := svc.issuesOf(t.Context(), clean, canonical); err != nil || len(issues) != 0 { + t.Fatalf("issues = %+v, %v", issues, err) + } + }) + + t.Run("the same text in two locales is not a duplicate", func(t *testing.T) { + two := issueData{locale: "en", entries: []models.Entry{entry("/pages/a", "en", "Name", "Text"), entry("/pl/pages/a", "pl", "Name", "Text")}} + if issues, _ := svc.issuesOf(t.Context(), two, canonical); len(issues) != 0 { + t.Fatalf("issues = %+v", issues) + } + }) + + t.Run("a canonical check that fails stops the report", func(t *testing.T) { + broken := issueData{locale: "en", entries: []models.Entry{func() models.Entry { e := entry("/pages/a", "en", "A", "B"); e.Canonical = "/boom"; return e }()}} + if _, err := svc.issuesOf(t.Context(), broken, canonical); err == nil { + t.Fatal("the resolver error was dropped") + } + }) + + t.Run("canonicals on other sites and empty canonicals are not checked", func(t *testing.T) { + for _, value := range []string{"", " ", "https://other.test/a"} { + kind, target, err := svc.canonicalProblem(t.Context(), value) + if kind != "" || target != "" || err != nil { + t.Errorf("canonicalProblem(%q) = %q %q %v", value, kind, target, err) + } + } + }) +} diff --git a/classes/normalize_test.go b/classes/normalize_test.go new file mode 100644 index 0000000..ddae863 --- /dev/null +++ b/classes/normalize_test.go @@ -0,0 +1,121 @@ +package classes + +import ( + "net/url" + "testing" + + "git.golem15.com/golem15/sm-seo-plugin/models" +) + +// TestNormalizeMatrix: every combination of the address rules on every +// input form reaches its final address in one hop (normalising the result +// changes nothing), never touches the query, keeps the root and does not +// modify its input. +func TestNormalizeMatrix(t *testing.T) { + inputs := []string{ + "/", "/a", "/a/", "/A/b", "//a//b/", "/a/index.html", "/A/Index.PHP", "/index.htm", + "/sitemap.xml", "/files/Doc.PDF/", "/%C5%81%C3%B3d%C5%BA/Abc/", "/a?Page=2&x=Y", "/a#frag", + "http://example.test", "http://example.test/A//B/", "https://www.example.test/a/index.html?q=1", + "https://WWW.Example.Test/Blog/", "http://www.example.test/", + } + var all []NormalizeOptions + for _, host := range []string{models.HostKeep, models.HostWWW, models.HostBare} { + for _, slash := range []string{models.SlashKeep, models.SlashAdd, models.SlashDrop} { + for _, lower := range []bool{false, true} { + all = append(all, NormalizeOptions{Host: host, Slash: slash, Lowercase: lower}) + } + } + } + for _, opts := range all { + for _, in := range inputs { + u, err := url.Parse(in) + if err != nil { + t.Fatal(err) + } + before := *u + once, _ := Normalize(u, opts) + if *u != before { + t.Fatalf("%+v: Normalize(%s) modified its input", opts, in) + } + twice, changed := Normalize(once, opts) + if changed || twice.String() != once.String() { + t.Fatalf("%+v: %s -> %s -> %s (changed %v): not one hop", opts, in, once, twice, changed) + } + if once.RawQuery != u.RawQuery { + t.Fatalf("%+v: %s: query %q became %q", opts, in, u.RawQuery, once.RawQuery) + } + if once.Fragment != "" { + t.Fatalf("%+v: %s kept its fragment", opts, in) + } + if once.Host != "" && once.Scheme != "https" { + t.Fatalf("%+v: %s scheme %s", opts, in, once.Scheme) + } + if u.Path == "/" && once.Path != "/" { + t.Fatalf("%+v: the root became %s", opts, once.Path) + } + } + } +} + +func TestNormalizeMatrixExpectations(t *testing.T) { + for _, tc := range []struct { + in string + opts NormalizeOptions + want string + }{ + {"/A//B/index.html", NormalizeOptions{Host: models.HostKeep, Slash: models.SlashKeep}, "/A/B"}, + {"/A//B/index.html", NormalizeOptions{Slash: models.SlashAdd, Lowercase: true}, "/a/b/"}, + {"/a/b///", NormalizeOptions{Slash: models.SlashDrop}, "/a/b"}, + {"/a/b///", NormalizeOptions{Slash: models.SlashKeep}, "/a/b/"}, + {"/report.PDF", NormalizeOptions{Slash: models.SlashAdd, Lowercase: true}, "/report.pdf"}, + {"/v1.2/page", NormalizeOptions{Slash: models.SlashAdd}, "/v1.2/page/"}, + {"http://EXAMPLE.test/a", NormalizeOptions{Host: models.HostKeep}, "https://example.test/a"}, + {"http://www.example.test/a/", NormalizeOptions{Host: models.HostBare, Slash: models.SlashDrop}, "https://example.test/a"}, + {"https://example.test", NormalizeOptions{Host: models.HostWWW, Slash: models.SlashAdd}, "https://www.example.test/"}, + {"/%c5%81/%C5%82X", NormalizeOptions{Lowercase: true}, "/%C5%81/%C5%82x"}, + } { + u, _ := url.Parse(tc.in) + got, _ := Normalize(u, tc.opts) + if got.String() != tc.want { + t.Errorf("Normalize(%s, %+v) = %s, want %s", tc.in, tc.opts, got, tc.want) + } + } + if got, changed := Normalize(nil, NormalizeOptions{}); got != nil || changed { + t.Fatal("Normalize(nil) answered a URL") + } + if got := NormalizeOptionsOf(models.Settings{URLHost: models.HostWWW, URLSlash: models.SlashAdd, URLLowercase: true}); got != (NormalizeOptions{Host: models.HostWWW, Slash: models.SlashAdd, Lowercase: true}) { + t.Fatalf("NormalizeOptionsOf = %+v", got) + } +} + +func TestNormalizeHelpers(t *testing.T) { + for in, want := range map[string]string{"/": "/", "//": "/", "/a//b": "/a/b", "///a///": "/a/", "/a/b": "/a/b"} { + if got := collapseSlashes(in); got != want { + t.Errorf("collapseSlashes(%q) = %q", in, got) + } + } + if got := lowerASCII("/ŁÓDŹ/ABC-z"); got != "/ŁÓdŹ/abc-z" { + t.Errorf("lowerASCII = %q", got) + } + for in, want := range map[string]string{"/a/b.c": "b.c", "/a/": "", "x": "x", "/": ""} { + if got := lastSegment(in); got != want { + t.Errorf("lastSegment(%q) = %q", in, got) + } + } + for _, tc := range []struct { + site, host string + same bool + }{ + {"https://example.test", "example.test", true}, + {"https://example.test", "WWW.example.test", true}, + {"https://www.example.test", "example.test", true}, + {"https://example.test", "evil.test", false}, + {"https://example.test", "", false}, + {"", "example.test", false}, + {"https://example.test", "example.test.evil", false}, + } { + if got := sameSite(tc.site, tc.host); got != tc.same { + t.Errorf("sameSite(%q, %q) = %v", tc.site, tc.host, got) + } + } +} diff --git a/classes/owner_test.go b/classes/owner_test.go new file mode 100644 index 0000000..5933f42 --- /dev/null +++ b/classes/owner_test.go @@ -0,0 +1,74 @@ +package classes + +import ( + "errors" + "testing" +) + +// TestOwnerResolution: a URL's SEO belongs to the record behind it in the +// matched locale when the URL is the record's exact address without a +// query; otherwise, and with force_rule, to an exact URL rule on the +// normalised address (D-11, P-25). +func TestOwnerResolution(t *testing.T) { + svc := testServiceWithPages(t, "https://example.test", + testPage{id: "1", slugs: map[string]string{"en": "about", "pl": "o-nas"}, published: true}, + testPage{id: "2", slugs: map[string]string{"en": "draft"}, published: false}, + ) + db := dryDB(t) + for _, tc := range []struct { + name, url string + force bool + writtenTo string + id string + locale string + address string + published bool + }{ + {"the record's own address", "/pages/about", false, WrittenToRecord, "1", "en", "/pages/about", true}, + {"an absolute URL of the site", "https://example.test/pages/about", false, WrittenToRecord, "1", "en", "/pages/about", true}, + {"the locale prefix names the locale", "/pl/pages/o-nas", false, WrittenToRecord, "1", "pl", "/pl/pages/o-nas", true}, + {"a trailing slash still finds the record", "/pages/about/", false, WrittenToRecord, "1", "en", "/pages/about/", true}, + {"double slashes are normalised first", "//pages//about", false, "", "", "", "", false}, + {"force_rule writes a rule on the record's address", "/pages/about", true, WrittenToRule, "", "", "/pages/about", false}, + {"a query makes it a rule", "/pages/about?page=2", false, WrittenToRule, "", "", "/pages/about?page=2", false}, + {"another locale's slug is not the record's address", "/pages/o-nas", false, WrittenToRule, "", "", "/pages/o-nas", false}, + {"a draft is not matched", "/pages/draft", false, WrittenToRule, "", "", "/pages/draft", false}, + {"an address without a record", "/landing", false, WrittenToRule, "", "", "/landing", false}, + {"the home page", "/", false, WrittenToRule, "", "", "/", false}, + {"an index file is normalised away", "/pages/about/index.html", false, WrittenToRecord, "1", "en", "/pages/about", true}, + } { + t.Run(tc.name, func(t *testing.T) { + owner, typ, err := svc.FindOwner(t.Context(), db, tc.url, tc.force) + if tc.writtenTo == "" { + if !errors.Is(err, ErrBadURL) { + t.Fatalf("FindOwner(%q) = %+v, %v; want ErrBadURL", tc.url, owner, err) + } + return + } + if err != nil { + t.Fatal(err) + } + if owner.WrittenTo != tc.writtenTo || owner.ID != tc.id || owner.Locale != tc.locale || owner.URL != tc.address || owner.Published != tc.published { + t.Fatalf("FindOwner(%q) = %+v", tc.url, owner) + } + if tc.writtenTo == WrittenToRecord && (owner.Type != "acme-page" || typ.Key != "acme-page") { + t.Fatalf("type = %q %q", owner.Type, typ.Key) + } + if tc.writtenTo == WrittenToRule && (owner.Type != "" || typ.Key != "" || owner.RuleID != 0) { + t.Fatalf("rule owner = %+v (%s)", owner, typ.Key) + } + }) + } + for _, bad := range []string{"", "pages/about", "ftp://example.test/a", "javascript:alert(1)"} { + if _, _, err := svc.FindOwner(t.Context(), db, bad, false); !errors.Is(err, ErrBadURL) { + t.Errorf("FindOwner(%q) = %v, want ErrBadURL", bad, err) + } + } + + t.Run("without a signpost registry every URL is a rule", func(t *testing.T) { + owner, _, err := testService(t, "production", "https://example.test").FindOwner(t.Context(), db, "/pages/about", false) + if err != nil || owner.WrittenTo != WrittenToRule || owner.URL != "/pages/about" { + t.Fatalf("owner = %+v, %v", owner, err) + } + }) +} diff --git a/classes/redirect_graph_test.go b/classes/redirect_graph_test.go new file mode 100644 index 0000000..0b5e2cb --- /dev/null +++ b/classes/redirect_graph_test.go @@ -0,0 +1,184 @@ +package classes + +import ( + "strconv" + "strings" + "testing" + + "git.golem15.com/golem15/sm-seo-plugin/models" +) + +// rd is an exact 301 rule with defaults. +func rd(id uint, source, target string) models.Redirect { + r := models.NewRedirect() + r.ID, r.Source, r.Target = id, source, target + return r +} + +func rdMatch(id uint, match, source, target string) models.Redirect { + r := rd(id, source, target) + r.Match = match + return r +} + +const rdSite = "https://example.test" + +// TestRedirectGraph: the redirect graph rules of D-18/D-19 over rule lists +// (the database writes of AddressChanged are covered by TestSlugChange301 +// and TestAddressChangeFromEvent in the plugin package). +func TestRedirectGraph(t *testing.T) { + t.Run("a chain A to B to C is a chain notice from A to C", func(t *testing.T) { + stored := []models.Redirect{rd(1, "/b", "/c")} + check, err := checkRedirectRules(rdSite, stored, rd(0, "/a", "/b")) + if err != nil || check.Loop || !check.Chain || check.Hops != 2 || check.Source != "/a" || check.Final != "/c" || check.PathText() != "/a → /b → /c" { + t.Fatalf("check = %+v, %v", check, err) + } + notice := ChainNotice(check) + if len(notice) != 1 || notice[0].Params["count"] != "2" || notice[0].Params["final"] != "/c" || notice[0].Params["source"] != "/a" { + t.Fatalf("notice = %+v", notice) + } + }) + + t.Run("a rule that another rule leads into is the second hop of a chain", func(t *testing.T) { + stored := []models.Redirect{rd(1, "/a", "/b")} + check, err := checkRedirectRules(rdSite, stored, rd(0, "/b", "/c")) + if err != nil || !check.Chain || check.Hops != 2 || check.Source != "/a" || check.Final != "/c" { + t.Fatalf("check = %+v, %v", check, err) + } + }) + + t.Run("the flattened graph A to C, B to C is one hop each", func(t *testing.T) { + set := NewRedirectSet([]models.Redirect{rd(1, "/a", "/c"), rd(2, "/b", "/c")}) + for _, from := range []string{"/a", "/b"} { + ch, err := set.Follow(rdSite, from, "", MaxRedirectHops) + if err != nil || len(ch.Hops) != 1 || ch.Final != "/c" || ch.Loop { + t.Fatalf("%s: chain = %+v, %v", from, ch, err) + } + } + for _, r := range []models.Redirect{rd(1, "/a", "/c"), rd(2, "/b", "/c")} { + others := []models.Redirect{rd(1, "/a", "/c"), rd(2, "/b", "/c")} + if check, err := checkRedirectRules(rdSite, others, r); err != nil || check.Chain || check.Loop { + t.Fatalf("%s: check = %+v, %v", r.Source, check, err) + } + } + }) + + t.Run("back and forth A to B to A to C ends as A to C and B to C", func(t *testing.T) { + // After A→B, B→A (the address returned) and then A→C, the graph + // AddressChanged leaves is A→C and B→C: no rule shadows the live + // address and every old address reaches C in one hop. + set := NewRedirectSet([]models.Redirect{rd(1, "/a", "/c"), rd(2, "/b", "/c")}) + for _, from := range []string{"/a", "/b"} { + if ch, _ := set.Follow(rdSite, from, "", MaxRedirectHops); len(ch.Hops) != 1 || ch.Final != "/c" { + t.Fatalf("%s: chain = %+v", from, ch) + } + } + if _, ok, _ := set.Match("/c", ""); ok { + t.Fatal("the live address is shadowed") + } + // Back and forth without the cleanup would be a loop. + check, err := checkRedirectRules(rdSite, []models.Redirect{rd(1, "/a", "/b")}, rd(0, "/b", "/a")) + if err != nil || !check.Loop { + t.Fatalf("A→B then B→A = %+v, %v; want a loop", check, err) + } + }) + + loops := []struct { + name string + stored []models.Redirect + rule models.Redirect + path string + }{ + {"exact self loop", nil, rd(0, "/a", "/a"), "/a → /a"}, + {"exact two-rule loop", []models.Redirect{rd(1, "/b", "/a")}, rd(0, "/a", "/b"), "/a → /b → /a"}, + {"exact three-rule loop", []models.Redirect{rd(1, "/b", "/c"), rd(2, "/c", "/a")}, rd(0, "/a", "/b"), "/a → /b → /c → /a"}, + {"exact loop through the site's absolute URL", []models.Redirect{rd(1, "/b", rdSite+"/a")}, rd(0, "/a", "/b"), "/a → /b → /a"}, + {"exact loop through case and trailing slash", []models.Redirect{rd(1, "/b", "/A/")}, func() models.Redirect { r := rd(0, "/a", "/b"); r.IgnoreCase = true; return r }(), ""}, + {"pattern probe loop", []models.Redirect{rd(1, "/new/seoprobe", "/old/x")}, rdMatch(0, models.RedirectPattern, "/old/*", "/new/*"), ""}, + {"pattern onto itself", nil, rdMatch(0, models.RedirectPattern, "/docs/*", "/docs/*"), ""}, + {"regex probe loop", []models.Redirect{rdMatch(1, models.RedirectPattern, "/b/*", "/a/*")}, rdMatch(0, models.RedirectRegex, `^/a/(\d+)$`, "/b/$1"), `^/a/(\d+)$ → /b/0 → /a/0`}, + {"regex onto itself", nil, rdMatch(0, models.RedirectRegex, `^/x/([a-z]+)$`, "/x/$1"), ""}, + } + for _, tc := range loops { + t.Run("loop refused: "+tc.name, func(t *testing.T) { + check, err := checkRedirectRules(rdSite, tc.stored, tc.rule) + if err != nil || !check.Loop { + t.Fatalf("check = %+v, %v; want a loop", check, err) + } + if tc.path != "" && check.PathText() != tc.path { + t.Fatalf("path = %q, want %q", check.PathText(), tc.path) + } + if msg := (&LoopError{Check: check}).Error(); !strings.Contains(msg, "loop") { + t.Fatalf("LoopError = %q", msg) + } + }) + } + + t.Run("a 410 rule ends every chain and is never a loop", func(t *testing.T) { + gone := rd(0, "/a", "") + gone.Status = models.Status410 + if check, err := checkRedirectRules(rdSite, []models.Redirect{rd(1, "/b", "/a")}, gone); err != nil || check.Loop || check.Chain { + t.Fatalf("check = %+v, %v", check, err) + } + set := NewRedirectSet([]models.Redirect{rd(1, "/b", "/a"), func() models.Redirect { g := gone; g.ID = 2; return g }()}) + ch, err := set.Follow(rdSite, "/b", "", MaxRedirectHops) + if err != nil || !ch.Gone || ch.Loop || len(ch.Hops) != 2 { + t.Fatalf("chain = %+v, %v", ch, err) + } + }) + + t.Run("an external target ends the chain", func(t *testing.T) { + set := NewRedirectSet([]models.Redirect{rd(1, "/a", "https://other.test/a"), rd(2, "/b", "/a")}) + ch, err := set.Follow(rdSite, "/b", "", MaxRedirectHops) + if err != nil || ch.Loop || len(ch.Hops) != 2 || ch.Final != "https://other.test/a" { + t.Fatalf("chain = %+v, %v", ch, err) + } + }) + + t.Run("the runtime stops after 10 hops", func(t *testing.T) { + var rules []models.Redirect + for i := 0; i < 15; i++ { + rules = append(rules, rd(uint(i+1), "/p"+strconv.Itoa(i), "/p"+strconv.Itoa(i+1))) + } + set := NewRedirectSet(rules) + ch, err := set.Follow(rdSite, "/p0", "", MaxRedirectHops) + if err != nil || !ch.Loop || len(ch.Hops) != MaxRedirectHops || MaxRedirectHops != 10 { + t.Fatalf("chain = %d hops, loop %v, %v", len(ch.Hops), ch.Loop, err) + } + // Reaching the cap counts as a loop; a chain one hop shorter ends. + capped, _ := set.Follow(rdSite, "/p5", "", MaxRedirectHops) + if !capped.Loop || len(capped.Hops) != 10 { + t.Fatalf("ten hops: %d hops, loop %v", len(capped.Hops), capped.Loop) + } + short, err := set.Follow(rdSite, "/p6", "", MaxRedirectHops) + if err != nil || short.Loop || len(short.Hops) != 9 || short.Final != "/p15" { + t.Fatalf("nine hops: %d hops, loop %v, final %s", len(short.Hops), short.Loop, short.Final) + } + // A new rule that makes a chain longer than the cap is a loop. + check, err := checkRedirectRules(rdSite, rules[1:], rd(0, "/p0", "/p1")) + if err != nil || !check.Loop { + t.Fatalf("an 11-hop chain = %+v, %v; want refused", check, err) + } + }) + + t.Run("probes", func(t *testing.T) { + for _, tc := range []struct { + rule models.Redirect + path string + ok bool + }{ + {rd(0, "/a?x=1", "/b"), "/a", true}, + {rdMatch(0, models.RedirectPattern, "/old/*/x", "/n"), "/old/seoprobe/x", true}, + {rdMatch(0, models.RedirectRegex, `^/a/[0-9]{2}/(x|y)$`, "/n"), "/a/00/x", true}, + {rdMatch(0, models.RedirectRegex, `^/a/\w+$`, "/n"), "/a/a", true}, + {rdMatch(0, models.RedirectRegex, `^a$`, "/n"), "", false}, + {rdMatch(0, models.RedirectRegex, `(`, "/n"), "", false}, + {rdMatch(0, models.RedirectRegex, `^/a\?q$`, "/n"), "", false}, + } { + path, _, ok := redirectProbe(tc.rule) + if ok != tc.ok || (ok && path != tc.path) { + t.Errorf("redirectProbe(%s %q) = %q %v, want %q %v", tc.rule.Match, tc.rule.Source, path, ok, tc.path, tc.ok) + } + } + }) +} diff --git a/classes/resolver_test.go b/classes/resolver_test.go new file mode 100644 index 0000000..e20d1f5 --- /dev/null +++ b/classes/resolver_test.go @@ -0,0 +1,160 @@ +package classes + +import ( + "testing" + + "git.golem15.com/golem15/sm-seo-plugin/models" +) + +// layer is one layer of a value chain: its source and the value it holds +// when present. +type layer struct{ source, value string } + +// TestResolverPrecedence: every value chain of the resolver (P-14) with +// each layer present and absent picks the first present layer and reports +// its source; robots merge per directive; outside production the lock +// wins; a draft is not found. The Postgres-backed answer with stored rules +// and entries is TestResolverPrecedence in the plugin package. +func TestResolverPrecedence(t *testing.T) { + chains := map[string][]layer{ + // rule > record > type or general template > content > site name + "title": {{SourceRule, "Rule title"}, {SourceRecord, "Record title"}, {SourceTemplate, "Post | Site"}, {SourceContent, "Post"}, {SourceGlobal, "Site"}}, + // rule > record > type template > content > default description + "description": {{SourceRule, "Rule desc"}, {SourceRecord, "Record desc"}, {SourceTemplate, "Template desc"}, {SourceContent, "Excerpt"}, {SourceGlobal, "Default desc"}}, + // rule > record (else the address itself) + "canonical": {{SourceRule, "/rule"}, {SourceRecord, "/record"}}, + // override rule > record > the effective title + "og_title": {{SourceRule, "OG rule"}, {SourceRecord, "OG record"}, {SourceTemplate, "Effective title"}}, + // override rule > record > the effective description + "og_description": {{SourceRule, "OG rule"}, {SourceRecord, "OG record"}, {SourceContent, "Effective description"}}, + // rule > record > content image > default social image + "og_image": {{SourceRule, "/r.png"}, {SourceRecord, "/e.png"}, {SourceContent, "/c.png"}, {SourceGlobal, "/d.png"}}, + } + for name, chain := range chains { + t.Run(name, func(t *testing.T) { + for mask := 0; mask < 1< %q", value, got.String()) + } + if got.Header() != strings.ReplaceAll(value, ",", ", ") { + t.Fatalf("Header(%q) = %q", value, got.Header()) + } + if got.Empty() != (value == "") { + t.Fatalf("Empty(%q) = %v", value, got.Empty()) + } + // Any order, spaces and case read the same value back in the + // fixed order. + parts := strings.Split(value, ",") + rng.Shuffle(len(parts), func(i, j int) { parts[i], parts[j] = parts[j], parts[i] }) + for i := range parts { + if i%2 == 0 { + parts[i] = " " + strings.ToUpper(parts[i]) + " " + } + } + if value != "" { + norm, err := NormalizeRobots(strings.Join(parts, ",")) + if err != nil || norm != value { + t.Fatalf("NormalizeRobots(%q) = %q, %v; want %q", strings.Join(parts, ","), norm, err, value) + } + } + } +} + +func TestRobotsValueAllInheritIsEmpty(t *testing.T) { + for _, value := range []string{"", " ", "\t"} { + r, err := ParseRobots(value) + if err != nil || !r.Empty() || r.String() != "" { + t.Fatalf("ParseRobots(%q) = %+v, %v", value, r, err) + } + } + if s := (Robots{}).String(); s != "" { + t.Fatalf("all-inherit serialises to %q", s) + } +} + +func TestRobotsValueMalformed(t *testing.T) { + for _, value := range []string{ + ",", "index,", ",noindex", "index,,follow", + "index,noindex", "follow,nofollow", "noindex,noindex", "noarchive,noarchive", + "max-snippet:1,max-snippet:2", + "index:1", "nofollow:yes", "noarchive:1", + "max-snippet", "max-snippet:", "max-snippet:abc", "max-snippet:-2", "max-snippet:1.5", + "max-video-preview", "max-video-preview:-5", + "max-image-preview", "max-image-preview:huge", "max-image-preview:", + "unavailable_after", "unavailable_after:2027-13-01", "unavailable_after:2027-02-30", + "unavailable_after:27-01-01", "unavailable_after:2027-1-1", + "none", "all", "noodp", "indexx", + } { + if _, err := ParseRobots(value); !errors.Is(err, ErrRobotsValue) { + t.Errorf("ParseRobots(%q) = %v, want ErrRobotsValue", value, err) + } + if _, err := NormalizeRobots(value); err == nil { + t.Errorf("NormalizeRobots(%q) accepted", value) + } + } +} + +func TestRobotsValueInherit(t *testing.T) { + parse := func(v string) Robots { + t.Helper() + r, err := ParseRobots(v) + if err != nil { + t.Fatal(err) + } + return r + } + for _, tc := range []struct{ upper, lower, want string }{ + {"", "", ""}, + {"noindex", "index,nofollow", "noindex,nofollow"}, + {"", "noindex,follow", "noindex,follow"}, + {"index,follow", "noindex,nofollow,noarchive", "index,follow,noarchive"}, + {"max-snippet:10", "max-snippet:50,max-image-preview:large", "max-snippet:10,max-image-preview:large"}, + {"unavailable_after:2027-01-01", "unavailable_after:2028-01-01,max-video-preview:5", "max-video-preview:5,unavailable_after:2027-01-01"}, + {"nosnippet", "noimageindex", "nosnippet,noimageindex"}, + } { + if got := parse(tc.upper).Inherit(parse(tc.lower)).String(); got != tc.want { + t.Errorf("%q over %q = %q, want %q", tc.upper, tc.lower, got, tc.want) + } + } +} diff --git a/classes/templates_test.go b/classes/templates_test.go new file mode 100644 index 0000000..16e74b4 --- /dev/null +++ b/classes/templates_test.go @@ -0,0 +1,75 @@ +package classes + +import ( + "testing" + + "git.golem15.com/golem15/sm-seo-plugin/models" +) + +// TestTemplates: rendering fills the three placeholders, collapses spaces +// and trims separators an empty placeholder leaves; an unknown placeholder +// is refused when the template is saved. +func TestTemplates(t *testing.T) { + vars := TemplateVars{Title: "Grzyby leśne", Description: "O grzybach", Site: "Atlas"} + for _, tc := range []struct { + name, tpl string + vars TemplateVars + want string + }{ + {"title and site", "%title% | %site%", vars, "Grzyby leśne | Atlas"}, + {"description", "%description% – %site%", vars, "O grzybach – Atlas"}, + {"every placeholder twice", "%title% %title% %site%", vars, "Grzyby leśne Grzyby leśne Atlas"}, + {"an empty site leaves no separator", "%title% | %site%", TemplateVars{Title: "About"}, "About"}, + {"an empty title leaves no separator", "%title% · %site%", TemplateVars{Site: "Atlas"}, "Atlas"}, + {"separators of every kind are trimmed", "%title% — : , %site%", TemplateVars{Title: "X"}, "X"}, + {"inner separators stay", "%title% | Blog | %site%", TemplateVars{Title: "X", Site: "Y"}, "X | Blog | Y"}, + {"runs of spaces collapse", " %title% \t %site% ", vars, "Grzyby leśne Atlas"}, + {"an empty template renders nothing", "", vars, ""}, + {"a blank template renders nothing", " ", vars, ""}, + {"text without placeholders", "Static title", vars, "Static title"}, + {"an unknown placeholder renders as written", "%title% %author%", vars, "Grzyby leśne %author%"}, + {"everything empty", "%title% | %site%", TemplateVars{}, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := RenderTemplate(tc.tpl, tc.vars); got != tc.want { + t.Fatalf("RenderTemplate(%q) = %q, want %q", tc.tpl, got, tc.want) + } + }) + } + + for _, tc := range []struct { + tpl, unknown string + }{ + {"%title% | %site%", ""}, + {"%description%", ""}, + {"", ""}, + {"100% sure", ""}, + {"%author% | %site%", "%author%"}, + {"%title% %Title%", "%Title%"}, + {"%%", "%%"}, + {"%site_name%", "%site_name%"}, + {"%title% %date% %page%", "%date%"}, + } { + if got := models.UnknownPlaceholder(tc.tpl); got != tc.unknown { + t.Errorf("UnknownPlaceholder(%q) = %q, want %q", tc.tpl, got, tc.unknown) + } + problems := models.TemplateProblems(tc.tpl) + if tc.unknown == "" && problems != nil { + t.Errorf("TemplateProblems(%q) = %+v", tc.tpl, problems) + } + if tc.unknown != "" && (len(problems) != 1 || problems[0].Key != "golem15.seo::lang.settings.template_unknown" || problems[0].Params["placeholder"] != tc.unknown) { + t.Errorf("TemplateProblems(%q) = %+v", tc.tpl, problems) + } + } + + refused := models.Settings{TitleTemplate: "%title% | %brand%", HomeTemplate: "%site% %x%"}.Problems() + if len(refused["title_template"]) != 1 || len(refused["home_template"]) != 1 { + t.Fatalf("settings problems = %+v", refused) + } + if ok := (models.Settings{TitleTemplate: "%title% | %site%", HomeTemplate: "%site%"}).Problems(); len(ok) != 0 { + t.Fatalf("valid templates refused: %+v", ok) + } + if len(models.KnownPlaceholders) != 3 || PlaceholderTitle != models.KnownPlaceholders[0] || PlaceholderDescription != models.KnownPlaceholders[1] || PlaceholderSite != models.KnownPlaceholders[2] { + t.Fatalf("the rendered and the validated placeholders differ: %v", models.KnownPlaceholders) + } +} diff --git a/go.mod b/go.mod index 0d3ff9b..6c1a05c 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/testcontainers/testcontainers-go v0.44.0 github.com/testcontainers/testcontainers-go/modules/postgres v0.44.0 gocloud.dev v0.46.0 + gorm.io/driver/postgres v1.6.3 gorm.io/gorm v1.31.2 ) @@ -178,7 +179,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20260316180232-0b37fe3546d5 // indirect google.golang.org/grpc v1.79.3 // indirect google.golang.org/protobuf v1.36.11 // indirect - gorm.io/driver/postgres v1.6.3 // indirect ) replace git.golem15.com/golem15/summercms => ../summercms.go