From 4374f25f455dd1ca445ce4ab080958344d42ad44 Mon Sep 17 00:00:00 2001 From: Oliver Hine Date: Thu, 24 Jul 2014 10:35:20 +0000 Subject: [PATCH 1/3] Making test work regardless of environment you're in --- src/stoic/bootstrap.clj | 3 +++ test-resources/config/test.edn | 3 +-- test/stoic/bootstrap_test.clj | 19 ++++++++++--------- test/stoic/config/file_test.clj | 2 +- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/stoic/bootstrap.clj b/src/stoic/bootstrap.clj index c7f54d4..803df74 100644 --- a/src/stoic/bootstrap.clj +++ b/src/stoic/bootstrap.clj @@ -53,9 +53,12 @@ ([system] (bootstrap (choose-supplier) system)) ([config-supplier system] + (println "system" system) (let [config-supplier-component (component/start config-supplier) component-settings (fetch-settings config-supplier-component system) + _ (println "settings" component-settings) system (inject-components component-settings system)] + (println "system" system) (bounce-components-if-config-changes! config-supplier-component system component-settings) (assoc system :stoic-config config-supplier-component)))) diff --git a/test-resources/config/test.edn b/test-resources/config/test.edn index b01148f..7f68209 100644 --- a/test-resources/config/test.edn +++ b/test-resources/config/test.edn @@ -2,5 +2,4 @@ {:consumer-portal-url "http://test.com:8080", :storm-ui-url "http://test2.com:8080", :media-url-prefix "http://images.test.com/"}, - :http-kit {:port 8080, :threads 4} - } \ No newline at end of file + :http-kit {:port 8080, :threads 4}} diff --git a/test/stoic/bootstrap_test.clj b/test/stoic/bootstrap_test.clj index db09688..d41509f 100644 --- a/test/stoic/bootstrap_test.clj +++ b/test/stoic/bootstrap_test.clj @@ -1,6 +1,7 @@ (ns stoic.bootstrap-test (:use [clojure.core.async :only [chan timeout >!! TestAsyncComponent starts stops) + (component/system-map :test1 (->TestAsyncComponent starts stops) :test2 (component/using (map->TestBarfingComponent {}) [:test1]))))] diff --git a/test/stoic/config/file_test.clj b/test/stoic/config/file_test.clj index bb30417..13fe6b7 100644 --- a/test/stoic/config/file_test.clj +++ b/test/stoic/config/file_test.clj @@ -20,4 +20,4 @@ (let [am-config-path "./test-resources/config/test.edn"] (binding [*read-config-path* (constantly am-config-path)] (let [config (:config (component/start (->FileConfigSupplier)))] - (is (not= (:http-kit @config) nil)))))) + (is (= {:port 8080 :threads 4} (:http-kit @config))))))) From caced23ad24ad02fe31d1d0412c40b42d3e5c49c Mon Sep 17 00:00:00 2001 From: Oliver Hine Date: Thu, 24 Jul 2014 11:18:40 +0000 Subject: [PATCH 2/3] Not all system components have to be associative Adding concept of application --- src/stoic/bootstrap.clj | 21 ++++----- src/stoic/config/curator.clj | 6 +-- src/stoic/config/file.clj | 51 ++++++++++++++-------- test-resources/config/test-application.edn | 2 + test-resources/config/test.edn | 7 +-- test/stoic/bootstrap_test.clj | 15 +++++++ test/stoic/config/file_test.clj | 28 +++++++++--- test/stoic/config/zk_test.clj | 2 +- 8 files changed, 90 insertions(+), 42 deletions(-) create mode 100644 test-resources/config/test-application.edn diff --git a/src/stoic/bootstrap.clj b/src/stoic/bootstrap.clj index 803df74..e688a53 100644 --- a/src/stoic/bootstrap.clj +++ b/src/stoic/bootstrap.clj @@ -8,10 +8,10 @@ [stoic.config.curator] [clojure.tools.logging :as log])) -(defn choose-supplier [] +(defn build-config-supplier [system-name] (if (file/enabled?) - (file/config-supplier) - (stoic.config.curator/config-supplier))) + (file/config-supplier system-name) + (stoic.config.curator/config-supplier system-name))) (defn- inject-components "Inject components associating in the respective settings as an atom. @@ -21,14 +21,18 @@ (reduce into [] (for [[k c] system] [k (or (and (:settings c) c) + (when-not (associative? c) c) (assoc c :settings (get component-settings k)))])))) (defn- fetch-settings "Fetch settings from the config supplier and wrap in atoms." [config-supplier system] - (into {} (for [[k c] system - :when (not (:settings c))] - [k (atom (cs/fetch config-supplier k))]))) + (let [system-settings (when-let [system-name (:name system)] + {system-name (cs/fetch config-supplier (:name system))})] + (into {} (for [[k c] system + :when (not (:settings c))] + [k (atom (merge system-settings + (cs/fetch config-supplier k)))])))) (defn- bounce-component! [config-supplier k c settings-atom] (let [settings (cs/fetch config-supplier k)] @@ -51,14 +55,11 @@ Components will be bounced when their respective settings change. Returns a SystemMap with Stoic config attached." ([system] - (bootstrap (choose-supplier) system)) + (bootstrap (build-config-supplier (:name system)) system)) ([config-supplier system] - (println "system" system) (let [config-supplier-component (component/start config-supplier) component-settings (fetch-settings config-supplier-component system) - _ (println "settings" component-settings) system (inject-components component-settings system)] - (println "system" system) (bounce-components-if-config-changes! config-supplier-component system component-settings) (assoc system :stoic-config config-supplier-component)))) diff --git a/src/stoic/config/curator.clj b/src/stoic/config/curator.clj index c7d5e2f..d6aa6a8 100644 --- a/src/stoic/config/curator.clj +++ b/src/stoic/config/curator.clj @@ -32,7 +32,7 @@ (.. client checkExists watched (usingWatcher watcher) (forPath path))) -(defrecord CuratorConfigSupplier [root] +(defrecord CuratorConfigSupplier [root system-name] stoic.protocols.config-supplier/ConfigSupplier component/Lifecycle @@ -61,5 +61,5 @@ (watcher-fn) (watch-path client path this)))))))) -(defn config-supplier [] - (CuratorConfigSupplier. (zk-root))) +(defn config-supplier [system-name] + (CuratorConfigSupplier. (zk-root) system-name)) diff --git a/src/stoic/config/file.clj b/src/stoic/config/file.clj index fa19018..e0f2dba 100644 --- a/src/stoic/config/file.clj +++ b/src/stoic/config/file.clj @@ -25,41 +25,56 @@ true (catch IllegalArgumentException e false))) -(defn read-config [file-path] +(defn- deep-merge [& maps] + (if (every? map? maps) + (apply merge-with deep-merge maps) + (last maps))) + +(defn read-config [config-files] "Reads the edn based config from the specified file" - (log/info "Reading config from file: " file-path) - (let [config (edn/read-string (slurp file-path))] + (log/info "Reading config from files: " (map #(.getAbsolutePath %) config-files)) + (let [config (apply deep-merge (map #(edn/read-string (slurp %)) config-files))] (log/debug "Config: " config) config)) -(defn- config-file-change? [config-path f] +(defn- config-file-change? [config-files f] "Filter out all file system events that do not match the stoic config file modification or creation" - (and (= config-path (.getAbsolutePath (:file f))) (not= (:action f) :delete))) + (and (contains? (set config-files) (:file f)) (not= (:action f) :delete))) -(defn reload-config! [config-atom config-path watch-fn-atom file-events] +(defn reload-config! [config-atom config-files watch-fn-atom file-events] "If the stoic config file has changed, reload the config and call the optional watch function" - (when (not-empty (filter (partial config-file-change? config-path) file-events)) + (when (not-empty (filter (partial config-file-change? config-files) file-events)) (log/info "Reloading config...") - (reset! config-atom (read-config config-path)) + (reset! config-atom (read-config config-files)) (when @watch-fn-atom (@watch-fn-atom)))) -(defrecord FileConfigSupplier [] +(defn- config-files [config-paths system-name] + (for [path config-paths + :let [f (io/file path)]] + (if (.isDirectory f) + (let [system-settings (io/file f (str (name system-name) ".edn"))] + (when (.exists system-settings) system-settings)) + f))) + +(defrecord FileConfigSupplier [system-name] stoic.protocols.config-supplier/ConfigSupplier component/Lifecycle (start [this] - (let [config-path (*read-config-path*) - config-dir (.getParentFile (io/as-file config-path))] + (let [config-paths (string/split (*read-config-path*) #":") + config-files (config-files config-paths system-name) + config-dirs (set (map #(.getParentFile %) config-files))] + (println "Config files" config-files) + (println "Config dirs" config-dirs) (try - (let [ - config-atom (atom (read-config config-path)) - watch-fn-atom (atom nil) - config-watcher (watch-dir (partial reload-config! config-atom config-path watch-fn-atom) config-dir)] + (let [config-atom (atom (read-config config-files)) + watch-fn-atom (atom nil) + config-watcher (apply watch-dir (partial reload-config! config-atom config-files watch-fn-atom) config-dirs)] (assoc this :config config-atom :config-watcher config-watcher :watch-fn watch-fn-atom)) (catch AccessDeniedException e (do - (log/fatal "Unable to assign watcher to directory " (.getAbsolutePath config-dir) " check permissions") + (log/fatal "Unable to assign watcher to directories " (map #(.getAbsolutePath %) config-dirs) " check permissions") (throw e)))))) (stop [this] @@ -73,5 +88,5 @@ (watch! [{:keys [watch-fn]} k watcher-function] (reset! watch-fn watcher-function))) -(defn config-supplier [] - (FileConfigSupplier.)) \ No newline at end of file +(defn config-supplier [system-name] + (FileConfigSupplier. system-name)) diff --git a/test-resources/config/test-application.edn b/test-resources/config/test-application.edn new file mode 100644 index 0000000..7624e67 --- /dev/null +++ b/test-resources/config/test-application.edn @@ -0,0 +1,2 @@ +{:test-application {:pqr :stu} + :http-kit {:threads 16}} diff --git a/test-resources/config/test.edn b/test-resources/config/test.edn index 7f68209..d44d96c 100644 --- a/test-resources/config/test.edn +++ b/test-resources/config/test.edn @@ -1,5 +1,2 @@ -{:sauron - {:consumer-portal-url "http://test.com:8080", - :storm-ui-url "http://test2.com:8080", - :media-url-prefix "http://images.test.com/"}, - :http-kit {:port 8080, :threads 4}} +{:ptth-tik {:abc :def} + :http-kit {:port 8080 :threads 4}} diff --git a/test/stoic/bootstrap_test.clj b/test/stoic/bootstrap_test.clj index d41509f..3be358b 100644 --- a/test/stoic/bootstrap_test.clj +++ b/test/stoic/bootstrap_test.clj @@ -110,3 +110,18 @@ [:test1]))))] (is (= :initial-value (first (alts!! [(timeout 2000) starts])))) (is (= :initial-value (first (alts!! [(timeout 2000) stops]))))))) + +(deftest system-with-a-name-has-application-settings-merged + (harness + (stoic-zk/add-to-zk client (path-for (zk-root) :test1) {:a :test-1-value}) + (stoic-zk/add-to-zk client (path-for (zk-root) :test-application) {:y :z}) + + (testing "Application settings are merged with component settings" + (let [system (component/start + (b/bootstrap + (component/system-map :name :test-application + :test1 (map->TestComponent {}))))] + + (is (= :test-application (-> system :name))) + (is (= {:a :test-1-value + :test-application {:y :z}} (-> system :test1 :settings deref))))))) diff --git a/test/stoic/config/file_test.clj b/test/stoic/config/file_test.clj index 13fe6b7..c2eb45c 100644 --- a/test/stoic/config/file_test.clj +++ b/test/stoic/config/file_test.clj @@ -1,12 +1,13 @@ (ns stoic.config.file-test - (:require [stoic.config.file :refer :all] - [stoic.config.data :refer :all] + (:require [clojure.java.io :as io] [clojure.test :refer :all] - [com.stuartsierra.component :as component])) + [com.stuartsierra.component :as component] + [stoic.config.data :refer :all] + [stoic.config.file :refer :all])) (deftest test-read-file [] (let [am-config-path "./test-resources/config/test.edn"] - (is (not= (read-config am-config-path) nil)))) + (is (not= (read-config [(io/file am-config-path)]) nil)))) (deftest test-is-not-enabled [] (is (= (enabled?) false))) @@ -19,5 +20,22 @@ (deftest test-read-config [] (let [am-config-path "./test-resources/config/test.edn"] (binding [*read-config-path* (constantly am-config-path)] - (let [config (:config (component/start (->FileConfigSupplier)))] + (let [config (:config (component/start (->FileConfigSupplier :foo)))] (is (= {:port 8080 :threads 4} (:http-kit @config))))))) + +(deftest discovers-file-called-system-name-when-supplied-with-directory + (let [am-config-path "./test-resources/config"] + (binding [*read-config-path* (constantly am-config-path)] + (let [config (:config (component/start (->FileConfigSupplier :test-application)))] + (is (= {:http-kit {:threads 16} + :test-application {:pqr :stu}} + @config)))))) + +(deftest deep-merges-multiple-files-in-config + (let [am-config-path "./test-resources/config/test.edn:./test-resources/config"] + (binding [*read-config-path* (constantly am-config-path)] + (let [config (:config (component/start (->FileConfigSupplier :test-application)))] + (is (= {:ptth-tik {:abc :def} + :http-kit {:port 8080 :threads 16} + :test-application {:pqr :stu}} + @config)))))) diff --git a/test/stoic/config/zk_test.clj b/test/stoic/config/zk_test.clj index c4d393a..8ecde6c 100644 --- a/test/stoic/config/zk_test.clj +++ b/test/stoic/config/zk_test.clj @@ -8,5 +8,5 @@ (deftest can-write-and-read-from-zookeeper (let [expected {:a :b} zk (component/start (zk-config-supplier))] - (add-to-zk (connect) (path-for :default :foo) expected) + (add-to-zk (connect) (path-for (zk-root) :foo) expected) (is (= {:a :b} (cs/fetch zk :foo))))) From df646f3bf4d182036a5c2b143548aa201e062924 Mon Sep 17 00:00:00 2001 From: Oliver Hine Date: Thu, 24 Jul 2014 14:40:15 +0000 Subject: [PATCH 3/3] Deleting raw zookeeper client in favour of curator --- src/stoic/bootstrap.clj | 1 - src/stoic/config/curator.clj | 29 ++++++++------ src/stoic/config/file.clj | 8 +--- src/stoic/config/zk.clj | 63 ------------------------------ src/stoic/merge.clj | 6 +++ test/stoic/bootstrap_test.clj | 2 +- test/stoic/config/curator_test.clj | 20 ++++++++++ test/stoic/config/zk_test.clj | 12 ------ 8 files changed, 45 insertions(+), 96 deletions(-) delete mode 100644 src/stoic/config/zk.clj create mode 100644 src/stoic/merge.clj create mode 100644 test/stoic/config/curator_test.clj delete mode 100644 test/stoic/config/zk_test.clj diff --git a/src/stoic/bootstrap.clj b/src/stoic/bootstrap.clj index e688a53..efc1308 100644 --- a/src/stoic/bootstrap.clj +++ b/src/stoic/bootstrap.clj @@ -3,7 +3,6 @@ (:require [com.stuartsierra.component :as component] [stoic.components.foo] [stoic.protocols.config-supplier :as cs] - [stoic.config.zk] [stoic.config.file :as file] [stoic.config.curator] [clojure.tools.logging :as log])) diff --git a/src/stoic/config/curator.clj b/src/stoic/config/curator.clj index d6aa6a8..2524e05 100644 --- a/src/stoic/config/curator.clj +++ b/src/stoic/config/curator.clj @@ -4,6 +4,7 @@ [stoic.protocols.config-supplier] [stoic.config.data :refer :all] [stoic.config.env :refer :all] + [stoic.merge :refer [deep-merge]] [com.stuartsierra.component :as component] [clojure.tools.logging :as log] [stoic.config.exhibitor :refer :all]) @@ -32,6 +33,20 @@ (.. client checkExists watched (usingWatcher watcher) (forPath path))) +(defn- safe-read [client path] + (when-not (.. client checkExists (forPath path)) + (.. client create (forPath path nil))) + (read-from-zk client path)) + +(defn- register-watch [client path watcher-fn] + (watch-path client path + (reify CuratorWatcher + (process [this event] + (when (= :NodeDataChanged (keyword (.. event getType name))) + (log/info "Data changed, firing watcher" event) + (watcher-fn) + (watch-path client path this)))))) + (defrecord CuratorConfigSupplier [root system-name] stoic.protocols.config-supplier/ConfigSupplier component/Lifecycle @@ -46,20 +61,10 @@ this) (fetch [{:keys [client]} k] - (let [path (path-for root k)] - (when-not (.. client checkExists (forPath path)) - (.. client create (forPath path nil))) - (read-from-zk client path))) + (safe-read client (path-for root k))) (watch! [{:keys [client]} k watcher-fn] - (let [path (path-for root k)] - (watch-path client path - (reify CuratorWatcher - (process [this event] - (when (= :NodeDataChanged (keyword (.. event getType name))) - (log/info "Data changed, firing watcher" event) - (watcher-fn) - (watch-path client path this)))))))) + (register-watch client (path-for root k) watcher-fn))) (defn config-supplier [system-name] (CuratorConfigSupplier. (zk-root) system-name)) diff --git a/src/stoic/config/file.clj b/src/stoic/config/file.clj index e0f2dba..891719c 100644 --- a/src/stoic/config/file.clj +++ b/src/stoic/config/file.clj @@ -2,6 +2,7 @@ "Loads stoic config from a file containing edn.The file is watched for changes and config is reloaded when they occur." (:import (java.nio.file AccessDeniedException)) (:require [stoic.protocols.config-supplier] + [stoic.merge :refer [deep-merge]] [clojure.tools.logging :as log] [clojure.edn :as edn] [clojure.java.io :as io] @@ -25,11 +26,6 @@ true (catch IllegalArgumentException e false))) -(defn- deep-merge [& maps] - (if (every? map? maps) - (apply merge-with deep-merge maps) - (last maps))) - (defn read-config [config-files] "Reads the edn based config from the specified file" (log/info "Reading config from files: " (map #(.getAbsolutePath %) config-files)) @@ -65,8 +61,6 @@ (let [config-paths (string/split (*read-config-path*) #":") config-files (config-files config-paths system-name) config-dirs (set (map #(.getParentFile %) config-files))] - (println "Config files" config-files) - (println "Config dirs" config-dirs) (try (let [config-atom (atom (read-config config-files)) watch-fn-atom (atom nil) diff --git a/src/stoic/config/zk.clj b/src/stoic/config/zk.clj deleted file mode 100644 index ac2589d..0000000 --- a/src/stoic/config/zk.clj +++ /dev/null @@ -1,63 +0,0 @@ -(ns stoic.config.zk - "Namespace to faciliate Stoic interaction with Zookeeper." - (:require [zookeeper :as zk] - [zookeeper.data :as zk-data] - [stoic.protocols.config-supplier] - [stoic.config.data :refer :all] - [stoic.config.env :refer :all] - [clojure.tools.logging :as log] - [com.stuartsierra.component :as component])) - -(defn connect [] - (log/info "Connecting to ZK") - (zk/connect (zk-ips) - :timeout-msec 10000)) - -(defn close [client] - (zk/close client)) - -(defn add-to-zk [client path m] - (when-not (zk/exists client path) - (zk/create-all client path :persistent? true)) - (let [v (:version (zk/exists client path))] - (zk/set-data client path (serialize-form m) v))) - -(defn read-from-zk [client path] - (deserialize-form (:data (zk/data client path)))) - -(defn profiles [client] - (map keyword (zk/children client "/stoic"))) - -(defn components [client profile] - (map keyword (zk/children client (format "/stoic/%s/components" profile)))) - -(defrecord ZkConfigSupplier [root] - stoic.protocols.config-supplier/ConfigSupplier - component/Lifecycle - - (start [{:keys [client] :as this}] - (if client this (assoc this :client (connect)))) - - (stop [{:keys [client] :as this}] - (when client - (log/info "Disconnecting from ZK") - (close client)) - this) - - (fetch [{:keys [client]} k] - (let [path (path-for root k)] - (when-not (zk/exists client path) - (zk/create-all client path :persistent? true)) - (read-from-zk client path))) - - (watch! [{:keys [client]} k watcher-fn] - (let [path (path-for root k)] - (zk/exists client path :watcher - (fn the-watcher [event] - (when (= :NodeDataChanged (:event-type event)) - (log/info "Data changed, firing watcher" event) - (watcher-fn) - (zk/exists client path :watcher the-watcher))))))) - -(defn zk-config-supplier [] - (ZkConfigSupplier. (zk-root))) diff --git a/src/stoic/merge.clj b/src/stoic/merge.clj new file mode 100644 index 0000000..ce64de7 --- /dev/null +++ b/src/stoic/merge.clj @@ -0,0 +1,6 @@ +(ns stoic.merge) + +(defn deep-merge [& maps] + (if (every? map? maps) + (apply merge-with deep-merge maps) + (last maps))) diff --git a/test/stoic/bootstrap_test.clj b/test/stoic/bootstrap_test.clj index 3be358b..28a51e1 100644 --- a/test/stoic/bootstrap_test.clj +++ b/test/stoic/bootstrap_test.clj @@ -1,6 +1,6 @@ (ns stoic.bootstrap-test (:use [clojure.core.async :only [chan timeout >!! FileConfigSupplier :test-application)))] + (is (= {:ptth-tik {:abc :def} + :http-kit {:port 8080 :threads 16} + :test-application {:pqr :stu}} + @config)))) diff --git a/test/stoic/config/zk_test.clj b/test/stoic/config/zk_test.clj deleted file mode 100644 index 8ecde6c..0000000 --- a/test/stoic/config/zk_test.clj +++ /dev/null @@ -1,12 +0,0 @@ -(ns stoic.config.zk-test - (:require [stoic.config.zk :refer :all] - [stoic.config.data :refer :all] - [clojure.test :refer :all] - [stoic.protocols.config-supplier :as cs] - [com.stuartsierra.component :as component])) - -(deftest can-write-and-read-from-zookeeper - (let [expected {:a :b} - zk (component/start (zk-config-supplier))] - (add-to-zk (connect) (path-for (zk-root) :foo) expected) - (is (= {:a :b} (cs/fetch zk :foo)))))