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

0x00002a · 2021-06-28T11:56:43Z

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::futuremalloy::error_code 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

…o feat-custom-requests

0x00002a · 2021-06-28T12:16:02Z

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

0x00002a · 2021-06-28T13:00:23Z

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 · 2021-06-28T13:14:23Z

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

Tectu · 2021-06-29T09:29:47Z

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 · 2021-06-29T09:34:36Z

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 :)

Ref #22, #28

0x00002a · 2021-06-29T15:26:54Z

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 · 2021-06-29T15:37:18Z

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

0x00002a · 2021-06-29T15:46:51Z

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 · 2021-06-29T15:50:30Z

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 · 2021-06-29T16:09:22Z

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 · 2021-06-29T16:10:10Z

Agreed.

Tectu · 2021-06-28T22:33:34Z

examples/client/http-file-download/main.cpp

+		spdlog::error(ec.message());
+		return EXIT_FAILURE;
+	}
+


Unnecessary empty line(s)

Tectu · 2021-06-29T16:11:17Z

.gitignore

+# End of https://www.toptal.com/developers/gitignore/api/clion+all
+/.vs


This shouldn't be part of this PR as it seems unrelated to the subject of this PR.

Tectu · 2021-06-29T16:14:34Z

examples/ws_handlers.hpp

+using ws_connection = std::shared_ptr<malloy::websocket::connection<isClient>>;
+
+
+template<bool isClient>


This could use some short comment describing it's functionality.

Tectu · 2021-06-29T16:14:44Z

examples/ws_handlers.hpp

+    });
+}
+
+template<bool isClient>


This could use some short comment describing it's functionality.

Tectu · 2021-06-29T16:14:50Z

examples/ws_handlers.hpp

+    conn->accept(req, [msg, conn] { conn->send(malloy::buffer(*msg), [msg](auto, auto) {}); });
+}
+
+template<bool isClient>


This could use some short comment describing it's functionality.

I think it should really be named ~~oneshot_send~~ confusing myself :p, it also accepts but still needs a rename

Tectu · 2021-06-29T16:18:13Z

lib/malloy/http/filters/file.hpp

+
+namespace malloy::http::filters {
+
+template<bool isRequest>


Missing class/struct documentation.

Tectu · 2021-06-29T16:18:58Z

lib/malloy/http/type_traits.hpp

+
+namespace malloy::http::concepts {
+
+template<typename B>


Missing indentation.

Tectu · 2021-06-29T16:20:11Z

lib/malloy/server/routing/endpoint_http_files.hpp

-
-    struct endpoint_http_files :
-        endpoint_http
+    class endpoint_http_files :


Missing class documentation.

Tectu · 2021-06-29T16:22:36Z

test/test_suites/components/uri.cpp

@@ -2,6 +2,8 @@

 #include <malloy/http/uri.hpp>

+#include <iostream>


Is this really necessary? This include was added to test.hpp:

malloy/test/test.hpp

Line 12 in a20816b

#include <iostream>

Tectu · 2021-06-29T16:22:53Z

test/test_suites/components/websockets.cpp

+
+		stop_token.wait();
+		CHECK(server_recieved);
+


Lots of empty lines.

0x00002a · 2021-06-29T18:34:58Z

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

0x00002a added 30 commits June 19, 2021 19:22

chore: Setup templated response

8c9b43a

chore: Switch generator file to use file body

dbfda00

fix: Dependency loop between router and connection

60933ac

fix: Dependency loop between router and connection

af2ece1

feat: Switched to variant to allow multiple response types

20abb18

fix: generator::file returning multiple types

5e1710b

fix: session manager

a56bb4d

fix: Redefinition error for make_websocket_connection

bb55212

fix: client compile

7b1036f

fix: router::add passing lambdas

42e6633

feat: generator::file uses file_body in implementation

95270ce

chore: Added missing header

cb54a16

chore: Adding handler

be454a9

chore: Make request templated

9290037

chore: Setting up templated request

73995ce

chore: Setting up routers

d36d8f7

chore: Setup regex endpoint

4323780

chore: Save

b249dcd

chore: Merge branch 'feat-regex-captures' into feat-custom-requests

cd1a13d

fix: Form compile

67a6dae

chore: Fixing build

9bf081c

chore: fixing build

93cea42

chore: Fixing build

c3cc69f

chore: Merge upstream/main

180bfb8

fix: Apply Tectu#20

a33b380

chore: changed ordering of body() and fixes for router

d3769fa

chore: Remove request requirement to send_message

1c44f88

chore: Switch to use setup callabck

5116b85

chore: Fix body()

81649de

chore: Compiles

b75c322

0x00002a added 6 commits June 27, 2021 22:24

Merge branch 'feat-custom-requests' of github.com:0x00002a/malloy int…

33b2399

…o feat-custom-requests

fix: websocket examples build

517638f

fix: websocket examples and helper code

47baecb

fix: linux build + link

67a47c4

fix: generator::file only accepting string_body requests

809990e

Merge branch 'feat-custom-requests' of github.com:0x00002a/malloy int…

0864324

…o feat-custom-requests

chore: merge upstream/main

2dc6009

Tectu added a commit that referenced this pull request Jun 29, 2021

Update coding_style.md

a20816b

Ref #22, #28

0x00002a added 3 commits June 29, 2021 16:07

chore: Add and update docs

2094b42

chore: Move requires bc doxygen doesn't parse it right

2824a42

chore: fix doxygen usage example for stream

5734dd5

Tectu requested changes Jun 29, 2021

View reviewed changes

0x00002a added 3 commits June 29, 2021 19:18

chore: misc cleanup

257b875

chore: add more docs to file filter and namespace

b033000

chore: misc cleanup of concepts

2b23759

Tectu approved these changes Jun 29, 2021

View reviewed changes

Tectu merged commit ab37991 into Tectu:main Jun 29, 2021

0x00002a deleted the feat-custom-requests branch July 6, 2021 23:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

0x00002a commented Jun 28, 2021 •

edited

Loading

0x00002a commented Jun 28, 2021

0x00002a commented Jun 28, 2021 •

edited

Loading

Tectu commented Jun 28, 2021

Tectu commented Jun 29, 2021

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021 •

edited

Loading

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021

Tectu commented Jun 29, 2021

Tectu Jun 28, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

0x00002a Jun 29, 2021 •

edited

Loading

Tectu Jun 29, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

Tectu Jun 29, 2021

0x00002a commented Jun 29, 2021

		# End of https://www.toptal.com/developers/gitignore/api/clion+all
		/.vs

		using ws_connection = std::shared_ptr<malloy::websocket::connection<isClient>>;


		template<bool isClient>

		@@ -2,6 +2,8 @@

		#include <malloy/http/uri.hpp>

		#include <iostream>


		namespace malloy::http::concepts {

		template<typename B>

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

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

Conversation

0x00002a commented Jun 28, 2021 • edited Loading

Changes

Core

Client

Server

Websockets

Server

Client

0x00002a commented Jun 28, 2021

0x00002a commented Jun 28, 2021 • edited Loading

Tectu commented Jun 28, 2021

Tectu commented Jun 29, 2021

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021 • edited Loading

Tectu commented Jun 29, 2021

0x00002a commented Jun 29, 2021

Tectu commented Jun 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0x00002a Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0x00002a commented Jun 29, 2021

0x00002a commented Jun 28, 2021 •

edited

Loading

0x00002a commented Jun 28, 2021 •

edited

Loading

0x00002a commented Jun 29, 2021 •

edited

Loading

0x00002a Jun 29, 2021 •

edited

Loading