Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Responses with any body type for regex route #14

Merged
merged 12 commits into from
Jun 21, 2021

Conversation

0x00002a
Copy link
Contributor

@0x00002a 0x00002a commented Jun 20, 2021

This implements #13. Its not the most pretty implementation and I welcome any feedback on it or alternatives. All tests pass on both windows (msvc v142) and linux (fedora 33/gcc 1.11.1). On MSVC it needs /bigobj to compile router.

Note this is, unfortunately, a breaking change. Most non-trivial code that currently uses this library will not compile against this patch. It is a compile error though, and is easy enough to fix, mostly just adding <> to the response and making some handlers return std::variant<...>. Again, feedback on this is welcome.

I've done some basic testing of this server-side but I don't have a project currently that uses this client-side. I also haven't tested it with tls.

Notable changes:

  • connection now accesses router through an interface, this was needed to break the include dependency cycle
  • router no longer uses a template for connection, it uses a variant since there are only 4 it can be
  • concepts support is now required. Seems latest msvc and gcc both support them enough to compile
  • response is now templated
  • generator::file now returns std::variant<...file_body,...string_body...> which is either an error response (as a string body) or the file
  • router::add is now templated and will accept any function that takes in const request_type& and returns either a variant containing responses or a single response type (which is wrapped before being passed on). This allows mostly seamless compatability with the exception being handlers that need to return multiple types of response (which now need to specifiy the variant explicitly, which is kind of horrible, maybe should be a using with only the body types passed?)
  • endpoint_http_regex and endpoint_http_files use some horrible spagetti code to handle the non-string bodies

closes: #13

@Tectu
Copy link
Owner

Tectu commented Jun 20, 2021

I'll have a detailed look at this tomorrow (hopefully). Glancing over your summary this seems like a very good extension.

I don't mind that this introduces breaking changes as there is no initial release yet anyway. This is currently under heavy development. However, I do believe that it should be doable to get 0.1.0 out in a month or two.

@Tectu Tectu closed this Jun 20, 2021
@Tectu Tectu reopened this Jun 20, 2021
@Tectu
Copy link
Owner

Tectu commented Jun 20, 2021

Your PR includes the changes for MSVC builds.
Can you update the PR to only exclude the changes actually outlined by he PR?

@Tectu Tectu merged commit 9160424 into Tectu:main Jun 21, 2021
@Tectu
Copy link
Owner

Tectu commented Jun 22, 2021

Hum.... I can't build anymore, even with -Wa,-mbig-obj and -O3 on Windows using MinGW:

[ 91%] Building CXX object lib/malloy/CMakeFiles/malloy-objs.dir/controller.cpp.obj
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\malloy-objs.dir\client\controller.cpp.obj: section .pdata$_ZNK5boost4asio20basic_deadline_timerINS_10posix_time5ptimeENS0_11time_traitsIS3_EENS0_9execution12any_executorIJNS6_12context_as_tIRNS0_17execution_contextEEENS6_6detail8blocking7never_tILi0EEENS6_11prefer_onlyINSD_10possibly_tILi0EEEEENSG_INSC_16outstanding_work9tracked_tILi0EEEEENSG_INSK_11untracked_tILi0EEEEENSG_INSC_12relationship6fork_tILi0EEEEENSG_INSR_14continuation_tILi0EEEEEEEEE19initiate_async_waitclINS0_3ssl6detail5io_opINS_5beast12basic_streamINS0_2ip3tcpESY_NS15_21unlimited_rate_policyEEENS13_8write_opINS0_14mutable_bufferEEENS15_11flat_streamINS12_6streamIS1A_EEE3ops8write_opINS15_4http6detail13write_some_opINS1L_8write_opINS1L_12write_msg_opINS15_6detail18bind_front_wrapperIMN6malloy6client4http10connectionINS1T_14connection_tlsEEEFvRKNS_6system10error_codeEyEJSt10shared_ptrIS1V_EEEENS15_10ssl_streamIS1A_EELb1ENS1K_17basic_string_bodyIcSt11char_traitsIcESaIcEEENS1K_12basic_fieldsIS2B_EEEES27_NS1L_18serializer_is_doneELb1ES2C_S2E_EES27_Lb1ES2C_S2E_EEEEEEEEvOT_: string table overflow at offset 10000913
C:\Users\joel\AppData\Local\Temp\cc8Bnytz.s: Assembler messages:
C:\Users\joel\AppData\Local\Temp\cc8Bnytz.s: Fatal error: CMakeFiles\malloy-objs.dir\client\controller.cpp.obj: file too big
mingw32-make[3]: *** [lib\malloy\CMakeFiles\malloy-objs.dir\build.make:82: lib/malloy/CMakeFiles/malloy-objs.dir/client/controller.cpp.obj] Error 1
mingw32-make[3]: *** Waiting for unfinished jobs....
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\malloy-objs.dir\server\http\connection\connection_detector.cpp.obj: too many sections (102336)
C:\Users\joel\AppData\Local\Temp\ccAozzlN.s: Assembler messages:
C:\Users\joel\AppData\Local\Temp\ccAozzlN.s: Fatal error: can't write 142 bytes to section .text of CMakeFiles\malloy-objs.dir\server\http\connection\connection_detector.cpp.obj: 'file too big'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\malloy-objs.dir\server\http\connection\connection_detector.cpp.obj: too many sections (102336)
C:\Users\joel\AppData\Local\Temp\ccAozzlN.s: Fatal error: CMakeFiles\malloy-objs.dir\server\http\connection\connection_detector.cpp.obj: file too big
mingw32-make[3]: *** [lib\malloy\CMakeFiles\malloy-objs.dir\build.make:166: lib/malloy/CMakeFiles/malloy-objs.dir/server/http/connection/connection_detector.cpp.obj] Error 1
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\malloy-objs.dir\server\routing\router.cpp.obj: too many sections (118560)
C:\Users\joel\AppData\Local\Temp\ccceCHES.s: Assembler messages:
C:\Users\joel\AppData\Local\Temp\ccceCHES.s: Fatal error: can't write 111 bytes to section .text of CMakeFiles\malloy-objs.dir\server\routing\router.cpp.obj: 'file too big'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.3.0/../../../../x86_64-w64-mingw32/bin/as.exe: CMakeFiles\malloy-objs.dir\server\routing\router.cpp.obj: too many sections (118560)
C:\Users\joel\AppData\Local\Temp\ccceCHES.s: Fatal error: CMakeFiles\malloy-objs.dir\server\routing\router.cpp.obj: file too big
mingw32-make[3]: *** [lib\malloy\CMakeFiles\malloy-objs.dir\build.make:180: lib/malloy/CMakeFiles/malloy-objs.dir/server/routing/router.cpp.obj] Error 1
mingw32-make[2]: *** [CMakeFiles\Makefile2:731: lib/malloy/CMakeFiles/malloy-objs.dir/all] Error 2
mingw32-make[1]: *** [CMakeFiles\Makefile2:793: examples/client/http-plain/CMakeFiles/malloy-example-client-http-plain.dir/rule] Error 2
mingw32-make: *** [Makefile:162: malloy-example-client-http-plain] Error 2

@0x00002a
Copy link
Contributor Author

The tests don't add those flags, could that be the problem? I'm not very familier with object libraries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non string_body responses
2 participants