service-ize

test/tbd/mcp/routes_test.clj

@@ -211,40 +211,31 @@
       (is (= 200 (:status (app {:request-method :get :uri "/api/add" :query-string "x=2&y=3"})))))))
 
 (deftest todo-tools-test
+  ;; The rules (ownership, validation, not-found) are tbd.todo.service's and tested there; this is
+  ;; the tools' side: arguments in, JSON out, the caller's identity passed on, errors as results.
   (let [{:keys [app alice bob]} (fixture)
         alice-sid (initialize app alice)
         bob-sid (initialize app bob)
-        call (fn [token sid tool arguments] (call-tool app token sid tool arguments))
-        as-alice (partial call alice alice-sid)
-        as-bob (partial call bob bob-sid)
-        result (fn [resp] (:result resp))
+        as-alice (partial call-tool app alice alice-sid)
+        as-bob (partial call-tool app bob bob-sid)
         data (fn [resp] (m/decode "application/json" (text resp)))
         error (fn [resp] (when (true? (get-in resp [:result :isError])) (text resp)))]
-    (testing "no todos yet"
-      (is (= [] (data (as-alice "list_todos" {})))))
+    (is (= [] (data (as-alice "list_todos" {}))))
     (let [created (data (as-alice "create_todo" {:title "  write tests "}))
           id (:id created)]
-      (testing "create trims the title, as the API does, and returns the todo"
-        (is (= {:title "write tests" :done false :owner-id 7} (select-keys created [:title :done :owner-id])))
-        (is (integer? id)))
-      (testing "get and list"
-        (is (= created (data (as-alice "get_todo" {:id id}))))
-        (is (= [created] (data (as-alice "list_todos" {})))))
-      (testing "update changes the title and/or done"
-        (is (= {:title "write tests" :done true} (select-keys (data (as-alice "update_todo" {:id id :done true})) [:title :done])))
-        (is (= {:title "ship it" :done true} (select-keys (data (as-alice "update_todo" {:id id :title "ship it"})) [:title :done]))))
-      (testing "todos are private to their owner, and someone else's is reported as missing"
+      (is (= {:title "write tests" :done false :owner-id 7} (select-keys created [:title :done :owner-id])))
+      (is (= created (data (as-alice "get_todo" {:id id}))))
+      (is (= [created] (data (as-alice "list_todos" {}))))
+      (is (= {:title "ship it" :done true}
+             (select-keys (data (as-alice "update_todo" {:id id :title "ship it" :done true})) [:title :done])))
+      (testing "the tools act as whoever the token belongs to"
         (is (= [] (data (as-bob "list_todos" {}))))
-        (doseq [[tool args] [["get_todo" {:id id}] ["update_todo" {:id id :done false}] ["delete_todo" {:id id}]]]
-          (is (= "todo not found" (error (as-bob tool args))) tool))
-        (is (= "ship it" (:title (data (as-alice "get_todo" {:id id})))) "and bob's attempts changed nothing"))
-      (testing "the rules of tbd.todo.core apply here too"
+        (is (= "todo not found" (error (as-bob "get_todo" {:id id})))))
+      (testing "a service error is an error result the model can read"
         (is (= "title must not be blank" (error (as-alice "create_todo" {:title "   "}))))
         (is (= "nothing to change: give a title and/or done" (error (as-alice "update_todo" {:id id})))))
-      (testing "delete"
-        (is (= "deleted" (text (as-alice "delete_todo" {:id id}))))
-        (is (= "todo not found" (error (as-alice "get_todo" {:id id}))))
-        (is (= [] (data (as-alice "list_todos" {}))))))))
+      (is (= "deleted" (text (as-alice "delete_todo" {:id id}))))
+      (is (= "todo not found" (error (as-alice "get_todo" {:id id})))))))
 
 (deftest tool-arguments-are-validated-test
   (let [{:keys [app alice]} (fixture)

test/tbd/todo/service_test.clj

@@ -0,0 +1,75 @@
+(ns tbd.todo.service-test
+  (:require [clojure.test :refer [deftest is testing]]
+            [tbd.todo.core :as todo]
+            [tbd.todo.db :as db]
+            [tbd.todo.db-test :as db-test]
+            [tbd.todo.service :as service]))
+
+;; The rules every front end (HTTP routes, MCP tools) gets: ownership, not-found and validation.
+;; They are tested here once; the front ends' tests only cover their own translation.
+
+(def alice {:id 7 :login "alice"})
+(def bob {:id 8 :login "bob"})
+
+(defn- fresh-ds [] (let [path (db-test/temp-db-path)] (delay (db/open path))))
+
+(def ^:private not-found {:error "todo not found" :kind :not-found})
+
+(deftest lifecycle-test
+  (let [ds (fresh-ds)]
+    (is (= {:ok []} (service/list-todos ds alice)))
+    (let [{created :ok} (service/create! ds alice "  buy milk ")
+          id (:id created)]
+      (testing "create trims the title, makes it the user's and returns it stored"
+        (is (= {:owner-id 7 :title "buy milk" :done false} (select-keys created [:owner-id :title :done])))
+        (is (integer? id)))
+      (is (= {:ok created} (service/find-todo ds alice id)))
+      (is (= {:ok [created]} (service/list-todos ds alice)))
+      (testing "update changes the title and/or done"
+        (is (= {:title "buy milk" :done true}
+               (select-keys (:ok (service/update! ds alice id {:done true})) [:title :done])))
+        (is (= {:title "oat milk" :done true}
+               (select-keys (:ok (service/update! ds alice id {:title "oat milk"})) [:title :done])))
+        (is (= "oat milk" (:title (:ok (service/find-todo ds alice id)))) "and it is stored"))
+      (testing "delete"
+        (is (= {:ok true} (service/delete! ds alice id)))
+        (is (= not-found (service/find-todo ds alice id)))
+        (is (= not-found (service/delete! ds alice id)))
+        (is (= {:ok []} (service/list-todos ds alice)))))))
+
+(deftest validation-test
+  (let [ds (fresh-ds)
+        id (:id (:ok (service/create! ds alice "keep")))]
+    (testing "the rules of tbd.todo.core apply, as :invalid"
+      (is (= {:error "title must not be blank" :kind :invalid} (service/create! ds alice "   ")))
+      (is (= {:error (format "title must be at most %d characters" todo/max-title-length) :kind :invalid}
+             (service/create! ds alice (apply str (repeat (inc todo/max-title-length) "x")))))
+      (is (= {:error "title must not be blank" :kind :invalid} (service/update! ds alice id {:title " "})))
+      (is (= {:error "nothing to change: give a title and/or done" :kind :invalid}
+             (service/update! ds alice id {}))))
+    (is (= ["keep"] (map :title (:ok (service/list-todos ds alice)))) "nothing above changed anything")))
+
+(deftest ownership-test
+  (let [ds (fresh-ds)
+        {mine :ok} (service/create! ds alice "alice's")
+        id (:id mine)]
+    (testing "each user has their own list"
+      (service/create! ds bob "bob's")
+      (is (= ["alice's"] (map :title (:ok (service/list-todos ds alice)))))
+      (is (= ["bob's"] (map :title (:ok (service/list-todos ds bob))))))
+    (testing "someone else's todo is reported exactly like a missing one"
+      (let [missing (service/find-todo ds bob 999999)]
+        (is (= not-found missing))
+        (is (= missing (service/find-todo ds bob id)))
+        (is (= missing (service/update! ds bob id {:title "hijacked"})))
+        (is (= missing (service/delete! ds bob id)))))
+    (is (= {:ok mine} (service/find-todo ds alice id)) "and alice's todo is untouched")))
+
+(deftest database-is-opened-on-use-test
+  (testing "nothing opens the database until a service fn needs it"
+    (let [opened (atom 0)
+          ds (delay (swap! opened inc) (db/open (db-test/temp-db-path)))]
+      (is (zero? @opened))
+      (service/list-todos ds alice)
+      (service/list-todos ds alice)
+      (is (= 1 @opened)))))