refactor(http): replace service/method with simple HTTP path

include/dsn/http/http_server.h

@@ -8,9 +8,12 @@
 #include <dsn/tool-api/rpc_message.h>
 #include <dsn/cpp/serverlet.h>
 #include <dsn/utility/errors.h>
+#include <dsn/utility/flags.h>
 
 namespace dsn {
 
+DSN_DECLARE_bool(enable_http_server);
+
 enum http_method
 {
     HTTP_METHOD_GET = 1,
@@ -21,8 +24,7 @@ struct http_request
 {
     static error_with<http_request> parse(dsn::message_ex *m);
 
-    // http://ip:port/<service>/<method>
-    std::pair<std::string, std::string> service_method;
+    std::string path;
     // <args_name, args_val>
     std::unordered_map<std::string, std::string> query_args;
     blob body;
@@ -51,78 +53,37 @@ struct http_response
     message_ptr to_message(message_ex *req) const;
 };
 
+typedef std::function<void(const http_request &req, http_response &resp)> http_callback;
+
+// Defines the structure of an HTTP call.
+struct http_call
+{
+    std::string path;
+    std::string help;
+    http_callback callback;
+};
+
 class http_service
 {
 public:
-    typedef std::function<void(const http_request &req, http_response &resp)> http_callback;
-
     virtual ~http_service() = default;
 
     virtual std::string path() const = 0;
 
-    void register_handler(std::string path, http_callback cb, std::string help)
-    {
-        _cb_map.emplace(std::move(path), std::make_pair(std::move(cb), std::move(help)));
-    }
-
-    void call(const http_request &req, http_response &resp)
-    {
-        auto it = _cb_map.find(req.service_method.second);
-        if (it != _cb_map.end()) {
-            it->second.first(req, resp);
-        } else {
-            resp.status_code = http_status_code::not_found;
-            resp.body = std::string("method not found for \"") + req.service_method.second + "\"";
-        }
-    }
-
-    struct method_help_entry
-    {
-        std::string name;
-        std::string help;
-    };
-    std::vector<method_help_entry> get_help() const
-    {
-        std::vector<method_help_entry> ret;
-        ret.reserve(_cb_map.size());
-        for (const auto &method : _cb_map) {
-            ret.push_back({method.first, method.second.second});
-        }
-        return ret;
-    }
-
-private:
-    std::map<std::string, std::pair<http_callback, std::string>> _cb_map;
+    void register_handler(std::string path, http_callback cb, std::string help);
 };
 
 class http_server : public serverlet<http_server>
 {
 public:
-    explicit http_server(bool start = true);
+    explicit http_server();
 
     ~http_server() override = default;
 
     void add_service(http_service *service);
 
     void serve(message_ex *msg);
 
-    struct service_method_help_entry
-    {
-        std::string name;
-        std::string method;
-        std::string help;
-    };
-    std::vector<service_method_help_entry> get_help() const
-    {
-        std::vector<service_method_help_entry> ret;
-        for (const auto &service : _service_map) {
-            for (const auto &method : service.second->get_help()) {
-                ret.push_back({service.first, method.name, method.help});
-            }
-        }
-        return ret;
-    }
-
 private:
     std::map<std::string, std::unique_ptr<http_service>> _service_map;
 };

src/http/http_call_registry.h

@@ -0,0 +1,75 @@
+// 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.
+
+#pragma once
+
+#include <dsn/http/http_server.h>
+#include <dsn/utility/errors.h>
+
+namespace dsn {
+
+// A singleton registry for all the HTTP calls
+class http_call_registry : public utils::singleton<http_call_registry>
+{
+public:
+    std::shared_ptr<http_call> find(const std::string &path) const
+    {
+        std::lock_guard<std::mutex> guard(_mu);
+        auto it = _call_map.find(path);
+        if (it == _call_map.end()) {
+            return nullptr;
+        }
+        return it->second;
+    }
+
+    void remove(const std::string &path)
+    {
+        std::lock_guard<std::mutex> guard(_mu);
+        _call_map.erase(path);
+    }
+
+    void add(std::unique_ptr<http_call> call_uptr)
+    {
+        auto call = std::shared_ptr<http_call>(call_uptr.release());
+        std::lock_guard<std::mutex> guard(_mu);
+        dassert(_call_map.find(call->path) == _call_map.end(),
+                "repeatedly register http call \"%s\"",
+                call->path.c_str());
+        _call_map[call->path] = call;
+    }
+
+    std::vector<std::shared_ptr<http_call>> list_all_calls() const
+    {
+        std::lock_guard<std::mutex> guard(_mu);
+
+        std::vector<std::shared_ptr<http_call>> ret;
+        for (const auto &kv : _call_map) {
+            ret.push_back(kv.second);
+        }
+        return ret;
+    }
+
+private:
+    friend class utils::singleton<http_call_registry>;
+    http_call_registry() = default;
+
+private:
+    mutable std::mutex _mu;
+    std::map<std::string, std::shared_ptr<http_call>> _call_map;
+};
+
+} // namespace dsn

src/http/http_server.cpp

@@ -12,9 +12,12 @@
 #include "pprof_http_service.h"
 #include "perf_counter_http_service.h"
 #include "uri_decoder.h"
+#include "http_call_registry.h"
 
 namespace dsn {
 
+DSN_DEFINE_bool("http", enable_http_server, true, "whether to enable the embedded HTTP server");
+
 /*extern*/ std::string http_status_code_to_string(http_status_code code)
 {
     switch (code) {
@@ -34,9 +37,24 @@ namespace dsn {
     }
 }
 
-http_server::http_server(bool start /*default=true*/) : serverlet<http_server>("http_server")
+void http_service::register_handler(std::string path, http_callback cb, std::string help)
+{
+    if (!FLAGS_enable_http_server) {
+        return;
+    }
+    auto call = make_unique<http_call>();
+    call->path = this->path();
+    if (!path.empty()) {
+        call->path += "/" + std::move(path);
+    }
+    call->callback = std::move(cb);
+    call->help = std::move(help);
+    http_call_registry::instance().add(std::move(call));
+}
+
+http_server::http_server() : serverlet<http_server>("http_server")
 {
-    if (!start) {
+    if (!FLAGS_enable_http_server) {
         return;
     }
 
@@ -45,7 +63,7 @@ http_server::http_server(bool start /*default=true*/) : serverlet<http_server>("
     tools::register_message_header_parser<http_message_parser>(NET_HDR_HTTP, {"GET ", "POST"});
 
     // add builtin services
-    add_service(new root_http_service(this));
+    add_service(new root_http_service());
 
 #ifdef DSN_ENABLE_GPERF
     add_service(new pprof_http_service());
@@ -63,12 +81,12 @@ void http_server::serve(message_ex *msg)
         resp.body = fmt::format("failed to parse request: {}", res.get_error());
     } else {
         const http_request &req = res.get_value();
-        auto it = _service_map.find(req.service_method.first);
-        if (it != _service_map.end()) {
-            it->second->call(req, resp);
+        std::shared_ptr<http_call> call = http_call_registry::instance().find(req.path);
+        if (call != nullptr) {
+            call->callback(req, resp);
         } else {
             resp.status_code = http_status_code::not_found;
-            resp.body = fmt::format("service not found for \"{}\"", req.service_method.first);
+            resp.body = fmt::format("service not found for \"{}\"", req.path);
         }
     }
 
@@ -131,26 +149,7 @@ void http_server::add_service(http_service *service)
     if (!unresolved_path.empty() && *unresolved_path.crbegin() == '\0') {
         unresolved_path.pop_back();
     }
-    std::vector<std::string> args;
-    boost::split(args, unresolved_path, boost::is_any_of("/"));
-    std::vector<std::string> real_args;
-    for (std::string &arg : args) {
-        if (!arg.empty()) {
-            real_args.emplace_back(std::move(arg));
-        }
-    }
-    if (real_args.size() == 0) {
-        ret.service_method = {"", ""};
-    } else if (real_args.size() == 1) {
-        ret.service_method = {std::move(real_args[0]), ""};
-    } else {
-        std::string method = std::move(real_args[1]);
-        for (int i = 2; i < real_args.size(); i++) {
-            method += '/';
-            method += real_args[i];
-        }
-        ret.service_method = {std::move(real_args[0]), std::move(method)};
-    }
+    ret.path = std::move(unresolved_path);
 
     // find if there are method args (<ip>:<port>/<service>/<method>?<arg>=<val>&<arg>=<val>)
     if (!unresolved_query.empty()) {

src/http/root_http_service.h

@@ -6,12 +6,14 @@
 #include <dsn/utility/output_utils.h>
 #include <string>
 
+#include "http_call_registry.h"
+
 namespace dsn {
 
 class root_http_service : public http_service
 {
 public:
-    explicit root_http_service(http_server *server) : _server(server)
+    explicit root_http_service()
     {
         // url: ip:port/
         register_handler("",
@@ -28,19 +30,14 @@ class root_http_service : public http_service
     {
         utils::table_printer tp;
         std::ostringstream oss;
-        auto help_entries = _server->get_help();
-        for (const auto &ent : help_entries) {
-            tp.add_row_name_and_data(std::string("/") + ent.name + (ent.method.empty() ? "" : "/") +
-                                         ent.method,
-                                     ent.help);
+        auto calls = http_call_registry::instance().list_all_calls();
+        for (const auto &call : calls) {
+            tp.add_row_name_and_data(std::string("/") + call->path, call->help);
         }
         tp.output(oss, utils::table_printer::output_format::kJsonCompact);
         resp.body = oss.str();
         resp.status_code = http_status_code::ok;
     }
-
-private:
-    http_server *_server;
 };
 
 } // namespace dsn

src/http/test/http_server_test.cpp

@@ -18,17 +18,17 @@ TEST(http_server, parse_url)
         std::string url;
 
         error_code err;
-        std::pair<std::string, std::string> result;
+        std::string path;
     } tests[] = {
-        {"http://127.0.0.1:34601", ERR_OK, {"", ""}},
-        {"http://127.0.0.1:34601/", ERR_OK, {"", ""}},
-        {"http://127.0.0.1:34601///", ERR_OK, {"", ""}},
-        {"http://127.0.0.1:34601/threads", ERR_OK, {"threads", ""}},
-        {"http://127.0.0.1:34601/threads/", ERR_OK, {"threads", ""}},
-        {"http://127.0.0.1:34601//pprof/heap/", ERR_OK, {"pprof", "heap"}},
-        {"http://127.0.0.1:34601//pprof///heap", ERR_OK, {"pprof", "heap"}},
-        {"http://127.0.0.1:34601/pprof/heap/arg/", ERR_OK, {"pprof", "heap/arg"}},
-        {"http://127.0.0.1:34601/pprof///heap///arg/", ERR_OK, {"pprof", "heap/arg"}},
+        {"http://127.0.0.1:34601", ERR_OK, ""},
+        {"http://127.0.0.1:34601/", ERR_OK, "/"},
+        {"http://127.0.0.1:34601///", ERR_OK, "///"},
+        {"http://127.0.0.1:34601/threads", ERR_OK, "/threads"},
+        {"http://127.0.0.1:34601/threads/?detail", ERR_OK, "/threads/"},
+        {"http://127.0.0.1:34601//pprof/heap/", ERR_OK, "//pprof/heap/"},
+        {"http://127.0.0.1:34601//pprof///heap?detailed=true", ERR_OK, "//pprof///heap"},
+        {"http://127.0.0.1:34601/pprof/heap/arg/", ERR_OK, "/pprof/heap/arg/"},
+        {"http://127.0.0.1:34601/pprof///heap///arg/", ERR_OK, "/pprof///heap///arg/"},
     };
 
     for (auto tt : tests) {
@@ -38,7 +38,7 @@ TEST(http_server, parse_url)
 
         auto res = http_request::parse(m.get());
         if (res.is_ok()) {
-            ASSERT_EQ(res.get_value().service_method, tt.result) << tt.url;
+            ASSERT_EQ(res.get_value().path, tt.path) << tt.url;
         } else {
             ASSERT_EQ(res.get_error().code(), tt.err);
         }
@@ -47,22 +47,24 @@ TEST(http_server, parse_url)
 
 TEST(root_http_service_test, get_help)
 {
-    http_server server(false);
-    auto root = new root_http_service(&server);
-    server.add_service(root);
-    ASSERT_EQ(server.get_help().size(), 1);
+    for (const auto &call : http_call_registry::instance().list_all_calls()) {
+        http_call_registry::instance().remove(call->path);
+    }
 
+    root_http_service root;
     http_request req;
     http_response resp;
-    root->default_handler(req, resp);
+    root.default_handler(req, resp);
     ASSERT_EQ(resp.status_code, http_status_code::ok);
     ASSERT_EQ(resp.body, "{\"/\":\"ip:port/\"}\n");
 
-    auto ver = new version_http_service();
-    server.add_service(ver);
-    ASSERT_EQ(server.get_help().size(), 2);
-    root->default_handler(req, resp);
+    version_http_service ver;
+    root.default_handler(req, resp);
     ASSERT_EQ(resp.body, "{\"/\":\"ip:port/\",\"/version\":\"ip:port/version\"}\n");
+
+    for (const auto &call : http_call_registry::instance().list_all_calls()) {
+        http_call_registry::instance().remove(call->path);
+    }
 }
 
 class http_message_parser_test : public testing::Test