internal/postgres: change UpsertModuleVersionStates to update

UpsertModuleVersionStates is changed to UpdateModuleVersionStates. There
should never be a situation where UpsertModuleVersionStates is called
and a row does not already exist for that module.

If that happens, an error is now returned.

For golang/go#46985
Fixes golang/go#39628

Change-Id: I357396cee6eb743513ae249609f76f4cd4c19e9b
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/341860
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>

5 files changed, 100 insertions, 57 deletions

diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go
index 00e178d9..3c44d872 100644
--- a/internal/postgres/insert_module_test.go
+++ b/internal/postgres/insert_module_test.go
@@ -458,7 +458,7 @@ func TestPostgres_NewerAlternative(t *testing.T) {
 		okVersion  = "v1.0.0"
 	)
 
-	mvs := &ModuleVersionStateForUpsert{
+	mvs := &ModuleVersionStateForUpdate{
 		ModulePath: "example.com/Mod",
 		Version:    altVersion,
 		AppVersion: "appVersion",
@@ -467,8 +467,17 @@ func TestPostgres_NewerAlternative(t *testing.T) {
 		GoModPath:  "example.com/mod",
 		FetchErr:   derrors.AlternativeModule,
 	}
+	if err := testDB.InsertIndexVersions(ctx, []*internal.IndexVersion{
+		{
+			Path:      mvs.ModulePath,
+			Version:   mvs.Version,
+			Timestamp: mvs.Timestamp,
+		},
+	}); err != nil {
+		t.Fatal(err)
+	}
 	addLatest(ctx, t, testDB, mvs.ModulePath, altVersion, "")
-	if err := testDB.UpsertModuleVersionState(ctx, mvs); err != nil {
+	if err := testDB.UpdateModuleVersionState(ctx, mvs); err != nil {
 		t.Fatal(err)
 	}
 	m := sample.Module(mvs.ModulePath, okVersion, "p")
@@ -592,17 +601,27 @@ func TestIsAlternativeModulePath(t *testing.T) {
 		{modulePathA, "v1.4.0", 200, false},                 // new latest version is OK
 		{modulePathA, "v1.5.0", altModuleStatus, true},      // "I can do this all day." --Captain America
 	} {
-		addLatest(ctx, t, testDB, test.modulePath, test.version, "")
-		if err := testDB.UpsertModuleVersionState(ctx, &ModuleVersionStateForUpsert{
+		mvs := &ModuleVersionStateForUpdate{
 			ModulePath: test.modulePath,
 			Version:    test.version,
 			AppVersion: "appVersion",
 			Timestamp:  time.Now(),
 			Status:     test.status,
 			HasGoMod:   true,
+		}
+		if err := testDB.InsertIndexVersions(ctx, []*internal.IndexVersion{
+			{
+				Path:      mvs.ModulePath,
+				Version:   mvs.Version,
+				Timestamp: mvs.Timestamp,
+			},
 		}); err != nil {
 			t.Fatal(err)
 		}
+		addLatest(ctx, t, testDB, test.modulePath, test.version, "")
+		if err := testDB.UpdateModuleVersionState(ctx, mvs); err != nil {
+			t.Fatal(err)
+		}
 
 		got, err := isAlternativeModulePath(ctx, testDB.db, modulePathA)
 		if err != nil {
@@ -630,7 +649,17 @@ func TestReconcileSearch(t *testing.T) {
 			addLatest(ctx, t, testDB, modulePath, version, modfile)
 		}
 
-		if err := testDB.UpsertModuleVersionState(ctx, &ModuleVersionStateForUpsert{
+		if err := testDB.InsertIndexVersions(ctx,
+			[]*internal.IndexVersion{
+				{
+					Path:      modulePath,
+					Version:   version,
+					Timestamp: time.Now(),
+				},
+			}); err != nil {
+			t.Fatal(err)
+		}
+		if err := testDB.UpdateModuleVersionState(ctx, &ModuleVersionStateForUpdate{
 			ModulePath: modulePath,
 			Version:    version,
 			AppVersion: "app",
diff --git a/internal/postgres/requeue_test.go b/internal/postgres/requeue_test.go
index b2c77264..6198b65e 100644
--- a/internal/postgres/requeue_test.go
+++ b/internal/postgres/requeue_test.go
@@ -159,7 +159,7 @@ func TestGetNextModulesToFetchAndUpdateModuleVersionStatesForReprocessing(t *tes
 	checkNextToRequeue(want, len(mods))
 
 	for _, m := range mods {
-		mvs := &ModuleVersionStateForUpsert{
+		mvs := &ModuleVersionStateForUpdate{
 			ModulePath: m.modulePath,
 			Version:    m.version,
 			AppVersion: "2020-04-29t14",
@@ -168,7 +168,7 @@ func TestGetNextModulesToFetchAndUpdateModuleVersionStatesForReprocessing(t *tes
 			GoModPath:  m.modulePath,
 			FetchErr:   derrors.FromStatus(m.status, "test string"),
 		}
-		if err := upsertModuleVersionState(ctx, testDB.db, &m.numPackages, mvs); err != nil {
+		if err := updateModuleVersionState(ctx, testDB.db, &m.numPackages, mvs); err != nil {
 			t.Fatal(err)
 		}
 	}
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index 77261b70..96b621c9 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -1185,14 +1185,23 @@ func TestUpdateSearchDocumentsImportedByCount(t *testing.T) {
 		// We add that information to module_version_states.
 		alternativeModulePath := strings.ToLower(canonicalModule.ModulePath)
 		alternativeStatus := derrors.ToStatus(derrors.AlternativeModule)
-		mvs := &ModuleVersionStateForUpsert{
+		mvs := &ModuleVersionStateForUpdate{
 			ModulePath: alternativeModulePath,
 			Version:    "v1.2.0",
 			Timestamp:  time.Now(),
 			Status:     alternativeStatus,
 			GoModPath:  canonicalModule.ModulePath,
 		}
-		if err := testDB.UpsertModuleVersionState(ctx, mvs); err != nil {
+		if err := testDB.InsertIndexVersions(ctx, []*internal.IndexVersion{
+			{
+				Path:      mvs.ModulePath,
+				Version:   mvs.Version,
+				Timestamp: mvs.Timestamp,
+			},
+		}); err != nil {
+			t.Fatal(err)
+		}
+		if err := testDB.UpdateModuleVersionState(ctx, mvs); err != nil {
 			t.Fatal(err)
 		}
 
diff --git a/internal/postgres/versionstate.go b/internal/postgres/versionstate.go
index 85f8d54a..2ccaa020 100644
--- a/internal/postgres/versionstate.go
+++ b/internal/postgres/versionstate.go
@@ -105,7 +105,7 @@ func insertIndexVersions(ctx context.Context, ddb *database.DB, versions []*inte
 	})
 }
 
-type ModuleVersionStateForUpsert struct {
+type ModuleVersionStateForUpdate struct {
 	ModulePath           string
 	Version              string
 	AppVersion           string
@@ -117,9 +117,9 @@ type ModuleVersionStateForUpsert struct {
 	PackageVersionStates []*internal.PackageVersionState
 }
 
-// UpsertModuleVersionState inserts or updates the module_version_state table with
+// UpdateModuleVersionState inserts or updates the module_version_state table with
 // the results of a fetch operation for a given module version.
-func (db *DB) UpsertModuleVersionState(ctx context.Context, mvs *ModuleVersionStateForUpsert) (err error) {
+func (db *DB) UpdateModuleVersionState(ctx context.Context, mvs *ModuleVersionStateForUpdate) (err error) {
 	defer derrors.WrapStack(&err, "UpsertModuleVersionState(ctx, %s@%s)", mvs.ModulePath, mvs.Version)
 	ctx, span := trace.StartSpan(ctx, "UpsertModuleVersionState")
 	defer span.End()
@@ -131,9 +131,8 @@ func (db *DB) UpsertModuleVersionState(ctx context.Context, mvs *ModuleVersionSt
 		n := len(mvs.PackageVersionStates)
 		numPackages = &n
 	}
-
 	return db.db.Transact(ctx, sql.LevelDefault, func(tx *database.DB) error {
-		if err := upsertModuleVersionState(ctx, tx, numPackages, mvs); err != nil {
+		if err := updateModuleVersionState(ctx, tx, numPackages, mvs); err != nil {
 			return err
 		}
 		// Sync modules.status if the module exists in the modules table.
@@ -147,7 +146,7 @@ func (db *DB) UpsertModuleVersionState(ctx context.Context, mvs *ModuleVersionSt
 	})
 }
 
-func upsertModuleVersionState(ctx context.Context, db *database.DB, numPackages *int, mvs *ModuleVersionStateForUpsert) (err error) {
+func updateModuleVersionState(ctx context.Context, db *database.DB, numPackages *int, mvs *ModuleVersionStateForUpdate) (err error) {
 	defer derrors.WrapStack(&err, "upsertModuleVersionState(%q, %q, ...)", mvs.ModulePath, mvs.Version)
 	ctx, span := trace.StartSpan(ctx, "upsertModuleVersionState")
 	defer span.End()
@@ -158,42 +157,35 @@ func upsertModuleVersionState(ctx context.Context, db *database.DB, numPackages
 	}
 
 	affected, err := db.Exec(ctx, `
-			INSERT INTO module_version_states AS mvs (
-				module_path,
-				version,
-				sort_version,
-				app_version,
-				index_timestamp,
-				status,
-				has_go_mod,
-				go_mod_path,
-				error,
-				num_packages,
-				incompatible)
-			VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
-			ON CONFLICT (module_path, version)
-			DO UPDATE
-			SET
-				app_version=excluded.app_version,
-				status=excluded.status,
-				has_go_mod=excluded.has_go_mod,
-				go_mod_path=excluded.go_mod_path,
-				error=excluded.error,
-				num_packages=excluded.num_packages,
-				try_count=mvs.try_count+1,
-				last_processed_at=CURRENT_TIMESTAMP,
-			    -- back off exponentially until 1 hour, then at constant 1-hour intervals
-				next_processed_after=CASE
-					WHEN mvs.last_processed_at IS NULL THEN
-						CURRENT_TIMESTAMP + INTERVAL '1 minute'
-					WHEN 2*(mvs.next_processed_after - mvs.last_processed_at) < INTERVAL '1 hour' THEN
-						CURRENT_TIMESTAMP + 2*(mvs.next_processed_after - mvs.last_processed_at)
-					ELSE
-						CURRENT_TIMESTAMP + INTERVAL '1 hour'
-					END;`,
-		mvs.ModulePath, mvs.Version, version.ForSorting(mvs.Version),
-		mvs.AppVersion, mvs.Timestamp, mvs.Status, mvs.HasGoMod, mvs.GoModPath, sqlErrorMsg, numPackages,
-		version.IsIncompatible(mvs.Version))
+		UPDATE module_version_states
+		SET app_version=$1,
+			status=$2,
+			has_go_mod=$3,
+			go_mod_path=$4,
+			error=$5,
+			num_packages=$6,
+			try_count=try_count+1,
+			last_processed_at=CURRENT_TIMESTAMP,
+			-- back off exponentially until 1 hour, then at constant 1-hour intervals
+			next_processed_after=CASE
+				WHEN last_processed_at IS NULL THEN
+					CURRENT_TIMESTAMP + INTERVAL '1 minute'
+				WHEN 2*(next_processed_after - last_processed_at) < INTERVAL '1 hour' THEN
+					CURRENT_TIMESTAMP + 2*(next_processed_after - last_processed_at)
+				ELSE
+					CURRENT_TIMESTAMP + INTERVAL '1 hour'
+				END
+		WHERE
+			module_path=$7
+			AND version=$8`,
+		mvs.AppVersion,
+		mvs.Status,
+		mvs.HasGoMod,
+		mvs.GoModPath,
+		sqlErrorMsg,
+		numPackages,
+		mvs.ModulePath,
+		mvs.Version)
 	if err != nil {
 		return err
 	}
diff --git a/internal/postgres/versionstate_test.go b/internal/postgres/versionstate_test.go
index cfd0f5f1..953097ca 100644
--- a/internal/postgres/versionstate_test.go
+++ b/internal/postgres/versionstate_test.go
@@ -137,7 +137,7 @@ func TestModuleVersionState(t *testing.T) {
 			Status:      500,
 		}
 	)
-	mvs := &ModuleVersionStateForUpsert{
+	mvs := &ModuleVersionStateForUpdate{
 		ModulePath:           fooVersion.Path,
 		Version:              fooVersion.Version,
 		Timestamp:            fooVersion.Timestamp,
@@ -146,7 +146,7 @@ func TestModuleVersionState(t *testing.T) {
 		FetchErr:             fetchErr,
 		PackageVersionStates: []*internal.PackageVersionState{pkgVersionState},
 	}
-	must(t, testDB.UpsertModuleVersionState(ctx, mvs))
+	must(t, testDB.UpdateModuleVersionState(ctx, mvs))
 	errString := fetchErr.Error()
 	numPackages := 1
 	wantFooState := &internal.ModuleVersionState{
@@ -274,7 +274,7 @@ func TestUpsertModuleVersionStates(t *testing.T) {
 				MustInsertModule(ctx, t, testDB, m)
 			}
 
-			mvsu := &ModuleVersionStateForUpsert{
+			mvsu := &ModuleVersionStateForUpdate{
 				ModulePath: m.ModulePath,
 				Version:    m.Version,
 				AppVersion: appVersion,
@@ -282,7 +282,16 @@ func TestUpsertModuleVersionStates(t *testing.T) {
 				Status:     test.status,
 				HasGoMod:   true,
 			}
-			err := testDB.UpsertModuleVersionState(ctx, mvsu)
+			if err := testDB.InsertIndexVersions(ctx, []*internal.IndexVersion{
+				{
+					Path:      mvsu.ModulePath,
+					Version:   mvsu.Version,
+					Timestamp: mvsu.Timestamp,
+				},
+			}); err != nil {
+				t.Fatal(err)
+			}
+			err := testDB.UpdateModuleVersionState(ctx, mvsu)
 			if test.wantUpsertMVSError != (err != nil) {
 				t.Fatalf("db.UpsertModuleVersionState(): %v, want error: %t", err, test.wantUpsertMVSError)
 			}
@@ -328,13 +337,17 @@ func TestUpdateModuleVersionStatus(t *testing.T) {
 	testDB, release := acquire(t)
 	defer release()
 
-	mvs := &ModuleVersionStateForUpsert{
+	mvs := &ModuleVersionStateForUpdate{
 		ModulePath: "m.com",
 		Version:    "v1.2.3",
 		Timestamp:  time.Now(),
 		Status:     200,
 	}
-	must(t, testDB.UpsertModuleVersionState(ctx, mvs))
+	must(t, testDB.InsertIndexVersions(
+		ctx,
+		[]*internal.IndexVersion{{Path: mvs.ModulePath, Version: mvs.Version, Timestamp: time.Now()}},
+	))
+	must(t, testDB.UpdateModuleVersionState(ctx, mvs))
 	wantStatus := 999
 	wantError := "Error"
 	must(t, testDB.UpdateModuleVersionStatus(ctx, mvs.ModulePath, mvs.Version, wantStatus, wantError))