Prefer testing through public APIs

design.md

@@ -9,6 +9,8 @@
 5. [Common Patterns](#common-patterns)
 6. [Reference](#reference)
 
+**See also:** [style.md](style.md) for comprehensive C++ coding standards and conventions.
+
 ---
 
 ## Project Overview
@@ -325,6 +327,7 @@ This write-side component is designed to integrate with:
 
 - **Configuration**: All configuration is TOML-based using `config.toml` (see `config.md`)
 - **Testing Strategy**: Run unit tests, benchmarks, and debug tools before submitting changes
+- **Test Design**: Prefer testing through public interfaces rather than implementation details (see `style.md` for detailed testing guidelines)
 - **Build System**: CMake generates gperf hash tables at build time; always use ninja
 - **Test Synchronization**:
   - **ABSOLUTELY NEVER use sleep(), std::this_thread::sleep_for(), or any timeout-based waiting in tests**

src/arena_allocator.hpp

@@ -83,7 +83,7 @@
  * @warning Do not share ArenaAllocator instances between threads. Use separate
  * instances per thread or per logical unit of work.
  */
-class ArenaAllocator {
+struct ArenaAllocator {
 private:
   /**
    * @brief Internal block structure for the intrusive linked list.

src/commit_request.hpp

@@ -60,7 +60,7 @@ struct Operation {
  * memory management and ownership. This class has no knowledge of any
  * specific serialization formats or encoding schemes.
  */
-class CommitRequest {
+struct CommitRequest {
 private:
   ArenaAllocator arena_;
   std::optional<std::string_view> request_id_;

src/commit_request_parser.hpp

@@ -13,7 +13,7 @@
  * and streaming parsing (for incremental data processing). This allows
  * efficient handling of network protocols where data may arrive in chunks.
  */
-class CommitRequestParser {
+struct CommitRequestParser {
 public:
   /**
    * @brief Status returned by streaming parse operations.

src/config.hpp

@@ -83,7 +83,7 @@ struct Config {
  * auto config2 = ConfigParser::parse_toml_string(toml);
  * ```
  */
-class ConfigParser {
+struct ConfigParser {
 public:
   /**
    * @brief Load configuration from a TOML file.

src/connection.cpp

@@ -1,11 +1,13 @@
 #include "connection.hpp"
-#include "server.hpp" // Need this for releaseBackToServer implementation
+
 #include <climits>
 #include <cstdio>
 #include <cstdlib>
 #include <errno.h>
 #include <limits.h>
 
+#include "server.hpp" // Need this for releaseBackToServer implementation
+
 // Static thread-local storage for iovec buffer
 static thread_local std::vector<struct iovec> g_iovec_buffer{IOV_MAX};
 

src/connection.hpp

@@ -1,7 +1,5 @@
 #pragma once
 
-#include "arena_allocator.hpp"
-#include "connection_handler.hpp"
 #include <cassert>
 #include <cstring>
 #include <deque>
@@ -10,6 +8,9 @@
 #include <sys/uio.h>
 #include <unistd.h>
 
+#include "arena_allocator.hpp"
+#include "connection_handler.hpp"
+
 #ifndef __has_feature
 #define __has_feature(x) 0
 #endif
@@ -39,7 +40,7 @@
  * private.
  */
 // Forward declaration
-class Server;
+struct Server;
 
 struct Connection {
   // No public constructor or factory method - only Server can create
@@ -305,7 +306,7 @@ struct Connection {
 
 private:
   // Server is a friend and can access all networking internals
-  friend class Server;
+  friend struct Server;
 
   /**
    * @brief Private constructor - only accessible by Server.

src/connection_handler.hpp

@@ -17,7 +17,7 @@ struct Connection;
  * The networking layer manages connection lifecycle, I/O multiplexing,
  * and efficient data transfer, while handlers focus purely on protocol logic.
  */
-class ConnectionHandler {
+struct ConnectionHandler {
 public:
   virtual ~ConnectionHandler() = default;
 

src/connection_registry.hpp

@@ -16,7 +16,7 @@ struct Connection;
  * allocated on-demand as connections are created.
  *
  */
-class ConnectionRegistry {
+struct ConnectionRegistry {
 public:
   /**
    * Initialize the connection registry.

src/http_handler.hpp

@@ -1,14 +1,16 @@
 #pragma once
 
-#include "ThreadPipeline.h"
+#include <memory>
+#include <string_view>
+#include <thread>
+
+#include <llhttp.h>
+
 #include "connection.hpp"
 #include "connection_handler.hpp"
 #include "perfetto_categories.hpp"
 #include "server.hpp"
-#include <llhttp.h>
-#include <memory>
-#include <string_view>
-#include <thread>
+#include "thread_pipeline.hpp"
 
 /**
  * HTTP routes supported by WeaselDB server.
@@ -60,14 +62,7 @@ struct HttpConnectionState {
  * HTTP/1.1 server implementation using llhttp for parsing.
  * Supports the WeaselDB REST API endpoints with enum-based routing.
  */
-class HttpHandler : public ConnectionHandler {
-  static constexpr int kFinalStageThreads = 2;
-  static constexpr int kLogSize = 12;
-  ThreadPipeline<std::unique_ptr<Connection>> pipeline{kLogSize,
-                                                       {kFinalStageThreads}};
-  std::vector<std::thread> finalStageThreads;
-
-public:
+struct HttpHandler : ConnectionHandler {
   HttpHandler() {
     for (int threadId = 0; threadId < kFinalStageThreads; ++threadId) {
       finalStageThreads.emplace_back([this, threadId]() {
@@ -124,6 +119,12 @@ public:
   static int onMessageComplete(llhttp_t *parser);
 
 private:
+  static constexpr int kFinalStageThreads = 2;
+  static constexpr int kLogSize = 12;
+  ThreadPipeline<std::unique_ptr<Connection>> pipeline{kLogSize,
+                                                       {kFinalStageThreads}};
+  std::vector<std::thread> finalStageThreads;
+
   // Route handlers
   void handleGetVersion(Connection &conn, const HttpConnectionState &state);
   void handlePostCommit(Connection &conn, const HttpConnectionState &state);

src/json_commit_request_parser.hpp

@@ -12,7 +12,7 @@
  * This parser uses the weaseljson library to parse JSON-formatted
  * commit requests into CommitRequest objects.
  */
-class JsonCommitRequestParser : public CommitRequestParser {
+struct JsonCommitRequestParser : CommitRequestParser {
 public:
   // Parser state
   enum class ParseState {

src/json_tokens.hpp

@@ -33,7 +33,7 @@ struct JsonToken {
  * }
  * ```
  */
-class PerfectHash {
+struct PerfectHash {
 public:
   /**
    * @brief Look up a JSON token by name using perfect hash.

src/server.cpp

@@ -1,6 +1,5 @@
 #include "server.hpp"
-#include "connection.hpp"
-#include "connection_registry.hpp"
+
 #include <csignal>
 #include <cstdio>
 #include <cstdlib>
@@ -17,6 +16,9 @@
 #include <unistd.h>
 #include <vector>
 
+#include "connection.hpp"
+#include "connection_registry.hpp"
+
 // Static thread-local storage for read buffer (used across different functions)
 static thread_local std::vector<char> g_read_buffer;
 

src/server.hpp

@@ -1,14 +1,15 @@
 #pragma once
 
-#include "config.hpp"
-#include "connection_handler.hpp"
-#include "connection_registry.hpp"
 #include <atomic>
 #include <memory>
 #include <span>
 #include <thread>
 #include <vector>
 
+#include "config.hpp"
+#include "connection_handler.hpp"
+#include "connection_registry.hpp"
+
 /**
  * High-performance multi-threaded server for handling network connections.
  *
@@ -32,8 +33,7 @@
  * - Prevention of accidental stack allocation that would break safety
  * guarantees
  */
-class Server : public std::enable_shared_from_this<Server> {
-public:
+struct Server : std::enable_shared_from_this<Server> {
   /**
    * Factory method to create a Server instance.
    *

style.md

@@ -0,0 +1,443 @@
+# WeaselDB C++ Style Guide
+
+This document describes the C++ coding style used in the WeaselDB project. These conventions ensure consistency, readability, and maintainability across the codebase.
+
+## Table of Contents
+
+1. [General Principles](#general-principles)
+2. [Naming Conventions](#naming-conventions)
+3. [File Organization](#file-organization)
+4. [Code Structure](#code-structure)
+5. [Memory Management](#memory-management)
+6. [Error Handling](#error-handling)
+7. [Documentation](#documentation)
+8. [Testing](#testing)
+
+---
+
+## General Principles
+
+### Language Standard
+- **C++20** is the target standard
+- Use modern C++ features: RAII, move semantics, constexpr, concepts where appropriate
+- Prefer standard library containers and algorithms over custom implementations
+
+### Data Types
+- **Almost always signed** - prefer `int`, `int64_t`, `size_t` over unsigned types except for:
+  - Bit manipulation operations
+  - Interfacing with APIs that require unsigned types
+  - Memory sizes where overflow is impossible (`size_t`, `uint32_t` for arena block sizes)
+  - Where defined unsigned overflow behavior (wraparound) is intentional and desired
+- **Almost always auto** - let the compiler deduce types except when:
+  - The type is not obvious from context (prefer explicit for clarity)
+  - Specific type requirements matter (numeric conversions, template parameters)
+  - Interface contracts need explicit types (public APIs, function signatures)
+- **Prefer uninitialized memory to default initialization** when using before initializing would be an error
+  - Valgrind will catch uninitialized memory usage bugs
+  - Avoid hiding logic errors with unnecessary zero-initialization
+  - Default initialization can mask bugs and hurt performance
+- **Floating point is for metrics only** - avoid `float`/`double` in core data structures and algorithms
+  - Use for performance measurements, statistics, and monitoring data
+  - Never use for counts, sizes, or business logic
+
+### Performance Focus
+- **Performance-first design** - optimize for the hot path
+- **Simple is fast** - find exactly what's necessary, strip away everything else
+- **Complexity must be justified with benchmarks** - measure performance impact before adding complexity
+- **Strive for 0% CPU usage when idle** - avoid polling, busy waiting, or unnecessary background activity
+- Use **inline functions** for performance-critical code (e.g., `allocate_raw`)
+- **Zero-copy operations** with `std::string_view` over string copying
+- **Arena allocation** for efficient memory management
+
+### Complexity Control
+- **Encapsulation is the main tool for controlling complexity**
+- **Header files define the interface** - they are the contract with users of your code
+- **Headers should be complete** - include everything needed to use the interface effectively:
+  - Usage examples in comments
+  - Preconditions and postconditions
+  - Thread safety guarantees
+  - Performance characteristics
+  - Ownership and lifetime semantics
+- **Do not rely on undocumented interface properties** - if it's not in the header, don't depend on it
+
+---
+
+## Naming Conventions
+
+### Variables and Functions
+- **snake_case** for all variables, functions, and member functions
+```cpp
+size_t used_bytes() const;
+void add_block(size_t size);
+uint32_t initial_block_size_;
+```
+
+### Structs
+- **PascalCase** for struct names
+- **Always use struct** - eliminates debates about complexity and maintains consistency
+- **Public members first, private after** - leverages struct's default public access
+- Use `private:` sections when encapsulation is needed
+```cpp
+struct ArenaAllocator {
+  // Public interface first
+  explicit ArenaAllocator(size_t initial_size = 1024);
+  void* allocate_raw(size_t size);
+
+private:
+  // Private members after
+  uint32_t initial_block_size_;
+  Block* current_block_;
+};
+```
+
+### Enums
+- **PascalCase** for enum class names
+- **PascalCase** for enum values (not SCREAMING_SNAKE_CASE)
+```cpp
+enum class Type {
+  PointRead,
+  RangeRead
+};
+
+enum class ParseState {
+  Root,
+  PreconditionsArray,
+  OperationObject
+};
+```
+
+### Constants and Macros
+- **snake_case** for constants
+- Avoid macros when possible; prefer `constexpr` variables
+```cpp
+static const WeaselJsonCallbacks json_callbacks;
+```
+
+### Member Variables
+- **Trailing underscore** for private member variables
+```cpp
+private:
+  uint32_t initial_block_size_;
+  Block *current_block_;
+```
+
+### Template Parameters
+- **PascalCase** for template type parameters
+```cpp
+template <typename T, typename... Args>
+template <typename U> struct rebind {};
+```
+
+---
+
+## File Organization
+
+### Header Files
+- Use **`#pragma once`** instead of include guards
+- **Never `using namespace std`** - always use fully qualified names for clarity and safety
+- **Include order:**
+  1. Corresponding header file (for .cpp files)
+  2. Standard library headers (alphabetical)
+  3. Third-party library headers
+  4. Project headers
+```cpp
+#pragma once
+
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <string_view>
+
+#include <simdutf.h>
+#include <weaseljson/weaseljson.h>
+
+#include "arena_allocator.hpp"
+#include "commit_request.hpp"
+
+// Never this:
+// using namespace std;
+
+// Always this:
+std::vector<int> data;
+std::unique_ptr<Parser> parser;
+```
+
+### Source Files
+- Include corresponding header first
+- Follow same include order as headers
+```cpp
+#include "json_commit_request_parser.hpp"
+
+#include <cassert>
+#include <charconv>
+#include <cstring>
+
+#include <simdutf.h>
+
+#include "json_token_enum.hpp"
+```
+
+---
+
+## Code Structure
+
+### Struct Design
+- **Move-only semantics** for resource-owning structs
+- **Explicit constructors** to prevent implicit conversions
+- **Delete copy operations** when inappropriate
+```cpp
+struct ArenaAllocator {
+  explicit ArenaAllocator(size_t initial_size = 1024);
+
+  // Copy construction is not allowed
+  ArenaAllocator(const ArenaAllocator &) = delete;
+  ArenaAllocator &operator=(const ArenaAllocator &) = delete;
+
+  // Move semantics
+  ArenaAllocator(ArenaAllocator &&other) noexcept;
+  ArenaAllocator &operator=(ArenaAllocator &&other) noexcept;
+
+private:
+  uint32_t initial_block_size_;
+  Block *current_block_;
+};
+```
+
+### Function Design
+- **Const correctness** - mark methods const when appropriate
+- **Parameter passing:**
+  - Pass by value for types ≤ 16 bytes (int, pointers, string_view, small structs)
+  - Pass by const reference for types > 16 bytes (containers, large objects)
+- **Return by value** for small types (≤ 16 bytes), **string_view** for zero-copy over strings
+- **noexcept specification** for move operations and non-throwing functions
+```cpp
+std::span<const Operation> operations() const { return operations_; }
+void process_data(std::string_view data);  // ≤ 16 bytes, pass by value
+void process_request(const CommitRequest& req);  // > 16 bytes, pass by reference
+ArenaAllocator(ArenaAllocator &&other) noexcept;
+```
+
+### Template Usage
+- **Template constraints** using static_assert for better error messages
+- **SFINAE** or concepts for template specialization
+```cpp
+template <typename T> T *construct(Args &&...args) {
+  static_assert(
+      std::is_trivially_destructible_v<T>,
+      "ArenaAllocator::construct requires trivially destructible types.");
+  // ...
+}
+```
+
+### Control Flow
+- **Early returns** to reduce nesting
+- **Range-based for loops** when possible
+```cpp
+if (size == 0) {
+  return nullptr;
+}
+
+for (auto &precondition : preconditions_) {
+  // ...
+}
+```
+
+---
+
+## Memory Management
+
+### Ownership & Allocation
+- **Arena allocators** for request-scoped memory with **STL allocator adapters**
+- **String views** pointing to arena-allocated memory for zero-copy
+- **Prefer unique_ptr** for exclusive ownership
+- **shared_ptr only if shared ownership is necessary** - most objects have single owners
+- **Factory patterns** for complex construction and ownership control
+```cpp
+// Static factory methods for complex objects requiring specific initialization
+auto server = Server::create(config, handler);  // Ensures shared_ptr semantics
+Block *block = Block::create(size, prev);       // Custom allocation + setup
+
+// Friend-based factories for access control
+struct Connection {
+  // Public interface first
+  void appendMessage(std::string_view data);
+  bool writeBytes();
+
+private:
+  Connection(/* args */);  // Private constructor
+  friend struct Server;   // Only Server can construct Connections
+};
+
+// Usage in Server
+auto conn = std::unique_ptr<Connection>(new Connection(args));
+```
+
+### Resource Management
+- **RAII** everywhere - constructors acquire, destructors release
+- **Move semantics** for efficient resource transfer
+- **Explicit cleanup** methods where appropriate
+```cpp
+~ArenaAllocator() {
+  while (current_block_) {
+    Block *prev = current_block_->prev;
+    std::free(current_block_);
+    current_block_ = prev;
+  }
+}
+```
+
+---
+
+## Error Handling
+
+### Error Reporting
+- **Return codes** for expected errors
+- **Exceptions** only for exceptional circumstances
+- **fprintf + abort()** for unrecoverable errors
+```cpp
+enum class ParseResult { Success, InvalidJson, MissingField };
+
+if (!memory) {
+  std::fprintf(stderr, "ArenaAllocator: Failed to allocate memory\n");
+  std::abort();
+}
+```
+
+### Assertions
+- Use **assert()** for debug-time checks
+- **Static assertions** for compile-time validation
+```cpp
+assert(current_block_ && "realloc called with non-null ptr but no current block");
+static_assert(std::is_trivially_destructible_v<T>, "Arena requires trivially destructible types");
+```
+
+---
+
+## Documentation
+
+### Doxygen Style
+- **/** for struct and public method documentation
+- **@brief** for short descriptions
+- **@param** and **@return** for function parameters
+- **@note** for important implementation notes
+- **@warning** for critical usage warnings
+```cpp
+/**
+ * @brief Type-safe version of realloc_raw for arrays of type T.
+ * @param ptr Pointer to the existing allocation
+ * @param old_size Size in number of T objects
+ * @param new_size Desired new size in number of T objects
+ * @return Pointer to reallocated memory
+ * @note Prints error to stderr and calls std::abort() if allocation fails
+ */
+template <typename T>
+T *realloc(T *ptr, uint32_t old_size, uint32_t new_size);
+```
+
+### Code Comments
+- **Explain why, not what** - code should be self-documenting
+- **Performance notes** for optimization decisions
+- **Thread safety** and ownership semantics
+```cpp
+// Uses O(1) accumulated counters for fast retrieval
+size_t total_allocated() const;
+
+// Only Server can create connections - no public constructor
+Connection(struct sockaddr_storage addr, int fd, int64_t id,
+           ConnectionHandler *handler, std::weak_ptr<Server> server);
+```
+
+---
+
+## Testing
+
+### Test Framework
+- **doctest** for unit testing
+- **TEST_CASE** and **SUBCASE** for test organization
+- **CHECK** for assertions (non-terminating)
+- **REQUIRE** for critical assertions (terminating)
+
+### Test Structure
+- **Descriptive test names** explaining the scenario
+- **SUBCASE** for related test variations
+- **Fresh instances** for each test to avoid state contamination
+```cpp
+TEST_CASE("ArenaAllocator basic allocation") {
+  ArenaAllocator arena;
+
+  SUBCASE("allocate zero bytes returns nullptr") {
+    void *ptr = arena.allocate_raw(0);
+    CHECK(ptr == nullptr);
+  }
+
+  SUBCASE("allocate single byte") {
+    void *ptr = arena.allocate_raw(1);
+    CHECK(ptr != nullptr);
+    CHECK(arena.used_bytes() >= 1);
+  }
+}
+```
+
+### Test Design Principles
+- **Prefer testing through public interfaces** - focus on observable behavior rather than implementation details
+- **Test the contract, not the implementation** - validate what the API promises to deliver
+- **Avoid testing private methods directly** - if private functionality needs testing, consider if it should be public or extracted
+- **Integration over isolation** - test components working together when practical
+- **Mock only external dependencies** - prefer real implementations for internal components
+```cpp
+// Good: Testing through public API
+TEST_CASE("Server accepts connections") {
+  auto config = Config::defaultConfig();
+  auto handler = std::make_unique<TestHandler>();
+  auto server = Server::create(config, std::move(handler));
+
+  // Test observable behavior - server can accept connections
+  auto result = connectToServer(server->getPort());
+  CHECK(result.connected);
+}
+
+// Avoid: Testing internal implementation details
+// TEST_CASE("Server creates epoll instance") { /* implementation detail */ }
+```
+
+### Test Synchronization
+- **NEVER use timeouts** or sleep-based synchronization
+- **Deterministic synchronization only:**
+  - Blocking I/O operations
+  - `condition_variable.wait()` (no timeout variant)
+  - `std::latch`, `std::barrier`, futures/promises
+  - RAII guards and resource management
+
+---
+
+## Build Integration
+
+### CMake Integration
+- **Generated code** (gperf hash tables) in build directory
+- **Ninja** as the preferred generator
+- **Export compile commands** for tooling support
+```bash
+cmake .. -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+```
+
+### Code Generation
+- **gperf** for perfect hash table generation
+- **Build-time generation** of token lookup tables
+- **Include generated headers** from build directory
+
+---
+
+## Style Enforcement
+
+### Consistency
+- Follow existing patterns in the codebase
+- Use the same style for similar constructs
+- Maintain consistency within each translation unit
+
+### Tools
+- **clang-format** configuration (when available)
+- **Static analysis** tools for code quality
+- **Address sanitizer** for memory safety testing
+
+This style guide reflects the existing codebase patterns and should be followed for all new code contributions to maintain consistency and readability.

tests/test_http_handler.cpp

@@ -68,7 +68,7 @@ TEST_CASE("HttpHandler route parsing") {
   }
 }
 
-TEST_CASE("HttpHandler route parsing functionality") {
+TEST_CASE("HttpHandler route parsing edge cases") {
   // Test just the static route parsing method since full integration testing
   // would require complex Connection setup with server dependencies
 

tests/test_server_connection_return.cpp

@@ -1,4 +1,4 @@
-#include "../src/ThreadPipeline.h"
+#include "../src/thread_pipeline.hpp"
 #include "config.hpp"
 #include "connection.hpp"
 #include "perfetto_categories.hpp"
@@ -35,7 +35,8 @@ public:
   }
 };
 
-TEST_CASE("Echo server with connection ownership transfer") {
+TEST_CASE(
+    "Server correctly handles connection ownership transfer via pipeline") {
   weaseldb::Config config;
   config.server.io_threads = 1;
   config.server.epoll_instances = 1;