Skip to content
Snippets Groups Projects
Commit add12c47 authored by Ryan Senior's avatar Ryan Senior
Browse files

Immediately write pulse card graphs to disk 

Previously images in pulse emails were serialized to bytes and the
bytes were kept in map stored in an atom. After all of the graphs were
serialized, they were written to disk and the file handle was passed
to the email library.

This commit immediately writes the file to disk and returns a
URL. This will no longer hold onto the memory need for all of the
cards and can be GC'd right away. This also avoids writing static
images like the no results image to disk evertime, rather than just
reading it out of the JAR.
parent a3a9c8f1
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/pulse.clj
+ 2
− 2
View file @ add12c47
......@@ -114,7 +114,7 @@
:card-id id})]
{:status 200, :body (html [:html [:body {:style "margin: 0;"} (binding [render/*include-title* true
render/*include-buttons* true]
(render/render-pulse-card (p/defaulted-timezone card) card result))]])}))
(render/render-pulse-card-for-display (p/defaulted-timezone card) card result))]])}))
(api/defendpoint GET "/preview_card_info/:id"
"Get JSON object containing HTML rendering of a `Card` with ID and other information."
......@@ -126,7 +126,7 @@
data (:data result)
card-type (render/detect-pulse-card-type card data)
card-html (html (binding [render/*include-title* true]
(render/render-pulse-card (p/defaulted-timezone card) card result)))]
(render/render-pulse-card-for-display (p/defaulted-timezone card) card result)))]
{:id id
:pulse_card_type card-type
:pulse_card_html card-html
......
src/metabase/email.clj
+ 12
− 11
View file @ add12c47
......@@ -6,20 +6,21 @@
[postal
[core :as postal]
[support :refer [make-props]]]
[puppetlabs.i18n.core :refer [tru trs]]
[schema.core :as s])
(:import javax.mail.Session))
;;; CONFIG
;; TODO - smtp-port should be switched to type :integer
(defsetting email-from-address "Email address you want to use as the sender of Metabase." :default "notifications@metabase.com")
(defsetting email-smtp-host "The address of the SMTP server that handles your emails.")
(defsetting email-smtp-username "SMTP username.")
(defsetting email-smtp-password "SMTP password.")
(defsetting email-smtp-port "The port your SMTP server uses for outgoing emails.")
(defsetting email-from-address (tru "Email address you want to use as the sender of Metabase.") :default "notifications@metabase.com")
(defsetting email-smtp-host (tru "The address of the SMTP server that handles your emails."))
(defsetting email-smtp-username (tru "SMTP username."))
(defsetting email-smtp-password (tru "SMTP password."))
(defsetting email-smtp-port (tru "The port your SMTP server uses for outgoing emails."))
(defsetting email-smtp-security
"SMTP secure connection protocol. (tls, ssl, starttls, or none)"
:default "none"
(tru "SMTP secure connection protocol. (tls, ssl, starttls, or none)")
:default (tru "none")
:setter (fn [new-value]
(when-not (nil? new-value)
(assert (contains? #{"tls" "ssl" "none" "starttls"} new-value)))
......@@ -71,7 +72,8 @@
{:style/indent 0}
[{:keys [subject recipients message-type message]} :- EmailMessage]
(when-not (email-smtp-host)
(throw (Exception. "SMTP host is not set.")))
(let [^String msg (tru "SMTP host is not set.")]
(throw (Exception. msg))))
;; Now send the email
(send-email! (smtp-settings)
{:from (email-from-address)
......@@ -100,8 +102,7 @@
(try
(send-message-or-throw! msg-args)
(catch Throwable e
(println "Failed to send email:" e)
(log/warn e "Failed to send email")
(log/warn e (trs "Failed to send email"))
{:error :ERROR
:message (.getMessage e)})))
......@@ -126,7 +127,7 @@
{:error :SUCCESS
:message nil}
(catch Throwable e
(log/error "Error testing SMTP connection:" (.getMessage e))
(log/error e (trs "Error testing SMTP connection"))
{:error :ERROR
:message (.getMessage e)})))
......
src/metabase/email/messages.clj
+ 15
− 42
View file @ add12c47
......@@ -178,55 +178,33 @@
:message-type :html
:message message-body)))
;; HACK: temporary workaround to postal requiring a file as the attachment
(defn- write-byte-array-to-temp-file
[^bytes img-bytes]
(u/prog1 (doto (File/createTempFile "metabase_pulse_image_" ".png")
.deleteOnExit)
(with-open [fos (FileOutputStream. <>)]
(.write fos img-bytes))))
(defn- hash-bytes
"Generate a hash to be used in a Content-ID"
[^bytes img-bytes]
(Math/abs ^Integer (Arrays/hashCode img-bytes)))
(defn- render-image [images-atom, ^bytes image-bytes]
(let [content-id (str (hash-bytes image-bytes) "@metabase")]
(if-not (contains? @images-atom content-id)
(swap! images-atom assoc content-id image-bytes))
(str "cid:" content-id)))
(defn- write-image-content [[content-id bytes]]
(defn- make-message-attachment [[content-id url]]
{:type :inline
:content-id content-id
:content-type "image/png"
:content (write-byte-array-to-temp-file bytes)})
:content url})
(defn- pulse-context [body pulse]
(defn- pulse-context [pulse]
(merge {:emailType "pulse"
:pulse (html body)
:pulseName (:name pulse)
:sectionStyle render/section-style
:colorGrey4 render/color-gray-4
:logoFooter true}
(random-quote-context)))
(defn- render-message-body
[message-template message-context images]
(vec (cons {:type "text/html; charset=utf-8" :content (stencil/render-file message-template message-context)}
(map write-image-content images))))
(defn- render-message-body [message-template message-context timezone results]
(let [rendered-cards (binding [render/*include-title* true]
;; doall to ensure we haven't exited the binding before the valures are created
(doall (map #(render/render-pulse-section timezone %) results)))
message-body (assoc message-context :pulse (html (vec (cons :div (map :content rendered-cards)))))
attachments (apply merge (map :attachments rendered-cards))]
(vec (cons {:type "text/html; charset=utf-8" :content (stencil/render-file message-template message-body)}
(map make-message-attachment attachments)))))
(defn render-pulse-email
"Take a pulse object and list of results, returns an array of attachment objects for an email"
[timezone pulse results]
(let [images (atom {})
body (binding [render/*include-title* true
render/*render-img-fn* (partial render-image images)]
(vec (cons :div (for [result results]
(render/render-pulse-section timezone result)))))]
(render-message-body "metabase/email/pulse" (pulse-context body pulse) (seq @images))))
(render-message-body "metabase/email/pulse" (pulse-context pulse) timezone results))
(defn pulse->alert-condition-kwd
"Given an `ALERT` return a keyword representing what kind of goal needs to be met."
......@@ -267,15 +245,10 @@
(defn render-alert-email
"Take a pulse object and list of results, returns an array of attachment objects for an email"
[timezone {:keys [alert_first_only] :as alert} results goal-value]
(let [images (atom {})
body (binding [render/*include-title* true
render/*render-img-fn* (partial render-image images)]
(html (vec (cons :div (for [result results]
(render/render-pulse-section timezone result))))))
message-ctx (default-alert-context alert (alert-results-condition-text goal-value))]
(let [message-ctx (default-alert-context alert (alert-results-condition-text goal-value))]
(render-message-body "metabase/email/alert"
(assoc message-ctx :pulse body :firstRunOnly? alert_first_only)
(seq @images))))
(assoc message-ctx :firstRunOnly? alert_first_only)
timezone results)))
(def ^:private alert-condition-text
{:meets "when this question meets its goal"
......
src/metabase/pulse/render.clj
+ 196
− 111
View file @ add12c47
......@@ -5,18 +5,21 @@
[format :as f]]
[clojure
[pprint :refer [cl-format]]
[string :as s]]
[string :as str]]
[clojure.java.io :as io]
[clojure.tools.logging :as log]
[hiccup.core :refer [h html]]
[metabase.util :as u]
[metabase.util.urls :as urls])
[metabase.util.urls :as urls]
[puppetlabs.i18n.core :refer [tru trs]]
[schema.core :as s])
(:import cz.vutbr.web.css.MediaSpec
[java.awt BasicStroke Color Dimension RenderingHints]
java.awt.image.BufferedImage
[java.io ByteArrayInputStream ByteArrayOutputStream]
java.net.URL
java.nio.charset.StandardCharsets
java.util.Date
[java.util Arrays Date]
javax.imageio.ImageIO
org.apache.commons.io.IOUtils
[org.fit.cssbox.css CSSNorm DOMAnalyzer DOMAnalyzer$Origin]
......@@ -75,6 +78,10 @@
:padding-right :1em
:padding-top :8px}))
(def ^:private RenderedPulseCard
"Schema used for functions that operate on pulse card contents and their attachments"
{:attachments (s/maybe {s/Str URL})
:content [s/Any]})
;;; # ------------------------------------------------------------ HELPER FNS ------------------------------------------------------------
......@@ -83,9 +90,9 @@
(style {:font-weight 400, :color \"white\"}) -> \"font-weight: 400; color: white;\""
[& style-maps]
(s/join " " (for [[k v] (into {} style-maps)
:let [v (if (keyword? v) (name v) v)]]
(str (name k) ": " v ";"))))
(str/join " " (for [[k v] (into {} style-maps)
:let [v (if (keyword? v) (name v) v)]]
(str (name k) ": " v ";"))))
(defn- datetime-field?
......@@ -229,12 +236,13 @@
(.createLayout content-canvas window-size)
(ImageIO/write (.getImage content-canvas) "png" os)))
(defn- render-html-to-png
^bytes [html-body width]
(s/defn ^:private render-html-to-png :- bytes
[{:keys [content]} :- RenderedPulseCard
width]
(let [html (html [:html [:body {:style (style {:margin 0
:padding 0
:background-color :white})}
html-body]])
content]])
os (ByteArrayOutputStream.)]
(render-to-png html os width)
(.toByteArray os)))
......@@ -288,7 +296,7 @@
;; If this column is remapped from another, it's already
;; in the output and should be skipped
:when (not (:remapped_from maybe-remapped-col))]
(s/upper-case (name (or (:display_name col) (:name col)))))
(str/upper-case (name (or (:display_name col) (:name col)))))
:bar-width (when include-bar? 99)})
(defn- query-results->row-seq
......@@ -336,23 +344,26 @@
" of " [:strong {:style (style {:color color-gray-3})} (format-number col-count)]
" columns."])]))
(defn- render:table
(s/defn ^:private render:table :- RenderedPulseCard
[timezone card {:keys [cols rows] :as data}]
[:div
(render-table (prep-for-html-rendering timezone cols rows nil nil cols-limit))
(render-truncation-warning cols-limit (count cols) rows-limit (count rows))])
{:attachments nil
:content [:div
(render-table (prep-for-html-rendering timezone cols rows nil nil cols-limit))
(render-truncation-warning cols-limit (count cols) rows-limit (count rows))]})
(defn- render:bar
(s/defn ^:private render:bar :- RenderedPulseCard
[timezone card {:keys [cols rows] :as data}]
(let [max-value (apply max (map second rows))]
[:div
(render-table (prep-for-html-rendering timezone cols rows second max-value 2))
(render-truncation-warning 2 (count cols) rows-limit (count rows))]))
{:attachments nil
:content [:div
(render-table (prep-for-html-rendering timezone cols rows second max-value 2))
(render-truncation-warning 2 (count cols) rows-limit (count rows))]}))
(defn- render:scalar
(s/defn ^:private render:scalar :- RenderedPulseCard
[timezone card {:keys [cols rows]}]
[:div {:style (style scalar-style)}
(h (format-cell timezone (ffirst rows) (first cols)))])
{:attachments nil
:content [:div {:style (style scalar-style)}
(h (format-cell timezone (ffirst rows) (first cols)))]})
(defn- render-sparkline-to-png
"Takes two arrays of numbers between 0 and 1 and plots them as a sparkline"
......@@ -380,10 +391,54 @@
(* 2 sparkline-dot-radius)
(* 2 sparkline-dot-radius)))
(when-not (ImageIO/write image "png" os) ; returns `true` if successful -- see JavaDoc
(throw (Exception. "No approprate image writer found!")))
(let [^String msg (tru "No approprate image writer found!")]
(throw (Exception. msg))))
(.toByteArray os)))
(defn- render:sparkline
(defn- hash-bytes
"Generate a hash to be used in a Content-ID"
[^bytes img-bytes]
(Math/abs ^Integer (Arrays/hashCode img-bytes)))
(defn- hash-image-url
"Generate a hash to be used in a Content-ID"
[^java.net.URL url]
(-> url io/input-stream IOUtils/toByteArray hash-bytes))
(defn- content-id-reference [content-id]
(str "cid:" content-id))
(defn- mb-hash-str [image-hash]
(str image-hash "@metabase"))
(defn- resource-path->content-id-pair [path]
(let [external-link-url (io/resource path)]
[(mb-hash-str (hash-image-url external-link-url))
external-link-url]))
(defn- write-byte-array-to-temp-file
[^bytes img-bytes]
(let [f (doto (java.io.File/createTempFile "metabase_pulse_image_" ".png")
.deleteOnExit)]
(with-open [fos (java.io.FileOutputStream. f)]
(.write fos img-bytes))
f))
(defn- byte-array->content-id-pair
[^bytes img-bytes]
(let [f (write-byte-array-to-temp-file img-bytes)]
[(mb-hash-str (hash-bytes img-bytes))
(io/as-url f)]))
(def ^:private external-link-image
(delay
(resource-path->content-id-pair "frontend_client/app/assets/img/external_link.png")))
(def ^:private no-results-image
(delay
(resource-path->content-id-pair "frontend_client/app/assets/img/pulse_no_results@2x.png")))
(s/defn ^:private render:sparkline :- RenderedPulseCard
[timezone card {:keys [rows cols]}]
(let [ft-row (if (datetime-field? (first cols))
#(.getTime ^Date (u/->Timestamp %))
......@@ -406,42 +461,47 @@
ys' (map #(/ (double (- % ymin)) yrange) ys) ; cast to double to avoid "Non-terminating decimal expansion" errors
rows' (reverse (take-last 2 rows))
values (map (comp format-number second) rows')
labels (format-timestamp-pair timezone (map first rows') (first cols))]
[:div
[:img {:style (style {:display :block
:width :100%})
:src (*render-img-fn* (render-sparkline-to-png xs' ys' 524 130))}]
[:table
[:tr
[:td {:style (style {:color color-brand
:font-size :24px
:font-weight 700
:padding-right :16px})}
(first values)]
[:td {:style (style {:color color-gray-3
:font-size :24px
:font-weight 700})}
(second values)]]
[:tr
[:td {:style (style {:color color-brand
:font-size :16px
:font-weight 700
:padding-right :16px})}
(first labels)]
[:td {:style (style {:color color-gray-3
:font-size :16px})}
(second labels)]]]]))
(defn- render-image-with-filename [^String filename]
(*render-img-fn* (IOUtils/toByteArray (io/input-stream (io/resource filename)))))
(defn- render:empty [_ _]
[:div {:style (style {:text-align :center})}
[:img {:style (style {:width :104px})
:src (render-image-with-filename "frontend_client/app/assets/img/pulse_no_results@2x.png")}]
[:div {:style (style {:margin-top :8px
:color color-gray-4})}
"No results"]])
labels (format-timestamp-pair timezone (map first rows') (first cols))
[content-id png-url] (byte-array->content-id-pair (render-sparkline-to-png xs' ys' 524 130))]
{:attachments (when (and content-id png-url)
{content-id png-url})
:content [:div
[:img {:style (style {:display :block
:width :100%})
:src (content-id-reference content-id)}]
[:table
[:tr
[:td {:style (style {:color color-brand
:font-size :24px
:font-weight 700
:padding-right :16px})}
(first values)]
[:td {:style (style {:color color-gray-3
:font-size :24px
:font-weight 700})}
(second values)]]
[:tr
[:td {:style (style {:color color-brand
:font-size :16px
:font-weight 700
:padding-right :16px})}
(first labels)]
[:td {:style (style {:color color-gray-3
:font-size :16px})}
(second labels)]]]]}))
(s/defn ^:private render:empty :- RenderedPulseCard
[_ _]
(let [[content-id no-results-url] @no-results-image]
{:attachments (when (and content-id no-results-url)
{content-id no-results-url})
:content [:div {:style (style {:text-align :center})}
[:img {:style (style {:width :104px})
:src (content-id-reference content-id)}]
[:div {:style (style {:margin-top :8px
:color color-gray-4})}
"No results"]]}))
(defn detect-pulse-card-type
"Determine the pulse (visualization) type of a CARD, e.g. `:scalar` or `:bar`."
......@@ -467,61 +527,86 @@
(number-field? col-2)) :bar
:else :table)))
(defn render-pulse-card
"Render a single CARD for a `Pulse` to Hiccup HTML. RESULT is the QP results."
(s/defn ^:private make-title-if-needed :- (s/maybe RenderedPulseCard)
[card]
(when *include-title*
(let [[content-id link-url] (when *include-buttons*
@external-link-image)]
{:attachments (when (and content-id link-url)
{content-id link-url})
:content [:table {:style (style {:margin-bottom :8px
:width :100%})}
[:tbody
[:tr
[:td [:span {:style header-style}
(-> card :name h)]]
[:td {:style (style {:text-align :right})}
(when *include-buttons*
[:img {:style (style {:width :16px})
:width 16
:src (content-id-reference content-id)}])]]]]})))
(s/defn ^:private render-pulse-card-body :- RenderedPulseCard
[timezone card {:keys [data error]}]
[:a {:href (card-href card)
:target "_blank"
:style (style section-style
{:margin :16px
:margin-bottom :16px
:display :block
:text-decoration :none})}
(when *include-title*
[:table {:style (style {:margin-bottom :8px
:width :100%})}
[:tbody
[:tr
[:td [:span {:style header-style}
(-> card :name h)]]
[:td {:style (style {:text-align :right})}
(when *include-buttons*
[:img {:style (style {:width :16px})
:width 16
:src (render-image-with-filename "frontend_client/app/assets/img/external_link.png")}])]]]])
(try
(when error
(throw (Exception. (str "Card has errors: " error))))
(case (detect-pulse-card-type card data)
:empty (render:empty card data)
:scalar (render:scalar timezone card data)
:sparkline (render:sparkline timezone card data)
:bar (render:bar timezone card data)
:table (render:table timezone card data)
[:div {:style (style font-style
{:color "#F9D45C"
:font-weight 700})}
"We were unable to display this card." [:br] "Please view this card in Metabase."])
(catch Throwable e
(log/warn "Pulse card render error:" e)
[:div {:style (style font-style
{:color "#EF8C8C"
:font-weight 700
:padding :16px})}
"An error occurred while displaying this card."]))])
(defn render-pulse-section
(try
(when error
(let [^String msg (tru "Card has errors: {0}" error)]
(throw (Exception. msg))))
(case (detect-pulse-card-type card data)
:empty (render:empty card data)
:scalar (render:scalar timezone card data)
:sparkline (render:sparkline timezone card data)
:bar (render:bar timezone card data)
:table (render:table timezone card data)
{:attachments nil
:content [:div {:style (style font-style
{:color "#F9D45C"
:font-weight 700})}
"We were unable to display this card." [:br] "Please view this card in Metabase."]})
(catch Throwable e
(log/error e (trs "Pulse card render error"))
{:attachments nil
:content [:div {:style (style font-style
{:color "#EF8C8C"
:font-weight 700
:padding :16px})}
"An error occurred while displaying this card."]})))
(s/defn render-pulse-card :- RenderedPulseCard
"Render a single CARD for a `Pulse` to Hiccup HTML. RESULT is the QP results."
[timezone card results]
(let [{title :content title-attachments :attachments} (make-title-if-needed card)
{pulse-body :content body-attachments :attachments} (render-pulse-card-body timezone card results)]
{:attachments (merge title-attachments body-attachments)
:content [:a {:href (card-href card)
:target "_blank"
:style (style section-style
{:margin :16px
:margin-bottom :16px
:display :block
:text-decoration :none})}
title
pulse-body]}))
(defn render-pulse-card-for-display
"Same as `render-pulse-card` but isn't intended for an email, rather for previewing so there is no need for
attachments"
[timezone card results]
(:content (render-pulse-card timezone card results)))
(s/defn render-pulse-section :- RenderedPulseCard
"Render a specific section of a Pulse, i.e. a single Card, to Hiccup HTML."
[timezone {:keys [card result]}]
[:div {:style (style {:margin-top :10px
:margin-bottom :20px
:border "1px solid #dddddd"
:border-radius :2px
:background-color :white
:box-shadow "0 1px 2px rgba(0, 0, 0, .08)"})}
(binding [*include-title* true]
(render-pulse-card timezone card result))])
(let [{:keys [attachments content]} (binding [*include-title* true]
(render-pulse-card timezone card result))]
{:attachments attachments
:content [:div {:style (style {:margin-top :10px
:margin-bottom :20px
:border "1px solid #dddddd"
:border-radius :2px
:background-color :white
:box-shadow "0 1px 2px rgba(0, 0, 0, .08)"})}
content]}))
(defn render-pulse-card-to-png
"Render a PULSE-CARD as a PNG. DATA is the `:data` from a QP result (I think...)"
......
test/metabase/pulse/render_test.clj
+ 25
− 20
View file @ add12c47
......@@ -134,33 +134,38 @@
{:bar-width nil, :row ["3" "34.05" "Aug 1, 2014" "The Gorbals"]}]
(rest (prep-for-html-rendering pacific-tz test-columns-with-date-special-type test-data nil nil (count test-columns))))
(defn- render-scalar-value [results]
(-> (render:scalar pacific-tz nil results)
:content
last))
(expect
"10"
(last (render:scalar pacific-tz nil {:cols [{:name "ID",
:display_name "ID",
:base_type :type/BigInteger
:special_type nil}]
:rows [[10]]})))
(render-scalar-value {:cols [{:name "ID",
:display_name "ID",
:base_type :type/BigInteger
:special_type nil}]
:rows [[10]]}))
(expect
"10.12"
(last (render:scalar pacific-tz nil {:cols [{:name "floatnum",
:display_name "FLOATNUM",
:base_type :type/Float
:special_type nil}]
:rows [[10.12345]]})))
(render-scalar-value {:cols [{:name "floatnum",
:display_name "FLOATNUM",
:base_type :type/Float
:special_type nil}]
:rows [[10.12345]]}))
(expect
"foo"
(last (render:scalar pacific-tz nil {:cols [{:name "stringvalue",
:display_name "STRINGVALUE",
:base_type :type/Text
:special_type nil}]
:rows [["foo"]]})))
(render-scalar-value {:cols [{:name "stringvalue",
:display_name "STRINGVALUE",
:base_type :type/Text
:special_type nil}]
:rows [["foo"]]}))
(expect
"Apr 1, 2014"
(last (render:scalar pacific-tz nil {:cols [{:name "date",
:display_name "DATE",
:base_type :type/DateTime
:special_type nil}]
:rows [["2014-04-01T08:30:00.0000"]]})))
(render-scalar-value {:cols [{:name "date",
:display_name "DATE",
:base_type :type/DateTime
:special_type nil}]
:rows [["2014-04-01T08:30:00.0000"]]}))
test/metabase/pulse_test.clj
+ 2
− 2
View file @ add12c47
......@@ -65,7 +65,7 @@
{:type :inline,
:content-id true,
:content-type "image/png",
:content java.io.File})
:content java.net.URL})
(defn- rasta-pulse-email [& [email]]
(et/email-to :rasta (merge {:subject "Pulse: Pulse Name",
......@@ -366,7 +366,7 @@
(defn- attachment? [{message-type :type content-type :content-type content :content}]
(and (= :inline message-type)
(= "image/png" content-type)
(instance? java.io.File content)))
(instance? java.net.URL content)))
;; Test with a slack channel and an email
(tt/expect-with-temp [Card [{card-id :id} (checkins-query {:breakout [["datetime-field" (data/id :checkins :date) "hour"]]})]
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment