From 20148e061aff1e63f0549aa444d41b12d6cc041f Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 4 Jun 2024 23:20:20 +0200 Subject: [PATCH 1/2] test(storage): export UninitializedStorage to simulate failure --- modules/storage/helper.go | 2 +- modules/storage/helper_test.go | 2 +- modules/storage/storage.go | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/storage/helper.go b/modules/storage/helper.go index f8dff9e64..4c24209f4 100644 --- a/modules/storage/helper.go +++ b/modules/storage/helper.go @@ -10,7 +10,7 @@ import ( "os" ) -var uninitializedStorage = discardStorage("uninitialized storage") +var UninitializedStorage = discardStorage("uninitialized storage") type discardStorage string diff --git a/modules/storage/helper_test.go b/modules/storage/helper_test.go index f4c2d0467..d0665b6dc 100644 --- a/modules/storage/helper_test.go +++ b/modules/storage/helper_test.go @@ -12,7 +12,7 @@ import ( func Test_discardStorage(t *testing.T) { tests := []discardStorage{ - uninitializedStorage, + UninitializedStorage, discardStorage("empty"), } for _, tt := range tests { diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 8f970b5df..90f74eb2b 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -109,26 +109,26 @@ func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) err var ( // Attachments represents attachments storage - Attachments ObjectStorage = uninitializedStorage + Attachments ObjectStorage = UninitializedStorage // LFS represents lfs storage - LFS ObjectStorage = uninitializedStorage + LFS ObjectStorage = UninitializedStorage // Avatars represents user avatars storage - Avatars ObjectStorage = uninitializedStorage + Avatars ObjectStorage = UninitializedStorage // RepoAvatars represents repository avatars storage - RepoAvatars ObjectStorage = uninitializedStorage + RepoAvatars ObjectStorage = UninitializedStorage // RepoArchives represents repository archives storage - RepoArchives ObjectStorage = uninitializedStorage + RepoArchives ObjectStorage = UninitializedStorage // Packages represents packages storage - Packages ObjectStorage = uninitializedStorage + Packages ObjectStorage = UninitializedStorage // Actions represents actions storage - Actions ObjectStorage = uninitializedStorage + Actions ObjectStorage = UninitializedStorage // Actions Artifacts represents actions artifacts storage - ActionsArtifacts ObjectStorage = uninitializedStorage + ActionsArtifacts ObjectStorage = UninitializedStorage ) // Init init the stoarge From c139efb1e9e65998ad557159b0c93ddc871b6b59 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 4 Jun 2024 23:21:00 +0200 Subject: [PATCH 2/2] test(avatar): deleting a user avatar and file is atomic The avatar must not be unset in the database if there is a failure to remove the avatar file from storage (file or S3). The two operations are wrapped in a transaction for that purpose and this test verifies it is effective. See 1be797faba301503e29db9de7eb32335a684464c Fix bug on avatar --- services/user/avatar_test.go | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 services/user/avatar_test.go diff --git a/services/user/avatar_test.go b/services/user/avatar_test.go new file mode 100644 index 000000000..0dc4dec65 --- /dev/null +++ b/services/user/avatar_test.go @@ -0,0 +1,47 @@ +// Copyright The Forgejo Authors. +// SPDX-License-Identifier: MIT + +package user + +import ( + "bytes" + "image" + "image/png" + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" +) + +func TestUserDeleteAvatar(t *testing.T) { + myImage := image.NewRGBA(image.Rect(0, 0, 1, 1)) + var buff bytes.Buffer + png.Encode(&buff, myImage) + + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + err := UploadAvatar(db.DefaultContext, user, buff.Bytes()) + assert.NoError(t, err) + verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.NotEqual(t, "", verification.Avatar) + + t.Run("AtomicStorageFailure", func(t *testing.T) { + defer test.MockVariableValue[storage.ObjectStorage](&storage.Avatars, storage.UninitializedStorage)() + err = DeleteAvatar(db.DefaultContext, user) + assert.Error(t, err) + verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.True(t, verification.UseCustomAvatar) + }) + + err = DeleteAvatar(db.DefaultContext, user) + assert.NoError(t, err) + + verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.Equal(t, "", verification.Avatar) +}