From 07b936b5ee1e7733cdd5c05582dda6aa5f3aaff6 Mon Sep 17 00:00:00 2001
From: Metin Cakircali <metin.cakircali@ecmwf.int>
Date: Wed, 25 Dec 2024 15:23:39 +0100
Subject: [PATCH] fix(S3): asString output and test config

---
 src/eckit/io/s3/S3BucketPath.h  |  2 +-
 src/eckit/io/s3/S3Config.cc     | 13 ++++++++-----
 src/eckit/io/s3/S3Credential.cc | 15 +++++++++------
 src/eckit/io/s3/S3ObjectPath.h  |  2 +-
 tests/io/s3/test_s3_config.h    |  2 +-
 tests/io/s3/test_s3_config.h.in |  2 +-
 6 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/eckit/io/s3/S3BucketPath.h b/src/eckit/io/s3/S3BucketPath.h
index 3d3cf9507..5c9663a4b 100644
--- a/src/eckit/io/s3/S3BucketPath.h
+++ b/src/eckit/io/s3/S3BucketPath.h
@@ -31,7 +31,7 @@ struct S3BucketPath {
 
     S3BucketPath(std::string bucket) : bucket {std::move(bucket)} { }
 
-    auto asString() const -> std::string { return '/' + bucket; }
+    auto asString() const -> std::string { return bucket; }
 
     operator std::string() const { return asString(); }
 
diff --git a/src/eckit/io/s3/S3Config.cc b/src/eckit/io/s3/S3Config.cc
index 8cb000bc2..63973ae16 100644
--- a/src/eckit/io/s3/S3Config.cc
+++ b/src/eckit/io/s3/S3Config.cc
@@ -69,16 +69,19 @@ auto S3Config::make(std::string path) -> std::vector<S3Config> {
         path = Resource<std::string>("s3ConfigFile;$ECKIT_S3_CONFIG_FILE", "~/.config/eckit/S3Config.yaml");
     }
 
-    PathName configFile(path);
+    PathName configPath(path);
 
-    if (!configFile.exists()) {
-        Log::debug<LibEcKit>() << "S3 configuration file does not exist: " << configFile << std::endl;
+    if (!configPath.exists()) {
+        Log::debug<LibEcKit>() << "S3 configuration file does not exist: " << configPath << std::endl;
         return {};
     }
 
-    LOG_DEBUG_LIB(LibEcKit) << "Reading S3 configuration from: " << configFile << std::endl;
+    if (configPath.isDir()) {
+        Log::debug<LibEcKit>() << "Path " << configPath << " is a directory. Expecting a file!" << std::endl;
+        return {};
+    }
 
-    const auto servers = YAMLConfiguration(configFile).getSubConfigurations("servers");
+    const auto servers = YAMLConfiguration(configPath).getSubConfigurations("servers");
 
     std::vector<S3Config> result;
     result.reserve(servers.size());
diff --git a/src/eckit/io/s3/S3Credential.cc b/src/eckit/io/s3/S3Credential.cc
index 4465f8265..762fe8bdc 100644
--- a/src/eckit/io/s3/S3Credential.cc
+++ b/src/eckit/io/s3/S3Credential.cc
@@ -51,18 +51,21 @@ auto S3Credential::fromFile(std::string path) -> std::vector<S3Credential> {
 
     if (path.empty()) { path = Resource<std::string>("s3CredentialsFile;$ECKIT_S3_CREDENTIALS_FILE", defaultCredFile); }
 
-    PathName credFile(path);
+    PathName credPath(path);
 
-    if (!credFile.exists()) {
-        Log::debug<LibEcKit>() << "S3 credentials file does not exist: " << credFile << std::endl;
+    if (!credPath.exists()) {
+        Log::debug<LibEcKit>() << "S3 credentials file does not exist: " << credPath << std::endl;
         return {};
     }
 
-    LOG_DEBUG_LIB(LibEcKit) << "Reading S3 credentials from: " << credFile << std::endl;
+    if (credPath.isDir()) {
+        Log::debug<LibEcKit>() << "Path " << credPath << " is a directory. Expecting a file!" << std::endl;
+        return {};
+    }
 
-    std::vector<S3Credential> result;
+    const auto creds = YAMLConfiguration(credPath).getSubConfigurations("credentials");
 
-    const auto creds = YAMLConfiguration(credFile).getSubConfigurations("credentials");
+    std::vector<S3Credential> result;
     result.reserve(creds.size());
     for (const auto& cred : creds) { result.emplace_back(fromYAML(cred)); }
 
diff --git a/src/eckit/io/s3/S3ObjectPath.h b/src/eckit/io/s3/S3ObjectPath.h
index 478d26706..304379229 100644
--- a/src/eckit/io/s3/S3ObjectPath.h
+++ b/src/eckit/io/s3/S3ObjectPath.h
@@ -31,7 +31,7 @@ struct S3ObjectPath {
 
     S3ObjectPath(std::string bucket, std::string object) : bucket {std::move(bucket)}, object {std::move(object)} { }
 
-    auto asString() const -> std::string { return '/' + bucket + '/' + object; }
+    auto asString() const -> std::string { return bucket + '/' + object; }
 
     operator std::string() const { return asString(); }
 
diff --git a/tests/io/s3/test_s3_config.h b/tests/io/s3/test_s3_config.h
index 004d1320e..1864adcbb 100644
--- a/tests/io/s3/test_s3_config.h
+++ b/tests/io/s3/test_s3_config.h
@@ -57,7 +57,7 @@ inline void cleanup(const std::vector<std::string>& bucketPaths) {
     }
 }
 
-void writePerformance(S3BucketName& bucket, const int count) {
+inline void writePerformance(S3BucketName& bucket, const int count) {
     eckit::Timer timer;
 
     Buffer buffer(1024 * 1024);
diff --git a/tests/io/s3/test_s3_config.h.in b/tests/io/s3/test_s3_config.h.in
index 3d05cdc84..aedef16a0 100644
--- a/tests/io/s3/test_s3_config.h.in
+++ b/tests/io/s3/test_s3_config.h.in
@@ -57,7 +57,7 @@ inline void cleanup(const std::vector<std::string>& bucketPaths) {
     }
 }
 
-void writePerformance(S3BucketName& bucket, const int count) {
+inline void writePerformance(S3BucketName& bucket, const int count) {
     eckit::Timer timer;
 
     Buffer buffer(1024 * 1024);