Given that this is a quite frequently overridden method, there are lots of copies of this parameter strings. Some of these are obviously related, like user, request (which actually is the request body), url, and params all describe attributes of the HTTP request being handled. So why not group them into a struct or class to simplify the signature?
Most handler implementations currently have unused parameters. For example, some only exist for the event stream handler, still, all implementations have to explicitly list them in their signature.
Related: when doing bigger changes to the interface there, one other improvement that comes to mind is how HttpServerConnection::StartStreaming() works: currently, to take control over the whole connection, this has to be called, but the underlying ASIO stream is still passed to every handler but it must not be used without calling StartStreaming(), otherwise, there's a good chance the connection ends up in a broken state. This could be improved by only exposing the underlying stream as a return value of the StartStreaming() method, similar to how it works in Go's net/http package.
The exact implementation is still up to discussion. For example: Should there be separate parameters for the request and response or a common parameter? Could that common parameter just be the existing HttpServerConnection object as it has to keep track of all that information already anyways?
HttpHandler::HandleRequest()currently takes quite a number of arguments, some of which also have quite complex types:https://github.com/Icinga/icinga2/blob/894d6aa290e83797d001fcc2887611b23707dbf9/lib/remote/httphandler.hpp#L29-L38
Given that this is a quite frequently overridden method, there are lots of copies of this parameter strings. Some of these are obviously related, like
user,request(which actually is the request body),url, andparamsall describe attributes of the HTTP request being handled. So why not group them into a struct or class to simplify the signature?Most handler implementations currently have unused parameters. For example, some only exist for the event stream handler, still, all implementations have to explicitly list them in their signature.
Related: when doing bigger changes to the interface there, one other improvement that comes to mind is how
HttpServerConnection::StartStreaming()works: currently, to take control over the whole connection, this has to be called, but the underlying ASIO stream is still passed to every handler but it must not be used without callingStartStreaming(), otherwise, there's a good chance the connection ends up in a broken state. This could be improved by only exposing the underlying stream as a return value of theStartStreaming()method, similar to how it works in Go'snet/httppackage.The exact implementation is still up to discussion. For example: Should there be separate parameters for the request and response or a common parameter? Could that common parameter just be the existing
HttpServerConnectionobject as it has to keep track of all that information already anyways?The following
HttpHandler::ProcessRequest()has a similar signature, however it's not overridden, so there aren't many copies of it, but maybe it could benefit from similar changes: https://github.com/Icinga/icinga2/blob/894d6aa290e83797d001fcc2887611b23707dbf9/lib/remote/httphandler.hpp#L41-L48