internal: move UnitMeta.IsRedistributable to Unit

Change-Id: Ia0a18df32d752f20b0400b47860638c7d0910eb7
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/556435
Reviewed-by: Jonathan Amsterdam <jba@google.com>
kokoro-CI: kokoro <noreply+kokoro@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>

5 files changed, 42 insertions, 215 deletions

diff --git a/internal/postgres/licenses_test.go b/internal/postgres/licenses_test.go
index c0b02f9b..300d5c52 100644
--- a/internal/postgres/licenses_test.go
+++ b/internal/postgres/licenses_test.go
@@ -212,20 +212,3 @@ func nonRedistributableModule() *internal.Module {
 	m.Units[0].IsRedistributable = false
 	return m
 }
-
-// makeModuleNonRedistributable mutates the passed-in module by marking it
-// non-redistributable along with each of its packages and units. It allows
-// us to re-use existing test data without defining a non-redistributable
-// counterpart to each.
-func makeModuleNonRedistributable(m *internal.Module) {
-	sample.AddLicense(m, sample.NonRedistributableLicense)
-	m.IsRedistributable = false
-
-	for _, p := range m.Packages() {
-		p.IsRedistributable = false
-	}
-
-	for i := range m.Units {
-		m.Units[i].IsRedistributable = false
-	}
-}
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index 40455a0d..8a3c0239 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -310,9 +310,10 @@ func importGraph(popularPath, importerModule string, importerCount int) []*inter
 			name := fmt.Sprintf("importer%d", i)
 			fullPath := importerModule + "/" + name
 			u := &internal.Unit{
-				UnitMeta:      *sample.UnitMeta(fullPath, importerModule, m.Version, name, true),
-				Documentation: []*internal.Documentation{sample.Doc},
-				Imports:       []string{popularPath},
+				UnitMeta:          *sample.UnitMeta(fullPath, importerModule, m.Version, name, true),
+				IsRedistributable: true,
+				Documentation:     []*internal.Documentation{sample.Doc},
+				Imports:           []string{popularPath},
 			}
 			sample.AddUnit(m, u)
 		}
@@ -562,10 +563,10 @@ func TestInsertSearchDocumentAndSearch(t *testing.T) {
 		modGoCDK = "gocloud.dev"
 		pkgGoCDK = &internal.Unit{
 			UnitMeta: internal.UnitMeta{
-				Name:              "cloud",
-				Path:              "gocloud.dev/cloud",
-				IsRedistributable: true, // required because some test cases depend on the README contents
+				Name: "cloud",
+				Path: "gocloud.dev/cloud",
 			},
+			IsRedistributable: true, // required because some test cases depend on the README contents
 			Documentation: []*internal.Documentation{{
 				GOOS:     sample.GOOS,
 				GOARCH:   sample.GOARCH,
@@ -577,10 +578,10 @@ func TestInsertSearchDocumentAndSearch(t *testing.T) {
 		modKube = "k8s.io"
 		pkgKube = &internal.Unit{
 			UnitMeta: internal.UnitMeta{
-				Name:              "client-go",
-				Path:              "k8s.io/client-go",
-				IsRedistributable: true, // required because some test cases depend on the README contents
+				Name: "client-go",
+				Path: "k8s.io/client-go",
 			},
+			IsRedistributable: true, // required because some test cases depend on the README contents
 			Documentation: []*internal.Documentation{{
 				GOOS:     sample.GOOS,
 				GOARCH:   sample.GOARCH,
diff --git a/internal/postgres/symbol_test.go b/internal/postgres/symbol_test.go
index 07a289a3..b70cb038 100644
--- a/internal/postgres/symbol_test.go
+++ b/internal/postgres/symbol_test.go
@@ -391,7 +391,7 @@ func moduleWithSymbols(t *testing.T, version string, symbols []*internal.Symbol)
 func compareUnitSymbols(ctx context.Context, t *testing.T, testDB *DB,
 	path, modulePath, version string, wantBuildToSymbols map[internal.BuildContext][]*internal.Symbol) {
 	t.Helper()
-	unitID, err := testDB.getUnitID(ctx, path, modulePath, version)
+	unitID, _, err := testDB.getUnitID(ctx, path, modulePath, version)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/internal/postgres/unit.go b/internal/postgres/unit.go
index 5ef3d28e..33c58401 100644
--- a/internal/postgres/unit.go
+++ b/internal/postgres/unit.go
@@ -65,8 +65,7 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP
 		"m.source_info",
 		"m.has_go_mod",
 		"m.redistributable",
-		"u.name",
-		"u.redistributable").
+		"u.name").
 		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}).
@@ -100,8 +99,7 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP
 		jsonbScanner{&um.SourceInfo},
 		&um.HasGoMod,
 		&um.ModuleInfo.IsRedistributable,
-		&um.Name,
-		&um.IsRedistributable)
+		&um.Name)
 	if err == sql.ErrNoRows {
 		return nil, derrors.NotFound
 	}
@@ -109,10 +107,6 @@ func (db *DB) getUnitMetaWithKnownVersion(ctx context.Context, fullPath, moduleP
 		return nil, err
 	}
 
-	if db.bypassLicenseCheck {
-		um.IsRedistributable = true
-	}
-
 	// 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
 	// some work, but it's fast), or if we are ignoring the info (for instance,
@@ -249,10 +243,11 @@ func (db *DB) GetUnit(ctx context.Context, um *internal.UnitMeta, fields interna
 	}
 
 	defer stats.Elapsed(ctx, "GetUnit")()
-	unitID, err := db.getUnitID(ctx, um.Path, um.ModulePath, um.Version)
+	unitID, isRedistributable, err := db.getUnitID(ctx, um.Path, um.ModulePath, um.Version)
 	if err != nil {
 		return nil, err
 	}
+	u.IsRedistributable = isRedistributable
 
 	if fields&internal.WithImports != 0 {
 		imports, err := db.getImports(ctx, unitID)
@@ -279,12 +274,13 @@ func (db *DB) GetUnit(ctx context.Context, um *internal.UnitMeta, fields interna
 	return u, nil
 }
 
-func (db *DB) getUnitID(ctx context.Context, fullPath, modulePath, resolvedVersion string) (_ int, err error) {
+func (db *DB) getUnitID(ctx context.Context, fullPath, modulePath, resolvedVersion string) (_ int, _ bool, err error) {
 	defer derrors.WrapStack(&err, "getUnitID(ctx, %q, %q, %q)", fullPath, modulePath, resolvedVersion)
 	defer stats.Elapsed(ctx, "getUnitID")()
 	var unitID int
+	var isRedistributable bool
 	query := `
-		SELECT u.id
+		SELECT u.id, u.redistributable
 		FROM units u
 		INNER JOIN paths p ON (p.id = u.path_id)
 		INNER JOIN modules m ON (u.module_id = m.id)
@@ -292,14 +288,14 @@ func (db *DB) getUnitID(ctx context.Context, fullPath, modulePath, resolvedVersi
 			p.path = $1
 			AND m.module_path = $2
 			AND m.version = $3;`
-	err = db.db.QueryRow(ctx, query, fullPath, modulePath, resolvedVersion).Scan(&unitID)
+	err = db.db.QueryRow(ctx, query, fullPath, modulePath, resolvedVersion).Scan(&unitID, &isRedistributable)
 	switch err {
 	case sql.ErrNoRows:
-		return 0, derrors.NotFound
+		return 0, false, derrors.NotFound
 	case nil:
-		return unitID, nil
+		return unitID, isRedistributable, nil
 	default:
-		return 0, err
+		return 0, false, err
 	}
 }
 
@@ -422,8 +418,9 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b
 	var pathID, unitID, moduleID int
 	var bcs []internal.BuildContext
 	var licenseMetas []*licenses.Metadata
+	var isRedistributable bool
 	err = db.db.RunQuery(ctx, `
-		SELECT d.goos, d.goarch, u.id, p.id, u.module_id, u.license_types, u.license_paths
+		SELECT d.goos, d.goarch, u.id, p.id, u.module_id, u.license_types, u.license_paths, u.redistributable
 		FROM units u
 		INNER JOIN paths p ON p.id = u.path_id
 		INNER JOIN modules m ON m.id = u.module_id
@@ -441,9 +438,14 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b
 			licensePaths []string
 		)
 
-		if err := rows.Scan(database.NullIsEmpty(&bc.GOOS), database.NullIsEmpty(&bc.GOARCH), &unitID, &pathID, &moduleID, pq.Array(&licenseTypes), pq.Array(&licensePaths)); err != nil {
+		if err := rows.Scan(database.NullIsEmpty(&bc.GOOS), database.NullIsEmpty(&bc.GOARCH), &unitID, &pathID, &moduleID, pq.Array(&licenseTypes), pq.Array(&licensePaths), &isRedistributable); err != nil {
 			return err
 		}
+
+		if db.bypassLicenseCheck {
+			isRedistributable = true
+		}
+
 		if bc.GOOS != "" && bc.GOARCH != "" {
 			bcs = append(bcs, bc)
 		}
@@ -539,6 +541,7 @@ func (db *DB) getUnitWithAllFields(ctx context.Context, um *internal.UnitMeta, b
 	u.Subdirectories = pkgs
 	u.UnitMeta = *um
 	u.Licenses = licenseMetas
+	u.IsRedistributable = isRedistributable
 
 	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 db575db8..835344a9 100644
--- a/internal/postgres/unit_test.go
+++ b/internal/postgres/unit_test.go
@@ -93,8 +93,7 @@ func testGetUnitMeta(t *testing.T, ctx context.Context) {
 				Version:           version,
 				IsRedistributable: true,
 			},
-			Name:              name,
-			IsRedistributable: true,
+			Name: name,
 		}
 	}
 
@@ -191,8 +190,9 @@ func testGetUnitMeta(t *testing.T, ctx context.Context) {
 				test.want.ModulePath,
 				test.want.Version,
 				test.want.Name,
-				test.want.IsRedistributable,
+				true,
 			)
+			want.IsRedistributable = true
 			want.CommitTime = sample.CommitTime
 			want.Retracted = test.want.Retracted
 			want.RetractionRationale = test.want.RetractionRationale
@@ -202,169 +202,6 @@ func testGetUnitMeta(t *testing.T, ctx context.Context) {
 	}
 }
 
-func TestGetUnitMetaBypass(t *testing.T) {
-	t.Parallel()
-	testDB, release := acquire(t)
-	defer release()
-	ctx := context.Background()
-
-	bypassDB := NewBypassingLicenseCheck(testDB.db)
-
-	for _, testModule := range []struct {
-		module, version, packageSuffix string
-		isMaster                       bool
-	}{
-		{"m.com", "v2.0.0+incompatible", "a", false},
-		{"m.com/b", "v2.0.0+incompatible", "a", true},
-		{"m.com", "v1.0.0", "a", false},
-		{"m.com", "v1.0.1", "dir/a", false},
-		{"m.com", "v1.1.0", "a/b", false},
-		{"m.com", "v1.2.0-pre", "a", true},
-		{"m.com/a", "v1.1.0", "b", false},
-	} {
-		m := sample.Module(testModule.module, testModule.version, testModule.packageSuffix)
-		makeModuleNonRedistributable(m)
-
-		MustInsertModule(ctx, t, bypassDB, m)
-		requested := m.Version
-		if testModule.isMaster {
-			requested = "master"
-		}
-		if err := bypassDB.UpsertVersionMap(ctx, &internal.VersionMap{
-			ModulePath:       m.ModulePath,
-			RequestedVersion: requested,
-			ResolvedVersion:  m.Version,
-		}); err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	wantUnitMeta := func(modPath, version, name string, isRedist bool) *internal.UnitMeta {
-		return &internal.UnitMeta{
-			ModuleInfo: internal.ModuleInfo{
-				ModulePath:        modPath,
-				Version:           version,
-				IsRedistributable: false,
-			},
-			Name:              name,
-			IsRedistributable: isRedist,
-		}
-	}
-
-	for _, bypassLicenseCheck := range []bool{false, true} {
-		for _, test := range []struct {
-			name                  string
-			path, module, version string
-			want                  *internal.UnitMeta
-		}{
-			{
-				name:    "known module and version",
-				path:    "m.com/a",
-				module:  "m.com",
-				version: "v1.2.0-pre",
-				want:    wantUnitMeta("m.com", "v1.2.0-pre", "a", bypassLicenseCheck),
-			},
-			{
-				name:    "unknown module, known version",
-				path:    "m.com/a/b",
-				version: "v1.1.0",
-				// The path is in two modules at v1.1.0. Prefer the longer one.
-				want: wantUnitMeta("m.com/a", "v1.1.0", "b", bypassLicenseCheck),
-			},
-			{
-				name:   "known module, unknown version",
-				path:   "m.com/a",
-				module: "m.com",
-				// Choose the latest release version.
-				want: wantUnitMeta("m.com", "v1.1.0", "", bypassLicenseCheck),
-			},
-			{
-				name: "unknown module and version",
-				path: "m.com/a/b",
-				// Select the latest release version, longest module.
-				want: wantUnitMeta("m.com/a", "v1.1.0", "b", bypassLicenseCheck),
-			},
-			{
-				name: "module",
-				path: "m.com",
-				// Select the latest version of the module.
-				want: wantUnitMeta("m.com", "v1.1.0", "", bypassLicenseCheck),
-			},
-			{
-				name:    "longest module",
-				path:    "m.com/a",
-				version: "v1.1.0",
-				// Prefer module m/a over module m, directory a.
-				want: wantUnitMeta("m.com/a", "v1.1.0", "", bypassLicenseCheck),
-			},
-			{
-				name: "directory",
-				path: "m.com/dir",
-				want: wantUnitMeta("m.com", "v1.0.1", "", bypassLicenseCheck),
-			},
-			{
-				name:    "module at master version",
-				path:    "m.com",
-				version: "master",
-				want:    wantUnitMeta("m.com", "v1.2.0-pre", "", bypassLicenseCheck),
-			},
-			{
-				name:    "package at master version",
-				path:    "m.com/a",
-				version: "master",
-				want:    wantUnitMeta("m.com", "v1.2.0-pre", "a", bypassLicenseCheck),
-			},
-			{
-				name:    "incompatible module",
-				path:    "m.com/b",
-				version: "master",
-				want:    wantUnitMeta("m.com/b", "v2.0.0+incompatible", "", bypassLicenseCheck),
-			},
-		} {
-			name := fmt.Sprintf("bypass %v %s", bypassLicenseCheck, test.name)
-			t.Run(name, func(t *testing.T) {
-				if test.module == "" {
-					test.module = internal.UnknownModulePath
-				}
-				if test.version == "" {
-					test.version = version.Latest
-				}
-				test.want = sample.UnitMeta(
-					test.path,
-					test.want.ModulePath,
-					test.want.Version,
-					test.want.Name,
-					test.want.IsRedistributable,
-				)
-				test.want.ModuleInfo.IsRedistributable = false
-				test.want.CommitTime = sample.CommitTime
-
-				var db *DB
-
-				if bypassLicenseCheck {
-					db = bypassDB
-				} else {
-					db = testDB
-				}
-
-				got, err := db.GetUnitMeta(ctx, test.path, test.module, test.version)
-				if err != nil {
-					t.Fatal(err)
-				}
-				opts := []cmp.Option{
-					cmpopts.EquateEmpty(),
-					cmpopts.IgnoreFields(licenses.Metadata{}, "Coverage"),
-					cmpopts.IgnoreFields(internal.UnitMeta{}, "HasGoMod"),
-					cmp.AllowUnexported(source.Info{}, safehtml.HTML{}),
-				}
-				if diff := cmp.Diff(test.want, got, opts...); diff != "" {
-					t.Errorf("mismatch (-want +got):\n%s", diff)
-				}
-			})
-		}
-	}
-}
-
 func TestGetLatestUnitVersion(t *testing.T) {
 	ctx := context.Background()
 
@@ -935,12 +772,12 @@ func unitNoLicenses(fullPath, modulePath, version, name string, readme *internal
 				Version:           version,
 				IsRedistributable: true,
 			},
-			Path:              fullPath,
-			IsRedistributable: true,
-			Name:              name,
+			Path: fullPath,
+			Name: name,
 		},
-		LicenseContents: sample.Licenses(),
-		Readme:          readme,
+		IsRedistributable: true,
+		LicenseContents:   sample.Licenses(),
+		Readme:            readme,
 	}
 
 	u.Subdirectories = subdirectories(modulePath, suffixes)
@@ -997,6 +834,9 @@ func TestGetUnitBypass(t *testing.T) {
 		if got := (d.Documentation == nil); got != test.wantEmpty {
 			t.Errorf("doc empty: got %t, want %t", got, test.wantEmpty)
 		}
+		if got := d.IsRedistributable; got != !test.wantEmpty { // wantEmpty iff !IsRedistributable
+			t.Errorf("IsRedistributable is %v: want %v", got, !test.wantEmpty)
+		}
 		pkgs := d.Subdirectories
 		if len(pkgs) != 1 {
 			t.Fatal("len(pkgs) != 1")