Add on_write_buffer_drained
This commit is contained in:
@@ -43,11 +43,37 @@ public:
|
||||
std::unique_ptr<Connection> &) {};
|
||||
|
||||
/**
|
||||
* Successfully wrote data on the connection.
|
||||
* Called when data has been successfully written to the connection.
|
||||
*
|
||||
* This is called during write operations to indicate progress, useful for:
|
||||
* - Streaming large responses (files, data feeds, etc.)
|
||||
* - Implementing backpressure for continuous data streams
|
||||
* - Progress monitoring for long-running transfers
|
||||
*
|
||||
* @param conn_ptr Connection that made write progress - handler can take
|
||||
* ownership
|
||||
* @note May be called from an arbitrary server thread.
|
||||
* @note Called during writes, not necessarily when buffer becomes empty
|
||||
*/
|
||||
virtual void on_write_progress(std::unique_ptr<Connection> &) {}
|
||||
|
||||
/**
|
||||
* Called when the connection's outgoing write buffer becomes empty.
|
||||
*
|
||||
* This indicates all queued messages have been successfully written
|
||||
* to the socket. Useful for:
|
||||
* - Resetting arena allocators safely
|
||||
* - Implementing keep-alive connection reuse
|
||||
* - Closing connections after final response
|
||||
* - Relieving backpressure conditions
|
||||
*
|
||||
* @param conn_ptr Connection with empty write buffer - handler can take
|
||||
* ownership
|
||||
* @note May be called from an arbitrary server thread.
|
||||
* @note Only called on transitions from non-empty → empty buffer
|
||||
*/
|
||||
virtual void on_write_buffer_drained(std::unique_ptr<Connection> &) {}
|
||||
|
||||
/**
|
||||
* Called when a new connection is established.
|
||||
*
|
||||
@@ -73,10 +99,11 @@ public:
|
||||
/**
|
||||
* @brief Called after a batch of connections has been processed.
|
||||
*
|
||||
* This hook is called after on_data_arrived or on_write_progress has been
|
||||
* called for each connection in the batch. The handler can take ownership of
|
||||
* the connections by moving the unique_ptr out of the span. Any connections
|
||||
* left in the span will remain owned by the server.
|
||||
* This hook is called after on_data_arrived, on_write_progress, or
|
||||
* on_write_buffer_drained has been called for each connection in the batch.
|
||||
* The handler can take ownership of the connections by moving the unique_ptr
|
||||
* out of the span. Any connections left in the span will remain owned by the
|
||||
* server.
|
||||
*
|
||||
* @param batch A span of unique_ptrs to the connections in the batch.
|
||||
*/
|
||||
|
||||
@@ -42,13 +42,12 @@ void HttpHandler::on_connection_closed(Connection &conn) {
|
||||
conn.user_data = nullptr;
|
||||
}
|
||||
|
||||
void HttpHandler::on_write_progress(std::unique_ptr<Connection> &conn_ptr) {
|
||||
void HttpHandler::on_write_buffer_drained(
|
||||
std::unique_ptr<Connection> &conn_ptr) {
|
||||
// Reset arena after all messages have been written for the next request
|
||||
if (conn_ptr->outgoingBytesQueued() == 0) {
|
||||
on_connection_closed(*conn_ptr);
|
||||
conn_ptr->reset();
|
||||
on_connection_established(*conn_ptr);
|
||||
}
|
||||
on_connection_closed(*conn_ptr);
|
||||
conn_ptr->reset();
|
||||
on_connection_established(*conn_ptr);
|
||||
}
|
||||
|
||||
void HttpHandler::on_post_batch(std::span<std::unique_ptr<Connection>> batch) {
|
||||
|
||||
@@ -102,7 +102,7 @@ struct HttpHandler : ConnectionHandler {
|
||||
void on_connection_closed(Connection &conn) override;
|
||||
void on_data_arrived(std::string_view data,
|
||||
std::unique_ptr<Connection> &conn_ptr) override;
|
||||
void on_write_progress(std::unique_ptr<Connection> &conn_ptr) override;
|
||||
void on_write_buffer_drained(std::unique_ptr<Connection> &conn_ptr) override;
|
||||
void on_post_batch(std::span<std::unique_ptr<Connection>> /*batch*/) override;
|
||||
|
||||
// Route parsing (public for testing)
|
||||
|
||||
@@ -485,6 +485,7 @@ void Server::process_connection_writes(std::unique_ptr<Connection> &conn,
|
||||
// Send immediately if we have outgoing messages (either from EPOLLOUT or
|
||||
// after reading)
|
||||
if ((events & EPOLLOUT) || ((events & EPOLLIN) && conn->hasMessages())) {
|
||||
bool had_messages = conn->hasMessages();
|
||||
bool error = conn->writeBytes();
|
||||
if (error) {
|
||||
conn.reset(); // Connection should be closed
|
||||
@@ -499,8 +500,17 @@ void Server::process_connection_writes(std::unique_ptr<Connection> &conn,
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if buffer became empty (transition from non-empty -> empty)
|
||||
if (had_messages && !conn->hasMessages()) {
|
||||
handler_.on_write_buffer_drained(conn);
|
||||
// If handler took ownership (conn is now null), return
|
||||
if (!conn) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Check if we should close the connection according to application
|
||||
if (!conn->hasMessages() && conn->closeConnection_) {
|
||||
if (!conn->hasMessages() && conn->shouldClose()) {
|
||||
conn.reset(); // Connection should be closed
|
||||
return;
|
||||
}
|
||||
|
||||
15
style.md
15
style.md
@@ -522,6 +522,21 @@ TEST_CASE("Server accepts connections") {
|
||||
// TEST_CASE("Server creates epoll instance") { /* implementation detail */ }
|
||||
```
|
||||
|
||||
### What NOT to Test
|
||||
|
||||
**Avoid testing language features and plumbing:**
|
||||
- Don't test that virtual functions dispatch correctly
|
||||
- Don't test that standard library types work (unique_ptr, containers, etc.)
|
||||
- Don't test basic constructor/destructor calls
|
||||
|
||||
**Test business logic instead:**
|
||||
- When does your code call hooks/callbacks and why?
|
||||
- What state transitions trigger behavior changes?
|
||||
- How does your code handle error conditions?
|
||||
- What promises does your API make to users?
|
||||
|
||||
**Ask: "Am I testing the C++ compiler or my application logic?"**
|
||||
|
||||
### Test Synchronization (Authoritative Rules)
|
||||
- **ABSOLUTELY NEVER use timeouts** (`sleep_for`, `wait_for`, etc.)
|
||||
- **Deterministic synchronization only:**
|
||||
|
||||
@@ -88,3 +88,35 @@ TEST_CASE("HttpHandler route parsing edge cases") {
|
||||
HttpRoute::GET_retention);
|
||||
}
|
||||
}
|
||||
|
||||
// Test helper to verify the new hook functionality
|
||||
struct MockConnectionHandler : public ConnectionHandler {
|
||||
bool write_progress_called = false;
|
||||
bool write_buffer_drained_called = false;
|
||||
|
||||
void on_write_progress(std::unique_ptr<Connection> &) override {
|
||||
write_progress_called = true;
|
||||
}
|
||||
|
||||
void on_write_buffer_drained(std::unique_ptr<Connection> &) override {
|
||||
write_buffer_drained_called = true;
|
||||
}
|
||||
};
|
||||
|
||||
TEST_CASE("ConnectionHandler hooks") {
|
||||
SUBCASE("on_write_buffer_drained hook exists") {
|
||||
MockConnectionHandler handler;
|
||||
|
||||
// Verify hooks are available and can be overridden
|
||||
CHECK_FALSE(handler.write_progress_called);
|
||||
CHECK_FALSE(handler.write_buffer_drained_called);
|
||||
|
||||
// Would normally be called by Server during write operations
|
||||
std::unique_ptr<Connection> null_conn;
|
||||
handler.on_write_progress(null_conn);
|
||||
handler.on_write_buffer_drained(null_conn);
|
||||
|
||||
CHECK(handler.write_progress_called);
|
||||
CHECK(handler.write_buffer_drained_called);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user