lei_mail_sync: account for non-unique cases

NNTP servers, IMAP servers, and various MUAs may recycle
"unique" identifiers due to software bugs or careless BOFHs.
Warn about them, but always be prepared to account for them.

6 files changed, 34 insertions, 29 deletions

diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 40530914..3c30db8d 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -37,8 +37,12 @@ sub pmdir_cb { # called via wq_io_do from LeiPmdir->each_mdir_fn
         substr($folder, 0, 0) = 'maildir:'; # add prefix
         my $lse = $self->{lse} //= $self->{lei}->{sto}->search;
         my $lms = $self->{-lms_ro} //= $self->{lei}->lms; # may be 0 or undef
-        my $oidbin = $lms ? $lms->name_oidbin($folder, $bn) : undef;
-        my @docids = defined($oidbin) ? $lse->over->oidbin_exists($oidbin) : ();
+        my @oidbin = $lms ? $lms->name_oidbin($folder, $bn) : ();
+        @oidbin > 1 and $self->{lei}->err("W: $folder/*/$$bn not unique:\n",
+                                map { "\t".unpack('H*', $_)."\n" } @oidbin);
+        my %seen;
+        my @docids = sort { $a <=> $b } grep { !$seen{$_}++ }
+                        map { $lse->over->oidbin_exists($_) } @oidbin;
         my $vmd = $self->{-import_kw} ? { kw => $kw } : undef;
         if (scalar @docids) {
                 $lse->kw_changed(undef, $kw, \@docids) or return;
diff --git a/lib/PublicInbox/LeiImportKw.pm b/lib/PublicInbox/LeiImportKw.pm
index 379101c2..21c93515 100644
--- a/lib/PublicInbox/LeiImportKw.pm
+++ b/lib/PublicInbox/LeiImportKw.pm
@@ -34,11 +34,15 @@ sub ipc_atfork_child {
 
 sub ck_update_kw { # via wq_io_do
         my ($self, $url, $uid, $kw) = @_;
-        my $oidbin = $self->{-lms_ro}->imap_oidbin($url, $uid) // return;
-        my @docids = $self->{over}->oidbin_exists($oidbin) or return;
+        my @oidbin = $self->{-lms_ro}->num_oidbin($url, $uid);
+        my $uid_url = "$url/;UID=$uid";
+        @oidbin > 1 and $self->{lei}->err("W: $uid_url not unique:\n",
+                                map { "\t".unpack('H*', $_)."\n" } @oidbin);
+        my %seen;
+        my @docids = sort { $a <=> $b } grep { !$seen{$_}++ }
+                map { $self->{over}->oidbin_exists($_) } @oidbin;
         $self->{lse}->kw_changed(undef, $kw, \@docids) or return;
-        $self->{verbose} and
-                $self->{lei}->qerr('# '.unpack('H*', $oidbin)." => @$kw\n");
+        $self->{verbose} and $self->{lei}->qerr("# $uid_url => @$kw\n");
         $self->{sto}->wq_do('set_eml_vmd', undef, { kw => $kw }, \@docids);
 }
 
diff --git a/lib/PublicInbox/LeiInspect.pm b/lib/PublicInbox/LeiInspect.pm
index ab2c98d9..722ba5b2 100644
--- a/lib/PublicInbox/LeiInspect.pm
+++ b/lib/PublicInbox/LeiInspect.pm
@@ -32,11 +32,9 @@ sub inspect_imap_uid ($$) {
         my ($lei, $uid_uri) = @_;
         my $ent = {};
         my $lms = $lei->lms or return $ent;
-        my $oidhex = $lms->imap_oid($lei, $uid_uri);
-        if (ref(my $err = $oidhex)) { # arg2folder error
-                $lei->qerr(@{$err->{qerr}}) if $err->{qerr};
-        }
-        $ent->{$$uid_uri} = $oidhex;
+        my @oidhex = $lms->imap_oidhex($lei, $uid_uri);
+        $ent->{$$uid_uri} = @oidhex == 1 ? $oidhex[0] :
+                        ((@oidhex == 0) ? undef : \@oidhex);
         $ent;
 }
 
@@ -54,8 +52,10 @@ sub inspect_nntp_range {
         }
         $end //= $beg;
         for my $art ($beg..$end) {
-                my $oidbin = $lms->imap_oidbin($folders->[0], $art);
-                $ent->{$art} = $oidbin ? unpack('H*', $oidbin) : undef;
+                my @oidhex = map { unpack('H*', $_) }
+                        $lms->num_oidbin($folders->[0], $art);
+                $ent->{$art} = @oidhex == 1 ? $oidhex[0] :
+                                ((@oidhex == 0) ? undef : \@oidhex);
         }
         $ret;
 }
diff --git a/lib/PublicInbox/LeiLcat.pm b/lib/PublicInbox/LeiLcat.pm
index 1e54c3bf..8f8e83bc 100644
--- a/lib/PublicInbox/LeiLcat.pm
+++ b/lib/PublicInbox/LeiLcat.pm
@@ -32,11 +32,8 @@ sub lcat_imap_uri ($$) {
         my $lms = $lei->lms or return;
         # cf. LeiXsearch->lcat_dump
         if (defined $uri->uid) {
-                my $oidhex = $lms->imap_oid($lei, $uri);
-                if (ref(my $err = $oidhex)) { # art2folder error
-                        $lei->qerr(@{$err->{qerr}}) if $err->{qerr};
-                }
-                push @{$lei->{lcat_blob}}, $oidhex;
+                my @oidhex = $lms->imap_oidhex($lei, $uri);
+                push @{$lei->{lcat_blob}}, @oidhex;
         } elsif (defined(my $fid = $lms->fid_for($$uri))) {
                 push @{$lei->{lcat_fid}}, $fid;
         } else {
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index d9b9e117..3e725d30 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -66,6 +66,7 @@ CREATE TABLE IF NOT EXISTS blob2num (
         oidbin VARBINARY NOT NULL,
         fid INTEGER NOT NULL, /* folder ID */
         uid INTEGER NOT NULL, /* NNTP article number, IMAP UID, MH number */
+        /* not UNIQUE(fid, uid), since we may have broken servers */
         UNIQUE (oidbin, fid, uid)
 )
 
@@ -78,6 +79,7 @@ CREATE TABLE IF NOT EXISTS blob2name (
         oidbin VARBINARY NOT NULL,
         fid INTEGER NOT NULL, /* folder ID */
         name VARBINARY NOT NULL, /* Maildir basename, JMAP blobId */
+        /* not UNIQUE(fid, name), since we may have broken software */
         UNIQUE (oidbin, fid, name)
 )
 
@@ -522,14 +524,14 @@ EOM
         }
 }
 
-sub imap_oidbin ($$$) {
-        my ($self, $url, $uid) = @_; # $url MUST have UIDVALIDITY
-        my $fid = $self->{fmap}->{$url} //= fid_for($self, $url) // return;
+sub num_oidbin ($$$) {
+        my ($self, $url, $uid) = @_; # $url MUST have UIDVALIDITY if IMAP
+        my $fid = $self->{fmap}->{$url} //= fid_for($self, $url) // return ();
         my $sth = $self->{dbh}->prepare_cached(<<EOM, undef, 1);
-SELECT oidbin FROM blob2num WHERE fid = ? AND uid = ?
+SELECT oidbin FROM blob2num WHERE fid = ? AND uid = ? ORDER BY _rowid_
 EOM
         $sth->execute($fid, $uid);
-        $sth->fetchrow_array;
+        map { $_->[0] } @{$sth->fetchall_arrayref};
 }
 
 sub name_oidbin ($$$) {
@@ -539,10 +541,10 @@ sub name_oidbin ($$$) {
 SELECT oidbin FROM blob2name WHERE fid = ? AND name = ?
 EOM
         $sth->execute($fid, $nm);
-        $sth->fetchrow_array;
+        map { $_->[0] } @{$sth->fetchall_arrayref};
 }
 
-sub imap_oid {
+sub imap_oidhex {
         my ($self, $lei, $uid_uri) = @_;
         my $mailbox_uri = $uid_uri->clone;
         $mailbox_uri->uid(undef);
@@ -550,12 +552,10 @@ sub imap_oid {
         if (my $err = $self->arg2folder($lei, $folders)) {
                 if ($err->{fail}) {
                         $lei->qerr("# no sync information for $mailbox_uri");
-                        return;
                 }
                 $lei->qerr(@{$err->{qerr}}) if $err->{qerr};
         }
-        my $oidbin = imap_oidbin($self, $folders->[0], $uid_uri->uid);
-        $oidbin ? unpack('H*', $oidbin) : undef;
+        map { unpack('H*',$_) } num_oidbin($self, $folders->[0], $uid_uri->uid)
 }
 
 1;
diff --git a/lib/PublicInbox/LeiRefreshMailSync.pm b/lib/PublicInbox/LeiRefreshMailSync.pm
index 2f105005..92673492 100644
--- a/lib/PublicInbox/LeiRefreshMailSync.pm
+++ b/lib/PublicInbox/LeiRefreshMailSync.pm
@@ -36,7 +36,7 @@ sub pmdir_cb { # called via LeiPmdir->each_mdir_fn
         my ($folder, $bn) = ($f =~ m!\A(.+?)/(?:new|cur)/([^/]+)\z!) or
                 die "BUG: $f was not from a Maildir?";
         substr($folder, 0, 0) = 'maildir:'; # add prefix
-        return if defined($self->{lms}->name_oidbin($folder, $bn));
+        return if scalar($self->{lms}->name_oidbin($folder, $bn));
         my $eml = eml_from_path($f) // return;
         my $oidbin = $self->{lei}->git_oid($eml)->digest;
         $self->{lms}->set_src($oidbin, $folder, \$bn);