New endpoint for fetching geojson from URL (#16607)

src/metabase/api/geojson.clj

@@ -5,6 +5,7 @@
             [metabase.models.setting :as setting :refer [defsetting]]
             [metabase.util.i18n :as ui18n :refer [deferred-tru tru]]
             [metabase.util.schema :as su]
+            [ring.util.codec :as rc]
             [ring.util.response :as rr]
             [schema.core :as s])
   (:import java.net.URL
@@ -70,7 +71,7 @@
   (try
     (s/validate CustomGeoJSON geojson)
     (catch Throwable e
-      (throw (ex-info (tru "Invalid custom GeoJSON.") {:status-code 400} e))))
+      (throw (ex-info (tru "Invalid custom GeoJSON") {:status-code 400} e))))
   (or (valid-geojson-url? geojson)
       (throw (ex-info (invalid-location-msg) {:status-code 400}))))
 
@@ -85,19 +86,37 @@
              (setting/set-json! :custom-geojson new-value))
   :visibility :public)
 
-
 (api/defendpoint-async GET "/:key"
   "Fetch a custom GeoJSON file as defined in the `custom-geojson` setting. (This just acts as a simple proxy for the
   file specified for `key`)."
   [{{:keys [key]} :params} respond raise]
   {key su/NonBlankString}
   (if-let [url (get-in (custom-geojson) [(keyword key) :url])]
-    (with-open [reader (io/reader (or (io/resource url)
-                                      url))
-                is     (ReaderInputStream. reader)]
-      (respond (-> (rr/response is)
-                   (rr/content-type "application/json"))))
-    (raise (ex-info (tru "Invalid custom GeoJSON key: {0}" key)
-             {:status-code 400}))))
+    (try
+      (with-open [reader (io/reader (or (io/resource url)
+                                        url))
+                  is     (ReaderInputStream. reader)]
+        (respond (-> (rr/response is)
+                     (rr/content-type "application/json"))))
+      (catch Throwable e
+        (raise (ex-info (tru "GeoJSON URL failed to load") {:status-code 400}))))
+    (raise (ex-info (tru "Invalid custom GeoJSON key: {0}" key) {:status-code 400}))))
+
+(api/defendpoint-async GET "/"
+  "Load a custom GeoJSON file based on a URL or file path provided as a query parameter.
+  This behaves similarly to /api/geojson/:key but doesn't require the custom map to be saved to the DB first."
+  [{{:keys [url]} :params} respond raise]
+  {url su/NonBlankString}
+  (let [decoded-url (rc/url-decode url)]
+    (or (io/resource decoded-url)
+        (valid-url? decoded-url))
+    (try
+      (with-open [reader (io/reader (or (io/resource decoded-url)
+                                        decoded-url))
+                  is     (ReaderInputStream. reader)]
+        (respond (-> (rr/response is)
+                     (rr/content-type "application/json"))))
+      (catch Throwable e
+        (raise (ex-info (tru "GeoJSON URL failed to load") {:status-code 400}))))))
 
 (api/define-routes)

test/metabase/api/geojson_test.clj

@@ -11,6 +11,10 @@
   "URL of a GeoJSON file used for test purposes."
   "https://raw.githubusercontent.com/metabase/metabase/master/test_resources/test.geojson")
 
+(def ^:private ^String test-broken-geojson-url
+  "URL of a GeoJSON file that is a valid URL but which cannot be connected to."
+  "https://raw.githubusercontent.com/metabase/metabase/master/test_resources/broken.geojson")
+
 (def ^:private test-custom-geojson
   {:middle-earth {:name        "Middle Earth"
                   :url         test-geojson-url
@@ -18,13 +22,12 @@
                   :region_key  nil
                   :region_name nil}})
 
-(def ^:private test-geojson-value
-  {:type "FeatureCollection"
-   :features [{:type "Feature"
-               :geometry {:type "Point",
-                          :coordinates [55.948609737988384
-                                        -3.1919722001105044]}
-               :properties []}]})
+(def ^:private test-broken-custom-geojson
+  {:middle-earth {:name        "Middle Earth"
+                  :url         test-broken-geojson-url
+                  :builtin     true
+                  :region_key  nil
+                  :region_name nil}})
 
 (deftest geojson-schema-test
   (is (= true
@@ -57,8 +60,8 @@
           valid?   #'geojson-api/validate-geojson]
       (doseq [[url should-pass?] examples]
         (let [geojson {:deadb33f {:name        "Rivendell"
-                                   :url         url
-                                   :region_key  nil
+                                  :url         url
+                                  :region_key  nil
                                   :region_name nil}}]
           (if should-pass?
             (is (valid? geojson) url)
@@ -92,10 +95,20 @@
                     {:value resource-geojson})
                    ((mt/user->client :crowberto) :get 200 "setting/custom-geojson")))))))))
 
-(deftest proxy-endpoint-test
+(deftest url-proxy-endpoint-test
+  (testing "GET /api/geojson"
+    (testing "test the endpoint that fetches JSON files given a URL"
+      (is (= {:type        "Point"
+              :coordinates [37.77986 -122.429]}
+             ((mt/user->client :rasta) :get 200 "geojson" :url test-geojson-url))))
+    (testing "error is returned if URL connection fails"
+      (is (= "GeoJSON URL failed to load"
+             ((mt/user->client :rasta) :get 400 "geojson" :url test-broken-geojson-url))))))
+
+(deftest key-proxy-endpoint-test
   (testing "GET /api/geojson/:key"
     (mt/with-temporary-setting-values [custom-geojson test-custom-geojson]
-      (testing "test the endpoint that acts as a proxy for JSON files"
+      (testing "test the endpoint that fetches JSON files given a GeoJSON key"
         (is (= {:type        "Point"
                 :coordinates [37.77986 -122.429]}
                ((mt/user->client :rasta) :get 200 "geojson/middle-earth"))))
@@ -107,7 +120,10 @@
         (is (= {:type        "Point"
                 :coordinates [37.77986 -122.429]}
                (client/client :get 200 "geojson/middle-earth"))))
-      (testing "error conditions"
         (testing "try fetching an invalid key; should fail"
           (is (= "Invalid custom GeoJSON key: invalid-key"
-                 ((mt/user->client :rasta) :get 400 "geojson/invalid-key"))))))))
+                 ((mt/user->client :rasta) :get 400 "geojson/invalid-key")))))
+    (mt/with-temporary-setting-values [custom-geojson test-broken-custom-geojson]
+      (testing "fetching a broken URL should fail"
+        (is (= "GeoJSON URL failed to load"
+               ((mt/user->client :rasta) :get 400 "geojson/middle-earth")))))))