diff --git a/src/main/logseq/cli/command/add.cljs b/src/main/logseq/cli/command/add.cljs index b3851fa2fc..4b682bb05a 100644 --- a/src/main/logseq/cli/command/add.cljs +++ b/src/main/logseq/cli/command/add.cljs @@ -11,6 +11,7 @@ [logseq.common.util.date-time :as date-time-util] [logseq.common.util.page-ref :as page-ref] [logseq.common.uuid :as common-uuid] + [logseq.db :as ldb] [logseq.db.frontend.content :as db-content] [logseq.db.frontend.property :as db-property] [logseq.db.frontend.property.type :as db-property-type] @@ -24,7 +25,55 @@ now (tc/from-date (js/Date.))] (date-time-util/format now formatter))) +(defn find-pages-by-name + "Query all live (non-recycled) pages matching a given name. Returns a vector + of entity maps. Because :block/name is not unique (a tag, property and page + can share the same name), callers should check for ambiguity." + [config repo page-name selector] + (p/let [results (transport/invoke config :thread-api/q false + [repo + [{:find [[(list 'pull '?e selector) '...]] + :in '[$ ?name] + :where '[[?e :block/name ?name]]} + (common-util/page-name-sanity-lc page-name)]])] + (vec (remove ldb/recycled? (or results []))))) + +(defn throw-ambiguous-page-error! + "Throws when multiple pages match the same name. Callers should present the + candidates so the user can rerun with --id." + [page-name matches] + (let [candidates (->> matches + (map (fn [item] + {:id (:db/id item) + :name (or (:block/title item) (:block/name item))})) + (filter :id) + vec)] + (throw (ex-info (str "multiple pages match name: " page-name "; rerun with --id") + {:code :ambiguous-page-name + :candidates candidates})))) + (defn- ensure-page! + "Ensure a page only if it is unique. Otherwise returns an ambiugous error to let user choose the specific + intended page" + [config repo page-name] + (p/let [live (find-pages-by-name config repo page-name + [:db/id :block/uuid :block/name :block/title]) + _ (when (> (count live) 1) + (throw-ambiguous-page-error! page-name live)) + page (first live)] + (if (:db/id page) + page + (let [page-name-lc (common-util/page-name-sanity-lc page-name)] + (p/let [_ (transport/invoke config :thread-api/apply-outliner-ops false + [repo [[:create-page [page-name {}]]] {}])] + (transport/invoke config :thread-api/pull false + [repo [:db/id :block/uuid :block/name :block/title] [:block/name page-name-lc]])))))) + +;; TODO: Replace uses of this fn with ensure-page! when users are able to specify ids for contexts this +;; is used in +(defn- ensure-first-page! + "Unlike ensure-page!, chooses the first random page and doesn't ensure the page is unique. Only use this + when the user unable to specificy the specific page e.g. page refs in a block/title" [config repo page-name] (let [page-name-lc (common-util/page-name-sanity-lc page-name)] (p/let [page (transport/invoke config :thread-api/pull false @@ -246,7 +295,7 @@ page-refs)] (p/let [resolved (p/all (map (fn [[_ page-name]] - (p/let [page (ensure-page! config repo page-name) + (p/let [page (ensure-first-page! config repo page-name) page-uuid (:block/uuid page)] (when-not page-uuid (throw (ex-info "page not found" @@ -705,7 +754,7 @@ (and (string? value) (common-util/uuid-string? (string/trim value))) (resolve-entity-id config repo [:block/uuid (uuid (string/trim value))]) (string? value) - (p/let [page (ensure-page! config repo value)] + (p/let [page (ensure-first-page! config repo value)] (or (:db/id page) (throw (ex-info "page not found" {:code :page-not-found :value value})))) :else diff --git a/src/main/logseq/cli/command/remove.cljs b/src/main/logseq/cli/command/remove.cljs index bb31eb791e..96123be0ad 100644 --- a/src/main/logseq/cli/command/remove.cljs +++ b/src/main/logseq/cli/command/remove.cljs @@ -1,12 +1,12 @@ (ns logseq.cli.command.remove "Remove-related CLI commands." (:require [clojure.string :as string] + [logseq.cli.command.add :as add-command] [logseq.cli.command.core :as core] [logseq.cli.command.id :as id-command] [logseq.cli.server :as cli-server] [logseq.cli.transport :as transport] [logseq.common.util :as common-util] - [logseq.db :as ldb] [promesa.core :as p])) (def ^:private remove-block-spec @@ -186,18 +186,7 @@ (defn- resolve-page-by-name [config repo name] - ;; `:block/name` is not unique — multiple pages can share the same - ;; sanity-lc'd name. Query for ALL matches via :thread-api/q rather than - ;; using :thread-api/pull (which only returns the first hit via - ;; `get-first-page-by-name`), filter out recycled entries, and let the - ;; caller error out on ambiguity instead of randomly removing one of them. - (p/let [results (transport/invoke config :thread-api/q false - [repo - [{:find [[(list 'pull '?e page-id-selector) '...]] - :in '[$ ?name] - :where '[[?e :block/name ?name]]} - (common-util/page-name-sanity-lc name)]]) - live (vec (remove ldb/recycled? (or results [])))] + (p/let [live (add-command/find-pages-by-name config repo name page-id-selector)] (cond (empty? live) nil (> (count live) 1) {:ambiguous? true :matches live} diff --git a/src/main/logseq/cli/command/upsert.cljs b/src/main/logseq/cli/command/upsert.cljs index 61b98160e6..b5eebfa2f0 100644 --- a/src/main/logseq/cli/command/upsert.cljs +++ b/src/main/logseq/cli/command/upsert.cljs @@ -709,9 +709,12 @@ (defn- ensure-page-entity! [config repo page-name] - (p/let [existing (pull-page-by-name config repo page-name - [:db/id :block/uuid :logseq.property/deleted-at])] - (if (and (:db/id existing) (not (ldb/recycled? existing))) + (p/let [live (add-command/find-pages-by-name config repo page-name + [:db/id :block/uuid]) + _ (when (> (count live) 1) + (add-command/throw-ambiguous-page-error! page-name live)) + existing (first live)] + (if (:db/id existing) existing ;; Either no page exists, or only a recycled one does. Calling ;; :create-page in both cases is correct: outliner-page/create has a @@ -1024,8 +1027,10 @@ :data {:result created-ids}})) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) (defn execute-upsert-page [action config] @@ -1064,8 +1069,10 @@ :data {:result [page-id]}}) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) (defn- normalize-status-input [value] @@ -1128,8 +1135,10 @@ :message "invalid upsert task mode"}})))) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) (defn- asset-file-exists? [path] @@ -1256,8 +1265,10 @@ :message "invalid upsert asset mode"}})) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) (defn execute-upsert-tag [action config] @@ -1313,8 +1324,10 @@ :data {:result [page-id]}}))))) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) (defn execute-upsert-property [action config] @@ -1354,5 +1367,7 @@ :message "property not found after upsert"}}))))) (p/catch (fn [e] {:status :error - :error {:code (or (get-in (ex-data e) [:code]) :exception) - :message (or (ex-message e) (str e))}})))) + :error (merge {:code (or (:code (ex-data e)) :exception) + :message (or (ex-message e) (str e))} + (when-let [candidates (:candidates (ex-data e))] + {:candidates candidates}))})))) diff --git a/src/test/logseq/cli/command/upsert_test.cljs b/src/test/logseq/cli/command/upsert_test.cljs index 74802ce0b0..6a56d47e8d 100644 --- a/src/test/logseq/cli/command/upsert_test.cljs +++ b/src/test/logseq/cli/command/upsert_test.cljs @@ -363,9 +363,12 @@ transport/invoke (fn [_ method _ args] (case method :thread-api/q - (p/resolved [:logseq.property/status.todo - :logseq.property/status.doing - :logseq.property/status.done]) + (let [[_ [_query input]] args] + (if (= input "taskhome") + (p/resolved [{:db/id 42 :block/uuid (uuid "00000000-0000-0000-0000-000000000042")}]) + (p/resolved [:logseq.property/status.todo + :logseq.property/status.doing + :logseq.property/status.done]))) :thread-api/pull (let [[_ selector lookup] args] @@ -421,6 +424,12 @@ (p/resolved (assoc config :base-url "http://example"))) transport/invoke (fn [_ method _ args] (case method + :thread-api/q + (let [[_ [_query input]] args] + (if (= input "taskhome") + (p/resolved [{:db/id 42 :block/uuid (uuid "00000000-0000-0000-0000-000000000042")}]) + (p/resolved []))) + :thread-api/pull (let [[_ selector lookup] args] (cond @@ -474,6 +483,12 @@ (p/resolved (assoc config :base-url "http://example"))) transport/invoke (fn [_ method _ args] (case method + :thread-api/q + (let [[_ [_query input]] args] + (if (= input "taskhome") + (p/resolved [{:db/id 42 :block/uuid (uuid "00000000-0000-0000-0000-000000000042")}]) + (p/resolved []))) + :thread-api/pull (let [[_ selector lookup] args] (cond diff --git a/src/test/logseq/cli/commands_test.cljs b/src/test/logseq/cli/commands_test.cljs index 7527e72964..152f3f8038 100644 --- a/src/test/logseq/cli/commands_test.cljs +++ b/src/test/logseq/cli/commands_test.cljs @@ -3351,6 +3351,10 @@ add-command/resolve-property-identifiers (fn [_ _ properties & _] (p/resolved properties)) transport/invoke (fn [_ method _ args] (case method + :thread-api/q (let [[_ [_query input]] args] + (if (= input "home") + [{:db/id 50 :block/uuid (uuid "00000000-0000-0000-0000-000000000050")}] + [])) :thread-api/pull (let [[_ _ lookup] args] (cond (= lookup [:block/name "home"]) @@ -3391,6 +3395,12 @@ add-command/resolve-property-identifiers (fn [_ _ properties & _] (p/resolved properties)) transport/invoke (fn [_ method _ args] (case method + :thread-api/q (let [[_ [_query input]] args] + (if (= input "home") + [{:db/id 50 + :block/uuid recycled-uuid + :logseq.property/deleted-at 1712000000000}] + [])) :thread-api/pull (let [[_ _ lookup] args] (cond (= lookup [:block/name "home"]) @@ -3415,6 +3425,37 @@ (p/catch (fn [e] (is false (str "unexpected error: " e)))) (p/finally done))))) +(deftest test-execute-upsert-page-errors-on-ambiguous-name + (async done + (let [action {:type :upsert-page :repo "demo" :page "c1" + :update-tags [:tag/next]}] + (-> (p/with-redefs [cli-server/list-graphs (fn [_] ["demo"]) + cli-server/ensure-server! (fn [_ _] {:base-url "http://example"}) + add-command/resolve-tags (fn [_ _ _] (p/resolved nil)) + add-command/resolve-properties (fn [_ _ _ & _] (p/resolved nil)) + add-command/resolve-property-identifiers (fn [_ _ _ & _] (p/resolved nil)) + transport/invoke (fn [_ method _ _] + (case method + :thread-api/q [{:db/id 21 + :block/name "c1" + :block/title "c1" + :block/uuid (uuid "00000000-0000-0000-0000-0000000000c1")} + {:db/id 22 + :block/name "c1" + :block/title "c1" + :block/uuid (uuid "00000000-0000-0000-0000-0000000000c2")}] + :thread-api/apply-outliner-ops + (throw (ex-info "should not modify on ambiguous match" {:method method})) + (throw (ex-info "unexpected invoke" {:method method}))))] + (p/let [result (commands/execute action {})] + (is (= :error (:status result))) + (is (= :ambiguous-page-name (get-in result [:error :code]))) + (is (= #{21 22} + (set (map :id (get-in result [:error :candidates]))))) + (is (string/includes? (get-in result [:error :message]) "rerun with --id")))) + (p/catch (fn [e] (is false (str "unexpected error: " e)))) + (p/finally done))))) + (deftest test-execute-upsert-page-by-id-rejects-recycled-page (async done (let [action {:type :upsert-page :mode :update :repo "demo" :id 50}]