fix: duplicate key violates unique constraint in concurrent debian package uploads (#11776)

Fixes #11438.

Whenever a "unique constraint violation" error is encountered by package mutation, detect if a `xorm.ErrUniqueConstraintViolation` error occurs.  If it does, retry the entire transaction.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests for Go changes

(can be removed for JavaScript changes)

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
  - [ ] `make pr-go` before pushing

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11776
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>

models/db/context.go

@@ -6,6 +6,8 @@ package db
 import (
 	"context"
 	"database/sql"
+	"errors"
+	"fmt"
 
 	"xorm.io/builder"
 	"xorm.io/xorm"
@@ -416,3 +418,42 @@ func inTransaction(ctx context.Context) (*xorm.Session, bool) {
 		return nil, false
 	}
 }
+
+type RetryConfig struct {
+	ErrorIs      []error
+	AttemptCount int
+}
+
+// Execute the given function in a transaction. RetryConfig will retry the function on an error, if it matches the
+// ErrorIs parameter, up to the total of AttemptCount number of tries. RetryTx cannot be invoked when already within a
+// transaction and will return an error immediately.
+func RetryTx(ctx context.Context, config RetryConfig, f func(ctx context.Context) error) error {
+	if InTransaction(ctx) {
+		return errors.New("unsupported operation: attempted to use RetryTx while already within a transaction")
+	} else if config.AttemptCount == 0 {
+		return errors.New("unsupported operation: attempted to use RetryTx with 0 attempts")
+	}
+
+	var lastError error
+	for range config.AttemptCount {
+		err := WithTx(ctx, f)
+		if err == nil {
+			return nil
+		}
+
+		foundMatch := false
+		for _, possibleError := range config.ErrorIs {
+			if errors.Is(err, possibleError) {
+				foundMatch = true
+				break
+			}
+		}
+		if !foundMatch {
+			return err
+		}
+
+		lastError = err
+	}
+
+	return fmt.Errorf("retry tx failed after %d attempts; last error: %w", config.AttemptCount, lastError)
+}

models/db/context_test.go

@@ -220,3 +220,59 @@ func TestAfterTx(t *testing.T) {
 		})
 	}
 }
+
+func TestRetryTx(t *testing.T) {
+	t.Run("success", func(t *testing.T) {
+		err := db.RetryTx(t.Context(), db.RetryConfig{AttemptCount: 1}, func(ctx context.Context) error {
+			assert.True(t, db.InTransaction(ctx))
+			return nil
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("fail constantly", func(t *testing.T) {
+		attemptCount := 0
+		testError := errors.New("hello")
+		err := db.RetryTx(t.Context(), db.RetryConfig{
+			AttemptCount: 2,
+			ErrorIs:      []error{testError},
+		}, func(ctx context.Context) error {
+			attemptCount++
+			return testError
+		})
+		require.ErrorIs(t, err, testError)
+		require.ErrorContains(t, err, "2 attempts")
+		assert.Equal(t, 2, attemptCount)
+	})
+
+	t.Run("fail w/ non retriable error", func(t *testing.T) {
+		attemptCount := 0
+		testError := errors.New("hello")
+		err := db.RetryTx(t.Context(), db.RetryConfig{
+			AttemptCount: 2,
+			ErrorIs:      []error{},
+		}, func(ctx context.Context) error {
+			attemptCount++
+			return testError
+		})
+		require.ErrorIs(t, err, testError)
+		assert.Equal(t, 1, attemptCount)
+	})
+
+	t.Run("succeed on retry", func(t *testing.T) {
+		attemptCount := 0
+		testError := errors.New("hello")
+		err := db.RetryTx(t.Context(), db.RetryConfig{
+			AttemptCount: 2,
+			ErrorIs:      []error{testError},
+		}, func(ctx context.Context) error {
+			attemptCount++
+			if attemptCount == 1 {
+				return testError
+			}
+			return nil
+		})
+		require.NoError(t, err)
+		assert.Equal(t, 2, attemptCount)
+	})
+}

models/packages/package_version.go

@@ -5,11 +5,13 @@ package packages
 
 import (
 	"context"
+	"errors"
 	"strconv"
 	"strings"
 
 	"forgejo.org/models/db"
 	"forgejo.org/modules/optional"
+	"forgejo.org/modules/setting"
 	"forgejo.org/modules/timeutil"
 	"forgejo.org/modules/util"
 
@@ -155,6 +157,25 @@ func HasVersionFileReferences(ctx context.Context, versionID int64) (bool, error
 	})
 }
 
+func (pv *PackageVersion) LockForUpdate(ctx context.Context) error {
+	if !db.InTransaction(ctx) {
+		return errors.New("invalid state for PackageVersion.LockForUpdate: database is not in a transaction")
+	} else if setting.Database.Type.IsSQLite3() {
+		// SQLite both doesn't support "SELECT ... FOR UPDATE", and it's irrelevant for SQLite as the entire database is
+		// locked for write when a write transaction is open.
+		return nil
+	}
+
+	pvfu := PackageVersion{}
+	has, err := db.GetEngine(ctx).ID(pv.ID).ForUpdate().Get(&pvfu)
+	if err != nil {
+		return err
+	} else if !has {
+		return ErrPackageNotExist
+	}
+	return nil
+}
+
 // SearchValue describes a value to search
 // If ExactMatch is true, the field must match the value otherwise a LIKE search is performed.
 type SearchValue struct {