Add cycle detection mechanism to Group::dump() (#5693)

This PR adds a cycle-detection mechanism to prevent `Group::dump()` from
hanging when there is a group cycle, avoiding infinite looping.

Closes CORE-429

---
TYPE: IMPROVEMENT
DESC: Add cycle detection to `Group::dump()`.

Author: kounelisagis

test/src/unit-cppapi-group.cc

@@ -1165,3 +1165,47 @@ TEST_CASE(
     CHECK(group.is_relative(name));
   }
 }
+
+TEST_CASE_METHOD(
+    GroupCPPFx,
+    "C++ API: Group with cycle, dump and is_relative",
+    "[cppapi][group][cycle][non-rest]") {
+  // Create groups that form a cycle
+  std::string group_a_uri = vfs_test_setup_.array_uri("group_a");
+  std::string group_b_uri = vfs_test_setup_.array_uri("group_b");
+  tiledb::Group::create(ctx_, group_a_uri);
+  tiledb::Group::create(ctx_, group_b_uri);
+
+  // Open group A and add group B as a member
+  {
+    tiledb::Group group_a(ctx_, group_a_uri, TILEDB_WRITE);
+    group_a.add_member(group_b_uri, false, "group_b");
+    group_a.close();
+  }
+
+  // Open group B and add group A as a member, creating a cycle
+  {
+    tiledb::Group group_b(ctx_, group_b_uri, TILEDB_WRITE);
+    group_b.add_member(group_a_uri, false, "group_a");
+    group_b.close();
+  }
+
+  // Open group A for reading and test cycle handling
+  tiledb::Group group_a(ctx_, group_a_uri, TILEDB_READ);
+
+  // Test is_relative - this should not hang even with cycles
+  CHECK_NOTHROW(group_a.is_relative("group_b"));
+
+  // Test dump with recursive=false - this should not hang
+  std::string dump_non_recursive;
+  CHECK_NOTHROW(dump_non_recursive = group_a.dump(false));
+  CHECK(dump_non_recursive.find("group_b") != std::string::npos);
+
+  // Test dump with recursive=true - this should not hang even with cycles
+  std::string dump_recursive;
+  CHECK_NOTHROW(dump_recursive = group_a.dump(true));
+  CHECK(dump_recursive.find("group_b") != std::string::npos);
+  CHECK(dump_recursive.find("group_a") != std::string::npos);
+
+  group_a.close();
+}

tiledb/sm/group/group.cc

@@ -765,11 +765,30 @@ std::string Group::dump(
     const uint64_t num_indents,
     bool recursive,
     bool print_self) const {
+  // Create a set to track visited groups and prevent cycles
+  std::unordered_set<std::reference_wrapper<URI>, URIRefHash, URIRefEqual>
+      visited;
+  visited.insert(std::ref(const_cast<URI&>(group_uri_)));
+
+  // Create a string stream to hold the dump output
+  std::stringstream ss;
+
+  dump(indent_size, num_indents, recursive, print_self, visited, ss);
+  return ss.str();
+}
+
+void Group::dump(
+    const uint64_t indent_size,
+    const uint64_t num_indents,
+    bool recursive,
+    bool print_self,
+    std::unordered_set<std::reference_wrapper<URI>, URIRefHash, URIRefEqual>&
+        visited,
+    std::stringstream& ss) const {
   // Build the indentation literal and the leading indentation literal.
   const std::string indent(indent_size, '-');
   const std::string l_indent(indent_size * num_indents, '-');
 
-  std::stringstream ss;
   if (print_self) {
     ss << l_indent << group_uri_.last_path_part() << " "
        << object_type_str(ObjectType::GROUP) << std::endl;
@@ -784,11 +803,23 @@ std::string Group::dump(
         member_uri = group_uri_.join_path(it->uri().to_string());
       }
 
+      // Check if we've already visited this group to avoid cycles
+      if (visited.find(std::ref(member_uri)) != visited.end()) {
+        ss << std::endl;
+        continue;
+      }
+
       Group group_rec(resources_, member_uri);
       try {
         group_rec.open(QueryType::READ);
         ss << std::endl;
-        ss << group_rec.dump(indent_size, num_indents + 2, recursive, false);
+        // Mark this group as visited before recursing
+        visited.insert(std::ref(member_uri));
+        group_rec.dump(
+            indent_size, num_indents + 2, recursive, false, visited, ss);
+        // Remove from visited set after recursion to allow the same group
+        // to appear in different branches (but not in the same path)
+        visited.erase(std::ref(member_uri));
         group_rec.close();
       } catch (GroupNotFoundException&) {
         // If the group no longer exists in storage it will be listed but we
@@ -799,8 +830,6 @@ std::string Group::dump(
       ss << std::endl;
     }
   }
-
-  return ss.str();
 }
 
 /* ********************************* */

tiledb/sm/group/group.h

@@ -34,6 +34,9 @@
 #define TILEDB_GROUP_H
 
 #include <atomic>
+#include <functional>
+#include <sstream>
+#include <unordered_set>
 
 #include "tiledb/sm/config/config.h"
 #include "tiledb/sm/crypto/encryption_key.h"
@@ -48,6 +51,7 @@ using namespace tiledb::common;
 namespace tiledb::sm {
 
 class ContextResources;
+class URI;
 
 class GroupDetailsException : public StatusException {
  public:
@@ -523,6 +527,38 @@ class Group {
 
   /** Opens an group for writes. */
   void group_open_for_writes();
+
+  // Hash and equality for std::reference_wrapper<URI>
+  struct URIRefHash {
+    size_t operator()(std::reference_wrapper<URI> ref) const {
+      return std::hash<URI>{}(ref.get());
+    }
+  };
+  struct URIRefEqual {
+    bool operator()(
+        std::reference_wrapper<URI> a, std::reference_wrapper<URI> b) const {
+      return a.get() == b.get();
+    }
+  };
+
+  /**
+   * Helper for dump with cycle detection
+   *
+   * @param indent_size
+   * @param num_indents
+   * @param recursive
+   * @param print_self
+   * @param visited Set of visited group URI references to detect cycles
+   * @param ss Stringstream to append output to
+   */
+  void dump(
+      const uint64_t indent_size,
+      const uint64_t num_indents,
+      bool recursive,
+      bool print_self,
+      std::unordered_set<std::reference_wrapper<URI>, URIRefHash, URIRefEqual>&
+          visited,
+      std::stringstream& ss) const;
 };
 }  // namespace tiledb::sm