From 96e6c01448fd0f4594f48963b4aedb4e5679bfef Mon Sep 17 00:00:00 2001 From: Li Xianggang Date: Thu, 20 Aug 2026 16:46:46 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=E4=B8=8A=E4=BC=A0obs?= =?UTF-8?q?=E8=B7=AF=E5=BE=84=E9=94=99=E8=AF=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/application/service/file/obs.go | 64 ++++----- internal/application/service/file/obs_test.go | 127 ++++++++++++++++++ internal/application/service/resource_test.go | 14 ++ 3 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 internal/application/service/file/obs_test.go diff --git a/internal/application/service/file/obs.go b/internal/application/service/file/obs.go index 451d92db5c..005844bafc 100644 --- a/internal/application/service/file/obs.go +++ b/internal/application/service/file/obs.go @@ -28,6 +28,8 @@ type obsFileService struct { proxyDomain string } +const obsScheme = "obs://" + type obsEndpointResolver struct { url string } @@ -118,33 +120,39 @@ func (s *obsFileService) CheckConnectivity(ctx context.Context) error { } func (s *obsFileService) parseObsFilePath(filePath string) (string, error) { - prefix := s.getPrifix() - - if strings.HasPrefix(filePath, prefix) { - rest := strings.TrimPrefix(filePath, prefix) - // With proxy domain: path is {prefix}/{objectKey} (no bucket name) - if s.proxyDomain != "" { - rest = strings.TrimPrefix(rest, "/") - if rest != "" { - return rest, nil - } - return "", fmt.Errorf("invalid OBS file path: %s", filePath) - } - // Without proxy domain: path is {prefix}/{bucketName}/{objectKey} + if strings.HasPrefix(filePath, obsScheme) { + rest := strings.TrimPrefix(filePath, obsScheme) parts := strings.SplitN(rest, "/", 2) if len(parts) == 2 && parts[0] == s.bucketName && parts[1] != "" { return parts[1], nil } return "", fmt.Errorf("invalid OBS file path: %s", filePath) } + + if s.proxyDomain != "" { + prefix := s.proxyDomain + "/" + if strings.HasPrefix(filePath, prefix) { + objectKey := strings.TrimPrefix(filePath, prefix) + if objectKey != "" { + return objectKey, nil + } + return "", fmt.Errorf("invalid OBS file path: %s", filePath) + } + } + + // Preserve support for legacy callers that pass a bare object key. return filePath, nil } -func (s *obsFileService) getPrifix() string { - if s.proxyDomain != "" { - return s.proxyDomain + "/" +func (s *obsFileService) buildObsFilePath(objectKey string) string { + return fmt.Sprintf("%s%s/%s", obsScheme, s.bucketName, strings.TrimPrefix(objectKey, "/")) +} + +func (s *obsFileService) ownsObsFilePath(filePath string) bool { + if strings.HasPrefix(filePath, obsScheme) { + return true } - return "obs://" + return s.proxyDomain != "" && strings.HasPrefix(filePath, s.proxyDomain+"/") } func (s *obsFileService) SaveFile(ctx context.Context, @@ -181,11 +189,7 @@ func (s *obsFileService) SaveFile(ctx context.Context, if err != nil { return "", fmt.Errorf("failed to upload file to OBS: %w", err) } - prefix := s.getPrifix() - if s.proxyDomain != "" { - return fmt.Sprintf("%s%s", prefix, objectKey), nil - } - return fmt.Sprintf("%s%s/%s", prefix, s.bucketName, objectKey), nil + return s.buildObsFilePath(objectKey), nil } func (s *obsFileService) GetFile(ctx context.Context, filePath string) (io.ReadCloser, error) { @@ -249,7 +253,7 @@ func (s *obsFileService) CopyFile(ctx context.Context, // Reject paths that do not use this service's prefix (proxy domain or obs://). // parseObsFilePath falls back to returning the raw input for unknown prefixes, // so guard explicitly here to detect cross-backend sources. - if !strings.HasPrefix(srcPath, s.getPrifix()) { + if !s.ownsObsFilePath(srcPath) { return "", fmt.Errorf("obs copy rejected source %q: %w", srcPath, ErrCrossBackendCopy) } srcKey, err := s.parseObsFilePath(srcPath) @@ -276,13 +280,7 @@ func (s *obsFileService) CopyFile(ctx context.Context, return "", fmt.Errorf("failed to copy file in OBS: %w", err) } - prefix := s.getPrifix() - var newPath string - if s.proxyDomain != "" { - newPath = fmt.Sprintf("%s%s", prefix, destKey) - } else { - newPath = fmt.Sprintf("%s%s/%s", prefix, s.bucketName, destKey) - } + newPath := s.buildObsFilePath(destKey) logger.Infof(ctx, "Copied OBS object %s to %s", srcPath, newPath) return newPath, nil } @@ -316,9 +314,5 @@ func (s *obsFileService) SaveBytes(ctx context.Context, data []byte, tenantID ui return "", fmt.Errorf("failed to upload bytes to OBS: %w", err) } - prefix := s.getPrifix() - if s.proxyDomain != "" { - return fmt.Sprintf("%s%s", prefix, objectKey), nil - } - return fmt.Sprintf("%s%s/%s", prefix, s.bucketName, objectKey), nil + return s.buildObsFilePath(objectKey), nil } diff --git a/internal/application/service/file/obs_test.go b/internal/application/service/file/obs_test.go new file mode 100644 index 0000000000..0fdeb3adf6 --- /dev/null +++ b/internal/application/service/file/obs_test.go @@ -0,0 +1,127 @@ +package file + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/credentials" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/stretchr/testify/require" +) + +func newTestOBSClient(endpoint string) *s3.Client { + return s3.New(s3.Options{ + Region: "test-region", + EndpointResolver: &obsEndpointResolver{url: endpoint}, + Credentials: credentials.NewStaticCredentialsProvider("test-access-key", "test-secret-key", ""), + UsePathStyle: true, + }) +} + +func TestOBSSaveBytesWithProxyReturnsProviderPath(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + svc := &obsFileService{ + client: newTestOBSClient(server.URL), + bucketName: "documents", + pathPrefix: "weknora", + proxyDomain: "https://vnia.ctyun.cn:8081", + } + + path, err := svc.SaveBytes(context.Background(), []byte("content"), 10000, "report.pdf", false) + require.NoError(t, err) + require.Regexp(t, `^obs://documents/weknora/10000/[0-9a-f-]+\.pdf$`, path) +} + +func TestOBSPathParsingSupportsProviderAndLegacyProxyPaths(t *testing.T) { + svc := &obsFileService{ + bucketName: "documents", + proxyDomain: "https://vnia.ctyun.cn:8081", + } + + tests := []struct { + name string + path string + want string + }{ + {name: "provider path", path: "obs://documents/weknora/10000/report.pdf", want: "weknora/10000/report.pdf"}, + {name: "legacy proxy URL", path: "https://vnia.ctyun.cn:8081/weknora/10000/report.pdf", want: "weknora/10000/report.pdf"}, + {name: "bare object key", path: "weknora/10000/report.pdf", want: "weknora/10000/report.pdf"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := svc.parseObsFilePath(tt.path) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } + + _, err := svc.parseObsFilePath("obs://another-bucket/weknora/10000/report.pdf") + require.ErrorContains(t, err, "invalid OBS file path") +} + +func TestOBSGetFileURLUsesProxyForProviderPath(t *testing.T) { + svc := &obsFileService{ + bucketName: "documents", + proxyDomain: "https://vnia.ctyun.cn:8081", + } + + got, err := svc.GetFileURL(context.Background(), "obs://documents/weknora/10000/report.pdf") + require.NoError(t, err) + require.Equal(t, "https://vnia.ctyun.cn:8081/weknora/10000/report.pdf", got) + + legacy := "https://vnia.ctyun.cn:8081/weknora/10000/legacy.pdf" + got, err = svc.GetFileURL(context.Background(), legacy) + require.NoError(t, err) + require.Equal(t, legacy, got) +} + +func TestOBSPathOwnershipSupportsProviderAndLegacyProxyPaths(t *testing.T) { + svc := &obsFileService{ + bucketName: "documents", + proxyDomain: "https://vnia.ctyun.cn:8081", + } + + require.True(t, svc.ownsObsFilePath("obs://documents/weknora/report.pdf")) + require.True(t, svc.ownsObsFilePath("https://vnia.ctyun.cn:8081/weknora/report.pdf")) + require.False(t, svc.ownsObsFilePath("s3://documents/weknora/report.pdf")) + require.False(t, svc.ownsObsFilePath("https://example.com/weknora/report.pdf")) +} + +func TestOBSCopyFileReturnsProviderPathWithProxyConfigured(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-Amz-Copy-Source") == "" { + http.Error(w, "missing copy source", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/xml") + _, _ = w.Write([]byte(`"test-etag"2026-08-20T00:00:00Z`)) + })) + defer server.Close() + + svc := &obsFileService{ + client: newTestOBSClient(server.URL), + bucketName: "documents", + pathPrefix: "weknora", + proxyDomain: "https://vnia.ctyun.cn:8081", + } + + for _, source := range []string{ + "obs://documents/weknora/10000/source.pdf", + "https://vnia.ctyun.cn:8081/weknora/10000/source.pdf", + } { + copied, err := svc.CopyFile(context.Background(), source, 10000, "knowledge-1") + require.NoError(t, err) + require.True(t, strings.HasPrefix(copied, "obs://documents/weknora/10000/knowledge-1/")) + require.True(t, strings.HasSuffix(copied, ".pdf")) + } + + _, err := svc.CopyFile(context.Background(), "s3://documents/weknora/source.pdf", 10000, "knowledge-1") + require.ErrorIs(t, err, ErrCrossBackendCopy) +} diff --git a/internal/application/service/resource_test.go b/internal/application/service/resource_test.go index b870efa616..41f144d0f0 100644 --- a/internal/application/service/resource_test.go +++ b/internal/application/service/resource_test.go @@ -155,3 +155,17 @@ func TestResourceCatalogRejectsUnsupportedPhysicalPath(t *testing.T) { _, err := catalog.Register(context.Background(), 7, "https://example.com/a.png", interfaces.ResourceRegistration{}) require.ErrorContains(t, err, "unsupported provider") } + +func TestResourceCatalogAcceptsBackendScopedOBSPath(t *testing.T) { + catalog, _ := newResourceCatalogForTest(t) + physical := "storage://backend-obs/obs://documents/weknora/7/report.pdf" + + ref, err := catalog.Register(context.Background(), 7, physical, interfaces.ResourceRegistration{}) + require.NoError(t, err) + + resolved, resource, err := catalog.ResolvePath(context.Background(), ref) + require.NoError(t, err) + require.Equal(t, physical, resolved) + require.Equal(t, "backend-obs", resource.StorageBackendID) + require.Equal(t, "obs", resource.Provider) +}