diff --git a/content/static/css/stylesheet.css b/content/static/css/stylesheet.css
index c5a6675b..4918f63e 100644
--- a/content/static/css/stylesheet.css
+++ b/content/static/css/stylesheet.css
@@ -582,3 +582,10 @@ code {
.Directory-header {
border-bottom: 1px solid var(--gray-8);
}
+
+.Details-content {
+ margin-left: 40px
+}
+.Details-indent {
+ margin-left: 1.1rem
+}
diff --git a/content/static/html/pages/pkg_importedby.tmpl b/content/static/html/pages/pkg_importedby.tmpl
index 7a1a10e4..079b8b87 100644
--- a/content/static/html/pages/pkg_importedby.tmpl
+++ b/content/static/html/pages/pkg_importedby.tmpl
@@ -9,16 +9,32 @@
{{if .ImportedBy}}
Imported by
- {{template "pagination_summary" .Pagination}} {{pluralize .Pagination.TotalCount "importer"}}
+ Known {{pluralize .Total "importer"}}: {{.Total}}{{if not .TotalIsExact}}+{{end}}
-
- {{range .ImportedBy}}
- - {{.}}
- {{end}}
-
- {{template "pagination_nav" .Pagination}}
+ {{template "sections" .ImportedBy}}
{{else}}
{{template "empty_content" "No known importers for this package!"}}
{{end}}
{{end}}
+
+{{define "sections"}}
+
+ {{range .}}
+ {{template "section" .}}
+ {{end}}
+
+{{end}}
+
+{{define "section"}}
+ {{if .Subs}}
+
+ {{.Prefix}} ({{.NumLines}})
+
+ {{template "sections" .Subs}}
+
+
+ {{else}}
+ {{.Prefix}}
+ {{end}}
+{{end}}
diff --git a/internal/frontend/details.go b/internal/frontend/details.go
index 4a6ad129..1b1a1cbe 100644
--- a/internal/frontend/details.go
+++ b/internal/frontend/details.go
@@ -158,7 +158,7 @@ func fetchDetailsForPackage(ctx context.Context, r *http.Request, tab string, db
case "imports":
return fetchImportsDetails(ctx, db, pkg)
case "importedby":
- return fetchImportedByDetails(ctx, db, pkg, newPaginationParams(r, 100))
+ return fetchImportedByDetails(ctx, db, pkg)
case "licenses":
return fetchPackageLicensesDetails(ctx, db, pkg)
case "readme":
diff --git a/internal/frontend/imports.go b/internal/frontend/imports.go
index 568d02a5..beeb79ee 100644
--- a/internal/frontend/imports.go
+++ b/internal/frontend/imports.go
@@ -59,26 +59,40 @@ func fetchImportsDetails(ctx context.Context, db *postgres.DB, pkg *internal.Ver
// ImportedByDetails contains information for the collection of packages that
// import a given package.
type ImportedByDetails struct {
- Pagination pagination
-
ModulePath string
// ImportedBy is the collection of packages that import the
// given package and are not part of the same module.
- ImportedBy []string
+ // They are organized into a tree of sections by prefix.
+ ImportedBy []*Section
+
+ Total int // number of packages in ImportedBy
+ TotalIsExact bool // if false, then there may be more than Total
}
+const importedByLimit = 7001
+
// fetchImportedByDetails fetches importers for the package version specified by
// path and version from the database and returns a ImportedByDetails.
-func fetchImportedByDetails(ctx context.Context, db *postgres.DB, pkg *internal.VersionedPackage, pageParams paginationParams) (*ImportedByDetails, error) {
-
- importedBy, total, err := db.GetImportedBy(ctx, pkg.Path, pkg.ModulePath, pageParams.limit, pageParams.offset())
+func fetchImportedByDetails(ctx context.Context, db *postgres.DB, pkg *internal.VersionedPackage) (*ImportedByDetails, error) {
+ importedBy, err := db.GetImportedBy(ctx, pkg.Path, pkg.ModulePath, importedByLimit)
if err != nil {
return nil, err
}
+ // If we reached the query limit, then we don't know the total.
+ // Say so, and show one less than the limit.
+ // For example, if the limit is 101 and we get 101 results, then we'll
+ // say there are more than 100, and show the first 100.
+ totalIsExact := true
+ if len(importedBy) == importedByLimit {
+ importedBy = importedBy[:len(importedBy)-1]
+ totalIsExact = false
+ }
+ sections := Sections(importedBy, nextPrefixAccount)
return &ImportedByDetails{
- ModulePath: pkg.VersionInfo.ModulePath,
- ImportedBy: importedBy,
- Pagination: newPagination(pageParams, len(importedBy), total),
+ ModulePath: pkg.VersionInfo.ModulePath,
+ ImportedBy: sections,
+ Total: len(importedBy),
+ TotalIsExact: totalIsExact,
}, nil
}
diff --git a/internal/frontend/imports_test.go b/internal/frontend/imports_test.go
index 7b8b6091..58d2fea1 100644
--- a/internal/frontend/imports_test.go
+++ b/internal/frontend/imports_test.go
@@ -9,7 +9,6 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
- "github.com/google/go-cmp/cmp/cmpopts"
"golang.org/x/discovery/internal"
"golang.org/x/discovery/internal/postgres"
"golang.org/x/discovery/internal/sample"
@@ -114,18 +113,25 @@ func TestFetchImportedByDetails(t *testing.T) {
}{
{
pkg: pkg3,
- wantDetails: &ImportedByDetails{},
+ wantDetails: &ImportedByDetails{TotalIsExact: true},
},
{
pkg: pkg2,
wantDetails: &ImportedByDetails{
- ImportedBy: []string{pkg3.Path},
+ ImportedBy: []*Section{{Prefix: pkg3.Path, NumLines: 0}},
+ Total: 1,
+ TotalIsExact: true,
},
},
{
pkg: pkg1,
wantDetails: &ImportedByDetails{
- ImportedBy: []string{pkg2.Path, pkg3.Path},
+ ImportedBy: []*Section{
+ {Prefix: pkg2.Path, NumLines: 0},
+ {Prefix: pkg3.Path, NumLines: 0},
+ },
+ Total: 2,
+ TotalIsExact: true,
},
},
} {
@@ -133,15 +139,15 @@ func TestFetchImportedByDetails(t *testing.T) {
otherVersion := newVersion("path.to/foo", tc.pkg)
otherVersion.Version = "v1.0.5"
vp := firstVersionedPackage(otherVersion)
- got, err := fetchImportedByDetails(ctx, testDB, vp, paginationParams{limit: 20, page: 1})
+ got, err := fetchImportedByDetails(ctx, testDB, vp)
if err != nil {
- t.Fatalf("fetchImportedByDetails(ctx, db, %q, 1) = %v err = %v, want %v",
+ t.Fatalf("fetchImportedByDetails(ctx, db, %q) = %v err = %v, want %v",
tc.pkg.Path, got, err, tc.wantDetails)
}
tc.wantDetails.ModulePath = vp.VersionInfo.ModulePath
- if diff := cmp.Diff(tc.wantDetails, got, cmpopts.IgnoreFields(ImportedByDetails{}, "Pagination")); diff != "" {
- t.Errorf("fetchImportedByDetails(ctx, db, %q, 1) mismatch (-want +got):\n%s", tc.pkg.Path, diff)
+ if diff := cmp.Diff(tc.wantDetails, got); diff != "" {
+ t.Errorf("fetchImportedByDetails(ctx, db, %q) mismatch (-want +got):\n%s", tc.pkg.Path, diff)
}
})
}
diff --git a/internal/frontend/section.go b/internal/frontend/section.go
new file mode 100644
index 00000000..e428c8e6
--- /dev/null
+++ b/internal/frontend/section.go
@@ -0,0 +1,134 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package frontend
+
+import (
+ "strings"
+)
+
+// A Section represents a collection of lines with a common prefix. The
+// collection is itself divided into sections by prefix, forming a tree.
+type Section struct {
+ Prefix string // prefix for this section, or if Subs==nil, a single line
+ Subs []*Section // subsections
+ NumLines int // total number of lines in subsections
+}
+
+func newSection(prefix string) *Section {
+ return &Section{Prefix: prefix, NumLines: 0}
+}
+
+func (s *Section) add(sub *Section) {
+ s.Subs = append(s.Subs, sub)
+ if sub.Subs == nil {
+ s.NumLines++
+ } else {
+ s.NumLines += sub.NumLines
+ }
+}
+
+// A prefixFunc returns the next prefix of s, given the current prefix.
+// It should return the empty string if there are no more prefixes.
+type prefixFunc func(s, prefix string) string
+
+// Sections transforms a list of lines, which must be sorted, into a list
+// of Sections. Each Section in the result contains all the contiguous lines
+// with the same prefix.
+//
+// The nextPrefix function is responsible for extracting prefixes from lines.
+func Sections(lines []string, nextPrefix prefixFunc) []*Section {
+ s, _ := section("", lines, nextPrefix)
+ return s.Subs
+}
+
+// section collects all lines with the same prefix into a section. It assumes
+// that lines is sorted. It returns the section along with the remaining lines.
+func section(prefix string, lines []string, nextPrefix prefixFunc) (*Section, []string) {
+ s := newSection(prefix)
+ for len(lines) > 0 {
+ l := lines[0]
+ if !strings.HasPrefix(l, prefix) {
+ break
+ }
+
+ np := nextPrefix(l, prefix)
+ var sub *Section
+ if np == "" {
+ sub = newSection(l)
+ lines = lines[1:]
+ } else {
+ sub, lines = section(np, lines, nextPrefix)
+ }
+ s.add(sub)
+ }
+ // Collapse a section with a single subsection, except at top level.
+ if len(s.Subs) == 1 && prefix != "" {
+ s = s.Subs[0]
+ }
+ return s, lines
+}
+
+// nextPrefixAccount is a prefixFunc (see above). Its first argument
+// is an import path, and its second is the previous prefix that it returned
+// for that path, or "" if this is the first prefix.
+//
+// nextPrefixAccount tries to return an initial prefix for path
+// that consists of the "account": the entity controlling the
+// remainder of the path. In the most common case, paths beginning
+// with "github.com", the account is the second path element, the GitHub user or org.
+// So for example, the first prefix of "github.com/google/go-cmp/cmp" is
+// "github.com/google/".
+//
+// nextPrefixAccount returns a second prefix that is one path element past the
+// account. For github.com paths, this is the repo. Continuing the above example,
+// the second prefix is "github.com/google/go-cmp/".
+//
+// nextPrefixAccount does not return any prefixes beyond those two.
+func nextPrefixAccount(path, pre string) string {
+ // If the last prefix consisted of the entire path, then
+ // there is no next prefix.
+ if path == pre {
+ return ""
+ }
+ parts := strings.Split(path, "/")
+ prefix1, acctParts := accountPrefix(parts)
+ if pre == "" {
+ return prefix1
+ }
+ if pre == prefix1 {
+ // Second prefix: one element past the first.
+ // The +1 is safe because we know that pre is shorter than path from
+ // the first test of the function.
+ prefix2 := strings.Join(parts[:len(acctParts)+1], "/")
+ if prefix2 != path {
+ prefix2 += "/"
+ }
+ return prefix2
+ }
+ // No more prefixes after the first two.
+ return ""
+}
+
+// accountPrefix guesses the prefix of the path (split into parts at "/")
+// that corresponds to the account.
+func accountPrefix(parts []string) (string, []string) {
+ // TODO(jba): handle repo import paths like "example.org/foo/bar.hg".
+ var n int // index of account in parts
+ // The first two cases below handle the special cases that the go command does.
+ // See "go help importpath".
+ switch parts[0] {
+ case "github.com", "bitbucket.org", "launchpad.net":
+ n = 1
+ case "hub.jazz.net":
+ n = 2
+ default:
+ // For custom import paths, use the host as the first prefix.
+ n = 0
+ }
+ if n >= len(parts)-1 {
+ return strings.Join(parts, "/"), parts
+ }
+ return strings.Join(parts[:n+1], "/") + "/", parts[:n+1]
+}
diff --git a/internal/frontend/section_test.go b/internal/frontend/section_test.go
new file mode 100644
index 00000000..23ee97a3
--- /dev/null
+++ b/internal/frontend/section_test.go
@@ -0,0 +1,110 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package frontend
+
+import (
+ "testing"
+
+ "github.com/google/go-cmp/cmp"
+)
+
+func TestNextPrefixAccount(t *testing.T) {
+ for _, tc := range []struct {
+ path, want1, want2 string
+ }{
+ {"", "", ""},
+ {"github.com", "github.com", ""},
+ {"github.com/user", "github.com/user", ""},
+ {"github.com/user/repo", "github.com/user/", "github.com/user/repo"},
+ {"github.com/user/repo/more", "github.com/user/", "github.com/user/repo/"},
+ {"hub.jazz.net/git/user/project/more", "hub.jazz.net/git/user/", "hub.jazz.net/git/user/project/"},
+ {"k8s.io/a", "k8s.io/", "k8s.io/a"},
+ {"k8s.io/a/b", "k8s.io/", "k8s.io/a/"},
+ {"example.com", "example.com", ""},
+ {"example.com/foo", "example.com/", "example.com/foo"},
+ {"example.com/foo/bar", "example.com/", "example.com/foo/"},
+ } {
+ got1 := nextPrefixAccount(tc.path, "")
+ if got1 != tc.want1 {
+ t.Errorf(`nextPrefixAccount(%q, "") = %q, want %q`, tc.path, got1, tc.want1)
+ continue
+ }
+ got2 := nextPrefixAccount(tc.path, got1)
+ if got2 != tc.want2 {
+ t.Errorf(`nextPrefixAccount(%q, %q) = %q, want %q`, tc.path, got1, got2, tc.want2)
+ continue
+ }
+ if got2 == "" {
+ continue
+ }
+ got3 := nextPrefixAccount(tc.path, got2)
+ if got3 != "" {
+ t.Errorf(`nextPrefixAccount(%q, %q) = %q, want ""`, tc.path, got2, got3)
+ }
+ }
+}
+
+func TestPrefixSections(t *testing.T) {
+ for _, tc := range []struct {
+ lines []string
+ want []*Section
+ }{
+ {
+ []string{"foo.com/a", "bar.com/a", "baz.com/a"},
+ []*Section{
+ {"foo.com/a", nil, 0},
+ {"bar.com/a", nil, 0},
+ {"baz.com/a", nil, 0},
+ },
+ },
+ {
+ []string{"k8s.io/a", "k8s.io/b", "k8s.io/c"},
+ []*Section{
+ {
+ "k8s.io/",
+ []*Section{
+ {"k8s.io/a", nil, 0},
+ {"k8s.io/b", nil, 0},
+ {"k8s.io/c", nil, 0},
+ },
+ 3,
+ },
+ },
+ },
+ {
+ []string{
+ "github.com/eliben/gocdkx/blob",
+ "github.com/eliben/gocdkx/blob/azureblob",
+ "github.com/eliben/gocdkx/blob/fileblob",
+ "github.com/eliben/gocdkx/internal/docstore/dynamodocstore",
+ "github.com/eliben/gocdkx/internal/testing/octest",
+ "github.com/eliben/gocdkx/internal/trace",
+ "github.com/eliben/gocdkx/pubsub",
+ "github.com/eliben/gocdkx/pubsub/awspubsub",
+ },
+ []*Section{
+ {
+ "github.com/eliben/gocdkx/",
+ []*Section{
+ {"github.com/eliben/gocdkx/blob", nil, 0},
+ {"github.com/eliben/gocdkx/blob/azureblob", nil, 0},
+ {"github.com/eliben/gocdkx/blob/fileblob", nil, 0},
+ {"github.com/eliben/gocdkx/internal/docstore/dynamodocstore", nil, 0},
+ {"github.com/eliben/gocdkx/internal/testing/octest", nil, 0},
+ {"github.com/eliben/gocdkx/internal/trace", nil, 0},
+ {"github.com/eliben/gocdkx/pubsub", nil, 0},
+ {"github.com/eliben/gocdkx/pubsub/awspubsub", nil, 0},
+ },
+ 8,
+ },
+ },
+ },
+ } {
+ got := Sections(tc.lines, nextPrefixAccount)
+ if diff := cmp.Diff(tc.want, got); diff != "" {
+ t.Errorf("%v: mismatch (-want, +got):\n%s", tc.lines, diff)
+ }
+ }
+}
diff --git a/internal/postgres/details.go b/internal/postgres/details.go
index 02155b9a..ad79c44b 100644
--- a/internal/postgres/details.go
+++ b/internal/postgres/details.go
@@ -425,48 +425,46 @@ func (db *DB) GetImports(ctx context.Context, path, version string) (paths []str
return imports, nil
}
+// The limit used in the GetImportedBy query.
+const ImportedByLimit = 7001
+
// GetImportedBy fetches and returns all of the packages that import the
// package with path.
// The returned error may be checked with derrors.IsInvalidArgument to
// determine if it resulted from an invalid package path or version.
-func (db *DB) GetImportedBy(ctx context.Context, path, modulePath string, limit, offset int) (paths []string, total int, err error) {
- defer derrors.Wrap(&err, "GetImportedBy(ctx, %q, %q, %d, %d)", path, modulePath, limit, offset)
+//
+// Instead of supporting pagination, this query runs with a limit.
+func (db *DB) GetImportedBy(ctx context.Context, path, modulePath string, limit int) (paths []string, err error) {
+ defer derrors.Wrap(&err, "GetImportedBy(ctx, %q, %q)", path, modulePath)
if path == "" {
- return nil, 0, xerrors.Errorf("path cannot be empty: %w", derrors.InvalidArgument)
+ return nil, xerrors.Errorf("path cannot be empty: %w", derrors.InvalidArgument)
}
-
query := `
SELECT
- from_path,
- COUNT(*) OVER() as total
- FROM (
- SELECT
- DISTINCT ON (from_path) from_path
- FROM
- imports
- WHERE
- to_path = $1
- AND
- from_module_path <> $2
- ORDER BY
- from_path
- ) t
- LIMIT $3
- OFFSET $4;`
+ DISTINCT from_path
+ FROM
+ imports
+ WHERE
+ to_path = $1
+ AND
+ from_module_path <> $2
+ ORDER BY
+ from_path
+ LIMIT $3`
var importedby []string
collect := func(rows *sql.Rows) error {
var fromPath string
- if err := rows.Scan(&fromPath, &total); err != nil {
+ if err := rows.Scan(&fromPath); err != nil {
return fmt.Errorf("row.Scan(): %v", err)
}
importedby = append(importedby, fromPath)
return nil
}
- if err := db.runQuery(ctx, query, collect, path, modulePath, limit, offset); err != nil {
- return nil, 0, err
+ if err := db.runQuery(ctx, query, collect, path, modulePath, limit); err != nil {
+ return nil, err
}
- return importedby, total, nil
+ return importedby, nil
}
// GetModuleLicenses returns all licenses associated with the given module path and
diff --git a/internal/postgres/details_test.go b/internal/postgres/details_test.go
index aeb54ac3..8047b42a 100644
--- a/internal/postgres/details_test.go
+++ b/internal/postgres/details_test.go
@@ -193,70 +193,36 @@ func TestPostgres_GetImportsAndImportedBy(t *testing.T) {
for _, tc := range []struct {
path, modulePath, version string
- limit, offset int
wantImports []string
wantImportedBy []string
- wantTotalImportedBy int
}{
{
- path: pkg3.Path,
- modulePath: modulePath3,
- version: "v1.3.0",
- limit: 10,
- offset: 0,
- wantImports: pkg3.Imports,
- wantImportedBy: nil,
- wantTotalImportedBy: 0,
+ path: pkg3.Path,
+ modulePath: modulePath3,
+ version: "v1.3.0",
+ wantImports: pkg3.Imports,
+ wantImportedBy: nil,
},
{
- path: pkg2.Path,
- modulePath: modulePath2,
- limit: 10,
- offset: 0,
- version: "v1.2.0",
- wantImports: pkg2.Imports,
- wantImportedBy: []string{pkg3.Path},
- wantTotalImportedBy: 1,
+ path: pkg2.Path,
+ modulePath: modulePath2,
+ version: "v1.2.0",
+ wantImports: pkg2.Imports,
+ wantImportedBy: []string{pkg3.Path},
},
{
- path: pkg1.Path,
- modulePath: modulePath1,
- limit: 10,
- offset: 0,
- version: "v1.1.0",
- wantImports: nil,
- wantImportedBy: []string{pkg2.Path, pkg3.Path},
- wantTotalImportedBy: 2,
+ path: pkg1.Path,
+ modulePath: modulePath1,
+ version: "v1.1.0",
+ wantImports: nil,
+ wantImportedBy: []string{pkg2.Path, pkg3.Path},
},
{
- path: pkg1.Path,
- modulePath: modulePath1,
- version: "v1.1.0",
- limit: 1,
- offset: 0,
- wantImports: nil,
- wantImportedBy: []string{pkg2.Path},
- wantTotalImportedBy: 2,
- },
- {
- path: pkg1.Path,
- modulePath: modulePath1,
- version: "v1.1.0",
- limit: 1,
- offset: 1,
- wantImports: nil,
- wantImportedBy: []string{pkg3.Path},
- wantTotalImportedBy: 2,
- },
- {
- path: pkg1.Path,
- modulePath: modulePath2, // should cause pkg2 to be excluded.
- version: "v1.1.0",
- limit: 10,
- offset: 0,
- wantImports: nil,
- wantImportedBy: []string{pkg3.Path},
- wantTotalImportedBy: 1,
+ path: pkg1.Path,
+ modulePath: modulePath2, // should cause pkg2 to be excluded.
+ version: "v1.1.0",
+ wantImports: nil,
+ wantImportedBy: []string{pkg3.Path},
},
} {
t.Run(tc.path, func(t *testing.T) {
@@ -282,16 +248,12 @@ func TestPostgres_GetImportsAndImportedBy(t *testing.T) {
t.Errorf("testDB.GetImports(%q, %q) mismatch (-want +got):\n%s", tc.path, tc.version, diff)
}
- gotImportedBy, total, err := testDB.GetImportedBy(ctx, tc.path, tc.modulePath, tc.limit, tc.offset)
+ gotImportedBy, err := testDB.GetImportedBy(ctx, tc.path, tc.modulePath, 100)
if err != nil {
t.Fatal(err)
}
- if total != tc.wantTotalImportedBy {
- t.Errorf("testDB.GetImportedBy(%q, %q, %d, %d): total = %d, want %d", tc.path, tc.modulePath, tc.limit, tc.offset, total, tc.wantTotalImportedBy)
- }
-
if diff := cmp.Diff(tc.wantImportedBy, gotImportedBy); diff != "" {
- t.Errorf("testDB.GetImportedBy(%q, %q, %d, %d) mismatch (-want +got):\n%s", tc.path, tc.modulePath, tc.limit, tc.offset, diff)
+ t.Errorf("testDB.GetImportedBy(%q, %q) mismatch (-want +got):\n%s", tc.path, tc.modulePath, diff)
}
})
}