diff --git a/app/other/apis/file.go b/app/other/apis/file.go index 8cea78b4..fe8f56d2 100644 --- a/app/other/apis/file.go +++ b/app/other/apis/file.go @@ -14,6 +14,7 @@ import ( "github.com/google/uuid" "go-admin/common/file_store" + "go-admin/config" ) type FileResponse struct { @@ -95,7 +96,7 @@ func (e File) baseImg(c *gin.Context, fileResponse FileResponse, urlPrefix strin source, _ := c.GetPostForm("source") if err := thirdUpload(source, fileName, base64File); err != nil { - e.Error(200, errors.New(""), "上传第三方失败") + e.Error(200, err, "上传第三方失败") return fileResponse } @@ -126,7 +127,7 @@ func (e File) multipleFile(c *gin.Context, urlPrefix string) []FileResponse { fileType, _ := utils.GetType(multipartFileName) if err := thirdUpload(source, fileName, multipartFileName); err != nil { - e.Error(500, errors.New(""), "上传第三方失败") + e.Error(500, err, "上传第三方失败") continue } @@ -176,22 +177,36 @@ func (e File) buildFileResponse(filePath, urlPrefix, fileName, fileType string) } } +// thirdUpload copies the file that was already stored locally to the object +// store the request asked for. source "1", and anything unrecognised, keeps the +// local copy only. +// +// Both branches used to construct a zero-value ALiYunOSS and call UpLoad on it, +// which panicked - and the qiniu branch constructed the aliyun client, so +// source=3 never reached qiniu even in principle. func thirdUpload(source string, name string, path string) error { switch source { case "2": - return ossUpload("img/"+name, path) + return upload(file_store.AliYunOSS, config.ExtConfig.FileStore.AliYun, "img/"+name, path) case "3": - return qiniuUpload("img/"+name, path) + return upload(file_store.QiNiuKodo, config.ExtConfig.FileStore.QiNiu, "img/"+name, path) } return nil } -func ossUpload(name string, path string) error { - oss := file_store.ALiYunOSS{} - return oss.UpLoad(name, path) -} - -func qiniuUpload(name string, path string) error { - oss := file_store.ALiYunOSS{} - return oss.UpLoad(name, path) +func upload(driver file_store.DriverType, store config.ObjectStore, name, path string) error { + if !store.Configured() { + return fmt.Errorf("file store %s is not configured; set it under extend.fileStore", driver) + } + oxs := file_store.OXS{ + Endpoint: store.Endpoint, + AccessKeyID: store.AccessKeyID, + AccessKeySecret: store.AccessKeySecret, + BucketName: store.BucketName, + } + client, err := oxs.Setup(driver) + if err != nil { + return err + } + return client.UpLoad(name, path) } diff --git a/app/other/apis/upload_test.go b/app/other/apis/upload_test.go new file mode 100644 index 00000000..9831ff2d --- /dev/null +++ b/app/other/apis/upload_test.go @@ -0,0 +1,38 @@ +package apis + +import ( + "strings" + "testing" + + "go-admin/config" +) + +// Both branches used to construct a zero-value ALiYunOSS and call UpLoad on it, +// which panicked; the qiniu branch built the aliyun client, so source=3 could +// not have reached qiniu even with credentials. Unconfigured now reports which +// store is missing. +func TestThirdUploadReportsAnUnconfiguredStore(t *testing.T) { + previous := config.ExtConfig.FileStore + config.ExtConfig.FileStore = config.FileStore{} + t.Cleanup(func() { config.ExtConfig.FileStore = previous }) + + for source, want := range map[string]string{"2": "AliYunOSS", "3": "QiNiuKodo"} { + err := thirdUpload(source, "x.png", "/tmp/x.png") + if err == nil { + t.Errorf("source=%s: no error from an unconfigured store", source) + continue + } + if !strings.Contains(err.Error(), want) { + t.Errorf("source=%s: error names %q, want it to mention %s", source, err, want) + } + } +} + +// source 1 and anything unrecognised keep the local copy and do nothing else. +func TestThirdUploadIgnoresLocalAndUnknownSources(t *testing.T) { + for _, source := range []string{"", "1", "9"} { + if err := thirdUpload(source, "x.png", "/tmp/x.png"); err != nil { + t.Errorf("source=%q returned %v, want nil", source, err) + } + } +} diff --git a/common/file_store/errors.go b/common/file_store/errors.go new file mode 100644 index 00000000..2464d2c3 --- /dev/null +++ b/common/file_store/errors.go @@ -0,0 +1,20 @@ +package file_store + +import "fmt" + +// ErrNotConfigured is returned by an object store that was never given +// credentials. The Client field of every implementation is an interface{} +// assigned in Setup, so an unconfigured store holds nil, and asserting nil to +// the provider's client type panics. The upload endpoint reaches this path +// whenever a request asks for a provider the deployment has not configured. +type ErrNotConfigured struct { + Driver DriverType +} + +func (e *ErrNotConfigured) Error() string { + return fmt.Sprintf("file store %s is not configured; set it under extend.fileStore", e.Driver) +} + +func notConfigured(driver DriverType) error { + return &ErrNotConfigured{Driver: driver} +} diff --git a/common/file_store/file_store_test.go b/common/file_store/file_store_test.go new file mode 100644 index 00000000..3ea07f89 --- /dev/null +++ b/common/file_store/file_store_test.go @@ -0,0 +1,70 @@ +package file_store + +import ( + "errors" + "os" + "testing" +) + +// The three implementations keep their provider client in an interface{} field +// that Setup assigns, so an unconfigured store holds nil. Asserting nil to the +// provider's type panics, and the upload endpoint reaches that path whenever a +// request names a provider the deployment never configured. +func TestUnconfiguredStoresReportItInsteadOfPanicking(t *testing.T) { + stores := map[DriverType]FileStoreType{ + AliYunOSS: new(ALiYunOSS), + HuaweiOBS: new(HuaWeiOBS), + QiNiuKodo: new(QiNiuKODO), + } + + for driver, store := range stores { + t.Run(string(driver), func(t *testing.T) { + err := store.UpLoad("img/x.png", "/tmp/x.png") + if err == nil { + t.Fatal("upload on an unconfigured store returned no error") + } + var notCfg *ErrNotConfigured + if !errors.As(err, ¬Cfg) { + t.Fatalf("want ErrNotConfigured, got %v", err) + } + if notCfg.Driver != driver { + t.Errorf("error names %s, want %s", notCfg.Driver, driver) + } + }) + } +} + +func TestUnconfiguredTokenReportsItToo(t *testing.T) { + if _, err := new(QiNiuKODO).GetTempToken(); err == nil { + t.Fatal("token from an unconfigured store returned no error") + } +} + +func TestSetupRejectsAnUnknownDriver(t *testing.T) { + if _, err := (&OXS{}).Setup("NoSuchCloud"); err == nil { + t.Fatal("unknown driver was accepted") + } +} + +// Setup reaching the provider needs credentials, so it runs only when they are +// supplied. Previously the test carried a comment telling the reader to paste +// their own, which meant it failed for everyone who did not. +func TestSetupWithRealCredentials(t *testing.T) { + endpoint := os.Getenv("GOADMIN_OSS_ENDPOINT") + if endpoint == "" { + t.Skip("set GOADMIN_OSS_ENDPOINT, GOADMIN_OSS_AK, GOADMIN_OSS_SK, GOADMIN_OSS_BUCKET to run") + } + oxs := OXS{ + Endpoint: endpoint, + AccessKeyID: os.Getenv("GOADMIN_OSS_AK"), + AccessKeySecret: os.Getenv("GOADMIN_OSS_SK"), + BucketName: os.Getenv("GOADMIN_OSS_BUCKET"), + } + store, err := oxs.Setup(AliYunOSS) + if err != nil { + t.Fatalf("setup: %v", err) + } + if store == nil { + t.Fatal("setup returned no store and no error") + } +} diff --git a/common/file_store/initialize.go b/common/file_store/initialize.go index fabce9fb..377972fa 100644 --- a/common/file_store/initialize.go +++ b/common/file_store/initialize.go @@ -14,32 +14,24 @@ type OXS struct { } // Setup 配置文件存储driver -func (e *OXS) Setup(driver DriverType, options ...ClientOption) FileStoreType { - fileStoreType := driver +// +// A failed Setup used to be printed and the store returned anyway, so the +// caller received one whose Client was nil - which panicked on first use. The +// error is returned instead. +func (e *OXS) Setup(driver DriverType, options ...ClientOption) (FileStoreType, error) { var fileStore FileStoreType - switch fileStoreType { + switch driver { case AliYunOSS: fileStore = new(ALiYunOSS) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore case HuaweiOBS: fileStore = new(HuaWeiOBS) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore case QiNiuKodo: fileStore = new(QiNiuKODO) - err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName) - if err != nil { - fmt.Println(err) - } - return fileStore + default: + return nil, fmt.Errorf("unsupported file store driver %q", driver) } - - return nil + if err := fileStore.Setup(e.Endpoint, e.AccessKeyID, e.AccessKeySecret, e.BucketName, options...); err != nil { + return nil, fmt.Errorf("file store %s: %w", driver, err) + } + return fileStore, nil } diff --git a/common/file_store/kodo.go b/common/file_store/kodo.go index db896e27..1cad8d29 100644 --- a/common/file_store/kodo.go +++ b/common/file_store/kodo.go @@ -29,15 +29,20 @@ type QiNiuKODO struct { options []ClientOption } -func (e *QiNiuKODO) getToken() string { +func (e *QiNiuKODO) getToken() (string, error) { + mac, ok := e.Client.(*qbox.Mac) + if !ok { + return "", notConfigured(QiNiuKodo) + } putPolicy := storage.PutPolicy{ Scope: e.BucketName, } - if len(e.options) > 0 && e.options[0]["Expires"] != nil { - putPolicy.Expires = e.options[0]["Expires"].(uint64) + if len(e.options) > 0 { + if expires, ok := e.options[0]["Expires"].(uint64); ok { + putPolicy.Expires = expires + } } - upToken := putPolicy.UploadToken(e.Client.(*qbox.Mac)) - return upToken + return putPolicy.UploadToken(mac), nil } //Setup 装载 @@ -96,16 +101,21 @@ func (e *QiNiuKODO) UpLoad(yourObjectName string, localFile interface{}) error { "x:name": "github logo", }, } - err := formUploader.PutFile(context.Background(), &ret, e.getToken(), yourObjectName, localFile.(string), &putExtra) + token, err := e.getToken() if err != nil { - fmt.Println(err) return err } + source, ok := localFile.(string) + if !ok { + return fmt.Errorf("kodo upload wants a path, got %T", localFile) + } + if err := formUploader.PutFile(context.Background(), &ret, token, yourObjectName, source, &putExtra); err != nil { + return fmt.Errorf("kodo upload: %w", err) + } fmt.Println(ret.Key, ret.Hash) return nil } func (e *QiNiuKODO) GetTempToken() (string, error) { - token := e.getToken() - return token, nil + return e.getToken() } diff --git a/common/file_store/kodo_test.go b/common/file_store/kodo_test.go deleted file mode 100644 index a8767ae7..00000000 --- a/common/file_store/kodo_test.go +++ /dev/null @@ -1,23 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestKODOUpload(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(QiNiuKodo, map[string]interface{}{"Zone": "华东"}) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} - -func TestKODOGetTempToken(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(QiNiuKodo, map[string]interface{}{"Zone": "华东"}) - token, _ := oxs.GetTempToken() - t.Log(token) - t.Log("ok") -} diff --git a/common/file_store/obs.go b/common/file_store/obs.go index e9377291..a7d2ad9e 100644 --- a/common/file_store/obs.go +++ b/common/file_store/obs.go @@ -26,24 +26,30 @@ func (e *HuaWeiOBS) Setup(endpoint, accessKeyID, accessKeySecret, BucketName str // UpLoad 文件上传 // yourObjectName 文件路径名称,与objectKey是同一概念,表示断点续传上传文件到OSS时需要指定包含文件后缀在内的完整路径,例如abc/efg/123.jpg func (e *HuaWeiOBS) UpLoad(yourObjectName string, localFile interface{}) error { + client, ok := e.Client.(*obs.ObsClient) + if !ok { + return notConfigured(HuaweiOBS) + } + source, ok := localFile.(string) + if !ok { + return fmt.Errorf("obs upload wants a path, got %T", localFile) + } + // 获取存储空间。 input := &obs.PutFileInput{} input.Bucket = e.BucketName input.Key = yourObjectName - input.SourceFile = localFile.(string) - output, err := e.Client.(*obs.ObsClient).PutFile(input) - - if err == nil { - fmt.Printf("RequestId:%s\n", output.RequestId) - fmt.Printf("ETag:%s, StorageClass:%s\n", output.ETag, output.StorageClass) - } else { + input.SourceFile = source + output, err := client.PutFile(input) + if err != nil { + // The error used to be printed and nil returned, so a failed upload + // reported success to the caller. if obsError, ok := err.(obs.ObsError); ok { - fmt.Println(obsError.Code) - fmt.Println(obsError.Message) - } else { - fmt.Println(err) + return fmt.Errorf("obs upload: %s: %s", obsError.Code, obsError.Message) } + return fmt.Errorf("obs upload: %w", err) } + log.Printf("obs upload ok, requestId=%s etag=%s", output.RequestId, output.ETag) return nil } diff --git a/common/file_store/obs_test.go b/common/file_store/obs_test.go deleted file mode 100644 index 09607508..00000000 --- a/common/file_store/obs_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestOBSUpload(t *testing.T) { - e := OXS{"", "", "", ""} - var oxs = e.Setup(HuaweiOBS) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} diff --git a/common/file_store/oss.go b/common/file_store/oss.go index c35627b9..2a182eaf 100644 --- a/common/file_store/oss.go +++ b/common/file_store/oss.go @@ -26,8 +26,12 @@ func (e *ALiYunOSS) Setup(endpoint, accessKeyID, accessKeySecret, BucketName str // UpLoad 文件上传 func (e *ALiYunOSS) UpLoad(yourObjectName string, localFile interface{}) error { + client, ok := e.Client.(*oss.Client) + if !ok { + return notConfigured(AliYunOSS) + } // 获取存储空间。 - bucket, err := e.Client.(*oss.Client).Bucket(e.BucketName) + bucket, err := client.Bucket(e.BucketName) if err != nil { log.Println("Error:", err) return err diff --git a/common/file_store/oss_test.go b/common/file_store/oss_test.go deleted file mode 100644 index f04dd225..00000000 --- a/common/file_store/oss_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package file_store - -import ( - "testing" -) - -func TestOSSUpload(t *testing.T) { - // 打括号内填写自己的测试信息即可 - e := OXS{} - var oxs = e.Setup(AliYunOSS) - err := oxs.UpLoad("test.png", "./test.png") - if err != nil { - t.Error(err) - } - t.Log("ok") -} diff --git a/config/extend.go b/config/extend.go index bd06e031..e8d6daa6 100644 --- a/config/extend.go +++ b/config/extend.go @@ -3,14 +3,39 @@ package config var ExtConfig Extend // Extend 扩展配置 -// extend: -// demo: -// name: demo-name +// +// extend: +// demo: +// name: demo-name +// // 使用方法: config.ExtConfig......即可!! type Extend struct { - AMap AMap // 这里配置对应配置文件的结构即可 + AMap AMap // 这里配置对应配置文件的结构即可 + FileStore FileStore } type AMap struct { Key string } + +// FileStore 对象存储。上传接口的 source 参数决定走哪一家:2 是阿里云,3 是七牛。 +// 没有填的那一家在被请求时返回明确错误,而不是上传到别处或者崩溃。 +// +// common/file_store 里还实现了华为云 OBS,但上传接口没有对应的 source 取值, +// 所以这里也不为它提供配置。 +type FileStore struct { + AliYun ObjectStore + QiNiu ObjectStore +} + +type ObjectStore struct { + Endpoint string + AccessKeyID string + AccessKeySecret string + BucketName string +} + +// Configured reports whether enough was filled in to attempt a connection. +func (o ObjectStore) Configured() bool { + return o.Endpoint != "" && o.AccessKeyID != "" && o.AccessKeySecret != "" && o.BucketName != "" +} diff --git a/config/extend_test.go b/config/extend_test.go new file mode 100644 index 00000000..230174f4 --- /dev/null +++ b/config/extend_test.go @@ -0,0 +1,16 @@ +package config + +import "testing" + +func TestObjectStoreConfigured(t *testing.T) { + if (ObjectStore{}).Configured() { + t.Fatal("empty store reported as configured") + } + full := ObjectStore{Endpoint: "e", AccessKeyID: "a", AccessKeySecret: "s", BucketName: "b"} + if !full.Configured() { + t.Fatal("complete store reported as unconfigured") + } + if (ObjectStore{Endpoint: "e", AccessKeyID: "a"}).Configured() { + t.Fatal("partial store reported as configured") + } +} diff --git a/config/settings.full.yml b/config/settings.full.yml index bcca7e29..a1fdacce 100644 --- a/config/settings.full.yml +++ b/config/settings.full.yml @@ -56,3 +56,17 @@ settings: extend: # 扩展项使用说明 demo: name: data + # fileStore 对象存储。上传接口的 source 参数决定走哪一家: + # source=1 只存本地,source=2 阿里云 OSS,source=3 七牛 Kodo + # 没有填的那一家在被请求时会返回明确错误,不会静默存到别处。 + fileStore: + aliYun: + endpoint: oss-cn-hangzhou.aliyuncs.com + accessKeyId: "" + accessKeySecret: "" + bucketName: "" + qiNiu: + endpoint: "" + accessKeyId: "" + accessKeySecret: "" + bucketName: ""