diff --git a/src/metabase/upload/parsing.clj b/src/metabase/upload/parsing.clj index 6541752b232f0ce56a7eb57b5830452fa20b1b54..97ed156e450e9f23338ab8e164f010380a9fbe54 100644 --- a/src/metabase/upload/parsing.clj +++ b/src/metabase/upload/parsing.clj @@ -5,9 +5,9 @@ [metabase.public-settings :as public-settings] [metabase.util.i18n :refer [tru]]) (:import + (java.text NumberFormat) (java.time LocalDate) (java.time.format DateTimeFormatter DateTimeFormatterBuilder ResolverStyle) - (java.text NumberFormat) (java.util Locale))) (set! *warn-on-reflection* true) @@ -162,7 +162,10 @@ (defn- parse-as-biginteger "Parses a string representing a number as a java.math.BigInteger, rounding down if necessary." [number-separators s] - (biginteger (parse-number number-separators s))) + (let [n (parse-number number-separators s)] + (when-not (zero? (mod n 1)) + (throw (IllegalArgumentException. (tru "''{0}'' is not an integer" s)))) + (biginteger n))) (defmulti upload-type->parser "Returns a function for the given `metabase.upload` column type that will parse a string value (from a CSV) into a value diff --git a/test/metabase/upload_test.clj b/test/metabase/upload_test.clj index 6b6e5decd8c59774a329617ab9ce77fc71f811aa..f8915e5d9386ff210cb590ecb51e3396b3547516 100644 --- a/test/metabase/upload_test.clj +++ b/test/metabase/upload_test.clj @@ -1600,26 +1600,35 @@ ;; for drivers that insert rows in chunks, we change the chunk size to 1 so that we can test that the ;; inserted rows are rolled back (binding [driver/*insert-chunk-rows* 1] - (doseq [{:keys [upload-type uncoerced coerced]} - [{:upload-type ::upload/int, :uncoerced "2.1", :coerced 2} - {:upload-type ::upload/int, :uncoerced "2.5", :coerced 2} - {:upload-type ::upload/int, :uncoerced "2.9", :coerced 2} + (doseq [{:keys [upload-type uncoerced coerced fail-msg] :as args} + [{:upload-type ::upload/int, :uncoerced "2.1", :fail-msg "'2.1' is not an integer"} + {:upload-type ::upload/int, :uncoerced "2.0", :coerced 2} {:upload-type ::upload/float, :uncoerced "2", :coerced 2.0} - {:upload-type ::upload/boolean, :uncoerced "0", :coerced false}]] - (testing (format "\nUploading %s into a column of type %s should be coerced to %s" - uncoerced (name upload-type) coerced) - (let [table (create-upload-table! - {:col->upload-type (ordered-map/ordered-map - (keyword upload/auto-pk-column-name) ::upload/auto-incrementing-int-pk - :test_column upload-type) - :rows []}) - csv-rows ["test_column" uncoerced] - file (csv-file-with csv-rows (mt/random-name))] - (testing "\nAppend should succeed" - (is (= {:row-count 1} - (append-csv! {:file file - :table-id (:id table)})))) - (testing "\nCheck the value was coerced correctly" + {:upload-type ::upload/boolean, :uncoerced "0", :coerced false} + {:upload-type ::upload/boolean, :uncoerced "1.0", :fail-msg "'1.0' is not a recognizable boolean"} + {:upload-type ::upload/boolean, :uncoerced "0.0", :fail-msg "'0.0' is not a recognizable boolean"}]] + (let [table (create-upload-table! + {:col->upload-type (ordered-map/ordered-map + (keyword @#'upload/auto-pk-column-name) ::upload/auto-incrementing-int-pk + :test_column upload-type) + :rows []}) + csv-rows ["test_column" uncoerced] + file (csv-file-with csv-rows (mt/random-name)) + append! (fn [] + (append-csv! {:file file + :table-id (:id table)}))] + (if (contains? args :coerced) + (testing (format "\nUploading %s into a column of type %s should be coerced to %s" + uncoerced (name upload-type) coerced) + (testing "\nAppend should succeed" + (is (= {:row-count 1} + (append!)))) (is (= [[1 coerced]] (rows-for-table table)))) - (io/delete-file file))))))))))) + (testing (format "\nUploading %s into a column of type %s should fail to coerce" + uncoerced (name upload-type)) + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + (re-pattern (str "^" fail-msg "$")) + (append!))))) + (io/delete-file file))))))))))