From de7d8e06ed9ff89c492dcd16fc839b81e1cd662f Mon Sep 17 00:00:00 2001 From: Martin Klepsch Date: Fri, 1 Nov 2019 14:44:51 +0100 Subject: [PATCH 1/3] make it more explicit that transformer is about HTML rendering, pass it down less --- src/cljdoc/server/pedestal.clj | 12 +++---- src/cljdoc/server/pedestal_util.clj | 53 +++++++++++++++-------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/cljdoc/server/pedestal.clj b/src/cljdoc/server/pedestal.clj index a27b3f8f..1958461d 100644 --- a/src/cljdoc/server/pedestal.clj +++ b/src/cljdoc/server/pedestal.clj @@ -90,13 +90,13 @@ `route-name` can be either `:artifact/index`, `:group/index` or `:cljdoc/index`." [store route-name] [(pu/coerce-body-conf - (fn html [{:keys [path-params]} content-type body] - (if (= content-type "text/html") + (fn html-render-fn [ctx] + (let [artifact-ent (-> ctx :request :path-params) + body (-> ctx :response :body)] (case route-name - :artifact/index (index-pages/artifact-index path-params body) - :group/index (index-pages/group-index path-params body) - :cljdoc/index (index-pages/full-index body)) - body))) + :artifact/index (index-pages/artifact-index artifact-ent body) + :group/index (index-pages/group-index artifact-ent body) + :cljdoc/index (index-pages/full-index body))))) (pu/negotiate-content #{"text/html" "application/edn" "application/json"}) (interceptor/interceptor {:name ::releases-loader diff --git a/src/cljdoc/server/pedestal_util.clj b/src/cljdoc/server/pedestal_util.clj index ae0f0c97..639e0488 100644 --- a/src/cljdoc/server/pedestal_util.clj +++ b/src/cljdoc/server/pedestal_util.clj @@ -10,37 +10,40 @@ [context] (get-in context [:request :accept :field] "text/html")) -(defn transform-content - ([body content-type] transform-content body content-type nil) - ([body content-type transformer] - (let [body' ((or transformer identity) body)] - (case content-type - "text/html" (str body') - "application/edn" (pr-str body') - "application/json" (json/generate-string body') - "application/x-suggestions+json" (json/generate-string body'))))) +(defn- render-body + [body content-type] + (case content-type + "text/html" (str body) + "application/edn" (pr-str body) + "application/json" (json/generate-string body) + "application/x-suggestions+json" (json/generate-string body))) -(defn coerce-to - ([response content-type] (coerce-to response content-type nil)) - ([response content-type transformer] - (-> response - (update :body transform-content content-type transformer) - (assoc-in [:headers "Content-Type"] content-type)))) +(defn- coerce-to + [response content-type] + (-> response + (update :body render-body content-type) + (assoc-in [:headers "Content-Type"] content-type))) (defn coerce-body-conf - "Coerce the `:response :body` to the `accepted-type`, optionally passing it - through the `transformer`, a `(fn [request content-type body] (modify body))` - See [[transform-content]]." - [transformer] + "Coerce the `:response :body` to the `accepted-type`, passing it through `html-render-fn` + if the resulting content type should be `text/html`. `html-render-fn` will receive the request + `context` as its only argument. + + Maybe HTML rendering could also be handled in a separate interceptor that uses [[accepted-type]] + to conditionally convert the data provided via `:body` to it's HTML representation." + [html-render-fn] (interceptor/interceptor {:name ::coerce-body :leave (fn [context] - (let [content-type (accepted-type context) - transformer' (when transformer - (partial transformer (:request context) content-type))] - (cond-> context - (nil? (get-in context [:response :headers "Content-Type"])) - (update-in [:response] coerce-to content-type transformer'))))})) + (if (get-in context [:response :headers "Content-Type"]) + context + (let [content-type (accepted-type context) + rendered-body (if (= content-type "text/html") + (html-render-fn context) + (-> context :response :body))] + (-> context + (assoc-in [:response :body] rendered-body) + (update :response coerce-to content-type)))))})) (def coerce-body (coerce-body-conf nil)) From 249e631b90c5aebcd933682b66fb5a2e385651d6 Mon Sep 17 00:00:00 2001 From: Martin Klepsch Date: Sat, 2 Nov 2019 14:47:55 +0100 Subject: [PATCH 2/3] rename let binding for version data in index page --- src/cljdoc/server/pedestal.clj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cljdoc/server/pedestal.clj b/src/cljdoc/server/pedestal.clj index 1958461d..e93cf3da 100644 --- a/src/cljdoc/server/pedestal.clj +++ b/src/cljdoc/server/pedestal.clj @@ -92,11 +92,11 @@ [(pu/coerce-body-conf (fn html-render-fn [ctx] (let [artifact-ent (-> ctx :request :path-params) - body (-> ctx :response :body)] + versions-data (-> ctx :response :body)] (case route-name - :artifact/index (index-pages/artifact-index artifact-ent body) - :group/index (index-pages/group-index artifact-ent body) - :cljdoc/index (index-pages/full-index body))))) + :artifact/index (index-pages/artifact-index artifact-ent versions-data) + :group/index (index-pages/group-index artifact-ent versions-data) + :cljdoc/index (index-pages/full-index versions-data))))) (pu/negotiate-content #{"text/html" "application/edn" "application/json"}) (interceptor/interceptor {:name ::releases-loader From 4c1799ae30aa4b62cc70b62ec38b68960a258d21 Mon Sep 17 00:00:00 2001 From: Martin Klepsch Date: Sat, 2 Nov 2019 14:48:58 +0100 Subject: [PATCH 3/3] fix rendering of build pages with refactored body coercion --- src/cljdoc/server/pedestal.clj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cljdoc/server/pedestal.clj b/src/cljdoc/server/pedestal.clj index e93cf3da..c03ab3b0 100644 --- a/src/cljdoc/server/pedestal.clj +++ b/src/cljdoc/server/pedestal.clj @@ -259,9 +259,7 @@ :enter (fn build-show-render [ctx] (if-let [build-info (->> ctx :request :path-params :id (build-log/get-build build-tracker))] - (if (= "text/html" (pu/accepted-type ctx)) - (pu/ok ctx (cljdoc.render.build-log/build-page build-info)) - (pu/ok ctx build-info)) + (pu/ok ctx build-info) ;; Not setting :response implies 404 response ctx))})) @@ -411,7 +409,9 @@ :search [(interceptor/interceptor {:name ::search :enter #(pu/ok-html % (render-search/search-page %))})] :shortcuts [(interceptor/interceptor {:name ::shortcuts :enter #(pu/ok-html % (render-meta/shortcuts))})] :sitemap [(sitemap-interceptor storage)] - :show-build [pu/coerce-body + :show-build [(pu/coerce-body-conf + (fn html-render [ctx] + (cljdoc.render.build-log/build-page (-> ctx :response :body)))) (pu/negotiate-content #{"text/html" "application/edn" "application/json"}) (show-build build-tracker)] :all-builds [(all-builds build-tracker)]