JESI-2566

.circleci/config.yml

@@ -99,6 +99,32 @@ jobs:
           command: lein check-namespace-decls
           no_output_timeout: 5m
 
+  snapshot:
+    working_directory: /home/circleci/metabase/toucan
+    docker:
+      - image: circleci/clojure:lein-2.8.1
+    steps:
+      - attach_workspace:
+          at: /home/circleci
+      - run:
+          name: Change project group
+          command: |-
+            if [ -n "$CLOJARS_GROUP" ];then
+              lein change group-id set "\"$CLOJARS_GROUP\""
+            fi
+      - run:
+          name: Append branch name and -SNAPSHOT to version
+          command: |-
+            if [ "$CLOJARS_SNAPSHOT" = 'true' ];then
+              lein change version str "\"-${CIRCLE_BRANCH}-SNAPSHOT\""
+            fi
+      - run:
+          name: Deploy to clojars
+          command: |-
+            if [ "$CLOJARS_SNAPSHOT" = 'true' ];then
+              lein deploy clojars
+            fi
+
   deploy:
     working_directory: /home/circleci/metabase/toucan
     docker:
@@ -130,6 +156,16 @@ workflows:
       - lint-namespace-decls:
           requires:
             - checkout
+      - snapshot:
+          requires:
+            - test
+            - lint-bikeshed
+            - lint-eastwood
+            - lint-docstrings
+            - lint-namespace-decls
+          filters:
+            branches:
+              ignore: master
       - deploy:
           requires:
             - test

project.clj

@@ -1,4 +1,4 @@
-(defproject toucan "1.11.0-SNAPSHOT"
+(defproject toucan "1.11.0"
   :description "Functionality for defining your application's models and querying the database."
   :url "https://github.com/metabase/toucan"
   :license {:name "Eclipse Public License"
@@ -11,7 +11,7 @@
                  [honeysql "0.9.4"]]
   :javac-options ["-target" "1.7", "-source" "1.7"]
   :aliases {"bikeshed" ["bikeshed" "--max-line-length" "118" "--var-redefs" "false"]
-            "lint" ["do" ["eastwood"] "bikeshed" "docstring-checker" "check-namespace-decls"]
+            "lint" ["do" ["eastwood"] ["bikeshed"] ["docstring-checker"] ["check-namespace-decls"]]
             "test" ["expectations"]
             "start-db" ["shell" "./start-db"] ; `lein start-db` and stop-db are conveniences for running a test database via Docker
             "stop-db" ["shell" "docker" "stop" "toucan_test"]}
@@ -23,7 +23,8 @@
                               :exclusions [org.clojure/clojure]]
                              [lein-bikeshed "0.5.1"]
                              [lein-check-namespace-decls "1.0.1"]
-                             [lein-expectations "0.0.8"]]
+                             [lein-expectations "0.0.8"]
+                             [lein-shell "0.5.0"]]
                    :jvm-opts ["-Xverify:none"]
                    :eastwood {:add-linters [:unused-locals
                                             :unused-private-vars]

src/toucan/db.clj

@@ -13,8 +13,7 @@
              [helpers :as h]]
             [toucan
              [models :as models]
-             [util :as u]])
-  (:import clojure.lang.Keyword))
+             [util :as u]]))
 
 ;;;                                                   CONFIGURATION
 ;;; ==================================================================================================================
@@ -247,18 +246,18 @@
   [& body]
   `(-do-with-debug-print-queries (fn [] ~@body)))
 
-
-(defn- honeysql->sql
+(defn honeysql->sql
   "Compile `honeysql-form` to SQL.
   This returns a vector with the SQL string as its first item and prepared statement params as the remaining items."
   [honeysql-form]
   {:pre [(map? honeysql-form)]}
   ;; Not sure *why* but without setting this binding on *rare* occasion HoneySQL will unwantedly
   ;; generate SQL for a subquery and wrap the query in parens like "(UPDATE ...)" which is invalid
-  (let [[sql & args :as sql+args] (binding [hformat/*subquery?* false]
-                                    (hsql/format honeysql-form
-                                      :quoting             (quoting-style)
-                                      :allow-dashed-names? (not (automatically-convert-dashes-and-underscores?))))]
+  (let [[sql & args :as sql+args]
+        (binding [hformat/*subquery?* false]
+          (hsql/format honeysql-form
+                       :quoting             (quoting-style)
+                       :allow-dashed-names? (not (automatically-convert-dashes-and-underscores?))))]
     (when *debug-print-queries*
       (println (pprint honeysql-form)
                (format "\n%s\n%s" (format-sql sql) args)))
@@ -448,6 +447,11 @@
       first-arg            (recur (merge honeysql-form first-arg)            butfirst)
       :else                honeysql-form)))
 
+(defn- resolve-primary-key
+  "Resolves the model and returns its primary key"
+  [model]
+  (models/primary-key (resolve-model model)))
+
 
 ;;; ### UPDATE!
 
@@ -470,11 +474,12 @@
 
   (^Boolean [model id kvs]
    {:pre [(some? id) (map? kvs) (every? keyword? (keys kvs))]}
-   (let [model (resolve-model model)
-         kvs    (-> (models/do-pre-update model (assoc kvs :id id))
-                    (dissoc :id))
-         updated? (update! model (-> (h/sset {} kvs)
-                                     (where :id id)))]
+   (let [model       (resolve-model model)
+         primary-key (models/primary-key model)
+         kvs         (-> (models/do-pre-update model (assoc kvs primary-key id))
+                         (dissoc primary-key))
+         updated?    (update! model (-> (h/sset {} kvs)
+                                        (where primary-key id)))]
         (when (and updated?
                    (method-implemented? :post-update model))
           (models/post-update (model id)))
@@ -511,7 +516,7 @@
 
 (def ^:private inserted-id-keys
   "Different possible keys that might come back for the ID of a newly inserted row. Differs by DB."
-  [ ;; Postgres, newer H2, and most others return :id
+  [;; Postgres, newer H2, and most others return :id
    :id
    ;; :generated_key is returned by MySQL
    :generated_key
@@ -524,9 +529,9 @@
 
 (defn get-inserted-id
   "Get the ID of a row inserted by `jdbc/db-do-prepared-return-keys`."
-  [insert-result]
+  [primary-key insert-result]
   (when insert-result
-    (some insert-result inserted-id-keys)))
+    (some insert-result (cons primary-key inserted-id-keys))))
 
 (defn simple-insert-many!
   "Do a simple JDBC `insert!` of multiple objects into the database.
@@ -540,12 +545,13 @@
   [model row-maps]
   {:pre [(sequential? row-maps) (every? map? row-maps)]}
   (when (seq row-maps)
-    (let [model (resolve-model model)]
+    (let [model       (resolve-model model)
+          primary-key (models/primary-key model)]
       (doall
        (for [row-map row-maps
              :let    [sql (honeysql->sql {:insert-into model, :values [row-map]})]]
-         (-> (jdbc/db-do-prepared-return-keys (connection) false sql {}) ; false = don't use a transaction
-             get-inserted-id))))))
+         (->> (jdbc/db-do-prepared-return-keys (connection) false sql {}) ; false = don't use a transaction
+              (get-inserted-id primary-key)))))))
 
 (defn insert-many!
   "Insert several new rows into the Database. Resolves `entity`, and calls `pre-insert` on each of the `row-maps`.
@@ -596,7 +602,6 @@
   ([model k v & more]
    (insert! model (apply array-map k v more))))
 
-
 ;;; ### SELECT
 
 ;; All of the following functions are based off of the old `sel` macro and can do things like select
@@ -626,8 +631,7 @@
      (select-one-id 'Database :name \"Sample Dataset\") -> 1"
   {:style/indent 1}
   [model & options]
-  (let [model (resolve-model model)]
-    (apply select-one-field :id model options)))
+  (apply select-one-field (resolve-primary-key model) model options))
 
 (defn count
   "Select the count of objects matching some condition.
@@ -676,7 +680,7 @@
      (select-ids 'Table :db_id 1) -> #{1 2 3 4}"
   {:style/indent 1}
   [model & options]
-  (apply select-field :id model options))
+  (apply select-field (resolve-primary-key model) model options))
 
 (defn select-field->field
   "Select fields `k` and `v` from objects in the database, and return them as a map from `k` to `v`.
@@ -694,29 +698,26 @@
      (select-field->id :name 'Database) -> {\"Sample Dataset\" 1, \"test-data\" 2}"
   {:style/indent 2}
   [field model & options]
-  (apply select-field->field field :id model options))
+  (apply select-field->field field (resolve-primary-key model) model options))
 
 (defn select-id->field
   "Select `field` and `:id` from objects in the database, and return them as a map from `:id` to `field`.
 
      (select-id->field :name 'Database) -> {1 \"Sample Dataset\", 2 \"test-data\"}"
   {:style/indent 2}
   [field model & options]
-  (apply select-field->field :id field model options))
-
+  (apply select-field->field (resolve-primary-key model) field model options))
 
 ;;; ### EXISTS?
 
 (defn exists?
   "Easy way to see if something exists in the DB.
 
-    (db/exists? User :id 100)
-
-  NOTE: This only works for objects that have an `:id` field."
+    (db/exists? User :id 100)"
   {:style/indent 1}
   ^Boolean [model & kvs]
-  (boolean (select-one model (apply where (h/select {} :id) kvs))))
-
+  ;TODO don't retrieve entire object, return single record from db
+  (boolean (select-one model (apply where (h/select {} (resolve-primary-key model)) kvs))))
 
 ;;; ### DELETE!
 
@@ -746,14 +747,11 @@
   be deleted), or otherwise enforce preconditions before deleting (such as refusing to delete the object if
   something else depends on it).
 
-     (delete! Database :id 1)
-
-  NOTE: This function assumes objects have an `:id` column. There's an [open
-  issue](https://github.com/metabase/toucan/issues/3) to support objects that don't have one; until that is resolved,
-  you'll have to use `simple-delete!` instead when deleting objects with no `:id`."
+     (delete! Database :id 1)"
   {:style/indent 1}
   [model & conditions]
-  (let [model (resolve-model model)]
+  (let [model       (resolve-model model)
+        primary-key (models/primary-key model)]
     (doseq [object (apply select model conditions)]
       (models/pre-delete object)
-      (simple-delete! model :id (:id object)))))
+      (simple-delete! model primary-key (primary-key object)))))

src/toucan/hydrate.clj

@@ -205,9 +205,10 @@
                                :let   [k (some result source-keys)]
                                :when  k]
                            k))
+        primary-key (models/primary-key model)
         objs        (if (seq ids)
-                      (into {} (for [item (db/select model, :id [:in ids])]
-                                 {(:id item) item}))
+                      (into {} (for [item (db/select model, primary-key [:in ids])]
+                                 {(primary-key item) item}))
                       (constantly nil))]
     (for [result results
           :let [source-id (some result source-keys)]]

src/toucan/models.clj

@@ -4,8 +4,7 @@
   (:require [clojure.walk :refer [postwalk]]
             [honeysql.format :as hformat]
             [toucan.util :as u])
-  (:import clojure.lang.IFn
-           honeysql.format.ToSql))
+  (:import honeysql.format.ToSql))
 
 ;;;                                                   Configuration
 ;;; ==================================================================================================================
@@ -188,6 +187,7 @@
      (extend (class MyModel)
        IModel
        (merge IModelDefaults {...}))"
+
   (pre-insert [this]
     "Gets called by `insert!` immediately before inserting a new object.
      This provides an opportunity to do things like encode JSON or provide default values for certain fields.
@@ -196,6 +196,14 @@
            (let [defaults {:version 1}]
              (merge defaults query))) ; set some default values")
 
+  ;TODO add support for composite keys
+  (primary-key ^clojure.lang.Keyword [this]
+    "Defines the primary key. Defaults to :id
+
+        (primary-key [_] :id)
+
+    NOTE: composite keys are currently not supported")
+
   (post-insert [this]
     "Gets called by `insert!` with an object that was newly inserted into the database.
      This provides an opportunity to trigger specific logic that should occur when an object is inserted or modify the
@@ -345,6 +353,7 @@
 (def IModelDefaults
   "Default implementations for `IModel` methods."
   {:default-fields (constantly nil)
+   :primary-key    (constantly :id)
    :types          (constantly nil)
    :properties     (constantly nil)
    :pre-insert     identity
@@ -365,7 +374,7 @@
    ((resolve 'toucan.db/select) model))
   ([model id]
    (when id
-     (invoke-model model :id id)))
+     (invoke-model model (primary-key model) id)))
   ([model k v & more]
    (apply (resolve 'toucan.db/select-one) model k v more)))
 
@@ -484,7 +493,7 @@
 
                                clojure.lang.IFn
                                ~@(ifn-invoke-forms)
-                               (~'applyTo [~'this ^ISeq ~'args]
+                               (~'applyTo [~'this ^clojure.lang.ISeq ~'args]
                                 (apply invoke-model-or-instance ~'this ~'args)))
 
         ;; Replace the implementation of `empty`. It's either this, or using the

test/toucan/db_test.clj

@@ -6,6 +6,7 @@
             [toucan.test-models
              [address :refer [Address]]
              [category :refer [Category]]
+             [phone-number :refer [PhoneNumber]]
              [user :refer [User]]
              [venue :refer [Venue]]]))
 
@@ -217,6 +218,14 @@
    (db/update! User 1 :last-name "Era")
    (db/select-one User :id 1)))
 
+(expect
+  #toucan.test_models.phone_number.PhoneNumberInstance{:number "012345678", :country_code "AU"}
+  (test/with-clean-db
+    (let [id "012345678"]
+      (db/simple-insert! PhoneNumber {:number id, :country_code "US"})
+      (db/update! PhoneNumber id :country_code "AU")
+      (db/select-one PhoneNumber :number id))))
+
 ;; Test update-where!
 (expect
  [#toucan.test_models.user.UserInstance{:id 1, :first-name "Cam", :last-name "Saul"}
@@ -295,6 +304,11 @@
  (test/with-clean-db
    (db/insert! User {:first-name "Trash", :last-name "Bird"})))
 
+(expect
+  #toucan.test_models.phone_number.PhoneNumberInstance{:number "012345678", :country_code "AU"}
+  (test/with-clean-db
+    (db/insert! PhoneNumber {:number "012345678", :country_code "AU"})))
+
 ;; The returned data must match what's been inserted in the table
 (expect
  #toucan.test_models.user.UserInstance{:id 4, :first-name "Grass", :last-name "HOPPER"}
@@ -304,7 +318,7 @@
 ;; get-inserted-id shouldn't fail if nothing is returned for some reason
 (expect
  nil
- (db/get-inserted-id nil))
+ (db/get-inserted-id :id nil))
 
 ;; Test select-one
 (expect

test/toucan/hydrate_test.clj

@@ -559,3 +559,5 @@
   (hydrate [{:type :toucan}
             {:type :pigeon}]
            :is-bird?))
+
+;TODO add test for selecting hydration for where (not= pk :id)

test/toucan/test_models/phone_number.clj

@@ -0,0 +1,6 @@
+(ns toucan.test-models.phone-number
+  (:require [toucan.models :as models]))
+
+(models/defmodel PhoneNumber :phone_numbers
+  models/IModel
+  (primary-key [_] :number))