Clean up url_manip (#3086)

mamba-org · Dec 27, 2023 · 4ee7531 · 4ee7531
1 parent bcf8141
commit 4ee7531
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 35 deletions.
diff --git a/libmamba/include/mamba/util/url_manip.hpp b/libmamba/include/mamba/util/url_manip.hpp
@@ -53,8 +53,15 @@ namespace mamba::util
      */
     [[nodiscard]] auto path_or_url_to_url(std::string_view path) -> std::string;
 
-    template <class S, class... Args>
-    std::string join_url(const S& s, const Args&... args);
+    /**
+     * Join folder elements of a URL.
+     *
+     * Concatenate arguments making sure they are separated by a unique slash separator.
+     *
+     * @see path_concat
+     */
+    template <typename... Args>
+    [[nodiscard]] auto url_concat(const Args&... args) -> std::string;
 
     /**
      * Convert UNC2 file URI to UNC4.
@@ -75,50 +82,54 @@ namespace mamba::util
      */
     [[nodiscard]] auto file_uri_unc2_to_unc4(std::string_view url) -> std::string;
 
+    /********************
+     *  Implementation  *
+     ********************/
+
     namespace detail
     {
-        inline std::string join_url_impl(std::string& s)
+        inline auto as_string_view(std::string_view str) -> std::string_view
         {
-            return s;
+            return str;
         }
 
-        template <class S, class... Args>
-        inline std::string join_url_impl(std::string& s1, const S& s2, const Args&... args)
+        inline auto as_string_view(const char& c) -> std::string_view
         {
-            if (!s2.empty())
-            {
-                if (s1.empty() || s1.back() != '/')
-                {
-                    s1 += '/';
-                }
-                s1 += s2;
-            }
-            return join_url_impl(s1, args...);
+            return { &c, 1 };
         }
 
-        template <class... Args>
-        inline std::string join_url_impl(std::string& s1, const char* s2, const Args&... args)
+        template <typename... Args>
+        auto url_concat_impl(const Args&... args) -> std::string
         {
-            if (s1.size() && s1.back() != '/')
+            auto join_two = [](std::string& out, std::string_view to_add)
             {
-                s1 += '/';
-            }
-            s1 += s2;
-            return join_url_impl(s1, args...);
-        }
-    }  // namespace detail
+                if (!out.empty() && !to_add.empty())
+                {
+                    bool const out_has_slash = out.back() == '/';
+                    bool const to_add_has_slash = to_add.front() == '/';
+                    if (out_has_slash && to_add_has_slash)
+                    {
+                        to_add = to_add.substr(1);
+                    }
+                    if (!out_has_slash && !to_add_has_slash)
+                    {
+                        out += '/';
+                    }
+                }
+                out += to_add;
+            };
 
-    inline std::string join_url()
-    {
-        return "";
+            std::string result;
+            result.reserve(((args.size() + 1) + ...));
+            (join_two(result, args), ...);
+            return result;
+        }
     }
 
-    template <class S, class... Args>
-    inline std::string join_url(const S& s, const Args&... args)
+    template <typename... Args>
+    auto url_concat(const Args&... args) -> std::string
     {
-        std::string res = s;
-        return detail::join_url_impl(res, args...);
+        return detail::url_concat_impl(detail::as_string_view(args)...);
     }
-}  // namespace mamba
-
+}
 #endif
diff --git a/libmamba/src/core/mirror.cpp b/libmamba/src/core/mirror.cpp
@@ -108,7 +108,7 @@ namespace mamba
 
     std::string Mirror::format_url(const std::string& path) const
     {
-        return util::join_url(m_url, path);
+        return util::url_concat(m_url, path);
     }
 
     std::string Mirror::get_auth_header(const std::string&) const

diff --git a/libmamba/src/core/subdirdata.cpp b/libmamba/src/core/subdirdata.cpp
@@ -540,7 +540,7 @@ namespace mamba
         , m_expired_cache_path("")
         , m_writable_pkgs_dir(caches.first_writable_path())
         , m_repodata_url(util::concat(url, "/", repodata_fn))
-        , m_name(util::join_url(channel.display_name(), platform))
+        , m_name(util::url_concat(channel.display_name(), platform))
         , m_json_fn(cache_fn_url(m_repodata_url))
         , m_solv_fn(m_json_fn.substr(0, m_json_fn.size() - 4) + "solv")
         , m_is_noarch(platform == "noarch")

diff --git a/libmamba/tests/src/util/test_url_manip.cpp b/libmamba/tests/src/util/test_url_manip.cpp
@@ -133,6 +133,24 @@ TEST_SUITE("util::url_manip")
         }
     }
 
+    TEST_CASE("url_concat")
+    {
+        CHECK_EQ(url_concat("", ""), "");
+        CHECK_EQ(url_concat("", "/"), "/");
+        CHECK_EQ(url_concat("/", ""), "/");
+        CHECK_EQ(url_concat("/", "/"), "/");
+
+        CHECK_EQ(url_concat("mamba.org", "folder"), "mamba.org/folder");
+        CHECK_EQ(url_concat("mamba.org", "/folder"), "mamba.org/folder");
+        CHECK_EQ(url_concat("mamba.org/", "folder"), "mamba.org/folder");
+        CHECK_EQ(url_concat("mamba.org/", "/folder"), "mamba.org/folder");
+
+        CHECK_EQ(
+            url_concat("mamba.org", 't', std::string("/sometoken/"), std::string_view("conda-forge")),
+            "mamba.org/t/sometoken/conda-forge"
+        );
+    }
+
     TEST_CASE("file_uri_unc2_to_unc4")
     {
         for (const std::string uri : {