all: remove Path metadata-based redirects

Back when the entire site had to live in $GOROOT/doc, we needed
some way to specify content that was served from URLs outside of golang.org/doc,
so we added the ability for a doc to declare its own URL (the Path metadata).
That meant the file system layout did not match the URL layout.
That meant the content for any particular URL could be anywhere in the file system.
That meant the entire file system had to be scanned to serve a URL.
That meant an index of the file system had to be built and updated.

Now that we have a file tree (_content) for the whole of golang.org,
we can move files to make the file system layout match the URL space.
Then each URL can be served by just reading the right file.
Then the index and its updater can be deleted.

And now if you want to edit /doc/gdb it's obvious which file to open.

Change-Id: I3357f275e61a31c8de3091af580cac80753e71a4
Reviewed-on: https://go-review.googlesource.com/c/website/+/296383
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

_content/conduct.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Go Community Code of Conduct",
-	"Path":  "/conduct"
+	"Title": "Go Community Code of Conduct"
 }-->
 
 <style>

_content/doc/conduct.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/conduct"
+}-->

_content/doc/contrib.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/project/"
+}-->

_content/doc/debugging_with_gdb.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/doc/gdb"
+}-->

_content/doc/docs.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/doc/"
+}-->

_content/doc/faq.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Frequently Asked Questions (FAQ)",
-	"Path": "/doc/faq"
+	"Title": "Frequently Asked Questions (FAQ)"
 }-->
 
 <h2 id="Origins">Origins</h2>

_content/doc/gdb.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Debugging Go Code with GDB",
-	"Path": "/doc/gdb"
+	"Title": "Debugging Go Code with GDB"
 }-->
 
 <!--

_content/doc/go_faq.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/doc/faq"
+}-->

_content/doc/help.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/help"
+}-->

_content/doc/index.html

@@ -1,6 +1,5 @@
 <!--{
 	"Title": "Documentation",
-	"Path": "/doc/",
 	"Template": true
 }-->
 

_content/doc/install/gccgo.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Setting up and using gccgo",
-	"Path": "/doc/install/gccgo"
+	"Title": "Setting up and using gccgo"
 }-->
 
 <p>

_content/doc/install/source.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Installing Go from source",
-	"Path": "/doc/install/source"
+	"Title": "Installing Go from source"
 }-->
 
 <p>

_content/doc/root.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/"
+}-->

_content/doc/security.md

@@ -0,0 +1,3 @@
+<!--{
+	"Redirect": "/security"
+}-->

_content/help.html

@@ -1,6 +1,5 @@
 <!--{
 	"Title": "Help",
-	"Path": "/help/",
 	"Template": true
 }-->
 

_content/index.html

@@ -1,5 +1,4 @@
 <!--{
-  "Path": "/",
   "Template": true
 }-->
 

_content/project.html

@@ -1,6 +1,5 @@
 <!--{
 	"Title": "The Go Project",
-	"Path": "/project/",
 	"Template": true
 }-->
 

_content/ref/mod.md

@@ -1,6 +1,5 @@
 <!--{
-  "Title": "Go Modules Reference",
-  "Path": "/ref/mod"
+  "Title": "Go Modules Reference"
 }-->
 <!-- TODO(golang.org/issue/33637): Write focused "guide" articles on specific
 module topics and tasks. Link to those instead of the blog, which will probably

_content/security.html

@@ -1,6 +1,5 @@
 <!--{
-	"Title": "Go Security Policy",
-	"Path":  "/security"
+	"Title": "Go Security Policy"
 }-->
 
 <h2>Implementation</h2>

cmd/golangorg/godoc_test.go

@@ -19,6 +19,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 )
@@ -171,12 +172,69 @@ func testWeb(t *testing.T) {
 		contains    []string // substring
 		match       []string // regexp
 		notContains []string
+		redirect    string
 		releaseTag  string // optional release tag that must be in go/build.ReleaseTags
 	}{
 		{
 			path:     "/",
 			contains: []string{"Go is an open source programming language"},
 		},
+		{
+			path:     "/conduct",
+			contains: []string{"Project Stewards"},
+		},
+		{
+			path:     "/doc/asm",
+			contains: []string{"Quick Guide", "Assembler"},
+		},
+		{
+			path:     "/doc/gdb",
+			contains: []string{"Debugging Go Code"},
+		},
+		{
+			path:     "/doc/debugging_with_gdb.html",
+			redirect: "/doc/gdb",
+		},
+		{
+			path:     "/ref/spec",
+			contains: []string{"Go Programming Language Specification"},
+		},
+		{
+			path:     "/doc/go_spec",
+			redirect: "/ref/spec",
+		},
+		{
+			path:     "/doc/go_spec.html",
+			redirect: "/ref/spec",
+		},
+		{
+			path:     "/doc/go_spec.md",
+			redirect: "/ref/spec",
+		},
+		{
+			path:     "/ref/mem",
+			contains: []string{"Memory Model"},
+		},
+		{
+			path:     "/doc/go_mem.html",
+			redirect: "/ref/mem",
+		},
+		{
+			path:     "/doc/go_mem.md",
+			redirect: "/ref/mem",
+		},
+		{
+			path:     "/doc/help.html",
+			redirect: "/help",
+		},
+		{
+			path:     "/help/",
+			redirect: "/help",
+		},
+		{
+			path:     "/help",
+			contains: []string{"Get help"},
+		},
 		{
 			path:     "/pkg/fmt/",
 			contains: []string{"Package fmt implements formatted I/O"},
@@ -190,7 +248,11 @@ func testWeb(t *testing.T) {
 			contains: []string{"// Println formats using"},
 		},
 		{
-			path: "/pkg",
+			path:     "/pkg",
+			redirect: "/pkg/",
+		},
+		{
+			path: "/pkg/",
 			contains: []string{
 				"Standard library",
 				"Package fmt implements formatted I/O",
@@ -260,26 +322,56 @@ func testWeb(t *testing.T) {
 			releaseTag: "go1.11",
 		},
 		{
-			path: "/project/",
+			path: "/project",
 			contains: []string{
 				`<li><a href="/doc/go1.14">Go 1.14</a> <small>(February 2020)</small></li>`,
 				`<li><a href="/doc/go1.1">Go 1.1</a> <small>(May 2013)</small></li>`,
 			},
 		},
+		{
+			path:     "/doc/go1.16.html",
+			redirect: "/doc/go1.16",
+		},
+		{
+			path:     "/doc/go1.16",
+			contains: []string{"Go 1.16"},
+		},
 	}
 	for _, test := range tests {
 		url := fmt.Sprintf("http://%s%s", addr, test.path)
-		resp, err := http.Get(url)
+		var redirect string
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				redirect = strings.TrimPrefix(req.URL.String(), "http://"+addr)
+				return fmt.Errorf("not following redirects")
+			},
+		}
+		resp, err := client.Get(url)
+		if redirect != "" {
+			resp.Body.Close()
+			if test.redirect == "" {
+				t.Errorf("GET %s: unexpected redirect -> %s", url, redirect)
+				continue
+			}
+			if test.redirect != redirect {
+				t.Errorf("GET %s: redirect -> %s, want %s", url, redirect, test.redirect)
+				continue
+			}
+			continue
+		}
 		if err != nil {
 			t.Errorf("GET %s failed: %s", url, err)
 			continue
 		}
 		body, err := ioutil.ReadAll(resp.Body)
-		strBody := string(body)
 		resp.Body.Close()
 		if err != nil {
 			t.Errorf("GET %s: failed to read body: %s (response: %v)", url, err, resp)
 		}
+		if test.redirect != "" {
+			t.Errorf("GET %s: have direct response, want redirect -> %s", url, test.redirect)
+		}
+		strBody := string(body)
 		isErr := false
 		for _, substr := range test.contains {
 			if test.releaseTag != "" && !hasTag(test.releaseTag) {

cmd/golangorg/main.go

@@ -79,10 +79,6 @@ func main() {
 	fsys = unionFS{content, os.DirFS(*goroot)}
 
 	corpus := godoc.NewCorpus(fsys)
-	corpus.Verbose = *verbose
-	if err := corpus.Init(); err != nil {
-		log.Fatal(err)
-	}
 	// Initialize the version info before readTemplates, which saves
 	// the map value in a method value.
 	corpus.InitVersionInfo()

cmd/golangorg/release_test.go

@@ -31,7 +31,7 @@ func TestReleaseHistory(t *testing.T) {
 	readTemplates(pres)
 	mux := registerHandlers(pres)
 
-	req := httptest.NewRequest(http.MethodGet, "/doc/devel/release.html", nil)
+	req := httptest.NewRequest(http.MethodGet, "/doc/devel/release", nil)
 	rr := httptest.NewRecorder()
 	mux.ServeHTTP(rr, req)
 	resp := rr.Result()

internal/godoc/corpus.go

@@ -9,8 +9,6 @@ package godoc
 
 import (
 	"io/fs"
-	"sync"
-	"time"
 
 	"golang.org/x/website/internal/api"
 )
@@ -23,22 +21,6 @@ import (
 type Corpus struct {
 	fs fs.FS
 
-	// Verbose logging.
-	Verbose bool
-
-	// Send a value on this channel to trigger a metadata refresh.
-	// It is buffered so that if a signal is not lost if sent
-	// during a refresh.
-	refreshMetadataSignal chan bool
-
-	// file system information
-	fsModified  rwValue // timestamp of last call to invalidateIndex
-	docMetadata rwValue // mapping from paths to *Metadata
-
-	// flag to check whether a corpus is initialized or not
-	initMu   sync.RWMutex
-	initDone bool
-
 	// pkgAPIInfo contains the information about which package API
 	// features were added in which version of Go.
 	pkgAPIInfo api.DB
@@ -49,25 +31,7 @@ type Corpus struct {
 // Change or set any options on Corpus before calling the Corpus.Init method.
 func NewCorpus(fsys fs.FS) *Corpus {
 	c := &Corpus{
-		fs:                    fsys,
-		refreshMetadataSignal: make(chan bool, 1),
+		fs: fsys,
 	}
 	return c
 }
-
-func (c *Corpus) FSModifiedTime() time.Time {
-	_, ts := c.fsModified.Get()
-	return ts
-}
-
-// Init initializes Corpus, once options on Corpus are set.
-// It must be called before any subsequent method calls.
-func (c *Corpus) Init() error {
-	c.updateMetadata()
-	go c.refreshMetadataLoop()
-
-	c.initMu.Lock()
-	c.initDone = true
-	c.initMu.Unlock()
-	return nil
-}

internal/godoc/meta.go

@@ -10,13 +10,9 @@ package godoc
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"io/fs"
 	"log"
-	"os"
-	"path"
 	"strings"
-	"time"
 )
 
 var (
@@ -25,28 +21,28 @@ var (
 	jsonEnd   = []byte("}-->")
 )
 
-// ----------------------------------------------------------------------------
-// Documentation Metadata
-
 type Metadata struct {
-	// These fields can be set in the JSON header at the top of a doc.
+	// Copied from document metadata
 	Title    string
 	Subtitle string
-	Template bool     // execute as template
-	Path     string   // canonical path for this page
-	AltPaths []string // redirect these other paths to this page
+	Template bool
 
-	// These are internal to the implementation.
-	filePath string // filesystem path relative to goroot
+	Path     string // URL path
+	FilePath string // filesystem path relative to goroot
 }
 
-func (m *Metadata) FilePath() string { return m.filePath }
+type MetaJSON struct {
+	Title    string
+	Subtitle string
+	Template bool
+	Redirect string // if set, redirect to other URL
+}
 
-// extractMetadata extracts the Metadata from a byte slice.
+// extractMetadata extracts the MetaJSON from a byte slice.
 // It returns the Metadata value and the remaining data.
 // If no metadata is present the original byte slice is returned.
 //
-func extractMetadata(b []byte) (meta Metadata, tail []byte, err error) {
+func extractMetadata(b []byte) (meta MetaJSON, tail []byte, err error) {
 	tail = b
 	if !bytes.HasPrefix(b, jsonStart) {
 		return
@@ -63,101 +59,66 @@ func extractMetadata(b []byte) (meta Metadata, tail []byte, err error) {
 	return
 }
 
-// UpdateMetadata scans $GOROOT/doc for HTML and Markdown files, reads their metadata,
-// and updates the DocMetadata map.
-func (c *Corpus) updateMetadata() {
-	metadata := make(map[string]*Metadata)
-	var scan func(string) // scan is recursive
-	scan = func(dir string) {
-		fis, err := fs.ReadDir(c.fs, toFS(dir))
-		if err != nil {
-			if dir == "/doc" && errors.Is(err, os.ErrNotExist) {
-				// Be quiet during tests that don't have a /doc tree.
-				return
-			}
-			log.Printf("updateMetadata %s: %v", dir, err)
-			return
-		}
-		for _, fi := range fis {
-			name := path.Join(dir, fi.Name())
-			if fi.IsDir() {
-				scan(name) // recurse
-				continue
-			}
-			if !strings.HasSuffix(name, ".html") && !strings.HasSuffix(name, ".md") {
-				continue
-			}
-			// Extract metadata from the file.
-			b, err := fs.ReadFile(c.fs, toFS(name))
-			if err != nil {
-				log.Printf("updateMetadata %s: %v", name, err)
-				continue
-			}
-			meta, _, err := extractMetadata(b)
-			if err != nil {
-				log.Printf("updateMetadata: %s: %v", name, err)
-				continue
-			}
-			// Present all .md as if they were .html,
-			// so that it doesn't matter which one a page is written in.
-			if strings.HasSuffix(name, ".md") {
-				name = strings.TrimSuffix(name, ".md") + ".html"
-			}
-			// Store relative filesystem path in Metadata.
-			meta.filePath = name
-			if meta.Path == "" {
-				// If no Path, canonical path is actual path with .html removed.
-				meta.Path = strings.TrimSuffix(name, ".html")
-			}
-			// Store under both paths.
-			metadata[meta.Path] = &meta
-			metadata[meta.filePath] = &meta
-			for _, path := range meta.AltPaths {
-				metadata[path] = &meta
-			}
-		}
+// MetadataFor returns the *Metadata for a given absolute path
+// or nil if none exists.
+func (c *Corpus) MetadataFor(path string) *Metadata {
+	// Strip any .html or .md; it all names the same page.
+	if strings.HasSuffix(path, ".html") {
+		path = strings.TrimSuffix(path, ".html")
+	} else if strings.HasSuffix(path, ".md") {
+		path = strings.TrimSuffix(path, ".md")
 	}
-	scan("/doc")
-	c.docMetadata.Set(metadata)
-}
 
-// MetadataFor returns the *Metadata for a given relative path or nil if none
-// exists.
-//
-func (c *Corpus) MetadataFor(relpath string) *Metadata {
-	if m, _ := c.docMetadata.Get(); m != nil {
-		meta := m.(map[string]*Metadata)
-		// If metadata for this relpath exists, return it.
-		if p := meta[relpath]; p != nil {
-			return p
+	file := path + ".html"
+	b, err := fs.ReadFile(c.fs, toFS(file))
+	if err != nil {
+		file = path + ".md"
+		b, err = fs.ReadFile(c.fs, toFS(file))
+	}
+	if err != nil {
+		// Special case for memory model and spec, which live
+		// in the main Go repo's doc directory and therefore have not
+		// been renamed to their serving paths.
+		// We wait until the ReadFiles above have failed so that the
+		// code works if these are ever moved to /ref/spec and /ref/mem.
+		switch path {
+		case "/ref/spec":
+			if m := c.MetadataFor("/doc/go_spec"); m != nil {
+				return m
+			}
+		case "/ref/mem":
+			if m := c.MetadataFor("/doc/go_mem"); m != nil {
+				return m
+			}
 		}
-		// Try with or without trailing slash.
-		if strings.HasSuffix(relpath, "/") {
-			relpath = relpath[:len(relpath)-1]
-		} else {
-			relpath = relpath + "/"
-		}
-		return meta[relpath]
+		return nil
 	}
-	return nil
-}
 
-// refreshMetadata sends a signal to update DocMetadata. If a refresh is in
-// progress the metadata will be refreshed again afterward.
-//
-func (c *Corpus) refreshMetadata() {
-	select {
-	case c.refreshMetadataSignal <- true:
-	default:
+	js, _, err := extractMetadata(b)
+	if err != nil {
+		log.Printf("MetadataFor %s: %v", path, err)
+		return nil
 	}
-}
 
-// RefreshMetadataLoop runs forever, updating DocMetadata when the underlying
-// file system changes. It should be launched in a goroutine.
-func (c *Corpus) refreshMetadataLoop() {
-	for {
-		<-c.refreshMetadataSignal
-		c.updateMetadata()
-		time.Sleep(10 * time.Second) // at most once every 10 seconds
+	meta := &Metadata{
+		Title:    js.Title,
+		Subtitle: js.Subtitle,
+		Template: js.Template,
+		Path:     path,
+		FilePath: file,
 	}
+	if js.Redirect != "" {
+		// Allow (placeholder) documents to declare a redirect.
+		meta.Path = js.Redirect
+	}
+
+	// Special case for memory model and spec, continued.
+	switch path {
+	case "/doc/go_spec":
+		meta.Path = "/ref/spec"
+	case "/doc/go_mem":
+		meta.Path = "/ref/mem"
+	}
+
+	return meta
 }

internal/godoc/server.go

@@ -269,20 +269,13 @@ func (p *Presentation) serveDirectory(w http.ResponseWriter, r *http.Request, ab
 
 func (p *Presentation) ServeHTMLDoc(w http.ResponseWriter, r *http.Request, abspath, relpath string) {
 	// get HTML body contents
-	isMarkdown := false
 	src, err := fs.ReadFile(p.Corpus.fs, toFS(abspath))
-	if err != nil && strings.HasSuffix(abspath, ".html") {
-		if md, errMD := fs.ReadFile(p.Corpus.fs, toFS(strings.TrimSuffix(abspath, ".html")+".md")); errMD == nil {
-			src = md
-			isMarkdown = true
-			err = nil
-		}
-	}
 	if err != nil {
 		log.Printf("ReadFile: %s", err)
 		p.ServeError(w, r, relpath, err)
 		return
 	}
+	isMarkdown := strings.HasSuffix(abspath, ".md")
 
 	// if it begins with "<!DOCTYPE " assume it is standalone
 	// html that doesn't need the template wrapping.
@@ -356,19 +349,19 @@ func (p *Presentation) serveFile(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Check to see if we need to redirect or serve another file.
-	relpath := r.URL.Path
-	if m := p.Corpus.MetadataFor(relpath); m != nil {
-		if m.Path != relpath {
+	abspath := r.URL.Path
+	if m := p.Corpus.MetadataFor(abspath); m != nil {
+		if m.Path != abspath {
 			// Redirect to canonical path.
 			http.Redirect(w, r, m.Path, http.StatusMovedPermanently)
 			return
 		}
 		// Serve from the actual filesystem path.
-		relpath = m.filePath
+		p.ServeHTMLDoc(w, r, m.FilePath, m.Path)
+		return
 	}
 
-	abspath := relpath
-	relpath = relpath[1:] // strip leading slash
+	relpath := abspath[1:] // strip leading slash
 
 	switch path.Ext(relpath) {
 	case ".html":
@@ -382,7 +375,15 @@ func (p *Presentation) serveFile(w http.ResponseWriter, r *http.Request) {
 
 	dir, err := fs.Stat(p.Corpus.fs, toFS(abspath))
 	if err != nil {
-		log.Print(err)
+		// Check for spurious trailing slash.
+		if strings.HasSuffix(abspath, "/") {
+			trimmed := abspath[:len(abspath)-1]
+			if _, err := fs.Stat(p.Corpus.fs, toFS(trimmed)); err == nil ||
+				p.Corpus.MetadataFor(trimmed) != nil {
+				http.Redirect(w, r, trimmed, http.StatusMovedPermanently)
+				return
+			}
+		}
 		p.ServeError(w, r, relpath, err)
 		return
 	}

internal/godoc/server_test.go

@@ -30,15 +30,8 @@ func testServeBody(t *testing.T, p *Presentation, path, body string) {
 
 func TestRedirectAndMetadata(t *testing.T) {
 	c := NewCorpus(fstest.MapFS{
-		"doc/y/index.html": {Data: []byte("Hello, y.")},
-		"doc/x/index.html": {Data: []byte(`<!--{
-		"Path": "/doc/x/"
-}-->
-
-Hello, x.
-`)},
+		"doc/x/index.html": {Data: []byte("Hello, x.")},
 	})
-	c.updateMetadata()
 	p := &Presentation{
 		Corpus:    c,
 		GodocHTML: template.Must(template.New("").Parse(`{{printf "%s" .Body}}`)),
@@ -46,19 +39,17 @@ Hello, x.
 
 	// Test that redirect is sent back correctly.
 	// Used to panic. See golang.org/issue/40665.
-	for _, elem := range []string{"x", "y"} {
-		dir := "/doc/" + elem + "/"
+	dir := "/doc/x/"
 
-		r := &http.Request{URL: &url.URL{Path: dir + "index.html"}}
-		rw := httptest.NewRecorder()
-		p.ServeFile(rw, r)
-		loc := rw.Result().Header.Get("Location")
-		if rw.Code != 301 || loc != dir {
-			t.Errorf("GET %s: expected 301 -> %q, got %d -> %q", r.URL.Path, dir, rw.Code, loc)
-		}
-
-		testServeBody(t, p, dir, "Hello, "+elem)
+	r := &http.Request{URL: &url.URL{Path: dir + "index.html"}}
+	rw := httptest.NewRecorder()
+	p.ServeFile(rw, r)
+	loc := rw.Result().Header.Get("Location")
+	if rw.Code != 301 || loc != dir {
+		t.Errorf("GET %s: expected 301 -> %q, got %d -> %q", r.URL.Path, dir, rw.Code, loc)
 	}
+
+	testServeBody(t, p, dir, "Hello, x")
 }
 
 func TestMarkdown(t *testing.T) {
@@ -70,6 +61,6 @@ func TestMarkdown(t *testing.T) {
 		GodocHTML: template.Must(template.New("").Parse(`{{printf "%s" .Body}}`)),
 	}
 
-	testServeBody(t, p, "/doc/test.html", "<strong>bold</strong>")
-	testServeBody(t, p, "/doc/test2.html", "<em>template</em>")
+	testServeBody(t, p, "/doc/test", "<strong>bold</strong>")
+	testServeBody(t, p, "/doc/test2", "<em>template</em>")
 }

internal/godoc/util.go

@@ -10,32 +10,9 @@ package godoc
 import (
 	"io/fs"
 	"path"
-	"sync"
-	"time"
 	"unicode/utf8"
 )
 
-// An rwValue wraps a value and permits mutually exclusive
-// access to it and records the time the value was last set.
-type rwValue struct {
-	mutex     sync.RWMutex
-	value     interface{}
-	timestamp time.Time // time of last set()
-}
-
-func (v *rwValue) Set(value interface{}) {
-	v.mutex.Lock()
-	v.value = value
-	v.timestamp = time.Now()
-	v.mutex.Unlock()
-}
-
-func (v *rwValue) Get() (interface{}, time.Time) {
-	v.mutex.RLock()
-	defer v.mutex.RUnlock()
-	return v.value, v.timestamp
-}
-
 // IsText reports whether a significant prefix of s looks like correct UTF-8;
 // that is, if it is likely that s is human-readable text.
 func IsText(s []byte) bool {