coding-style

* use trailing underscore for private vars
* private after public

src/http/include/mysqlrouter/http_client.h

@@ -29,18 +29,19 @@
 #include "mysqlrouter/http_common.h"
 
 class HTTP_CLIENT_EXPORT HttpClient {
-  class impl;
-
-  std::unique_ptr<impl> pImpl;
-
-  IOContext &io_ctx_;
 public:
   HttpClient();
   ~HttpClient();
   HttpClient(IOContext &io_ctx, const std::string &address, uint16_t port);
 
   void make_request(HttpRequest *req, HttpMethod::type method, const std::string &uri);
   void make_request_sync(HttpRequest *req, HttpMethod::type method, const std::string &uri);
+private:
+  class impl;
+
+  std::unique_ptr<impl> pImpl_;
+
+  IOContext &io_ctx_;
 };
 
 #endif

src/http/include/mysqlrouter/http_common.h

@@ -188,26 +188,21 @@ namespace HttpStatusCode {
 }
 
 class HTTP_COMMON_EXPORT HttpUri {
-  struct impl;
-
-  std::unique_ptr<impl> pImpl;
-
 public:
   HttpUri(std::unique_ptr<evhttp_uri, std::function<void(evhttp_uri *)>> uri);
   HttpUri(HttpUri &&);
   ~HttpUri();
   static HttpUri parse(const std::string &uri_str);
 
   std::string get_path() const;
+private:
+  struct impl;
+
+  std::unique_ptr<impl> pImpl_;
 };
 
 // wrapper around evbuffer
 class HTTP_COMMON_EXPORT HttpBuffer {
-  struct impl;
-
-  std::unique_ptr<impl> pImpl;
-
-  friend class HttpRequest;
 public:
   HttpBuffer(std::unique_ptr<evbuffer, std::function<void(evbuffer *)>> buffer);
 
@@ -220,19 +215,22 @@ class HTTP_COMMON_EXPORT HttpBuffer {
 
   size_t length() const;
   std::vector<uint8_t> pop_front(size_t length);
+private:
+  struct impl;
+
+  std::unique_ptr<impl> pImpl_;
+
+  friend class HttpRequest;
 };
 
 class HTTP_COMMON_EXPORT HttpHeaders {
-  struct impl;
-
-  std::unique_ptr<impl> pImpl;
 public:
   class HTTP_COMMON_EXPORT Iterator {
     evkeyval *node_;
   public:
     Iterator(evkeyval *node):
       node_{node}
-    {};
+    {}
     std::pair<std::string, std::string> operator*();
     Iterator& operator++();
     bool operator!=(const Iterator &it) const;
@@ -246,6 +244,10 @@ class HTTP_COMMON_EXPORT HttpHeaders {
 
   Iterator begin();
   Iterator end();
+private:
+  struct impl;
+
+  std::unique_ptr<impl> pImpl_;
 };
 
 namespace HttpMethod {
@@ -262,8 +264,8 @@ namespace HttpMethod {
     constexpr pos_type Connect = 7;
     constexpr pos_type Patch = 8;
 
-    constexpr unsigned _LAST = Patch;
-  };
+    constexpr pos_type _LAST = Patch;
+  }
   using Bitset = std::bitset<Pos::_LAST>;
 
   constexpr type Get { 1 << Pos::Get };
@@ -275,25 +277,30 @@ namespace HttpMethod {
   constexpr type Trace { 1 << Pos::Trace };
   constexpr type Connect { 1 << Pos::Connect };
   constexpr type Patch { 1 << Pos::Patch };
-};
+}
 
 class HTTP_COMMON_EXPORT IOContext {
-  class impl;
-
-  std::unique_ptr<impl> pImpl;
-  friend class HttpClient;
 public:
   IOContext();
   ~IOContext();
-  void dispatch();
-};
 
-class HTTP_COMMON_EXPORT HttpRequest {
+  /**
+   * wait for events to fire and calls handlers.
+   *
+   * exits if no more pending events
+   *
+   * @returns false if no events were pending nor active, true otherwise
+   * @throws  std::runtime_error on internal, unexpected error
+   */
+  bool dispatch();
+private:
   class impl;
 
-  std::unique_ptr<impl> pImpl;
-
+  std::unique_ptr<impl> pImpl_;
   friend class HttpClient;
+};
+
+class HTTP_COMMON_EXPORT HttpRequest {
 public:
   using RequestHandler = void (*)(HttpRequest *, void *);
 
@@ -332,12 +339,18 @@ class HTTP_COMMON_EXPORT HttpRequest {
   int error_code();
   void error_code(int);
   std::string error_msg();
+private:
+  class impl;
+
+  std::unique_ptr<impl> pImpl_;
+
+  friend class HttpClient;
 };
 
 // http_time.cc
 
 HTTP_COMMON_EXPORT bool is_modified_since(const HttpRequest &req, time_t last_modified);
-HTTP_COMMON_EXPORT void add_last_modified(HttpRequest &req, time_t last_modified);
+HTTP_COMMON_EXPORT bool add_last_modified(HttpRequest &req, time_t last_modified);
 HTTP_COMMON_EXPORT time_t time_from_rfc5322_fixdate(const char *date_buf);
 HTTP_COMMON_EXPORT int time_to_rfc5322_fixdate(time_t ts, char *date_buf, size_t date_buf_len);
 

src/http/include/mysqlrouter/http_server_component.h

@@ -43,6 +43,12 @@ class HTTP_SERVER_EXPORT BaseRequestHandler {
 };
 
 class HTTP_SERVER_EXPORT HttpServerComponent {
+public:
+  static HttpServerComponent& getInstance();
+  void init(std::shared_ptr<HttpServer> srv);
+  void add_route(const std::string &url_regex, std::unique_ptr<BaseRequestHandler> cb);
+  void remove_route(const std::string &url_regex);
+private:
   // disable copy, as we are a single-instance
   HttpServerComponent(HttpServerComponent const &) = delete;
   void operator=(HttpServerComponent const &) = delete;
@@ -58,11 +64,6 @@ class HTTP_SERVER_EXPORT HttpServerComponent {
   std::weak_ptr<HttpServer> srv_;
 
   HttpServerComponent() = default;
-public:
-  static HttpServerComponent& getInstance();
-  void init(std::shared_ptr<HttpServer> srv);
-  void add_route(const std::string &url_regex, std::unique_ptr<BaseRequestHandler> cb);
-  void remove_route(const std::string &url_regex);
 };
 
 

src/http/include/mysqlrouter/rest_client.h

@@ -46,7 +46,7 @@ class RestClient {
       out_buf.add(request_body.data(), request_body.size());
     }
 
-    // ask the server the close the connection after this request
+    // ask the server to close the connection after this request
     req.get_output_headers().add("Connection", "close");
     req.get_output_headers().add("Host", hostname_.c_str());
     http_client_.make_request_sync(&req, method, uri);

src/http/src/http_client.cc

@@ -41,12 +41,19 @@ class IOContext::impl {
 };
 
 IOContext::IOContext():
-  pImpl{new impl()}
+  pImpl_{new impl()}
 {
 }
 
-void IOContext::dispatch() {
-  event_base_dispatch(pImpl->ev_base.get());
+bool IOContext::dispatch() {
+  int ret = event_base_dispatch(pImpl_->ev_base.get());
+
+  if (ret == -1) {
+    // we don't have an better error here
+    throw std::runtime_error("event_base_dispath() error");
+  }
+
+  return ret == 0;
 }
 
 IOContext::~IOContext() = default;
@@ -60,33 +67,35 @@ class HttpClient::impl {
 };
 
 HttpClient::HttpClient(IOContext &io_ctx, const std::string &address, uint16_t port):
-  pImpl{new impl()},
+  pImpl_{new impl()},
 
   // gcc-4.8 requires a () here, instead of {}
   //
   // invalid initialization of non-const reference of type ‘IOContext&’ from an rvalue of type ‘<brace-enclosed initializer list>’
   io_ctx_(io_ctx)
 {
-  auto *ev_base = io_ctx_.pImpl->ev_base.get();
-  pImpl->conn.reset(evhttp_connection_base_new(ev_base, NULL, address.c_str(), port));
+  auto *ev_base = io_ctx_.pImpl_->ev_base.get();
+  pImpl_->conn.reset(evhttp_connection_base_new(ev_base, NULL, address.c_str(), port));
 }
 
 void HttpClient::make_request(HttpRequest *req, HttpMethod::type method, const std::string &uri) {
-  auto *ev_req = req->pImpl->req.get();
+  auto *ev_req = req->pImpl_->req.get();
 
-  evhttp_make_request(pImpl->conn.get(), ev_req,
-      static_cast<enum evhttp_cmd_type>(method), uri.c_str());
+  if (0 != evhttp_make_request(pImpl_->conn.get(), ev_req,
+      static_cast<enum evhttp_cmd_type>(method), uri.c_str())) {
+    throw std::runtime_error("evhttp_make_request() failed");
+  }
 
   // don't free the evhttp_request() when HttpRequest gets destructed
   // as the eventloop will do it
-  req->pImpl->disown();
-};
+  req->pImpl_->disown();
+}
 
 void HttpClient::make_request_sync(HttpRequest *req, HttpMethod::type method, const std::string &uri) {
   make_request(req, method, uri);
 
   io_ctx_.dispatch();
-};
+}
 
 
 HttpClient::~HttpClient() = default;