feat: Custom request types, websocket rewrite, client custom responses

0x00002a commented 3 years ago

So this one is a bit of a mess. The websocket rewrite should probably have been its own PR but I didn't realise that until I was almost done with it. Anyway, deep breath:

Changes

Core

Filter concept introduced, the client and server each have speciailised versions of this
file filter added as a utility filter for reading or writing to/from a file with a request/response. Its more an example for now
some beast and asio stuff has been forwarded to the malloy namespace. error.hpp now holds a forwarded malloy::error_code

Client

Filter concept which requires:
- request_type
- header_type
- auto body_for(const header_type&) -> std::variant<...>
- void setup_body(const header_type&, T&) where T is one of the types in the variant returned by body_for (its a visitor)
- std::move_constructible
http(s)_request now
- Uses an async handler function which models a visitor over the return type of Filter::body_for
- Has an optional Filter param to provide a custom filter for the response
- Returns std::future instead of a request. It has an async handler
The async handler is a visitor over response<T>... where T is the bodies returned by setup_body for the filter (this means that with the default handler, it will always be response<string_body>)

Server

router::add now takes an optional argument that allows setting a request filter. Request filters are simpler than response filters based on the assumption that the server will expect 1 and only 1 type of body. This could be changed
request_filter concept which requires
- request_type (which must also be a single request type, NOT a variant)
- setup_body(const request_type::header_type&, request_type::body_type::value_type&)
- std::move_constructible

Websockets

The websocket API has been redesigned. The server and client parts have been merged into a single class which uses a stream type that does the handling of tls vs plain (although I don't think websocket tls is implemented yet?). The interface is based around providing a connection object which represents the websocket session and can be used to manage said session. The API is entirely async (apart from stop). It uses callbacks although really I would like to use coroutines here (but I lack experience with them).

Server

On the server side, the new interface passes the connection object as a shared_ptr directly to the handler, along with the request (as a string_body) that kicked it off, it is then up to the handler whether to call accept on the connection object.

Client

The client side is similar except its callback takes in error_code instead of the request. If the error code is truthy then the connection object will be nullptr, else its safe to use. The handshake is done before the handler is invoked so the handler should call either read or send.

I've added examples for the new filters and fixed the examples for the websockets so check those out for, well, examples. I will sort out the merge conflicts if/when this is ready for merging, as I think its going to require quite a bit of manual sorting out. I don't think its ready for merge currently, I want feedback on it and it could do with some more testing. Also needs some documentation, I haven't updated the doxygen comments or added new ones for the most part.

Closes: #17

0x00002a commented 3 years ago

Just rebasing this now, so don't pull it :p

0x00002a commented 3 years ago

Alright nvm, trying to rebase this is a nightmare. I just merged the upstream changes, I guess its ok anyway since it will be squashed on merge?

Tectu commented 3 years ago

Wow. I eeeh... will need some time to process this PR :-D

Tectu commented 3 years ago

I started reviewing this. So far, most seems to be related to coding style, can you modify this PR accordingly?

coding style fixes (I'll update coding_style.md accordingly):

No empty lines at the beginning of a file
Empty line at the end of a file
Opening curly-braces of a function body should be on a new line

Tectu commented 3 years ago

It would also be great if you could add some Doxygen to document the new classes/interfaces. Doesn't need to be complete at the moment - I'll put some effort into documentation once v0.1 is ready to be released. But at least class/interface documentation would be great :)

0x00002a commented 3 years ago

I've added some doxygen comments. Let me know if it needs more, or there are any inaccuracies (I've checked the HTML output but checking your own work and all that :p)

Tectu commented 3 years ago

Did you by any chance miss my other post on this PR regarding the coding style changes? :p

0x00002a commented 3 years ago

Ah, yes I did 🤦. I would prefer to do it via clang-format, but that will affect everything not just changes from this PR. I could localise it to only the files affected by the PR though (although I think thats most of them). Is that acceptable?

Tectu commented 3 years ago

Works for me. Alternatively we can also ignore the coding style for this PR (as an exception - because it's you 😛), I will finish the review process ignoring coding style and after that I fix/improve the .clang-format and we add this to the workflow.

What do you think?

0x00002a commented 3 years ago

This PR is already quite messy, it would be cleaner to merge then run clang-format over the whole codebase imo, since not all of that follows the current standard (at least mine doesn't :p). Once the codebase is formatted according to the .clang-format file then we stop anything being merged that doesn't follow it, that way we keep the main clean and ensure formatting PRs doesn't cause a pile of conflicts.

Tectu commented 3 years ago

Agreed.

0x00002a commented 3 years ago

oneshot has been renamed to accept_and_send, I've removed filter_helper since its unused

Tectu / malloy