(PDB-5645) Add Response to request map when :include-response set

If :include-response is true in an add-ring-handler call, then include
the pending request Response instance in the request map as
:puppetlabs.trapperkeeper.services.webserver.jetty9/response so that
the handler can interrogate/manipulate the response directly if
desired.

PuppetDB intends to use this to detect client disconnects via the
SocketChannel (via the Response getHttpChannel) in order to halt a
potentially expensive query sooner.

CHANGELOG.md

@@ -1,3 +1,11 @@
+## pending
+
+* Add `:include-response` option to request that the pending response
+ instance be provided to
+ [`add-ring-handler`](./README.md#add-ring-handler) as a second
+ argument.
+ [(PDB-5645)](https://tickets.puppetlabs.com/browse/PDB-5645)
+
 ## 4.4.3
 * restore jdk 8 compatibliity
 

README.md

@@ -87,27 +87,41 @@ You may specify `""` as the value for `path` if you are only registering a singl
 handler and do not need to prefix the URL.
 
 There is also a three argument version of this function which takes these arguments:
-`[handler path options]`. `options` is a map containing three optional keys.
-
-The first is
-`:server-id`, which specifies which server you want to add the ring-handler to. If
-`:server-id` is specified, the ring handler will be added to the server with id
-`:server-id`. If no `:server-id` is specified, or the two argument version is called,
-the ring handler will be added to the default server. Calling the two-argument version or
-leaving out `:server-id` will not work in a multiserver set-up if no default server is specified.
-
-The second optional argument is `:redirect-if-no-trailing-slash`. When set to `true`,
-all requests made to the endpoint at which the ring-handler was registered will, if
-no trailing slash is present, return a 302 redirect response to the same URL but with a trailing slash
-added. If the option is set to `false`, no redirect will occur, and the request will be
-routed through to the registered handler. This option defaults to `false`.
-
-The third optional argument is `:normalize-request-uri`. When set to `true`, the
-URI made available to the ring handler request map via the `:uri` key will have
-been "normalized". See the [Request URI Normalization]
-(#request-uri-normalization) section for more information on the
-normalization process. When set to `false` (the default value), the raw path
-component from the HTTP request URI will be the value for the `:uri` key.
+`[handler path options]`. `options` is a map containing optional keys.
+
+* `:server-id`
+
+ This option specifies which server you want to add the ring-handler
+ to. If `:server-id` is specified, the ring handler will be added to
+ the server with id `:server-id`. If no `:server-id` is specified, or
+ the two argument version is called, the ring handler will be added
+ to the default server. Calling the two-argument version or leaving
+ out `:server-id` will not work in a multiserver set-up if no default
+ server is specified.
+
+* `:redirect-if-no-trailing-slash`
+
+ When set to `true`, all requests made to the endpoint at which the
+ ring-handler was registered will, if no trailing slash is present,
+ return a 302 redirect response to the same URL but with a trailing
+ slash added. If the option is set to `false`, no redirect will
+ occur, and the request will be routed through to the registered
+ handler. This option defaults to `false`.
+
+* `:normalize-request-uri`
+
+ When set to `true`, the URI made available to the ring handler
+ request map via the `:uri` key will have been "normalized". See the
+ [Request URI Normalization] (#request-uri-normalization) section for
+ more information on the normalization process. When set to `false`
+ (the default value), the raw path component from the HTTP request
+ URI will be the value for the `:uri` key.
+
+* `:include-response`
+
+ When set to `true`, the request will include the pending jetty
+ [`Response`](https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/Response.html)
+ instance as `:puppetlabs.trapperkeeper.services.webserver.jetty9/response`.
 
 Here's an example of how to use the `:WebserverService`:
 

src/puppetlabs/trapperkeeper/services/webrouting/webrouting_service_core.clj

@@ -26,6 +26,9 @@
 (def ContextHandlerOptions
  (dissoc (merge jetty9-core/ContextHandlerOptions RouteOption) :server-id))
 
+(def RingHandlerOptions
+ (dissoc (merge jetty9-core/RingHandlerOptions RouteOption) :server-id))
+
 (def ServletHandlerOptions
  (dissoc (merge jetty9-core/ServletHandlerOptions RouteOption) :server-id))
 
@@ -123,7 +126,7 @@
 (schema/defn ^:always-validate add-ring-handler!
  [context webserver-service
  svc :- (schema/protocol tk-services/Service)
- handler options :- CommonOptions]
+ handler options :- RingHandlerOptions]
  (let [{:keys [path opts]} (compute-common-elements context svc options)
  add-ring-handler (:add-ring-handler webserver-service)]
  (add-ring-handler handler path opts)))

src/puppetlabs/trapperkeeper/services/webserver/jetty9_core.clj

@@ -31,6 +31,7 @@
  [ring.util.codec :as codec]
  [clojure.string :as str]
  [clojure.tools.logging :as log]
+ [puppetlabs.trapperkeeper.services.webserver.jetty9 :as jetty9]
  [puppetlabs.trapperkeeper.services.webserver.jetty9-config :as config]
  [puppetlabs.trapperkeeper.services.webserver.experimental.jetty9-websockets :as websockets]
  [puppetlabs.trapperkeeper.services.webserver.normalized-uri-helpers
@@ -86,6 +87,9 @@
  (assoc CommonOptions (schema/optional-key :context-listeners) [ServletContextListener]
  (schema/optional-key :follow-links) schema/Bool))
 
+(def RingHandlerOptions
+ (assoc CommonOptions (schema/optional-key :include-response) schema/Bool))
+
 (def ServletHandlerOptions
  (assoc CommonOptions (schema/optional-key :servlet-init-params) {schema/Str schema/Str}))
 
@@ -448,10 +452,11 @@
 
 (defn- ring-handler
  "Returns an Jetty Handler implementation for the given Ring handler."
- [handler]
+ [handler include-response]
  (proxy [AbstractHandler] []
  (handle [_ ^Request base-request request response]
- (let [request-map (servlet/build-request-map request)
+ (let [request-map (cond-> (servlet/build-request-map request)
+ include-response (assoc :jetty9/response response))
  response-map (handler request-map)]
  (when response-map
  (servlet/update-servlet-response response response-map)
@@ -466,7 +471,7 @@
  options :- ProxyOptions]
  (let [custom-ssl-ctxt-factory (when (map? (:ssl-config options))
  (get-proxy-client-context-factory
-  (:ssl-config options)))
+ (:ssl-config options)))
  {:keys [request-buffer-size idle-timeout]} options]
  (proxy [ProxyServlet] []
  (rewriteTarget [req]
@@ -497,7 +502,7 @@
  (let [client (if custom-ssl-ctxt-factory
  (HttpClient. custom-ssl-ctxt-factory)
  (if-let [ssl-ctxt-factory (:ssl-context-factory
-  @(:state webserver-context))]
+ @(:state webserver-context))]
  (HttpClient. ssl-ctxt-factory)
  (HttpClient.)))]
  (when request-buffer-size
@@ -519,8 +524,8 @@
 
  (sendProxyRequest [req resp proxy-req]
  (if-let [callback-fn (:callback-fn options)]
- (callback-fn proxy-req req))
- (proxy-super sendProxyRequest req resp proxy-req))
+  (callback-fn proxy-req req))
+  (proxy-super sendProxyRequest req resp proxy-req))
 
  ;; The implementation of onResponseFailure is duplicated heavily from:
  ;; https://github.com/eclipse/jetty.project/blob/jetty-9.4.1.v20170120/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java#L624-L658
@@ -755,11 +760,11 @@
  handler :- (schema/pred ifn? 'ifn?)
  path :- schema/Str
  enable-trailing-slash-redirect? :- schema/Bool
- normalize-request-uri? :- schema/Bool]
- (let [handler
-   (normalized-uri-helpers/handler-maybe-wrapped-with-normalized-uri
- (ring-handler handler)
- normalize-request-uri?)
+ normalize-request-uri? :- schema/Bool
+  include-response :- (schema/maybe schema/Bool)]
+ (let [handler (normalized-uri-helpers/handler-maybe-wrapped-with-normalized-uri
+  (ring-handler handler include-response)
+  normalize-request-uri?)
  path (if (= "" path) "/" path)
  ctxt-handler (doto (ContextHandler. path)
  (.setHandler handler))]
@@ -1060,16 +1065,17 @@
  (nil? new-config) (start-server-multiple context config))))
 
 (schema/defn ^:always-validate add-ring-handler!
- [context handler path options :- CommonOptions]
- (let [server-id (:server-id options)
+ [context handler path options :- RingHandlerOptions]
+ (let [{:keys [include-response server-id]} options
  s (get-server-context context server-id)
  state (:state s)
  endpoint-map {:type :ring}
 
  enable-redirect (get options :redirect-if-no-trailing-slash false)
  normalize-request-uri (get options :normalize-request-uri false)]
  (register-endpoint! state endpoint-map path)
- (add-ring-handler s handler path enable-redirect normalize-request-uri)))
+ (add-ring-handler s handler path enable-redirect normalize-request-uri
+ include-response)))
 
 (schema/defn ^:always-validate add-websocket-handler!
  [context

test/clj/puppetlabs/trapperkeeper/services/webrouting/webrouting_service_handlers_test.clj

@@ -1,15 +1,20 @@
 (ns puppetlabs.trapperkeeper.services.webrouting.webrouting-service-handlers-test
- (:import (servlet SimpleServlet))
- (:require [clojure.test :refer :all]
- [schema.test :as schema-test]
- [puppetlabs.trapperkeeper.services :as tk-services]
- [puppetlabs.trapperkeeper.services.webrouting.webrouting-service :refer :all]
- [puppetlabs.trapperkeeper.services.webserver.jetty9-service :refer [jetty9-service]]
- [puppetlabs.trapperkeeper.app :refer [get-service]]
- [puppetlabs.trapperkeeper.testutils.webrouting.common :refer :all]
- [puppetlabs.trapperkeeper.testutils.bootstrap :refer [with-app-with-config]]
- [puppetlabs.trapperkeeper.testutils.logging :refer [with-test-logging]]
- [puppetlabs.trapperkeeper.testutils.webserver :as testutils]))
+ (:require
+ [clojure.test :refer :all]
+ [schema.test :as schema-test]
+ [puppetlabs.trapperkeeper.services :as tk-services]
+ [puppetlabs.trapperkeeper.services.webrouting.webrouting-service :refer :all]
+ [puppetlabs.trapperkeeper.services.webserver.jetty9 :as jetty9]
+ [puppetlabs.trapperkeeper.services.webserver.jetty9-service :refer [jetty9-service]]
+ [puppetlabs.trapperkeeper.app :refer [get-service]]
+ [puppetlabs.trapperkeeper.testutils.webrouting.common
+ :refer [default-options-for-https-client http-get webrouting-plaintext-config]]
+ [puppetlabs.trapperkeeper.testutils.bootstrap :refer [with-app-with-config]]
+ [puppetlabs.trapperkeeper.testutils.logging :refer [with-test-logging]]
+ [puppetlabs.trapperkeeper.testutils.webserver :as testutils])
+ (:import
+ (org.eclipse.jetty.server Response)
+ (servlet SimpleServlet)))
 
 (use-fixtures :once
  schema-test/validate-schemas
@@ -215,4 +220,20 @@
  (is (logged? #"^\{\"\/foo\" \[\{:type :ring}\]\}$"))
  (is (logged? #"^\{\"\/foo\" \[\{:type :ring}\]\}$" :info)))))))
 
-
+(deftest ring-handler-include-response
+ (with-test-logging
+ (with-app-with-config app
+ [jetty9-service webrouting-service test-dummy]
+ webrouting-plaintext-config
+ (let [handler-args (atom nil)]
+ (add-ring-handler (get-service app :WebroutingService)
+ (get-service app :TestDummy)
+ (fn [& args]
+ (reset! handler-args args)
+ {:status 200 :body "yep"})
+ {:include-response true})
+ (let [{:keys [status body]} (http-get "http://localhost:8080/foo")]
+ (is (= 200 status))
+ (is (= "yep" body))
+ (is (= 1 (count @handler-args)))
+ (is (instance? Response (-> @handler-args first :jetty9/response))))))))

test/clj/puppetlabs/trapperkeeper/services/webserver/jetty9_core_test.clj

@@ -37,6 +37,7 @@
  :body "I am a handler"})
  "/"
  true
+ false
  false)
  (is (= (count (.getHandlers handlers)) 1)))))
 

test/clj/puppetlabs/trapperkeeper/testutils/webserver.clj

@@ -27,7 +27,7 @@
  `(let [srv# (jetty9/start-webserver!
  (jetty9/initialize-context)
  (assoc ~config :port 0))
- _# (jetty9/add-ring-handler srv# ~app "/" true false)
+ _# (jetty9/add-ring-handler srv# ~app "/" true false false)
  ~port-var (-> (:server srv#)
  (.getConnectors)
  (first)