From c8ca51f9216e606287b699308f221486c1b03b54 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 14 Oct 2023 18:34:42 -0700 Subject: [PATCH 1/5] Add support for sub-indexes
Background Sally renders two kinds of pages: - packages: These are for packages defined in sally.yaml and any route under the package path. - indexes: These list available packages. The latter--indexes was previously only supported at '/', the root page. This leads to a slight UX issue: if you have a package with a / in its name (e.g. net/metrics): - example.com/net/metrics gives you the package page - example.com/ lists net/metrics - However, example.com/net fails with a 404
This adds support for index pages on all parents of package pages. Therefore, if example.com/net/metrics exists, example.com/net will list all packages defined under that path. To make this possible, the internals have been rewritten to some degree: CreateHandler no longer builds an http.ServeMux with a bunch of pre-registered http.Handlers. It now builds a custom http.Handler with its own routing. Routing is backed by a new pathTree abstraction. pathTree is a specialized string-value storage with support for inheriting values from a parent. It has the following methods: - `Set(path, v)` associates v with path. - `Lookup(path)` returns the value at path, or the value inherited from the closest ancestor. - `ListByPath(path)` lists all values at or under the given path. For additional confidence in the implementation of pathTree, a property test based on rapid is included. Resolves #31 --- .github/workflows/go.yml | 2 +- CHANGELOG.md | 5 + Dockerfile | 2 +- go.mod | 5 +- go.sum | 2 + handler.go | 188 +++++++++++++----------- handler_test.go | 64 ++++++++- path_tree.go | 120 ++++++++++++++++ path_tree_test.go | 304 +++++++++++++++++++++++++++++++++++++++ sally.yaml | 2 + templates/index.html | 6 +- templates/package.html | 6 +- 12 files changed, 610 insertions(+), 96 deletions(-) create mode 100644 path_tree.go create mode 100644 path_tree_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index a04b4ab..9c644e1 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -20,7 +20,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v4 with: - go-version: 1.20.x + go-version: 1.21.x cache: true - name: Lint diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f67c60..eb44942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Added +- Generate a package listing for sub-paths + that match a subset of the known packages. + ## [1.4.0] ### Added - Publish a Docker image to GitHub Container Registry. diff --git a/Dockerfile b/Dockerfile index d6033ac..9fb023c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # It does not include a sally configuration. # A /sally.yaml file is required for this to run. -FROM golang:1.19-alpine +FROM golang:1.21-alpine COPY . /build WORKDIR /build diff --git a/go.mod b/go.mod index ee31bf0..3d1c17d 100644 --- a/go.mod +++ b/go.mod @@ -1,11 +1,14 @@ module go.uber.org/sally -go 1.18 +go 1.21 + +toolchain go1.21.3 require ( github.com/stretchr/testify v1.8.4 golang.org/x/net v0.15.0 gopkg.in/yaml.v3 v3.0.1 + pgregory.net/rapid v1.1.0 ) require ( diff --git a/go.sum b/go.sum index e4ac672..d3d2117 100644 --- a/go.sum +++ b/go.sum @@ -16,3 +16,5 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= +pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= diff --git a/handler.go b/handler.go index 18dd1f4..2540a98 100644 --- a/handler.go +++ b/handler.go @@ -1,10 +1,11 @@ package main import ( + "errors" "fmt" "html/template" "net/http" - "sort" + "path" "strings" "go.uber.org/sally/templates" @@ -17,112 +18,131 @@ var ( template.New("package.html").Parse(templates.Package)) ) -// CreateHandler creates a Sally http.Handler -func CreateHandler(config *Config) http.Handler { - mux := http.NewServeMux() +// Handler handles inbound HTTP requests. +// +// It provides the following endpoints: +// +// GET / +// Index page listing all packages. +// GET / +// Package page for the given package. +// GET / +// Page listing packages under the given directory, +// assuming that there's no package with the given name. +// GET // +// Package page for the given subpackage. +type Handler struct { + pkgs pathTree[*sallyPackage] +} - pkgs := make([]packageInfo, 0, len(config.Packages)) +// CreateHandler builds a new handler +// with the provided package configuration. +func CreateHandler(config *Config) *Handler { + var pkgs pathTree[*sallyPackage] for name, pkg := range config.Packages { - handler := newPackageHandler(config, name, pkg) - // Double-register so that "/foo" - // does not redirect to "/foo/" with a 300. - mux.Handle("/"+name, handler) - mux.Handle("/"+name+"/", handler) - - pkgs = append(pkgs, packageInfo{ + baseURL := config.URL + if pkg.URL != "" { + // Package-specific override for the base URL. + baseURL = pkg.URL + } + modulePath := path.Join(baseURL, name) + docURL := "https://" + path.Join(config.Godoc.Host, modulePath) + + pkgs.Set(name, &sallyPackage{ Desc: pkg.Desc, - ImportPath: handler.canonicalURL, - GitURL: handler.gitURL, - GodocHome: handler.godocHost + "/" + handler.canonicalURL, + ModulePath: modulePath, + DocURL: docURL, + GitURL: pkg.Repo, }) } - sort.Slice(pkgs, func(i, j int) bool { - return pkgs[i].ImportPath < pkgs[j].ImportPath - }) - mux.Handle("/", &indexHandler{pkgs: pkgs}) - - return mux -} -type indexHandler struct { - pkgs []packageInfo + return &Handler{ + pkgs: pkgs, + } } -type packageInfo struct { - Desc string // package description - ImportPath string // canonical import path - GitURL string // URL of the Git repository - GodocHome string // documentation home URL -} +var _ http.Handler = (*Handler)(nil) -func (h *indexHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Index handler only supports '/'. - // ServeMux will call us for any '/foo' that is not a known package. - if r.Method != http.MethodGet || r.URL.Path != "/" { - http.NotFound(w, r) - return - } +type sallyPackage struct { + // Canonical import path for the package. + ModulePath string - data := struct{ Packages []packageInfo }{ - Packages: h.pkgs, - } - if err := indexTemplate.Execute(w, data); err != nil { - http.Error(w, err.Error(), 500) - } -} + // Description of the package, if any. + Desc string -type packageHandler struct { - // Hostname of the godoc server, e.g. "godoc.org". - godocHost string + // URL at which documentation for the package can be found. + DocURL string - // Name of the package relative to the vanity base URL. - // For example, "zap" for "go.uber.org/zap". - name string + // URL at which the Git repository is hosted. + GitURL string +} - // Path at which the Git repository is hosted. - // For example, "github.com/uber-go/zap". - gitURL string +func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if err := h.serveHTTP(w, r); err != nil { + if herr := new(httpError); errors.As(err, &herr) { + http.Error(w, herr.Message, herr.Code) + } else { + http.Error(w, err.Error(), 500) + } + } +} - // Canonical import path for the package. - canonicalURL string +// httpError indicates that an HTTP error occurred. +// +// The caller will write the error code and message to the response. +type httpError struct { + Code int // HTTP status code + Message string // error message } -func newPackageHandler(cfg *Config, name string, pkg PackageConfig) *packageHandler { - baseURL := cfg.URL - if pkg.URL != "" { - baseURL = pkg.URL +func httpErrorf(code int, format string, args ...interface{}) error { + return &httpError{ + Code: code, + Message: fmt.Sprintf(format, args...), } - canonicalURL := fmt.Sprintf("%s/%s", baseURL, name) +} - return &packageHandler{ - godocHost: cfg.Godoc.Host, - name: name, - canonicalURL: canonicalURL, - gitURL: pkg.Repo, - } +func (e *httpError) Error() string { + return fmt.Sprintf("status %d: %s", e.Code, e.Message) } -func (h *packageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { +// serveHTTP is similar to ServeHTTP, except it returns an error. +// +// If it returns an httpError, +// the caller will write the error code and message to the response. +func (h *Handler) serveHTTP(w http.ResponseWriter, r *http.Request) error { if r.Method != http.MethodGet { - http.NotFound(w, r) - return + return httpErrorf(http.StatusNotFound, "method %q not allowed", r.Method) } - // Extract the relative path to subpackages, if any. - // "/foo/bar" => "/bar" - // "/foo" => "" - relPath := strings.TrimPrefix(r.URL.Path, "/"+h.name) - - data := struct { - Repo string - CanonicalURL string - GodocURL string - }{ - Repo: h.gitURL, - CanonicalURL: h.canonicalURL, - GodocURL: fmt.Sprintf("https://%s/%s%s", h.godocHost, h.canonicalURL, relPath), + path := strings.TrimPrefix(strings.TrimSuffix(r.URL.Path, "/"), "/") + + if pkg, suffix, ok := h.pkgs.Lookup(path); ok { + return h.servePackage(w, pkg, suffix) } - if err := packageTemplate.Execute(w, data); err != nil { - http.Error(w, err.Error(), 500) + return h.serveIndex(w, path, h.pkgs.ListByPath(path)) +} + +func (h *Handler) servePackage(w http.ResponseWriter, pkg *sallyPackage, suffix string) error { + return packageTemplate.Execute(w, + struct { + ModulePath string + GitURL string + DocURL string + }{ + ModulePath: pkg.ModulePath, + GitURL: pkg.GitURL, + DocURL: pkg.DocURL + suffix, + }) +} + +func (h *Handler) serveIndex(w http.ResponseWriter, path string, pkgs []*sallyPackage) error { + if len(pkgs) == 0 { + return httpErrorf(http.StatusNotFound, "no packages found under path: %s", path) } + + return indexTemplate.Execute(w, + struct{ Packages []*sallyPackage }{ + Packages: pkgs, + }) } diff --git a/handler_test.go b/handler_test.go index 370eb45..a95a1f8 100644 --- a/handler_test.go +++ b/handler_test.go @@ -23,6 +23,10 @@ packages: url: go.uberalt.org repo: github.com/uber-go/zap description: A fast, structured logging library. + net/metrics: + repo: github.com/yarpc/metrics + net/something: + repo: github.com/yarpc/something ` @@ -34,6 +38,19 @@ func TestIndex(t *testing.T) { assert.Contains(t, body, "github.com/thriftrw/thriftrw-go") assert.Contains(t, body, "github.com/yarpc/yarpc-go") assert.Contains(t, body, "A fast, structured logging library.") + assert.Contains(t, body, "github.com/yarpc/metrics") + assert.Contains(t, body, "github.com/yarpc/something") +} + +func TestSubindex(t *testing.T) { + rr := CallAndRecord(t, config, "/net") + assert.Equal(t, 200, rr.Code) + + body := rr.Body.String() + assert.NotContains(t, body, "github.com/thriftrw/thriftrw-go") + assert.NotContains(t, body, "github.com/yarpc/yarpc-go") + assert.Contains(t, body, "github.com/yarpc/metrics") + assert.Contains(t, body, "github.com/yarpc/something") } func TestPackageShouldExist(t *testing.T) { @@ -55,7 +72,7 @@ func TestPackageShouldExist(t *testing.T) { func TestNonExistentPackageShould404(t *testing.T) { rr := CallAndRecord(t, config, "/nonexistent") AssertResponse(t, rr, 404, ` -404 page not found +no packages found under path: nonexistent `) } @@ -66,10 +83,10 @@ func TestTrailingSlash(t *testing.T) { - + - Nothing to see here. Please move along. + Nothing to see here. Please move along. `) @@ -161,3 +178,44 @@ func TestPostRejected(t *testing.T) { }) } } + +func BenchmarkHandlerDispatch(b *testing.B) { + handler := CreateHandler(&Config{ + URL: "go.uberalt.org", + Packages: map[string]PackageConfig{ + "zap": { + Repo: "github.com/uber-go/zap", + }, + "net/metrics": { + Repo: "github.com/yarpc/metrics", + }, + }, + }) + resw := new(nopResponseWriter) + + tests := []struct { + name string + path string + }{ + {name: "index", path: "/"}, + {name: "subindex", path: "/net"}, + {name: "package", path: "/zap"}, + {name: "subpackage", path: "/zap/zapcore"}, + } + + for _, tt := range tests { + b.Run(tt.name, func(b *testing.B) { + req := httptest.NewRequest("GET", tt.path, nil) + b.ResetTimer() + for i := 0; i < b.N; i++ { + handler.ServeHTTP(resw, req) + } + }) + } +} + +type nopResponseWriter struct{} + +func (nopResponseWriter) Header() http.Header { return nil } +func (nopResponseWriter) Write([]byte) (int, error) { return 0, nil } +func (nopResponseWriter) WriteHeader(int) {} diff --git a/path_tree.go b/path_tree.go new file mode 100644 index 0000000..b6c4855 --- /dev/null +++ b/path_tree.go @@ -0,0 +1,120 @@ +package main + +import ( + "slices" + "strings" +) + +// pathTree holds values in a tree-like hierarchy +// defined by /-separate paths (e.g. import paths). +// +// It's expensive to add items to the tree, +// but lookups are fast. +// +// Its zero value is a valid empty tree. +type pathTree[T any] struct { + // We track two representations of the tree: + // + // 1. A list of key-value pairs, sorted by key. + // 2. A map of keys to values. + // + // (1) is used for fast scanning of all descendants of a path. + // + // (2) is used for fast lookups of specific paths. + + // paths is a list of all paths in the tree + // that have a value set for them. + paths []string // sorted + + // values[i] is the value for paths[i] + values []T + + // byPath is a map of all paths in the tree + // to their corresponding values. + byPath map[string]T // path => node +} + +// Set sets the value for path to value. +// If path already has a value, it is overwritten. +func (t *pathTree[T]) Set(path string, value T) { + if t.byPath == nil { + t.byPath = make(map[string]T) + } + + t.byPath[path] = value + idx, ok := slices.BinarySearch(t.paths, path) + if ok { + // t.paths[idx] already contains path. + t.values[idx] = value + } else { + t.paths = slices.Insert(t.paths, idx, path) + t.values = slices.Insert(t.values, idx, value) + } +} + +// Lookup retrieves the value for the given path. +// +// If the path doesn't have an explicit value set, +// the value for the closest ancestor with a value is returned. +// +// Suffix is the remaining, unmatched part of path. +// It has a leading '/' if the path wasn't an exact match. +// +// If no value is set for the path or its ancestors, +// ok is false. +func (t *pathTree[T]) Lookup(path string) (value T, suffix string, ok bool) { + idx := len(path) + for idx > 0 { + if value, ok := t.byPath[path[:idx]]; ok { + return value, path[idx:], true + } + + // No match. Trim the last path component. + // "foo/bar" => "foo" + // "foo" => "" + idx = strings.LastIndexByte(path[:idx], '/') + } + + return value, "", false +} + +// ListByPath returns all descendants of path in the tree, +// including the path itself if it exists. +// +// The values are returned in an unspecified order. +func (t *pathTree[T]) ListByPath(path string) []T { + start, end := t.rangeOf(path) + if start == end { + return nil + } + + descendants := make([]T, end-start) + for i := start; i < end; i++ { + descendants[i-start] = t.values[i] + } + return descendants +} + +func (t *pathTree[T]) rangeOf(path string) (start, end int) { + if len(path) == 0 { + return 0, len(t.paths) + } + + start, _ = slices.BinarySearch(t.paths, path) + for idx := start; idx < len(t.paths); idx++ { + if descends(path, t.paths[idx]) { + continue // path is an ancestor of p + } + + // End of matching sequences. + // The next path is not a descendant of path. + return start, idx + } + + // All paths following start are descendants of path. + return start, len(t.paths) +} + +func descends(from, to string) bool { + return to == from || (strings.HasPrefix(to, from) && to[len(from)] == '/') +} diff --git a/path_tree_test.go b/path_tree_test.go new file mode 100644 index 0000000..e5303de --- /dev/null +++ b/path_tree_test.go @@ -0,0 +1,304 @@ +package main + +import ( + "fmt" + "math/rand" + "slices" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "pgregory.net/rapid" +) + +func TestPathTree_empty(t *testing.T) { + var tree pathTree[int] + + _, _, ok := tree.Lookup("") + assert.False(t, ok) + + _, _, ok = tree.Lookup("foo") + assert.False(t, ok) + + assert.Empty(t, tree.ListByPath("")) +} + +func TestPathTree(t *testing.T) { + var tree pathTree[int] + mustHave := func(path string, want int, wantSuffix string) { + t.Helper() + + v, suffix, ok := tree.Lookup(path) + require.True(t, ok, "path %q", path) + assert.Equal(t, v, want, "path %q", path) + assert.Equal(t, wantSuffix, suffix, "path %q", path) + } + + mustNotHave := func(path string) { + t.Helper() + + _, _, ok := tree.Lookup(path) + require.False(t, ok, "path %q", path) + } + + mustList := func(path string, want ...int) { + t.Helper() + slices.Sort(want) + + got := tree.ListByPath(path) + slices.Sort(got) + + assert.Equal(t, want, got, "path %q", path) + } + + tree.Set("foo", 10) + t.Run("single", func(t *testing.T) { + mustHave("foo", 10, "") + mustHave("foo/bar", 10, "/bar") + mustHave("foo/bar/baz", 10, "/bar/baz") + mustNotHave("") + mustNotHave("bar") + mustNotHave("bar/baz") + + t.Run("list", func(t *testing.T) { + mustList("", 10) + mustList("foo", 10) + mustList("foo/bar") + }) + }) + + // Override a descendant value. + t.Run("descendant", func(t *testing.T) { + tree.Set("foo/bar", 20) + mustHave("foo", 10, "") + mustHave("foo/bar", 20, "") + mustHave("foo/bar/baz", 20, "/baz") + + t.Run("list", func(t *testing.T) { + mustList("", 10, 20) + mustList("foo", 10, 20) + mustList("foo/bar", 20) + mustList("foo/bar/baz") + }) + }) + + // Add a sibling. + t.Run("sibling", func(t *testing.T) { + tree.Set("bar", 30) + mustHave("bar", 30, "") + mustHave("bar/baz", 30, "/baz") + + t.Run("list", func(t *testing.T) { + mustList("", 10, 20, 30) + mustList("foo", 10, 20) + mustList("bar", 30) + mustList("bar/baz") + }) + }) + + // Replace an existing value. + t.Run("replace", func(t *testing.T) { + tree.Set("bar", 40) + mustHave("bar", 40, "") + mustHave("bar/baz", 40, "/baz") + + t.Run("list", func(t *testing.T) { + mustList("", 10, 20, 40) + mustList("foo", 10, 20) + mustList("bar", 40) + mustList("bar/baz") + }) + }) +} + +func TestPathTreeRapid(t *testing.T) { + pathGen := rapid.StringMatching(`[a-z]+(/[a-z]+)*`) + valueGen := rapid.Int() + + rapid.Check(t, func(t *rapid.T) { + var tree pathTree[int] + + // Exact lookup table. + exact := make(map[string]int) + var paths []string // known paths + + knownPathGen := rapid.Deferred(func() *rapid.Generator[string] { + return rapid.SampledFrom(paths) + }) + + drawKnownPath := func(t *rapid.T) string { + if len(paths) == 0 { + t.Skip() + } + return knownPathGen.Draw(t, "knownPath") + } + + t.Repeat(map[string]func(*rapid.T){ + "Set": func(t *rapid.T) { + path := pathGen.Draw(t, "path") + if _, ok := exact[path]; ok { + // Already set. + // Overwrite will handle this. + t.Skip() + } + + value := valueGen.Draw(t, "value") + tree.Set(path, value) + exact[path] = value + paths = append(paths, path) + }, + "Overwrite": func(t *rapid.T) { + path := drawKnownPath(t) + value := valueGen.Draw(t, "value") + + tree.Set(path, value) + exact[path] = value + // paths already contains path. + }, + "ExactLookup": func(t *rapid.T) { + if len(paths) == 0 { + t.Skip() + } + + path := drawKnownPath(t) + want := exact[path] + + got, suffix, ok := tree.Lookup(path) + assert.True(t, ok, "path %q", path) + assert.Equal(t, want, got, "path %q", path) + assert.Empty(t, suffix, "path %q", path) + }, + "DescendantLookup": func(t *rapid.T) { + parentPath := drawKnownPath(t) + want := exact[parentPath] + + var childPath string + for { + childPath = "/" + pathGen.Draw(t, "childPath") + if _, ok := exact[parentPath+childPath]; !ok { + break // found a unique child path + } + } + + path := parentPath + childPath + got, suffix, ok := tree.Lookup(path) + assert.True(t, ok, "path %q", path) + assert.Equal(t, want, got, "path %q", path) + assert.Equal(t, childPath, suffix, "path %q", path) + }, + "ListAll": func(t *rapid.T) { + var want []int + for _, v := range exact { + want = append(want, v) + } + slices.Sort(want) + + got := tree.ListByPath("") + slices.Sort(got) + + assert.Equal(t, want, got) + }, + "ListSubset": func(t *rapid.T) { + path := drawKnownPath(t) + + var want []int + for p, v := range exact { + if descends(path, p) { + want = append(want, v) + } + } + slices.Sort(want) + + got := tree.ListByPath(path) + slices.Sort(got) + + if len(want) == 0 { + // Guard against nil != empty. + assert.Empty(t, got, "path %q", path) + } else { + assert.Equal(t, want, got, "path %q", path) + } + }, + }) + }) +} + +func BenchmarkPathTreeDeep(b *testing.B) { + depths := []int{10, 100} + widths := []int{10, 100} + for _, depth := range depths { + b.Run(fmt.Sprintf("depth=%d", depth), func(b *testing.B) { + for _, width := range widths { + b.Run(fmt.Sprintf("width=%d", width), func(b *testing.B) { + benchmarkPathTree(b, depth, width) + }) + } + }) + } +} + +func benchmarkPathTree(b *testing.B, Depth, Width int) { + var ( + tree pathTree[int] + depthpb strings.Builder + ) + paths := make([]string, 0, Depth*Width) + for i := 0; i < Depth; i++ { + if depthpb.Len() > 0 { + depthpb.WriteByte('/') + } + depthpb.WriteString("a") + + depthPath := depthpb.String() + for j := 0; j < Width; j++ { + path := depthPath + "/" + strconv.Itoa(j) + paths = append(paths, path) + tree.Set(path, i+i) + } + } + + b.ResetTimer() + + b.Run("LookupExact", func(b *testing.B) { + path := paths[rand.Intn(len(paths))] + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, _, ok := tree.Lookup(path) + require.True(b, ok) + } + }) + }) + + b.Run("LookupDescendant", func(b *testing.B) { + path := paths[rand.Intn(len(paths))] + strings.Repeat("/xyz", 10) + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, _, ok := tree.Lookup(path) + require.True(b, ok) + } + }) + }) + + b.Run("ListSubtree", func(b *testing.B) { + path := paths[rand.Intn(len(paths))] + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + if len(tree.ListByPath(path)) == 0 { + b.Fatal("unexpected empty list") + } + } + }) + }) + + b.Run("ListAll", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + if len(tree.ListByPath("")) == 0 { + b.Fatal("unexpected empty list") + } + } + }) + }) +} diff --git a/sally.yaml b/sally.yaml index c9a3af8..3055d87 100644 --- a/sally.yaml +++ b/sally.yaml @@ -6,3 +6,5 @@ packages: description: A customizable implementation of Thrift. yarpc: repo: github.com/yarpc/yarpc-go + net/metrics: + repo: github.com/yarpc/metrics diff --git a/templates/index.html b/templates/index.html index fa061f1..7ffc1a6 100644 --- a/templates/index.html +++ b/templates/index.html @@ -29,15 +29,15 @@
Package: - {{ .ImportPath }} + {{ .ModulePath }}
Source: {{ .GitURL }}
- - Go Reference + + Go Reference
diff --git a/templates/package.html b/templates/package.html index a85ff77..bd56c38 100644 --- a/templates/package.html +++ b/templates/package.html @@ -1,10 +1,10 @@ - - + + - Nothing to see here. Please move along. + Nothing to see here. Please move along. From 30ef30e8334d5954cbfda37d1b51c569141bd09d Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Mon, 16 Oct 2023 21:08:31 -0700 Subject: [PATCH 2/5] Simplify the implementation Switch back to http.ServeMux for package handlers, and move the scanning logic to the index handler. --- go.mod | 1 - go.sum | 2 - handler.go | 169 ++++++++++++++++---------- handler_test.go | 87 ++++++++++++- path_tree.go | 120 ------------------ path_tree_test.go | 304 ---------------------------------------------- 6 files changed, 189 insertions(+), 494 deletions(-) delete mode 100644 path_tree.go delete mode 100644 path_tree_test.go diff --git a/go.mod b/go.mod index 3d1c17d..91027f4 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/stretchr/testify v1.8.4 golang.org/x/net v0.15.0 gopkg.in/yaml.v3 v3.0.1 - pgregory.net/rapid v1.1.0 ) require ( diff --git a/go.sum b/go.sum index d3d2117..e4ac672 100644 --- a/go.sum +++ b/go.sum @@ -16,5 +16,3 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -pgregory.net/rapid v1.1.0 h1:CMa0sjHSru3puNx+J0MIAuiiEV4N0qj8/cMWGBBCsjw= -pgregory.net/rapid v1.1.0/go.mod h1:PY5XlDGj0+V1FCq0o192FdRhpKHGTRIWBgqjDBTrq04= diff --git a/handler.go b/handler.go index 2540a98..f80ecdf 100644 --- a/handler.go +++ b/handler.go @@ -1,11 +1,12 @@ package main import ( - "errors" + "cmp" "fmt" "html/template" "net/http" "path" + "slices" "strings" "go.uber.org/sally/templates" @@ -18,27 +19,22 @@ var ( template.New("package.html").Parse(templates.Package)) ) -// Handler handles inbound HTTP requests. -// -// It provides the following endpoints: +// CreateHandler builds a new handler +// with the provided package configuration. +// The returned handler provides the following endpoints: // // GET / // Index page listing all packages. // GET / -// Package page for the given package. +// Package page for the given package. // GET / // Page listing packages under the given directory, // assuming that there's no package with the given name. // GET // // Package page for the given subpackage. -type Handler struct { - pkgs pathTree[*sallyPackage] -} - -// CreateHandler builds a new handler -// with the provided package configuration. -func CreateHandler(config *Config) *Handler { - var pkgs pathTree[*sallyPackage] +func CreateHandler(config *Config) http.Handler { + mux := http.NewServeMux() + pkgs := make([]*sallyPackage, 0, len(config.Packages)) for name, pkg := range config.Packages { baseURL := config.URL if pkg.URL != "" { @@ -48,22 +44,43 @@ func CreateHandler(config *Config) *Handler { modulePath := path.Join(baseURL, name) docURL := "https://" + path.Join(config.Godoc.Host, modulePath) - pkgs.Set(name, &sallyPackage{ + pkg := &sallyPackage{ + Name: name, Desc: pkg.Desc, ModulePath: modulePath, DocURL: docURL, GitURL: pkg.Repo, - }) - } + } + pkgs = append(pkgs, pkg) - return &Handler{ - pkgs: pkgs, + // Double-register so that "/foo" + // does not redirect to "/foo/" with a 300. + handler := &packageHandler{Pkg: pkg} + mux.Handle("/"+name, handler) + mux.Handle("/"+name+"/", handler) } + + mux.Handle("/", newIndexHandler(pkgs)) + return requireMethod(http.MethodGet, mux) } -var _ http.Handler = (*Handler)(nil) +func requireMethod(method string, handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != method { + http.NotFound(w, r) + return + } + + handler.ServeHTTP(w, r) + }) +} type sallyPackage struct { + // Name of the package. + // + // This is the part after the base URL. + Name string + // Canonical import path for the package. ModulePath string @@ -77,72 +94,94 @@ type sallyPackage struct { GitURL string } -func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if err := h.serveHTTP(w, r); err != nil { - if herr := new(httpError); errors.As(err, &herr) { - http.Error(w, herr.Message, herr.Code) - } else { - http.Error(w, err.Error(), 500) - } - } +type indexHandler struct { + pkgs []*sallyPackage // sorted by name } -// httpError indicates that an HTTP error occurred. -// -// The caller will write the error code and message to the response. -type httpError struct { - Code int // HTTP status code - Message string // error message -} +var _ http.Handler = (*indexHandler)(nil) + +func newIndexHandler(pkgs []*sallyPackage) *indexHandler { + slices.SortFunc(pkgs, func(a, b *sallyPackage) int { + return cmp.Compare(a.Name, b.Name) + }) -func httpErrorf(code int, format string, args ...interface{}) error { - return &httpError{ - Code: code, - Message: fmt.Sprintf(format, args...), + return &indexHandler{ + pkgs: pkgs, } } -func (e *httpError) Error() string { - return fmt.Sprintf("status %d: %s", e.Code, e.Message) -} +func (h *indexHandler) rangeOf(path string) (start, end int) { + if len(path) == 0 { + return 0, len(h.pkgs) + } -// serveHTTP is similar to ServeHTTP, except it returns an error. -// -// If it returns an httpError, -// the caller will write the error code and message to the response. -func (h *Handler) serveHTTP(w http.ResponseWriter, r *http.Request) error { - if r.Method != http.MethodGet { - return httpErrorf(http.StatusNotFound, "method %q not allowed", r.Method) + // If the packages are sorted by name, + // we can scan adjacent packages to find the range of packages + // whose name descends from path. + start, _ = slices.BinarySearchFunc(h.pkgs, path, func(pkg *sallyPackage, path string) int { + return cmp.Compare(pkg.Name, path) + }) + + for idx := start; idx < len(h.pkgs); idx++ { + if !descends(path, h.pkgs[idx].Name) { + // End of matching sequences. + // The next path is not a descendant of path. + return start, idx + } } + // All packages following start are descendants of path. + // Return the rest of the packages. + return start, len(h.pkgs) +} + +func (h *indexHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(strings.TrimSuffix(r.URL.Path, "/"), "/") + start, end := h.rangeOf(path) - if pkg, suffix, ok := h.pkgs.Lookup(path); ok { - return h.servePackage(w, pkg, suffix) + // If start == end, then there are no packages + if start == end { + w.WriteHeader(http.StatusNotFound) + fmt.Fprintf(w, "no packages found under path: %v\n", path) + return + } + + err := indexTemplate.Execute(w, + struct{ Packages []*sallyPackage }{ + Packages: h.pkgs[start:end], + }) + if err != nil { + http.Error(w, err.Error(), 500) } - return h.serveIndex(w, path, h.pkgs.ListByPath(path)) } -func (h *Handler) servePackage(w http.ResponseWriter, pkg *sallyPackage, suffix string) error { - return packageTemplate.Execute(w, +type packageHandler struct { + Pkg *sallyPackage +} + +var _ http.Handler = (*packageHandler)(nil) + +func (h *packageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Extract the relative path to subpackages, if any. + // "/foo/bar" => "/bar" + // "/foo" => "" + relPath := strings.TrimPrefix(r.URL.Path, "/"+h.Pkg.Name) + + err := packageTemplate.Execute(w, struct { ModulePath string GitURL string DocURL string }{ - ModulePath: pkg.ModulePath, - GitURL: pkg.GitURL, - DocURL: pkg.DocURL + suffix, + ModulePath: h.Pkg.ModulePath, + GitURL: h.Pkg.GitURL, + DocURL: h.Pkg.DocURL + relPath, }) -} - -func (h *Handler) serveIndex(w http.ResponseWriter, path string, pkgs []*sallyPackage) error { - if len(pkgs) == 0 { - return httpErrorf(http.StatusNotFound, "no packages found under path: %s", path) + if err != nil { + http.Error(w, err.Error(), 500) } +} - return indexTemplate.Execute(w, - struct{ Packages []*sallyPackage }{ - Packages: pkgs, - }) +func descends(from, to string) bool { + return to == from || (strings.HasPrefix(to, from) && to[len(from)] == '/') } diff --git a/handler_test.go b/handler_test.go index a95a1f8..c6470f9 100644 --- a/handler_test.go +++ b/handler_test.go @@ -4,6 +4,7 @@ import ( "io" "net/http" "net/http/httptest" + "sort" "strings" "testing" @@ -83,10 +84,10 @@ func TestTrailingSlash(t *testing.T) { - + - Nothing to see here. Please move along. + Nothing to see here. Please move along. `) @@ -179,6 +180,88 @@ func TestPostRejected(t *testing.T) { } } +func TestIndexHandler_rangeOf(t *testing.T) { + tests := []struct { + desc string + pkgs []*sallyPackage + path string + want []string // names + }{ + { + desc: "empty", + pkgs: []*sallyPackage{ + {Name: "foo"}, + {Name: "bar"}, + }, + want: []string{"foo", "bar"}, + }, + { + desc: "single child", + pkgs: []*sallyPackage{ + {Name: "foo/bar"}, + {Name: "baz"}, + }, + path: "foo", + want: []string{"foo/bar"}, + }, + { + desc: "multiple children", + pkgs: []*sallyPackage{ + {Name: "foo/bar"}, + {Name: "foo/baz"}, + {Name: "qux"}, + {Name: "quux/quuz"}, + }, + path: "foo", + want: []string{"foo/bar", "foo/baz"}, + }, + { + desc: "to end of list", + pkgs: []*sallyPackage{ + {Name: "a"}, + {Name: "b"}, + {Name: "c/d"}, + {Name: "c/e"}, + }, + path: "c", + want: []string{"c/d", "c/e"}, + }, + { + desc: "similar name", + pkgs: []*sallyPackage{ + {Name: "foobar"}, + {Name: "foo/bar"}, + }, + path: "foo", + want: []string{"foo/bar"}, + }, + { + desc: "no match", + pkgs: []*sallyPackage{ + {Name: "foo"}, + {Name: "bar"}, + }, + path: "baz", + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + h := newIndexHandler(tt.pkgs) + start, end := h.rangeOf(tt.path) + + var got []string + for _, pkg := range tt.pkgs[start:end] { + got = append(got, pkg.Name) + } + sort.Strings(got) + sort.Strings(tt.want) + + assert.Equal(t, tt.want, got) + }) + } +} + func BenchmarkHandlerDispatch(b *testing.B) { handler := CreateHandler(&Config{ URL: "go.uberalt.org", diff --git a/path_tree.go b/path_tree.go deleted file mode 100644 index b6c4855..0000000 --- a/path_tree.go +++ /dev/null @@ -1,120 +0,0 @@ -package main - -import ( - "slices" - "strings" -) - -// pathTree holds values in a tree-like hierarchy -// defined by /-separate paths (e.g. import paths). -// -// It's expensive to add items to the tree, -// but lookups are fast. -// -// Its zero value is a valid empty tree. -type pathTree[T any] struct { - // We track two representations of the tree: - // - // 1. A list of key-value pairs, sorted by key. - // 2. A map of keys to values. - // - // (1) is used for fast scanning of all descendants of a path. - // - // (2) is used for fast lookups of specific paths. - - // paths is a list of all paths in the tree - // that have a value set for them. - paths []string // sorted - - // values[i] is the value for paths[i] - values []T - - // byPath is a map of all paths in the tree - // to their corresponding values. - byPath map[string]T // path => node -} - -// Set sets the value for path to value. -// If path already has a value, it is overwritten. -func (t *pathTree[T]) Set(path string, value T) { - if t.byPath == nil { - t.byPath = make(map[string]T) - } - - t.byPath[path] = value - idx, ok := slices.BinarySearch(t.paths, path) - if ok { - // t.paths[idx] already contains path. - t.values[idx] = value - } else { - t.paths = slices.Insert(t.paths, idx, path) - t.values = slices.Insert(t.values, idx, value) - } -} - -// Lookup retrieves the value for the given path. -// -// If the path doesn't have an explicit value set, -// the value for the closest ancestor with a value is returned. -// -// Suffix is the remaining, unmatched part of path. -// It has a leading '/' if the path wasn't an exact match. -// -// If no value is set for the path or its ancestors, -// ok is false. -func (t *pathTree[T]) Lookup(path string) (value T, suffix string, ok bool) { - idx := len(path) - for idx > 0 { - if value, ok := t.byPath[path[:idx]]; ok { - return value, path[idx:], true - } - - // No match. Trim the last path component. - // "foo/bar" => "foo" - // "foo" => "" - idx = strings.LastIndexByte(path[:idx], '/') - } - - return value, "", false -} - -// ListByPath returns all descendants of path in the tree, -// including the path itself if it exists. -// -// The values are returned in an unspecified order. -func (t *pathTree[T]) ListByPath(path string) []T { - start, end := t.rangeOf(path) - if start == end { - return nil - } - - descendants := make([]T, end-start) - for i := start; i < end; i++ { - descendants[i-start] = t.values[i] - } - return descendants -} - -func (t *pathTree[T]) rangeOf(path string) (start, end int) { - if len(path) == 0 { - return 0, len(t.paths) - } - - start, _ = slices.BinarySearch(t.paths, path) - for idx := start; idx < len(t.paths); idx++ { - if descends(path, t.paths[idx]) { - continue // path is an ancestor of p - } - - // End of matching sequences. - // The next path is not a descendant of path. - return start, idx - } - - // All paths following start are descendants of path. - return start, len(t.paths) -} - -func descends(from, to string) bool { - return to == from || (strings.HasPrefix(to, from) && to[len(from)] == '/') -} diff --git a/path_tree_test.go b/path_tree_test.go deleted file mode 100644 index e5303de..0000000 --- a/path_tree_test.go +++ /dev/null @@ -1,304 +0,0 @@ -package main - -import ( - "fmt" - "math/rand" - "slices" - "strconv" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "pgregory.net/rapid" -) - -func TestPathTree_empty(t *testing.T) { - var tree pathTree[int] - - _, _, ok := tree.Lookup("") - assert.False(t, ok) - - _, _, ok = tree.Lookup("foo") - assert.False(t, ok) - - assert.Empty(t, tree.ListByPath("")) -} - -func TestPathTree(t *testing.T) { - var tree pathTree[int] - mustHave := func(path string, want int, wantSuffix string) { - t.Helper() - - v, suffix, ok := tree.Lookup(path) - require.True(t, ok, "path %q", path) - assert.Equal(t, v, want, "path %q", path) - assert.Equal(t, wantSuffix, suffix, "path %q", path) - } - - mustNotHave := func(path string) { - t.Helper() - - _, _, ok := tree.Lookup(path) - require.False(t, ok, "path %q", path) - } - - mustList := func(path string, want ...int) { - t.Helper() - slices.Sort(want) - - got := tree.ListByPath(path) - slices.Sort(got) - - assert.Equal(t, want, got, "path %q", path) - } - - tree.Set("foo", 10) - t.Run("single", func(t *testing.T) { - mustHave("foo", 10, "") - mustHave("foo/bar", 10, "/bar") - mustHave("foo/bar/baz", 10, "/bar/baz") - mustNotHave("") - mustNotHave("bar") - mustNotHave("bar/baz") - - t.Run("list", func(t *testing.T) { - mustList("", 10) - mustList("foo", 10) - mustList("foo/bar") - }) - }) - - // Override a descendant value. - t.Run("descendant", func(t *testing.T) { - tree.Set("foo/bar", 20) - mustHave("foo", 10, "") - mustHave("foo/bar", 20, "") - mustHave("foo/bar/baz", 20, "/baz") - - t.Run("list", func(t *testing.T) { - mustList("", 10, 20) - mustList("foo", 10, 20) - mustList("foo/bar", 20) - mustList("foo/bar/baz") - }) - }) - - // Add a sibling. - t.Run("sibling", func(t *testing.T) { - tree.Set("bar", 30) - mustHave("bar", 30, "") - mustHave("bar/baz", 30, "/baz") - - t.Run("list", func(t *testing.T) { - mustList("", 10, 20, 30) - mustList("foo", 10, 20) - mustList("bar", 30) - mustList("bar/baz") - }) - }) - - // Replace an existing value. - t.Run("replace", func(t *testing.T) { - tree.Set("bar", 40) - mustHave("bar", 40, "") - mustHave("bar/baz", 40, "/baz") - - t.Run("list", func(t *testing.T) { - mustList("", 10, 20, 40) - mustList("foo", 10, 20) - mustList("bar", 40) - mustList("bar/baz") - }) - }) -} - -func TestPathTreeRapid(t *testing.T) { - pathGen := rapid.StringMatching(`[a-z]+(/[a-z]+)*`) - valueGen := rapid.Int() - - rapid.Check(t, func(t *rapid.T) { - var tree pathTree[int] - - // Exact lookup table. - exact := make(map[string]int) - var paths []string // known paths - - knownPathGen := rapid.Deferred(func() *rapid.Generator[string] { - return rapid.SampledFrom(paths) - }) - - drawKnownPath := func(t *rapid.T) string { - if len(paths) == 0 { - t.Skip() - } - return knownPathGen.Draw(t, "knownPath") - } - - t.Repeat(map[string]func(*rapid.T){ - "Set": func(t *rapid.T) { - path := pathGen.Draw(t, "path") - if _, ok := exact[path]; ok { - // Already set. - // Overwrite will handle this. - t.Skip() - } - - value := valueGen.Draw(t, "value") - tree.Set(path, value) - exact[path] = value - paths = append(paths, path) - }, - "Overwrite": func(t *rapid.T) { - path := drawKnownPath(t) - value := valueGen.Draw(t, "value") - - tree.Set(path, value) - exact[path] = value - // paths already contains path. - }, - "ExactLookup": func(t *rapid.T) { - if len(paths) == 0 { - t.Skip() - } - - path := drawKnownPath(t) - want := exact[path] - - got, suffix, ok := tree.Lookup(path) - assert.True(t, ok, "path %q", path) - assert.Equal(t, want, got, "path %q", path) - assert.Empty(t, suffix, "path %q", path) - }, - "DescendantLookup": func(t *rapid.T) { - parentPath := drawKnownPath(t) - want := exact[parentPath] - - var childPath string - for { - childPath = "/" + pathGen.Draw(t, "childPath") - if _, ok := exact[parentPath+childPath]; !ok { - break // found a unique child path - } - } - - path := parentPath + childPath - got, suffix, ok := tree.Lookup(path) - assert.True(t, ok, "path %q", path) - assert.Equal(t, want, got, "path %q", path) - assert.Equal(t, childPath, suffix, "path %q", path) - }, - "ListAll": func(t *rapid.T) { - var want []int - for _, v := range exact { - want = append(want, v) - } - slices.Sort(want) - - got := tree.ListByPath("") - slices.Sort(got) - - assert.Equal(t, want, got) - }, - "ListSubset": func(t *rapid.T) { - path := drawKnownPath(t) - - var want []int - for p, v := range exact { - if descends(path, p) { - want = append(want, v) - } - } - slices.Sort(want) - - got := tree.ListByPath(path) - slices.Sort(got) - - if len(want) == 0 { - // Guard against nil != empty. - assert.Empty(t, got, "path %q", path) - } else { - assert.Equal(t, want, got, "path %q", path) - } - }, - }) - }) -} - -func BenchmarkPathTreeDeep(b *testing.B) { - depths := []int{10, 100} - widths := []int{10, 100} - for _, depth := range depths { - b.Run(fmt.Sprintf("depth=%d", depth), func(b *testing.B) { - for _, width := range widths { - b.Run(fmt.Sprintf("width=%d", width), func(b *testing.B) { - benchmarkPathTree(b, depth, width) - }) - } - }) - } -} - -func benchmarkPathTree(b *testing.B, Depth, Width int) { - var ( - tree pathTree[int] - depthpb strings.Builder - ) - paths := make([]string, 0, Depth*Width) - for i := 0; i < Depth; i++ { - if depthpb.Len() > 0 { - depthpb.WriteByte('/') - } - depthpb.WriteString("a") - - depthPath := depthpb.String() - for j := 0; j < Width; j++ { - path := depthPath + "/" + strconv.Itoa(j) - paths = append(paths, path) - tree.Set(path, i+i) - } - } - - b.ResetTimer() - - b.Run("LookupExact", func(b *testing.B) { - path := paths[rand.Intn(len(paths))] - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - _, _, ok := tree.Lookup(path) - require.True(b, ok) - } - }) - }) - - b.Run("LookupDescendant", func(b *testing.B) { - path := paths[rand.Intn(len(paths))] + strings.Repeat("/xyz", 10) - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - _, _, ok := tree.Lookup(path) - require.True(b, ok) - } - }) - }) - - b.Run("ListSubtree", func(b *testing.B) { - path := paths[rand.Intn(len(paths))] - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - if len(tree.ListByPath(path)) == 0 { - b.Fatal("unexpected empty list") - } - } - }) - }) - - b.Run("ListAll", func(b *testing.B) { - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - if len(tree.ListByPath("")) == 0 { - b.Fatal("unexpected empty list") - } - } - }) - }) -} From 6c18e7821fd72ab9c1001a9415da7b20d19ea659 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Wed, 18 Oct 2023 10:51:44 -0700 Subject: [PATCH 3/5] handler.go: Better formatting on packageTemplate Co-authored-by: Matt Way --- handler.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/handler.go b/handler.go index f80ecdf..f636359 100644 --- a/handler.go +++ b/handler.go @@ -167,16 +167,15 @@ func (h *packageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // "/foo" => "" relPath := strings.TrimPrefix(r.URL.Path, "/"+h.Pkg.Name) - err := packageTemplate.Execute(w, - struct { - ModulePath string - GitURL string - DocURL string - }{ - ModulePath: h.Pkg.ModulePath, - GitURL: h.Pkg.GitURL, - DocURL: h.Pkg.DocURL + relPath, - }) + err := packageTemplate.Execute(w, struct { + ModulePath string + GitURL string + DocURL string + }{ + ModulePath: h.Pkg.ModulePath, + GitURL: h.Pkg.GitURL, + DocURL: h.Pkg.DocURL + relPath, + }) if err != nil { http.Error(w, err.Error(), 500) } From f6f83a4f707062d1b08a62664a31876f4eef8e6c Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Wed, 18 Oct 2023 10:52:10 -0700 Subject: [PATCH 4/5] handler_test: Sort+Equal => ElementsMatch Co-authored-by: Sung Yoon Whang --- handler_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/handler_test.go b/handler_test.go index c6470f9..301fe48 100644 --- a/handler_test.go +++ b/handler_test.go @@ -254,10 +254,7 @@ func TestIndexHandler_rangeOf(t *testing.T) { for _, pkg := range tt.pkgs[start:end] { got = append(got, pkg.Name) } - sort.Strings(got) - sort.Strings(tt.want) - - assert.Equal(t, tt.want, got) + assert.ElementsMatch(t, tt.want, got) }) } } From 322469fe0c37e9423243deafe7d8ffe2fc76da10 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Wed, 18 Oct 2023 11:03:54 -0700 Subject: [PATCH 5/5] oops --- handler_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/handler_test.go b/handler_test.go index 301fe48..e910897 100644 --- a/handler_test.go +++ b/handler_test.go @@ -4,7 +4,6 @@ import ( "io" "net/http" "net/http/httptest" - "sort" "strings" "testing"