diff --git a/project.clj b/project.clj
index 666e4426448588c760e8725c9b91ee6ada8d20ea..7eca1755cfed6c2095c00a86b6ba104ee4d1d68d 100644
--- a/project.clj
+++ b/project.clj
@@ -88,7 +88,10 @@
:main ^:skip-aot metabase.core
:manifest {"Liquibase-Package" "liquibase.change,liquibase.changelog,liquibase.database,liquibase.parser,liquibase.precondition,liquibase.datatype,liquibase.serializer,liquibase.sqlgenerator,liquibase.executor,liquibase.snapshot,liquibase.logging,liquibase.diff,liquibase.structure,liquibase.structurecompare,liquibase.lockservice,liquibase.sdk,liquibase.ext"}
:target-path "target/%s"
- :jvm-opts ["-server" ; Run JVM in server mode as opposed to client -- see http://stackoverflow.com/questions/198577/real-differences-between-java-server-and-java-client for a good explanation of this
+ :jvm-opts ["-XX:MaxPermSize=256m" ; give the JVM a little more PermGen space to avoid PermGen OutOfMemoryErrors
+ "-Xverify:none" ; disable bytecode verification when running in dev so it starts slightly faster
+ "-XX:+CMSClassUnloadingEnabled" ; let Clojure's dynamically generated temporary classes be GC'ed from PermGen
+ "-XX:+UseConcMarkSweepGC" ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
"-Djava.awt.headless=true"] ; prevent Java icon from randomly popping up in dock when running `lein ring server`
:javac-options ["-target" "1.7", "-source" "1.7"]
:uberjar-name "metabase.jar"
@@ -122,10 +125,7 @@
:env {:mb-run-mode "dev"}
:jvm-opts ["-Dlogfile.path=target/log"
"-Xms1024m" ; give JVM a decent heap size to start with
- "-Xmx2048m" ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI
- "-Xverify:none" ; disable bytecode verification when running in dev so it starts slightly faster
- "-XX:+CMSClassUnloadingEnabled" ; let Clojure's dynamically generated temporary classes be GC'ed from PermGen
- "-XX:+UseConcMarkSweepGC"] ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
+ "-Xmx2048m"] ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI
:aot [metabase.logger]} ; Log appender class needs to be compiled for log4j to use it
:reflection-warnings {:global-vars {*warn-on-reflection* true}} ; run `lein check-reflection-warnings` to check for reflection warnings
:expectations {:injections [(require 'metabase.test-setup)]
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 3e0351d501b704ec3fa486935530b0348b3cb046..320c22b8aca3206fbe7b5f43586a177af84d9022 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -114,8 +114,8 @@
:seconds (hsql/call :to_timestamp expr)
:milliseconds (recur (hx// expr 1000) :seconds)))
-(defn- date-trunc [unit expr] (hsql/call :date_trunc (hx/literal unit) expr))
-(defn- extract [unit expr] (hsql/call :extract unit expr))
+(defn- date-trunc [unit expr] (hsql/call :date_trunc (hx/literal unit) (hx/cast :timestamp expr)))
+(defn- extract [unit expr] (hsql/call :extract unit (hx/cast :timestamp expr)))
(def ^:private extract-integer (comp hx/->integer extract))
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index fc5301a071e2cf0fe33b5f5fa40833c11afb2919..ce3aea62759dca69bcff184f877ce5c2723470ac 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -211,11 +211,11 @@
((user->client :rasta) :get 200 (str "card/" (u/get-id card))))
;; Check that a user without permissions isn't allowed to fetch the card
-(tt/expect-with-temp [Database [{database-id :id}]
- Table [{table-id :id} {:db_id database-id}]
- Card [card {:dataset_query (mbql-count-query database-id table-id)}]]
+(expect
"You don't have permissions to do that."
- (do
+ (tt/with-temp* [Database [{database-id :id}]
+ Table [{table-id :id} {:db_id database-id}]
+ Card [card {:dataset_query (mbql-count-query database-id table-id)}]]
;; revoke permissions for default group to this database
(perms/delete-related-permissions! (perms-group/all-users) (perms/object-path database-id))
;; now a non-admin user shouldn't be able to fetch this card
@@ -480,13 +480,14 @@
(db/delete! Card :id card-id)))))
;; Make sure we card creation fails if we try to set a `collection_id` we don't have permissions for
-(tt/expect-with-temp [Collection [collection]]
+(expect
"You don't have permissions to do that."
- ((user->client :rasta) :post 403 "card" {:name "My Cool Card"
- :display "scalar"
- :dataset_query (mbql-count-query (id) (id :venues))
- :visualization_settings {:global {:title nil}}
- :collection_id (u/get-id collection)}))
+ (tt/with-temp Collection [collection]
+ ((user->client :rasta) :post 403 "card" {:name "My Cool Card"
+ :display "scalar"
+ :dataset_query (mbql-count-query (id) (id :venues))
+ :visualization_settings {:global {:title nil}}
+ :collection_id (u/get-id collection)})))
;; Make sure we can change the `collection_id` of a Card if it's not in any collection
(tt/expect-with-temp [Card [card]
@@ -497,26 +498,27 @@
(db/select-one-field :collection_id Card :id (u/get-id card))))
;; Make sure we can still change *anything* for a Card if we don't have permissions for the Collection it belongs to
-(tt/expect-with-temp [Collection [collection]
- Card [card {:collection_id (u/get-id collection)}]]
+(expect
"You don't have permissions to do that."
- ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:name "Number of Blueberries Consumed Per Month"}))
+ (tt/with-temp* [Collection [collection]
+ Card [card {:collection_id (u/get-id collection)}]]
+ ((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:name "Number of Blueberries Consumed Per Month"})))
;; Make sure that we can't change the `collection_id` of a Card if we don't have write permissions for the new collection
-(tt/expect-with-temp [Collection [original-collection]
- Collection [new-collection]
- Card [card {:collection_id (u/get-id original-collection)}]]
+(expect
"You don't have permissions to do that."
- (do
+ (tt/with-temp* [Collection [original-collection]
+ Collection [new-collection]
+ Card [card {:collection_id (u/get-id original-collection)}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) original-collection)
((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})))
;; Make sure that we can't change the `collection_id` of a Card if we don't have write permissions for the current collection
-(tt/expect-with-temp [Collection [original-collection]
- Collection [new-collection]
- Card [card {:collection_id (u/get-id original-collection)}]]
+(expect
"You don't have permissions to do that."
- (do
+ (tt/with-temp* [Collection [original-collection]
+ Collection [new-collection]
+ Card [card {:collection_id (u/get-id original-collection)}]]
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) new-collection)
((user->client :rasta) :put 403 (str "card/" (u/get-id card)) {:collection_id (u/get-id new-collection)})))
@@ -606,20 +608,22 @@
(POST-card-collections! :crowberto 200 new-collection [card-1 card-2]))
;; Test that we can bulk remove some Cards from a collection
-(tt/expect-with-temp [Collection [collection]
- Card [card-1 {:collection_id (u/get-id collection)}]
- Card [card-2 {:collection_id (u/get-id collection)}]]
+(expect
[{:status "ok"}
[nil nil]]
- (POST-card-collections! :crowberto 200 nil [card-1 card-2]))
+ (tt/with-temp* [Collection [collection]
+ Card [card-1 {:collection_id (u/get-id collection)}]
+ Card [card-2 {:collection_id (u/get-id collection)}]]
+ (POST-card-collections! :crowberto 200 nil [card-1 card-2])))
;; Check that we aren't allowed to move Cards if we don't have permissions for destination collection
-(tt/expect-with-temp [Collection [collection]
- Card [card-1]
- Card [card-2]]
+(expect
["You don't have permissions to do that."
[nil nil]]
- (POST-card-collections! :rasta 403 collection [card-1 card-2]))
+ (tt/with-temp* [Collection [collection]
+ Card [card-1]
+ Card [card-2]]
+ (POST-card-collections! :rasta 403 collection [card-1 card-2])))
;; Check that we aren't allowed to move Cards if we don't have permissions for source collection
(tt/expect-with-temp [Collection [collection]
@@ -630,14 +634,14 @@
(POST-card-collections! :rasta 403 nil [card-1 card-2]))
;; Check that we aren't allowed to move Cards if we don't have permissions for the Card
-(tt/expect-with-temp [Collection [collection]
- Database [database]
- Table [table {:db_id (u/get-id database)}]
- Card [card-1 {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]
- Card [card-2 {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]]
+(expect
["You don't have permissions to do that."
[nil nil]]
- (do
+ (tt/with-temp* [Collection [collection]
+ Database [database]
+ Table [table {:db_id (u/get-id database)}]
+ Card [card-1 {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]
+ Card [card-2 {:dataset_query (mbql-count-query (u/get-id database) (u/get-id table))}]]
(perms/revoke-permissions! (perms-group/all-users) (u/get-id database))
(perms/grant-collection-readwrite-permissions! (perms-group/all-users) collection)
(POST-card-collections! :rasta 403 collection [card-1 card-2])))
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index ab0e165b7ef06d1808e41d2f1a076e62f9ee9f90..e461c3bc0010030fa5ed4ed16e4d429952046ac5 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -18,27 +18,27 @@
((user->client :crowberto) :get 200 "collection"))
;; check that we don't see collections if we don't have permissions for them
-(tt/expect-with-temp [Collection [collection-1 {:name "Collection 1"}]
- Collection [collection-2 {:name "Collection 2"}]]
+(expect
["Collection 1"]
- (do
+ (tt/with-temp* [Collection [collection-1 {:name "Collection 1"}]
+ Collection [collection-2 {:name "Collection 2"}]]
(perms/grant-collection-read-permissions! (group/all-users) collection-1)
(map :name ((user->client :rasta) :get 200 "collection"))))
;; check that we don't see collections if they're archived
-(tt/expect-with-temp [Collection [collection-1 {:name "Archived Collection", :archived true}]
- Collection [collection-2 {:name "Regular Collection"}]]
+(expect
["Regular Collection"]
- (do
+ (tt/with-temp* [Collection [collection-1 {:name "Archived Collection", :archived true}]
+ Collection [collection-2 {:name "Regular Collection"}]]
(perms/grant-collection-read-permissions! (group/all-users) collection-1)
(perms/grant-collection-read-permissions! (group/all-users) collection-2)
(map :name ((user->client :rasta) :get 200 "collection"))))
;; Check that if we pass `?archived=true` we instead see archived cards
-(tt/expect-with-temp [Collection [collection-1 {:name "Archived Collection", :archived true}]
- Collection [collection-2 {:name "Regular Collection"}]]
+(expect
["Archived Collection"]
- (do
+ (tt/with-temp* [Collection [collection-1 {:name "Archived Collection", :archived true}]
+ Collection [collection-2 {:name "Regular Collection"}]]
(perms/grant-collection-read-permissions! (group/all-users) collection-1)
(perms/grant-collection-read-permissions! (group/all-users) collection-2)
(map :name ((user->client :rasta) :get 200 "collection" :archived :true))))
@@ -100,7 +100,8 @@
{:name "My Beautiful Collection", :color "#ABCDEF"}))
;; check that non-admins aren't allowed to update a collection
-(tt/expect-with-temp [Collection [collection]]
+(expect
"You don't have permissions to do that."
- ((user->client :rasta) :put 403 (str "collection/" (u/get-id collection))
- {:name "My Beautiful Collection", :color "#ABCDEF"}))
+ (tt/with-temp Collection [collection]
+ ((user->client :rasta) :put 403 (str "collection/" (u/get-id collection))
+ {:name "My Beautiful Collection", :color "#ABCDEF"})))
diff --git a/test/metabase/api/label_test.clj b/test/metabase/api/label_test.clj
index b15fc75a6229b5db37d3258082eedb1e53a87d0c..8c7e20fe4a452526e289302802b2c3480ad7936c 100644
--- a/test/metabase/api/label_test.clj
+++ b/test/metabase/api/label_test.clj
@@ -9,8 +9,8 @@
;;; GET /api/label -- list all labels
(tt/expect-with-temp [Label [{label-1-id :id} {:name "Toucan-Approved"}]
- Label [{label-2-id :id} {:name "non-Toucan-Approved"}]]
- [{:id label-2-id, :name "non-Toucan-Approved", :slug "non_toucan_approved", :icon nil} ; should be sorted by name, case-insensitive
+ Label [{label-2-id :id} {:name "non-Toucan-Approved"}]]
+ [{:id label-2-id, :name "non-Toucan-Approved", :slug "non_toucan_approved", :icon nil} ; should be sorted by name, case-insensitive
{:id label-1-id, :name "Toucan-Approved", :slug "toucan_approved", :icon nil}]
((user->client :rasta) :get 200, "label"))
@@ -27,7 +27,8 @@
((user->client :rasta) :put 200, (str "label/" label-id) {:name "Bird-Friendly"}))
;;; DELETE /api/label/:id -- delete a label
-(tt/expect-with-temp [Label [{label-id :id} {:name "This will make the toucan very cross!"}]]
+(expect
nil
- (do ((user->client :rasta) :delete 204, (str "label/" label-id))
- (Label label-id)))
+ (tt/with-temp Label [{label-id :id} {:name "This will make the toucan very cross!"}]
+ ((user->client :rasta) :delete 204, (str "label/" label-id))
+ (Label label-id)))
diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj
index ab1feee536aa6b5583e6019f3a95c7b1d93e9042..499787cb67e19f08c799e8b1f00c2950ae4a7d7a 100644
--- a/test/metabase/api/pulse_test.clj
+++ b/test/metabase/api/pulse_test.clj
@@ -174,9 +174,9 @@
;; ## DELETE /api/pulse/:id
-(tt/expect-with-temp [Pulse [pulse]]
+(expect
nil
- (do
+ (tt/with-temp Pulse [pulse]
((user->client :rasta) :delete 204 (format "pulse/%d" (:id pulse)))
(pulse/retrieve-pulse (:id pulse))))
diff --git a/test/metabase/api/segment_test.clj b/test/metabase/api/segment_test.clj
index 59d61adfbab62d377505ef56a83b3cbda14436be..2a2582f4e07b41cefaa32f1e03d82e9b504a6b89 100644
--- a/test/metabase/api/segment_test.clj
+++ b/test/metabase/api/segment_test.clj
@@ -372,10 +372,11 @@
;;; PUT /api/segment/id. Can I update a segment's name without specifying `:points_of_interest` and `:show_in_getting_started`?
-(tt/expect-with-temp [Segment [segment]]
- :ok
- (do ((user->client :crowberto) :put 200 (str "segment/" (u/get-id segment))
- {:name "Cool name"
- :revision_message "WOW HOW COOL"
- :definition {}})
- :ok))
+(expect
+ (tt/with-temp Segment [segment]
+ ;; just make sure API call doesn't barf
+ ((user->client :crowberto) :put 200 (str "segment/" (u/get-id segment))
+ {:name "Cool name"
+ :revision_message "WOW HOW COOL"
+ :definition {}})
+ true))
diff --git a/test/metabase/query_processor_test/filter_test.clj b/test/metabase/query_processor_test/filter_test.clj
index 1061d553afb9f392d99fb7b6458051c5f892fd51..404115466401a966c31787b8288213f905f84c58 100644
--- a/test/metabase/query_processor_test/filter_test.clj
+++ b/test/metabase/query_processor_test/filter_test.clj
@@ -343,3 +343,11 @@
(ql/aggregation (ql/count))
(ql/filter (ql/and (ql/not (ql/> $id 32))
(ql/contains $name "BBQ")))))))
+
+
+;; make sure that filtering with dates truncating to minutes works (#4632)
+(expect-with-non-timeseries-dbs [107] (first-row
+ (format-rows-by [int]
+ (data/run-query checkins
+ (ql/aggregation (ql/count))
+ (ql/filter (ql/between (ql/datetime-field $date :minute) "2015-01-01T12:30:00" "2015-05-31"))))))
diff --git a/test/metabase/sync_database/analyze_test.clj b/test/metabase/sync_database/analyze_test.clj
index 6d829ad069d7216467a1263ab849f530a39ca47a..fe57c4d943a98aa1e62861684171ef3525b9b9fe 100644
--- a/test/metabase/sync_database/analyze_test.clj
+++ b/test/metabase/sync_database/analyze_test.clj
@@ -1,10 +1,20 @@
(ns metabase.sync-database.analyze-test
(:require [clojure.string :as str]
[expectations :refer :all]
+ [metabase
+ [driver :as driver]
+ [util :as u]]
[metabase.db.metadata-queries :as metadata-queries]
+ [metabase.models
+ [field :refer [Field]]
+ [table :as table :refer [Table]]]
[metabase.sync-database.analyze :refer :all]
- [metabase.test.util :as tu]))
-
+ [metabase.test
+ [data :as data]
+ [util :as tu]]
+ [metabase.test.data.users :refer :all]
+ [toucan.db :as db]
+ [toucan.util.test :as tt]))
;; test:cardinality-and-extract-field-values
;; (#2332) check that if field values are long we skip over them
@@ -22,8 +32,8 @@
;;; ## mark-json-field!
-(tu/resolve-private-vars metabase.sync-database.analyze values-are-valid-json?)
-(tu/resolve-private-vars metabase.sync-database.analyze values-are-valid-emails?)
+(tu/resolve-private-vars metabase.sync-database.analyze
+ values-are-valid-json? values-are-valid-emails?)
(def ^:const ^:private fake-values-seq-json
"A sequence of values that should be marked is valid JSON.")
@@ -72,3 +82,85 @@
(expect false (values-are-valid-emails? [100]))
(expect false (values-are-valid-emails? ["true"]))
(expect false (values-are-valid-emails? ["false"]))
+
+;; Tests to avoid analyzing hidden tables
+(defn- unanalyzed-fields-count [table]
+ (assert (pos? ;; don't let ourselves be fooled if the test passes because the table is
+ ;; totally broken or has no fields. Make sure we actually test something
+ (db/count Field :table_id (u/get-id table))))
+ (db/count Field :last_analyzed nil, :table_id (u/get-id table)))
+
+(defn- latest-sync-time [table]
+ (db/select-one-field :last_analyzed Field
+ :last_analyzed [:not= nil]
+ :table_id (u/get-id table)
+ {:order-by [[:last_analyzed :desc]]}))
+
+(defn- set-table-visibility-type! [table visibility-type]
+ ((user->client :crowberto) :put 200 (format "table/%d" (:id table)) {:display_name "hiddentable"
+ :entity_type "person"
+ :visibility_type visibility-type
+ :description "What a nice table!"}))
+
+(defn- api-sync! [table]
+ ((user->client :crowberto) :post 200 (format "database/%d/sync" (:db_id table))))
+
+(defn- analyze! [table]
+ (let [db-id (:db_id table)]
+ (analyze-data-shape-for-tables! (driver/database-id->driver db-id) {:id db-id})))
+
+;; expect all the kinds of hidden tables to stay un-analyzed through transitions and repeated syncing
+(expect
+ 1
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+ (set-table-visibility-type! table "hidden")
+ (api-sync! table)
+ (set-table-visibility-type! table "cruft")
+ (set-table-visibility-type! table "cruft")
+ (api-sync! table)
+ (set-table-visibility-type! table "technical")
+ (api-sync! table)
+ (set-table-visibility-type! table "technical")
+ (api-sync! table)
+ (api-sync! table)
+ (unanalyzed-fields-count table)))
+
+;; same test not coming through the api
+(expect
+ 1
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+ (set-table-visibility-type! table "hidden")
+ (analyze! table)
+ (set-table-visibility-type! table "cruft")
+ (set-table-visibility-type! table "cruft")
+ (analyze! table)
+ (set-table-visibility-type! table "technical")
+ (analyze! table)
+ (set-table-visibility-type! table "technical")
+ (analyze! table)
+ (analyze! table)
+ (unanalyzed-fields-count table)))
+
+;; un-hiding a table should cause it to be analyzed
+(expect
+ 0
+ (tt/with-temp* [Table [table {:rows 15}]
+ Field [field {:table_id (:id table)}]]
+ (set-table-visibility-type! table "hidden")
+ (set-table-visibility-type! table nil)
+ (unanalyzed-fields-count table)))
+
+;; re-hiding a table should not cause it to be analyzed
+(expect
+ ;; create an initially hidden table
+ (tt/with-temp* [Table [table {:rows 15, :visibility_type "hidden"}]
+ Field [field {:table_id (:id table)}]]
+ ;; switch the table to visible (triggering a sync) and get the last sync time
+ (let [last-sync-time (do (set-table-visibility-type! table nil)
+ (latest-sync-time table))]
+ ;; now make it hidden again
+ (set-table-visibility-type! table "hidden")
+ ;; sync time shouldn't change
+ (= last-sync-time (latest-sync-time table)))))
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index 09a49d748858a05f77da2fb9f6ead44c2676e365..a02c1905a37d7fed7afc45e0eea83370be57b0c1 100644
--- a/test/metabase/test/data.clj
+++ b/test/metabase/test/data.clj
@@ -9,6 +9,7 @@
[query-processor :as qp]
[sync-database :as sync-database]
[util :as u]]
+ metabase.driver.h2
[metabase.models
[database :refer [Database]]
[field :as field :refer [Field]]
@@ -19,6 +20,7 @@
[metabase.test.data
[dataset-definitions :as defs]
[datasets :refer [*driver*]]
+ h2
[interface :as i]]
[schema.core :as s]
[toucan.db :as db])