Commit e339043
committed
Merge bitcoin/bitcoin#35829: http: Make class fields private and make HTTPResponse a struct
5e0d7a2 refactor: Drastically narrow scope of http_bitcoin namespace and rename it to bitcoin_http (Hodlinator)
8f9fd86 refactor: Make HTTPRemoteClient fields private (Hodlinator)
d72f67f refactor: Expose additional HTTPRemoteClient fields through accessors (Hodlinator)
10bbae3 refactor: Expose HTTPRemoteClient fields to tests through methods (Hodlinator)
5b06d90 refactor: Replace HTTPServer::MaybeDispatchRequestsFromClient() with HTTPRemoteClient::TryReadRequest() (Hodlinator)
a1183c0 refactor: Extract Send() and Receive() into HTTPRemoteClient from HTTPServer (Hodlinator)
6d9b61d refactor: Extract HTTPRemoteClient::MaybeDisconnect() from HTTPServer::DisconnectClients() (Hodlinator)
6fec8d6 refactor: Make HTTPRequest fields private (Hodlinator)
b8cd772 refactor: Make HTTPRequest::GetHeader() return saner optional type (Hodlinator)
e5be0dc refactor: Make HTTPResponse a struct since all fields are public (Hodlinator)
Pull request description:
The new HTTP server implementation in v32 has `HTTPServer` reaching into and modifying fields of `HTTPRemoteClient` and `HTTPRequest`. This PR encapsulates field data of the latter 2 types which enforces invariants and reduces cognitive load[^1]. Exposing data through accessor methods also implies adding lock annotations.
Commits:
* Makes `HTTPResponse` a struct since it is used that way. (bitcoin/bitcoin#35182 (comment)) [^2]
* `HTTPRequest`:
* Saner return type for `GetHeader()` (old type was mirroring the now removed libevent-wrapper and made later commits ugly).
* Make fields private.
* Simplifies boolean logic in `HTTPServer::DisconnectClients()`. (bitcoin/bitcoin#35182 (comment))
* Extraction of `HTTPServer` functions into `HTTPRemoteClient`:
Refactors `HTTPRemoteClient` to be more self-contained rather than having `HTTPServer` reach into the fields of other objects. (bitcoin/bitcoin#35182 (comment), bitcoin/bitcoin#35182 (comment))
* Severely narrows `http_bitcoin` namespace and renames it to `bitcoin_http` (bitcoin/bitcoin#35182 (comment))
Follow-up to #35182.
[^1]: Core Guidelines: C.9: Minimize exposure of members - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c9-minimize-exposure-of-members
[^2]: Core Guidelines: C.2: Use class if the class has an invariant; use struct if the data members can vary independently - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently
ACKs for top commit:
achow101:
ACK 5e0d7a2
janb84:
ACK 5e0d7a2
winterrdog:
tACK 5e0d7a2
Tree-SHA512: e1c5aa067538e31247ca74923e451038c90750ccc941ae16711dd976c8cd750bd1afaee6e4378aeee91440f7727955d9bfb32aa25a0a745613d0d771a674ebc87 files changed
Lines changed: 386 additions & 384 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
30 | 29 | | |
31 | 30 | | |
32 | 31 | | |
| |||
202 | 201 | | |
203 | 202 | | |
204 | 203 | | |
205 | | - | |
206 | | - | |
| 204 | + | |
| 205 | + | |
207 | 206 | | |
208 | 207 | | |
209 | 208 | | |
| |||
213 | 212 | | |
214 | 213 | | |
215 | 214 | | |
216 | | - | |
| 215 | + | |
217 | 216 | | |
218 | 217 | | |
219 | 218 | | |
| |||
0 commit comments