diff --git a/src/metabase/api/tiles.clj b/src/metabase/api/tiles.clj index 81081bc8e5b3a06474f1a24c79860936b4b435bb..cfa4f93f283230fa343ee0f986e42ac00cdf2254 100644 --- a/src/metabase/api/tiles.clj +++ b/src/metabase/api/tiles.clj @@ -7,13 +7,14 @@ [metabase.mbql.normalize :as normalize] [metabase.mbql.util :as mbql.u] [metabase.query-processor :as qp] + [metabase.query-processor.util :as qp.util] [metabase.util :as u] [metabase.util.i18n :refer [tru]] [metabase.util.schema :as su] [schema.core :as s]) (:import java.awt.Color java.awt.image.BufferedImage - [java.io ByteArrayInputStream ByteArrayOutputStream] + java.io.ByteArrayOutputStream javax.imageio.ImageIO)) ;;; --------------------------------------------------- CONSTANTS ---------------------------------------------------- @@ -143,7 +144,7 @@ (let [id-or-name' (int-or-string id-or-name)] [:field id-or-name' (when (string? id-or-name') {:base-type :type/Float})])) -(defn query->tiles-query +(defn- query->tiles-query "Transform a card's query into a query finding coordinates in a particular region. - transform native queries into nested mbql queries from that native query @@ -151,16 +152,14 @@ - limit query results to `tile-coordinate-limit` number of results - only select lat and lon fields rather than entire query's fields" [query {:keys [zoom x y lat-field lon-field]}] - (let [lat-ref (field-ref lat-field) - lon-ref (field-ref lon-field)] - (-> query - native->source-query - (update :query query-with-inside-filter - lat-ref lon-ref - x y zoom) - (assoc-in [:query :fields] [lat-ref lon-ref]) - (assoc-in [:query :limit] tile-coordinate-limit) - (assoc :async? false)))) + (-> query + native->source-query + (update :query query-with-inside-filter + lat-field lon-field + x y zoom) + (assoc-in [:query :fields] [lat-field lon-field]) + (assoc-in [:query :limit] tile-coordinate-limit) + (assoc :async? false))) ;; TODO - this can be reworked to be `defendpoint-async` instead ;; @@ -178,26 +177,38 @@ lat-field s/Str lon-field s/Str query su/JSONString} - (let [zoom (Integer/parseInt zoom) - x (Integer/parseInt x) - y (Integer/parseInt y) + (let [zoom (Integer/parseInt zoom) + x (Integer/parseInt x) + y (Integer/parseInt y) + lat-field-ref (field-ref lat-field) + lon-field-ref (field-ref lon-field) query (normalize/normalize (json/parse-string query keyword)) updated-query (query->tiles-query query {:zoom zoom :x x :y y - :lat-field lat-field - :lon-field lon-field}) + :lat-field lat-field-ref + :lon-field lon-field-ref}) - {:keys [status], {:keys [rows]} :data, :as result} + {:keys [status], {:keys [rows cols]} :data, :as result} (qp/process-query-and-save-execution! updated-query {:executed-by api/*current-user-id* - :context :map-tiles})] - ;; manual ring response here. we simply create an inputstream from the byte[] of our image + :context :map-tiles}) + + lat-key (qp.util/field-ref->key lat-field-ref) + lon-key (qp.util/field-ref->key lon-field-ref) + find-fn (fn [lat-or-lon-key] + (first (keep-indexed + (fn [idx col] (when (= (qp.util/field-ref->key (:field_ref col)) lat-or-lon-key) idx)) + cols))) + lat-idx (find-fn lat-key) + lon-idx (find-fn lon-key) + points (for [row rows] + [(nth row lat-idx) (nth row lon-idx)])] (if (= status :completed) {:status 200 :headers {"Content-Type" "image/png"} - :body (ByteArrayInputStream. (tile->byte-array (create-tile zoom rows)))} + :body (tile->byte-array (create-tile zoom points))} (throw (ex-info (tru "Query failed") ;; `result` might be a `core.async` channel or something we're not expecting (assoc (when (map? result) result) :status-code 400)))))) diff --git a/test/metabase/api/tiles_test.clj b/test/metabase/api/tiles_test.clj index bbbac54a8dae7579672998323b5e47464ab65fe0..0c3f185a8d3eebb2e3cf1e0f687c2dab28dc10f0 100644 --- a/test/metabase/api/tiles_test.clj +++ b/test/metabase/api/tiles_test.clj @@ -46,8 +46,8 @@ :query {:source-table 88 :fields [[:field 562 nil] [:field 574 nil] ; lat - [:field 576 nil] ; lon - ] + [:field 576 nil]] ; lon + :limit 50000} :type :query}] (is (= {:database 19 @@ -58,10 +58,10 @@ :filter [:inside [:field 574 nil] [:field 576 nil]]} :type :query :async? false} - (clean (tiles/query->tiles-query query - {:zoom 2 :x 3 :y 1 - :lat-field "574" - :lon-field "576"}))))))) + (clean (#'tiles/query->tiles-query query + {:zoom 2 :x 3 :y 1 + :lat-field [:field 574 nil] + :lon-field [:field 576 nil]}))))))) (testing "native" (testing "nests the query, selects fields" (let [query {:type :native @@ -78,10 +78,31 @@ :limit 2000} :type :query :async? false} - (clean (tiles/query->tiles-query query - {:zoom 2 :x 2 :y 1 - :lat-field "latitude" - :lon-field "longitude"}))))))))) + (clean (@#'tiles/query->tiles-query query + {:zoom 2 :x 2 :y 1 + :lat-field [:field "latitude" {:base-type :type/Float}] + :lon-field [:field "longitude" {:base-type :type/Float}]}))))))))) + +(deftest breakout-query-test + (testing "the appropriate lat/lon fields are selected from the results, if the query contains a :breakout clause (#20182)" + (mt/dataset sample-dataset + (with-redefs [tiles/create-tile (fn [_ points] points) + tiles/tile->byte-array identity] + (let [result (mt/user-http-request + :rasta :get 200 (format "tiles/7/30/49/%d/%d" + (mt/id :people :latitude) + (mt/id :people :longitude)) + :query (json/generate-string + {:database (mt/id) + :type :query + :query {:source-table (mt/id :people) + :breakout [[:field (mt/id :people :latitude)] + [:field (mt/id :people :longitude)]] + :aggregation [[:count]]}}))] + (is (= [[36.6163612 -94.5197949] + [36.8177783 -93.8447328] + [36.8311004 -95.0253779]] + (take 3 result)))))))) (deftest failure-test (testing "if the query fails, don't attempt to generate a map without any points -- the endpoint should return a 400" @@ -104,3 +125,10 @@ :type :query :query {:source-table (mt/id :venues)} :async? true})))))) + +(deftest field-ref-test + (testing "Field refs can be constructed from strings representing integer field IDs or field names" + (is (= [:field 1 nil] + (@#'tiles/field-ref "1"))) + (is (= [:field "Latitude" {:base-type :type/Float}] + (@#'tiles/field-ref "Latitude")))))