From 18653a3d5d11711afc9d93266ee46509fcf07c43 Mon Sep 17 00:00:00 2001 From: Sandro Covo Date: Sun, 21 Jan 2024 14:28:08 +0100 Subject: [PATCH] Change handler factory creation of HTTPClient (issue #102) The handler factory was supplied in the apply method of a client together with the Payload for the request. If the client already had a session for the new host, port and scheme, no new session was created and the supplied handler factory wasn't used. The new design makes it clear, that one client uses the same handler factory for all requests it makes. If different handler factories are needed, different clients need to be created. --- examples/httpget/httpget.pony | 12 +++++++++--- http/_test.pony | 4 ++-- http/_test_client.pony | 6 ++---- http/_test_client_error_handling.pony | 15 ++++++-------- http/http_client.pony | 28 +++++++++++++-------------- http/test_netssl_105_regression.pony | 4 ++-- 6 files changed, 35 insertions(+), 34 deletions(-) diff --git a/examples/httpget/httpget.pony b/examples/httpget/httpget.pony index b923462..aa5cd65 100644 --- a/examples/httpget/httpget.pony +++ b/examples/httpget/httpget.pony @@ -90,13 +90,19 @@ actor _GetWork env.exitcode(1) end - // The Client manages all links. - let client = HTTPClient(TCPConnectAuth(env.root), consume sslctx where keepalive_timeout_secs = timeout.u32()) // The Notify Factory will create HTTPHandlers as required. It is // done this way because we do not know exactly when an HTTPSession // is created - they can be re-used. let dumpMaker = recover val NotifyFactory.create(this) end + // The Client manages all links. + let client = HTTPClient( + TCPConnectAuth(env.root), + dumpMaker, + consume sslctx + where keepalive_timeout_secs = timeout.u32() + ) + try // Start building a GET request. let req = Payload.request("GET", url) @@ -119,7 +125,7 @@ actor _GetWork end // Submit the request - let sentreq = client(consume req, dumpMaker)? + let sentreq = client(consume req)? // Could send body data via `sentreq`, if it was a POST else diff --git a/http/_test.pony b/http/_test.pony index fd538bc..c793e76 100644 --- a/http/_test.pony +++ b/http/_test.pony @@ -435,13 +435,13 @@ class \nodoc\ iso _HTTPConnTest is UnitTest let url = URL.build(us)? h.log("url.string()=" + url.string()) let hf = _HTTPConnTestHandlerFactory(h) - client = recover iso HTTPClient(TCPConnectAuth(h.env.root)) end + client = recover iso HTTPClient(TCPConnectAuth(h.env.root), hf) end for _ in Range(0, loops) do let payload: Payload iso = Payload.request("GET", url) payload.set_length(0) try - (client as HTTPClient iso)(consume payload, hf)? + (client as HTTPClient iso)(consume payload)? end end else diff --git a/http/_test_client.pony b/http/_test_client.pony index c9d416c..5fdff8a 100644 --- a/http/_test_client.pony +++ b/http/_test_client.pony @@ -58,16 +58,14 @@ class \nodoc\ iso _ClientStreamTransferTest is UnitTest try let client = HTTPClient( TCPConnectAuth(_h.env.root), + _StreamTransferHandlerFactory(_h), None where keepalive_timeout_secs = U32(2) ) (let host, let port) = listen.local_address().name()? _h.log("connecting to server at " + host + ":" + port) let req = Payload.request("GET", URL.build("http://" + host + ":" + port + "/bla")?) - client( - consume req, - _StreamTransferHandlerFactory(_h) - )? + client(consume req)? else _h.fail("request building failed") end diff --git a/http/_test_client_error_handling.pony b/http/_test_client_error_handling.pony index 37a1704..6109430 100644 --- a/http/_test_client_error_handling.pony +++ b/http/_test_client_error_handling.pony @@ -53,6 +53,7 @@ class \nodoc\ iso _ConnectionClosedTest is UnitTest try let client = HTTPClient( TCPConnectAuth(_h.env.root), + _ConnectionClosedHandlerFactory(_h), None where keepalive_timeout_secs = U32(2) ) @@ -60,10 +61,7 @@ class \nodoc\ iso _ConnectionClosedTest is UnitTest let req = Payload.request("GET", URL.build("http://" + host + ":" + port + "/bla")?) req.add_chunk("CHUNK") - client( - consume req, - _ConnectionClosedHandlerFactory(_h) - )? + client(consume req)? else _h.fail("request building failed") end @@ -123,13 +121,14 @@ actor \nodoc\ _Connecter try let client = HTTPClient( TCPConnectAuth(_h.env.root), + _ConnectFailedHandlerFactory(_h), None where keepalive_timeout_secs = U32(2) ) let req = Payload.request("GET", URL.build("http://" + host + ":" + port' + "/bla")?) req.add_chunk("CHUNK") - client(consume req, _ConnectFailedHandlerFactory(_h))? + client(consume req)? else _h.fail("request building failed") end @@ -303,6 +302,7 @@ class \nodoc\ iso _SSLAuthFailedTest is UnitTest end let client = HTTPClient( TCPConnectAuth(_h.env.root), + _SSLAuthFailedHandlerFactory(_h), ssl_ctx where keepalive_timeout_secs = U32(2) ) @@ -310,10 +310,7 @@ class \nodoc\ iso _SSLAuthFailedTest is UnitTest "GET", URL.build("https://" + host + ":" + port + "/bla")?) req.add_chunk("CHUNK") - client( - consume req, - _SSLAuthFailedHandlerFactory(_h) - )? + client(consume req)? else _h.fail("request building failed") end diff --git a/http/http_client.pony b/http/http_client.pony index c46fe48..081abc6 100644 --- a/http/http_client.pony +++ b/http/http_client.pony @@ -12,17 +12,24 @@ class HTTPClient let _pipeline: Bool let _keepalive_timeout_secs: U32 let _sessions: Map[_HostService, _ClientConnection] = _sessions.create() + let _handlermaker: HandlerFactory val new create( auth: TCPConnectAuth, + handlermaker: HandlerFactory val, sslctx: (SSLContext | None) = None, pipeline: Bool = true, keepalive_timeout_secs: U32 = 0) => """ - Create the context in which all HTTP sessions will originate. + Create the context in which all HTTP sessions will originate. The `handlermaker` + is used to create the `HTTPHandler` that is applied with each received + payload after making a request. All requests made with one client are created + using the same handler factory, if you need different handlers for different + requests, you need to create different clients. Parameters: + - keepalive_timeout_secs: Use TCP Keepalive and check if the other side is down every `keepalive_timeout_secs` seconds. """ @@ -40,12 +47,9 @@ class HTTPClient _pipeline = pipeline _keepalive_timeout_secs = keepalive_timeout_secs + _handlermaker = handlermaker - fun ref apply( - request: Payload trn, - handlermaker: HandlerFactory val) - : Payload val ? - => + fun ref apply(request: Payload trn) : Payload val ? => """ Schedule a request on an HTTP session. If a new connection is created, a new instance of the application's Receive Handler will be created @@ -54,7 +58,7 @@ class HTTPClient This is useful in Stream and Chunked transfer modes, so that the application can follow up with calls to `Client.send_body`. """ - let session = _get_session(request.url, handlermaker)? + let session = _get_session(request.url)? let mode = request.transfer_mode request.session = session let valrequest: Payload val = consume request @@ -80,11 +84,7 @@ class HTTPClient end */ - fun ref _get_session( - url: URL, - handlermaker: HandlerFactory val) - : _ClientConnection ? - => + fun ref _get_session(url: URL) : _ClientConnection ? => """ Gets or creates an HTTP Session for the given URL. If a new session is created, a new Receive Handler instance is created too. @@ -100,10 +100,10 @@ class HTTPClient match url.scheme | "http" => _ClientConnection(_auth, hs.host, hs.service, - None, _pipeline, _keepalive_timeout_secs, handlermaker) + None, _pipeline, _keepalive_timeout_secs, _handlermaker) | "https" => _ClientConnection(_auth, hs.host, hs.service, - _sslctx, _pipeline, _keepalive_timeout_secs, handlermaker) + _sslctx, _pipeline, _keepalive_timeout_secs, _handlermaker) else error end diff --git a/http/test_netssl_105_regression.pony b/http/test_netssl_105_regression.pony index 6980cc2..73043e1 100644 --- a/http/test_netssl_105_regression.pony +++ b/http/test_netssl_105_regression.pony @@ -40,9 +40,9 @@ class \nodoc\ iso _NetSSL105RegressionTest is UnitTest try let url = URL.build("https://echo.sacovo.ch")? let auth = TCPConnectAuth(h.env.root) - let client = HTTPClient(auth) + let client = HTTPClient(auth, _NetSSL105RegressionHandlerFactory(h)) let payload = Payload.request("GET", url) - client.apply(consume payload, _NetSSL105RegressionHandlerFactory(h))? + client.apply(consume payload)? else h.fail("Unable to setup test") end