fix: write an analysis namespace another writer changed meanwhile

CHANGELOG.md

@@ -58,6 +58,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   still notified the webhooks of an update that never happened.
 - On SQLite, a stored analysis namespace that was not a JSON object
   made every later `PATCH` on that row answer `500`.
+- A `PATCH` on the analysis dropped a namespace that another writer had
+  changed since the client read it.
 
 ### Security
 

internal/database/analysis.go

@@ -62,6 +62,12 @@ func MergeAnalysis(
 	return merged, nil
 }
 
+// analysisMergeExpression builds the SET expression of the analysis merge.
+// Each incoming namespace is compared with the stored one, "t" excluded:
+// equal keeps the stored namespace and its timestamp, different writes the
+// incoming one. The comparison runs on the row being updated, so a
+// namespace another writer changed after the client read it is still
+// written.
 func analysisMergeExpression(dialect Dialect, patch common.AnalysisData) (clause.Expr, error) {
 	patchJSON, err := json.Marshal(patch)
 	if err != nil {
@@ -70,8 +76,18 @@ func analysisMergeExpression(dialect Dialect, patch common.AnalysisData) (clause
 
 	switch dialect {
 	case Postgres:
+		// Postgres refuses to delete a key from a jsonb scalar, so when
+		// either side is not an object the incoming value is taken as is.
 		return gorm.Expr(
-			"COALESCE(analysis, '{}'::jsonb) || CAST(? AS jsonb)",
+			`COALESCE(analysis, '{}'::jsonb) || (
+				SELECT COALESCE(jsonb_object_agg(patch.key, CASE
+					WHEN jsonb_typeof(analysis -> patch.key) <> 'object'
+						OR jsonb_typeof(patch.value) <> 'object' THEN patch.value
+					WHEN (analysis -> patch.key) - 't' = patch.value - 't' THEN analysis -> patch.key
+					ELSE patch.value
+				END), '{}'::jsonb)
+				FROM jsonb_each(CAST(? AS jsonb)) AS patch
+			)`,
 			string(patchJSON),
 		), nil
 	case SQLite:
@@ -82,18 +98,27 @@ func analysisMergeExpression(dialect Dialect, patch common.AnalysisData) (clause
 		// string comes back unquoted, true and false as 1 and 0. The ->
 		// operator returns every value as JSON text, so the namespace is read
 		// with it from the document instead.
+		// SQLite compares the two namespaces as text, so the keys must be in
+		// the same order on both sides. A namespace written through this API
+		// keeps the key order of the request, so it matches. One stored in
+		// another order counts as changed and only gets a fresh timestamp.
 		return gorm.Expr(
 			`(
 				WITH patch AS MATERIALIZED (
 					SELECT key, json -> fullkey AS value FROM json_each(json(?))
+				), stored AS MATERIALIZED (
+					SELECT key, json -> fullkey AS value FROM json_each(COALESCE(analysis, '{}'))
 				)
 				SELECT json_group_object(merged.key, json(merged.value))
 				FROM (
-					SELECT key, json -> fullkey AS value
-					FROM json_each(COALESCE(analysis, '{}'))
-					WHERE key NOT IN (SELECT key FROM patch)
+					SELECT key, value FROM stored WHERE key NOT IN (SELECT key FROM patch)
 					UNION ALL
-					SELECT key, value FROM patch
+					SELECT patch.key, CASE
+						WHEN json(json_remove(stored.value, '$.t')) = json(json_remove(patch.value, '$.t'))
+							THEN stored.value
+						ELSE patch.value
+					END
+					FROM patch LEFT JOIN stored ON stored.key = patch.key
 				) AS merged
 			)`,
 			string(patchJSON),

internal/database/analysis_test.go

@@ -318,3 +318,59 @@ func decodeValue(t *testing.T, value json.RawMessage) any {
 
 	return decoded
 }
+
+func TestMergeAnalysisKeepsTheStoredNamespaceWhenOnlyTheTimestampDiffers(t *testing.T) {
+	gormdb := openAnalysisTestDatabase(t)
+	recordID := utils.UUIDv4()
+
+	// The stored and the incoming namespace use the same key order, as the
+	// API writes them, so the textual comparison on SQLite matches.
+	require.NoError(t, gormdb.Create(&analysisMergeRecord{
+		ID: recordID,
+		Analysis: common.AnalysisData{
+			"scanner": json.RawMessage(`{"v":1,"score":90,"t":"2020-01-01T00:00:00Z"}`),
+		},
+	}).Error)
+	t.Cleanup(func() {
+		require.NoError(t, gormdb.Delete(&analysisMergeRecord{}, "id = ?", recordID).Error)
+	})
+
+	target := analysisMergeRecord{ID: recordID}
+	merged, err := MergeAnalysis(gormdb, &target, common.AnalysisData{
+		"scanner": json.RawMessage(`{"v":1,"score":90,"t":"2026-09-04T10:00:00Z"}`),
+	})
+	require.NoError(t, err)
+	assert.JSONEq(t, `{"v":1,"score":90,"t":"2020-01-01T00:00:00Z"}`, string(merged["scanner"]))
+
+	var stored analysisMergeRecord
+
+	require.NoError(t, gormdb.First(&stored, "id = ?", recordID).Error)
+	assert.JSONEq(t, `{"v":1,"score":90,"t":"2020-01-01T00:00:00Z"}`, string(stored.Analysis["scanner"]))
+}
+
+func TestMergeAnalysisWritesTheIncomingNamespaceWhenTheValueDiffers(t *testing.T) {
+	gormdb := openAnalysisTestDatabase(t)
+	recordID := utils.UUIDv4()
+
+	require.NoError(t, gormdb.Create(&analysisMergeRecord{
+		ID: recordID,
+		Analysis: common.AnalysisData{
+			"scanner": json.RawMessage(`{"v":1,"score":50,"t":"2020-01-01T00:00:00Z"}`),
+		},
+	}).Error)
+	t.Cleanup(func() {
+		require.NoError(t, gormdb.Delete(&analysisMergeRecord{}, "id = ?", recordID).Error)
+	})
+
+	target := analysisMergeRecord{ID: recordID}
+	merged, err := MergeAnalysis(gormdb, &target, common.AnalysisData{
+		"scanner": json.RawMessage(`{"v":1,"score":90,"t":"2026-09-04T10:00:00Z"}`),
+	})
+	require.NoError(t, err)
+	assert.JSONEq(t, `{"v":1,"score":90,"t":"2026-09-04T10:00:00Z"}`, string(merged["scanner"]))
+
+	var stored analysisMergeRecord
+
+	require.NoError(t, gormdb.First(&stored, "id = ?", recordID).Error)
+	assert.JSONEq(t, `{"v":1,"score":90,"t":"2026-09-04T10:00:00Z"}`, string(stored.Analysis["scanner"]))
+}

internal/handlers/catalogs.go

@@ -604,9 +604,7 @@ func (c *Catalog) PatchCatalogAnalysis(ctx *fiber.Ctx) error {
 		return common.Error(fiber.StatusUnprocessableEntity, errMsg, "invalid or malformed JSON")
 	}
 
-	// Validate and timestamp only the changed namespaces. The returned map
-	// is a database patch, not a copy of the complete analysis document.
-	patch, err := timestampTouchedAnalysis(catalog.Analysis, incoming, time.Now())
+	patch, err := common.WithTimestamps(incoming, time.Now())
 	if err != nil {
 		return common.Error(fiber.StatusUnprocessableEntity, errMsg, err.Error())
 	}

internal/handlers/software.go

@@ -338,28 +338,18 @@ func (p *Software) GetSoftwareAnalysis(ctx *fiber.Ctx) error {
 func (p *Software) PatchSoftwareAnalysis(ctx *fiber.Ctx) error {
 	const errMsg = "can't update Software analysis"
 
-	found, err := findOne[models.Software](p.db, ctx.Params("id"), findOptions{
-		title: errMsg,
-		name:  softwareEntityName,
-	})
-	if err != nil {
-		return err
-	}
-
-	software := *found
-
 	var incoming common.AnalysisData
 	if err := json.Unmarshal(ctx.Body(), &incoming); err != nil {
 		return common.Error(fiber.StatusUnprocessableEntity, errMsg, "invalid or malformed JSON")
 	}
 
-	// Validate and timestamp only the changed namespaces. The returned map
-	// is a database patch, not a copy of the complete analysis document.
-	patch, err := timestampTouchedAnalysis(software.Analysis, incoming, time.Now())
+	patch, err := common.WithTimestamps(incoming, time.Now())
 	if err != nil {
 		return common.Error(fiber.StatusUnprocessableEntity, errMsg, err.Error())
 	}
 
+	software := models.Software{ID: ctx.Params("id")}
+
 	merged, err := database.MergeAnalysis(writeDB(ctx, p.db), &software, patch)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
@@ -372,26 +362,6 @@ func (p *Software) PatchSoftwareAnalysis(ctx *fiber.Ctx) error {
 	return ctx.JSON(merged)
 }
 
-// timestampTouchedAnalysis returns the added or changed namespaces with a
-// fresh timestamp. Namespaces absent from updated are deliberately omitted:
-// the database merges this patch atomically with the value it currently has.
-func timestampTouchedAnalysis(
-	original, updated common.AnalysisData,
-	now time.Time,
-) (common.AnalysisData, error) {
-	touched := make(common.AnalysisData)
-
-	for namespace, value := range updated {
-		originalValue, exists := original[namespace]
-		if !exists || string(originalValue) != string(value) {
-			touched[namespace] = value
-		}
-	}
-
-	//nolint:wrapcheck // validation errors are safe API messages with their namespace prefix
-	return common.WithTimestamps(touched, now)
-}
-
 // softwareURLFilter narrows stmt to the software owning the ?url query
 // parameter. found is false when no software has that url, so the caller
 // answers with an empty list instead of running the query. title is the

software_test.go

@@ -1914,6 +1914,85 @@ func TestSoftwareAnalysisDBChecks(t *testing.T) {
 		assert.Contains(t, analysis, "scanner-one")
 		assert.Contains(t, analysis, "scanner-two")
 	})
+
+	t.Run("PATCH analysis keeps the timestamp of a namespace it does not change", func(t *testing.T) {
+		loadFixtures(t)
+
+		const softwareID = "59803fb7-8eec-4fe5-a354-8926009c364a"
+		const storedAt = "2020-01-01T00:00:00Z"
+
+		writeAnalysis(t, softwareID, `{"scanner":{"v":1,"score":90,"t":"`+storedAt+`"}}`)
+
+		code, body := patch(t, "/v1/software/"+softwareID+"/analysis",
+			"application/merge-patch+json", `{"scanner": {"v": 1, "score": 90}}`)
+
+		assert.Equal(t, 200, code, "body: %s", body)
+
+		scanner := storedNamespace(t, softwareID, "scanner")
+		assert.Equal(t, storedAt, scanner["t"])
+		assert.Equal(t, float64(90), scanner["score"])
+	})
+
+	t.Run("PATCH analysis writes a namespace another writer changed", func(t *testing.T) {
+		loadFixtures(t)
+
+		const softwareID = "59803fb7-8eec-4fe5-a354-8926009c364a"
+		const body = `{"scanner": {"v": 1, "score": 90}}`
+
+		code, response := patch(t, "/v1/software/"+softwareID+"/analysis", "application/merge-patch+json", body)
+		require.Equal(t, 200, code, "body: %s", response)
+
+		seeded := assertRFC3339(t, storedNamespace(t, softwareID, "scanner")["t"])
+
+		writeAnalysis(t, softwareID, `{"scanner":{"v":1,"score":50,"t":"2020-01-01T00:00:00Z"}}`)
+
+		code, response = patch(t, "/v1/software/"+softwareID+"/analysis", "application/merge-patch+json", body)
+		assert.Equal(t, 200, code, "body: %s", response)
+
+		scanner := storedNamespace(t, softwareID, "scanner")
+		assert.Equal(t, float64(90), scanner["score"])
+		assert.False(t, assertRFC3339(t, scanner["t"]).Before(seeded))
+	})
+
+	t.Run("PATCH analysis with an identical body keeps the timestamp", func(t *testing.T) {
+		loadFixtures(t)
+
+		const softwareID = "59803fb7-8eec-4fe5-a354-8926009c364a"
+		const body = `{"scanner": {"v": 1, "score": 90}}`
+
+		code, response := patch(t, "/v1/software/"+softwareID+"/analysis", "application/merge-patch+json", body)
+		require.Equal(t, 200, code, "body: %s", response)
+
+		first := storedNamespace(t, softwareID, "scanner")["t"]
+
+		code, response = patch(t, "/v1/software/"+softwareID+"/analysis", "application/merge-patch+json", body)
+		assert.Equal(t, 200, code, "body: %s", response)
+
+		assert.Equal(t, first, storedNamespace(t, softwareID, "scanner")["t"])
+	})
+}
+
+// writeAnalysis sets the analysis column of a software row directly, as
+// another writer would between the client's read and its PATCH.
+func writeAnalysis(t *testing.T, softwareID, document string) {
+	t.Helper()
+
+	query := "UPDATE software SET analysis = " + placeholder(1) + " WHERE id = " + placeholder(2)
+
+	_, err := db.Exec(query, document, softwareID)
+	require.NoError(t, err)
+}
+
+// storedNamespace decodes one namespace out of the analysis column of a
+// software row.
+func storedNamespace(t *testing.T, softwareID, namespace string) map[string]any {
+	t.Helper()
+
+	var analysis map[string]map[string]any
+
+	require.NoError(t, json.Unmarshal([]byte(dbValue(t, "software", "analysis", "id", softwareID)), &analysis))
+
+	return analysis[namespace]
 }
 
 func TestSoftwareDeleteDBChecks(t *testing.T) {