Skip to content

Commit

Permalink
feat(core)!: make list return path itself (#4959)
Browse files Browse the repository at this point in the history
* rebase main

* if path not exists, don't return it

* support gcs && oss

* support azblob

* supoort dashmap, moka, mini-moka

* support sqlite && etcd

* support rocksdb && sled && swift

* support hdfs && webhdfs

* support webdav

* support sftp

* support seafile

* fix swift

* fix java binding

* fix remove_all for blocking_operator

* fix cpp binding

* fix ocaml binding

* fix golang binding

* fix integrations/dav-server

* fix integrations/fuse3

* cargo fmt

* restore the logic of right_range in serivices-memory
  • Loading branch information
meteorgan authored Aug 26, 2024
1 parent 74b4b0a commit 74a3e29
Show file tree
Hide file tree
Showing 48 changed files with 317 additions and 225 deletions.
1 change: 0 additions & 1 deletion .github/services/swift/ceph_rados_swift/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ runs:
shell: bash
run: |
cat << EOF >> $GITHUB_ENV
OPENDAL_DISABLE_RANDOM_ROOT=true
OPENDAL_SWIFT_CONTAINER=testing
OPENDAL_SWIFT_ROOT=/
EOF
1 change: 0 additions & 1 deletion .github/services/swift/swift/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ runs:
shell: bash
run: |
cat << EOF >> $GITHUB_ENV
OPENDAL_DISABLE_RANDOM_ROOT=true
OPENDAL_SWIFT_CONTAINER=testing
OPENDAL_SWIFT_ROOT=/
EOF
35 changes: 20 additions & 15 deletions bindings/cpp/tests/basic_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ TEST_F(OpendalTest, BasicTest) {
auto list_file_path = dir_path + file_path;
op.write(list_file_path, data);
auto entries = op.list(dir_path);
EXPECT_EQ(entries.size(), 1);
EXPECT_EQ(entries[0].path, list_file_path);
EXPECT_EQ(entries.size(), 2);
std::set<std::string> paths;
for (const auto &entry : entries) {
paths.insert(entry.path);
}
EXPECT_TRUE(paths.find(dir_path) != paths.end());
EXPECT_TRUE(paths.find(list_file_path) != paths.end());

// remove
op.remove(file_path_renamed);
Expand Down Expand Up @@ -133,23 +138,23 @@ TEST_F(OpendalTest, ReaderTest) {
}

TEST_F(OpendalTest, ListerTest) {
op.create_dir("test_dir/");
op.write("test_dir/test1", {1, 2, 3});
op.write("test_dir/test2", {4, 5, 6});
std::string dir_path = "test_dir/";
op.create_dir(dir_path);
auto test1_path = dir_path + "test1";
op.write(test1_path, {1, 2, 3});
auto test2_path = dir_path + "test2";
op.write(test2_path, {4, 5, 6});

int size = 0;
auto lister = op.lister("test_dir/");

std::set<std::string> paths;
for (const auto &entry : lister) {
EXPECT_TRUE(entry.path.find("test_dir/test") == 0);
size += 1;
paths.insert(entry.path);
}
EXPECT_EQ(size, 2);

lister = op.lister("test_dir/");
std::vector<opendal::Entry> paths(lister.begin(), lister.end());
EXPECT_EQ(paths.size(), 2);
EXPECT_EQ(paths[0].path, "test_dir/test1");
EXPECT_EQ(paths[1].path, "test_dir/test2");
EXPECT_EQ(paths.size(), 3);
EXPECT_TRUE(paths.find(dir_path) != paths.end());
EXPECT_TRUE(paths.find(test1_path) != paths.end());
EXPECT_TRUE(paths.find(test2_path) != paths.end());
}

int main(int argc, char **argv) {
Expand Down
14 changes: 11 additions & 3 deletions bindings/go/tests/behavior_tests/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func testListRichDir(assert *require.Assertions, op *opendal.Operator, fixture *
}
assert.Nil(obs.Error())

expected = append(expected, parent)
slices.Sort(expected)
slices.Sort(actual)

Expand All @@ -133,10 +134,12 @@ func testListEmptyDir(assert *require.Assertions, op *opendal.Operator, fixture
for obs.Next() {
entry := obs.Entry()
paths = append(paths, entry.Path())
assert.Equal(dir, entry.Path())
}
assert.Nil(obs.Error())
assert.Equal(0, len(paths), "dir should only return empty")
assert.Equal(1, len(paths), "dir should only return itself")

paths = nil
obs, err = op.List(strings.TrimSuffix(dir, "/"))
assert.Nil(err)
defer obs.Close()
Expand Down Expand Up @@ -211,26 +214,31 @@ func testListNestedDir(assert *require.Assertions, op *opendal.Operator, fixture
defer obs.Close()
paths = nil
var foundFile bool
var foundDirPath bool
var foundDir bool
for obs.Next() {
entry := obs.Entry()
paths = append(paths, entry.Path())
if entry.Path() == filePath {
foundFile = true
} else if entry.Path() == dirPath {
foundDirPath = true
} else if entry.Path() == dir {
foundDir = true
}
}
assert.Nil(obs.Error())
assert.Equal(2, len(paths), "parent should only got 2 entries")
assert.Equal(3, len(paths), "dir should only got 3 entries")

assert.True(foundDir, "dir should be found in list")

assert.True(foundFile, "file should be found in list")
meta, err := op.Stat(filePath)
assert.Nil(err)
assert.True(meta.IsFile())
assert.Equal(uint64(20), meta.ContentLength())

assert.True(foundDir, "dir should be found in list")
assert.True(foundDirPath, "dir path should be found in list")
meta, err = op.Stat(dirPath)
assert.Nil(err)
assert.True(meta.IsDir())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.opendal.Metadata;
import org.apache.opendal.OpenDALException;
import org.apache.opendal.test.condition.OpenDALExceptionCondition;
import org.assertj.core.util.Lists;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
Expand Down Expand Up @@ -77,24 +78,25 @@ public void testListDir() {
*/
@Test
public void testListRichDir() {
final String parent = "test_list_rich_dir";
asyncOp().createDir(parent + "/").join();
final String parentPath = "test_list_rich_dir/";
asyncOp().createDir(parentPath).join();
final List<String> expected = new ArrayList<>();
for (int i = 0; i < 10; i++) {
expected.add(String.format("%s/file-%d", parent, i));
expected.add(String.format("%sfile-%d", parentPath, i));
}

for (String path : expected) {
asyncOp().write(path, parent).join();
asyncOp().write(path, parentPath).join();
}

final List<Entry> entries = asyncOp().list(parent + "/").join();
final List<Entry> entries = asyncOp().list(parentPath).join();
final List<String> actual =
entries.stream().map(Entry::getPath).sorted().collect(Collectors.toList());

expected.add(parentPath);
Collections.sort(expected);
assertThat(actual).isEqualTo(expected);
asyncOp().removeAll(parent + "/").join();
asyncOp().removeAll(parentPath).join();
}

/**
Expand All @@ -106,7 +108,9 @@ public void testListEmptyDir() {
asyncOp().createDir(dir).join();

final List<Entry> entries = asyncOp().list(dir).join();
assertThat(entries).isEmpty();
final List<String> actual = entries.stream().map(Entry::getPath).collect(Collectors.toList());
assertThat(actual).hasSize(1);
assertThat(actual.get(0)).isEqualTo(dir);

asyncOp().delete(dir).join();
}
Expand Down Expand Up @@ -151,28 +155,37 @@ public void testListSubDir() {
public void testListNestedDir() {
final String dir = String.format("%s/%s/", UUID.randomUUID(), UUID.randomUUID());
final String fileName = UUID.randomUUID().toString();
final String filePath = String.format("%s/%s", dir, fileName);
final String filePath = String.format("%s%s", dir, fileName);
final String dirName = String.format("%s/", UUID.randomUUID());
final String dirPath = String.format("%s/%s", dir, dirName);
final String dirPath = String.format("%s%s", dir, dirName);
final String content = "test_list_nested_dir";

asyncOp().createDir(dir).join();
asyncOp().write(filePath, content).join();
asyncOp().createDir(dirPath).join();

final List<Entry> entries = asyncOp().list(dir).join();
assertThat(entries).hasSize(2);
assertThat(entries).hasSize(3);

final List<String> expectedPaths = Lists.newArrayList(dir, dirPath, filePath);
Collections.sort(expectedPaths);
final List<String> actualPaths =
entries.stream().map(Entry::getPath).sorted().collect(Collectors.toList());
assertThat(actualPaths).isEqualTo(expectedPaths);

for (Entry entry : entries) {
// check file
if (entry.getPath().equals(filePath)) {
Metadata metadata = entry.getMetadata();
Metadata metadata = asyncOp().stat(filePath).join();
assertTrue(metadata.isFile());
assertThat(metadata.getContentLength()).isEqualTo(content.length());
// check dir
} else if (entry.getPath().equals(dirPath)) {
Metadata metadata = entry.getMetadata();
assertTrue(metadata.isDir());
} else {
assertThat(entry.getPath()).isEqualTo(dir);
assertTrue(entry.metadata.isDir());
}
}

Expand Down
4 changes: 2 additions & 2 deletions bindings/ocaml/test/test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ let test_list test_ctxt =
(Operator.write bo "/testdir/bar" (Bytes.of_string "foo")));
let array = Operator.list bo "testdir/" |> test_check_result in
let actual = Array.map Operator.Entry.name array in
let expected = [| "foo"; "bar" |] in
let expected = [| "testdir/"; "foo"; "bar" |] in
List.iter (Array.sort compare) [ expected; actual ];
assert_equal expected actual;
assert_equal 2 (Array.length array);
assert_equal 3 (Array.length array);
()

let suite =
Expand Down
12 changes: 4 additions & 8 deletions core/src/layers/complete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,7 @@ impl<A: Access> CompleteAccessor<A> {
if path.ends_with('/') && capability.list_with_recursive {
let (_, mut l) = self
.inner
.list(
path.trim_end_matches('/'),
OpList::default().with_recursive(true).with_limit(1),
)
.list(path, OpList::default().with_recursive(true).with_limit(1))
.await?;

return if oio::List::next(&mut l).await?.is_some() {
Expand Down Expand Up @@ -246,10 +243,9 @@ impl<A: Access> CompleteAccessor<A> {

// Otherwise, we can simulate stat a dir path via `list`.
if path.ends_with('/') && capability.list_with_recursive {
let (_, mut l) = self.inner.blocking_list(
path.trim_end_matches('/'),
OpList::default().with_recursive(true).with_limit(1),
)?;
let (_, mut l) = self
.inner
.blocking_list(path, OpList::default().with_recursive(true).with_limit(1))?;

return if oio::BlockingList::next(&mut l)?.is_some() {
Ok(RpStat::new(Metadata::new(EntryMode::DIR)))
Expand Down
7 changes: 5 additions & 2 deletions core/src/raw/adapters/kv/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,11 @@ impl KvLister {
} else {
EntryMode::FILE
};

oio::Entry::new(&build_rel_path(&self.root, &v), Metadata::new(mode))
let mut path = build_rel_path(&self.root, &v);
if path.is_empty() {
path = "/".to_string();
}
oio::Entry::new(&path, Metadata::new(mode))
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/raw/adapters/typed_kv/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::Scheme;
///
/// `typed_kv::Adapter` is the typed version of `kv::Adapter`. It's more
/// efficient if the underlying kv service can store data with its type. For
/// example, we can store `Bytes` along with it's metadata so that we don't
/// example, we can store `Bytes` along with its metadata so that we don't
/// need to serialize/deserialize it when we get it from the service.
///
/// Ideally, we should use `typed_kv::Adapter` instead of `kv::Adapter` for
Expand Down
12 changes: 7 additions & 5 deletions core/src/raw/adapters/typed_kv/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
// specific language governing permissions and limitations
// under the License.

use std::sync::Arc;
use std::vec::IntoIter;

use super::Adapter;
use super::Value;
use crate::raw::oio::HierarchyLister;
use crate::raw::oio::QueueBuf;
use crate::raw::*;
use crate::*;
use std::sync::Arc;
use std::vec::IntoIter;

/// The typed kv backend which implements Accessor for typed kv adapter.
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -211,8 +210,11 @@ impl KvLister {
} else {
EntryMode::FILE
};

oio::Entry::new(&build_rel_path(&self.root, &v), Metadata::new(mode))
let mut path = build_rel_path(&self.root, &v);
if path.is_empty() {
path = "/".to_string();
}
oio::Entry::new(&path, Metadata::new(mode))
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/raw/http_util/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl HttpClient {
// Get content length from header so that we can check it.
//
// - If the request method is HEAD, we will ignore content length.
// - If response contains content_encoding, we should omit it's content length.
// - If response contains content_encoding, we should omit its content length.
let content_length = if is_head || parse_content_encoding(resp.headers())?.is_some() {
None
} else {
Expand Down
Loading

0 comments on commit 74a3e29

Please sign in to comment.