fix(S#): uri bug

ecmwf · Dec 18, 2024 · 30298b8 · 30298b8
1 parent 3647ce8
commit 30298b8
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 39 deletions.
diff --git a/src/eckit/io/s3/S3BucketName.cc b/src/eckit/io/s3/S3BucketName.cc
@@ -35,14 +35,22 @@ namespace eckit {
 //----------------------------------------------------------------------------------------------------------------------
 
 S3BucketName::S3BucketName(const URI& uri) : S3Name(uri) {
-    parse();
+    const auto pairs = parse(uri.name());
+    if (pairs.empty()) { throw S3SeriousBug("Could not parse bucket name!", Here()); }
+    bucket_ = pairs[0];
 }
 
 S3BucketName::S3BucketName(const net::Endpoint& endpoint, std::string bucket)
-    : S3Name(endpoint, "/"), bucket_ {std::move(bucket)} { }
+    : S3Name(endpoint), bucket_ {std::move(bucket)} { }
 
 //----------------------------------------------------------------------------------------------------------------------
 
+auto S3BucketName::uri() const -> URI {
+    auto uri = S3Name::uri();
+    uri.path("/" + bucket_);
+    return uri;
+}
+
 auto S3BucketName::asString() const -> std::string {
     return S3Name::asString() + "/" + bucket_;
 }
@@ -52,12 +60,6 @@ void S3BucketName::print(std::ostream& out) const {
     S3Name::print(out);
 }
 
-void S3BucketName::parse() {
-    const auto pairs = parseName();
-    if (pairs.empty()) { throw S3SeriousBug("Could not parse bucket name!", Here()); }
-    bucket_ = pairs[0];
-}
-
 //----------------------------------------------------------------------------------------------------------------------
 
 auto S3BucketName::makeObject(const std::string& object) const -> std::unique_ptr<S3ObjectName> {

diff --git a/src/eckit/io/s3/S3BucketName.h b/src/eckit/io/s3/S3BucketName.h
@@ -42,6 +42,8 @@ class S3BucketName : public S3Name {
 
     auto makeObject(const std::string& object) const -> std::unique_ptr<S3ObjectName>;
 
+    auto uri() const -> URI override;
+
     auto exists() const -> bool override;
 
     void create();
@@ -60,8 +62,6 @@ class S3BucketName : public S3Name {
 private:  // methods
     void print(std::ostream& out) const override;
 
-    void parse();
-
 private:  // members
     std::string bucket_;
 };

diff --git a/src/eckit/io/s3/S3Name.cc b/src/eckit/io/s3/S3Name.cc
@@ -24,42 +24,43 @@
 #include <memory>
 #include <ostream>
 #include <string>
-#include <utility>
 #include <vector>
 
 namespace eckit {
 
 //----------------------------------------------------------------------------------------------------------------------
 
-S3Name::S3Name(const URI& uri) : endpoint_ {uri}, name_ {uri.name()} {
+auto S3Name::parse(const std::string& name) -> std::vector<std::string> {
+    return Tokenizer("/").tokenize(name);
+}
+
+//----------------------------------------------------------------------------------------------------------------------
+
+S3Name::S3Name(const net::Endpoint& endpoint) : endpoint_ {endpoint} { }
+
+S3Name::S3Name(const URI& uri) : S3Name( {uri.host(), uri.port()}) {
     /// @todo is "s3://endpoint/bucket/object" a valid URI ?
     ASSERT(uri.scheme() == type);
 }
 
-S3Name::S3Name(const net::Endpoint& endpoint, std::string name) : endpoint_ {endpoint}, name_ {std::move(name)} { }
-
 S3Name::~S3Name() = default;
 
 //----------------------------------------------------------------------------------------------------------------------
 
 auto S3Name::uri() const -> URI {
-    return {type, asString()};
+    return {type, endpoint_.host(), endpoint_.port()};
 }
 
 auto S3Name::asString() const -> std::string {
     return std::string(endpoint_);
 }
 
 void S3Name::print(std::ostream& out) const {
-    out << ",endpoint=" << endpoint_ << ",name=" << name_ << "]";
+    out << ",endpoint=" << endpoint_ << ",]";
 }
 
 //----------------------------------------------------------------------------------------------------------------------
 
-auto S3Name::parseName() const -> std::vector<std::string> {
-    return Tokenizer("/").tokenize(name_);
-}
-
 auto S3Name::client() const -> S3Client& {
     if (!client_) { client_ = S3Session::instance().getClient(endpoint_); }
     return *client_;

diff --git a/src/eckit/io/s3/S3Name.h b/src/eckit/io/s3/S3Name.h
@@ -37,10 +37,12 @@ class S3Name {
 public:  // types
     static constexpr auto type = "s3";
 
+    static auto parse(const std::string& name) -> std::vector<std::string>;
+
 public:  // methods
-    explicit S3Name(const URI& uri);
+    explicit S3Name(const net::Endpoint& endpoint);
 
-    S3Name(const net::Endpoint& endpoint, std::string name);
+    explicit S3Name(const URI& uri);
 
     // rules
 
@@ -54,10 +56,10 @@ class S3Name {
 
     // accessors
 
-    auto uri() const -> URI;
-
     auto endpoint() const -> const net::Endpoint& { return endpoint_; }
 
+    virtual auto uri() const -> URI;
+
     virtual auto exists() const -> bool = 0;
 
     virtual auto asString() const -> std::string;
@@ -67,14 +69,10 @@ class S3Name {
 
     friend std::ostream& operator<<(std::ostream& out, const S3Name& name);
 
-    [[nodiscard]]
-    auto parseName() const -> std::vector<std::string>;
-
     auto client() const -> S3Client&;
 
 private:  // members
     net::Endpoint endpoint_;
-    std::string   name_;
 
     mutable std::shared_ptr<S3Client> client_;
 };

diff --git a/src/eckit/io/s3/S3ObjectName.cc b/src/eckit/io/s3/S3ObjectName.cc
@@ -32,11 +32,14 @@ namespace eckit {
 //----------------------------------------------------------------------------------------------------------------------
 
 S3ObjectName::S3ObjectName(const URI& uri) : S3Name(uri) {
-    parse();
+    const auto pairs = parse(uri.name());
+    if (pairs.size() != 2) { throw S3SeriousBug("Could not parse bucket and object names!", Here()); }
+    bucket_ = pairs[0];
+    object_ = pairs[1];
 }
 
 S3ObjectName::S3ObjectName(const net::Endpoint& endpoint, std::string bucket, std::string object)
-    : S3Name(endpoint, "/"), bucket_ {std::move(bucket)}, object_ {std::move(object)} { }
+    : S3Name(endpoint), bucket_ {std::move(bucket)}, object_ {std::move(object)} { }
 
 //----------------------------------------------------------------------------------------------------------------------
 
@@ -45,15 +48,14 @@ void S3ObjectName::print(std::ostream& out) const {
     S3Name::print(out);
 }
 
-void S3ObjectName::parse() {
-    const auto pairs = parseName();
-    if (pairs.size() != 2) { throw S3SeriousBug("Could not parse bucket and object names!", Here()); }
-    bucket_ = pairs[0];
-    object_ = pairs[1];
-}
-
 //----------------------------------------------------------------------------------------------------------------------
 
+auto S3ObjectName::uri() const -> URI {
+    auto uri = S3Name::uri();
+    uri.path("/" + bucket_ + "/" + object_);
+    return uri;
+}
+
 auto S3ObjectName::asString() const -> std::string {
     return S3Name::asString() + "/" + bucket_ + "/" + object_;
 }

diff --git a/src/eckit/io/s3/S3ObjectName.h b/src/eckit/io/s3/S3ObjectName.h
@@ -38,6 +38,8 @@ class S3ObjectName : public S3Name {
 
     S3ObjectName(const net::Endpoint& endpoint, std::string bucket, std::string object);
 
+    auto uri() const -> URI override;
+
     auto size() const -> long long;
 
     auto exists() const -> bool override;
@@ -65,8 +67,6 @@ class S3ObjectName : public S3Name {
 private:  // methods
     void print(std::ostream& out) const override;
 
-    void parse();
-
 private:  // members
     std::string bucket_;
     std::string object_;