GNOME Bugzilla – Bug 647331
Tracker backend hits SPARQL constraint failure when saving contact with email address
Last modified: 2011-04-11 15:26:47 UTC
Created attachment 185623 [details] Tracker backend output and stack trace for the email address insert warning I've hit the attached warning when saving changes to a contact that has an email address. It doesn't happen for phone numbers, which are nearly identical (from a Folks perspective).
This is related, if not a duplicate, to bug #646831.
Created attachment 185641 [details] [review] Add convience has_email method to EmailDetails interface
Created attachment 185642 [details] [review] Handle setting an email that alreaady exists in Tracker
Created attachment 185643 [details] [review] Add a test for the problem being approached (setting existing emails)
The attached patch-set fixes the problem, I also add a new public method (to the EmailDetails interface) so that should receive special attention when reviewed.
Created attachment 185692 [details] [review] Add convience has_email method to EmailDetails interface Refreshing the patch-set to also handle setting phones.
Created attachment 185693 [details] [review] Handle setting an email or phone that alreaady exists in Tracker
Created attachment 185694 [details] [review] Add a test for setting an existing email
Review of attachment 185692 [details] [review]: + public bool has_email (string email) I think this should be private. In theory, it's generally-useful, but I think it crowds the API more than it's worth. After fixing that, please merge.
Review of attachment 185693 [details] [review]: + private async void _build_update_query ( + Tracker.Sparql.Builder builder, + owned GLib.List<FieldDetails> properties, + string contact_var, + Trf.Attrib what) "what" isn't very descriptive for a variable. Please rename to "attrib" or "attribute" + internal async void _set_unique_attrib (Folks.Persona persona, + owned GLib.List<FieldDetails> properties, Trf.Attrib what) same here After those changes, please merge This seems to also completely fix bug #646831, so please include that in the NEWS as well, and close it after closing this.
Review of attachment 185694 [details] [review]: Looks good.
(In reply to comment #9) > Review of attachment 185692 [details] [review]: > > + public bool has_email (string email) > > I think this should be private. In theory, it's generally-useful, but I think > it crowds the API more than it's worth. > > After fixing that, please merge. As discussed in IRC, has_email can't be private (since it needs to be called from tests so I'll just move this code to the specific test that needs it.
Merged: commit 6064e680ea0154579d499dd3d9140f2a9fbcef54 Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Sun Apr 10 15:18:02 2011 +0100 Add test for setting an existing email an a Trf.Persona tests/tracker/Makefile.am | 6 + tests/tracker/set-duplicate-email.vala | 226 ++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 0 deletions(-) commit 80ad989ec0f785561986ab417917a46554fc9636 Author: Raul Gutierrez Segales <raul.gutierrez.segales@collabora.co.uk> Date: Sun Apr 10 15:17:30 2011 +0100 Handle setting existing emails/phones in a Trf.Persona NEWS | 2 + backends/tracker/lib/trf-persona-store.vala | 191 ++++++++++++++++----------- 2 files changed, 117 insertions(+), 76 deletions(-)