From 506adfeab1ce4a0423fcfd861f0fb728202831a8 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Thu, 11 Jan 2024 16:23:56 -0500 Subject: [PATCH] internal: move UnitMeta.Licenses to Unit Change-Id: If2b3c1acd4830948e2d134517608f89771a7abcb Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/555735 kokoro-CI: kokoro Reviewed-by: Jonathan Amsterdam LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley --- internal/fetch/helper_test.go | 1 - internal/fetch/unit.go | 2 +- internal/frontend/main.go | 2 +- internal/frontend/search_test.go | 4 ++-- internal/postgres/unit.go | 34 +++++++++++++-------------- internal/postgres/unit_test.go | 11 ++++++--- internal/testing/sample/sample.go | 4 +++- internal/unit.go | 2 +- internal/worker/fetch_test.go | 39 +++++++++++++++---------------- internal/worker/refetch_test.go | 6 ++--- 10 files changed, 55 insertions(+), 50 deletions(-) diff --git a/internal/fetch/helper_test.go b/internal/fetch/helper_test.go index d8b0cfba8..219bc4c66 100644 --- a/internal/fetch/helper_test.go +++ b/internal/fetch/helper_test.go @@ -62,7 +62,6 @@ func cleanFetchResult(t *testing.T, fr *FetchResult) *FetchResult { Path: u.Path, Name: u.Name, IsRedistributable: u.IsRedistributable, - Licenses: u.Licenses, } if u.IsPackage() && shouldSetPVS { fr.PackageVersionStates = append( diff --git a/internal/fetch/unit.go b/internal/fetch/unit.go index a6da5d474..237a7cc2b 100644 --- a/internal/fetch/unit.go +++ b/internal/fetch/unit.go @@ -52,8 +52,8 @@ func moduleUnits(modulePath string, minfo internal.ModuleInfo, ModuleInfo: minfo, Path: dirPath, IsRedistributable: isRedist, - Licenses: meta, }, + Licenses: meta, } if r, ok := readmeLookup[dirPath]; ok { dir.Readme = r diff --git a/internal/frontend/main.go b/internal/frontend/main.go index 32a4dc82e..a3ff91cd2 100644 --- a/internal/frontend/main.go +++ b/internal/frontend/main.go @@ -201,7 +201,7 @@ func fetchMainDetails(ctx context.Context, ds internal.DataSource, um *internal. return &MainDetails{ ExpandReadme: expandReadme, Directories: unitDirectories(append(subdirectories, nestedModules...)), - Licenses: transformLicenseMetadata(um.Licenses), + Licenses: transformLicenseMetadata(unit.Licenses), CommitTime: absoluteTime(um.CommitTime), Readme: readme.HTML, ReadmeOutline: readme.Outline, diff --git a/internal/frontend/search_test.go b/internal/frontend/search_test.go index abb16619a..75d690c9d 100644 --- a/internal/frontend/search_test.go +++ b/internal/frontend/search_test.go @@ -254,9 +254,9 @@ func TestFetchSearchPage(t *testing.T) { }, Name: "foo", Path: "github.com/mod/foo", - Licenses: sample.LicenseMetadata(), IsRedistributable: true, }, + Licenses: sample.LicenseMetadata(), Documentation: []*internal.Documentation{{ GOOS: sample.GOOS, GOARCH: sample.GOARCH, @@ -288,9 +288,9 @@ func TestFetchSearchPage(t *testing.T) { }, Name: "bar", Path: "github.com/mod/bar", - Licenses: sample.LicenseMetadata(), IsRedistributable: true, }, + Licenses: sample.LicenseMetadata(), Documentation: []*internal.Documentation{{ GOOS: sample.GOOS, GOARCH: sample.GOARCH, diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go index 387935283..5ef3d28e0 100644 --- a/internal/postgres/unit.go +++ b/internal/postgres/unit.go @@ -16,6 +16,7 @@ import ( "golang.org/x/pkgsite/internal" "golang.org/x/pkgsite/internal/database" "golang.org/x/pkgsite/internal/derrors" + "golang.org/x/pkgsite/internal/licenses" "golang.org/x/pkgsite/internal/middleware/stats" "golang.org/x/pkgsite/internal/stdlib" "golang.org/x/pkgsite/internal/version" @@ -65,9 +66,7 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP "m.has_go_mod", "m.redistributable", "u.name", - "u.redistributable", - "u.license_types", - "u.license_paths"). + "u.redistributable"). From("modules m"). Join("units u on u.module_id = m.id"). Join("paths p ON p.id = u.path_id").Where(squirrel.Eq{"p.path": fullPath}). @@ -92,9 +91,7 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP return nil, err } var ( - licenseTypes []string - licensePaths []string - um = internal.UnitMeta{Path: fullPath} + um = internal.UnitMeta{Path: fullPath} ) err = db.db.QueryRow(ctx, q, args...).Scan( &um.ModulePath, @@ -104,9 +101,7 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP &um.HasGoMod, &um.ModuleInfo.IsRedistributable, &um.Name, - &um.IsRedistributable, - pq.Array(&licenseTypes), - pq.Array(&licensePaths)) + &um.IsRedistributable) if err == sql.ErrNoRows { return nil, derrors.NotFound } @@ -114,15 +109,9 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP return nil, err } - lics, err := zipLicenseMetadata(licenseTypes, licensePaths) - if err != nil { - return nil, err - } - if db.bypassLicenseCheck { um.IsRedistributable = true } - um.Licenses = lics // If we don't have the latest version information, try to get it. // We can be here if there is really no info (in which case we are repeating @@ -432,8 +421,9 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b // Get build contexts and unit ID. var pathID, unitID, moduleID int var bcs []internal.BuildContext + var licenseMetas []*licenses.Metadata err = db.db.RunQuery(ctx, ` - SELECT d.goos, d.goarch, u.id, p.id, u.module_id + SELECT d.goos, d.goarch, u.id, p.id, u.module_id, u.license_types, u.license_paths FROM units u INNER JOIN paths p ON p.id = u.path_id INNER JOIN modules m ON m.id = u.module_id @@ -446,13 +436,22 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b var bc internal.BuildContext // GOOS and GOARCH will be NULL if there are no documentation rows for // the unit, but we still want the unit ID. + var ( + licenseTypes []string + licensePaths []string + ) - if err := rows.Scan(database.NullIsEmpty(&bc.GOOS), database.NullIsEmpty(&bc.GOARCH), &unitID, &pathID, &moduleID); err != nil { + if err := rows.Scan(database.NullIsEmpty(&bc.GOOS), database.NullIsEmpty(&bc.GOARCH), &unitID, &pathID, &moduleID, pq.Array(&licenseTypes), pq.Array(&licensePaths)); err != nil { return err } if bc.GOOS != "" && bc.GOARCH != "" { bcs = append(bcs, bc) } + lics, err := zipLicenseMetadata(licenseTypes, licensePaths) + if err != nil { + return err + } + licenseMetas = lics return nil }, um.Path, um.ModulePath, um.Version) if err != nil { @@ -539,6 +538,7 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b } u.Subdirectories = pkgs u.UnitMeta = *um + u.Licenses = licenseMetas if um.IsPackage() && !um.IsCommand() && doc.Source != nil { u.SymbolHistory, err = GetSymbolHistoryForBuildContext(ctx, db.db, pathID, um.ModulePath, bcMatched) diff --git a/internal/postgres/unit_test.go b/internal/postgres/unit_test.go index 5f83af4c4..db575db8b 100644 --- a/internal/postgres/unit_test.go +++ b/internal/postgres/unit_test.go @@ -833,12 +833,12 @@ func TestGetUnitFieldSet(t *testing.T) { { name: "WithImports", fields: internal.WithImports, - want: unit("a.com/m/dir/p", "a.com/m", "v1.2.3", "", nil, []string{}), + want: unitNoLicenses("a.com/m/dir/p", "a.com/m", "v1.2.3", "", nil, []string{}), }, { name: "WithLicenses", fields: internal.WithLicenses, - want: unit("a.com/m/dir/p", "a.com/m", "v1.2.3", "", nil, []string{}), + want: unitNoLicenses("a.com/m/dir/p", "a.com/m", "v1.2.3", "", nil, []string{}), }, } { t.Run(test.name, func(t *testing.T) { @@ -922,6 +922,12 @@ func TestGetUnitBuildContext(t *testing.T) { } func unit(fullPath, modulePath, version, name string, readme *internal.Readme, suffixes []string) *internal.Unit { + u := unitNoLicenses(fullPath, modulePath, version, name, readme, suffixes) + u.Licenses = sample.LicenseMetadata() + return u +} + +func unitNoLicenses(fullPath, modulePath, version, name string, readme *internal.Readme, suffixes []string) *internal.Unit { u := &internal.Unit{ UnitMeta: internal.UnitMeta{ ModuleInfo: internal.ModuleInfo{ @@ -931,7 +937,6 @@ func unit(fullPath, modulePath, version, name string, readme *internal.Readme, s }, Path: fullPath, IsRedistributable: true, - Licenses: sample.LicenseMetadata(), Name: name, }, LicenseContents: sample.Licenses(), diff --git a/internal/testing/sample/sample.go b/internal/testing/sample/sample.go index 1576ab1c1..c34bbcda9 100644 --- a/internal/testing/sample/sample.go +++ b/internal/testing/sample/sample.go @@ -202,6 +202,7 @@ func Module(modulePath, version string, suffixes ...string) *internal.Module { func UnitForModuleRoot(m *internal.ModuleInfo) *internal.Unit { u := &internal.Unit{ UnitMeta: *UnitMeta(m.ModulePath, m.ModulePath, m.Version, "", m.IsRedistributable), + Licenses: LicenseMetadata(), LicenseContents: Licenses(), } u.Readme = &internal.Readme{ @@ -224,6 +225,7 @@ func UnitForPackage(path, modulePath, version, name string, isRedistributable bo imps := Imports() return &internal.Unit{ UnitMeta: *UnitMeta(path, modulePath, version, name, isRedistributable), + Licenses: LicenseMetadata(), Documentation: []*internal.Documentation{&doc}, BuildContexts: []internal.BuildContext{{GOOS: doc.GOOS, GOARCH: doc.GOARCH}}, LicenseContents: Licenses(), @@ -337,6 +339,7 @@ func replaceLicense(lic *licenses.License, lics []*licenses.License) { func UnitEmpty(path, modulePath, version string) *internal.Unit { return &internal.Unit{ UnitMeta: *UnitMeta(path, modulePath, version, "", true), + Licenses: LicenseMetadata(), } } @@ -345,7 +348,6 @@ func UnitMeta(path, modulePath, version, name string, isRedistributable bool) *i Path: path, Name: name, IsRedistributable: isRedistributable, - Licenses: LicenseMetadata(), ModuleInfo: internal.ModuleInfo{ ModulePath: modulePath, Version: version, diff --git a/internal/unit.go b/internal/unit.go index 9d0d32ee2..dc3b9650c 100644 --- a/internal/unit.go +++ b/internal/unit.go @@ -15,7 +15,6 @@ type UnitMeta struct { Path string Name string IsRedistributable bool - Licenses []*licenses.Metadata // Module level information // Note: IsRedistributable (above) applies to the unit; @@ -53,6 +52,7 @@ type Unit struct { Symbols map[BuildContext][]*Symbol NumImports int NumImportedBy int + Licenses []*licenses.Metadata // SymbolHistory is a map of symbolName to the version when the symbol was // first added to the package. diff --git a/internal/worker/fetch_test.go b/internal/worker/fetch_test.go index 8bbe09b9d..f994dbf50 100644 --- a/internal/worker/fetch_test.go +++ b/internal/worker/fetch_test.go @@ -62,10 +62,10 @@ func TestFetchAndUpdateState(t *testing.T) { IsRedistributable: true, Path: "example.com/multi/bar", Name: "bar", - Licenses: []*licenses.Metadata{ - {Types: []string{"0BSD"}, FilePath: "LICENSE"}, - {Types: []string{"MIT"}, FilePath: "bar/LICENSE"}, - }, + }, + Licenses: []*licenses.Metadata{ + {Types: []string{"0BSD"}, FilePath: "LICENSE"}, + {Types: []string{"MIT"}, FilePath: "bar/LICENSE"}, }, Documentation: []*internal.Documentation{{ Synopsis: "package bar", @@ -118,11 +118,11 @@ func TestFetchAndUpdateState(t *testing.T) { IsRedistributable: true, Path: "example.com/nonredist/bar/baz", Name: "baz", - Licenses: []*licenses.Metadata{ - {Types: []string{"0BSD"}, FilePath: "LICENSE"}, - {Types: []string{"MIT"}, FilePath: "bar/LICENSE"}, - {Types: []string{"MIT"}, FilePath: "bar/baz/COPYING"}, - }, + }, + Licenses: []*licenses.Metadata{ + {Types: []string{"0BSD"}, FilePath: "LICENSE"}, + {Types: []string{"MIT"}, FilePath: "bar/LICENSE"}, + {Types: []string{"MIT"}, FilePath: "bar/baz/COPYING"}, }, Documentation: []*internal.Documentation{{ Synopsis: "package baz", @@ -148,10 +148,10 @@ func TestFetchAndUpdateState(t *testing.T) { IsRedistributable: false, Path: "example.com/nonredist/unk", Name: "unk", - Licenses: []*licenses.Metadata{ - {Types: []string{"0BSD"}, FilePath: "LICENSE"}, - {Types: []string{"UNKNOWN"}, FilePath: "unk/LICENSE.md"}, - }, + }, + Licenses: []*licenses.Metadata{ + {Types: []string{"0BSD"}, FilePath: "LICENSE"}, + {Types: []string{"UNKNOWN"}, FilePath: "unk/LICENSE.md"}, }, NumImports: 2, }, @@ -172,9 +172,9 @@ func TestFetchAndUpdateState(t *testing.T) { IsRedistributable: true, Path: buildConstraintsModulePath + "/cpu", Name: "cpu", - Licenses: []*licenses.Metadata{ - {Types: []string{"0BSD"}, FilePath: "LICENSE"}, - }, + }, + Licenses: []*licenses.Metadata{ + {Types: []string{"0BSD"}, FilePath: "LICENSE"}, }, Documentation: []*internal.Documentation{{ Synopsis: "Package cpu implements processor feature detection used by the Go standard library.", @@ -209,9 +209,6 @@ func TestFetchAndUpdateState(t *testing.T) { if err != nil { t.Fatal(err) } - sort.Slice(got.Licenses, func(i, j int) bool { - return got.Licenses[i].FilePath < got.Licenses[j].FilePath - }) if diff := cmp.Diff(test.want.UnitMeta, *got, cmpopts.EquateEmpty(), cmp.AllowUnexported(source.Info{})); diff != "" { t.Fatalf("testDB.GetUnitMeta(ctx, %q, %q) mismatch (-want +got):\n%s", test.modulePath, test.version, diff) } @@ -220,7 +217,9 @@ func TestFetchAndUpdateState(t *testing.T) { if err != nil { t.Fatal(err) } - + sort.Slice(gotPkg.Licenses, func(i, j int) bool { + return gotPkg.Licenses[i].FilePath < gotPkg.Licenses[j].FilePath + }) if diff := cmp.Diff(test.want, gotPkg, cmpopts.EquateEmpty(), cmp.AllowUnexported(source.Info{}), diff --git a/internal/worker/refetch_test.go b/internal/worker/refetch_test.go index d8e87d18b..69502d90c 100644 --- a/internal/worker/refetch_test.go +++ b/internal/worker/refetch_test.go @@ -96,9 +96,9 @@ func TestReFetch(t *testing.T) { IsRedistributable: true, Path: sample.ModulePath + "/bar", Name: "bar", - Licenses: []*licenses.Metadata{ - {Types: []string{"MIT"}, FilePath: "LICENSE"}, - }, + }, + Licenses: []*licenses.Metadata{ + {Types: []string{"MIT"}, FilePath: "LICENSE"}, }, Readme: &internal.Readme{ Filepath: "bar/README.md",