Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions core/models/article.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,25 @@ func LoadArticle(ctx context.Context, db DBorTx, sourceID SourceID, sectionID Ar
return article, nil
}

const sqlSelectReadable = `
SELECT ROW_TO_JSON(r) FROM (
SELECT ` + sqlArticleColumns + `, ` + sqlArticleParent + `
FROM knowledge_article a
JOIN knowledge_article p ON p.id = a.parent_id
WHERE ` + sqlReadable + ` AND p.parent_id IS NULL
ORDER BY p.sort_order, p.title, a.sort_order, a.title
) r;`

// LoadReadable loads every readable article on the site, with its body, in display order - section by section. Only an
// article in a section has a page, so one under another article isn't included.
func LoadReadable(ctx context.Context, db DBorTx, sourceID SourceID) ([]*Article, error) {
articles, err := queryJSON(ctx, db, func() *Article { return &Article{} }, sqlSelectReadable, sourceID)
if err != nil {
return nil, fmt.Errorf("error loading readable articles: %w", err)
}
return articles, nil
}

const sqlSelectReadableByUUID = `
SELECT ROW_TO_JSON(r) FROM (
SELECT ` + sqlArticleColumns + `, ` + sqlArticleParent + `
Expand Down
12 changes: 12 additions & 0 deletions core/models/article_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ func TestArticles(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "/helpsite/preview/getting-started/welcome/", targets[welcomeUUID])

// every readable article, section by section in display order - not one under another article, which has no page
testdb.InsertArticle(t, rt, source, welcome, "Nested", "nested", testdb.ArticleOptions{})

readable, err := models.LoadReadable(ctx, rt.DB, source)
require.NoError(t, err)
require.Len(t, readable, 3)
assert.Equal(t, invoices, readable[0].ID)
assert.Equal(t, "billing", readable[0].Parent.Slug)
assert.Equal(t, welcome, readable[1].ID)
assert.Equal(t, "<h2 id=\"hello\">Hello</h2><p>Welcome to <strong>the</strong> platform &amp; more.</p>", readable[1].BodyHTML)
assert.Equal(t, setup, readable[2].ID)

// readable lookups
byUUID, err := models.LoadReadableByUUID(ctx, rt.DB, source, []string{welcomeUUID, startedUUID})
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/stretchr/testify v1.12.1
github.com/vinovest/sqlx v1.7.2
go.uber.org/zap v1.28.0
golang.org/x/net v0.57.0
)

require (
Expand Down Expand Up @@ -62,7 +63,6 @@ require (
go.yaml.in/yaml/v3 v3.0.5 // indirect
golang.org/x/crypto v0.54.0 // indirect
golang.org/x/mod v0.38.0 // indirect
golang.org/x/net v0.57.0 // indirect
golang.org/x/sync v0.22.0 // indirect
golang.org/x/sys v0.47.0 // indirect
golang.org/x/text v0.41.0 // indirect
Expand Down
7 changes: 5 additions & 2 deletions web/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,15 @@ func (s *Server) requireAuthToken(next http.Handler) http.Handler {
})
}

// the files a site serves at its root, which crawlers look for at exactly these addresses
var rootFiles = map[string]bool{"/robots.txt": true, "/sitemap.xml": true}

// redirectToSlash sends a request for a page without its trailing slash to the address with it, the way the platform
// does - every page's address ends with one, and static files are the only paths that don't
// does - every page's address ends with one, and static and root files are the only paths that don't
func redirectToSlash(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
path := r.URL.Path
if path != "" && !strings.HasSuffix(path, "/") && !strings.HasPrefix(path, "/static/") {
if path != "" && !strings.HasSuffix(path, "/") && !strings.HasPrefix(path, "/static/") && !rootFiles[path] {
target := path + "/"
if r.URL.RawQuery != "" {
target += "?" + r.URL.RawQuery
Expand Down
92 changes: 92 additions & 0 deletions web/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,92 @@ func TestSitePages(t *testing.T) {
assert.Contains(t, body, "This help site isn&#39;t available")
}

func TestSitemap(t *testing.T) {
_, rt := testsuite.Runtime(t)
site, ids := setupSite(t, rt)
h := newServer(t, rt).SiteHandler()

// a draft isn't listed, and an article shows the same image twice, by relative and by absolute address
testdb.InsertArticle(t, rt, site.Source.ID, ids["billing"], "Refunds", "refunds", testdb.ArticleOptions{Draft: true})
drafts := testdb.InsertSection(t, rt, site.Source.ID, "Drafts", "drafts", testdb.ArticleOptions{Draft: true})
testdb.InsertArticle(t, rt, site.Source.ID, drafts, "Hidden", "hidden", testdb.ArticleOptions{}) // in a draft section
testdb.InsertArticle(t, rt, site.Source.ID, ids["billing"], "Receipts", "receipts", testdb.ArticleOptions{
SortOrder: 1,
BodyHTML: `<p><img src="img/receipt.png" alt="a"> <img src="/billing/receipts/img/receipt.png" alt="b">` +
`<img src="https://example.com/x.png"><img src="data:image/png;base64,AAAA"><img alt="none"></p>`,
})

modified := time.Date(2026, 3, 4, 5, 6, 7, 0, time.UTC)
_, err := rt.DB.ExecContext(t.Context(), `UPDATE knowledge_article SET modified_on = $1`, modified)
require.NoError(t, err)
_, err = rt.DB.ExecContext(t.Context(), `UPDATE knowledge_article SET modified_on = $2 WHERE id = $1`, ids["setup"], modified.Add(time.Hour))
require.NoError(t, err)

resp, body := get(t, h, "help.nyaruka.com", "/sitemap.xml", "help.nyaruka.com")
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, "application/xml; charset=utf-8", resp.Header.Get("Content-Type"))
assert.Equal(t, "public, max-age=300", resp.Header.Get("Cache-Control"))
assert.Equal(t, `<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:image="http://www.google.com/schemas/sitemap-image/1.1">
<url>
<loc>https://help.nyaruka.com/</loc>
</url>
<url>
<loc>https://help.nyaruka.com/billing/</loc>
<lastmod>2026-03-04T05:06:07Z</lastmod>
</url>
<url>
<loc>https://help.nyaruka.com/getting-started/</loc>
<lastmod>2026-03-04T06:06:07Z</lastmod>
</url>
<url>
<loc>https://help.nyaruka.com/billing/invoices/</loc>
<lastmod>2026-03-04T05:06:07Z</lastmod>
<image:image>
<image:loc>https://storage.example.com/bucket/orgs/1/knowledge/a%20b.png</image:loc>
</image:image>
<image:image>
<image:loc>https://example.com/x.png</image:loc>
</image:image>
</url>
<url>
<loc>https://help.nyaruka.com/billing/receipts/</loc>
<lastmod>2026-03-04T05:06:07Z</lastmod>
<image:image>
<image:loc>https://help.nyaruka.com/billing/receipts/img/receipt.png</image:loc>
</image:image>
<image:image>
<image:loc>https://example.com/x.png</image:loc>
</image:image>
</url>
<url>
<loc>https://help.nyaruka.com/getting-started/setup/</loc>
<lastmod>2026-03-04T06:06:07Z</lastmod>
</url>
<url>
<loc>https://help.nyaruka.com/getting-started/welcome/</loc>
<lastmod>2026-03-04T05:06:07Z</lastmod>
</url>
</urlset>`, body)

// on the host the site was reached on, however it was asked for
_, body = get(t, h, "www.help.nyaruka.com", "/sitemap.xml", "www.help.nyaruka.com")
assert.Contains(t, body, "<loc>https://www.help.nyaruka.com/billing/invoices/</loc>")

_, body = get(t, h, "Help.Nyaruka.com:443", "/sitemap.xml", "help.nyaruka.com")
assert.Contains(t, body, "<loc>https://help.nyaruka.com/billing/invoices/</loc>")

// and robots.txt points to it
resp, body = get(t, h, "help.nyaruka.com", "/robots.txt", "help.nyaruka.com")
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, "text/plain; charset=utf-8", resp.Header.Get("Content-Type"))
assert.Equal(t, "User-agent: *\nAllow: /\n\nSitemap: https://help.nyaruka.com/sitemap.xml\n", body)

// they're only at their exact addresses
resp, _ = get(t, h, "help.nyaruka.com", "/sitemap.xml/", "help.nyaruka.com")
assert.Equal(t, 404, resp.StatusCode)
}

func TestPreview(t *testing.T) {
_, rt := testsuite.Runtime(t)
site, ids := setupSite(t, rt)
Expand Down Expand Up @@ -313,6 +399,12 @@ func TestPreview(t *testing.T) {
resp, _ = preview("/static/css/helpsite.css", "sesame")
assert.Equal(t, 200, resp.StatusCode)

// a preview isn't for crawling
resp, _ = preview("/sitemap.xml", "sesame")
assert.Equal(t, 404, resp.StatusCode)
resp, _ = preview("/robots.txt", "sesame")
assert.Equal(t, 404, resp.StatusCode)

// an unknown site
req := httptest.NewRequest(http.MethodGet, "http://helpsites:8031/hi/preview/00000000-0000-0000-0000-000000000000/", nil)
req.Header.Set("Authorization", "Token sesame")
Expand Down
4 changes: 3 additions & 1 deletion web/site/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ const (
)

func init() {
// search comes before the section pattern so it can't be shadowed by a section of that name
// search and the root files come before the section pattern so they can't be shadowed by a section of that name
web.SiteRoute(http.MethodGet, "/", handleHome)
web.SiteRoute(http.MethodGet, "/robots.txt", handleRobots)
web.SiteRoute(http.MethodGet, "/sitemap.xml", handleSitemap)
web.SiteRoute(http.MethodGet, "/search/", handleSearch)
web.SiteRoute(http.MethodGet, "/{section:[\\w-]+}/", handleSection)
web.SiteRoute(http.MethodGet, "/{section:[\\w-]+}/{article:[\\w-]+}/", handleArticle)
Expand Down
162 changes: 162 additions & 0 deletions web/site/sitemap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package site

import (
"context"
"encoding/xml"
"fmt"
"log/slog"
"net/http"
"net/url"
"strings"
"time"

"github.com/nyaruka/helpsites/v26/core/models"
"github.com/nyaruka/helpsites/v26/runtime"
"github.com/nyaruka/helpsites/v26/web"
"golang.org/x/net/html"
)

// how long a crawler can keep a root file - pages are rendered fresh on every request, so not long
const rootFileCacheControl = "public, max-age=300"

// handleRobots serves the site's robots.txt, which lets crawlers have everything and tells them where the sitemap is.
// Only the site on its own domain has one - a preview isn't for crawling.
func handleRobots(ctx context.Context, rt *runtime.Runtime, r *http.Request, w http.ResponseWriter) error {
sc := web.GetSiteContext(ctx)
if sc.IsPreview {
return handleNotFound(ctx, rt, r, w)
}

w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.Header().Set("Cache-Control", rootFileCacheControl)
w.WriteHeader(http.StatusOK)
fmt.Fprintf(w, "User-agent: *\nAllow: /\n\nSitemap: %s\n", siteURL(r, "/sitemap.xml"))
return nil
}

type sitemapURLSet struct {
XMLName xml.Name `xml:"urlset"`
XMLNS string `xml:"xmlns,attr"`
XMLNSImage string `xml:"xmlns:image,attr"`
URLs []sitemapURL `xml:"url"`
}

type sitemapURL struct {
Loc string `xml:"loc"`
LastMod string `xml:"lastmod,omitempty"`
Images []sitemapImage `xml:"image:image"`
}

type sitemapImage struct {
Loc string `xml:"image:loc"`
}

// handleSitemap serves the site's sitemap: its home page, each section and each article, with the images each
// article shows - so that search engines can find all of them. Only the site on its own domain has one.
func handleSitemap(ctx context.Context, rt *runtime.Runtime, r *http.Request, w http.ResponseWriter) error {
sc := web.GetSiteContext(ctx)
if sc.IsPreview {
return handleNotFound(ctx, rt, r, w)
}

sections, err := models.LoadSections(ctx, rt.DB, sc.Site.Source.ID)
if err != nil {
return err
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance: this is an unauthenticated endpoint. Every hit loads the body of every readable article and HTML-parses each one. Cache-Control: max-age=300 only helps clients that respect it; anything else can request it repeatedly and cost DB load and CPU in proportion to the site's size. Two options:

  • Cache the generated XML in-process per site for a few minutes.
  • Select only what's needed: the image srcs can't come from SQL, but body_html could be fetched without the other columns.

Also, there's no guard for the sitemaps.org limits (50,000 URLs / 50MB uncompressed). That's probably fine for help sites, but it's worth a comment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left as is. That's two queries and an HTML parse of each body, about what a busy page already costs (an article page loads every link target on the site). An in-process cache would also go stale across instances until it expired. Help sites are orders of magnitude below the 50,000 URL / 50MB limits.

🤖 Addressed by Claude Code

articles, err := models.LoadReadable(ctx, rt.DB, sc.Site.Source.ID)
if err != nil {
return err
}

// a section's page lists its articles, so it changes whenever one of them does
sectionModified := make(map[models.ArticleID]time.Time, len(sections))
for _, s := range sections {
sectionModified[s.ID] = s.ModifiedOn
}
for _, a := range articles {
if a.ModifiedOn.After(sectionModified[a.Parent.ID]) {
sectionModified[a.Parent.ID] = a.ModifiedOn
}
}

set := &sitemapURLSet{
XMLNS: "http://www.sitemaps.org/schemas/sitemap/0.9",
XMLNSImage: "http://www.google.com/schemas/sitemap-image/1.1",
URLs: []sitemapURL{{Loc: siteURL(r, "/")}},
}
for _, s := range sections {
set.URLs = append(set.URLs, sitemapURL{
Loc: siteURL(r, "/"+s.Slug+"/"),
LastMod: sectionModified[s.ID].UTC().Format(time.RFC3339),
})
}
for _, a := range articles {
loc := siteURL(r, "/"+a.Parent.Slug+"/"+a.Slug+"/")
u := sitemapURL{Loc: loc, LastMod: a.ModifiedOn.UTC().Format(time.RFC3339)}
for _, src := range ImageURLs(ResolveImages(a.BodyHTML, rt.Config.StorageURL), loc) {
u.Images = append(u.Images, sitemapImage{Loc: src})
}
set.URLs = append(set.URLs, u)
}

out, err := xml.MarshalIndent(set, "", " ")
if err != nil {
return fmt.Errorf("error marshaling sitemap: %w", err)
}

w.Header().Set("Content-Type", "application/xml; charset=utf-8")
w.Header().Set("Cache-Control", rootFileCacheControl)
w.WriteHeader(http.StatusOK)
if _, err := w.Write(append([]byte(xml.Header), out...)); err != nil {
slog.Debug("error writing response", "error", err)
}
return nil
}

// siteURL returns the absolute URL of the given path on the site, on the host the request reached it on - lowercased
// and without its scheme's default port, so that the same host asked for differently gives the same addresses
func siteURL(r *http.Request, path string) string {
scheme, defaultPort := "http", ":80"
if r.TLS != nil {
scheme, defaultPort = "https", ":443"
}
host := strings.TrimSuffix(strings.ToLower(r.Host), defaultPort)
return (&url.URL{Scheme: scheme, Host: host, Path: path}).String()
}

// ImageURLs returns the addresses of the images the given rendered body shows, resolved against the address of its
// page, each once and in the order they appear. A fragment is only for the page, so isn't part of the image's address.
func ImageURLs(body, pageURL string) []string {
base, err := url.Parse(pageURL)
if err != nil {
return nil
}
doc, err := html.Parse(strings.NewReader(body))
if err != nil {
return nil
}

var urls []string
seen := map[string]bool{}
for n := range doc.Descendants() {
if n.Type != html.ElementNode || n.Data != "img" {
continue
}
for _, attr := range n.Attr {
value := strings.TrimSpace(attr.Val)
if attr.Namespace != "" || attr.Key != "src" || value == "" {
continue
}
src, err := base.Parse(value)
if err != nil || (src.Scheme != "http" && src.Scheme != "https") {
continue
}
src.Fragment, src.RawFragment = "", ""
if address := src.String(); !seen[address] {
seen[address] = true
urls = append(urls, address)
}
}
}
return urls
}
Loading
Loading