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

HTTP::Server vs ApplicationServer #13072

Open
wishdev opened this issue Feb 14, 2023 · 3 comments
Open

HTTP::Server vs ApplicationServer #13072

wishdev opened this issue Feb 14, 2023 · 3 comments

Comments

@wishdev
Copy link

wishdev commented Feb 14, 2023

The vast majority of HTTP::Server has no specific tie to HTTP.. The various socket binding and such is a standard need for application servers and would appear to be a good fit for another class.

Would there be any objection to splitting off the generic portions of the HTTP:Server into an ApplicationServer class and have HTTP:Server subclass that class? This would make it much easier to stand up a networked server.

@straight-shoota
Copy link
Member

This had previously been shortly mentioned in #5776 (comment). In that PR most of the generic application server logic was introduced to HTTP::Server.

So yeah, I think this could make sense. We'd need to develop an API for a generic implementation and how to integrate with the existing HTTP::Server API, ideally with little deprecations.

@HertzDevil
Copy link
Contributor

The client counterpart is #6011 (especially #6001)

@straight-shoota
Copy link
Member

#13080 is a concrete proposal to implement this. It extracts the generic parts of HTTP::Server into a parent type NetworkServer.

As a result, HTTP::Server is really nothing more than a thin proxy for RequestProcessor with little to no original implementation.
That makes me wonder if the abstractions are good.
There's the saying to favour composition over inheritance, and we should consider this here as well.

The function of HTTP::Server comes from hooking RequestProcessor up with NetworkServer. It does not override any behaviour of NetworkServer except implementing the handle_client method. And I don't see much reason to assume it would need more customizations later. So the inheritance HTTP::Server < NetworkServer is not very strongly motivated. The interface between the two is the single handle_client method. That can be solved many other ways.

A reasonable usage with no part of HTTP::Server could look like this:

request_processor = HTTP::Server::RequestProcessor.new do |context| 
   context.response.content_type = "text/plain"
   context.response.print "Hello world!"
end
http_server = NetworkServer.new(request_processor)
http_server.bind_tcp "0.0.0.0", 8080
http_server.listen

NetworkServer would store a reference to the processor and they'd need to agree on a method name to call for handling a connection (handle_client vs process), but that's minor details.

This is just to demonstrate the idea. Of course this could use some more polishing to make the API more convenient to use.

Take this as food for though. I'm not saying this is the way to go. Maybe keeping the inheritance relationship is actually best.
It certainly is the easiest to implement with backwards compatibility.

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

No branches or pull requests

4 participants