Skip to content

Commit

Permalink
internal/postgres: add WithImports fieldset to GetDirectory
Browse files Browse the repository at this point in the history
GetDirectory now supports the WithImports fieldset, which lets the
caller fetch only the imports.

The Package.Imports field is moved to Directory.Imports.

For golang/go#39629.

Change-Id: I5d597555fa372cc1ee70390460d349fa83f31425
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/251272
Reviewed-by: Jamal Carvalho <jamal@golang.org>
  • Loading branch information
julieqiu committed Aug 28, 2020
1 parent db8f07c commit dee9bdb
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 69 deletions.
3 changes: 2 additions & 1 deletion internal/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ type Directory struct {
DirectoryMeta
Readme *Readme
Package *Package
Imports []string
}

// PackageMeta represents the metadata of a package in a module version.
Expand Down Expand Up @@ -160,7 +161,6 @@ type Package struct {
Name string
Path string
Documentation *Documentation
Imports []string
}

// Documentation is the rendered documentation for a given package
Expand Down Expand Up @@ -296,6 +296,7 @@ const StringFieldMissing = "!MISSING"
const (
WithReadme FieldSet = 1 << iota
WithDocumentation
WithImports
)

// LegacyDirectory represents a directory in a module version, and all of the
Expand Down
6 changes: 3 additions & 3 deletions internal/fetch/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,16 @@ func moduleDirectories(modulePath string,
}
if pkg, ok := pkgLookup[dirPath]; ok {
dir.Package = &internal.Package{
Path: pkg.Path,
Name: pkg.Name,
Imports: pkg.Imports,
Path: pkg.Path,
Name: pkg.Name,
Documentation: &internal.Documentation{
GOOS: pkg.GOOS,
GOARCH: pkg.GOARCH,
Synopsis: pkg.Synopsis,
HTML: pkg.DocumentationHTML,
},
}
dir.Imports = pkg.Imports
}
directories = append(directories, dir)
}
Expand Down
82 changes: 41 additions & 41 deletions internal/fetch/fetchdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ var moduleOnePackage = &testModule{
Documentation: &internal.Documentation{
Synopsis: "package foo exports a helpful constant.",
},
Imports: []string{"net/http"},
},
Imports: []string{"net/http"},
},
},
},
Expand Down Expand Up @@ -156,8 +156,8 @@ var moduleMultiPackage = &testModule{
Synopsis: "package foo",
HTML: html("FooBar returns the string &#34;foo bar&#34;."),
},
Imports: []string{"fmt", "github.com/my/module/bar"},
},
Imports: []string{"fmt", "github.com/my/module/bar"},
},
},
},
Expand Down Expand Up @@ -467,8 +467,8 @@ var moduleNonRedist = &testModule{
Synopsis: "package foo",
HTML: html("FooBar returns the string"),
},
Imports: []string{"fmt", "github.com/my/module/bar"},
},
Imports: []string{"fmt", "github.com/my/module/bar"},
},
},
},
Expand Down Expand Up @@ -766,25 +766,25 @@ var moduleStd = &testModule{
Documentation: &internal.Documentation{
Synopsis: "Pprof interprets and displays profiles of Go programs.",
},
Imports: []string{
"cmd/internal/objfile",
"crypto/tls",
"debug/dwarf",
"fmt",
"github.com/google/pprof/driver",
"github.com/google/pprof/profile",
"golang.org/x/crypto/ssh/terminal",
"io",
"io/ioutil",
"net/http",
"net/url",
"os",
"regexp",
"strconv",
"strings",
"sync",
"time",
},
},
Imports: []string{
"cmd/internal/objfile",
"crypto/tls",
"debug/dwarf",
"fmt",
"github.com/google/pprof/driver",
"github.com/google/pprof/profile",
"golang.org/x/crypto/ssh/terminal",
"io",
"io/ioutil",
"net/http",
"net/url",
"os",
"regexp",
"strconv",
"strings",
"sync",
"time",
},
},
{
Expand All @@ -797,8 +797,8 @@ var moduleStd = &testModule{
Documentation: &internal.Documentation{
Synopsis: "Package context defines the Context type, which carries deadlines, cancelation signals, and other request-scoped values across API boundaries and between processes.",
},
Imports: []string{"errors", "fmt", "reflect", "sync", "time"},
},
Imports: []string{"errors", "fmt", "reflect", "sync", "time"},
},
{
DirectoryMeta: internal.DirectoryMeta{
Expand All @@ -816,23 +816,23 @@ var moduleStd = &testModule{
Documentation: &internal.Documentation{
Synopsis: "Package json implements encoding and decoding of JSON as defined in RFC 7159.",
},
Imports: []string{
"bytes",
"encoding",
"encoding/base64",
"errors",
"fmt",
"io",
"math",
"reflect",
"sort",
"strconv",
"strings",
"sync",
"unicode",
"unicode/utf16",
"unicode/utf8",
},
},
Imports: []string{
"bytes",
"encoding",
"encoding/base64",
"errors",
"fmt",
"io",
"math",
"reflect",
"sort",
"strconv",
"strings",
"sync",
"unicode",
"unicode/utf16",
"unicode/utf8",
},
},
{
Expand All @@ -852,12 +852,12 @@ var moduleStd = &testModule{
Path: "flag",
V1Path: "flag",
},
Imports: []string{"errors", "fmt", "io", "os", "reflect", "sort", "strconv", "strings", "time"},
Package: &internal.Package{
Name: "flag",
Documentation: &internal.Documentation{
Synopsis: "Package flag implements command-line flag parsing.",
},
Imports: []string{"errors", "fmt", "io", "os", "reflect", "sort", "strconv", "strings", "time"},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/fetch/test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func cleanFetchResult(fr *FetchResult, detector *licenses.Detector) *FetchResult
Name: dir.Package.Name,
Synopsis: dir.Package.Documentation.Synopsis,
DocumentationHTML: dir.Package.Documentation.HTML,
Imports: dir.Package.Imports,
Imports: dir.Imports,
GOOS: dir.Package.Documentation.GOOS,
GOARCH: dir.Package.Documentation.GOARCH,
IsRedistributable: dir.IsRedistributable,
Expand Down
6 changes: 3 additions & 3 deletions internal/postgres/details_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ func TestPostgres_GetImportsAndImportedBy(t *testing.T) {
pkg1.Imports = nil
pkg2.Imports = []string{pkg1.Path}
pkg3.Imports = []string{pkg2.Path, pkg1.Path}
m1.Directories[1].Package.Imports = pkg1.Imports
m2.Directories[1].Package.Imports = pkg2.Imports
m3.Directories[1].Package.Imports = pkg3.Imports
m1.Directories[1].Imports = pkg1.Imports
m2.Directories[1].Imports = pkg2.Imports
m3.Directories[1].Imports = pkg3.Imports

for _, tc := range []struct {
name, path, modulePath, version string
Expand Down
17 changes: 9 additions & 8 deletions internal/postgres/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ func (db *DB) GetDirectory(ctx context.Context, fullPath, modulePath, version st
}
dir.Readme = readme
}

if field&internal.WithDocumentation != 0 {
doc, err := db.getDocumentation(ctx, dir.PathID)
if err != nil && !errors.Is(err, derrors.NotFound) {
Expand All @@ -122,6 +121,15 @@ func (db *DB) GetDirectory(ctx context.Context, fullPath, modulePath, version st
}
}
}
if field&internal.WithImports != 0 {
imports, err := db.getImports(ctx, dir.PathID)
if err != nil {
return nil, err
}
if len(imports) > 0 {
dir.Imports = imports
}
}
if field == internal.AllFields {
dmeta, err := db.GetDirectoryMeta(ctx, fullPath, modulePath, version)
if err != nil {
Expand All @@ -134,13 +142,6 @@ func (db *DB) GetDirectory(ctx context.Context, fullPath, modulePath, version st
}
dir.Package.Name = dmeta.Name
}
imports, err := db.getImports(ctx, dir.PathID)
if err != nil {
return nil, err
}
if len(imports) > 0 {
dir.Package.Imports = imports
}
}
if !db.bypassLicenseCheck {
dir.RemoveNonRedistributableData()
Expand Down
15 changes: 10 additions & 5 deletions internal/postgres/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ func TestGetDirectoryFieldSet(t *testing.T) {
t.Fatal(err)
}

cleanFields := func(dir *internal.Directory) {
cleanFields := func(dir *internal.Directory, field internal.FieldSet) {
// Remove fields based on the FieldSet specified.
dir.DirectoryMeta = internal.DirectoryMeta{
Path: dir.Path,
Expand All @@ -625,8 +625,10 @@ func TestGetDirectoryFieldSet(t *testing.T) {
Version: dir.Version,
},
}
if field != internal.WithImports {
dir.Imports = nil
}
if dir.Package != nil {
dir.Package.Imports = nil
dir.Package.Name = ""
}
}
Expand Down Expand Up @@ -663,7 +665,7 @@ func TestGetDirectoryFieldSet(t *testing.T) {
cmpopts.IgnoreFields(licenses.Metadata{}, "Coverage"),
cmpopts.IgnoreFields(internal.DirectoryMeta{}, "PathID"),
}
cleanFields(test.want)
cleanFields(test.want, test.field)
if diff := cmp.Diff(test.want, got, opts...); diff != "" {
t.Errorf("mismatch (-want, +got):\n%s", diff)
}
Expand All @@ -672,7 +674,7 @@ func TestGetDirectoryFieldSet(t *testing.T) {
}

func newVdir(path, modulePath, version string, readme *internal.Readme, pkg *internal.Package) *internal.Directory {
return &internal.Directory{
dir := &internal.Directory{
DirectoryMeta: internal.DirectoryMeta{
ModuleInfo: *sample.ModuleInfo(modulePath, version),
Path: path,
Expand All @@ -683,6 +685,10 @@ func newVdir(path, modulePath, version string, readme *internal.Readme, pkg *int
Readme: readme,
Package: pkg,
}
if pkg != nil {
dir.Imports = sample.Imports
}
return dir
}

func newPackage(name, path string) *internal.Package {
Expand All @@ -695,7 +701,6 @@ func newPackage(name, path string) *internal.Package {
GOOS: sample.GOOS,
GOARCH: sample.GOARCH,
},
Imports: sample.Imports,
}
}

Expand Down
8 changes: 4 additions & 4 deletions internal/postgres/insert_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ func insertDirectories(ctx context.Context, db *database.DB, m *internal.Module,
return m.Directories[i].Path < m.Directories[j].Path
})
for _, d := range m.Directories {
if d.Package != nil && len(d.Package.Imports) > 1 {
sort.Strings(d.Package.Imports)
if d.Package != nil && len(d.Imports) > 1 {
sort.Strings(d.Imports)
}
}
var (
Expand Down Expand Up @@ -432,8 +432,8 @@ func insertDirectories(ctx context.Context, db *database.DB, m *internal.Module,
return errors.New("saveModule: package missing Documentation.HTML")
}
pathToDoc[d.Path] = d.Package.Documentation
if len(d.Package.Imports) > 0 {
pathToImports[d.Path] = d.Package.Imports
if len(d.Imports) > 0 {
pathToImports[d.Path] = d.Imports
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/testing/sample/sample.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,10 @@ func DirectoryForPackage(pkg *internal.LegacyPackage) *internal.Directory {
Licenses: pkg.Licenses,
V1Path: pkg.V1Path,
},
Imports: pkg.Imports,
Package: &internal.Package{
Name: pkg.Name,
Path: pkg.Path,
Imports: pkg.Imports,
Name: pkg.Name,
Path: pkg.Path,
Documentation: &internal.Documentation{
Synopsis: pkg.Synopsis,
HTML: pkg.DocumentationHTML,
Expand Down

0 comments on commit dee9bdb

Please sign in to comment.