diff --git a/bin/reflection-linter b/bin/reflection-linter index 5de9aad925c01a641510ec8cbea2cd771af686bf..c661f4b91ef525ec0afaa4c9b40e79e7e94e1efa 100755 --- a/bin/reflection-linter +++ b/bin/reflection-linter @@ -1,6 +1,8 @@ #!/usr/bin/env bash -warnings=`lein check 2>&1 | grep Reflection | grep metabase` +echo -e "\e[1;34mChecking for reflection warnings. This may take a few minutes, so sit tight...\e[0m" + +warnings=`lein check-reflection-warnings 2>&1 | grep Reflection | grep metabase | uniq` if [ ! -z "$warnings" ]; then echo -e "\e[1;31mYour code has cased introduced some reflection warnings.\e[0m 😞" diff --git a/circle.yml b/circle.yml index a4471f099e977aacf243972c58fa5c3bbc2bc3da..dd4de332ee9794a06e15486ba99bc45e5850aad5 100644 --- a/circle.yml +++ b/circle.yml @@ -26,10 +26,10 @@ test: # 1) runs unit tests w/ Postgres local DB. Runs against H2, SQL Server # 2) runs unit tests w/ MySQL local DB. Runs against H2, Postgres, SQLite # 3) runs unit tests w/ H2 local DB. Runs against H2, Redshift, Druid - # 4) runs docstring-checker linter && Eastwood linter & Bikeshed linter && ./bin/reflection-linter + # 4) runs Eastwood linter, Bikeshed linter, docstring-checker & ./bin/reflection-linter # 5) runs JS linter + JS test # 6) runs lein uberjar. (We don't run bin/build because we're not really concerned about `npm install` (etc) in this test, which runs elsewhere) - - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood 2>&1 | grep -v Reflection && lein bikeshed 2>&1 | grep -v Reflection && ./bin/reflection-linter && lein docstring-checker ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac: + - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac: parallel: true deployment: master: diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 2d00f4b5aa35bb9fbd798574dd2ce569f6fae36d..cd7af764339f551983dabacf3136b44a2de490a7 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -142,7 +142,7 @@ when testing since they are impossible to run locally (such as Redshift and Bigq Run the linters: - lein eastwood && lein bikeshed + lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter #### Developing with Emacs @@ -159,13 +159,6 @@ You'll probably want to tell Emacs to store customizations in a different file. (load-file custom-file)) ``` -#### Checking for Out-of-Date Dependencies - - lein ancient # list all out-of-date dependencies - lein ancient latest lein-ring # list latest version of artifact lein-ring - -Will give you a list of out-of-date dependencies. - ## Documentation #### Instant Cheatsheet diff --git a/project.clj b/project.clj index 24b01a93b7aa6aea6bfb18ba2aa05fe68ccef137..8eb7dbb9984a96934346cf92d2958dfa142f7506 100644 --- a/project.clj +++ b/project.clj @@ -5,7 +5,8 @@ :description "Metabase Community Edition" :url "http://metabase.com/" :min-lein-version "2.5.0" - :aliases {"bikeshed" ["with-profile" "+bikeshed" "bikeshed" "--max-line-length" "240"] + :aliases {"bikeshed" ["bikeshed" "--max-line-length" "240"] + "check-reflection-warnings" ["with-profile" "+reflection-warnings" "check"] "test" ["with-profile" "+expectations" "expectations"] "generate-sample-dataset" ["with-profile" "+generate-sample-dataset" "run"]} :dependencies [[org.clojure/clojure "1.8.0"] @@ -76,7 +77,8 @@ :ring {:handler metabase.core/app :init metabase.core/init! :destroy metabase.core/destroy} - :eastwood {:exclude-namespaces [:test-paths] + :eastwood {:exclude-namespaces [:test-paths + metabase.driver.generic-sql] ; ISQLDriver causes Eastwood to fail. Skip this ns until issue is fixed: https://github.com/jonase/eastwood/issues/191 :add-linters [:unused-private-vars] :exclude-linters [:constant-test ; korma macros generate some forms with if statements that are always logically true or false :suspicious-expression ; core.match macros generate some forms like (and expr) which is "suspicious" @@ -88,11 +90,9 @@ :profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.12"] ; REPL <3 [expectations "2.1.3"] ; unit tests [ring/ring-mock "0.3.0"]] - :plugins [[docstring-checker "1.0.0"] ; Check that all public vars have docstrings + :plugins [[docstring-checker "1.0.0"] ; Check that all public vars have docstrings. Run with 'lein docstring-checker' [jonase/eastwood "0.2.3" :exclusions [org.clojure/clojure]] ; Linting - [lein-ancient "0.6.8" ; Check project for outdated dependencies + plugins w/ 'lein ancient' - :exclusions [org.clojure/clojure]] [lein-bikeshed "0.3.0"] ; Linting [lein-expectations "0.0.8"] ; run unit tests with 'lein expectations' [lein-instant-cheatsheet "2.2.1" ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet' @@ -104,6 +104,7 @@ "-Xmx2048m" ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI "-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) + :reflection-warnings {:global-vars {*warn-on-reflection* true}} ; run `lein check-reflection-warnings` to check for reflection warnings :expectations {:injections [(require 'metabase.test-setup)] :resource-paths ["test_resources"] :env {:mb-test-setting-1 "ABCDEFG" diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj index 8fa74b67db51eb33e87eb8923850290394cbe48e..5f7f4391c3a4df2829a3c1a533104aaca4c0091f 100644 --- a/src/metabase/driver/druid/query_processor.clj +++ b/src/metabase/driver/druid/query_processor.clj @@ -97,7 +97,7 @@ :fnAggregate "function(current, x) { return current + (parseFloat(x) || 0); }" :fnCombine "function(x, y) { return x + y; }"})) -(defn- ag:filtered [filter aggregator] {:type :filtered, :filter filter, :aggregator aggregator}) +(defn- ag:filtered [filtr aggregator] {:type :filtered, :filter filtr, :aggregator aggregator}) (defn- ag:count ([output-name] {:type :count, :name output-name}) @@ -208,11 +208,11 @@ ;;; ### handle-filter -(defn- filter:not [filter] - {:pre [filter]} - (if (= (:type filter) :not) ; it looks like "two nots don't make an identity" with druid - (:field filter) - {:type :not, :field filter})) +(defn- filter:not [filtr] + {:pre [filtr]} + (if (= (:type filtr) :not) ; it looks like "two nots don't make an identity" with druid + (:field filtr) + {:type :not, :field filtr})) (defn- filter:= [field value] {:type :selector @@ -282,8 +282,13 @@ nil (parse-filter-subclause:filter clause))) -(defn- make-intervals [min max & more] - (vec (concat [(str (or (->rvalue min) -5000) "/" (or (->rvalue max) 5000))] +(defn- make-intervals + "Make a value for the `:intervals` in a Druid query. + + ;; Return results in 2012 or 2015 + (make-intervals 2012 2013 2015 2016) -> [\"2012/2013\" \"2015/2016\"]" + [interval-min interval-max & more] + (vec (concat [(str (or (->rvalue interval-min) -5000) "/" (or (->rvalue interval-max) 5000))] (when (seq more) (apply make-intervals more))))) diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj index e8eb3fb0830ba0a56170de86efcae4cadd1d30fd..70e338332bf6aaa545d7b33997f40561a368bb8d 100644 --- a/src/metabase/driver/generic_sql/query_processor.clj +++ b/src/metabase/driver/generic_sql/query_processor.clj @@ -122,7 +122,9 @@ :when (not (contains? (set fields-fields) field))] (as (formatted field) field))))) -(defn- apply-fields [_ korma-form {fields :fields}] +(defn apply-fields + "Apply a `fields` clause to KORMA-FORM. Default implementation of `apply-fields` for SQL drivers." + [_ korma-form {fields :fields}] (apply k/fields korma-form (for [field fields] (as (formatted field) field)))) @@ -150,10 +152,14 @@ :not (kfns/pred-not (kengine/pred-map (filter-subclause->predicate subclause))) nil (filter-subclause->predicate clause))) -(defn- apply-filter [_ korma-form {clause :filter}] +(defn apply-filter + "Apply a `filter` clause to KORMA-FORM. Default implementation of `apply-filter` for SQL drivers." + [_ korma-form {clause :filter}] (k/where korma-form (filter-clause->predicate clause))) -(defn- apply-join-tables [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}] +(defn apply-join-tables + "Apply expanded query `join-tables` clause to KORMA-FORM. Default implementation of `apply-join-tables` for SQL drivers." + [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}] (loop [korma-form korma-form, [{:keys [table-name pk-field source-field schema]} & more] join-tables] (let [table-name (if (seq schema) (str schema \. table-name) @@ -168,10 +174,14 @@ (recur korma-form more) korma-form)))) -(defn- apply-limit [_ korma-form {value :limit}] +(defn apply-limit + "Apply `limit` clause to KORMA-FORM. Default implementation of `apply-limit` for SQL drivers." + [_ korma-form {value :limit}] (k/limit korma-form value)) -(defn- apply-order-by [_ korma-form {subclauses :order-by}] +(defn apply-order-by + "Apply `order-by` clause to KORMA-FORM. Default implementation of `apply-order-by` for SQL drivers." + [_ korma-form {subclauses :order-by}] (loop [korma-form korma-form, [{:keys [field direction]} & more] subclauses] (let [korma-form (k/order korma-form (formatted field) (case direction :ascending :ASC @@ -180,7 +190,9 @@ (recur korma-form more) korma-form)))) -(defn- apply-page [_ korma-form {{:keys [items page]} :page}] +(defn apply-page + "Apply `page` clause to KORMA-FORM. Default implementation of `apply-page` for SQL drivers." + [_ korma-form {{:keys [items page]} :page}] (-> korma-form (k/limit items) (k/offset (* items (dec page))))) diff --git a/src/metabase/events/activity_feed.clj b/src/metabase/events/activity_feed.clj index 38fb17d11f1d45a79be06294f6475d68f3dd7833..e4f592e0690a52a7a5b8fce65b1e5c7795774fe8 100644 --- a/src/metabase/events/activity_feed.clj +++ b/src/metabase/events/activity_feed.clj @@ -149,5 +149,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start the events listener for the activity feed." + [] (events/start-event-listener activity-feed-topics activity-feed-channel process-activity-event)) diff --git a/src/metabase/events/dependencies.clj b/src/metabase/events/dependencies.clj index 5cbaa64cc066d389e5ef0ea86261fc3124a8d8a7..addad4a8c28150db6ef55c43580122f0150114ee 100644 --- a/src/metabase/events/dependencies.clj +++ b/src/metabase/events/dependencies.clj @@ -7,7 +7,7 @@ [metric :refer [Metric]]))) -(def ^:const dependencies-topics +(def ^:private ^:const dependencies-topics "The `Set` of event topics which are subscribed to for use in dependencies tracking." #{:card-create :card-update @@ -47,5 +47,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start the events listener for dependencies topics." + [] (events/start-event-listener dependencies-topics dependencies-channel process-dependencies-event)) diff --git a/src/metabase/events/last_login.clj b/src/metabase/events/last_login.clj index f7362b5b8bd34dec8f2dd5e5cfb359c68359ebec..fe9d1b059cbf7d393316bc945f6b1a060ad1792d 100644 --- a/src/metabase/events/last_login.clj +++ b/src/metabase/events/last_login.clj @@ -37,5 +37,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start the events listener for last login events." + [] (events/start-event-listener last-login-topics last-login-channel process-last-login-event)) diff --git a/src/metabase/events/notifications.clj b/src/metabase/events/notifications.clj index 940c60e5451889ecf650e7f3390f058a0077aa9d..03c2c1090acfd0b5e3354e3c87e54011cb068781 100644 --- a/src/metabase/events/notifications.clj +++ b/src/metabase/events/notifications.clj @@ -92,5 +92,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start event listener for notifications events." + [] (events/start-event-listener notifications-topics notifications-channel process-notifications-event)) diff --git a/src/metabase/events/revision.clj b/src/metabase/events/revision.clj index 8f2fdd97bfa133fbb28f0b7007a4bbe3bb5c5b8b..d77c94dbf6d0341e8806b19f6c96b68ddb76dee7 100644 --- a/src/metabase/events/revision.clj +++ b/src/metabase/events/revision.clj @@ -77,5 +77,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start event listener for revision events." + [] (events/start-event-listener revisions-topics revisions-channel process-revision-event)) diff --git a/src/metabase/events/sync_database.clj b/src/metabase/events/sync_database.clj index 4bdde22a292c7590e5d616b52941d8281ada78e5..59a0d7b32bcdf02683c4f540f2c8fe1557af4097 100644 --- a/src/metabase/events/sync_database.clj +++ b/src/metabase/events/sync_database.clj @@ -40,5 +40,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start event listener for database sync events." + [] (events/start-event-listener sync-database-topics sync-database-channel process-sync-database-event)) diff --git a/src/metabase/events/view_log.clj b/src/metabase/events/view_log.clj index 1fbde879c10cd28ca995ed27841c96cf55e62ad7..3dd553dd87424ff1aeb345b5507b5fc92b90d044 100644 --- a/src/metabase/events/view_log.clj +++ b/src/metabase/events/view_log.clj @@ -6,7 +6,7 @@ [metabase.models.view-log :refer [ViewLog]])) -(def ^:const view-counts-topics +(def ^:private ^:const view-counts-topics "The `Set` of event topics which we subscribe to for view counting." #{:card-create :card-read @@ -46,5 +46,7 @@ ;;; ## ---------------------------------------- LIFECYLE ---------------------------------------- -(defn- events-init [] +(defn events-init + "Automatically called during startup; start the events listener for view events." + [] (events/start-event-listener view-counts-topics view-counts-channel process-view-count-event)) diff --git a/src/metabase/task/send_pulses.clj b/src/metabase/task/send_pulses.clj index f5c6a8603e7758d8ccf2533a678ce1de50d38eff..0dbdc65cc6846496b8bbcd766afee91b83fb646b 100644 --- a/src/metabase/task/send_pulses.clj +++ b/src/metabase/task/send_pulses.clj @@ -64,7 +64,9 @@ curr-monthweek (monthweek now)] (send-pulses! curr-hour curr-weekday curr-monthday curr-monthweek))) -(defn- task-init [] +(defn task-init + "Automatically called during startup; start the job for sending pulses." + [] ;; build our job (reset! send-pulses-job (jobs/build (jobs/of-type SendPulses) diff --git a/src/metabase/task/sync_databases.clj b/src/metabase/task/sync_databases.clj index 532a5e24c1a8df5b7b827b9e1c374375f0c4b351..162c43e675021ba23197d7e981a9e1ac818a2387 100644 --- a/src/metabase/task/sync_databases.clj +++ b/src/metabase/task/sync_databases.clj @@ -26,7 +26,9 @@ (catch Throwable e (log/error "Error syncing database: " (:id database) e)))))) -(defn- task-init [] +(defn task-init + "Automatically called during startup; start the job for syncing databases." + [] ;; build our job (reset! sync-databases-job (jobs/build (jobs/of-type SyncDatabases)