Run the tests please

move snowplow transform metrics to OSS, as transforms are now available on OSS

.github/scripts/setup-python-runner.sh

@@ -19,7 +19,7 @@ while [[ $# -gt 0 ]]; do
 done
 
 echo "Cloning python-runner repository..."
-git clone --depth 1 https://${METABASE_AUTOMATION_USER_TOKEN}@github.com/metabase/python-runner-container.git python-runner
+git clone --depth 1 -b extensible-resource-schema https://${METABASE_AUTOMATION_USER_TOKEN}@github.com/metabase/python-runner-container.git python-runner
 
 if [ "$SKIP_START" = false ]; then
   echo "Starting python-runner..."

e2e/support/cypress-embedding-sdk-component-test.config.js

@@ -7,6 +7,8 @@ const embeddingSdkComponentTestConfig = {
   defaultCommandTimeout: 10000,
   requestTimeout: 10000,
   video: false,
+  experimentalMemoryManagement: true,
+  numTestsKeptInMemory: 1,
   specPattern: "e2e/test-component/scenarios/embedding-sdk/**/*.cy.spec.tsx",
   indexHtmlFile: "e2e/support/component-index.html",
   supportFile: "e2e/support/component-cypress.js",

enterprise/backend/src/metabase_enterprise/analytics/stats.clj

@@ -1,6 +1,5 @@
 (ns metabase-enterprise.analytics.stats
   (:require
-   [java-time.api :as t]
    [metabase-enterprise.advanced-config.settings :as advanced-config.settings]
    [metabase-enterprise.scim.core :as scim]
    [metabase-enterprise.semantic-search.core :as semantic-search]
@@ -36,12 +35,3 @@
    {:name      :semantic-search
     :available (premium-features/enable-semantic-search?)
     :enabled   (semantic-search/supported?)}])
-
-(defenterprise ee-transform-metrics
-  "Returns transform usage metrics for the Snowplow stats ping."
-  :feature :none
-  []
-  (let [one-day-ago (t/minus (t/offset-date-time) (t/days 1))]
-    {:transforms               (t2/count :model/Transform)
-     :transform_runs_last_24h  (t2/count :model/TransformRun
-                                         :start_time [:>= one-day-ago])}))

enterprise/backend/src/metabase_enterprise/transforms_python/api.clj

@@ -13,6 +13,8 @@
    [metabase.util.malli.schema :as ms]
    [toucan2.core :as t2]))
 
+;; dummy change to trigger CI
+
 (defn get-python-library-by-path
   "Get Python library details by path for use by other APIs."
   [path]

enterprise/backend/test/metabase_enterprise/analytics/stats_test.clj

@@ -1,12 +1,10 @@
 (ns metabase-enterprise.analytics.stats-test
   (:require
    [clojure.test :refer :all]
-   [java-time.api :as t]
    [metabase-enterprise.analytics.stats :as ee-stats]
    [metabase-enterprise.audit-app.audit :as ee-audit]
    [metabase.analytics.stats :as stats]
    [metabase.app-db.core :as mdb]
-   [metabase.lib.core :as lib]
    [metabase.test :as mt]
    [toucan2.core :as t2]))
 
@@ -38,29 +36,3 @@
                 :public             {}
                 :embedded           {}}
                (#'stats/dashboard-metrics)))))))
-
-(deftest ee-transform-metrics-test
-  (mt/with-temp-empty-app-db [_conn :h2]
-    (mdb/setup-db! :create-sample-content? false)
-    (testing "with no transforms"
-      (is (=? {:transforms 0 :transform_runs_last_24h 0}
-              (ee-stats/ee-transform-metrics))))
-    (testing "with transforms"
-      (mt/with-temp [:model/Transform transform {:target {:database (mt/id)
-                                                          :table "test_table"}
-                                                 :name "Test SQL transform"
-                                                 :source {:type "query"
-                                                          :query (lib/native-query (mt/metadata-provider) "SELECT 1")}}
-                     :model/TransformRun _ {:start_time (t/minus (t/offset-date-time) (t/hours 1))
-                                            :transform_id (:id transform)}]
-        (is (=? {:transforms 1 :transform_runs_last_24h 1}
-                (ee-stats/ee-transform-metrics))))
-      (testing "with old transform runs"
-        (mt/with-temp [:model/Transform transform {:target {:database (mt/id)
-                                                            :table "test_table"}
-                                                   :name "Test SQL transform"
-                                                   :source {:type "query"
-                                                            :query (lib/native-query (mt/metadata-provider) "SELECT 1")}}
-                       :model/TransformRun _ {:start_time (t/minus (t/offset-date-time) (t/hours 25))
-                                              :transform_id (:id transform)}]
-          (is (zero? (:transform_runs_last_24h (ee-stats/ee-transform-metrics)))))))))

enterprise/backend/test/metabase_enterprise/transforms_python/api_test.clj

@@ -11,6 +11,8 @@
 
 (set! *warn-on-reflection* true)
 
+;; Another dummy change to trigger CI
+
 (deftest get-library-path-test
   (testing "GET /api/ee/transforms-python/library/:path"
     (mt/with-premium-features #{:transforms-python :transforms-basic}

enterprise/backend/test/metabase_enterprise/transforms_python/transforms_api_test.clj

@@ -558,8 +558,9 @@
                 (transforms.tu/test-run transform-id)
                 (transforms.tu/wait-for-transform-completion transform-id 10000)
 
-                ;; hmmm, looks like QP needs a bit more time to update metadata
-                (Thread/sleep 2000)
+                ;; Sync runs asynchronously after succeed-started-run!, so wait for
+                ;; the new "friend" field to appear in metadata before querying.
+                (transforms.tu/wait-for-field table-name "friend" 10000)
                 (let [updated-rows (transforms.tu/table-rows table-name)]
                   (is (= [["Alice" "Bob"] ["Bob" "Alice"]] updated-rows)
                       "Updated data should show Alice/Bob with friends instead of ages"))))))))))

src/metabase/analytics/stats.clj

@@ -673,17 +673,13 @@
      :values (mapv (fn [[k v]] {:group k :value v}) eid-translations-24h)
      :tags ["embedding"]}]))
 
-(defn- ee-transform-metrics'
-  "OSS fallback for transform metrics. Returns zeros since transforms are an enterprise feature."
-  []
-  {:transforms               0
-   :transform_runs_last_24h  0})
-
-(defenterprise ee-transform-metrics
+(defn- transform-metrics
   "Returns transform usage metrics for the Snowplow stats ping."
-  metabase-enterprise.analytics.stats
   []
-  (ee-transform-metrics'))
+  (let [one-day-ago (->one-day-ago)]
+    {:transforms               (t2/count :model/Transform)
+     :transform_runs_last_24h  (t2/count :model/TransformRun
+                                         :start_time [:>= one-day-ago])}))
 
 (defn- ->snowplow-metric-info
   "Collects Snowplow metrics data that is not in the legacy stats format. Also clears entity id translation count."
@@ -705,7 +701,7 @@
       :scim_users_last_24h             (t2/count :model/User :sso_source :scim
                                                  :is_active true
                                                  :date_joined [:>= one-day-ago])}
-     (ee-transform-metrics))))
+     (transform-metrics))))
 
 (mu/defn- snowplow-metrics
   [stats metric-info :- [:map

src/metabase/pivot/js.cljs

@@ -2,6 +2,7 @@
   "Javascript-facing interface for pivot table postprocessing. Wraps functions in metabase.pivot.core."
   (:require
    [metabase.pivot.core :as pivot]
+   [metabase.util :as u]
    [metabase.util.performance :as perf]))
 
 (defn ^:export columns-without-pivot-group
@@ -9,7 +10,8 @@
   [cols]
   (let [cols (js->clj cols :keywordize-keys true)]
     (clj->js
-     (pivot/columns-without-pivot-group cols))))
+     (pivot/columns-without-pivot-group cols)
+     :keyword-fn u/qualified-name)))
 
 (defn ^:export split-pivot-data
   "Pulls apart different aggregations that were packed into one result set returned from the QP.

src/metabase/transforms/jobs.clj

@@ -238,35 +238,36 @@
   (if (transforms.job-run/running-run-for-job-id job-id)
     (log/info "Not executing transform job" (pr-str job-id) "because it is already running")
     (let [transforms (job-transform-ids job-id)]
-      (log/info "Executing transform job" (pr-str job-id) "with transforms" (pr-str transforms))
-      (let [{run-id :id} (transforms.job-run/start-run! job-id run-method)]
-        (tracing/with-span :tasks "task.transform.run-job" {:transform.job/id         job-id
-                                                            :transform.job/run-method (name run-method)
-                                                            :transform.job/count      (count transforms)}
-          (transforms.instrumentation/with-job-timing [job-id run-method]
-            (try ;; catch any catastrophic problems
-              (let [result (run-transforms! run-id transforms opts)]
-                (case (::status result)
-                  :succeeded (transforms.job-run/succeed-started-run! run-id)
-                  :failed (try
-                            (transforms.job-run/fail-started-run! run-id {:message (compile-transform-failure-messages (::failures result))})
-                            (when (= :cron run-method)
-                              (notify-transform-failures job-id (::failures result)))
-                            (catch Exception e
-                              (log/error e "Error when failing a transform run.")))))
-              (catch Throwable t
-                ;; We don't expect a catastrophic failure, but neither did the Titanic.
-                ;; We should clean up in this case and notify the admin users.
-                (try
-                  (transforms.job-run/fail-started-run! run-id {:message (.getMessage t)})
-                  (when (= :cron run-method)
-                    (if (::transform-failure (ex-data t))
-                      (notify-transform-failures job-id (::failures (ex-data t)))
-                      (notify-job-failure job-id (.getMessage t))))
-                  (catch Exception e
-                    (log/error e "Error when failing a transform job run.")))
-                (throw t)))))
-        run-id))))
+      (if (empty? transforms)
+        (log/info "Skipping transform job" (pr-str job-id) "because it has no transforms to run")
+        (let [{run-id :id} (transforms.job-run/start-run! job-id run-method)]
+          (tracing/with-span :tasks "task.transform.run-job" {:transform.job/id         job-id
+                                                              :transform.job/run-method (name run-method)
+                                                              :transform.job/count      (count transforms)}
+            (transforms.instrumentation/with-job-timing [job-id run-method]
+              (try ;; catch any catastrophic problems
+                (let [result (run-transforms! run-id transforms opts)]
+                  (case (::status result)
+                    :succeeded (transforms.job-run/succeed-started-run! run-id)
+                    :failed (try
+                              (transforms.job-run/fail-started-run! run-id {:message (compile-transform-failure-messages (::failures result))})
+                              (when (= :cron run-method)
+                                (notify-transform-failures job-id (::failures result)))
+                              (catch Exception e
+                                (log/error e "Error when failing a transform run.")))))
+                (catch Throwable t
+                  ;; We don't expect a catastrophic failure, but neither did the Titanic.
+                  ;; We should clean up in this case and notify the admin users.
+                  (try
+                    (transforms.job-run/fail-started-run! run-id {:message (.getMessage t)})
+                    (when (= :cron run-method)
+                      (if (::transform-failure (ex-data t))
+                        (notify-transform-failures job-id (::failures (ex-data t)))
+                        (notify-job-failure job-id (.getMessage t))))
+                    (catch Exception e
+                      (log/error e "Error when failing a transform job run.")))
+                  (throw t)))))
+          run-id)))))
 
 (def ^:private job-key "metabase.transforms.jobs.timeout-job")
 

src/metabase/util/compress.clj

@@ -47,13 +47,14 @@
   (with-open [tar (-> (io/input-stream archive)
                       (GzipCompressorInputStream.)
                       (TarArchiveInputStream.))]
-    (let [tar-entries (entries tar)]
+    (let [tar-entries (entries tar)
+          dst-path    (.toPath dst)]
       (count
        (for [^TarArchiveEntry e tar-entries
              :let [actual-name (last (.split (.getName e) "/"))]
              ;; skip hidden files
              :when (not (str/starts-with? actual-name "."))]
-         (let [f (io/file dst (.getName e))]
+         (let [f (.toFile (.resolveIn e dst-path))]
            (if (.isFile e)
              (io/copy tar f)
              (.mkdirs f))

test/metabase/analytics/stats_test.clj

@@ -10,6 +10,7 @@
    [metabase.channel.settings :as channel.settings]
    [metabase.config.core :as config]
    [metabase.core.core :as mbc]
+   [metabase.lib.core :as lib]
    [metabase.premium-features.settings :as premium-features.settings]
    [metabase.query-processor.util :as qp.util]
    [metabase.test :as mt]
@@ -706,7 +707,26 @@
                (#'stats/library-stats)))))))
 
 (deftest transform-metrics-test
-  (testing "ee-transform-metrics should return zeros for OSS"
-    (mt/with-empty-h2-app-db!
-      (is (= {:transforms 0, :transform_runs_last_24h 0}
-             (stats/ee-transform-metrics))))))
+  (mt/with-empty-h2-app-db!
+    (testing "with no transforms"
+      (is (=? {:transforms 0 :transform_runs_last_24h 0}
+              (#'stats/transform-metrics))))
+    (testing "with transforms and recent runs"
+      (mt/with-temp [:model/Transform transform {:target {:database (mt/id)
+                                                          :table "test_table"}
+                                                 :name "Test SQL transform"
+                                                 :source {:type "query"
+                                                          :query (lib/native-query (mt/metadata-provider) "SELECT 1")}}
+                     :model/TransformRun _ {:start_time (t/minus (t/offset-date-time) (t/hours 1))
+                                            :transform_id (:id transform)}]
+        (is (=? {:transforms 1 :transform_runs_last_24h 1}
+                (#'stats/transform-metrics)))))
+    (testing "with old transform runs"
+      (mt/with-temp [:model/Transform transform {:target {:database (mt/id)
+                                                          :table "test_table"}
+                                                 :name "Test SQL transform"
+                                                 :source {:type "query"
+                                                          :query (lib/native-query (mt/metadata-provider) "SELECT 1")}}
+                     :model/TransformRun _ {:start_time (t/minus (t/offset-date-time) (t/hours 25))
+                                            :transform_id (:id transform)}]
+        (is (zero? (:transform_runs_last_24h (#'stats/transform-metrics))))))))

test/metabase/pivot/js_test.cljs

@@ -0,0 +1,16 @@
+(ns metabase.pivot.js-test
+  (:require
+   [clojure.test :refer [deftest is testing]]
+   [metabase.pivot.js :as pivot.js]))
+
+(deftest ^:parallel columns-without-pivot-group-preserves-namespaced-keys-test
+  (testing "Namespaced keys like lib/breakout? survive the JS->CLJS->JS round-trip (#70358)"
+    (let [input    #js [#js {"name"          "CREATED_AT"
+                             "lib/breakout?" true}
+                        #js {"name" "pivot-grouping"}]
+          result   (pivot.js/columns-without-pivot-group input)
+          col      (first result)]
+      (is (= 1 (.-length result)))
+      (is (= "CREATED_AT" (unchecked-get col "name")))
+      (is (true? (unchecked-get col "lib/breakout?"))
+          "namespaced key lib/breakout? should be preserved through CLJS conversion"))))

test/metabase/search/appdb/scoring_test.clj

@@ -305,7 +305,7 @@
                   ["dataset" c3 "card unseen"]]
                  (search-results :user-recency "card" {:current-user-id user-id}))))))))
 
-(deftest ^:parallel mine-test
+(deftest mine-test
   (let [crowberto (mt/user->id :crowberto)
         rasta     (mt/user->id :rasta)]
     (with-index-contents [{:model "card" :id 1 :name "crow's fly card" :creator_id crowberto}

test/metabase/transforms/jobs_test.clj

@@ -93,6 +93,15 @@
     (mt/with-temp [:model/TransformJob job {:name "job-5" :schedule "0 0 * * * ? *"}]
       (is (= #{} (#'jobs/job-transform-ids (:id job)))))))
 
+(deftest run-job-skips-empty-transforms-test
+  (testing "run-job! returns nil and creates no job run when there are no transforms to execute"
+    (mt/with-temp [:model/TransformTag tag {:name "empty-tag"}
+                   :model/TransformJob job {:name "empty-job" :schedule "0 0 * * * ? *"}
+                   :model/TransformJobTransformTag _ {:job_id (:id job) :tag_id (:id tag) :position 0}]
+      (let [result (jobs/run-job! (:id job) {:run-method :cron})]
+        (is (nil? result))
+        (is (= 0 (t2/count :model/TransformJobRun :job_id (:id job))))))))
+
 (deftest next-transform-test
   (let [ordering {1 #{2 3}
                   2 #{3 4}

test/metabase/transforms/test_util.clj

@@ -147,6 +147,23 @@
           (throw (ex-info (format "Transform run failed with status %s" status)
                           {:resp resp :status status})))))))
 
+(defn wait-for-field
+  "Wait for a field with `field-name` to appear as active on the table named `table-name`.
+   Useful after schema changes where sync runs asynchronously."
+  [table-name field-name timeout-ms]
+  (let [timer (u/start-timer)]
+    (loop []
+      (let [table  (t2/select-one :model/Table :name table-name :db_id (mt/id))
+            field  (when table
+                     (t2/select-one :model/Field :table_id (:id table) :name field-name :active true))]
+        (cond
+          field    field
+          (> (u/since-ms timer) timeout-ms)
+          (throw (ex-info (format "Field %s on table %s did not appear after %dms" field-name table-name timeout-ms)
+                          {:table-name table-name :field-name field-name :timeout-ms timeout-ms}))
+          :else    (do (Thread/sleep 200)
+                       (recur)))))))
+
 (defn get-test-schema
   "Get the schema from the products table in the test dataset.
    This is needed for databases like BigQuery that require a schema/dataset."

test/metabase/util/compress_test.clj

@@ -6,7 +6,9 @@
    [metabase.util.compress :as u.compress])
   (:import
    (java.io File)
-   (java.nio.file Files)))
+   (java.nio.file Files)
+   (org.apache.commons.compress.archivers.tar TarArchiveEntry TarArchiveOutputStream)
+   (org.apache.commons.compress.compressors.gzip GzipCompressorOutputStream)))
 
 (set! *warn-on-reflection* true)
 
@@ -38,3 +40,31 @@
           (when (.exists archive)
             (io/delete-file archive))
           (run! io/delete-file (reverse (file-seq out))))))))
+
+(defn- create-tgz
+  "Create a tar.gz archive."
+  ^File [entry-name ^bytes content]
+  (let [archive (File/createTempFile "archive" ".tar.gz")]
+    (with-open [tar (-> (io/output-stream archive)
+                        (GzipCompressorOutputStream.)
+                        (TarArchiveOutputStream. 512 "UTF-8"))]
+      (let [entry (doto (TarArchiveEntry. ^String entry-name)
+                    (.setSize (alength content)))]
+        (.putArchiveEntry tar entry)
+        (.write tar content)
+        (.closeArchiveEntry tar)))
+    archive))
+
+(deftest untgz-path-traversal-test
+  (testing "untgz rejects tar entries with path traversal"
+    (let [content (.getBytes "content" "UTF-8")
+          out     (doto (io/file (System/getProperty "java.io.tmpdir") (mt/random-name))
+                    .mkdirs)]
+      (doseq [file-name ["../../etc/foo" "../escape" "foo/../../escape"]]
+        (let [archive (create-tgz file-name content)]
+          (try
+            (is (thrown? java.io.IOException
+                         (u.compress/untgz archive out)))
+            (finally
+              (io/delete-file archive true)
+              (run! #(io/delete-file % true) (reverse (file-seq out))))))))))