Fixes for Preview URL Redirects, CSS Asset Fetching, and Post-Merge P…

…review URL Availability (#96)

README.md

@@ -79,13 +79,13 @@ See the following examples:
 
 In addition to the `redirects` property in site data, you can also specify redirects in a text file named `redirects` at the top level of the `assets` VFS. The format is as follows:
 
-``` text
+```text
 FROM-PATH TO-URL STATUS-CODE
 ```
 
 For example:
 
-``` text
+```text
 # Comments are allowed
 /my/old/page /my/new/page 308
 /another/page https://example.com/page 308
@@ -96,34 +96,36 @@ For example:
 The `docsite` tool requires site data to be available in any of the following ways:
 
 - A `docsite.json` file (or other file specified in the `-config` flag's search paths), as in the following example:
-   ```json
-   {
-     "content": "../sourcegraph/doc",
-     "baseURLPath": "/",
-     "templates": "templates",
-     "assets": "assets",
-     "assetsBaseURLPath": "/assets/",
-     "check": {
-       "ignoreURLPattern": "(^https?://)|(^#)|(^mailto:support@sourcegraph\\.com$)|(^chrome://)"
-     }
-   }
-   ```
+  ```json
+  {
+    "content": "../sourcegraph/doc",
+    "baseURLPath": "/",
+    "templates": "templates",
+    "assets": "assets",
+    "assetsBaseURLPath": "/assets/",
+    "check": {
+      "ignoreURLPattern": "(^https?://)|(^#)|(^mailto:support@sourcegraph\\.com$)|(^chrome://)"
+    }
+  }
+  ```
 - In the `DOCSITE_CONFIG` env var, using Zip archive URLs for `templates`, `assets`, and `content`, as in the following example:
-   ```
-   DOCSITE_CONFIG='{"templates":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/main#*/doc/_resources/templates/","assets":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/main#*/doc/_resources/assets/","content":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/$VERSION#*/doc/","baseURLPath":"/","assetsBaseURLPath":"/assets/","defaultContentBranch":"main"}' docsite serve
-   ```
+  ```
+  DOCSITE_CONFIG='{"templates":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/main#*/doc/_resources/templates/","assets":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/main#*/doc/_resources/assets/","content":"https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/$VERSION#*/doc/","baseURLPath":"/","assetsBaseURLPath":"/assets/","defaultContentBranch":"main"}' docsite serve
+  ```
 
 ## Development
 
 ### Release a new version
 
 1. Build the Docker image for `linux/amd64`:
+
    ```sh
    docker build -t sourcegraph/docsite .
 
    # Use buildx if you're on M1
    docker buildx build --platform linux/amd64 -t sourcegraph/docsite .
    ```
+
 1. Tag and push the image to Docker Hub and GCR:
    ```sh
    export VERSION= # e.g. v1.9.1

check_test.go

@@ -18,33 +18,59 @@ func TestCheck(t *testing.T) {
 	}{
 		"valid links": {
 			pages: map[string]string{
-				"index.md":   "[a](index.md) [b](b/index.md)",
-				"b/index.md": "[a](../index.md) [b](index.md)",
+				"index.md":                           "[a](index.md) [b](b/index.md)",
+				"b/index.md":                         "[a](../index.md) [b](index.md)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
 			},
 			wantProblems: nil,
 		},
 		"non-relative link path": {
-			pages:        map[string]string{"index.md": "[a](/index.md)"},
+			pages: map[string]string{
+				"index.md":                           "[a](/index.md)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: []string{"index.md: must use relative, not absolute, link to /index.md"},
 		},
 		"scheme-relative link": {
-			pages:        map[string]string{"index.md": "[a](//example.com/a)"},
+			pages: map[string]string{
+				"index.md":                           "[a](//example.com/a)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: nil,
 		},
 		"broken link": {
-			pages:        map[string]string{"index.md": "[x](x.md)"},
+			pages: map[string]string{
+				"index.md":                           "[x](x.md)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: []string{"index.md: broken link to /x"},
 		},
 		"link to equivalent path not .md file": {
-			pages:        map[string]string{"index.md": "[a](a) [a](a.md)", "a.md": ""},
+			pages: map[string]string{
+				"index.md": "[a](a) [a](a.md)", "a.md": "",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: []string{"index.md: must link to .md file, not a"},
 		},
 		"disconnected page": {
-			pages:        map[string]string{"x.md": "[x](x.md)"},
+			pages: map[string]string{
+				"x.md":                               "[x](x.md)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: []string{"x.md: disconnected page (no inlinks from other pages)"},
 		},
 		"ignore disconnected page check": {
-			pages:        map[string]string{"x.md": "---\nignoreDisconnectedPageCheck: true\n---\n\n[x](x.md)"},
+			pages: map[string]string{
+				"x.md":                               "---\nignoreDisconnectedPageCheck: true\n---\n\n[x](x.md)",
+				"_resources/templates/root.html":     "{{markdown .Content}}",
+				"_resources/templates/document.html": "{{markdown .Content}}",
+			},
 			wantProblems: nil,
 		},
 	}
@@ -53,9 +79,9 @@ func TestCheck(t *testing.T) {
 			ctx := context.Background()
 			site := Site{
 				Content: versionedFileSystem{
-					"": httpfs.New(mapfs.New(test.pages)),
+					"":                     httpfs.New(mapfs.New(test.pages)),
+					"_resources/templates": httpfs.New(mapfs.New(map[string]string{"document.html": "{{markdown .Content}}"})),
 				},
-				Templates:             httpfs.New(mapfs.New(map[string]string{"document.html": "{{markdown .Content}}"})),
 				Base:                  &url.URL{Path: "/"},
 				CheckIgnoreURLPattern: regexp.MustCompile(`^//`),
 			}

cmd/docsite/site.go

@@ -141,7 +141,12 @@ func addSiteRedirect(site *docsite.Site, fromPath, toURLStr string) error {
 //
 // The format of each line is `PATH DESTINATION STATUSCODE` (e.g., `/my/old/page /my/new/page 308`).
 func addRedirectsFromAssets(site *docsite.Site) error {
-	raw, err := docsite.ReadFile(site.Assets, "redirects")
+	assets, err := site.GetResources("assets", "")
+	if err != nil {
+		return err
+	}
+
+	raw, err := docsite.ReadFile(assets, "redirects")
 	if err != nil && !os.IsNotExist(err) {
 		return err
 	}
@@ -165,6 +170,11 @@ func addRedirectsFromAssets(site *docsite.Site) error {
 	return nil
 }
 
+const (
+	DEBUG        = false
+	CODEHOST_URL = "https://codeload.github.com/sourcegraph/sourcegraph/zip/refs/heads/$VERSION#*/doc/"
+)
+
 // openDocsiteFromConfig reads the documentation site data from a docsite.json file. All file system
 // paths in docsite.json are resolved relative to baseDir.
 func openDocsiteFromConfig(configData []byte, baseDir string) (*docsite.Site, *docsiteConfig, error) {
@@ -184,12 +194,21 @@ func openDocsiteFromConfig(configData []byte, baseDir string) (*docsite.Site, *d
 		}
 		return http.Dir(filepath.Join(baseDir, dir))
 	}
-	site.Templates = httpDirOrNil(config.Templates)
-	site.Content = nonVersionedFileSystem{httpDirOrNil(config.Content)}
-	site.Assets = httpDirOrNil(config.Assets)
+
+	if DEBUG {
+		content := newVersionedFileSystemURL(CODEHOST_URL, "master")
+		if _, err := content.OpenVersion(context.Background(), ""); err != nil {
+			return nil, nil, errors.WithMessage(err, "downloading content default version")
+		}
+		site.Content = content
+	} else {
+		site.Content = nonVersionedFileSystem{httpDirOrNil(config.Content)}
+	}
+
 	if err := addRedirectsFromAssets(site); err != nil {
 		return nil, nil, err
 	}
+
 	return site, &config, nil
 }
 
@@ -221,17 +240,10 @@ func openDocsiteFromEnv() (*docsite.Site, *docsiteConfig, error) {
 
 	// Read site data.
 	log.Println("# Downloading site data...")
-	assets := newCachedFileSystem(func() (http.FileSystem, error) { return zipFileSystemFromURLWithDirFragment(config.Assets) })
-	if err := assets.fetchAndCache(); err != nil {
-		return nil, nil, errors.WithMessage(err, "prefetching assets")
-	}
-	templates := newCachedFileSystem(func() (http.FileSystem, error) { return zipFileSystemFromURLWithDirFragment(config.Templates) })
-	if err := templates.fetchAndCache(); err != nil {
-		return nil, nil, errors.WithMessage(err, "prefetching templates")
-	}
 
 	// Content is in a versioned file system.
 	content := &versionedFileSystemURL{url: config.Content, defaultBranch: config.DefaultContentBranch}
+
 	// Prefetch content at its default version. This ensures that the program exits if the content
 	// default version is unavailable.
 	if _, err := content.OpenVersion(context.Background(), ""); err != nil {
@@ -242,9 +254,7 @@ func openDocsiteFromEnv() (*docsite.Site, *docsiteConfig, error) {
 	if err != nil {
 		return nil, nil, err
 	}
-	site.Templates = templates
 	site.Content = content
-	site.Assets = assets
 	if err := addRedirectsFromAssets(site); err != nil {
 		return nil, nil, err
 	}
@@ -269,6 +279,10 @@ type fileSystemCacheEntry struct {
 
 const fileSystemCacheTTL = 5 * time.Minute
 
+func newVersionedFileSystemURL(url, branch string) *versionedFileSystemURL {
+	return &versionedFileSystemURL{url: url, defaultBranch: branch}
+}
+
 func (fs *versionedFileSystemURL) OpenVersion(ctx context.Context, version string) (http.FileSystem, error) {
 	// HACK(sqs): this works for codeload.github.com
 	if version == "" {

fs_test.go

@@ -3,12 +3,12 @@ package docsite
 import (
 	"context"
 	"net/http"
-	"os"
 	"path/filepath"
 	"reflect"
 	"sort"
 	"testing"
 
+	"github.com/pkg/errors"
 	"golang.org/x/tools/godoc/vfs/httpfs"
 	"golang.org/x/tools/godoc/vfs/mapfs"
 )
@@ -48,7 +48,7 @@ type versionedFileSystem map[string]http.FileSystem
 func (vfs versionedFileSystem) OpenVersion(_ context.Context, version string) (http.FileSystem, error) {
 	fs, ok := vfs[version]
 	if !ok {
-		return nil, &os.PathError{Op: "OpenVersion", Path: version, Err: os.ErrNotExist}
+		return nil, errors.New("version not found")
 	}
 	return fs, nil
 }

handler.go

@@ -41,8 +41,21 @@ func (s *Site) Handler() http.Handler {
 
 	// Serve assets using http.FileServer.
 	if s.AssetsBase != nil {
-		assetsFileServer := http.FileServer(s.Assets)
+		assets, err := s.GetResources("assets", "")
+		if err != nil {
+			panic("failed to open assets: " + err.Error())
+		}
+
+		assetsFileServer := http.FileServer(assets)
 		m.Handle(s.AssetsBase.Path, http.StripPrefix(s.AssetsBase.Path, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.RawQuery != "" {
+				versionAssets, err := s.GetResources("assets", r.URL.RawQuery)
+				if err != nil {
+					http.Error(w, "version assets error: "+err.Error(), http.StatusInternalServerError)
+					return
+				}
+				assetsFileServer = http.FileServer(versionAssets)
+			}
 			setCacheControl(w, r, cacheMaxAgeLong)
 			assetsFileServer.ServeHTTP(w, r)
 		})))
@@ -119,6 +132,7 @@ func (s *Site) Handler() http.Handler {
 			content, err := s.Content.OpenVersion(r.Context(), contentVersion)
 			if err != nil {
 				w.Header().Set("Cache-Control", cacheMaxAge0)
+
 				if os.IsNotExist(err) {
 					http.Error(w, "content version not found", http.StatusNotFound)
 				} else {
@@ -140,7 +154,7 @@ func (s *Site) Handler() http.Handler {
 			// Version not found.
 			if !os.IsNotExist(err) {
 				w.Header().Set("Cache-Control", cacheMaxAge0)
-				http.Error(w, "content version error: "+err.Error(), http.StatusInternalServerError)
+				http.Error(w, "content version error: "+err.Error(), http.StatusNotFound)
 				return
 			}
 			data.ContentVersionNotFoundError = true