Skip to content

Commit

Permalink
Move callback into sm::VFS
Browse files Browse the repository at this point in the history
+ Add unit_vfs
  • Loading branch information
shaunrd0 committed Oct 5, 2023
1 parent 2bb81a1 commit 8c6bda4
Show file tree
Hide file tree
Showing 10 changed files with 242 additions and 88 deletions.
2 changes: 1 addition & 1 deletion test/src/unit-capi-vfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ TEST_CASE_METHOD(
return 0;
}
ls_data->path_data_[ls_data->path_pos_] = (char*)malloc(path_len + 1);
strcpy(ls_data->path_data_[ls_data->path_pos_], path);
strncpy(ls_data->path_data_[ls_data->path_pos_], path, path_len + 1);
ls_data->object_sizes_[ls_data->path_pos_] = object_size;
ls_data->path_pos_++;
}
Expand Down
6 changes: 4 additions & 2 deletions test/src/unit-cppapi-vfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,10 @@ TEST_CASE_METHOD(
}
CHECKED_ELSE(expected_results_.empty()) {
SECTION("Throwing filter with N objects should throw") {
CHECK_THROWS(tiledb::VFSExperimental::ls_recursive(
ctx_, vfs_, temp_dir_.to_string(), filter));
CHECK_THROWS_AS(
tiledb::VFSExperimental::ls_recursive(
ctx_, vfs_, temp_dir_.to_string(), filter),
std::runtime_error);
}
}
}
Expand Down
17 changes: 2 additions & 15 deletions tiledb/api/c_api/vfs/vfs_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ capi_return_t tiledb_vfs_ls(
capi_return_t tiledb_vfs_ls_recursive(
tiledb_vfs_t* vfs,
const char* path,
int32_t (*callback)(const char*, size_t, uint64_t, void*),
tiledb::sm::VFS::LsCallback callback,
void* data) {
ensure_vfs_is_valid(vfs);
if (path == nullptr) {
Expand All @@ -310,20 +310,7 @@ capi_return_t tiledb_vfs_ls_recursive(
"Invalid TileDB object: Callback function is null.");
}
ensure_output_pointer_is_valid(data);
auto children = vfs->ls_recursive(tiledb::sm::URI(path));

int rc = 1;
for (const auto& entry : children) {
sm::URI uri{entry.path().native()};
rc = callback(uri.c_str(), uri.to_string().size(), entry.file_size(), data);
if (rc != 1) {
break;
}
}

if (rc == -1) {
return TILEDB_ERR;
}
auto children = vfs->ls_recursive(tiledb::sm::URI(path), callback, data);
return TILEDB_OK;
}

Expand Down
3 changes: 2 additions & 1 deletion tiledb/api/c_api/vfs/vfs_api_experimental.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "tiledb/api/c_api/api_external_common.h"
#include "tiledb/api/c_api/context/context_api_external.h"
#include "tiledb/api/c_api/vfs/vfs_api_external.h"
#include "tiledb/sm/filesystem/vfs.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -80,7 +81,7 @@ TILEDB_EXPORT capi_return_t tiledb_vfs_ls_recursive(
tiledb_ctx_t* ctx,
tiledb_vfs_t* vfs,
const char* path,
int32_t (*callback)(const char*, size_t, uint64_t, void*),
tiledb::sm::VFS::LsCallback callback,
void* data) TILEDB_NOEXCEPT;

#ifdef __cplusplus
Expand Down
6 changes: 4 additions & 2 deletions tiledb/api/c_api/vfs/vfs_api_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,10 @@ struct tiledb_vfs_handle_t
}

std::vector<filesystem::directory_entry> ls_recursive(
const tiledb::sm::URI& parent) const {
return vfs_.ls_recursive(parent);
const tiledb::sm::URI& parent,
tiledb::sm::VFS::LsCallback cb,
void* data) const {
return vfs_.ls_recursive(parent, cb, data);
}

Status touch(const tiledb::sm::URI& uri) const {
Expand Down
74 changes: 10 additions & 64 deletions tiledb/sm/cpp_api/vfs_experimental.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,36 +36,12 @@
#include <string>
#include <vector>
#include "context.h"
#include "tiledb/sm/filesystem/vfs.h"
#include "tiledb_experimental.h"
#include "vfs.h"

namespace tiledb {
class VFSExperimental {
/* ********************************* */
/* TYPE DEFINITIONS */
/* ********************************* */

/**
* Typedef for ls inclusion predicate function used to check if a single
* result should be included in the final results returned from ls or
* ls_recursive functions.
*
* @param path The path of a visited object for the relative filesystem.
* @return True if the result should be included, else false.
* @sa ls_include
*/
typedef std::function<bool(const std::string&)> LsInclude;

/**
* Struct to store recursive ls results data.
* 'object_paths_' Stores all paths collected as a vector of strings.
* `file_sizes` stores all file sizes as a vector of uint64_t.
*/
struct LsRecursiveData {
std::vector<std::string> object_paths_;
std::vector<uint64_t> object_sizes_;
};

public:
/* ********************************* */
/* PUBLIC STATIC METHODS */
Expand All @@ -77,66 +53,36 @@ class VFSExperimental {
* be included in the results. By default all entries are included using the
* ls_include predicate, which simply returns true for all results.
*
* @param ctx The TileDB context.
* @param vfs The VFS instance to use.
* @param uri The base URI to list results recursively.
* @param callback The callback predicate to invoke on each entry collected.
* @param predicate The inclusion predicate to invoke on each entry collected.
* @return Vector of pairs storing the path and object size of each result.
* @sa ls_recursive_gather
* @sa LsInclude
* @sa ls_include
*/
static std::vector<std::pair<std::string, uint64_t>> ls_recursive(
const Context& ctx,
const VFS& vfs,
const std::string& uri,
const LsInclude& callback = ls_include) {
LsRecursiveData ls_data;
const tiledb::sm::VFS::LsInclude& predicate =
tiledb::sm::VFS::ls_include) {
tiledb::sm::VFS::LsRecursiveData ls_data;
ctx.handle_error(tiledb_vfs_ls_recursive(
ctx.ptr().get(),
vfs.ptr().get(),
uri.c_str(),
ls_recursive_gather,
tiledb::sm::VFS::ls_recursive_gather,
&ls_data));
std::vector<std::pair<std::string, uint64_t>> results;
for (size_t i = 0; i < ls_data.object_paths_.size(); i++) {
if (callback(ls_data.object_paths_[i])) {
if (predicate(ls_data.object_paths_[i])) {
results.emplace_back(
ls_data.object_paths_[i], ls_data.object_sizes_[i]);
}
}
return results;
}

private:
/* ********************************* */
/* PRIVATE STATIC METHODS */
/* ********************************* */

/**
* Callback function to be used when invoking the C TileDB function
* for recursive ls. The callback will cast 'data' to LsRecursiveData struct.
* The struct stores a vector of strings for each path collected, and a uint64
* vector of file sizes for the objects at each path.
*
* @param path The path of a visited TileDB object
* @param data Cast to LsRecursiveData struct to store paths and offsets.
* @return If `1` then the walk should continue to the next object.
*/
static int ls_recursive_gather(
const char* path, size_t path_length, uint64_t file_size, void* data) {
auto ls_data = static_cast<LsRecursiveData*>(data);
ls_data->object_paths_.emplace_back(path, path_length);
ls_data->object_sizes_.push_back(file_size);
return 1;
}

/**
* Default inclusion predicate for ls functions. Optionally, a user can
* provide their own inclusion predicate to filter results.
*
* @return True if the result should be included, else false.
*/
static bool ls_include(const std::string_view&) {
return true;
}
};
} // namespace tiledb

Expand Down
7 changes: 6 additions & 1 deletion tiledb/sm/filesystem/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
#
include(unit_test)

commence(unit_test vfs)
commence(unit_test uri)
this_target_object_libraries(vfs)
this_target_sources(main.cc unit_uri.cc)
conclude(unit_test)

commence(unit_test vfs)
this_target_object_libraries(vfs)
this_target_sources(main.cc unit_vfs.cc)
conclude(unit_test)
114 changes: 114 additions & 0 deletions tiledb/sm/filesystem/test/unit_vfs.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* @file unit_vfs.cc
*
* @section LICENSE
*
* The MIT License
*
* @copyright Copyright (c) 2023 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @section DESCRIPTION
*
* Tests the `VFS` class.
*/

#include <test/support/tdb_catch.h>
#include "tiledb/sm/filesystem/vfs.h"

struct VfsFixture {
VfsFixture()
: stats_("unit_vfs")
, compute_(4)
, io_(4)
, vfs_(&stats_, &compute_, &io_, tiledb::sm::Config()) {
}

void create_objects(
const tiledb::sm::URI& dir, size_t count, const std::string& prefix) {
for (size_t i = 0; i < count; i++) {
auto uri = dir.join_path(prefix + std::to_string(i));
vfs_.touch(uri).ok();
std::string data(i * 10, 'a');
vfs_.write(uri, data.data(), data.size()).ok();
expected_results_.emplace_back(uri.to_path(), data.size(), false);
}
}

protected:
// A dummy `Stats` instance for testing internal VFS functions.
tiledb::sm::stats::Stats stats_;

ThreadPool compute_, io_;
tiledb::sm::VFS vfs_;

std::vector<filesystem::directory_entry> expected_results_;
};

TEST_CASE_METHOD(VfsFixture, "VFS: Default arguments ls_recursive", "[vfs]") {
tiledb::sm::URI temp_dir("vfs_default_args");
vfs_.create_dir(temp_dir).ok();
SECTION("Empty directory") {
auto results = vfs_.ls_recursive(temp_dir);
CHECK(results.empty());
}

SECTION("Single file") {
create_objects(temp_dir, 1, "file");
}

SECTION("Multiple files in single directory") {
create_objects(temp_dir, 10, "file");
auto results = vfs_.ls_recursive(temp_dir);
}

SECTION("Multiple files in nested directories") {
create_objects(temp_dir, 10, "file");

auto subdir = temp_dir.join_path("subdir");
vfs_.create_dir(subdir).ok();
create_objects(subdir, 10, "file");
}

auto results = vfs_.ls_recursive(temp_dir);
REQUIRE(results.size() == expected_results_.size());
for (size_t i = 0; i < expected_results_.size(); i++) {
CHECK(results[i].path().native() == expected_results_[i].path().native());
CHECK(results[i].file_size() == expected_results_[i].file_size());
}
vfs_.remove_dir(temp_dir).ok();
}

TEST_CASE_METHOD(VfsFixture, "VFS: Throwing callback ls_recursive", "[vfs]") {
auto cb = [](const char*, size_t, uint64_t, void*) -> int32_t {
throw std::logic_error("Throwing callback");
};
tiledb::sm::URI temp_dir("vfs_throwing_callback");
int data = 1;
SECTION("Throwing callback with 0 objects should not throw") {
CHECK_NOTHROW(vfs_.ls_recursive(temp_dir, cb, &data));
}
SECTION("Throwing callback with N objects should throw") {
vfs_.create_dir(temp_dir).ok();
vfs_.touch(temp_dir.join_path("file")).ok();
CHECK_THROWS_AS(vfs_.ls_recursive(temp_dir, cb, &data), std::logic_error);
vfs_.remove_dir(temp_dir).ok();
}
}
17 changes: 16 additions & 1 deletion tiledb/sm/filesystem/vfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ using tiledb::common::filesystem::directory_entry;

namespace tiledb::sm {

VFS::LsRecursiveData VFS::ls_recursive_data_;

/* ********************************* */
/* CONSTRUCTORS & DESTRUCTORS */
/* ********************************* */
Expand Down Expand Up @@ -740,7 +742,8 @@ Status VFS::ls(const URI& parent, std::vector<URI>* uris) const {
return Status::Ok();
}

std::vector<directory_entry> VFS::ls_recursive(const URI& parent) const {
std::vector<directory_entry> VFS::ls_recursive(
const URI& parent, LsCallback cb, void* data) const {
Status st;
optional<std::vector<directory_entry>> entries;
std::vector<directory_entry> results;
Expand Down Expand Up @@ -798,6 +801,18 @@ std::vector<directory_entry> VFS::ls_recursive(const URI& parent) const {
}
}

int rc = 1;
for (const auto& result : results) {
URI uri{result.path().native()};
rc = cb(uri.c_str(), uri.to_string().size(), result.file_size(), data);
if (rc != 1) {
break;
}
}
if (rc == -1) {
throw VFSException("Error in user callback");
}

return results;
}

Expand Down
Loading

0 comments on commit 8c6bda4

Please sign in to comment.