test: make the concurrent analysis patch test deterministic

main.go

@@ -38,7 +38,7 @@ func main() {
 		Use:          "open-catalog-api",
 		SilenceUsage: true,
 		RunE: func(_ *cobra.Command, _ []string) error {
-			app, debouncer, stopEventPurge := Setup()
+			app, debouncer, stopEventPurge, _ := Setup()
 
 			sigCh := make(chan os.Signal, 1)
 			signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
@@ -74,7 +74,9 @@ func main() {
 	}
 }
 
-func Setup() (*fiber.App, *webhooks.Debouncer, func()) {
+// Setup builds the app. It also returns the gorm handle: the tests hook
+// callbacks on the same connection the handlers use.
+func Setup() (*fiber.App, *webhooks.Debouncer, func(), *gorm.DB) {
 	if err := env.Parse(&common.EnvironmentConfig); err != nil {
 		panic(err)
 	}
@@ -167,7 +169,7 @@ func Setup() (*fiber.App, *webhooks.Debouncer, func()) {
 
 	setupHandlers(app, gormDB)
 
-	return app, debouncer, stopEventPurge
+	return app, debouncer, stopEventPurge, gormDB
 }
 
 // requiresAuth reports whether a request must carry a valid token. Reads

main_test.go

@@ -25,13 +25,15 @@ import (
 	"github.com/publiccodeyml/open-catalog-api/internal/database"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 )
 
 const UUID_REGEXP = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"
 
 var (
 	app       *fiber.App
 	db        *sql.DB
+	gormDB    *gorm.DB
 	dbDriver  string
 	goodToken = "Bearer v2.local.TwwHUQEi8hr2Eo881_Bs5vK9dHOR5BgEU24QRf-U7VmUwI1yOEA6mFT0EsXioMkFT_T-jjrtIJ_Nv8f6hR6ifJXUOuzWEkm9Ijq1mqSjQatD3aDqKMyjjBA"
 	badToken  = "Bearer v2.local.UngfrCDNwGUw4pff2oBNoyxYvOErcbVVqLndl6nzONafUCzktaOeMSmoI7B0h62zoxXXLqTm_Phl"
@@ -95,7 +97,7 @@ func init() {
 	}
 
 	// Setup the app as it is done in the main function
-	app, _, _ = Setup()
+	app, _, _, gormDB = Setup()
 }
 
 func TestMain(m *testing.M) {
@@ -369,11 +371,11 @@ func TestRateLimiterBuckets(t *testing.T) {
 	// An empty variable leaves the already parsed value alone, so the app
 	// trusting nobody has to be built before the one with a trusted proxy.
 	t.Setenv("TRUSTED_PROXIES", "")
-	directApp, _, _ := Setup()
+	directApp, _, _, _ := Setup()
 
 	// 0.0.0.0 is the peer address of a request made through app.Test().
 	t.Setenv("TRUSTED_PROXIES", "0.0.0.0/32")
-	proxiedApp, _, _ := Setup()
+	proxiedApp, _, _, _ := Setup()
 
 	t.Run("a client varying X-Forwarded-For stays in one bucket", func(t *testing.T) {
 		codes := make([]int, 0, maxRequests+1)

software_test.go

@@ -4,11 +4,14 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/publiccodeyml/open-catalog-api/internal/models"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 )
 
 func TestSoftwareEndpoints(t *testing.T) {
@@ -1857,55 +1860,24 @@ func TestSoftwareAnalysisDBChecks(t *testing.T) {
 
 		const softwareID = "59803fb7-8eec-4fe5-a354-8926009c364a"
 
-		bodies := []string{
-			`{"scanner-one":{"v":1,"score":90}}`,
-			`{"scanner-two":{"v":2,"grade":"A"}}`,
-		}
-		type patchResult struct {
-			status int
-			err    error
-		}
-
-		start := make(chan struct{})
-		results := make(chan patchResult, len(bodies))
-
-		for _, body := range bodies {
-			go func() {
-				req, err := newTestRequest(
-					"PATCH",
-					"/v1/software/"+softwareID+"/analysis",
-					strings.NewReader(body),
-				)
-				if err != nil {
-					results <- patchResult{err: err}
-
-					return
-				}
+		waitForPark, release := holdFirstAnalysisWrite(t, softwareID)
 
-				req.Header = map[string][]string{
-					"Authorization": {goodToken},
-					"Content-Type":  {"application/merge-patch+json"},
-				}
+		parked := make(chan patchResult, 1)
+		go func() {
+			parked <- sendAnalysisPatch(softwareID, `{"scanner-one":{"v":1,"score":90}}`)
+		}()
 
-				<-start
+		waitForPark()
 
-				res, err := app.Test(req, -1)
-				status := 0
-				if res != nil {
-					status = res.StatusCode
-					_ = res.Body.Close()
-				}
+		overtaking := sendAnalysisPatch(softwareID, `{"scanner-two":{"v":2,"grade":"A"}}`)
+		release()
 
-				results <- patchResult{status: status, err: err}
-			}()
-		}
+		first := <-parked
 
-		close(start)
-		for range bodies {
-			result := <-results
-			require.NoError(t, result.err)
-			assert.Equal(t, 200, result.status)
-		}
+		require.NoError(t, overtaking.err)
+		require.NoError(t, first.err)
+		assert.Equal(t, 200, overtaking.status)
+		assert.Equal(t, 200, first.status)
 
 		raw := dbValue(t, "software", "analysis", "id", softwareID)
 
@@ -2027,6 +1999,85 @@ func storedNamespace(t *testing.T, softwareID, namespace string) map[string]any
 	return analysis[namespace]
 }
 
+type patchResult struct {
+	status int
+	err    error
+}
+
+// sendAnalysisPatch runs one analysis PATCH and returns the outcome
+// instead of failing the test, so it can run on its own goroutine.
+func sendAnalysisPatch(softwareID, body string) patchResult {
+	req, err := newTestRequest(
+		"PATCH",
+		"/v1/software/"+softwareID+"/analysis",
+		strings.NewReader(body),
+	)
+	if err != nil {
+		return patchResult{err: err}
+	}
+
+	req.Header = map[string][]string{
+		"Authorization": {goodToken},
+		"Content-Type":  {"application/merge-patch+json"},
+	}
+
+	res, err := app.Test(req, -1)
+	if res == nil {
+		return patchResult{err: err}
+	}
+
+	_ = res.Body.Close()
+
+	return patchResult{status: res.StatusCode, err: err}
+}
+
+// holdFirstAnalysisWrite blocks the first UPDATE of the analysis of
+// softwareID until release is called. The test needs it because two
+// requests started together overlap only by chance: a handler that reads
+// the document, merges it in Go and writes it back passes whenever the
+// first request finishes before the second starts.
+func holdFirstAnalysisWrite(t *testing.T, softwareID string) (waitForPark, release func()) {
+	t.Helper()
+
+	const name = "test:hold_analysis_write"
+
+	parked := make(chan struct{})
+	proceed := make(chan struct{})
+
+	var once sync.Once
+
+	updates := gormDB.Callback().Update()
+
+	require.NoError(t, updates.Before("gorm:update").Register(name, func(tx *gorm.DB) {
+		software, ok := tx.Statement.Model.(*models.Software)
+		if !ok || software.ID != softwareID {
+			return
+		}
+
+		first := false
+		once.Do(func() { first = true })
+
+		if !first {
+			return
+		}
+
+		close(parked)
+		<-proceed
+	}))
+
+	t.Cleanup(func() { require.NoError(t, updates.Remove(name)) })
+
+	waitForPark = func() {
+		select {
+		case <-parked:
+		case <-time.After(10 * time.Second):
+			require.FailNow(t, "the first analysis PATCH never reached its write")
+		}
+	}
+
+	return waitForPark, func() { close(proceed) }
+}
+
 func TestSoftwareDeleteDBChecks(t *testing.T) {
 	t.Run("DELETE software removes software_urls rows", func(t *testing.T) {
 		loadFixtures(t)