Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix minio storage iterator path #24691

Merged
merged 15 commits into from
May 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/pull-db-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ jobs:
--health-retries 10
ports:
- 6379:6379
minio:
image: bitnami/minio:2021.3.17
env:
MINIO_ACCESS_KEY: 123456
MINIO_SECRET_KEY: 12345678
ports:
- "9000:9000"
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v4
Expand Down
4 changes: 2 additions & 2 deletions modules/storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ func (l *LocalStorage) URL(path, name string) (*url.URL, error) {
}

// IterateObjects iterates across the objects in the local storage
func (l *LocalStorage) IterateObjects(prefix string, fn func(path string, obj Object) error) error {
dir := l.buildLocalPath(prefix)
func (l *LocalStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
dir := l.buildLocalPath(dirName)
return filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
Expand Down
37 changes: 1 addition & 36 deletions modules/storage/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package storage

import (
"bytes"
"context"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -57,38 +55,5 @@ func TestBuildLocalPath(t *testing.T) {

func TestLocalStorageIterator(t *testing.T) {
dir := filepath.Join(os.TempDir(), "TestLocalStorageIteratorTestDir")
l, err := NewLocalStorage(context.Background(), LocalStorageConfig{Path: dir})
assert.NoError(t, err)

testFiles := [][]string{
{"a/1.txt", "a1"},
{"/a/1.txt", "aa1"}, // same as above, but with leading slash that will be trim
{"b/1.txt", "b1"},
{"b/2.txt", "b2"},
{"b/3.txt", "b3"},
{"b/x 4.txt", "bx4"},
}
for _, f := range testFiles {
_, err = l.Save(f[0], bytes.NewBufferString(f[1]), -1)
assert.NoError(t, err)
}

expectedList := map[string][]string{
"a": {"a/1.txt"},
"b": {"b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"},
"": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"},
"/": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"},
"a/b/../../a": {"a/1.txt"},
}
for dir, expected := range expectedList {
count := 0
err = l.IterateObjects(dir, func(path string, f Object) error {
defer f.Close()
assert.Contains(t, expected, path)
count++
return nil
})
assert.NoError(t, err)
assert.Len(t, expected, count)
}
testStorageIterator(t, string(LocalStorageType), LocalStorageConfig{Path: dir})
}
15 changes: 10 additions & 5 deletions modules/storage/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
}

func (m *MinioStorage) buildMinioPath(p string) string {
return util.PathJoinRelX(m.basePath, p)
p = util.PathJoinRelX(m.basePath, p)
if p == "." {
p = "" // minio doesn't use dot as relative path
}
return p
}

// Open opens a file
Expand Down Expand Up @@ -224,14 +228,15 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
}

// IterateObjects iterates across the objects in the miniostorage
func (m *MinioStorage) IterateObjects(prefix string, fn func(path string, obj Object) error) error {
func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
opts := minio.GetObjectOptions{}
lobjectCtx, cancel := context.WithCancel(m.ctx)
defer cancel()

basePath := m.basePath
if prefix != "" {
basePath = m.buildMinioPath(prefix)
if dirName != "" {
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
basePath = m.buildMinioPath(dirName) + "/"
}

for mObjInfo := range m.client.ListObjects(lobjectCtx, m.bucket, minio.ListObjectsOptions{
Expand All @@ -244,7 +249,7 @@ func (m *MinioStorage) IterateObjects(prefix string, fn func(path string, obj Ob
}
if err := func(object *minio.Object, fn func(path string, obj Object) error) error {
defer object.Close()
return fn(strings.TrimPrefix(mObjInfo.Key, basePath), &minioObject{object})
return fn(strings.TrimPrefix(mObjInfo.Key, m.basePath), &minioObject{object})
}(object, fn); err != nil {
return convertMinioErr(err)
}
Expand Down
18 changes: 18 additions & 0 deletions modules/storage/minio_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package storage

import (
"testing"
)

func TestMinioStorageIterator(t *testing.T) {
testStorageIterator(t, string(MinioStorageType), MinioStorageConfig{
Endpoint: "127.0.0.1:9000",
AccessKeyID: "123456",
SecretAccessKey: "12345678",
Bucket: "gitea",
Location: "us-east-1",
})
}
49 changes: 49 additions & 0 deletions modules/storage/storage_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package storage

import (
"bytes"
"testing"

"github.com/stretchr/testify/assert"
)

func testStorageIterator(t *testing.T, typStr string, cfg interface{}) {
l, err := NewStorage(typStr, cfg)
assert.NoError(t, err)

testFiles := [][]string{
{"a/1.txt", "a1"},
{"/a/1.txt", "aa1"}, // same as above, but with leading slash that will be trim
{"ab/1.txt", "ab1"},
{"b/1.txt", "b1"},
{"b/2.txt", "b2"},
{"b/3.txt", "b3"},
{"b/x 4.txt", "bx4"},
}
for _, f := range testFiles {
_, err = l.Save(f[0], bytes.NewBufferString(f[1]), -1)
assert.NoError(t, err)
}

expectedList := map[string][]string{
"a": {"a/1.txt"},
"b": {"b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt"},
"": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"},
"/": {"a/1.txt", "b/1.txt", "b/2.txt", "b/3.txt", "b/x 4.txt", "ab/1.txt"},
"a/b/../../a": {"a/1.txt"},
}
for dir, expected := range expectedList {
count := 0
err = l.IterateObjects(dir, func(path string, f Object) error {
defer f.Close()
assert.Contains(t, expected, path)
count++
return nil
})
assert.NoError(t, err)
assert.Len(t, expected, count)
}
}