Refactor: Overhaul message handlers so they can be tested in isolation #29
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustybee42
changed the title
rustybee42
force-pushed
the
rb/handler-overhaul
branch
8 times, most recently
from
dd2bea2 to
51b0674
Compare
* Serializer now requires a preallocated buffer in form of a &mut [u8]. This moves the responsibility to allocate the required memory to the user. Before it used a dynamically growing buffer (which would reallocate internally if needed). This change is crucial for future RDMA support. * Since we now no longer use `BytesMut`, remove the bytes crate. * The buffers handled by the store have a fixed size of 4 MiB. This is apparently the maximum size of a BeeMsg (see `WORKER_BUF(IN|OUT)_SIZE` in `Worker.h`). C++ server code also uses these fixed size. * Generalize `msg_feature_flags` to a `Header` that can be modified from within the `Serializable` implementation and can be read out from within the `Deserializable` implementation. * Collect all BeeMsg (de)serialization functions in one module and provide functions for header, body and both combined. The split is required because depending on where the data comes from / goes to different actions need to be taken. This also provides an easy interface for potential external users to handle BeeMsges. * Remove the MsgBuf struct, instead just pass a `&mut [u8]` into the dispatcher. * Add documentation * Various small code cleanups in BeeSerde and other locations
…e logging Found by test_empty_udp_packets_server integration test. The test needs to be changed as well to adapt to the corrected log output (it was not really correct before)
* Accept a trait impl (`App*` / `AppAll`) to instead of conrete `Context` type on handlers * Implement a runtime `App` that satifies the above and is passed to the handlers at runtime * Add a `TestApp` implementation with rudimentary functionality for testing the handlers
* gRPC: get_nodes, delete_node, set_alias, get_pools * BeeMsg: get_nodes, authenticate_channel
rustybee42
force-pushed
the
rb/handler-overhaul
branch
from
51b0674 to
f92dbfc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
App) instead of concrete object (Context) in handlers that gives access to the needed resourcesRuntimeAppfor runtime andTestAppfor testing, implement a simple mockHow to review:
This PR looks like a lot, but most of the changes come from changing the file structure and moving (unchanged) code to new files. Thus commits should be reviewed separately, in order
ctx: Contextwithapp: &impl App, defining the trait, providing aRuntimeAppandTestAppimplementation (the latter being rudimentary for now) and modifying the calls from formerctx.toapp.to match the changes. Turn off whitespace diff for this commit as in some places the indentation of some blocks changed.Closes #4