Fix storage config implementation (#14091)

The design is very flexible, but not implemented correctly.
This commit fixes several issues:
* Costom storage type stated in https://docs.gitea.io/en-us/config-cheat-sheet/#storage-storage
  not working
* [storage.attachments], [storage.minio] section not respected

Signed-off-by: 胡玮文 <huww98@outlook.com>

diff --git a/modules/setting/storage.go b/modules/setting/storage.go
index 27788da1ffdd02177d29bfc71fcea0488fc576cd..43890371fa9d95fd34dcf733c55235bc6d6b58e5 100644 (file)
--- a/modules/setting/storage.go
+++ b/modules/setting/storage.go
@@ -31,22 +31,10 @@ func (s *Storage) MapTo(v interface{}) error {
        return nil
 }
 
-func getStorage(name, typ string, overrides ...*ini.Section) Storage {
+func getStorage(name, typ string, targetSec *ini.Section) Storage {
        const sectionName = "storage"
        sec := Cfg.Section(sectionName)
 
-       if len(overrides) == 0 {
-               overrides = []*ini.Section{
-                       Cfg.Section(sectionName + "." + typ),
-                       Cfg.Section(sectionName + "." + name),
-               }
-       }
-
-       var storage Storage
-
-       storage.Type = sec.Key("STORAGE_TYPE").MustString(typ)
-       storage.ServeDirect = sec.Key("SERVE_DIRECT").MustBool(false)
-
        // Global Defaults
        sec.Key("MINIO_ENDPOINT").MustString("localhost:9000")
        sec.Key("MINIO_ACCESS_KEY_ID").MustString("")
@@ -55,18 +43,22 @@ func getStorage(name, typ string, overrides ...*ini.Section) Storage {
        sec.Key("MINIO_LOCATION").MustString("us-east-1")
        sec.Key("MINIO_USE_SSL").MustBool(false)
 
-       storage.Section = sec
-
-       for _, override := range overrides {
-               for _, key := range storage.Section.Keys() {
-                       if !override.HasKey(key.Name()) {
-                               _, _ = override.NewKey(key.Name(), key.Value())
+       nameSec := Cfg.Section(sectionName + "." + name)
+       typeSec := Cfg.Section(sectionName + "." + typ)
+       for _, override := range []*ini.Section{nameSec, typeSec, sec} {
+               for _, key := range override.Keys() {
+                       if !targetSec.HasKey(key.Name()) {
+                               _, _ = targetSec.NewKey(key.Name(), key.Value())
                        }
                }
-               storage.ServeDirect = override.Key("SERVE_DIRECT").MustBool(false)
-               storage.Section = override
        }
 
+       var storage Storage
+       storage.Section = targetSec
+
+       storage.Type = typeSec.Key("STORAGE_TYPE").MustString(typ)
+       storage.ServeDirect = storage.Section.Key("SERVE_DIRECT").MustBool(false)
+
        // Specific defaults
        storage.Path = storage.Section.Key("PATH").MustString(filepath.Join(AppDataPath, name))
        if !filepath.IsAbs(storage.Path) {

diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go
new file mode 100644 (file)
index 0000000..00c255a
--- /dev/null
+++ b/modules/setting/storage_test.go
@@ -0,0 +1,164 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package setting
+
+import (
+       "testing"
+
+       "github.com/stretchr/testify/assert"
+       ini "gopkg.in/ini.v1"
+)
+
+func Test_getStorageCustomType(t *testing.T) {
+       iniStr := `
+[attachment]
+STORAGE_TYPE = my_minio
+MINIO_BUCKET = gitea-attachment
+
+[storage.my_minio]
+STORAGE_TYPE = minio
+MINIO_ENDPOINT = my_minio:9000
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       sec := Cfg.Section("attachment")
+       storageType := sec.Key("STORAGE_TYPE").MustString("")
+       storage := getStorage("attachments", storageType, sec)
+
+       assert.EqualValues(t, "minio", storage.Type)
+       assert.EqualValues(t, "my_minio:9000", storage.Section.Key("MINIO_ENDPOINT").String())
+       assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String())
+}
+
+func Test_getStorageNameSectionOverridesTypeSection(t *testing.T) {
+       iniStr := `
+[attachment]
+STORAGE_TYPE = minio
+
+[storage.attachments]
+MINIO_BUCKET = gitea-attachment
+
+[storage.minio]
+MINIO_BUCKET = gitea
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       sec := Cfg.Section("attachment")
+       storageType := sec.Key("STORAGE_TYPE").MustString("")
+       storage := getStorage("attachments", storageType, sec)
+
+       assert.EqualValues(t, "minio", storage.Type)
+       assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String())
+}
+
+func Test_getStorageTypeSectionOverridesStorageSection(t *testing.T) {
+       iniStr := `
+[attachment]
+STORAGE_TYPE = minio
+
+[storage.minio]
+MINIO_BUCKET = gitea-minio
+
+[storage]
+MINIO_BUCKET = gitea
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       sec := Cfg.Section("attachment")
+       storageType := sec.Key("STORAGE_TYPE").MustString("")
+       storage := getStorage("attachments", storageType, sec)
+
+       assert.EqualValues(t, "minio", storage.Type)
+       assert.EqualValues(t, "gitea-minio", storage.Section.Key("MINIO_BUCKET").String())
+}
+
+func Test_getStorageSpecificOverridesStorage(t *testing.T) {
+       iniStr := `
+[attachment]
+MINIO_BUCKET = gitea-attachment
+
+[storage.attachments]
+MINIO_BUCKET = gitea
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       sec := Cfg.Section("attachment")
+       storageType := sec.Key("STORAGE_TYPE").MustString("")
+       storage := getStorage("attachments", storageType, sec)
+
+       assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String())
+}
+
+func Test_getStorageGetDefaults(t *testing.T) {
+       Cfg, _ = ini.Load([]byte(""))
+
+       sec := Cfg.Section("attachment")
+       storageType := sec.Key("STORAGE_TYPE").MustString("")
+       storage := getStorage("attachments", storageType, sec)
+
+       assert.EqualValues(t, "gitea", storage.Section.Key("MINIO_BUCKET").String())
+}
+
+func Test_getStorageMultipleName(t *testing.T) {
+       iniStr := `
+[lfs]
+MINIO_BUCKET = gitea-lfs
+
+[attachment]
+MINIO_BUCKET = gitea-attachment
+
+[storage]
+MINIO_BUCKET = gitea-storage
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       {
+               sec := Cfg.Section("attachment")
+               storageType := sec.Key("STORAGE_TYPE").MustString("")
+               storage := getStorage("attachments", storageType, sec)
+
+               assert.EqualValues(t, "gitea-attachment", storage.Section.Key("MINIO_BUCKET").String())
+       }
+       {
+               sec := Cfg.Section("lfs")
+               storageType := sec.Key("STORAGE_TYPE").MustString("")
+               storage := getStorage("lfs", storageType, sec)
+
+               assert.EqualValues(t, "gitea-lfs", storage.Section.Key("MINIO_BUCKET").String())
+       }
+       {
+               sec := Cfg.Section("avatar")
+               storageType := sec.Key("STORAGE_TYPE").MustString("")
+               storage := getStorage("avatars", storageType, sec)
+
+               assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String())
+       }
+}
+
+func Test_getStorageUseOtherNameAsType(t *testing.T) {
+       iniStr := `
+[attachment]
+STORAGE_TYPE = lfs
+
+[storage.lfs]
+MINIO_BUCKET = gitea-storage
+`
+       Cfg, _ = ini.Load([]byte(iniStr))
+
+       {
+               sec := Cfg.Section("attachment")
+               storageType := sec.Key("STORAGE_TYPE").MustString("")
+               storage := getStorage("attachments", storageType, sec)
+
+               assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String())
+       }
+       {
+               sec := Cfg.Section("lfs")
+               storageType := sec.Key("STORAGE_TYPE").MustString("")
+               storage := getStorage("lfs", storageType, sec)
+
+               assert.EqualValues(t, "gitea-storage", storage.Section.Key("MINIO_BUCKET").String())
+       }
+}