fix(cpp): Handle npos in PathToDirectory for S3 URIs without query strings

cpp/CMakeLists.txt

@@ -419,4 +419,4 @@ add_custom_target(graphar-clformat
 add_custom_target(graphar-cpplint
         COMMAND ${PROJECT_SOURCE_DIR}/misc/cpplint.py --root=${PROJECT_SOURCE_DIR}/include ${FILES_NEED_LINT}
         COMMENT "Running cpplint check."
-        VERBATIM)
+        VERBATIM)

cpp/src/graphar/graph_info.cc

@@ -28,6 +28,7 @@
 #include "graphar/graph_info.h"
 #include "graphar/result.h"
 #include "graphar/types.h"
+#include "graphar/util.h"
 #include "graphar/version_parser.h"
 #include "graphar/yaml.h"
 
@@ -1100,24 +1101,6 @@ Status EdgeInfo::Save(const std::string& path) const {
 
 namespace {
 
-static std::string PathToDirectory(const std::string& path) {
-  if (path.rfind("s3://", 0) == 0) {
-    size_t t = path.find_last_of('?');
-    std::string prefix = path.substr(0, t);
-    std::string suffix = path.substr(t);
-    const size_t last_slash_idx = prefix.rfind('/');
-    if (std::string::npos != last_slash_idx) {
-      return prefix.substr(0, last_slash_idx + 1) + suffix;
-    }
-  } else {
-    const size_t last_slash_idx = path.rfind('/');
-    if (std::string::npos != last_slash_idx) {
-      return path.substr(0, last_slash_idx + 1);  // +1 to include the slash
-    }
-  }
-  return path;
-}
-
 static Result<std::shared_ptr<GraphInfo>> ConstructGraphInfo(
     std::shared_ptr<Yaml> graph_meta, const std::string& default_name,
     const std::string& default_prefix, const std::shared_ptr<FileSystem> fs,
@@ -1409,8 +1392,8 @@ Result<std::shared_ptr<GraphInfo>> GraphInfo::Load(const std::string& path) {
                       fs->ReadFileToValue<std::string>(no_url_path));
   GAR_ASSIGN_OR_RAISE(auto graph_meta, Yaml::Load(yaml_content));
   std::string default_name = "graph";
-  std::string default_prefix = PathToDirectory(path);
-  no_url_path = PathToDirectory(no_url_path);
+  std::string default_prefix = util::PathToDirectory(path);
+  no_url_path = util::PathToDirectory(no_url_path);
   return ConstructGraphInfo(graph_meta, default_name, default_prefix, fs,
                             no_url_path);
 }
@@ -1485,4 +1468,25 @@ Status GraphInfo::Save(const std::string& path) const {
   return fs->WriteValueToFile(yaml_content, no_url_path);
 }
 
+namespace util {
+std::string PathToDirectory(const std::string& path) {
+  if (path.rfind("s3://", 0) == 0) {
+    size_t t = path.find_last_of('?');
+    // Your fix here
+    std::string prefix = (t == std::string::npos) ? path : path.substr(0, t);
+    std::string suffix = (t == std::string::npos) ? "" : path.substr(t);
+
+    const size_t last_slash_idx = prefix.rfind('/');
+    if (std::string::npos != last_slash_idx) {
+      return prefix.substr(0, last_slash_idx + 1) + suffix;
+    }
+  } else {
+    const size_t last_slash_idx = path.rfind('/');
+    if (std::string::npos != last_slash_idx) {
+      return path.substr(0, last_slash_idx + 1);
+    }
+  }
+  return path;
+}
+}  // namespace util
 }  // namespace graphar

cpp/src/graphar/util.h

@@ -273,4 +273,6 @@ static arrow::Status OpenParquetArrowReader(
   return arrow::Status::OK();
 }
 
+std::string PathToDirectory(const std::string& path);
+
 }  // namespace graphar::util

cpp/test/CMakeLists.txt

@@ -81,3 +81,4 @@ add_graphar_test(test_arrow_chunk_reader SRCS test_arrow_chunk_reader.cc)
 add_graphar_test(test_graph SRCS test_graph.cc)
 add_graphar_test(test_multi_label SRCS test_multi_label.cc)
 add_graphar_test(test_multi_property SRCS test_multi_property.cc)
+add_graphar_test(test_util SRCS test_util.cc)

cpp/test/test_util.cc

@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <iostream>
+#include <string>
+
+#include <catch2/catch_test_macros.hpp>
+#include "./util.h"
+#include "graphar/util.h"
+
+namespace graphar {
+
+TEST_CASE_METHOD(GlobalFixture, "PathUtilTest") {
+  SECTION("PathToDirectory_S3_Handling") {
+    // 1. Test standard S3 URI with query (existing functionality)
+    std::string s3_with_query = "s3://bucket/path/graph.yml?version=123";
+    REQUIRE(util::PathToDirectory(s3_with_query) ==
+            "s3://bucket/path/?version=123");
+
+    // 2. Test S3 URI WITHOUT query (The Bug Fix!)
+    std::string s3_no_query = "s3://bucket/path/graph.yml";
+    REQUIRE(util::PathToDirectory(s3_no_query) == "s3://bucket/path/");
+
+    // 3. Test S3 URI at root
+    std::string s3_root = "s3://bucket/graph.yml";
+    REQUIRE(util::PathToDirectory(s3_root) == "s3://bucket/");
+  }
+
+  SECTION("PathToDirectory_Local_Handling") {
+    // 4. Test standard local path
+    std::string local_path = "/tmp/data/graph.yml";
+    REQUIRE(util::PathToDirectory(local_path) == "/tmp/data/");
+
+    // 5. Test relative path
+    std::string relative_path = "graph.yml";
+    REQUIRE(util::PathToDirectory(relative_path) == "graph.yml");
+  }
+}
+
+}  // namespace graphar