Merge pull request puppetlabs#117 from cprice404/feature/master/PE-28…

…43-service-get-service

(PE-2843) Make it possible to get a reference to a service from a service

src/puppetlabs/trapperkeeper/internal.clj

@@ -364,11 +364,12 @@
         ;; here we build up a map of all of the services by calling the
         ;; constructor for each one
          services-by-id (into {} (map
-                                   (fn [sd] [(s/service-id sd)
+                                   (fn [sd] [(s/service-def-id sd)
                                              ((s/service-constructor sd) graph-instance app-context)])
                                    services))
-         ordered-services (map (fn [[service-id _]] [service-id (services-by-id service-id)]) graph)
-         _ (swap! app-context assoc :ordered-services ordered-services)]
+         ordered-services (map (fn [[service-id _]] [service-id (services-by-id service-id)]) graph)]
+    (swap! app-context assoc :services-by-id services-by-id)
+    (swap! app-context assoc :ordered-services ordered-services)
     ;; finally, create the app instance
     (reify
       a/TrapperkeeperApp

src/puppetlabs/trapperkeeper/services.clj

@@ -22,16 +22,19 @@
 
 (defprotocol Service
   "Common functions available to all services"
-  (service-context [this] "Returns the context map for this service"))
+  (service-id [this] "An identifier for the service")
+  (service-context [this] "Returns the context map for this service")
+  (get-service [this service-id] "Returns the service with the given service id"))
 
 (defprotocol ServiceDefinition
   "A service definition.  This protocol is for internal use only.  The service
   is not usable until it is instantiated (via `boot!`)."
-  (service-id [this] "An identifier for the service")
+  (service-def-id [this] "An identifier for the service")
   (service-map [this] "The map of service functions for the graph")
   (service-constructor [this] "A constructor function to instantiate the service"))
 
 (def lifecycle-fn-names (map :name (vals (:sigs Lifecycle))))
+(def tk-service-fn-names (map :name (vals (:sigs Service))))
 
 (defmacro service
   "Create a Trapperkeeper ServiceDefinition.
@@ -62,7 +65,7 @@
                             (map (fn [f] [(keyword f) schema/Any])
                                  (concat lifecycle-fn-names service-fn-names)))]
     `(reify ServiceDefinition
-       (service-id [this] ~service-id)
+       (service-def-id [this] ~service-id)
 
        ;; service map for prismatic graph
        (service-map [this]
@@ -72,11 +75,17 @@
            (fnk f :- ~output-schema
               ~dependencies
               ;; create a function that exposes the service context to the service.
-              (let [~'service-context (fn [] (get ~'@context ~service-id))
-                    ~'service-id      (fn [] ~service-id)
+              (let [~'service-id      (fn [] ~service-id)
+                    ~'service-context (fn [] (get ~'@context ~service-id))
+                    ~'get-service     (fn [svc-id#] (or (get-in ~'@context [:services-by-id svc-id#])
+                                                        (throw (IllegalArgumentException.
+                                                                 (format
+                                                                   "Call to 'get-service' failed; service '%s' does not exist."
+                                                                   svc-id#)))))
                     ;; here we create an inner graph for this service.  we need
                     ;; this in order to handle deps within a single service.
                     service-map#      ~(si/prismatic-service-map
+                                         tk-service-fn-names
                                          (concat lifecycle-fn-names service-fn-names)
                                          fns-map)
                     s-graph-inst#     (if (empty? service-map#)
@@ -92,7 +101,14 @@
              ;; now we instantiate the service and define all of its protocol functions
              (reify
                Service
+               (service-id [this] ~service-id)
                (service-context [this] (get @context# ~service-id {}))
+               (get-service [this service-id]
+                 (or (get-in @context# [:services-by-id service-id])
+                     (throw (IllegalArgumentException.
+                              (format
+                                "Call to 'get-service' failed; service '%s' does not exist."
+                                service-id)))))
 
                Lifecycle
                ~@(si/protocol-fns lifecycle-fn-names fns-map)

src/puppetlabs/trapperkeeper/services_internal.clj

@@ -2,7 +2,7 @@
   (:require [clojure.walk :refer [postwalk]]
             [clojure.set :refer [difference union intersection]]
             [plumbing.core :refer [fnk]]
-            [puppetlabs.kitchensink.core :refer [keyset]]))
+            [puppetlabs.kitchensink.core :as ks]))
 
 
 (defn protocol?
@@ -161,7 +161,7 @@
   {:pre [(fns-map? fns-map)
          (symbol? fn-name)]
    :post [(fns-map? %)
-          (= (keyset %) (conj (keyset fns-map) (keyword fn-name)))]}
+          (= (ks/keyset %) (conj (ks/keyset fns-map) (keyword fn-name)))]}
   (if (contains? fns-map (keyword fn-name))
     fns-map
     (assoc fns-map (keyword fn-name)
@@ -175,8 +175,8 @@
          (every? symbol? lifecycle-fn-names)
          (fns-map? fns-map)]
    :post [(map? %)
-          (= (keyset %)
-             (union (keyset fns-map)
+          (= (ks/keyset %)
+             (union (ks/keyset fns-map)
                     (set (map keyword lifecycle-fn-names))))]}
   (reduce add-default-lifecycle-fn fns-map lifecycle-fn-names))
 
@@ -195,7 +195,7 @@
          (every? symbol? lifecycle-fn-names)
          (every? seq? fns)]
    :post [(fns-map? %)
-          (= (keyset %)
+          (= (ks/keyset %)
              (union (set (map keyword service-fn-names))
                     (set (map keyword lifecycle-fn-names))))]}
 
@@ -215,7 +215,7 @@
     (validate-provided-fns!
       service-protocol-sym
       (set (map keyword service-fn-names))
-      (difference (keyset fns-map)
+      (difference (ks/keyset fns-map)
                   (set (map keyword lifecycle-fn-names))))
     (when service-protocol-sym
       (validate-required-fns! service-protocol-sym service-fn-names fns-map))
@@ -228,9 +228,10 @@
   [service-protocol-sym]
   {:pre [((some-fn nil? symbol?) service-protocol-sym)]
    :post [(keyword? %)]}
-  (if service-protocol-sym
-    (keyword service-protocol-sym)
-    (keyword (gensym "tk-service"))))
+  (ks/without-ns
+    (if service-protocol-sym
+      (keyword service-protocol-sym)
+      (keyword (gensym "tk-service")))))
 
 (defn get-service-fn-names
   "Get a list of service fn names based on a protocol.  Returns
@@ -261,7 +262,7 @@
          (seq? forms)]
    :post [(map? %)
           (= #{:service-protocol-sym :service-id :service-fn-names
-               :dependencies :fns-map} (keyset %))]}
+               :dependencies :fns-map} (ks/keyset %))]}
   (let [[service-protocol-sym dependencies fns]
                           (find-prot-and-deps-forms! forms)
         service-id        (get-service-id service-protocol-sym)
@@ -340,29 +341,32 @@
     [@acc result]))
 
 (defn protocol-fn->graph-fn
-  "Given a list of fn names provided by a service, and a list of fn forms for a
+  "Given the list of fn names that make up the main trapperkeeper Service protocol,
+  a list of fn names provided by a service, and a list of fn forms for a
   (potentially multi-arity) fn from the service macro, create a fn form that is
   suitable for use in a prismatic graph.  Returns a map containing the following
   keys:
 
   :deps - a list of fns from the service that this fn depends on
   :f    - the modified fn form, suitable for use in the graph"
-  [fn-names sigs]
+  [tk-svc-fn-names fn-names sigs]
   {:pre [(every? symbol? fn-names)
          (seq? sigs)
          (every? seq? sigs)]
    :post [(map? %)
-          (= #{:deps :f} (keyset %))
+          (= #{:deps :f} (ks/keyset %))
           (coll? (:deps %))
           (seq? (:f %))
           (= 'fn (first (:f %)))]}
   (let [bodies (for [sig sigs]
                   ;; first we destructure the function form into its various parts
                   (let [[_ [this & fn-args] & fn-body] sig
-                        ;; now we need to transform all calls to `service-context` from
-                        ;; protocol form to prismatic form.  we don't need to track this as
-                        ;; a dependency because it will be provided by the app.
-                        [_ fn-body] (replace-fn-calls #{'service-context 'service-id} this fn-body)
+                        ;; now we need to transform all calls to the fns from the
+                        ;; Service protocol from protocol form to prismatic form.
+                        ;; we don't need to track these as dependencies because it
+                        ;; will be provided by the app.
+                        [_ fn-body] (replace-fn-calls (set tk-svc-fn-names)
+                                                      this fn-body)
                         ;; transform all the functions from the service protocol, and keep
                         ;; a list of the dependencies so that prismatic can inject them
                         [deps fn-body] (replace-fn-calls (set fn-names) this fn-body)]
@@ -371,11 +375,12 @@
      :f    (cons 'fn (map :sig bodies))}))
 
 (defn add-prismatic-service-fnk
-  "Given the name of a fn from a service protocol, convert the raw fn form provided
+  "Given the list of fn names that make up the main trapperkeeper Service protocol,
+  the name of a fn from a service protocol, convert the raw fn form provided
   to the macro into a fn form suitable for use in a prismatic graph, wrap it in a
   prismatic fnk, and add it to the fnk accumulator map.  Returns the updated
   accumulator map."
-  [fn-names fns-map fnk-acc fn-name]
+  [tk-svc-fn-names fn-names fns-map fnk-acc fn-name]
   {:pre [(every? symbol? fn-names)
          (fns-map? fns-map)
          (map? fnk-acc)
@@ -387,25 +392,27 @@
           (every? seq? (vals %))
           (every? (fn [v] (= 'plumbing.core/fnk (first v))) (vals %))]}
   (let [{:keys [deps f]} (protocol-fn->graph-fn
+                           tk-svc-fn-names
                            fn-names
                            (fns-map fn-name))]
     (assoc fnk-acc fn-name
       (list 'plumbing.core/fnk (vec deps) f))))
 
 (defn prismatic-service-map
-  "Given a list of fn names and a map of the original fn forms provided to the
+  "Given the list of fn names that make up the main trapperkeeper Service protocol,
+  a list of fn names and a map of the original fn forms provided to the
   service macro, return a map of fnks suitable for use in a prismatic graph."
-  [fn-names fns-map]
+  [tk-svc-fn-names fn-names fns-map]
   {:pre [(every? symbol? fn-names)
          (fns-map? fns-map)]
    :post [(map? %)
           (every? keyword? (keys %))
           (every? seq? (vals %))
           (every? (fn [v] (= 'plumbing.core/fnk (first v))) (vals %))
           (= (set (map keyword fn-names))
-             (keyset fns-map))]}
+             (ks/keyset fns-map))]}
   (reduce
-    (partial add-prismatic-service-fnk fn-names fns-map)
+    (partial add-prismatic-service-fnk tk-svc-fn-names fn-names fns-map)
     {}
     (map keyword fn-names)))
 

test/puppetlabs/trapperkeeper/services/nrepl/nrepl_service_test.clj

@@ -3,13 +3,13 @@
             [clojure.tools.nrepl :as repl]
             [puppetlabs.trapperkeeper.testutils.bootstrap :refer [bootstrap-services-with-cli-data]]
             [puppetlabs.trapperkeeper.app :refer [get-service]]
-            [puppetlabs.trapperkeeper.services :refer [service-id stop service-context]]
+            [puppetlabs.trapperkeeper.services :refer [service-def-id stop service-context]]
             [puppetlabs.trapperkeeper.services.nrepl.nrepl-service :refer :all]))
 
 (deftest test-nrepl-service
   (testing "An nREPL service has been started"
     (let [app      (bootstrap-services-with-cli-data [nrepl-service] {:config "./test-resources/config/nrepl/nrepl.ini"})
-          si       (get-service app (service-id nrepl-service))]
+          si       (get-service app (service-def-id nrepl-service))]
       (try
         (is (= [2] (with-open [conn (repl/connect :port 7888)]
                      (-> (repl/client conn 1000)

test/puppetlabs/trapperkeeper/services_test.clj

@@ -3,7 +3,7 @@
             [puppetlabs.trapperkeeper.services :refer
                 [ServiceDefinition Service Lifecycle
                  defservice service service-context]]
-            [puppetlabs.trapperkeeper.app :refer [TrapperkeeperApp get-service]]
+            [puppetlabs.trapperkeeper.app :as app]
             [puppetlabs.trapperkeeper.testutils.bootstrap :refer
                 [bootstrap-services-with-empty-config]]
             [schema.test :as schema-test]))
@@ -26,9 +26,9 @@
 
   (let [app (bootstrap-services-with-empty-config [hello-service])]
     (testing "app satisfies protocol"
-      (is (satisfies? TrapperkeeperApp app)))
+      (is (satisfies? app/TrapperkeeperApp app)))
 
-    (let [h-s (get-service app :HelloService)]
+    (let [h-s (app/get-service app :HelloService)]
       (testing "service satisfies all protocols"
         (is (satisfies? Lifecycle h-s))
         (is (satisfies? Service h-s))
@@ -91,9 +91,36 @@
                             [[:Service1 service1-fn]]
                             (service2-fn [this] (str "HELLO " (service1-fn))))
           app (bootstrap-services-with-empty-config [service1 service2])
-          s2 (get-service app :Service2)]
+          s2  (app/get-service app :Service2)]
       (is (= "HELLO FOO!" (service2-fn s2)))))
 
+  (testing "services should be able to retrieve instances of services that they depend on"
+    (let [service1 (service Service1
+                            []
+                            (service1-fn [this] "FOO!"))
+          service2 (service Service2
+                            [[:Service1 service1-fn]]
+                            (init [this context]
+                                  (let [s1 (get-service this :Service1)]
+                                    (assoc context :s1 s1)))
+                            (service2-fn [this] ((service-context this) :s1)))
+          app               (bootstrap-services-with-empty-config [service1 service2])
+          s2                (app/get-service app :Service2)
+          s1                (service2-fn s2)]
+      (is (satisfies? Service1 s1))
+      (is (= "FOO!" (service1-fn s1)))))
+
+  (testing "an error should be thrown if calling get-service on a non-existent service"
+    (let [service1 (service Service1
+                            []
+                            (service1-fn [this] (get-service this :NonExistent)))
+          app               (bootstrap-services-with-empty-config [service1])
+          s1                (app/get-service app :Service1)]
+      (is (thrown-with-msg?
+            IllegalArgumentException
+            #"Call to 'get-service' failed; service ':NonExistent' does not exist."
+            (service1-fn s1)))))
+
   (testing "lifecycle functions should be able to call injected functions"
     (let [service1 (service Service1
                             []
@@ -103,7 +130,7 @@
                             (init [this context] (service1-fn) context)
                             (service2-fn [this] "service2"))
           app (bootstrap-services-with-empty-config [service1 service2])
-          s2 (get-service app :Service2)]
+          s2  (app/get-service app :Service2)]
       (is (= "service2" (service2-fn s2))))))
 
 (defprotocol Service4
@@ -117,7 +144,7 @@
                       (service4-fn1 [this] "foo!")
                       (service4-fn2 [this] (str (service4-fn1 this) " bar!")))
           app       (bootstrap-services-with-empty-config [service4])
-          s4        (get-service app :Service4)]
+          s4        (app/get-service app :Service4)]
       (is (= "foo! bar!" (service4-fn2 s4))))))
 
 (deftest context-test
@@ -157,7 +184,7 @@
                             (init [this context] (assoc context :foo :bar))
                             (service1-fn [this] (reset! sfn-context (service-context this))))
           app (bootstrap-services-with-empty-config [service1])
-          s1 (get-service app :Service1)]
+          s1  (app/get-service app :Service1)]
       (service1-fn s1)
       (is (= {:foo :bar} @sfn-context))
       (is (= {:foo :bar} (service-context s1)))))
@@ -171,7 +198,7 @@
                             [[:Service1 service1-fn]]
                             (service2-fn [this] (service1-fn)))
           app (bootstrap-services-with-empty-config [service1 service2])
-          s2 (get-service app :Service2)]
+          s2  (app/get-service app :Service2)]
       (is (= :bar (service2-fn s2)))))
 
   (testing "context works correctly in service functions called by other functions in same service"
@@ -181,7 +208,7 @@
                             (service4-fn1 [this] ((service-context this) :foo))
                             (service4-fn2 [this] (service4-fn1 this)))
           app (bootstrap-services-with-empty-config [service4])
-          s4 (get-service app :Service4)]
+          s4  (app/get-service app :Service4)]
       (is (= :bar (service4-fn2 s4)))))
 
   (testing "context from other services should not be visible"
@@ -238,8 +265,8 @@
                                (service1-fn [this]
                                             [(foo 5) (foo 3 6)]))
           app         (bootstrap-services-with-empty-config [ma-service service1])
-          mas         (get-service app :MultiArityService)
-          s1          (get-service app :Service1)]
+          mas         (app/get-service app :MultiArityService)
+          s1          (app/get-service app :Service1)]
       (is (= 3 (foo mas 3)))
       (is (= 5 (foo mas 4 1)))
       (is (= [5 9] (service1-fn s1))))))