amphp / http-server

An advanced async HTTP server library for PHP, perfect for real-time apps and APIs with high concurrency demands.
https://amphp.org/http-server
MIT License
1.29k stars 101 forks source link

Add HttpServerSupervisor interface #340

Closed trowski closed 1 year ago

trowski commented 1 year ago

Closes #338. @cspray and @kelunik what do you think?

trowski commented 1 year ago

Should the HttpServer interface be renamed to HttpServerObserver? Maybe this is more appropriate if we are going to have two interfaces.

kelunik commented 1 year ago

Maybe HttpServerListener?

trowski commented 1 year ago

Thinking about it, either HttpServerObserver or HttpServerListener seems wrong. This is the event emitter, so HttpServerEventEmitter would be better. However that doesn't really cover getServers() either, so…

interface HttpServer extends HttpServerEventEmitter, HttpServerSupervisor {
    // public function getServers(): array; // Removed
}

interface HttpServerEventEmitter {
    public function onStart(...): void;
    public function onStop(...): void;
}

Thoughts?

cspray commented 1 year ago

@trowski that looks really good to me and should cover my use case quite well. I like splitting up the interfaces.

trowski commented 1 year ago

I removed getServers() from HttpServer, as requiring a list of SocketServer seems specific to the socket implementation and probably wouldn't apply to any implementer. I only found it used in tests where a SocketHttpServer instance was being used directly.

trowski commented 1 year ago

Decided to simplify and just add start and stop to HttpServer.