Refactor s3 test

+ Fix azure failures

test/src/unit-vfs.cc

@@ -367,7 +367,7 @@ TEMPLATE_LIST_TEST_CASE(
 
   tiledb::sm::LsCallback cb =
       [](const char* path, size_t path_len, uint64_t size, void* data) {
-        auto ls_objects = static_cast<S3Test::LsObjects*>(data);
+        auto ls_objects = static_cast<VFSTest::LsObjects*>(data);
         ls_objects->emplace_back(std::string(path, path_len), size);
         return 1;
       };
@@ -392,7 +392,7 @@ TEST_CASE("VFS: ls_recursive callback stops traversal", "[vfs][ls_recursive]") {
   tiledb::sm::LsCallback cb =
       [&cb_count](
           const char* path, size_t path_len, uint64_t size, void* data) {
-        auto ls_objects = static_cast<S3Test::LsObjects*>(data);
+        auto ls_objects = static_cast<VFSTest::LsObjects*>(data);
         ls_objects->emplace_back(std::string(path, path_len), size);
         if (ls_objects->size() == cb_count) {
           return 0;

test/support/src/vfs_helpers.cc

@@ -65,6 +65,14 @@ tiledb::sm::Config create_test_config() {
     REQUIRE_NOTHROW(cfg.set("vfs.s3.use_virtual_addressing", "false"));
     REQUIRE_NOTHROW(cfg.set("vfs.s3.verify_ssl", "false"));
   }
+  REQUIRE_NOTHROW(
+      cfg.set("vfs.azure.storage_account_name", "devstoreaccount1"));
+  REQUIRE_NOTHROW(cfg.set(
+      "vfs.azure.storage_account_key",
+      "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/"
+      "K1SZFPTOtr/KBHBeksoGMGw=="));
+  REQUIRE_NOTHROW(cfg.set(
+      "vfs.azure.blob_endpoint", "http://127.0.0.1:10000/devstoreaccount1"));
   return cfg;
 }
 
@@ -507,40 +515,47 @@ void VFSTest::test_ls_recursive(
   CHECK(ls_objects == expected_results_);
 }
 
-#ifdef HAVE_S3
 S3Test::S3Test(const std::vector<size_t>& test_tree)
-    : VFSTest(test_tree, "s3://")
-    , s3_(&tiledb::test::g_helper_stats, &io_, vfs_.config()) {
+    : VFSTest(test_tree, "s3://") {
 }
 
 void S3Test::create_objects(
-    const sm::URI& uri, size_t count, const std::string& prefix) {
+    [[maybe_unused]] const sm::URI& uri,
+    [[maybe_unused]] size_t count,
+    [[maybe_unused]] const std::string& prefix) {
+#ifdef HAVE_S3
   for (size_t i = 1; i <= count; i++) {
     auto object_uri = uri.join_path(prefix + std::to_string(i));
-    s3_.touch(object_uri).ok();
+    vfs_.s3()->touch(object_uri).ok();
     std::string data(i * 10, 'a');
-    s3_.write(object_uri, data.data(), data.size()).ok();
-    s3_.flush_object(object_uri).ok();
+    vfs_.s3()->write(object_uri, data.data(), data.size()).ok();
+    vfs_.s3()->flush_object(object_uri).ok();
     expected_results_.emplace_back(object_uri.to_string(), data.size());
   }
+#endif
 }
 
 void S3Test::setup_test() {
+#ifdef HAVE_S3
   for (size_t i = 1; i <= test_tree_.size(); i++) {
     sm::URI path = temp_dir_.join_path("subdir_" + std::to_string(i));
     // VFS::create_dir is a no-op for S3; Just create objects.
     create_objects(path, test_tree_[i - 1], "test_file_");
   }
+#endif
 }
 
-void S3Test::test_ls_cb(tiledb::sm::LsCallback cb, bool recursive) {
-  S3Test::LsObjects ls_objects;
+void S3Test::test_ls_cb(
+    [[maybe_unused]] tiledb::sm::LsCallback cb,
+    [[maybe_unused]] bool recursive) {
+#ifdef HAVE_S3
+  LsObjects ls_objects;
   // If testing with recursion use the root directory, otherwise use a subdir.
   auto path = recursive ? temp_dir_ : temp_dir_.join_path("subdir_1");
   if (recursive) {
-    CHECK_NOTHROW(s3_.ls_cb(path, cb, &ls_objects, ""));
+    CHECK_NOTHROW(vfs_.s3()->ls_cb(path, cb, &ls_objects, ""));
   } else {
-    CHECK_NOTHROW(s3_.ls_cb(path, cb, &ls_objects));
+    CHECK_NOTHROW(vfs_.s3()->ls_cb(path, cb, &ls_objects));
   }
 
   if (!recursive) {
@@ -551,21 +566,8 @@ void S3Test::test_ls_cb(tiledb::sm::LsCallback cb, bool recursive) {
   std::sort(expected_results_.begin(), expected_results_.end());
   CHECK(ls_objects.size() == expected_results_.size());
   CHECK(ls_objects == expected_results_);
-}
-#else
-S3Test::S3Test(const std::vector<size_t>& test_tree)
-    : VFSTest(test_tree, "s3://") {
-}
-
-void S3Test::create_objects(const sm::URI&, size_t, const std::string&) {
-}
-
-void S3Test::setup_test() {
-}
-
-void S3Test::test_ls_cb(tiledb::sm::LsCallback, bool) {
-}
 #endif
+}
 
 }  // End of namespace test
 }  // End of namespace tiledb

test/support/src/vfs_helpers.h

@@ -710,7 +710,7 @@ class VFSTest {
  public:
   using LsObjects = std::vector<std::pair<std::string, uint64_t>>;
 
-  VFSTest(
+  explicit VFSTest(
       const std::vector<size_t>& test_tree,
       const std::string& prefix = "file://");
 
@@ -737,7 +737,7 @@ class VFSTest {
 
 class S3Test : public VFSTest {
  public:
-  S3Test(const std::vector<size_t>& test_tree);
+  explicit S3Test(const std::vector<size_t>& test_tree);
 
   ~S3Test() = default;
 
@@ -747,10 +747,6 @@ class S3Test : public VFSTest {
   void setup_test() override;
 
   void test_ls_cb(tiledb::sm::LsCallback cb, bool recursive);
-
-#ifdef HAVE_S3
-  tiledb::sm::S3 s3_;
-#endif
 };
 
 }  // End of namespace test

tiledb/sm/filesystem/ls_callback.h

@@ -0,0 +1,53 @@
+/**
+ * @file ls_callback.h
+ *
+ * @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
+ *
+ * This defines the type definition used for ls callback functions in VFS.
+ */
+
+#ifndef TILEDB_LS_CALLBACK_H
+#define TILEDB_LS_CALLBACK_H
+
+#include <cstdint>
+#include <functional>
+
+namespace tiledb::sm {
+/**
+ * Typedef for the callback function invoked on each object collected by ls.
+ *
+ * @param path The path of a visited object for the relative filesystem.
+ * @param path_len The length of the path string.
+ * @param object_size The size of the object at the path.
+ * @param data Cast to user defined struct to store paths and offsets.
+ * @return `1` if the walk should continue to the next object, `0` if the walk
+ *    should stop, and `-1` on error.
+ */
+using LsCallback = std::function<int32_t(const char*, size_t, uint64_t, void*)>;
+}  // namespace tiledb::sm
+
+#endif  // TILEDB_LS_CALLBACK_H

tiledb/sm/filesystem/vfs.cc

@@ -807,13 +807,14 @@ tuple<Status, optional<std::vector<directory_entry>>> VFS::ls_with_sizes(
   return {Status::Ok(), entries};
 }
 
-void VFS::ls_recursive(const URI& parent, LsCallback cb, void* data) const {
+void VFS::ls_recursive(
+    const URI& parent,
+    [[maybe_unused]] LsCallback cb,
+    [[maybe_unused]] void* data) const {
   if (parent.is_s3()) {
 #ifdef HAVE_S3
     s3_.ls_cb(parent, cb, data, "");
 #else
-    (void)cb;
-    (void)data;
     throw VFSException("TileDB was built without S3 support");
 #endif
   } else {

tiledb/sm/filesystem/vfs.h

@@ -170,11 +170,17 @@ struct VFSBase {
 /** The S3 filesystem. */
 #ifdef HAVE_S3
 struct S3_within_VFS {
-  S3 s3_;
   template <typename... Args>
   S3_within_VFS(Args&&... args)
       : s3_(std::forward<Args>(args)...) {
   }
+
+  inline S3* s3() {
+    return &s3_;
+  }
+
+ protected:
+  S3 s3_;
 };
 #else
 struct S3_within_VFS {
@@ -188,7 +194,7 @@ struct S3_within_VFS {
  * This class implements a virtual filesystem that directs filesystem-related
  * function execution to the appropriate backend based on the input URI.
  */
-class VFS : private VFSBase, S3_within_VFS {
+class VFS : private VFSBase, public S3_within_VFS {
  public:
   /* ********************************* */
   /*          TYPE DEFINITIONS         */