From 87d6e76773cf18975c97fa289f6e0fa170a1538d Mon Sep 17 00:00:00 2001 From: delvh Date: Mon, 7 Aug 2023 22:08:53 +0200 Subject: [PATCH] Revert "Fix the wrong derive path (#26271) (#26318)" This reverts commit 88f6f7579cdaa557333bc86b3e45bf6458d889b6. --- modules/setting/storage.go | 73 ++++----------- modules/setting/storage_test.go | 159 -------------------------------- 2 files changed, 17 insertions(+), 215 deletions(-) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index c28f2be4ed714..ed804a910a461 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -84,15 +84,12 @@ func getDefaultStorageSection(rootCfg ConfigProvider) ConfigSection { return storageSec } -// getStorage will find target section and extra special section first and then read override -// items from extra section func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*Storage, error) { if name == "" { return nil, errors.New("no name for storage") } var targetSec ConfigSection - // check typ first if typ != "" { var err error targetSec, err = rootCfg.GetSection(storageSectionName + "." + typ) @@ -114,40 +111,24 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S } } - if targetSec == nil && sec != nil { - secTyp := sec.Key("STORAGE_TYPE").String() - if IsValidStorageType(StorageType(secTyp)) { - targetSec = sec - } else if secTyp != "" { - targetSec, _ = rootCfg.GetSection(storageSectionName + "." + secTyp) - } - } - - targetSecIsStoragename := false storageNameSec, _ := rootCfg.GetSection(storageSectionName + "." + name) + + if targetSec == nil { + targetSec = sec + } if targetSec == nil { targetSec = storageNameSec - targetSecIsStoragename = storageNameSec != nil } - if targetSec == nil { targetSec = getDefaultStorageSection(rootCfg) } else { targetType := targetSec.Key("STORAGE_TYPE").String() switch { case targetType == "": - if targetSec != storageNameSec && storageNameSec != nil { - targetSec = storageNameSec - targetSecIsStoragename = true - if targetSec.Key("STORAGE_TYPE").String() == "" { - return nil, fmt.Errorf("storage section %s.%s has no STORAGE_TYPE", storageSectionName, name) - } + if targetSec.Key("PATH").String() == "" { + targetSec = getDefaultStorageSection(rootCfg) } else { - if targetSec.Key("PATH").String() == "" { - targetSec = getDefaultStorageSection(rootCfg) - } else { - targetSec.Key("STORAGE_TYPE").SetValue("local") - } + targetSec.Key("STORAGE_TYPE").SetValue("local") } default: newTargetSec, _ := rootCfg.GetSection(storageSectionName + "." + targetType) @@ -172,46 +153,26 @@ func getStorage(rootCfg ConfigProvider, name, typ string, sec ConfigSection) (*S return nil, fmt.Errorf("invalid storage type %q", targetType) } - // extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH, MINIO_BUCKET to override the targetsec when possible - extraConfigSec := sec - if extraConfigSec == nil { - extraConfigSec = storageNameSec - } - var storage Storage storage.Type = StorageType(targetType) switch targetType { case string(LocalStorageType): - targetPath := ConfigSectionKeyString(targetSec, "PATH", "") - if targetPath == "" { - targetPath = AppDataPath - } else if !filepath.IsAbs(targetPath) { - targetPath = filepath.Join(AppDataPath, targetPath) - } - - var fallbackPath string - if targetSecIsStoragename { - fallbackPath = targetPath - } else { - fallbackPath = filepath.Join(targetPath, name) - } - - if extraConfigSec == nil { - storage.Path = fallbackPath - } else { - storage.Path = ConfigSectionKeyString(extraConfigSec, "PATH", fallbackPath) - if !filepath.IsAbs(storage.Path) { - storage.Path = filepath.Join(targetPath, storage.Path) - } + storage.Path = ConfigSectionKeyString(targetSec, "PATH", filepath.Join(AppDataPath, name)) + if !filepath.IsAbs(storage.Path) { + storage.Path = filepath.Join(AppWorkPath, storage.Path) } - case string(MinioStorageType): + storage.MinioConfig.BasePath = name + "/" + if err := targetSec.MapTo(&storage.MinioConfig); err != nil { return nil, fmt.Errorf("map minio config failed: %v", err) } - - storage.MinioConfig.BasePath = name + "/" + // extra config section will be read SERVE_DIRECT, PATH, MINIO_BASE_PATH to override the targetsec + extraConfigSec := sec + if extraConfigSec == nil { + extraConfigSec = storageNameSec + } if extraConfigSec != nil { storage.MinioConfig.ServeDirect = ConfigSectionKeyBool(extraConfigSec, "SERVE_DIRECT", storage.MinioConfig.ServeDirect) diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 9a83f31d918b4..4eda7646e81d0 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -4,7 +4,6 @@ package setting import ( - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -91,161 +90,3 @@ STORAGE_TYPE = minio assert.EqualValues(t, "gitea", RepoAvatar.Storage.MinioConfig.Bucket) assert.EqualValues(t, "repo-avatars/", RepoAvatar.Storage.MinioConfig.BasePath) } - -type testLocalStoragePathCase struct { - loader func(rootCfg ConfigProvider) error - storagePtr **Storage - expectedPath string -} - -func testLocalStoragePath(t *testing.T, appDataPath, iniStr string, cases []testLocalStoragePathCase) { - cfg, err := NewConfigProviderFromData(iniStr) - assert.NoError(t, err) - AppDataPath = appDataPath - for _, c := range cases { - assert.NoError(t, c.loader(cfg)) - storage := *c.storagePtr - - assert.EqualValues(t, "local", storage.Type) - assert.True(t, filepath.IsAbs(storage.Path)) - assert.EqualValues(t, filepath.Clean(c.expectedPath), filepath.Clean(storage.Path)) - } -} - -func Test_getStorageInheritStorageTypeLocal(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/repo-archive"}, - {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPath(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -PATH = /data/gitea -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"}, - {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/data/gitea/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/data/gitea/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalRelativePath(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -PATH = storages -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/appdata/storages/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/appdata/storages/repo-archive"}, - {loadActionsFrom, &Actions.LogStorage, "/appdata/storages/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/appdata/storages/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/storages/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPathOverride(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -PATH = /data/gitea - -[repo-archive] -PATH = /data/gitea/the-archives-dir -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"}, - {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/data/gitea/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/data/gitea/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPathOverrideEmpty(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -PATH = /data/gitea - -[repo-archive] -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/repo-archive"}, - {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/data/gitea/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/data/gitea/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalRelativePathOverride(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage] -STORAGE_TYPE = local -PATH = /data/gitea - -[repo-archive] -PATH = the-archives-dir -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/data/gitea/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/the-archives-dir"}, - {loadActionsFrom, &Actions.LogStorage, "/data/gitea/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/data/gitea/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/data/gitea/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPathOverride3(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage.repo-archive] -STORAGE_TYPE = local -PATH = /data/gitea/archives -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"}, - {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPathOverride4(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage.repo-archive] -STORAGE_TYPE = local -PATH = /data/gitea/archives - -[repo-archive] -PATH = /tmp/gitea/archives -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/tmp/gitea/archives"}, - {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, - }) -} - -func Test_getStorageInheritStorageTypeLocalPathOverride5(t *testing.T) { - testLocalStoragePath(t, "/appdata", ` -[storage.repo-archive] -STORAGE_TYPE = local -PATH = /data/gitea/archives - -[repo-archive] -`, []testLocalStoragePathCase{ - {loadPackagesFrom, &Packages.Storage, "/appdata/packages"}, - {loadRepoArchiveFrom, &RepoArchive.Storage, "/data/gitea/archives"}, - {loadActionsFrom, &Actions.LogStorage, "/appdata/actions_log"}, - {loadAvatarsFrom, &Avatar.Storage, "/appdata/avatars"}, - {loadRepoAvatarFrom, &RepoAvatar.Storage, "/appdata/repo-avatars"}, - }) -}