(fix): org-roam-db-build-cache: fix order of processing (#1201)

* (fix): org-roam-db-build-cache: fix order of processing

Org-roam used to perform the removal of deleted files towards the end.
This can cause some issues with db rebuilds. Consider this scenario:

1. Create a file `foo.org` with `id:abc`
2. Run `org-roam-db-build-cache`
3. Delete `foo.org`
4. Create a file `bar.org` with `id:abc`
5. Run `org-roam-db-build-cache`

Here Org-roam will complain that the id `abc` is a duplicate, and error
out, because the data for `foo.org` has not yet been cleared from the
database. This PR reorders the db creation steps the following way:

1. Figure out which files no longer exist, and which files are modified
2. Clear the database for these files, leaving only files that are
unmodified in the database
3. Insert new data from modified files into the database

CHANGELOG.md

@@ -12,6 +12,7 @@
 - [#1193](https://github.com/org-roam/org-roam/issues/1193) fix `org-roam-db-build-cache` by not killing temporary buffer in `org-roam--extract-links`.
 - [#1195](https://github.com/org-roam/org-roam/issues/1195) fix ID face showing as invalid if within Org ID files, but not Org-roam's.
 - [#1199](https://github.com/org-roam/org-roam/issues/1199) make Org-roam link insertions respect `org-roam-link-title-format` everywhere.
+- [#1201](https://github.com/org-roam/org-roam/issues/1201) fix `org-roam-db-build-cache` failing in scenarios involving duplicate IDs and deleted files.
 
 ## 1.2.2 (06-10-2020)
 

org-roam-db.el

@@ -471,55 +471,55 @@ If FORCE, force a rebuild of the cache from scratch."
   (org-roam-db--close) ;; Force a reconnect
   (org-roam-db) ;; To initialize the database, no-op if already initialized
   (let* ((gc-cons-threshold org-roam-db-gc-threshold)
-	 (org-agenda-files nil)
+         (org-agenda-files nil)
          (org-roam-files (org-roam--list-all-files))
          (current-files (org-roam-db--get-current-files))
-         (file-count 0)
          (id-count 0)
          (link-count 0)
          (tag-count 0)
          (title-count 0)
          (ref-count 0)
          (deleted-count 0)
-         (processed-count 0))
+         (modified-count 0)
-    (emacsql-with-transaction (org-roam-db)
+         (modified-files nil))
-      ;; Two-step building
+    (dolist (file org-roam-files)
-      ;; First step: Rebuild files and ids
+      (let ((contents-hash (org-roam-db--file-hash file)))
-      (dolist (file org-roam-files)
+        (unless (string= (gethash file current-files)
-        (org-roam-message "Processed %s/%s files..." processed-count (length org-roam-files))
+                       contents-hash)
-        (let* ((attr (file-attributes file))
+          (push (cons file contents-hash) modified-files)))
-               (atime (file-attribute-access-time attr))
+      (remhash file current-files))
-               (mtime (file-attribute-modification-time attr)))
+    (dolist (file (hash-table-keys current-files))
-          (let ((contents-hash (org-roam-db--file-hash file)))
-            (unless (string= (gethash file current-files)
-                             contents-hash)
-              (condition-case nil
-                  (org-roam--with-temp-buffer file
-                    (org-roam-db--clear-file file)
-                    (org-roam-db-query
-                     [:insert :into files
-                      :values $v1]
-                     (vector file contents-hash (list :atime atime :mtime mtime)))
-                    (setq file-count (1+ file-count))
-                    (when org-roam-enable-headline-linking
-                      (setq id-count (+ id-count (org-roam-db--insert-ids))))
-                    (setq link-count (+ link-count (org-roam-db--insert-links)))
-                    (setq tag-count (+ tag-count (org-roam-db--insert-tags)))
-                    (setq title-count (+ title-count (org-roam-db--insert-titles)))
-                    (setq ref-count (+ ref-count (org-roam-db--insert-ref))))
-                (file-error
-                 (setq org-roam-files (remove file org-roam-files))
-                 (org-roam-db--clear-file file)
-                 (lwarn '(org-roam) :warning
-                        "Skipping unreadable file while building cache: %s" file))))
-            (remhash file current-files)
-            (setq processed-count (+ processed-count 1)))))
-      (dolist (file (hash-table-keys current-files))
         ;; These files are no longer around, remove from cache...
         (org-roam-db--clear-file file)
-        (setq deleted-count (1+ deleted-count))))
+        (setq deleted-count (1+ deleted-count)))
-    (org-roam-message "files: Δ%s, ids: Δ%s, links: Δ%s, tags: Δ%s, titles: Δ%s, refs: Δ%s, deleted: Δ%s"
+    (pcase-dolist (`(,file . _) modified-files)
-                      file-count
+      (org-roam-db--clear-file file))
+    (pcase-dolist (`(,file . ,contents-hash) modified-files)
+      (org-roam-message "Processed %s/%s modified files..." modified-count (length modified-files))
+      (let* ((attr (file-attributes file))
+             (atime (file-attribute-access-time attr))
+             (mtime (file-attribute-modification-time attr)))
+        (condition-case nil
+            (org-roam--with-temp-buffer file
+              (org-roam-db-query
+               [:insert :into files
+                :values $v1]
+               (vector file contents-hash (list :atime atime :mtime mtime)))
+              (setq modified-count (1+ modified-count))
+              (when org-roam-enable-headline-linking
+                (setq id-count (+ id-count (org-roam-db--insert-ids))))
+              (setq link-count (+ link-count (org-roam-db--insert-links)))
+              (setq tag-count (+ tag-count (org-roam-db--insert-tags)))
+              (setq title-count (+ title-count (org-roam-db--insert-titles)))
+              (setq ref-count (+ ref-count (org-roam-db--insert-ref))))
+          (file-error
+           (setq org-roam-files (remove file org-roam-files))
+           (org-roam-db--clear-file file)
+           (lwarn '(org-roam) :warning
+                  "Skipping unreadable file while building cache: %s" file)))))
+    (org-roam-message "total: Δ%s, files-modified: Δ%s, ids: Δ%s, links: Δ%s, tags: Δ%s, titles: Δ%s, refs: Δ%s, deleted: Δ%s"
+                      (length org-roam-files)
+                      modified-count
                       id-count
                       link-count
                       tag-count