internal/postgres: apply bypassLicenseCheck to IsRedistributable column

This enables symbol search for non-LICENSE'd repos when -bypass_license_check is supplied.

I think the way this works is that when these get insert into units, we don't consider bypassLicenseCheck, and the IsRedistributable column gets set to false. That all seems reasonable by itself.

But later, in upsertSymbolSearchDocuments, we only upsert symbol docs for units that are redistributable. That check happens in SQL, so it's hard to factor in the bypassLicenseCheck flag.

It's not really clear to me whether we should "pollute" the database with this column set to true under the flag. If you run the program again with the flag turned off, your database keeps the values set under the old run. Anyways, I think this CL is probably the pragmatic take: it's a small code change that achieves the desired result without added complexity, and it's unlikely that anyone will run into the undesired outcome I describe above (who is running this program sometimes with the flag on, sometimes with it off?).

Change-Id: Ie15bcf0605b945e4af6574abf689827ea946ddae
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/685457
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
kokoro-CI: kokoro <noreply+kokoro@google.com>
Commit-Queue: Jonathan Amsterdam <jba@google.com>

1 files changed, 40 insertions, 36 deletions

diff --git a/internal/postgres/unit_test.go b/internal/postgres/unit_test.go
index 835344a9..60ebfb83 100644
--- a/internal/postgres/unit_test.go
+++ b/internal/postgres/unit_test.go
@@ -804,46 +804,50 @@ func subdirectories(modulePath string, suffixes []string) []*internal.PackageMet
 }
 
 func TestGetUnitBypass(t *testing.T) {
-	t.Parallel()
-	testDB, release := acquire(t)
-	defer release()
-	ctx := context.Background()
-	bypassDB := NewBypassingLicenseCheck(testDB.db)
-
-	m := nonRedistributableModule()
-	MustInsertModule(ctx, t, bypassDB, m)
-
 	for _, test := range []struct {
-		db        *DB
+		bypass    bool
 		wantEmpty bool
 	}{
-		{testDB, true},
-		{bypassDB, false},
+		{false, true},
+		{true, false},
 	} {
-		pathInfo := newUnitMeta(m.ModulePath, m.ModulePath, m.Version)
-		d, err := test.db.GetUnit(ctx, pathInfo, internal.AllFields, internal.BuildContext{})
-		if err != nil {
-			t.Fatal(err)
-		}
-		if got := (d.Readme == nil); got != test.wantEmpty {
-			t.Errorf("readme empty: got %t, want %t", got, test.wantEmpty)
-		}
-		if got := (d.Documentation == nil); got != test.wantEmpty {
-			t.Errorf("synopsis empty: got %t, want %t", got, test.wantEmpty)
-		}
-		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")
-		}
-		if got := (pkgs[0].Synopsis == ""); got != test.wantEmpty {
-			t.Errorf("synopsis empty: got %t, want %t", got, test.wantEmpty)
-		}
+		t.Run(fmt.Sprintf("bypass %t", test.bypass), func(t *testing.T) {
+			t.Parallel()
+			testDB, release := acquire(t)
+			defer release()
+			ctx := context.Background()
+
+			if test.bypass {
+				testDB = NewBypassingLicenseCheck(testDB.db)
+			}
+
+			m := nonRedistributableModule()
+			MustInsertModule(ctx, t, testDB, m)
+			pathInfo := newUnitMeta(m.ModulePath, m.ModulePath, m.Version)
+			d, err := testDB.GetUnit(ctx, pathInfo, internal.AllFields, internal.BuildContext{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := (d.Readme == nil); got != test.wantEmpty {
+				t.Errorf("readme empty: got %t, want %t", got, test.wantEmpty)
+			}
+			if got := (d.Documentation == nil); got != test.wantEmpty {
+				t.Errorf("synopsis empty: got %t, want %t", got, test.wantEmpty)
+			}
+			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")
+			}
+			if got := (pkgs[0].Synopsis == ""); got != test.wantEmpty {
+				t.Errorf("synopsis empty: got %t, want %t", got, test.wantEmpty)
+			}
+		})
 	}
 }