From 11e0aa4c55c06a1a442a90f257d71a2c9a9a785a Mon Sep 17 00:00:00 2001
From: Jethro Kuan <jethrokuan95@gmail.com>
Date: Sun, 21 Jun 2020 17:00:20 +0800
Subject: [PATCH] (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   |  2 ++
 org-roam-db.el | 67 +++++++++++++++++++++++++++++++++-----------------
 2 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 51f3ff1..0f57be7 100644
--- a/CHANGELOG.md
+++ b/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.
diff --git a/org-roam-db.el b/org-roam-db.el
index 6bc01b7..3fb95ad 100644
--- a/org-roam-db.el
+++ b/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."
-  (org-roam-db-query
-   [:insert :into headlines
-    :values $v1]
-   headlines))
+  "Insert HEADLINES into the Org-roam cache.
+Returns t if the insertion was successful, nil otherwise.
+Insertions can fail when there is an ID conflict."
+  (condition-case nil
+      (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
-     [:insert :into refs :values $v1]
-     (list (vector key file type)))))
+    (condition-case nil
+        (progn
+         (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))
-                  (org-roam-db-query
-                   [:insert :into headlines
-                    :values $v1]
-                   headlines)
-                  (setq headline-count (1+ headline-count))))))))
+                (when-let ((headlines (org-roam--extract-headlines file)))
+                  (when (org-roam-db--insert-headlines 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))
-                          (type (car ref))
-                          (key (cdr ref)))
-                (org-roam-db-query
-                 [:insert :into refs
-                  :values $v1]
-                 (vector key file type))
-                (setq ref-count (1+ ref-count))))
+              (when-let* ((ref (org-roam--extract-ref)))
+                (when (org-roam-db--insert-ref file ref)
+                  (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...