From eb85681b4161d35c5b47aaedb7921b10ba606b7a Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Fri, 11 Apr 2025 12:28:03 +0000 Subject: [PATCH] fix: package_blob.has_blake2b may be null (#7520) - When looking for an existing blob, has_blake2b will be null when it was created prior to v26 migration in v11, when the field was introduced. - Add unit test and minimal refactoring to load fixtures. The AddFixture function should not be where it currently is because it cannot be used by some packages (circular import). But that's a refactor that needs to be elsewhere for backporting purposes. Fixes https://codeberg.org/forgejo/forgejo/issues/7519 Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- .../package_blob.yml | 17 +++++ models/packages/main_test.go | 31 +++++++++ models/packages/package_blob.go | 21 +++--- models/packages/package_blob_test.go | 65 +++++++++++++++++++ models/packages/package_test.go | 9 --- 5 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 models/fixtures/TestPackagesGetOrInsertBlob/package_blob.yml create mode 100644 models/packages/main_test.go create mode 100644 models/packages/package_blob_test.go diff --git a/models/fixtures/TestPackagesGetOrInsertBlob/package_blob.yml b/models/fixtures/TestPackagesGetOrInsertBlob/package_blob.yml new file mode 100644 index 0000000000..ec90787c43 --- /dev/null +++ b/models/fixtures/TestPackagesGetOrInsertBlob/package_blob.yml @@ -0,0 +1,17 @@ +- + id: 1 + size: 10 + hash_md5: HASHMD5_1 + hash_sha1: HASHSHA1_1 + hash_sha256: HASHSHA256_1 + hash_sha512: HASHSHA512_1 + hash_blake2b: HASHBLAKE2B_1 + created_unix: 946687980 +- + id: 2 + size: 20 + hash_md5: HASHMD5_2 + hash_sha1: HASHSHA1_2 + hash_sha256: HASHSHA256_2 + hash_sha512: HASHSHA512_2 + created_unix: 946687980 diff --git a/models/packages/main_test.go b/models/packages/main_test.go new file mode 100644 index 0000000000..8114461502 --- /dev/null +++ b/models/packages/main_test.go @@ -0,0 +1,31 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package packages + +import ( + "path/filepath" + "testing" + + "forgejo.org/models/unittest" + "forgejo.org/modules/setting" + + _ "forgejo.org/models" + _ "forgejo.org/models/actions" + _ "forgejo.org/models/activities" + _ "forgejo.org/models/forgefed" +) + +func AddFixtures(dirs ...string) func() { + return unittest.OverrideFixtures( + unittest.FixturesOptions{ + Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"), + Base: setting.AppWorkPath, + Dirs: dirs, + }, + ) +} + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} diff --git a/models/packages/package_blob.go b/models/packages/package_blob.go index e86f652540..0de4434ef8 100644 --- a/models/packages/package_blob.go +++ b/models/packages/package_blob.go @@ -44,14 +44,19 @@ func GetOrInsertBlob(ctx context.Context, pb *PackageBlob) (*PackageBlob, bool, existing := &PackageBlob{} - has, err := e.Where(builder.Eq{ - "size": pb.Size, - "hash_md5": pb.HashMD5, - "hash_sha1": pb.HashSHA1, - "hash_sha256": pb.HashSHA256, - "hash_sha512": pb.HashSHA512, - "hash_blake2b": pb.HashBlake2b, - }).Get(existing) + has, err := e.Where(builder.And( + builder.Eq{ + "size": pb.Size, + "hash_md5": pb.HashMD5, + "hash_sha1": pb.HashSHA1, + "hash_sha256": pb.HashSHA256, + "hash_sha512": pb.HashSHA512, + }, + builder.Or( + builder.Eq{"hash_blake2b": pb.HashBlake2b}, + builder.IsNull{"hash_blake2b"}, + ), + )).Get(existing) if err != nil { return nil, false, err } diff --git a/models/packages/package_blob_test.go b/models/packages/package_blob_test.go new file mode 100644 index 0000000000..3ad45a56f9 --- /dev/null +++ b/models/packages/package_blob_test.go @@ -0,0 +1,65 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: GPL-3.0-or-later + +package packages + +import ( + "testing" + + "forgejo.org/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPackagesGetOrInsertBlob(t *testing.T) { + defer AddFixtures("models/fixtures/TestPackagesGetOrInsertBlob/")() + require.NoError(t, unittest.PrepareTestDatabase()) + + blake2bIsSet := unittest.AssertExistsAndLoadBean(t, &PackageBlob{ID: 1}) + blake2bNotSet := unittest.AssertExistsAndLoadBean(t, &PackageBlob{ID: 2}) + + var blake2bSetToRandom PackageBlob + blake2bSetToRandom = *blake2bNotSet + blake2bSetToRandom.HashBlake2b = "SOMETHING RANDOM" + + for _, testCase := range []struct { + name string + exists bool + packageBlob *PackageBlob + }{ + { + name: "exists and blake2b is not null in the database", + exists: true, + packageBlob: blake2bIsSet, + }, + { + name: "exists and blake2b is null in the database", + exists: true, + packageBlob: &blake2bSetToRandom, + }, + { + name: "does not exists", + exists: false, + packageBlob: &PackageBlob{ + Size: 30, + HashMD5: "HASHMD5_3", + HashSHA1: "HASHSHA1_3", + HashSHA256: "HASHSHA256_3", + HashSHA512: "HASHSHA512_3", + HashBlake2b: "HASHBLAKE2B_3", + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + found, has, _ := GetOrInsertBlob(t.Context(), testCase.packageBlob) + assert.Equal(t, testCase.exists, has) + require.NotNil(t, found) + if testCase.exists { + assert.Equal(t, found.ID, testCase.packageBlob.ID) + } else { + unittest.BeanExists(t, &PackageBlob{ID: found.ID}) + } + }) + } +} diff --git a/models/packages/package_test.go b/models/packages/package_test.go index ab2d49c94c..3c1ec413fd 100644 --- a/models/packages/package_test.go +++ b/models/packages/package_test.go @@ -13,18 +13,9 @@ import ( "forgejo.org/models/unittest" user_model "forgejo.org/models/user" - _ "forgejo.org/models" - _ "forgejo.org/models/actions" - _ "forgejo.org/models/activities" - _ "forgejo.org/models/forgefed" - "github.com/stretchr/testify/require" ) -func TestMain(m *testing.M) { - unittest.MainTest(m) -} - func prepareExamplePackage(t *testing.T) *packages_model.Package { require.NoError(t, unittest.PrepareTestDatabase())