Merge pull request #251 from rbrw/pdb-5645-call-handler-with-response

(PDB-5645) Call handler with response when :include-response set

CHANGELOG.md

@@ -1,3 +1,11 @@
+## 4.5.0
+
+* 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`:
 

project.clj

@@ -1,6 +1,6 @@
 (def jetty-version "9.4.51.v20230217")
 
-(defproject puppetlabs/trapperkeeper-webserver-jetty9 "4.4.4-SNAPSHOT"
+(defproject puppetlabs/trapperkeeper-webserver-jetty9 "4.5.0-SNAPSHOT"
   :description "A jetty9-based webserver implementation for use with the puppetlabs/trapperkeeper service framework."
   :url "https://github.com/puppetlabs/trapperkeeper-webserver-jetty9"
   :license {:name "Apache License, Version 2.0"

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.clj

@@ -0,0 +1,14 @@
+(ns puppetlabs.trapperkeeper.services.webserver.jetty9
+  "Currently only provides support for aliased keyword access for
+  clojure versions without :as-alias (before 1.11)."
+  (:require
+   [clojure.spec.alpha :as s])
+  (:import
+   (org.eclipse.jetty.server Response)))
+
+;; Currently just informational, i.e. not committing to support the
+;; spec alpha declaration publicly for now.
+
+(defn- response? [x] (instance? Response x))
+
+(s/def ::response response?)

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)