Skip to content

Commit

Permalink
Fix multiple issues with global config
Browse files Browse the repository at this point in the history
- global config saved outside the app errored and had no effect on config
- global config saved in app, saved invalid state and invalid file
- removed outdated global error handling on alter-file
- Also fix validations for repo config
  • Loading branch information
logseq-cldwalker authored and andelf committed Mar 28, 2023
1 parent 07c27b0 commit 89ba13d
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 53 deletions.
2 changes: 1 addition & 1 deletion src/main/frontend/components/file.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
[path format] (:rum/args state)
repo-dir (config/get-repo-dir (state/get-current-repo))
[dir path] (cond
(string/starts-with? path (global-config-handler/global-config-dir))
(string/starts-with? path (str (global-config-handler/safe-global-config-dir)))
[nil path]

;; in-repo file, absolute path
Expand Down
3 changes: 2 additions & 1 deletion src/main/frontend/fs/watcher_handler.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@
(and (= "change" type)
(= dir (global-config-handler/global-config-dir)))
(when (= path "config.edn")
(file-handler/alter-global-file path content))
(file-handler/alter-global-file
(global-config-handler/global-config-path) content {:from-disk? true}))

(and (= "change" type)
(not exists-in-db?))
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/handler/code.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
(str (string/trim value) "\n")
{:re-render-root? true})))
;; global file
(file-handler/alter-global-file path (str (string/trim value) "\n"))))
(file-handler/alter-global-file path (str (string/trim value) "\n") {})))

:else
nil))))))
67 changes: 26 additions & 41 deletions src/main/frontend/handler/file.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,18 @@
(defn- detect-deprecations
[path content]
(when (or (= path "logseq/config.edn")
(and
(config/global-config-enabled?)
(= (path/dirname path) (global-config-handler/global-config-dir))))
(= (path/dirname path) (global-config-handler/safe-global-config-dir)))
(config-edn-common-handler/detect-deprecations path content)))

(defn- validate-file
"Returns true if valid and if false validator displays error message. Files
that are not validated just return true"
[repo path content]
[path content]
(cond
(= path (config/get-repo-config-path repo))
(= path "logseq/config.edn")
(config-edn-common-handler/validate-config-edn path content repo-config-schema/Config-edn)

(and
(config/global-config-enabled?)
(= (path/dirname path) (global-config-handler/global-config-dir)))
(= (path/dirname path) (global-config-handler/safe-global-config-dir))
(config-edn-common-handler/validate-config-edn path content global-config-schema/Config-edn)

:else
Expand All @@ -121,28 +117,27 @@
(fs/write-file! repo path-dir path content write-file-options')))

(defn alter-global-file
"Write global file, e.g. global config"
[path content]
(-> (p/let [repo (state/get-current-repo)
_ (fs/write-file! "" nil path content {:skip-compare? true})]
(cond
(and (config/global-config-enabled?)
(= path (global-config-handler/global-config-path)))
(p/do! (detect-deprecations path content)
(global-config-handler/restore-global-config!)
(validate-file repo path content)
(state/pub-event! [:shortcut/refresh]))
;; Add future global file handler here
))
(p/catch (fn [error]
(state/pub-event! [:notification/show
{:content (str "Failed to write to file " path ", error: " error)
:status :error}])
(println "Write file failed, path: " path ", content: " content)
(log/error :write/failed error)
(state/pub-event! [:capture-error
{:error error
:payload {:type :write-file/failed-for-alter-file}}])))))
"Does pre-checks on a global file, writes if it's not already written
(:from-disk? is not set) and then does post-checks. Currently only handles
global config.edn but can be extended as needed"
[path content {:keys [from-disk?]}]
(if (and path (= path (global-config-handler/safe-global-config-path)))
(do
(detect-deprecations path content)
(when (validate-file path content)
(-> (p/let [_ (when-not from-disk?
(fs/write-file! "" nil path content {:skip-compare? true}))]
(p/do! (global-config-handler/restore-global-config!)
(state/pub-event! [:shortcut/refresh])))
(p/catch (fn [error]
(state/pub-event! [:notification/show
{:content (str "Failed to write to file " path ", error: " error)
:status :error}])
(log/error :write/failed error)
(state/pub-event! [:capture-error
{:error error
:payload {:type :write-file/failed-for-alter-file}}]))))))
(log/error :msg "alter-global-file does not support this file" :file path)))

(defn alter-file
"Write any in-DB file, e.g. repo config, page, whiteboard, etc."
Expand All @@ -156,7 +151,7 @@
config-file? (= path "logseq/config.edn")
_ (when config-file?
(detect-deprecations path content))
config-valid? (and config-file? (validate-file repo path content))]
config-valid? (and config-file? (validate-file path content))]
(when (or config-valid? (not config-file?)) ; non-config file or valid config
(let [opts {:new-graph? new-graph?
:from-disk? from-disk?
Expand Down Expand Up @@ -190,16 +185,6 @@
(state/pub-event! [:shortcut/refresh]))))
(p/catch
(fn [error]
(when (and (config/global-config-enabled?)
;; Global-config not started correctly but don't
;; know root cause yet
;; https://sentry.io/organizations/logseq/issues/3587411237/events/4b5da8b8e58b4f929bd9e43562213d32/events/?cursor=0%3A0%3A1&project=5311485&statsPeriod=14d
(global-config-handler/global-config-dir-exists?)
(= path (global-config-handler/global-config-path)))
(state/pub-event! [:notification/show
{:content (str "Failed to write to file " path)
:status :error}]))

(println "Write file failed, path: " path ", content: " content)
(log/error :write/failed error)
(state/pub-event! [:capture-error
Expand Down
22 changes: 13 additions & 9 deletions src/main/frontend/handler/global_config.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,25 @@
(defonce root-dir
(atom nil))

(defn global-config-dir-exists?
"This is used in contexts where we are unusure whether global-config has been
started correctly e.g. an error handler"
(defn global-config-dir
"Fetch config dir in a global config context"
[]
(some? @root-dir))
(path/path-join @root-dir "config"))

(defn global-config-dir
(defn safe-global-config-dir
"Fetch config dir in a general context, not just for global config"
[]
(when @root-dir
(path/path-join @root-dir "config")))
(when @root-dir (global-config-dir)))

(defn global-config-path
"Fetch config path in a global config context"
[]
(path/path-join @root-dir "config" "config.edn"))

(defn safe-global-config-path
"Fetch config path in a general context, not just for global config"
[]
(when @root-dir
(path/path-join @root-dir "config" "config.edn")))
(when @root-dir (global-config-path)))

(defn set-global-config-state!
[content]
Expand Down

0 comments on commit 89ba13d

Please sign in to comment.