(feat): warn on duplicate IDs and refs rather than fail (#854)

Instead of having db update operations fail, a useful error is shown, and the db updates complete. Closes #816.

CHANGELOG.md

@@ -14,6 +14,8 @@
 
 ### Bugfixes
 
+- [#854](https://github.com/org-roam/org-roam/pull/854) Warn instead of fail when duplicate refs and IDs exist.
+
 ## 1.2.0 (12-06-2020)
 
 In this release, we improved the linking process by achieving feature parity between links to files and links to headlines. Before, we used the `file:foo::*bar` format to link to the headline `bar` in file `foo`, but this was prone to breakage upon renaming the file or modifying the headline. This is not the case anymore. Now, we use `org-id` to create IDs for those headlines, which are then stored in our database to compute the relationships and jump around. Note that this will work even if you’re not using `org-id` in your global configuration for Org-mode.

org-roam-db.el

@@ -240,11 +240,25 @@ This is equivalent to removing the node from the graph."
    (list (vector file titles))))
 
 (defun org-roam-db--insert-headlines (headlines)
-  "Insert HEADLINES into the Org-roam cache."
+  "Insert HEADLINES into the Org-roam cache.
-  (org-roam-db-query
+Returns t if the insertion was successful, nil otherwise.
-   [:insert :into headlines
+Insertions can fail when there is an ID conflict."
-    :values $v1]
+  (condition-case nil
-   headlines))
+      (progn
+        (org-roam-db-query
+           [:insert :into headlines
+            :values $v1]
+           headlines)
+        t)
+    (error
+     (unless (listp headlines)
+       (setq headlines (list headlines)))
+     (lwarn '(org-roam) :error
+            (format "Duplicate IDs in %s, one of:\n\n%s\n\nskipping..."
+                    (aref (car headlines) 1)
+                    (string-join (mapcar (lambda (hl)
+                                           (aref hl 0)) headlines) "\n")))
+     nil)))
 
 (defun org-roam-db--insert-tags (file tags)
   "Insert TAGS for a FILE into the Org-roam cache."
@@ -254,12 +268,27 @@ This is equivalent to removing the node from the graph."
    (list (vector file tags))))
 
 (defun org-roam-db--insert-ref (file ref)
-  "Insert REF for FILE into the Org-roam cache."
+  "Insert REF for FILE into the Org-roam cache.
+Returns t if successful, and nil otherwise.
+Insertions can fail if the key is already in the database."
   (let ((key (cdr ref))
         (type (car ref)))
-    (org-roam-db-query
+    (condition-case nil
-     [:insert :into refs :values $v1]
+        (progn
-     (list (vector key file type)))))
+         (org-roam-db-query
+          [:insert :into refs :values $v1]
+          (list (vector key file type)))
+         t)
+      (error
+       (lwarn '(org-roam) :error
+              (format "Duplicate ref %s in:\n\nA: %s\nB: %s\n\nskipping..."
+                      key
+                      file
+                      (caar (org-roam-db-query
+                             [:select file :from refs
+                              :where (= ref $v1)]
+                             (vector key)))))
+       nil))))
 
 ;;;;; Fetching
 (defun org-roam-db--get-current-files ()
@@ -440,12 +469,9 @@ If FORCE, force a rebuild of the cache from scratch."
                   :values $v1]
                  (vector file contents-hash (list :atime atime :mtime mtime)))
                 (setq file-count (1+ file-count))
-                (when-let (headlines (org-roam--extract-headlines file))
+                (when-let ((headlines (org-roam--extract-headlines file)))
-                  (org-roam-db-query
+                  (when (org-roam-db--insert-headlines headlines)
-                   [:insert :into headlines
+                    (setq headline-count (1+ headline-count)))))))))
-                    :values $v1]
-                   headlines)
-                  (setq headline-count (1+ headline-count))))))))
       ;; Second step: Rebuild the rest
       (dolist (file org-roam-files)
         (org-roam--with-temp-buffer file
@@ -470,14 +496,9 @@ If FORCE, force a rebuild of the cache from scratch."
                   :values $v1]
                  (vector file titles))
                 (setq title-count (1+ title-count)))
-              (when-let* ((ref (org-roam--extract-ref))
+              (when-let* ((ref (org-roam--extract-ref)))
-                          (type (car ref))
+                (when (org-roam-db--insert-ref file ref)
-                          (key (cdr ref)))
+                  (setq ref-count (1+ ref-count)))))
-                (org-roam-db-query
-                 [:insert :into refs
-                  :values $v1]
-                 (vector key file type))
-                (setq ref-count (1+ ref-count))))
             (remhash file current-files))))
       (dolist (file (hash-table-keys current-files))
         ;; These files are no longer around, remove from cache...