From add12c47d5256298fa26266d0a34fd046dc2a7d4 Mon Sep 17 00:00:00 2001
From: Ryan Senior <ryan@metabase.com>
Date: Wed, 22 Nov 2017 13:48:13 -0600
Subject: [PATCH] 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.
---
 src/metabase/api/pulse.clj          |   4 +-
 src/metabase/email.clj              |  23 ++-
 src/metabase/email/messages.clj     |  57 ++----
 src/metabase/pulse/render.clj       | 307 ++++++++++++++++++----------
 test/metabase/pulse/render_test.clj |  45 ++--
 test/metabase/pulse_test.clj        |   4 +-
 6 files changed, 252 insertions(+), 188 deletions(-)

diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj
index e7479c95bfb..81d5e852158 100644
--- a/src/metabase/api/pulse.clj
+++ b/src/metabase/api/pulse.clj
@@ -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
diff --git a/src/metabase/email.clj b/src/metabase/email.clj
index 1ae54db739f..258b543b1bb 100644
--- a/src/metabase/email.clj
+++ b/src/metabase/email.clj
@@ -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)})))
 
diff --git a/src/metabase/email/messages.clj b/src/metabase/email/messages.clj
index 3851986d5a1..3dcaa7e71af 100644
--- a/src/metabase/email/messages.clj
+++ b/src/metabase/email/messages.clj
@@ -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"
diff --git a/src/metabase/pulse/render.clj b/src/metabase/pulse/render.clj
index 56468aeec02..59b8c239863 100644
--- a/src/metabase/pulse/render.clj
+++ b/src/metabase/pulse/render.clj
@@ -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...)"
diff --git a/test/metabase/pulse/render_test.clj b/test/metabase/pulse/render_test.clj
index ba25f3160ad..68fba0d60f7 100644
--- a/test/metabase/pulse/render_test.clj
+++ b/test/metabase/pulse/render_test.clj
@@ -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"]]}))
diff --git a/test/metabase/pulse_test.clj b/test/metabase/pulse_test.clj
index 4c71edea5d3..670eba881f5 100644
--- a/test/metabase/pulse_test.clj
+++ b/test/metabase/pulse_test.clj
@@ -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"]]})]
-- 
GitLab