Skip to content

Commit f73cc6b

Browse files
librazlibraz
committed
refactor: Improve code quality and build system organization
- Refactor CMake dependency management for cleaner transitive linking - Add LZ4 library support in Docker build stages - Fix clang-tidy warnings: use structured bindings, range-based loops, const correctness - Add missing includes (cstddef, sstream) for better compilation compatibility - Enhance server bind address configuration with inet_pton validation - Move FormatBytes utility from memory_utils to string_utils for better organization - Make ConfigSchemaExplorer help formatting methods static for improved API - Clean up test dependencies and remove redundant library links - Update memory_utils_test expectations to match string_utils::FormatBytes behavior
1 parent e9d950d commit f73cc6b

24 files changed

+129
-128
lines changed

‎Dockerfile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ RUN apt-get update && apt-get install -y \
1212
git \
1313
pkg-config \
1414
libmysqlclient-dev \
15+
liblz4-dev \
1516
libicu-dev \
1617
libreadline-dev \
1718
ca-certificates \
@@ -42,6 +43,7 @@ FROM ubuntu:22.04
4243
RUN apt-get update && apt-get install -y \
4344
libstdc++6 \
4445
libmysqlclient21 \
46+
liblz4-1 \
4547
libicu70 \
4648
libreadline8 \
4749
ca-certificates \

‎src/CMakeLists.txt‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,7 @@ add_subdirectory(client)
2626
add_executable(mygramdb main.cpp)
2727

2828
target_link_libraries(mygramdb PRIVATE
29-
mygramdb_config
30-
mygramdb_server # Includes mygramdb_cache, mygramdb_query, mygramdb_index, mygramdb_storage transitively
29+
mygramdb_server # Includes config, cache, query, index, storage transitively
3130
)
3231

33-
# Link MySQL library if available
34-
if(USE_MYSQL AND MYSQL_FOUND)
35-
target_link_libraries(mygramdb PRIVATE mygramdb_mysql)
36-
endif()
37-
3832
target_include_directories(mygramdb PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})

‎src/cache/result_compressor.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#pragma once
77

8+
#include <cstddef>
89
#include <cstdint>
910
#include <vector>
1011

‎src/config/config_help.cpp‎

Lines changed: 49 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace {
2525
std::string ToLower(const std::string& str) {
2626
std::string result = str;
2727
std::transform(result.begin(), result.end(), result.begin(),
28-
[](unsigned char c) { return std::tolower(c); });
28+
[](unsigned char ch_value) { return std::tolower(ch_value); });
2929
return result;
3030
}
3131

@@ -38,15 +38,17 @@ std::string ToLower(const std::string& str) {
3838
std::string JsonValueToString(const nlohmann::json& value) {
3939
if (value.is_string()) {
4040
return "\"" + value.get<std::string>() + "\"";
41-
} else if (value.is_boolean()) {
41+
}
42+
if (value.is_boolean()) {
4243
return value.get<bool>() ? "true" : "false";
43-
} else if (value.is_number()) {
44+
}
45+
if (value.is_number()) {
4446
return value.dump();
45-
} else if (value.is_null()) {
47+
}
48+
if (value.is_null()) {
4649
return "null";
47-
} else {
48-
return value.dump();
4950
}
51+
return value.dump();
5052
}
5153

5254
/**
@@ -286,17 +288,25 @@ std::optional<nlohmann::json> NavigateJsonPath(const nlohmann::json& json, const
286288
*/
287289
void MaskSensitiveFieldsRecursive(nlohmann::json& json, const std::string& path) {
288290
if (json.is_object()) {
289-
for (auto it = json.begin(); it != json.end(); ++it) {
290-
std::string child_path = path.empty() ? it.key() : path + "." + it.key();
291+
for (const auto& [key, child] : json.items()) {
292+
std::string child_path;
293+
if (path.empty()) {
294+
child_path = key;
295+
} else {
296+
child_path.reserve(path.size() + 1 + key.size());
297+
child_path.assign(path);
298+
child_path.push_back('.');
299+
child_path.append(key);
300+
}
291301
if (IsSensitiveField(child_path)) {
292-
it.value() = "***";
293-
} else if (it.value().is_object() || it.value().is_array()) {
294-
MaskSensitiveFieldsRecursive(it.value(), child_path);
302+
json[key] = "***";
303+
} else if (child.is_object() || child.is_array()) {
304+
MaskSensitiveFieldsRecursive(json[key], child_path);
295305
}
296306
}
297307
} else if (json.is_array()) {
298-
for (size_t i = 0; i < json.size(); ++i) {
299-
MaskSensitiveFieldsRecursive(json[i], path);
308+
for (auto& child : json) {
309+
MaskSensitiveFieldsRecursive(child, path);
300310
}
301311
}
302312
}
@@ -313,12 +323,12 @@ std::string JsonToYaml(const nlohmann::json& json, int indent = 0) {
313323
std::string indent_str(indent * 2, ' ');
314324

315325
if (json.is_object()) {
316-
for (auto it = json.begin(); it != json.end(); ++it) {
317-
oss << indent_str << it.key() << ":";
318-
if (it.value().is_object() || it.value().is_array()) {
319-
oss << "\n" << JsonToYaml(it.value(), indent + 1);
326+
for (const auto& [key, child] : json.items()) {
327+
oss << indent_str << key << ":";
328+
if (child.is_object() || child.is_array()) {
329+
oss << "\n" << JsonToYaml(child, indent + 1);
320330
} else {
321-
oss << " " << JsonValueToString(it.value()) << "\n";
331+
oss << " " << JsonValueToString(child) << "\n";
322332
}
323333
}
324334
} else if (json.is_array()) {
@@ -327,21 +337,21 @@ std::string JsonToYaml(const nlohmann::json& json, int indent = 0) {
327337
if (item.is_object()) {
328338
// First property on same line, rest indented
329339
bool first = true;
330-
for (auto it = item.begin(); it != item.end(); ++it) {
340+
for (const auto& [key, value] : item.items()) {
331341
if (first) {
332-
oss << " " << it.key() << ":";
333-
if (it.value().is_object() || it.value().is_array()) {
334-
oss << "\n" << JsonToYaml(it.value(), indent + 2);
342+
oss << " " << key << ":";
343+
if (value.is_object() || value.is_array()) {
344+
oss << "\n" << JsonToYaml(value, indent + 2);
335345
} else {
336-
oss << " " << JsonValueToString(it.value()) << "\n";
346+
oss << " " << JsonValueToString(value) << "\n";
337347
}
338348
first = false;
339349
} else {
340-
oss << std::string((indent + 1) * 2, ' ') << it.key() << ":";
341-
if (it.value().is_object() || it.value().is_array()) {
342-
oss << "\n" << JsonToYaml(it.value(), indent + 2);
350+
oss << std::string((indent + 1) * 2, ' ') << key << ":";
351+
if (value.is_object() || value.is_array()) {
352+
oss << "\n" << JsonToYaml(value, indent + 2);
343353
} else {
344-
oss << " " << JsonValueToString(it.value()) << "\n";
354+
oss << " " << JsonValueToString(value) << "\n";
345355
}
346356
}
347357
}
@@ -395,19 +405,19 @@ std::map<std::string, std::string> ConfigSchemaExplorer::ListPaths(const std::st
395405
// List properties
396406
if (current.contains("properties")) {
397407
const auto& properties = current["properties"];
398-
for (auto it = properties.begin(); it != properties.end(); ++it) {
408+
for (const auto& [key, property] : properties.items()) {
399409
std::string description;
400-
if (it.value().contains("description")) {
401-
description = it.value()["description"].get<std::string>();
410+
if (property.contains("description")) {
411+
description = property["description"].get<std::string>();
402412
}
403-
result[it.key()] = description;
413+
result[key] = description;
404414
}
405415
}
406416

407417
return result;
408418
}
409419

410-
std::string ConfigSchemaExplorer::FormatHelp(const ConfigHelpInfo& info) const {
420+
std::string ConfigSchemaExplorer::FormatHelp(const ConfigHelpInfo& info) {
411421
std::ostringstream oss;
412422

413423
oss << info.path << "\n\n";
@@ -477,7 +487,7 @@ std::string ConfigSchemaExplorer::FormatHelp(const ConfigHelpInfo& info) const {
477487
}
478488

479489
std::string ConfigSchemaExplorer::FormatPathList(const std::map<std::string, std::string>& paths,
480-
const std::string& parent_path) const {
490+
const std::string& parent_path) {
481491
std::ostringstream oss;
482492

483493
if (parent_path.empty()) {
@@ -538,8 +548,7 @@ std::optional<nlohmann::json> ConfigSchemaExplorer::FindSchemaNode(const std::st
538548
return current;
539549
}
540550

541-
ConfigHelpInfo ConfigSchemaExplorer::ExtractHelpInfo(const std::string& path,
542-
const nlohmann::json& node) const {
551+
ConfigHelpInfo ConfigSchemaExplorer::ExtractHelpInfo(const std::string& path, const nlohmann::json& node) {
543552
ConfigHelpInfo info;
544553
info.path = path;
545554

@@ -552,7 +561,9 @@ ConfigHelpInfo ConfigSchemaExplorer::ExtractHelpInfo(const std::string& path,
552561
std::ostringstream oss;
553562
bool first = true;
554563
for (const auto& type : node["type"]) {
555-
if (!first) oss << " | ";
564+
if (!first) {
565+
oss << " | ";
566+
}
556567
oss << type.get<std::string>();
557568
first = false;
558569
}
@@ -621,9 +632,8 @@ std::vector<std::string> ConfigSchemaExplorer::SplitPath(const std::string& path
621632

622633
bool IsSensitiveField(const std::string& path) {
623634
std::string lower_path = ToLower(path);
624-
return lower_path.find("password") != std::string::npos ||
625-
lower_path.find("secret") != std::string::npos || lower_path.find("key") != std::string::npos ||
626-
lower_path.find("token") != std::string::npos;
635+
return lower_path.find("password") != std::string::npos || lower_path.find("secret") != std::string::npos ||
636+
lower_path.find("key") != std::string::npos || lower_path.find("token") != std::string::npos;
627637
}
628638

629639
std::string MaskSensitiveValue(const std::string& path, const std::string& value) {

‎src/config/config_help.h‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@
77

88
#include <cstdint>
99
#include <map>
10+
#include <nlohmann/json.hpp>
1011
#include <optional>
1112
#include <string>
1213
#include <vector>
1314

14-
#include <nlohmann/json.hpp>
15-
1615
#include "config/config.h"
1716

1817
namespace mygramdb::config {
@@ -21,16 +20,16 @@ namespace mygramdb::config {
2120
* @brief Configuration help information
2221
*/
2322
struct ConfigHelpInfo {
24-
std::string path; // e.g., "mysql.port"
25-
std::string type; // e.g., "integer"
26-
std::string description; // From schema
27-
std::optional<std::string> default_value; // If specified
28-
std::vector<std::string> allowed_values; // For enums
29-
std::optional<int64_t> minimum; // For numbers
30-
std::optional<int64_t> maximum; // For numbers
31-
std::optional<double> minimum_number; // For floating point numbers
32-
std::optional<double> maximum_number; // For floating point numbers
33-
bool required = false; // If required in parent
23+
std::string path; // e.g., "mysql.port"
24+
std::string type; // e.g., "integer"
25+
std::string description; // From schema
26+
std::optional<std::string> default_value; // If specified
27+
std::vector<std::string> allowed_values; // For enums
28+
std::optional<int64_t> minimum; // For numbers
29+
std::optional<int64_t> maximum; // For numbers
30+
std::optional<double> minimum_number; // For floating point numbers
31+
std::optional<double> maximum_number; // For floating point numbers
32+
bool required = false; // If required in parent
3433
};
3534

3635
/**
@@ -69,7 +68,7 @@ class ConfigSchemaExplorer {
6968
* @param info Help information
7069
* @return Formatted help text
7170
*/
72-
std::string FormatHelp(const ConfigHelpInfo& info) const;
71+
static std::string FormatHelp(const ConfigHelpInfo& info);
7372

7473
/**
7574
* @brief Format path listing as human-readable text
@@ -78,8 +77,8 @@ class ConfigSchemaExplorer {
7877
* @param parent_path Parent path for context (optional)
7978
* @return Formatted path list
8079
*/
81-
std::string FormatPathList(const std::map<std::string, std::string>& paths,
82-
const std::string& parent_path = "") const;
80+
static std::string FormatPathList(const std::map<std::string, std::string>& paths,
81+
const std::string& parent_path = "");
8382

8483
private:
8584
nlohmann::json schema_; // Parsed schema
@@ -99,7 +98,7 @@ class ConfigSchemaExplorer {
9998
* @param node Schema node
10099
* @return Help information
101100
*/
102-
ConfigHelpInfo ExtractHelpInfo(const std::string& path, const nlohmann::json& node) const;
101+
static ConfigHelpInfo ExtractHelpInfo(const std::string& path, const nlohmann::json& node);
103102

104103
/**
105104
* @brief Split path into components

‎src/server/CMakeLists.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,7 @@ target_link_libraries(mygramdb_server PUBLIC
3131
nlohmann_json::nlohmann_json
3232
Threads::Threads
3333
)
34+
35+
if(USE_MYSQL AND MYSQL_FOUND)
36+
target_link_libraries(mygramdb_server PUBLIC mygramdb_mysql)
37+
endif()

‎src/server/connection_acceptor.cpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,23 @@ bool ConnectionAcceptor::Start() {
6666
struct sockaddr_in address = {};
6767
std::memset(&address, 0, sizeof(address));
6868
address.sin_family = AF_INET;
69-
address.sin_addr.s_addr = INADDR_ANY;
7069
address.sin_port = htons(config_.port);
7170

71+
// Determine bind address (default to 0.0.0.0 for backward compatibility)
72+
in_addr bind_addr{};
73+
if (config_.host.empty() || config_.host == "0.0.0.0") {
74+
bind_addr.s_addr = INADDR_ANY;
75+
} else {
76+
if (inet_pton(AF_INET, config_.host.c_str(), &bind_addr) != 1) {
77+
last_error_ = "Invalid bind address: " + config_.host;
78+
spdlog::error("{}", last_error_);
79+
close(server_fd_);
80+
server_fd_ = -1;
81+
return false;
82+
}
83+
}
84+
address.sin_addr = bind_addr;
85+
7286
if (bind(server_fd_, ToSockaddr(&address), sizeof(address)) < 0) {
7387
last_error_ = "Failed to bind to port " + std::to_string(config_.port) + ": " + std::string(strerror(errno));
7488
spdlog::error("{}", last_error_);

‎src/server/handlers/admin_handler.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ std::string AdminHandler::HandleConfigHelp(const std::string& path) {
4949
if (path.empty()) {
5050
// Show top-level sections
5151
auto paths = explorer.ListPaths("");
52-
std::string result = explorer.FormatPathList(paths, "");
52+
std::string result = config::ConfigSchemaExplorer::FormatPathList(paths, "");
5353
return "+OK\n" + result;
5454
}
5555

@@ -59,7 +59,7 @@ std::string AdminHandler::HandleConfigHelp(const std::string& path) {
5959
return ResponseFormatter::FormatError("Configuration path not found: " + path);
6060
}
6161

62-
std::string result = explorer.FormatHelp(help_info.value());
62+
std::string result = config::ConfigSchemaExplorer::FormatHelp(help_info.value());
6363
return "+OK\n" + result;
6464

6565
} catch (const std::exception& e) {
@@ -70,6 +70,11 @@ std::string AdminHandler::HandleConfigHelp(const std::string& path) {
7070

7171
std::string AdminHandler::HandleConfigShow(const std::string& path) {
7272
try {
73+
if (ctx_.full_config == nullptr) {
74+
spdlog::warn("CONFIG SHOW requested but full configuration is not available");
75+
return ResponseFormatter::FormatError("Server configuration is not available");
76+
}
77+
7378
std::string result = config::FormatConfigForDisplay(*ctx_.full_config, path);
7479
return "+OK\n" + result;
7580
} catch (const std::exception& e) {
@@ -102,8 +107,7 @@ std::string AdminHandler::HandleConfigVerify(const std::string& filepath) {
102107
summary << ")";
103108
}
104109
summary << "\n";
105-
summary << " MySQL: " << test_config.mysql.user << "@" << test_config.mysql.host << ":"
106-
<< test_config.mysql.port;
110+
summary << " MySQL: " << test_config.mysql.user << "@" << test_config.mysql.host << ":" << test_config.mysql.port;
107111

108112
return "+OK\n" + summary.str();
109113

‎src/server/handlers/admin_handler.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class AdminHandler : public CommandHandler {
2626
* @param path Configuration path (empty for root)
2727
* @return Response string
2828
*/
29-
std::string HandleConfigHelp(const std::string& path);
29+
static std::string HandleConfigHelp(const std::string& path);
3030

3131
/**
3232
* @brief Handle CONFIG SHOW command
@@ -40,7 +40,7 @@ class AdminHandler : public CommandHandler {
4040
* @param filepath Path to configuration file
4141
* @return Response string
4242
*/
43-
std::string HandleConfigVerify(const std::string& filepath);
43+
static std::string HandleConfigVerify(const std::string& filepath);
4444
};
4545

4646
} // namespace mygramdb::server

‎src/server/handlers/debug_handler.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <sstream>
1111

1212
#include "utils/memory_utils.h"
13+
#include "utils/string_utils.h"
1314

1415
namespace mygramdb::server {
1516

0 commit comments

Comments
 (0)