merge

.dir-locals.el

@@ -39,7 +39,7 @@
                               (project 1)
                               (qp-expect-with-engines 1)
                               (render-file 1)
-                              (resolve-private-fns 1)
+                              (resolve-private-vars 1)
                               (select 1)
                               (sync-in-context 2)
                               (when-testing-engine 1)

frontend/src/metabase/admin/settings/components/SettingsCustomMaps.jsx

@@ -247,7 +247,7 @@ const EditMap = ({ map, onMapChange, originalMap, geoJson, geoJsonLoading, geoJs
         </SettingContainer>
         { (geoJson || geoJsonLoading || geoJsonError) &&
             <LoadingAndErrorWrapper loading={geoJsonLoading} error={geoJsonError}>
-            { () =>
+            { () =>  // eslint-disable-line react/display-name
                 <div>
                     <SettingContainer description="Which property specifies the region’s identifier?">
                         <GeoJsonPropertySelect

frontend/src/metabase/visualizations/components/LegacyChoropleth.jsx

@@ -14,7 +14,7 @@ const LegacyChoropleth = ({ series, geoJson, projection, getColor, onHoverFeatur
     return (
         <div className="absolute top bottom left right flex layout-centered">
             <ShouldUpdate series={series} shouldUpdate={(props, nextProps) => !isSameSeries(props.series, nextProps.series)}>
-                { () =>
+                { () =>  // eslint-disable-line react/display-name
                     <svg className="flex-full m1" viewBox={`0 0 ${width} ${height}`}>
                     {geoJson.features.map((feature, index) =>
                         <path

src/metabase/api/geojson.clj

 (ns metabase.api.geojson
-  (:require [cheshire.core :as json]
+  (:require [clojure.java.io :as io]
+            [cheshire.core :as json]
             [compojure.core :refer [defroutes GET]]
             [schema.core :as s]
             [metabase.api.common :refer :all]
             [metabase.models.setting :refer [defsetting], :as setting]
             [metabase.util :as u]))
 
+(defn- valid-json?
+  "Does this URL-OR-RESOURCE point to valid JSON?
+   URL-OR-RESOURCE should be something that can be passed to `slurp`, like an HTTP URL or a `java.net.URL` (which is what `io/resource` returns below)."
+  [url-or-resource]
+  (u/with-timeout 5000
+    (json/parse-string (slurp url-or-resource)))
+  true)
 
-(def ^:private valid-json-url?
+(defn- valid-json-resource?
+  "Does this RELATIVE-PATH point to a valid local JSON resource? (RELATIVE-PATH is something like \"app/charts/us-states.json\".)"
+  [relative-path]
+  (when-let [^java.net.URI uri (u/ignore-exceptions (java.net.URI. relative-path))]
+    (when-not (.isAbsolute uri)
+      (valid-json? (io/resource (str "frontend_client" uri))))))
+
+(defn- valid-json-url?
+  "Is URL a valid HTTP URL and does it point to valid JSON?"
+  [url]
+  (when (u/is-url? url)
+    (valid-json? url)))
+
+(def ^:private valid-json-url-or-resource?
   "Check that remote URL points to a valid JSON file, or throw an exception.
    Since the remote file isn't likely to change, this check isn't repeated for URLs that have already succeded;
    if the check fails, an exception is thrown (thereby preventing memoization)."
-  (memoize (fn [url]
-             (assert (u/is-url? url)
-               (str "Invalid URL: " url))
-             (u/with-timeout 5000
-               (json/parse-string (slurp url)))
-             true)))
+  (memoize (fn [url-or-resource-path]
+             (or (valid-json-url? url-or-resource-path)
+                 (valid-json-resource? url-or-resource-path)
+                 (throw (Exception. (str "Invalid JSON URL or resource: " url-or-resource-path)))))))
 
 (def ^:private CustomGeoJSON
-  {s/Keyword {:name        s/Str
-              :url         (s/constrained s/Str valid-json-url? "URL must point to a valid JSON file.")
-              :region_key  (s/maybe s/Str)
-              :region_name (s/maybe s/Str)}})
+  {s/Keyword {:name                     s/Str
+              :url                      (s/constrained s/Str valid-json-url-or-resource? "URL must point to a valid JSON file.")
+              :region_key               (s/maybe s/Str)
+              :region_name              (s/maybe s/Str)
+              (s/optional-key :builtin) s/Bool}})
 
-(def ^:private builtin-geojson
-  {:us_states {:name "United States" :url "/app/charts/us-states.json" :region_key "name" :region_name "name" :builtin true}
-   :world_countries {:name "World" :url "/app/charts/world.json" :region_key "ISO_A2" :region_name "NAME" :builtin true}})
+(def ^:private ^:const builtin-geojson
+  {:us_states       {:name        "United States"
+                     :url         "/app/charts/us-states.json"
+                     :region_key  "name"
+                     :region_name "name"
+                     :builtin     true}
+   :world_countries {:name        "World"
+                     :url         "/app/charts/world.json"
+                     :region_key  "ISO_A2"
+                     :region_name "NAME"
+                     :builtin     true}})
 
 (defsetting custom-geojson
   "JSON containing information about custom GeoJSON files for use in map visualizations instead of the default US State or World GeoJSON."
-  :type   :json
+  :type    :json
   :default {}
-  :getter (fn [] (merge (setting/get-json :custom-geojson) builtin-geojson))
-  :setter (fn [new-value]
-            (when new-value
-              (s/validate CustomGeoJSON new-value))
-            (setting/set-json! :custom-geojson new-value)))
+  :getter  (fn [] (merge (setting/get-json :custom-geojson) builtin-geojson))
+  :setter  (fn [new-value]
+             (when new-value
+               (s/validate CustomGeoJSON new-value))
+             (setting/set-json! :custom-geojson new-value)))
 
 
 (defendpoint GET "/:key"

test/metabase/api/activity_test.clj

@@ -8,7 +8,7 @@
                              [view-log :refer [ViewLog]])
             [metabase.test.data :refer :all]
             [metabase.test.data.users :refer :all]
-            [metabase.test.util :refer [match-$ expect-with-temp resolve-private-fns], :as tu]
+            [metabase.test.util :refer [match-$ expect-with-temp resolve-private-vars], :as tu]
             [metabase.util :as u]))
 
 ;; GET /
@@ -172,7 +172,7 @@
 
 ;;; activities->referenced-objects, referenced-objects->existing-objects, add-model-exists-info
 
-(resolve-private-fns metabase.api.activity activities->referenced-objects referenced-objects->existing-objects add-model-exists-info)
+(resolve-private-vars metabase.api.activity activities->referenced-objects referenced-objects->existing-objects add-model-exists-info)
 
 (def ^:private ^:const fake-activities
   [{:model "dashboard", :model_id  43, :topic :dashboard-create,    :details {}}

test/metabase/api/geojson_test.clj

@@ -5,8 +5,10 @@
             [metabase.test.data.users :refer [user->client]]
             [metabase.test.util :as tu]))
 
-(tu/resolve-private-fns metabase.api.geojson
+(tu/resolve-private-vars metabase.api.geojson
   valid-json-url?
+  valid-json-resource?
+  builtin-geojson
   CustomGeoJSON)
 
 (def ^:private ^:const ^String test-geojson-url
@@ -16,6 +18,7 @@
 (def ^:private ^:const test-custom-geojson
   {:middle-earth {:name        "Middle Earth"
                   :url         test-geojson-url
+                  :builtin     true
                   :region_key  nil
                   :region_name nil}})
 
@@ -25,6 +28,11 @@
   (valid-json-url? test-geojson-url))
 
 
+;;; test valid-json-resource?
+(expect
+  (valid-json-resource? "/app/charts/us-states.json"))
+
+
 ;;; test the CustomGeoJSON schema
 (expect
   (boolean (s/validate @CustomGeoJSON test-custom-geojson)))
@@ -47,7 +55,7 @@
 
 ;;; test that we can set the value of custom-geojson via the normal routes
 (expect
-  test-custom-geojson
+  (merge @builtin-geojson test-custom-geojson)
   ;; bind a temporary value so it will get set back to its old value here after the API calls are done stomping all over it
   (tu/with-temporary-setting-values [custom-geojson nil]
     ((user->client :crowberto) :put 200 "setting/custom-geojson" {:value test-custom-geojson})

test/metabase/api/session_test.clj

@@ -11,7 +11,7 @@
             [metabase.test.data :refer :all]
             [metabase.test.data.users :refer :all]
             [metabase.util :as u]
-            [metabase.test.util :refer [random-name resolve-private-fns with-temporary-setting-values with-temp], :as tu]))
+            [metabase.test.util :refer [random-name resolve-private-vars with-temporary-setting-values with-temp], :as tu]))
 
 ;; ## POST /api/session
 ;; Test that we can login
@@ -182,7 +182,7 @@
 
 ;;; ------------------------------------------------------------ TESTS FOR GOOGLE AUTH STUFF ------------------------------------------------------------
 
-(resolve-private-fns metabase.api.session email->domain email-in-domain? autocreate-user-allowed-for-email? google-auth-create-new-user! google-auth-fetch-or-create-user!)
+(resolve-private-vars metabase.api.session email->domain email-in-domain? autocreate-user-allowed-for-email? google-auth-create-new-user! google-auth-fetch-or-create-user!)
 
 ;;; tests for email->domain
 (expect "metabase.com"   (email->domain "cam@metabase.com"))

test/metabase/api/table_test.clj

@@ -11,10 +11,10 @@
             (metabase.test.data [dataset-definitions :as defs]
                                 [datasets :as datasets]
                                 [users :refer :all])
-            [metabase.test.util :refer [match-$ resolve-private-fns], :as tu]
+            [metabase.test.util :refer [match-$ resolve-private-vars], :as tu]
             [metabase.util :as u]))
 
-(resolve-private-fns metabase.models.table pk-field-id)
+(resolve-private-vars metabase.models.table pk-field-id)
 
 
 ;; ## /api/org/* AUTHENTICATION Tests

test/metabase/db_test.clj

@@ -3,7 +3,7 @@
             [metabase.db :as db]
             [metabase.test.util :as tu]))
 
-(tu/resolve-private-fns metabase.db parse-connection-string)
+(tu/resolve-private-vars metabase.db parse-connection-string)
 
 ;; parse minimal connection string
 (expect {:type :postgres :user nil :password nil :host "localhost" :port nil :dbname "toms_cool_db" }

test/metabase/driver/generic_sql_test.clj

@@ -9,7 +9,7 @@
             [metabase.test.data :refer :all]
             (metabase.test.data [dataset-definitions :as defs]
                                 [datasets :as datasets])
-            [metabase.test.util :refer [resolve-private-fns]])
+            [metabase.test.util :refer [resolve-private-vars]])
   (:import metabase.driver.h2.H2Driver))
 
 (def ^:private users-table      (delay (Table :name "USERS")))
@@ -79,7 +79,7 @@
                {:id (id :venues :price), :values [1 2 3 4]}]}
   (driver/analyze-table (H2Driver.) @venues-table (set (mapv :id (table/fields @venues-table)))))
 
-(resolve-private-fns metabase.driver.generic-sql field-avg-length field-values-lazy-seq table-rows-seq)
+(resolve-private-vars metabase.driver.generic-sql field-avg-length field-values-lazy-seq table-rows-seq)
 
 ;;; FIELD-AVG-LENGTH
 (datasets/expect-with-engines @generic-sql-engines

test/metabase/driver/h2_test.clj

@@ -3,10 +3,10 @@
             [metabase.db :as db]
             [metabase.driver :as driver]
             [metabase.driver.h2 :refer :all]
-            [metabase.test.util :refer [resolve-private-fns]])
+            [metabase.test.util :refer [resolve-private-vars]])
   (:import metabase.driver.h2.H2Driver))
 
-(resolve-private-fns metabase.driver.h2 connection-string->file+options file+options->connection-string connection-string-set-safe-options)
+(resolve-private-vars metabase.driver.h2 connection-string->file+options file+options->connection-string connection-string-set-safe-options)
 
 ;; Check that the functions for exploding a connection string's options work as expected
 (expect

test/metabase/driver/postgres_test.clj

@@ -172,7 +172,7 @@
 
 ;; timezone tests
 
-(tu/resolve-private-fns metabase.driver.generic-sql.query-processor
+(tu/resolve-private-vars metabase.driver.generic-sql.query-processor
   run-query-with-timezone)
 
 (defn- get-timezone-with-report-timezone [report-timezone]

test/metabase/middleware_test.clj

@@ -64,7 +64,7 @@
   (str (java.util.UUID/randomUUID)))
 
 
-(tu/resolve-private-fns metabase.db simple-insert!)
+(tu/resolve-private-vars metabase.db simple-insert!)
 
 ;; valid session ID
 (expect

test/metabase/models/dashboard_test.clj

@@ -110,7 +110,7 @@
 
 ;;; revert-dashboard!
 
-(tu/resolve-private-fns metabase.models.dashboard revert-dashboard!)
+(tu/resolve-private-vars metabase.models.dashboard revert-dashboard!)
 
 (expect
   [{:name         "Test Dashboard"

test/metabase/models/field_test.clj

@@ -73,7 +73,7 @@
 
 ;;; infer-field-special-type
 
-(tu/resolve-private-fns metabase.models.field infer-field-special-type)
+(tu/resolve-private-vars metabase.models.field infer-field-special-type)
 
 (expect nil            (infer-field-special-type nil       nil))
 (expect nil            (infer-field-special-type "id"      nil))

test/metabase/models/humanization_test.clj

@@ -6,7 +6,7 @@
             [metabase.test.util :as tu]
             [metabase.db :as db]))
 
-(tu/resolve-private-fns metabase.models.humanization
+(tu/resolve-private-vars metabase.models.humanization
   name->human-readable-name:simple name->human-readable-name:advanced)
 
 ;;; name->human-readable-name:advanced

test/metabase/models/metric_test.clj

@@ -160,7 +160,7 @@
 
 ;; ## Metric Revisions
 
-(tu/resolve-private-fns metabase.models.metric serialize-metric diff-metrics)
+(tu/resolve-private-vars metabase.models.metric serialize-metric diff-metrics)
 
 ;; serialize-metric
 (expect

test/metabase/models/segment_test.clj

@@ -154,7 +154,7 @@
 
 ;; ## Segment Revisions
 
-(tu/resolve-private-fns metabase.models.segment serialize-segment diff-segments)
+(tu/resolve-private-vars metabase.models.segment serialize-segment diff-segments)
 
 ;; serialize-segment
 (expect

test/metabase/models/session_test.clj

@@ -7,7 +7,7 @@
             [metabase.test.data.users :refer :all]
             [metabase.util :as u]))
 
-(resolve-private-fns metabase.db simple-insert-many!)
+(resolve-private-vars metabase.db simple-insert-many!)
 ;; first-session-for-user
 (expect
   "the-greatest-day-ever"

test/metabase/models/setting_test.clj

@@ -114,7 +114,7 @@
 
 ;;; ------------------------------------------------------------ all & user-facing-info ------------------------------------------------------------
 
-(resolve-private-fns metabase.models.setting resolve-setting user-facing-info)
+(resolve-private-vars metabase.models.setting resolve-setting user-facing-info)
 
 ;; these tests are to check that settings get returned with the correct information; these functions are what feed into the API